Skip to content

Commit

Permalink
Fancier error messages upon static analysis check failure
Browse files Browse the repository at this point in the history
commit_hash:f939fba86939275047d2eca49b11bec3d0ea3ce7
  • Loading branch information
E1pp committed Oct 16, 2024
1 parent 2b1d621 commit 529a340
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 43 deletions.
18 changes: 9 additions & 9 deletions library/cpp/yt/logging/unittests/static_analysis_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ TEST(TStaticAnalysisTest, ValidFormats)
}

// Uncomment this test to see that we don't have false negatives!
// TEST(TStaticAnalysisTest, InvalidFormats)
// {
// YT_LOG_INFO("Hello", 1);
// YT_LOG_INFO("Hello %");
// YT_LOG_INFO("Hello %false");
// YT_LOG_INFO("Hello ", "World");
// YT_LOG_INFO("Hello ", "(World: %v)", 42);
// YT_LOG_INFO("Hello %lbov", 42); // There is no 'b' flag.
// }
TEST(TStaticAnalysisTest, InvalidFormats)
{
// YT_LOG_INFO("Hello", 1);
// YT_LOG_INFO("Hello %");
// YT_LOG_INFO("Hello %false");
// YT_LOG_INFO("Hello ", "World");
// YT_LOG_INFO("Hello ", "(World: %v)", 42);
// YT_LOG_INFO("Hello %lbov", 42); // There is no 'b' flag.
}

////////////////////////////////////////////////////////////////////////////////

Expand Down
40 changes: 6 additions & 34 deletions library/cpp/yt/string/format_analyser.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,27 +24,6 @@ struct TFormatAnalyser
}

private:
// Non-constexpr function call will terminate compilation.
// Purposefully undefined and non-constexpr/consteval
template <class T>
static void CrashCompilerNotFormattable(std::string_view /*msg*/)
{ /*Suppress "internal linkage but undefined" warning*/ }
static void CrashCompilerNotEnoughArguments(std::string_view /*msg*/)
{ }

static void CrashCompilerTooManyArguments(std::string_view /*msg*/)
{ }

static void CrashCompilerWrongTermination(std::string_view /*msg*/)
{ }

static void CrashCompilerMissingTermination(std::string_view /*msg*/)
{ }

static void CrashCompilerWrongFlagSpecifier(std::string_view /*msg*/)
{ }


static consteval bool Contains(std::string_view sv, char symbol)
{
return sv.find(symbol) != std::string_view::npos;
Expand All @@ -59,10 +38,6 @@ struct TFormatAnalyser
template <class TArg>
static consteval auto GetSpecifiers()
{
if constexpr (!CFormattable<TArg>) {
CrashCompilerNotFormattable<TArg>("Your specialization of TFormatArg is broken");
}

return TSpecifiers{
.Conversion = std::string_view{
std::data(TFormatArg<TArg>::ConversionSpecifiers),
Expand Down Expand Up @@ -103,8 +78,7 @@ struct TFormatAnalyser
if (symbol == IntroductorySymbol) {
if (currentMarkerStart + 1 != index) {
// '%a% detected'
CrashCompilerWrongTermination("You may not terminate flag sequence other than %% with \'%\' symbol");
return;
throw "You may not terminate flag sequence other than %% with \'%\' symbol";
}
// '%%' detected --- skip
currentMarkerStart = -1;
Expand All @@ -113,9 +87,8 @@ struct TFormatAnalyser

// We are inside of marker.
if (markerCount == std::ssize(markers)) {
// To many markers
CrashCompilerNotEnoughArguments("Number of arguments supplied to format is smaller than the number of flag sequences");
return;
// Too many markers
throw "Number of arguments supplied to format is smaller than the number of flag sequences";
}

if (Contains(specifiers[markerCount].Conversion, symbol)) {
Expand All @@ -130,20 +103,19 @@ struct TFormatAnalyser
}

if (!Contains(specifiers[markerCount].Flags, symbol)) {
CrashCompilerWrongFlagSpecifier("Symbol is not a valid flag specifier; See FlagSpecifiers");
throw "Symbol is not a valid flag specifier; See FlagSpecifiers";
}
}

if (currentMarkerStart != -1) {
// Runaway marker.
CrashCompilerMissingTermination("Unterminated flag sequence detected; Use \'%%\' to type plain %");
throw "Unterminated flag sequence detected; Use \'%%\' to type plain %";
return;
}

if (markerCount < std::ssize(markers)) {
// Missing markers.
CrashCompilerTooManyArguments("Number of arguments supplied to format is greater than the number of flag sequences");
return;
throw "Number of arguments supplied to format is greater than the number of flag sequences";
}

// TODO(arkady-e1ppa): Consider per-type verification
Expand Down

0 comments on commit 529a340

Please sign in to comment.