-
Notifications
You must be signed in to change notification settings - Fork 15.2k
Fix DEBUGLOG_WITH_STREAM_TYPE_AND_FILE
broken in #150750
#150920
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-llvm-support Author: Ingo Müller (ingomueller-net) ChangesThis PR fixes the Full diff: https://github.com/llvm/llvm-project/pull/150920.diff 1 Files Affected:
diff --git a/llvm/include/llvm/Support/DebugLog.h b/llvm/include/llvm/Support/DebugLog.h
index 19d309865bbd4..5e308eb993036 100644
--- a/llvm/include/llvm/Support/DebugLog.h
+++ b/llvm/include/llvm/Support/DebugLog.h
@@ -40,8 +40,8 @@ namespace llvm {
DEBUGLOG_WITH_STREAM_TYPE_AND_FILE(STREAM, TYPE, __SHORT_FILE__)
#else
#define DEBUGLOG_WITH_STREAM_AND_TYPE(STREAM, TYPE) \
- DEBUGLOG_WITH_STREAM_TYPE_AND_FILE( \
- STREAM, TYPE, ::llvm::impl::LogWithNewline::getShortFileName(__FILE__))
+ DEBUGLOG_WITH_STREAM_TYPE_AND_FILE(STREAM, TYPE, \
+ ::llvm::impl::getShortFileName(__FILE__))
#endif
namespace impl {
|
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!
We don't exercise this code path upstream right now.
What about adding this diff to your PR:
diff --git a/llvm/unittests/Support/DebugLogTest.cpp b/llvm/unittests/Support/DebugLogTest.cpp
index 8356ae856be2..3c82376734b9 100644
--- a/llvm/unittests/Support/DebugLogTest.cpp
+++ b/llvm/unittests/Support/DebugLogTest.cpp
@@ -6,6 +6,11 @@
//
//===----------------------------------------------------------------------===//
+// This macro is defined in the LLVM build system, but we undefine it here
+// so that we test at least once in-tree the case where __SHORT_FILE__ is not
+// defined.
+#undef __SHORT_FILE__
+
#include "llvm/Support/DebugLog.h"
#include "llvm/ADT/Sequence.h"
#include "llvm/Support/raw_ostream.h"
Sounds good! Will update the PR shortly. |
fe079da
to
c51eb1f
Compare
This PR fixes the `DEBUGLOG_WITH_STREAM_TYPE_AND_FILE` macro that got broken in llvm#150750. That PR introduces a more sophisitaced version of that macro and refactored some code in that process, making the `getShortFileName` a free function instead of a class member function, but did not adapt this macro to the refactored code. Signed-off-by: Ingo Müller <[email protected]>
As suggested by @joker-ehp, this commit undefines `__SHORTFILE__` in `DebugLogTest.cpp` order to make the previously broken variant of `DEBUGLOG_WITH_STREAM_AND_TYPE` being tested at least in this one test, as it is otherwise never used in CI. Signed-off-by: Ingo Müller <[email protected]>
c51eb1f
to
9029837
Compare
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 fix!
This PR fixes the
DEBUGLOG_WITH_STREAM_TYPE_AND_FILE
macro that got broken in #150750. That PR introduces a more sophisitaced version of that macro and refactored some code in that process, making thegetShortFileName
a free function instead of a class member function, but did not adapt this macro to the refactored code.