Implement a custom stream for LDBG macro to handle newlines #150750
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-llvm-support Author: Mehdi Amini (joker-eph) ChangesThis prints the prefix on every new line, allowing for an output that looks like: [dead-code-analysis] DeadCodeAnalysis.cpp:288 Visiting operation: func.func private @private_1() -> (i32, i32) { Full diff: https://github.com/llvm/llvm-project/pull/150750.diff 1 Files Affected:
diff --git a/llvm/include/llvm/Support/DebugLog.h b/llvm/include/llvm/Support/DebugLog.h
index b1b17e3ede950..a57a7e9f2a42f 100644
--- a/llvm/include/llvm/Support/DebugLog.h
+++ b/llvm/include/llvm/Support/DebugLog.h
@@ -30,30 +30,62 @@ namespace llvm {
#define DEBUGLOG_WITH_STREAM_AND_TYPE(STREAM, TYPE) \
for (bool _c = (::llvm::DebugFlag && ::llvm::isCurrentDebugType(TYPE)); _c; \
_c = false) \
- ::llvm::impl::LogWithNewline(TYPE, __SHORT_FILE__, __LINE__, (STREAM))
+ ::llvm::impl::LogWithNewline(TYPE, __SHORT_FILE__, __LINE__, (STREAM)) \
+ .stream()
#else
#define DEBUGLOG_WITH_STREAM_AND_TYPE(STREAM, TYPE) \
for (bool _c = (::llvm::DebugFlag && ::llvm::isCurrentDebugType(TYPE)); _c; \
_c = false) \
- ::llvm::impl::LogWithNewline(TYPE, __FILE__, __LINE__, (STREAM))
+ ::llvm::impl::LogWithNewline(TYPE, __FILE__, __LINE__, (STREAM)).stream()
#endif
namespace impl {
-class LogWithNewline {
+
+/// A raw_ostream that tracks `\n` and print the prefix.
+class LLVM_ABI raw_ldbg_ostream : public raw_ostream {
+ std::string Prefix;
+ raw_ostream &Os;
+
+ /// Split the line on newlines and insert the prefix before each newline.
+ /// Forward everything to the underlying stream.
+ void write_impl(const char *Ptr, size_t Size) final {
+ auto Str = StringRef(Ptr, Size);
+ auto Eol = Str.find('\n');
+ while (Eol != StringRef::npos) {
+ StringRef Line = Str.take_front(Eol + 1);
+ Os.write(Line.data(), Line.size());
+ Os.write(Prefix.c_str(), Prefix.size());
+
+ Str = Str.drop_front(Eol + 1);
+ Eol = Str.find('\n');
+ }
+ Os.write(Str.data(), Str.size());
+ }
+
public:
- LogWithNewline(const char *debug_type, const char *file, int line,
- raw_ostream &os)
- : os(os) {
-#if !defined(__SHORT_FILE__)
- file = ::llvm::impl::LogWithNewline::getShortFileName(file);
-#endif
- if (debug_type)
- os << "[" << debug_type << "] ";
- os << file << ":" << line << " ";
+ explicit raw_ldbg_ostream(std::string Prefix, raw_ostream &Os)
+ : Prefix(std::move(Prefix)), Os(Os) {
+ SetUnbuffered();
}
- ~LogWithNewline() { os << '\n'; }
- template <typename T> raw_ostream &operator<<(const T &t) && {
- return os << t;
+
+ void emitPrefix() { Os.write(Prefix.c_str(), Prefix.size()); }
+
+ raw_ostream &getOs() { return Os; }
+
+ /// Forward the current_pos method to the underlying stream.
+ uint64_t current_pos() const final { return Os.tell(); }
+};
+
+class LogWithNewline {
+public:
+ LogWithNewline(const char *DebugType, const char *File, int Line,
+ raw_ostream &Os)
+ : Os(computePrefix(DebugType, File, Line), Os) {}
+ ~LogWithNewline() { Os.getOs() << '\n'; }
+
+ raw_ostream &stream() {
+ Os.emitPrefix();
+ return Os;
}
// Prevent copying, as this class manages newline responsibility and is
@@ -61,6 +93,8 @@ class LogWithNewline {
LogWithNewline(const LogWithNewline &) = delete;
LogWithNewline &operator=(const LogWithNewline &) = delete;
LogWithNewline &operator=(LogWithNewline &&) = delete;
+
+private:
static constexpr const char *getShortFileName(const char *path) {
// Remove the path prefix from the file name.
const char *filename = path;
@@ -71,9 +105,16 @@ class LogWithNewline {
}
return filename;
}
-
-private:
- raw_ostream &os;
+ static std::string computePrefix(const char *DebugType, const char *File,
+ int Line) {
+ std::string Prefix;
+ raw_string_ostream OsPrefix(Prefix);
+ if (DebugType)
+ OsPrefix << "[" << DebugType << "] ";
+ OsPrefix << File << ":" << Line << " ";
+ return OsPrefix.str();
+ }
+ raw_ldbg_ostream Os;
};
} // end namespace impl
#else
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
jpienaar
approved these changes
Jul 27, 2025
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
joker-eph
force-pushed
the
stream
branch
2 times, most recently
from
e53033e to
2c58712
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull Request Overview
This PR implements a custom stream (raw_ldbg_ostream) for the LDBG macro to handle newlines properly by prefixing each line with debug information. The change ensures that multi-line debug output maintains consistent formatting with the debug prefix appearing on every line.
Key changes:
- Replaced the simple
LogWithNewlineclass with a more sophisticatedraw_ldbg_ostreamthat inherits fromraw_ostream - Added logic to detect newlines and automatically insert prefixes for each new line
- Refactored the macro definitions to use the new stream class
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| llvm/include/llvm/Support/DebugLog.h | Implements the new raw_ldbg_ostream class and refactors debug logging macros to use it |
| llvm/unittests/Support/DebugLogTest.cpp | Adds unit tests to verify the new stream prefix functionality works correctly |
Comments suppressed due to low confidence (1)
llvm/include/llvm/Support/DebugLog.h:111
- The if statement is missing braces around the single statement body. This creates inconsistency with the surrounding code style and could lead to errors if additional statements are added later.
if (*p == '/' || *p == '\\')
This prints the prefix on every new line, allowing for an output that looks like:
[dead-code-analysis] DeadCodeAnalysis.cpp:284 Visiting program point: 0x55ea5be2d4c8 <after operation>:func.func private @private_1() -> (i32, i32) {...}
[dead-code-analysis] DeadCodeAnalysis.cpp:288 Visiting operation: func.func private @private_1() -> (i32, i32) {
[dead-code-analysis] DeadCodeAnalysis.cpp:288 %c0_i32 = arith.constant 0 : i32
[dead-code-analysis] DeadCodeAnalysis.cpp:288 %0 = arith.addi %c0_i32, %c0_i32 {tag = "one"} : i32
[dead-code-analysis] DeadCodeAnalysis.cpp:288 return %c0_i32, %0 : i32, i32
[dead-code-analysis] DeadCodeAnalysis.cpp:288 }
[dead-code-analysis] DeadCodeAnalysis.cpp:313 Visiting callable operation: func.func private @private_1() -> (i32, i32) {
[dead-code-analysis] DeadCodeAnalysis.cpp:313 %c0_i32 = arith.constant 0 : i32
[dead-code-analysis] DeadCodeAnalysis.cpp:313 %0 = arith.addi %c0_i32, %c0_i32 {tag = "one"} : i32
[dead-code-analysis] DeadCodeAnalysis.cpp:313 return %c0_i32, %0 : i32, i32
[dead-code-analysis] DeadCodeAnalysis.cpp:313 }
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/11/builds/20486 Here is the relevant piece of the build log for the reference |
ingomueller-net
added a commit
to ingomueller-net/llvm-project
that referenced
this pull request
Jul 28, 2025
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]>
ingomueller-net
added a commit
to ingomueller-net/llvm-project
that referenced
this pull request
Jul 28, 2025
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]>
ingomueller-net
added a commit
to ingomueller-net/llvm-project
that referenced
this pull request
Jul 28, 2025
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]>
ingomueller-net
added a commit
that referenced
this pull request
Jul 28, 2025
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 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]>
mahesh-attarde
pushed a commit
to mahesh-attarde/llvm-project
that referenced
this pull request
Jul 28, 2025
) This prints the prefix on every new line, allowing for an output that looks like: ``` [dead-code-analysis] DeadCodeAnalysis.cpp:288 Visiting operation: func.func private @private_1() -> (i32, i32) { [dead-code-analysis] DeadCodeAnalysis.cpp:288 %c0_i32 = arith.constant 0 : i32 [dead-code-analysis] DeadCodeAnalysis.cpp:288 %0 = arith.addi %c0_i32, %c0_i32 {tag = "one"} : i32 [dead-code-analysis] DeadCodeAnalysis.cpp:288 return %c0_i32, %0 : i32, i32 [dead-code-analysis] DeadCodeAnalysis.cpp:288 } [dead-code-analysis] DeadCodeAnalysis.cpp:313 Visiting callable operation: func.func private @private_1() -> (i32, i32) { [dead-code-analysis] DeadCodeAnalysis.cpp:313 %c0_i32 = arith.constant 0 : i32 [dead-code-analysis] DeadCodeAnalysis.cpp:313 %0 = arith.addi %c0_i32, %c0_i32 {tag = "one"} : i32 [dead-code-analysis] DeadCodeAnalysis.cpp:313 return %c0_i32, %0 : i32, i32 [dead-code-analysis] DeadCodeAnalysis.cpp:313 } ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This prints the prefix on every new line, allowing for an output that looks like: