YT-22512: Static analysis for TError method, ctors and related macros

Static analysis enabled for TError creation and related macros

TRuntimeFormat can be used to disable this feature, but requires copying the viewed object.
See NYT::TError::DisableFormat overloads to optimize constructions which want to move the given string

Note to future readers: TError is not "perfect-forwarding" unfriendly class. This means that the code
```
template <class... TArgs>
TError MakeError(TArgs&&... args) {
  return TError(std::forward<TArgs>(args)...);
}
```
will not compile and needs to be properly adjusted (see. TError::Wrap for implementation example)

This implies that emplace construction in containers will not work either. Use move construction instead, as it is simply a pointer swap and therefore free

Annotations: [nodiff:caesar]
cff12f05849402d09a4487bad26ffcd968215dc7

library/cpp/yt/logging/logger-inl.h

@@ -181,12 +181,6 @@ struct TLogMessage
     TStringBuf Anchor;
 };
 
-template <class T>
-concept CLiteralLogFormat =
-    requires (T& t) {
-        [] (const char*) { } (t);
-    };
-
 template <class... TArgs>
 TLogMessage BuildLogMessage(
     const TLoggingContext& loggingContext,
@@ -200,7 +194,7 @@ TLogMessage BuildLogMessage(
 }
 
 template <CFormattable T>
-    requires (!CLiteralLogFormat<std::remove_cvref_t<T>>)
+    requires (!CStringLiteral<std::remove_cvref_t<T>>)
 TLogMessage BuildLogMessage(
     const TLoggingContext& loggingContext,
     const TLogger& logger,

library/cpp/yt/string/format_analyser-inl.h

@@ -1,112 +0,0 @@
-#ifndef FORMAT_ANALYSER_INL_H_
-#error "Direct inclusion of this file is not allowed, include format_analyser.h"
-// For the sake of sane code completion.
-#include "format_analyser.h"
-#endif
-
-#include "format_arg.h"
-
-namespace NYT::NDetail {
-
-////////////////////////////////////////////////////////////////////////////////
-
-consteval bool Contains(std::string_view sv, char symbol)
-{
-    return sv.find(symbol) != std::string_view::npos;
-}
-
-template <class... TArgs>
-consteval void TFormatAnalyser::ValidateFormat(std::string_view format)
-{
-    std::array<std::string_view, sizeof...(TArgs)> markers = {};
-    std::array<TSpecifiers, sizeof...(TArgs)> specifiers{GetSpecifiers<TArgs>()...};
-
-    int markerCount = 0;
-    int currentMarkerStart = -1;
-
-    for (int index = 0; index < std::ssize(format); ++index) {
-        auto symbol = format[index];
-
-        // Parse verbatim text.
-        if (currentMarkerStart == -1) {
-            if (symbol == IntroductorySymbol) {
-                // Marker maybe begins.
-                currentMarkerStart = index;
-            }
-            continue;
-        }
-
-        // NB: We check for %% first since
-        // in order to verify if symbol is a specifier
-        // we need markerCount to be within range of our
-        // specifier array.
-        if (symbol == IntroductorySymbol) {
-            if (currentMarkerStart + 1 != index) {
-                // '%a% detected'
-                CrashCompilerWrongTermination("You may not terminate flag sequence other than %% with \'%\' symbol");
-                return;
-            }
-            // '%%' detected --- skip
-            currentMarkerStart = -1;
-            continue;
-        }
-
-        // 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;
-        }
-
-        if (Contains(specifiers[markerCount].Conversion, symbol)) {
-            // Marker has finished.
-
-            markers[markerCount]
-                = std::string_view(format.begin() + currentMarkerStart, index - currentMarkerStart + 1);
-            currentMarkerStart = -1;
-            ++markerCount;
-
-            continue;
-        }
-
-        if (!Contains(specifiers[markerCount].Flags, symbol)) {
-            CrashCompilerWrongFlagSpecifier("Symbol is not a valid flag specifier; See FlagSpecifiers");
-        }
-    }
-
-    if (currentMarkerStart != -1) {
-        // Runaway marker.
-        CrashCompilerMissingTermination("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;
-    }
-
-    // TODO(arkady-e1ppa): Consider per-type verification
-    // of markers.
-}
-
-template <class TArg>
-consteval auto TFormatAnalyser::GetSpecifiers()
-{
-    if constexpr (!CFormattable<TArg>) {
-        CrashCompilerNotFormattable<TArg>("Your specialization of TFormatArg is broken");
-    }
-
-    return TSpecifiers{
-        .Conversion = std::string_view{
-            std::data(TFormatArg<TArg>::ConversionSpecifiers),
-            std::size(TFormatArg<TArg>::ConversionSpecifiers)},
-        .Flags = std::string_view{
-            std::data(TFormatArg<TArg>::FlagSpecifiers),
-            std::size(TFormatArg<TArg>::FlagSpecifiers)},
-    };
-}
-
-////////////////////////////////////////////////////////////////////////////////
-
-} // namespace NYT::NDetail

library/cpp/yt/string/format_analyser.h

@@ -1,5 +1,7 @@
 #pragma once
 
+#include "format_arg.h"
+
 #include <util/generic/strbuf.h>
 
 #include <array>
@@ -12,36 +14,143 @@ namespace NYT::NDetail {
 struct TFormatAnalyser
 {
 public:
+    // TODO(arkady-e1ppa): Until clang-19 consteval functions
+    // defined out of line produce symbols in rare cases
+    // causing linker to crash.
     template <class... TArgs>
-    static consteval void ValidateFormat(std::string_view fmt);
+    static consteval void ValidateFormat(std::string_view fmt)
+    {
+        DoValidateFormat<TArgs...>(fmt);
+    }
 
-public:
+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 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;
+    }
 
     struct TSpecifiers
     {
         std::string_view Conversion;
         std::string_view Flags;
     };
+
     template <class TArg>
-    static consteval auto GetSpecifiers();
+    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),
+                std::size(TFormatArg<TArg>::ConversionSpecifiers)},
+            .Flags = std::string_view{
+                std::data(TFormatArg<TArg>::FlagSpecifiers),
+                std::size(TFormatArg<TArg>::FlagSpecifiers)},
+        };
+    }
 
     static constexpr char IntroductorySymbol = '%';
+
+    template <class... TArgs>
+    static consteval void DoValidateFormat(std::string_view format)
+    {
+        std::array<std::string_view, sizeof...(TArgs)> markers = {};
+        std::array<TSpecifiers, sizeof...(TArgs)> specifiers{GetSpecifiers<TArgs>()...};
+
+        int markerCount = 0;
+        int currentMarkerStart = -1;
+
+        for (int index = 0; index < std::ssize(format); ++index) {
+            auto symbol = format[index];
+
+            // Parse verbatim text.
+            if (currentMarkerStart == -1) {
+                if (symbol == IntroductorySymbol) {
+                    // Marker maybe begins.
+                    currentMarkerStart = index;
+                }
+                continue;
+            }
+
+            // NB: We check for %% first since
+            // in order to verify if symbol is a specifier
+            // we need markerCount to be within range of our
+            // specifier array.
+            if (symbol == IntroductorySymbol) {
+                if (currentMarkerStart + 1 != index) {
+                    // '%a% detected'
+                    CrashCompilerWrongTermination("You may not terminate flag sequence other than %% with \'%\' symbol");
+                    return;
+                }
+                // '%%' detected --- skip
+                currentMarkerStart = -1;
+                continue;
+            }
+
+            // 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;
+            }
+
+            if (Contains(specifiers[markerCount].Conversion, symbol)) {
+                // Marker has finished.
+
+                markers[markerCount]
+                    = std::string_view(format.begin() + currentMarkerStart, index - currentMarkerStart + 1);
+                currentMarkerStart = -1;
+                ++markerCount;
+
+                continue;
+            }
+
+            if (!Contains(specifiers[markerCount].Flags, symbol)) {
+                CrashCompilerWrongFlagSpecifier("Symbol is not a valid flag specifier; See FlagSpecifiers");
+            }
+        }
+
+        if (currentMarkerStart != -1) {
+            // Runaway marker.
+            CrashCompilerMissingTermination("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;
+        }
+
+        // TODO(arkady-e1ppa): Consider per-type verification
+        // of markers.
+    }
 };
 
 ////////////////////////////////////////////////////////////////////////////////
 
 } // namespace NYT::NDetail
