[lld-link] Add context-aware diagnostic functions #118430
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
Similar to llvm#112319 for ELF. While there is some initial boilerplate, it can simplify some call sites that use Twine, especially when a printed element uses `ctx` or toString.
Member
|
@llvm/pr-subscribers-lld @llvm/pr-subscribers-lld-coff Author: Fangrui Song (MaskRay) ChangesSimilar to #112319 for ELF. While there is some initial boilerplate, it can simplify some call sites that use Twine, especially when a printed element uses Full diff: https://github.com/llvm/llvm-project/pull/118430.diff 6 Files Affected:
diff --git a/lld/COFF/Config.h b/lld/COFF/Config.h
index 57cb443798cd8f..9e6b17e87c9e70 100644
--- a/lld/COFF/Config.h
+++ b/lld/COFF/Config.h
@@ -9,6 +9,7 @@
#ifndef LLD_COFF_CONFIG_H
#define LLD_COFF_CONFIG_H
+#include "lld/Common/ErrorHandler.h"
#include "llvm/ADT/MapVector.h"
#include "llvm/ADT/SetVector.h"
#include "llvm/ADT/SmallVector.h"
@@ -27,6 +28,7 @@ namespace lld::coff {
using llvm::COFF::IMAGE_FILE_MACHINE_UNKNOWN;
using llvm::COFF::WindowsSubsystem;
using llvm::StringRef;
+class COFFLinkerContext;
class DefinedAbsolute;
class StringChunk;
class Symbol;
@@ -332,6 +334,48 @@ struct Configuration {
BuildIDHash buildIDHash = BuildIDHash::None;
};
+struct COFFSyncStream : SyncStream {
+ COFFLinkerContext &ctx;
+ COFFSyncStream(COFFLinkerContext &ctx, DiagLevel level);
+};
+
+template <typename T>
+std::enable_if_t<!std::is_pointer_v<std::remove_reference_t<T>>,
+ const COFFSyncStream &>
+operator<<(const COFFSyncStream &s, T &&v) {
+ s.os << std::forward<T>(v);
+ return s;
+}
+
+inline const COFFSyncStream &operator<<(const COFFSyncStream &s,
+ const char *v) {
+ s.os << v;
+ return s;
+}
+
+inline const COFFSyncStream &operator<<(const COFFSyncStream &s, Error v) {
+ s.os << llvm::toString(std::move(v));
+ return s;
+}
+
+// Report a log if -verbose is specified.
+COFFSyncStream Log(COFFLinkerContext &ctx);
+
+// Print a message to stdout.
+COFFSyncStream Msg(COFFLinkerContext &ctx);
+
+// Report a warning. Upgraded to an error if /WX is specified.
+COFFSyncStream Warn(COFFLinkerContext &ctx);
+
+// Report an error that will suppress the output file generation.
+COFFSyncStream Err(COFFLinkerContext &ctx);
+
+// Report a fatal error that exits immediately. This should generally be avoided
+// in favor of Err.
+COFFSyncStream Fatal(COFFLinkerContext &ctx);
+
+uint64_t errCount(COFFLinkerContext &ctx);
+
} // namespace lld::coff
#endif
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index a0bff69c6302a2..e4cfcb335869a4 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -56,11 +56,33 @@
#include <optional>
#include <tuple>
+using namespace lld;
+using namespace lld::coff;
using namespace llvm;
using namespace llvm::object;
using namespace llvm::COFF;
using namespace llvm::sys;
+COFFSyncStream::COFFSyncStream(COFFLinkerContext &ctx, DiagLevel level)
+ : SyncStream(ctx.e, level), ctx(ctx) {}
+
+COFFSyncStream coff::Log(COFFLinkerContext &ctx) {
+ return {ctx, DiagLevel::Log};
+}
+COFFSyncStream coff::Msg(COFFLinkerContext &ctx) {
+ return {ctx, DiagLevel::Msg};
+}
+COFFSyncStream coff::Warn(COFFLinkerContext &ctx) {
+ return {ctx, DiagLevel::Warn};
+}
+COFFSyncStream coff::Err(COFFLinkerContext &ctx) {
+ return {ctx, DiagLevel::Err};
+}
+COFFSyncStream coff::Fatal(COFFLinkerContext &ctx) {
+ return {ctx, DiagLevel::Fatal};
+}
+uint64_t coff::errCount(COFFLinkerContext &ctx) { return ctx.e.errorCount; }
+
namespace lld::coff {
bool link(ArrayRef<const char *> args, llvm::raw_ostream &stdoutOS,
@@ -75,7 +97,7 @@ bool link(ArrayRef<const char *> args, llvm::raw_ostream &stdoutOS,
ctx->driver.linkerMain(args);
- return errorCount() == 0;
+ return errCount(*ctx) == 0;
}
// Parse options of the form "old;new".
@@ -212,7 +234,8 @@ void LinkerDriver::addBuffer(std::unique_ptr<MemoryBuffer> mb,
ctx.symtab.addFile(make<PDBInputFile>(ctx, mbref));
break;
case file_magic::coff_cl_gl_object:
- error(filename + ": is not a native COFF file. Recompile without /GL");
+ Err(ctx) << filename
+ << ": is not a native COFF file. Recompile without /GL";
break;
case file_magic::pecoff_executable:
if (ctx.config.mingw) {
@@ -302,7 +325,7 @@ void LinkerDriver::addArchiveBuffer(MemoryBufferRef mb, StringRef symName,
obj->parentName = parentName;
ctx.symtab.addFile(obj);
- log("Loaded " + toString(obj) + " for " + symName);
+ Log(ctx) << "Loaded " << obj << " for " << symName;
}
void LinkerDriver::enqueueArchiveMember(const Archive::Child &c,
@@ -310,9 +333,9 @@ void LinkerDriver::enqueueArchiveMember(const Archive::Child &c,
StringRef parentName) {
auto reportBufferError = [=](Error &&e, StringRef childName) {
- fatal("could not get the buffer for the member defining symbol " +
- toCOFFString(ctx, sym) + ": " + parentName + "(" + childName +
- "): " + toString(std::move(e)));
+ Fatal(ctx) << "could not get the buffer for the member defining symbol "
+ << &sym << ": " << parentName << "(" << childName
+ << "): " << std::move(e);
};
if (!c.getParent()->isThin()) {
@@ -361,7 +384,7 @@ void LinkerDriver::parseDirectives(InputFile *file) {
if (s.empty())
return;
- log("Directives: " + toString(file) + ": " + s);
+ Log(ctx) << "Directives: " << file << ": " << s;
ArgParser parser(ctx);
// .drectve is always tokenized using Windows shell rules.
@@ -414,7 +437,7 @@ void LinkerDriver::parseDirectives(InputFile *file) {
break;
case OPT_entry:
if (!arg->getValue()[0])
- fatal("missing entry point symbol name");
+ Fatal(ctx) << "missing entry point symbol name";
ctx.config.entry = addUndefined(mangle(arg->getValue()), true);
break;
case OPT_failifmismatch:
@@ -779,7 +802,7 @@ StringRef LinkerDriver::findDefaultEntry() {
if (findUnderscoreMangle("wWinMain")) {
if (!findUnderscoreMangle("WinMain"))
return mangle("wWinMainCRTStartup");
- warn("found both wWinMain and WinMain; using latter");
+ Warn(ctx) << "found both wWinMain and WinMain; using latter";
}
return mangle("WinMainCRTStartup");
}
@@ -2200,7 +2223,7 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
config->incremental = false;
}
- if (errorCount())
+ if (errCount(ctx))
return;
std::set<sys::fs::UniqueID> wholeArchives;
@@ -2279,7 +2302,7 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
stream << " " << path << "\n";
}
- message(buffer);
+ Msg(ctx) << buffer;
}
// Process files specified as /defaultlib. These must be processed after
diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp
index 6b5efb34b3f3e7..9e33774d695fab 100644
--- a/lld/COFF/InputFiles.cpp
+++ b/lld/COFF/InputFiles.cpp
@@ -70,6 +70,11 @@ std::string lld::toString(const coff::InputFile *file) {
.str();
}
+const COFFSyncStream &coff::operator<<(const COFFSyncStream &s,
+ const InputFile *f) {
+ return s << toString(f);
+}
+
/// Checks that Source is compatible with being a weak alias to Target.
/// If Source is Undefined and has no weak alias set, makes it a weak
/// alias to Target.
diff --git a/lld/COFF/InputFiles.h b/lld/COFF/InputFiles.h
index 77f7e298166eec..e727d1376e2f2d 100644
--- a/lld/COFF/InputFiles.h
+++ b/lld/COFF/InputFiles.h
@@ -40,6 +40,8 @@ class DWARFCache;
namespace coff {
class COFFLinkerContext;
+const COFFSyncStream &operator<<(const COFFSyncStream &, const InputFile *);
+
std::vector<MemoryBufferRef> getArchiveMembers(llvm::object::Archive *file);
using llvm::COFF::IMAGE_FILE_MACHINE_UNKNOWN;
diff --git a/lld/COFF/Symbols.cpp b/lld/COFF/Symbols.cpp
index f2fa2392ecbbcd..383f62afd8e1d3 100644
--- a/lld/COFF/Symbols.cpp
+++ b/lld/COFF/Symbols.cpp
@@ -53,6 +53,13 @@ std::string toCOFFString(const COFFLinkerContext &ctx,
return maybeDemangleSymbol(ctx, b.getName());
}
+const COFFSyncStream &
+coff::operator<<(const COFFSyncStream &s,
+ const llvm::object::Archive::Symbol *sym) {
+ s << maybeDemangleSymbol(s.ctx, sym->getName());
+ return s;
+}
+
namespace coff {
void Symbol::computeName() {
diff --git a/lld/COFF/Symbols.h b/lld/COFF/Symbols.h
index 203a542466c68e..6fabed9fc8f2b3 100644
--- a/lld/COFF/Symbols.h
+++ b/lld/COFF/Symbols.h
@@ -35,6 +35,9 @@ class InputFile;
class ObjFile;
class SymbolTable;
+const COFFSyncStream &operator<<(const COFFSyncStream &,
+ const llvm::object::Archive::Symbol *);
+
// The base class for real symbol classes.
class Symbol {
public:
|
aganea
approved these changes
Dec 3, 2024
Member
aganea
left a comment
aganea
left a comment
There was a problem hiding this comment.
Looks much better, thanks for doing this!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Similar to #112319 for ELF. While there is some initial boilerplate, it can simplify some call sites that use Twine, especially when a printed element uses
ctxor toString.