-
Notifications
You must be signed in to change notification settings - Fork 15.2k
Introduce LDBG_OS() macro as a variant of LDBG() #157194
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@llvm/pr-subscribers-mlir @llvm/pr-subscribers-llvm-support Author: Mehdi Amini (joker-eph) ChangesAlso, improve LDBG() to accept debug type and level in any order, and add unit-tests for LDBG() and LGDB_OS(). LDBG_OS() is a macro that behaves like LDBG() but instead of directly using it to stream the output, it takes a callback function that will be called with a raw_ostream. Patch is 22.31 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/157194.diff 3 Files Affected:
diff --git a/llvm/include/llvm/Support/Debug.h b/llvm/include/llvm/Support/Debug.h
index a7795d403721c..b73f2d7c8b852 100644
--- a/llvm/include/llvm/Support/Debug.h
+++ b/llvm/include/llvm/Support/Debug.h
@@ -44,11 +44,6 @@ class raw_ostream;
/// level, return false.
LLVM_ABI bool isCurrentDebugType(const char *Type, int Level = 0);
-/// Overload allowing to swap the order of the Type and Level arguments.
-LLVM_ABI inline bool isCurrentDebugType(int Level, const char *Type) {
- return isCurrentDebugType(Type, Level);
-}
-
/// setCurrentDebugType - Set the current debug type, as if the -debug-only=X
/// option were specified. Note that DebugFlag also needs to be set to true for
/// debug output to be produced.
diff --git a/llvm/include/llvm/Support/DebugLog.h b/llvm/include/llvm/Support/DebugLog.h
index dce706e196bde..7963c38761ac1 100644
--- a/llvm/include/llvm/Support/DebugLog.h
+++ b/llvm/include/llvm/Support/DebugLog.h
@@ -19,52 +19,81 @@
namespace llvm {
#ifndef NDEBUG
-// LDBG() is a macro that can be used as a raw_ostream for debugging.
-// It will stream the output to the dbgs() stream, with a prefix of the
-// debug type and the file and line number. A trailing newline is added to the
-// output automatically. If the streamed content contains a newline, the prefix
-// is added to each beginning of a new line. Nothing is printed if the debug
-// output is not enabled or the debug type does not match.
-//
-// E.g.,
-// LDBG() << "Bitset contains: " << Bitset;
-// is somehow equivalent to
-// LLVM_DEBUG(dbgs() << "[" << DEBUG_TYPE << "] " << __FILE__ << ":" <<
-// __LINE__ << " "
-// << "Bitset contains: " << Bitset << "\n");
-//
+/// LDBG() is a macro that can be used as a raw_ostream for debugging.
+/// It will stream the output to the dbgs() stream, with a prefix of the
+/// debug type and the file and line number. A trailing newline is added to the
+/// output automatically. If the streamed content contains a newline, the prefix
+/// is added to each beginning of a new line. Nothing is printed if the debug
+/// output is not enabled or the debug type does not match.
+///
+/// E.g.,
+/// LDBG() << "Bitset contains: " << Bitset;
+/// is somehow equivalent to
+/// LLVM_DEBUG(dbgs() << "[" << DEBUG_TYPE << "] " << __FILE__ << ":" <<
+/// __LINE__ << " "
+/// << "Bitset contains: " << Bitset << "\n");
+///
// An optional `level` argument can be provided to control the verbosity of the
-// output. The default level is 1, and is in increasing level of verbosity.
-//
-// The `level` argument can be a literal integer, or a macro that evaluates to
-// an integer.
-//
-// An optional `type` argument can be provided to control the debug type. The
-// default type is DEBUG_TYPE. The `type` argument can be a literal string, or a
-// macro that evaluates to a string.
+/// output. The default level is 1, and is in increasing level of verbosity.
+///
+/// The `level` argument can be a literal integer, or a macro that evaluates to
+/// an integer.
+///
+/// An optional `type` argument can be provided to control the debug type. The
+/// default type is DEBUG_TYPE. The `type` argument can be a literal string, or
+/// a macro that evaluates to a string.
+///
+/// E.g.,
+/// LDBG(2) << "Bitset contains: " << Bitset;
+/// LDBG("debug_type") << "Bitset contains: " << Bitset;
+/// LDBG(2, "debug_type") << "Bitset contains: " << Bitset;
+/// LDBG("debug_type", 2) << "Bitset contains: " << Bitset;
#define LDBG(...) _GET_LDBG_MACRO(__VA_ARGS__)(__VA_ARGS__)
-// Helper macros to choose the correct macro based on the number of arguments.
+/// LDBG_OS() is a macro that behaves like LDBG() but instead of directly using
+/// it to stream the output, it takes a callback function that will be called
+/// with a raw_ostream.
+/// This is useful when you need to pass a `raw_ostream` to a helper function to
+/// be able to print (when the `<<` operator is not available).
+///
+/// E.g.,
+/// LDBG_OS([&] (raw_ostream &Os) {
+/// Os << "Pass Manager contains: ";
+/// pm.printAsTextual(Os);
+/// });
+///
+/// Just like LDBG(), it optionally accepts a `level` and `type` arguments.
+/// E.g.,
+/// LDBG_OS(2, [&] (raw_ostream &Os) { ... });
+/// LDBG_OS("debug_type", [&] (raw_ostream &Os) { ... });
+/// LDBG_OS(2, "debug_type", [&] (raw_ostream &Os) { ... });
+/// LDBG_OS("debug_type", 2, [&] (raw_ostream &Os) { ... });
+///
+#define LDBG_OS(...) _GET_LDBG_OS_MACRO(__VA_ARGS__)(__VA_ARGS__)
+
+// Helper macros to choose the correct LDBG() macro based on the number of
+// arguments.
#define LDBG_FUNC_CHOOSER(_f1, _f2, _f3, ...) _f3
#define LDBG_FUNC_RECOMPOSER(argsWithParentheses) \
LDBG_FUNC_CHOOSER argsWithParentheses
#define LDBG_CHOOSE_FROM_ARG_COUNT(...) \
LDBG_FUNC_RECOMPOSER( \
- (__VA_ARGS__, LDBG_LOG_LEVEL_WITH_TYPE, LDBG_LOG_LEVEL, ))
-#define LDBG_NO_ARG_EXPANDER() , , LDBG_LOG_LEVEL_1
+ (__VA_ARGS__, LDBG_LOG_LEVEL_WITH_TYPE, LDBG_LOG_LEVEL_1_ARG, ))
+#define LDBG_NO_ARG_EXPANDER() , , LDBG_LOG_LEVEL_NO_ARG
#define _GET_LDBG_MACRO(...) \
LDBG_CHOOSE_FROM_ARG_COUNT(LDBG_NO_ARG_EXPANDER __VA_ARGS__())
-// Dispatch macros to support the `level` argument or none (default to 1)
-#define LDBG_LOG_LEVEL(LEVEL) \
- DEBUGLOG_WITH_STREAM_AND_TYPE(llvm::dbgs(), LEVEL, DEBUG_TYPE)
-#define LDBG_LOG_LEVEL_1() LDBG_LOG_LEVEL(1)
-// This macro is a helper when LDBG() is called with 2 arguments.
-// In this case we want to allow the order of the arguments to be swapped.
-// We rely on the fact that the `level` argument is an integer, and the `type`
-// is a string and dispatch to a C++ API that is overloaded.
-#define LDBG_LOG_LEVEL_WITH_TYPE(LEVEL_OR_TYPE, TYPE_OR_LEVEL) \
- DEBUGLOG_WITH_STREAM_AND_TYPE(llvm::dbgs(), (LEVEL_OR_TYPE), (TYPE_OR_LEVEL))
+// Helper macros to choose the correct LDBG_OS() macro based on the number of
+// arguments.
+#define LDBG_OS_FUNC_CHOOSER(_f1, _f2, _f3, _f4, ...) _f4
+#define LDBG_OS_FUNC_RECOMPOSER(argsWithParentheses) \
+ LDBG_OS_FUNC_CHOOSER argsWithParentheses
+#define LDBG_OS_CHOOSE_FROM_ARG_COUNT(...) \
+ LDBG_OS_FUNC_RECOMPOSER( \
+ (__VA_ARGS__, LDBG_OS_3_ARGS, LDBG_OS_2_ARGS, LDBG_OS_1_ARG, ))
+#define LDBG_OS_NO_ARG_EXPANDER() , , , LDBG_OS_1_ARG
+#define _GET_LDBG_OS_MACRO(...) \
+ LDBG_OS_CHOOSE_FROM_ARG_COUNT(LDBG_OS_NO_ARG_EXPANDER __VA_ARGS__())
// We want the filename without the full path. We are using the __FILE__ macro
// and a constexpr function to strip the path prefix. We can avoid the frontend
@@ -76,22 +105,129 @@ namespace llvm {
#define __LLVM_FILE_NAME__ ::llvm::impl::getShortFileName(__FILE__)
#endif
-#define DEBUGLOG_WITH_STREAM_TYPE_FILE_AND_LINE(STREAM, LEVEL, TYPE, FILE, \
- LINE) \
- for (bool _c = \
- (::llvm::DebugFlag && ::llvm::isCurrentDebugType(TYPE, LEVEL)); \
+/// These macros are detecting if the DEBUG_TYPE or LDBG_DEBUG_STREAM macros are
+/// defined. They are using a combination of preprocessor tricks and C++17
+// used-defined string literals to achieve this.
+// For example, if DEBUG_TYPE is defined to "foo", the preprocessor will expand
+// the macro and then stringify the result to
+// "foo"_LDBG_VARIABLE_CHECK
+// This dispatch to the user-defined string literal operator named
+// _LDBG_VARIABLE_CHECK which returns true. Otherwise it expands to
+// DEBUG_TYPE_LDBG_VARIABLE_CHECK which we define as a macro that returns false.
+#define LDBG_VARIABLE_CHECK_(VARIABLE, ...) VARIABLE##__VA_ARGS__
+#define LDBG_VARIABLE_CHECK(VARIABLE) \
+ LDBG_VARIABLE_CHECK_(VARIABLE, _LDBG_VARIABLE_CHECK)
+// User-defined string literal operator for the LDBG_VARIABLE_CHECK macro.
+constexpr bool operator""_LDBG_VARIABLE_CHECK(const char *, std::size_t) {
+ return true;
+}
+
+#define IS_DEBUG_TYPE_DEFINED() LDBG_VARIABLE_CHECK(DEBUG_TYPE)
+#define DEBUG_TYPE_LDBG_VARIABLE_CHECK 0
+
+#define IS_LDBG_DEBUG_STREAM_DEFINED() LDBG_VARIABLE_CHECK(LDBG_DEBUG_STREAM)
+#define LDBG_DEBUG_STREAM_LDBG_VARIABLE_CHECK 0
+
+/// Helpers to get DEBUG_TYPE as a string literal, even when DEBUG_TYPE is not
+/// defined (in which case it expand to "DEBUG_TYPE")
+#define LDBG_GET_DEBUG_TYPE_STR__(X) #X##_LDBG_DEBUG_STRING
+#define LDBG_GET_DEBUG_TYPE_STR_(X) LDBG_GET_DEBUG_TYPE_STR__(X)
+#define LDBG_GET_DEBUG_TYPE_STR() LDBG_GET_DEBUG_TYPE_STR_(DEBUG_TYPE)
+/// If DEBUG_TYPE is defined, we get the string with the quotes, we need to
+/// remove them here.
+constexpr ::llvm::StringRef operator""_LDBG_DEBUG_STRING(const char *Str,
+ std::size_t) {
+ ::llvm::StringRef S(Str);
+ if (S.front() == '"' && S.back() == '"')
+ return S.drop_front().drop_back();
+ return S;
+}
+
+/// Helper to provide the default level (=1) or type (=DEBUG_TYPE). This is used
+/// when a single argument is passed, if it is an integer we return DEBUG_TYPE
+/// and if it is a string we return 1. This fails with a static_assert if we
+/// pass an integer and DEBUG_TYPE is not defined.
+#define LDBG_GET_DEFAULT_TYPE_OR_LEVEL(LEVEL_OR_TYPE) \
+ [](auto LevelOrType) { \
+ if constexpr (std::is_integral_v<decltype(LevelOrType)>) { \
+ using ::llvm::operator""_LDBG_VARIABLE_CHECK; \
+ using ::llvm::operator""_LDBG_DEBUG_STRING; \
+ if constexpr (IS_DEBUG_TYPE_DEFINED()) \
+ return LDBG_GET_DEBUG_TYPE_STR(); \
+ else \
+ static_assert(false, "DEBUG_TYPE is not defined"); \
+ } else { \
+ return 1; \
+ } \
+ }(LEVEL_OR_TYPE)
+
+/// Use a macro to allow unit-testing to override.
+#define LDBG_STREAM ::llvm::dbgs()
+
+#define DEBUGLOG_WITH_STREAM_TYPE_FILE_AND_LINE(STREAM, LEVEL_OR_TYPE, \
+ TYPE_OR_LEVEL, FILE, LINE) \
+ for (bool _c = (::llvm::DebugFlag && ::llvm::impl::ldbgIsCurrentDebugType( \
+ TYPE_OR_LEVEL, LEVEL_OR_TYPE)); \
_c; _c = false) \
for (::llvm::impl::raw_ldbg_ostream LdbgOS{ \
- ::llvm::impl::computePrefix(TYPE, FILE, LINE, LEVEL), (STREAM)}; \
+ ::llvm::impl::computePrefix(TYPE_OR_LEVEL, FILE, LINE, \
+ LEVEL_OR_TYPE), \
+ (STREAM), /*ShouldPrefixNextString=*/true, \
+ /*ShouldEmitNewLineOnDestruction=*/true}; \
_c; _c = false) \
- ::llvm::impl::RAIINewLineStream{LdbgOS}.asLvalue()
+ LdbgOS
-#define DEBUGLOG_WITH_STREAM_TYPE_AND_FILE(STREAM, LEVEL, TYPE, FILE) \
- DEBUGLOG_WITH_STREAM_TYPE_FILE_AND_LINE(STREAM, LEVEL, TYPE, FILE, __LINE__)
-#define DEBUGLOG_WITH_STREAM_AND_TYPE(STREAM, LEVEL, TYPE) \
- DEBUGLOG_WITH_STREAM_TYPE_AND_FILE(STREAM, LEVEL, TYPE, __LLVM_FILE_NAME__)
+#define DEBUGLOG_WITH_STREAM_TYPE_AND_FILE(STREAM, LEVEL_OR_TYPE, \
+ TYPE_OR_LEVEL, FILE) \
+ DEBUGLOG_WITH_STREAM_TYPE_FILE_AND_LINE(STREAM, LEVEL_OR_TYPE, \
+ TYPE_OR_LEVEL, FILE, __LINE__)
+#define DEBUGLOG_WITH_STREAM_AND_TYPE(STREAM, LEVEL_OR_TYPE, TYPE_OR_LEVEL) \
+ DEBUGLOG_WITH_STREAM_TYPE_AND_FILE(STREAM, LEVEL_OR_TYPE, TYPE_OR_LEVEL, \
+ __LLVM_FILE_NAME__)
+
+// Dispatch macros when a signle argument is provided. This can be either a
+// level of the debug type.
+#define LDBG_LOG_LEVEL_1_ARG(LEVEL_OR_TYPE) \
+ DEBUGLOG_WITH_STREAM_AND_TYPE(LDBG_STREAM, LEVEL_OR_TYPE, \
+ LDBG_GET_DEFAULT_TYPE_OR_LEVEL(LEVEL_OR_TYPE))
+#define LDBG_LOG_LEVEL_NO_ARG() LDBG_LOG_LEVEL_1_ARG(1)
+// This macro is a helper when LDBG() is called with 2 arguments.
+// In this case we want to allow the order of the arguments to be swapped.
+// We rely on the fact that the `level` argument is an integer, and the `type`
+// is a string and dispatch to a C++ API that is overloaded.
+#define LDBG_LOG_LEVEL_WITH_TYPE(LEVEL_OR_TYPE, TYPE_OR_LEVEL) \
+ DEBUGLOG_WITH_STREAM_AND_TYPE(LDBG_STREAM, (LEVEL_OR_TYPE), (TYPE_OR_LEVEL))
+
+#define LDBG_OS_IMPL(TYPE_OR_LEVEL, LEVEL_OR_TYPE, CALLBACK, STREAM, FILE, \
+ LINE) \
+ if (::llvm::DebugFlag && \
+ ::llvm::impl::ldbgIsCurrentDebugType(TYPE_OR_LEVEL, LEVEL_OR_TYPE)) { \
+ ::llvm::impl::raw_ldbg_ostream LdbgOS{ \
+ ::llvm::impl::computePrefix(TYPE_OR_LEVEL, FILE, LINE, LEVEL_OR_TYPE), \
+ (STREAM), /*ShouldPrefixNextString=*/true, \
+ /*ShouldEmitNewLineOnDestruction=*/true}; \
+ CALLBACK(LdbgOS); \
+ }
+
+#define LDBG_OS_3_ARGS(TYPE_OR_LEVEL, LEVEL_OR_TYPE, CALLBACK) \
+ LDBG_OS_IMPL(TYPE_OR_LEVEL, LEVEL_OR_TYPE, CALLBACK, LDBG_STREAM, \
+ __LLVM_FILE_NAME__, __LINE__)
+
+#define LDBG_OS_2_ARGS(LEVEL_OR_TYPE, CALLBACK) \
+ LDBG_OS_3_ARGS(LDBG_GET_DEFAULT_TYPE_OR_LEVEL(LEVEL_OR_TYPE), LEVEL_OR_TYPE, \
+ CALLBACK)
+#define LDBG_OS_1_ARG(CALLBACK) LDBG_OS_2_ARGS(1, CALLBACK)
namespace impl {
+/// Helper to call isCurrentDebugType with a StringRef.
+static LLVM_ATTRIBUTE_UNUSED bool ldbgIsCurrentDebugType(StringRef Type,
+ int Level) {
+ return ::llvm::isCurrentDebugType(Type.str().c_str(), Level);
+}
+static LLVM_ATTRIBUTE_UNUSED bool ldbgIsCurrentDebugType(int Level,
+ StringRef Type) {
+ return ::llvm::isCurrentDebugType(Type.str().c_str(), Level);
+}
/// A raw_ostream that tracks `\n` and print the prefix after each
/// newline.
@@ -99,6 +235,7 @@ class LLVM_ABI raw_ldbg_ostream final : public raw_ostream {
std::string Prefix;
raw_ostream &Os;
bool ShouldPrefixNextString;
+ bool ShouldEmitNewLineOnDestruction;
/// Split the line on newlines and insert the prefix before each
/// newline. Forward everything to the underlying stream.
@@ -131,12 +268,17 @@ class LLVM_ABI raw_ldbg_ostream final : public raw_ostream {
public:
explicit raw_ldbg_ostream(std::string Prefix, raw_ostream &Os,
- bool ShouldPrefixNextString = true)
+ bool ShouldPrefixNextString = true,
+ bool ShouldEmitNewLineOnDestruction = false)
: Prefix(std::move(Prefix)), Os(Os),
- ShouldPrefixNextString(ShouldPrefixNextString) {
+ ShouldPrefixNextString(ShouldPrefixNextString),
+ ShouldEmitNewLineOnDestruction(ShouldEmitNewLineOnDestruction) {
SetUnbuffered();
}
- ~raw_ldbg_ostream() final {}
+ ~raw_ldbg_ostream() final {
+ if (ShouldEmitNewLineOnDestruction)
+ Os << '\n';
+ }
/// Forward the current_pos method to the underlying stream.
uint64_t current_pos() const final { return Os.tell(); }
@@ -173,17 +315,17 @@ getShortFileName(const char *path) {
/// "[DebugType] File:Line "
/// Where the File is the file name without the path prefix.
static LLVM_ATTRIBUTE_UNUSED std::string
-computePrefix(const char *DebugType, const char *File, int Line, int Level) {
+computePrefix(StringRef DebugType, const char *File, int Line, int Level) {
std::string Prefix;
raw_string_ostream OsPrefix(Prefix);
- if (DebugType)
+ if (!DebugType.empty())
OsPrefix << "[" << DebugType << ":" << Level << "] ";
OsPrefix << File << ":" << Line << " ";
return OsPrefix.str();
}
/// Overload allowing to swap the order of the DebugType and Level arguments.
static LLVM_ATTRIBUTE_UNUSED std::string
-computePrefix(int Level, const char *File, int Line, const char *DebugType) {
+computePrefix(int Level, const char *File, int Line, StringRef DebugType) {
return computePrefix(DebugType, File, Line, Level);
}
@@ -194,6 +336,7 @@ computePrefix(int Level, const char *File, int Line, const char *DebugType) {
#define LDBG(...) \
for (bool _c = false; _c; _c = false) \
::llvm::nulls()
+#define LDBG_OS(...)
#endif
} // end namespace llvm
diff --git a/llvm/unittests/Support/DebugLogTest.cpp b/llvm/unittests/Support/DebugLogTest.cpp
index e087705b72586..62b269dc5b3e4 100644
--- a/llvm/unittests/Support/DebugLogTest.cpp
+++ b/llvm/unittests/Support/DebugLogTest.cpp
@@ -27,7 +27,7 @@ TEST(DebugLogTest, Basic) {
{
std::string str;
raw_string_ostream os(str);
- DEBUGLOG_WITH_STREAM_AND_TYPE(os, 0, nullptr) << "NoType";
+ DEBUGLOG_WITH_STREAM_AND_TYPE(os, "", 0) << "NoType";
EXPECT_FALSE(StringRef(os.str()).starts_with('['));
EXPECT_TRUE(StringRef(os.str()).ends_with("NoType\n"));
}
@@ -37,7 +37,7 @@ TEST(DebugLogTest, Basic) {
std::string str;
raw_string_ostream os(str);
DEBUGLOG_WITH_STREAM_AND_TYPE(os, 0, "A") << "A";
- DEBUGLOG_WITH_STREAM_AND_TYPE(os, 0, "B") << "B";
+ DEBUGLOG_WITH_STREAM_AND_TYPE(os, "B", 0) << "B";
EXPECT_TRUE(StringRef(os.str()).starts_with('['));
EXPECT_THAT(os.str(), AllOf(HasSubstr("A\n"), HasSubstr("B\n")));
}
@@ -128,6 +128,131 @@ TEST(DebugLogTest, DestructorPrefix) {
// After destructors, nothing should have been printed.
EXPECT_EQ(os.str(), "");
}
+
+TEST(DebugLogTest, LDBG_MACROS) {
+ llvm::DebugFlag = true;
+ static const char *DT[] = {"A:3", "B:2"};
+ setCurrentDebugTypes(DT, sizeof(DT) / sizeof(DT[0]));
+ std::string Str;
+ raw_string_ostream DebugOs(Str);
+ std::string StrRef;
+ raw_string_ostream RefOs(StrRef);
+#undef LDBG_STREAM
+#define LDBG_STREAM DebugOs
+#define DEBUG_TYPE "A"
+ LDBG() << "Hello, world!";
+ RefOs << "[A:1] " << __LLVM_FILE_NAME__ << ":" << (__LINE__ - 1)
+ << " Hello, world!\n";
+ EXPECT_EQ(DebugOs.str(), RefOs.str());
+ Str.clear();
+ StrRef.clear();
+
+ // Test with a level, no type.
+ LDBG(2) << "Hello, world!";
+ RefOs << "[A:2] " << __LLVM_FILE_NAME__ << ":" << (__LINE__ - 1)
+ << " Hello, world!\n";
+ EXPECT_EQ(DebugOs.str(), RefOs.str());
+ Str.clear();
+ StrRef.clear();
+
+// Now the type will be explicit, check we don't use DEBUG_TYPE.
+#undef DEBUG_TYPE
+
+ // Test with a type
+ LDBG("B") << "Hello, world!";
+ RefOs << "[B:1] " << __LLVM_FILE_NAME__ << ":" << (__LINE__ - 1)
+ << " Hello, world!\n";
+ EXPECT_EQ(DebugOs.str(), RefOs.str());
+ Str.clear();
+ StrRef.clear();
+
+ // Test with a level and a type
+ LDBG(2, "B") << "Hello, world!";
+ RefOs << "[B:2] " << __LLVM_FILE_NAME__ << ":" << (__LINE__ - 1)
+ << " Hello, world!\n";
+ EXPECT_EQ(DebugOs.str(), RefOs.str());
+ Str.clear();
+ StrRef.clear();
+
+ // Test with a type and a level
+ LDBG("B", 2) << "Hello, world!";
+ RefOs << "[B:2] " << __LLVM_FILE_NAME__ << ":" << (__LINE__ - 1)
+ << " Hello, world!\n";
+ EXPECT_EQ(DebugOs.str(), RefOs.str());
+ Str.clear();...
[truncated]
|
banach-space
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like how you refactored LDBG_OS not to require LLVM_DEBUG and to be more consistent with LDBG 🙏🏻
This change is actually quite dense and not something I contribute to, so it took me a while to parse. Mostly make sense - I've left some minor suggestion. I will do another pass once these are addressed.
Thanks! In particular the unit tests will be a great source of documentation.
d25e088 to
e582c36
Compare
banach-space
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates!
I've left a few minor suggestions. I would also be good to somehow group+document the macros defined towards the end (see my comment inline).
Otherwise LG!
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
@banach-space : I revamped the grouping, now I start with a section: where I also ensure that all the macros are prefixed with LDBG_ now and the suffixes matches with the LDBG_OS_ impl. Then there is the section for: Modeled around the first section for LDBG() implementation. And finally a section with: PTAL. |
llvm/include/llvm/Support/DebugLog.h
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How important is this change? (E.g, StringRef vs const char*)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's needed because of the auto-detection for DEBUG_TYPE mean that we get a literal that is:
"\"my-debug-type\"" and we use a StringRef in operator""_LDBG_DEBUG_STRING to remove the extra surrounding double quotes.
If someone knows a better macro expansion, I'm interested, but it took me 2h to come up with these tricks :)
1f90052 to
73e1581
Compare
banach-space
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for addressing my comments!
Please wait for @jpienaar to approve before landing.
|
Working with @math-fehr and iterating on the implementation, I managed to simplify if further in this last push: https://github.com/llvm/llvm-project/compare/13b70c3611f1bc49191698c8ddd52a74c81f5201..8b8b41a5157645de0addc70869e2d5d8e5a9cb57 |
jpienaar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks!
Also, improve LDBG() to accept debug type and level in any order, and add unit-tests for LDBG() and LGDB_OS(). LDBG_OS() is a macro that behaves like LDBG() but instead of directly using it to stream the output, it takes a callback function that will be called with a raw_ostream. Co-authored-by: Andrzej Warzyński <[email protected]>
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/204/builds/21732 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/140/builds/30535 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/203/builds/22920 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/10/builds/13288 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/205/builds/21709 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/116/builds/18197 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/27244 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/144/builds/35196 Here is the relevant piece of the build log for the reference |
This reverts commit c84f34b.
Reverts #157194 Bots are broken, investigation needed.
…(#158058) Reverts llvm/llvm-project#157194 Bots are broken, investigation needed.
Also, improve LDBG() to accept debug type and level in any order, and add unit-tests for LDBG() and LGDB_OS(). LDBG_OS() is a macro that behaves like LDBG() but instead of directly using it to stream the output, it takes a callback function that will be called with a raw_ostream. Co-authored-by: Andrzej Warzyński <[email protected]>
Also, improve LDBG() to accept debug type and level in any order, and add unit-tests for LDBG() and LGDB_OS(). LDBG_OS() is a macro that behaves like LDBG() but instead of directly using it to stream the output, it takes a callback function that will be called with a raw_ostream. Co-authored-by: Andrzej Warzyński <[email protected]> Co-authored-by: Andrzej Warzyński <[email protected]>
…158260) Also, improve LDBG() to accept debug type and level in any order, and add unit-tests for LDBG() and LGDB_OS(). LDBG_OS() is a macro that behaves like LDBG() but instead of directly using it to stream the output, it takes a callback function that will be called with a raw_ostream. Co-authored-by: Andrzej Warzyński <[email protected]> Co-authored-by: Andrzej Warzyński <[email protected]>
Also, improve LDBG() to accept debug type and level in any order, and add unit-tests for LDBG() and LGDB_OS(). LDBG_OS() is a macro that behaves like LDBG() but instead of directly using it to stream the output, it takes a callback function that will be called with a raw_ostream. This is a re-land with workarounds for older gcc and clang versions. Previous attempts in #157194 and #158260 Co-authored-by: Andrzej Warzyński <[email protected]>
Also, improve LDBG() to accept debug type and level in any order, and add unit-tests for LDBG() and LGDB_OS().
LDBG_OS() is a macro that behaves like LDBG() but instead of directly using it to stream the output, it takes a callback function that will be called with a raw_ostream.