-
-#define FORMAT_ANALYSER_INL_H_
-#include "format_analyser-inl.h"
-#undef FORMAT_ANALYSER_INL_H_

library/cpp/yt/string/format_string.h

@@ -56,6 +56,13 @@ using TFormatString = TBasicFormatString<std::type_identity_t<TArgs>...>;
 
 ////////////////////////////////////////////////////////////////////////////////
 
+template <class T>
+concept CStringLiteral = requires (T& t) {
+    [] (const char*) { } (t);
+};
+
+////////////////////////////////////////////////////////////////////////////////
+
 } // namespace NYT
 
 #define FORMAT_STRING_INL_H_

yt/yt/client/api/persistent_queue.cpp

@@ -679,10 +679,7 @@ private:
                 batch.EndRowIndex - 1,
                 transaction->GetId());
         } catch (const std::exception& ex) {
-            THROW_ERROR_EXCEPTION("Error confirming persistent queue rows",
-                batch.TabletIndex,
-                batch.BeginRowIndex,
-                batch.EndRowIndex - 1)
+            THROW_ERROR_EXCEPTION("Error confirming persistent queue rows")
                 << TErrorAttribute("poller_id", PollerId_)
                 << TErrorAttribute("transaction_id", transaction->GetId())
                 << TErrorAttribute("tablet_index", batch.TabletIndex)

yt/yt/client/table_client/merge_table_schemas.cpp

@@ -55,7 +55,9 @@ TTableSchemaPtr MergeTableSchemas(
             }
             if (firstSchemaColumn->SortOrder() != secondSchemaColumn.SortOrder()) {
                 THROW_ERROR_EXCEPTION("Mismatching sort orders in column %Qv: %Qv and %Qv",
-                    firstSchemaColumn->Name());
+                    firstSchemaColumn->Name(),
+                    firstSchemaColumn->SortOrder(),
+                    secondSchemaColumn.SortOrder());
             }
 
             try {

yt/yt/client/table_client/validate_logical_type.cpp

@@ -235,8 +235,7 @@ private:
         if (Cursor_.GetCurrent().GetType() == EYsonItemType::EndList) {
             THROW_ERROR_EXCEPTION(EErrorCode::SchemaViolation,
                 "Cannot parse %Qv; empty yson",
-                GetDescription(fieldId),
-                Cursor_.GetCurrent().GetType());
+                GetDescription(fieldId));
         }
         ValidateLogicalType(type.GetElement(), fieldId.OptionalElement());
         if (Cursor_.GetCurrent().GetType() != EYsonItemType::EndList) {

yt/yt/core/bus/tcp/server.cpp

@@ -285,7 +285,7 @@ protected:
                 if (attempt == Config_->BindRetryCount) {
                     CloseServerSocket();
 
-                    THROW_ERROR_EXCEPTION(NRpc::EErrorCode::TransportError, errorMessage)
+                    THROW_ERROR_EXCEPTION(NRpc::EErrorCode::TransportError, TRuntimeFormat(errorMessage))
                         << ex;
                 } else {
                     YT_LOG_WARNING(ex, "Error binding socket, starting %v retry", attempt + 1);

yt/yt/core/crypto/crypto.cpp

@@ -291,7 +291,7 @@ TString GenerateCryptoStrongRandomString(int length)
         auto* data = reinterpret_cast<char*>(bytes.data());
         return TString{data, static_cast<size_t>(length)};
     } else {
-        THROW_ERROR_EXCEPTION("Failed to generate %v random bytes")
+        THROW_ERROR_EXCEPTION("Failed to generate %v random bytes", length)
             << TErrorAttribute("openssl_error_code", ERR_get_error());
     }
 }

yt/yt/core/dns/ares_dns_resolver.cpp

@@ -737,7 +737,7 @@ private:
             request->HostName)
             << TErrorAttribute("enable_ipv4", request->Options.EnableIPv4)
             << TErrorAttribute("enable_ipv6", request->Options.EnableIPv6)
-            << TError(ares_strerror(status));
+            << TError(TRuntimeFormat(ares_strerror(status)));
     }
 
     void FailRequest(