Merge branch 'rightlib' into mergelibs-240603-0842

6 files changed, 220 insertions, 11 deletions

diff --git a/library/cpp/yt/logging/logger-inl.h b/library/cpp/yt/logging/logger-inl.h
index dcf40d9c119..b20526532d5 100644
--- a/library/cpp/yt/logging/logger-inl.h
+++ b/library/cpp/yt/logging/logger-inl.h
@@ -277,6 +277,7 @@ inline void LogEventImpl(
     const TLogger& logger,
     ELogLevel level,
     ::TSourceLocation sourceLocation,
+    TLoggingAnchor* anchor,
     TSharedRef message)
 {
     auto event = CreateLogEvent(loggingContext, logger, level);
@@ -285,6 +286,7 @@ inline void LogEventImpl(
     event.Family = ELogFamily::PlainText;
     event.SourceFile = sourceLocation.File;
     event.SourceLine = sourceLocation.Line;
+    event.Anchor = anchor;
     logger.Write(std::move(event));
     if (Y_UNLIKELY(event.Level >= ELogLevel::Alert)) {
         OnCriticalLogEvent(logger, event);
diff --git a/library/cpp/yt/logging/logger.h b/library/cpp/yt/logging/logger.h
index 693ea2b9cd2..abeb9a1c9f0 100644
--- a/library/cpp/yt/logging/logger.h
+++ b/library/cpp/yt/logging/logger.h
@@ -22,6 +22,10 @@
 
 #include <atomic>
 
+#if !defined(NDEBUG) && !defined(YT_DISABLE_FORMAT_STATIC_ANALYSIS)
+    #include "static_analysis.h"
+#endif
+
 namespace NYT::NLogging {
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -56,7 +60,7 @@ struct TLoggingAnchor
 
     struct TCounter
     {
-        std::atomic<i64> Current = 0;
+        i64 Current = 0;
         i64 Previous = 0;
     };
 
@@ -100,6 +104,8 @@ struct TLogEvent
 
     TStringBuf SourceFile;
     int SourceLine = -1;
+
+    TLoggingAnchor* Anchor = nullptr;
 };
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -294,6 +300,12 @@ void LogStructuredEvent(
 #define YT_LOG_EVENT(logger, level, ...) \
     YT_LOG_EVENT_WITH_ANCHOR(logger, level, nullptr, __VA_ARGS__)
 
+#if !defined(NDEBUG) && !defined(YT_DISABLE_FORMAT_STATIC_ANALYSIS)
+    #define YT_LOG_CHECK_FORMAT(...) STATIC_ANALYSIS_CHECK_LOG_FORMAT(__VA_ARGS__)
+#else
+    #define YT_LOG_CHECK_FORMAT(...)
+#endif
+
 #define YT_LOG_EVENT_WITH_ANCHOR(logger, level, anchor, ...) \
     do { \
         const auto& logger__ = (logger)(); \
@@ -317,6 +329,7 @@ void LogStructuredEvent(
         } \
         \
         auto loggingContext__ = ::NYT::NLogging::GetLoggingContext(); \
+        YT_LOG_CHECK_FORMAT(__VA_ARGS__); \
         auto message__ = ::NYT::NLogging::NDetail::BuildLogMessage(loggingContext__, logger__, __VA_ARGS__); \
         \
         if (!anchorUpToDate__) { \
@@ -328,21 +341,12 @@ void LogStructuredEvent(
             break; \
         } \
         \
-        static thread_local i64 localByteCounter__; \
-        static thread_local ui8 localMessageCounter__; \
-        \
-        localByteCounter__ += message__.MessageRef.Size(); \
-        if (Y_UNLIKELY(++localMessageCounter__ == 0)) { \
-            anchor__->MessageCounter.Current += 256; \
-            anchor__->ByteCounter.Current += localByteCounter__; \
-            localByteCounter__ = 0; \
-        } \
-        \
         ::NYT::NLogging::NDetail::LogEventImpl( \
             loggingContext__, \
             logger__, \
             level__, \
             location__, \
+            anchor__, \
             std::move(message__.MessageRef)); \
     } while (false)
 
diff --git a/library/cpp/yt/logging/static_analysis-inl.h b/library/cpp/yt/logging/static_analysis-inl.h
new file mode 100644
index 00000000000..d4ec5343bc1
--- /dev/null
+++ b/library/cpp/yt/logging/static_analysis-inl.h
@@ -0,0 +1,139 @@
+#ifndef STATIC_ANALYSIS_INL_H_
+#error "Direct inclusion of this file is not allowed, include static_analysis.h"
+// For the sake of sane code completion.
+#include "static_analysis.h"
+#endif
+
+#include <library/cpp/yt/misc/preprocessor.h>
+
+#include <library/cpp/yt/string/format_analyser.h>
+
+#include <string_view>
+#include <variant> // monostate
+
+namespace NYT::NLogging::NDetail {
+
+////////////////////////////////////////////////////////////////////////////////
+
+// Tag for dispatching proper TFormatArg specialization.
+template <class T>
+struct TLoggerFormatArg
+{ };
+
+////////////////////////////////////////////////////////////////////////////////
+
+// Stateless constexpr way of capturing arg types
+// without invoking any ctors. With the help of macros
+// can turn non-constexpr argument pack of arguments
+// into constexpr pack of types.
+template <class... TArgs>
+struct TLoggerFormatArgs
+{ };
+
+// Used for macro conversion. Purposefully undefined.
+template <class... TArgs>
+TLoggerFormatArgs<TArgs...> AsFormatArgs(TArgs&&...);
+
+////////////////////////////////////////////////////////////////////////////////
+
+template <bool First, bool Second>
+struct TAnalyserDispatcher
+{
+    template <class... TArgs>
+    static consteval void Do(std::string_view, std::string_view, TLoggerFormatArgs<TArgs...>)
+    {
+        // Give up :(
+        // We can't crash here, because, for example, YT_LOG_ERROR(error) exists
+        // and we can't really check if error is actually TError or something else here.
+        // and probably shouldn't bother trying.
+    }
+};
+
+template <bool Second>
+struct TAnalyserDispatcher<true, Second>
+{
+    template <class TFirst, class... TArgs>
+    static consteval void Do(std::string_view str, std::string_view, TLoggerFormatArgs<TFirst, TArgs...>)
+    {
+        // Remove outer \"'s generated by PP_STRINGIZE.
+        auto stripped = std::string_view(std::begin(str) + 1, std::size(str) - 2);
+        ::NYT::NDetail::TFormatAnalyser::ValidateFormat<TLoggerFormatArg<TArgs>...>(stripped);
+    }
+};
+
+template <>
+struct TAnalyserDispatcher<false, true>
+{
+    template <class TFirst, class TSecond, class... TArgs>
+    static consteval void Do(std::string_view, std::string_view str, TLoggerFormatArgs<TFirst, TSecond, TArgs...>)
+    {
+        // Remove outer \"'s generated by PP_STRINGIZE.
+        auto stripped = std::string_view(std::begin(str) + 1, std::size(str) - 2);
+        ::NYT::NDetail::TFormatAnalyser::ValidateFormat<TLoggerFormatArg<TArgs>...>(stripped);
+    }
+};
+
+////////////////////////////////////////////////////////////////////////////////
+
+// This value is never read since homogenization works for unevaluated expressions.
+inline constexpr auto InvalidToken = std::monostate{};
+
+////////////////////////////////////////////////////////////////////////////////
+
+#define PP_VA_PICK_1_IMPL(N, ...) N
+#define PP_VA_PICK_2_IMPL(_1, N, ...) N
+
+////////////////////////////////////////////////////////////////////////////////
+
+//! Parameter pack parsing.
+
+#define STATIC_ANALYSIS_CAPTURE_TYPES(...) \
+    decltype(::NYT::NLogging::NDetail::AsFormatArgs(__VA_ARGS__)){}
+
+#define STATIC_ANALYSIS_FIRST_TOKEN(...) \
+    PP_STRINGIZE( \
+        PP_VA_PICK_1_IMPL(__VA_ARGS__ __VA_OPT__(,) ::NYT::NLogging::NDetail::InvalidToken))
+
+#define STATIC_ANALYSIS_SECOND_TOKEN(...) \
+    PP_STRINGIZE(\
+        PP_VA_PICK_2_IMPL( \
+            __VA_ARGS__ __VA_OPT__(,) \
+            ::NYT::NLogging::NDetail::InvalidToken, \
+            ::NYT::NLogging::NDetail::InvalidToken))
+
+#define STATIC_ANALYSIS_FIRST_TOKEN_COND(...) \
+    STATIC_ANALYSIS_FIRST_TOKEN(__VA_ARGS__)[0] == '\"'
+
+#define STATIC_ANALYSIS_SECOND_TOKEN_COND(...) \
+    STATIC_ANALYSIS_SECOND_TOKEN(__VA_ARGS__)[0] == '\"'
+
+#undef STATIC_ANALYSIS_CHECK_LOG_FORMAT
+#define STATIC_ANALYSIS_CHECK_LOG_FORMAT(...) \
+    ::NYT \
+    ::NLogging \
+    ::NDetail \
+    ::TAnalyserDispatcher< \
+        STATIC_ANALYSIS_FIRST_TOKEN_COND(__VA_ARGS__), \
+        STATIC_ANALYSIS_SECOND_TOKEN_COND(__VA_ARGS__) \
+    >::Do( \
+        STATIC_ANALYSIS_FIRST_TOKEN(__VA_ARGS__), \
+        STATIC_ANALYSIS_SECOND_TOKEN(__VA_ARGS__), \
+        STATIC_ANALYSIS_CAPTURE_TYPES(__VA_ARGS__))
+
+////////////////////////////////////////////////////////////////////////////////
+
+} // namespace NYT::NLogging::NDetail
+
+template <class T>
+struct NYT::TFormatArg<NYT::NLogging::NDetail::TLoggerFormatArg<T>>
+    : public NYT::TFormatArgBase
+{
+    // We mix in '\"' and ' ' which is an artifact of logging stringize.
+    // We want to support YT_LOG_XXX("Value: %" PRIu64, 42)
+    // for plantform independent prints of numbers.
+    // String below may be converted to a token:
+    // "\"Value: %\" \"u\""
+    // Thus adding a \" \" sequence.
+    static constexpr auto FlagSpecifiers
+        = TFormatArgBase::ExtendFlags</*Hot*/ false, 2, std::array{'\"', ' '}, /*TFrom*/ NYT::TFormatArg<T>>();
+};
diff --git a/library/cpp/yt/logging/static_analysis.h b/library/cpp/yt/logging/static_analysis.h
new file mode 100644
index 00000000000..a335d8c6cc1
--- /dev/null
+++ b/library/cpp/yt/logging/static_analysis.h
@@ -0,0 +1,22 @@
+#pragma once
+
+namespace NYT::NLogging {
+
+////////////////////////////////////////////////////////////////////////////////
+
+// Performs a compile-time check of log arguments validity.
+// Valid argument lists are:
+// 1. (format, args...)
+// 2. (error, format, args...)
+// If format is not a string literal or argument list
+// is not valid, no check is made -- macro turns to
+// a no-op.
+#define STATIC_ANALYSIS_CHECK_LOG_FORMAT(...)
+
+////////////////////////////////////////////////////////////////////////////////
+
+} // namespace NYT::NLogging
+
+#define STATIC_ANALYSIS_INL_H_
+#include "static_analysis-inl.h"
+#undef STATIC_ANALYSIS_INL_H_
diff --git a/library/cpp/yt/logging/unittests/static_analysis_ut.cpp b/library/cpp/yt/logging/unittests/static_analysis_ut.cpp
new file mode 100644
index 00000000000..1c705dc9674
--- /dev/null
+++ b/library/cpp/yt/logging/unittests/static_analysis_ut.cpp
@@ -0,0 +1,41 @@
+#include <library/cpp/testing/gtest/gtest.h>
+
+#include <library/cpp/yt/logging/logger.h>
+
+namespace NYT::NLogging {
+namespace {
+
+////////////////////////////////////////////////////////////////////////////////
+
+TEST(TStaticAnalysis, ValidFormats)
+{
+    // Mock for actual error -- we only care that
+    // it is some runtime object.
+    [[maybe_unused]] struct TError
+    { } error;
+
+    YT_LOG_CHECK_FORMAT("Hello");
+    YT_LOG_CHECK_FORMAT("Hello %v", "World!");
+    YT_LOG_CHECK_FORMAT("Hello %qv", "World!");
+    YT_LOG_CHECK_FORMAT(error);
+    YT_LOG_CHECK_FORMAT(error, "Hello");
+    YT_LOG_CHECK_FORMAT(error, "Hello %Qhs", "World!");
+    YT_LOG_CHECK_FORMAT("Hello %%");
+    YT_LOG_CHECK_FORMAT("Hello %" PRIu64, 42);
+}
+
+// Uncomment this test to see that we don't have false negatives!
+// TEST(TStaticAnalysis, InvalidFormats)
+// {
+//     YT_LOG_CHECK_FORMAT("Hello", 1);
+//     YT_LOG_CHECK_FORMAT("Hello %");
+//     YT_LOG_CHECK_FORMAT("Hello %false");
+//     YT_LOG_CHECK_FORMAT("Hello ", "World");
+//     YT_LOG_CHECK_FORMAT("Hello ", "(World: %v)", 42);
+//     YT_LOG_CHECK_FORMAT("Hello %lbov", 42); // There is no 'b' flag.
+// }
+
+////////////////////////////////////////////////////////////////////////////////
+
+} // namespace
+} // namespace NYT::NLogging
diff --git a/library/cpp/yt/logging/unittests/ya.make b/library/cpp/yt/logging/unittests/ya.make
index 42268d3db2f..021b0d09d6a 100644
--- a/library/cpp/yt/logging/unittests/ya.make
+++ b/library/cpp/yt/logging/unittests/ya.make
@@ -8,6 +8,7 @@ ENDIF()
 
 SRCS(
     logger_ut.cpp
+    static_analysis_ut.cpp
 )
 
 PEERDIR(