[clang] Multilib error fixes (PR #110804)
Simon Tatham via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 2 01:51:28 PDT 2024
https://github.com/statham-arm updated https://github.com/llvm/llvm-project/pull/110804
>From 531253ab8c33cc69a927b28a1608675cd9ef709c Mon Sep 17 00:00:00 2001
From: Simon Tatham <simon.tatham at arm.com>
Date: Mon, 30 Sep 2024 16:12:00 +0100
Subject: [PATCH 1/2] [clang][Driver] Rename "FatalError" key to "Error" in
multilib.yaml.
This is a late-breaking change to #105684, suggested after the
original patch was already landed.
---
clang/docs/Multilib.rst | 4 +--
clang/include/clang/Driver/Multilib.h | 8 +++---
clang/lib/Driver/Driver.cpp | 2 +-
clang/lib/Driver/Multilib.cpp | 25 +++++++++----------
clang/lib/Driver/ToolChains/BareMetal.cpp | 2 +-
.../baremetal-multilib-custom-error.yaml | 2 +-
clang/unittests/Driver/MultilibTest.cpp | 2 +-
7 files changed, 22 insertions(+), 23 deletions(-)
diff --git a/clang/docs/Multilib.rst b/clang/docs/Multilib.rst
index 6d77fda3623b20..7637d0db9565b8 100644
--- a/clang/docs/Multilib.rst
+++ b/clang/docs/Multilib.rst
@@ -202,8 +202,8 @@ For a more comprehensive example see
# If there is no multilib available for a particular set of flags, and the
# other multilibs are not adequate fallbacks, then you can define a variant
- # record with a FatalError key in place of the Dir key.
- - FatalError: this multilib collection has no hard-float ABI support
+ # record with an Error key in place of the Dir key.
+ - Error: this multilib collection has no hard-float ABI support
Flags: [--target=thumbv7m-none-eabi, -mfloat-abi=hard]
diff --git a/clang/include/clang/Driver/Multilib.h b/clang/include/clang/Driver/Multilib.h
index 1a79417111eece..dbed70f4f9008f 100644
--- a/clang/include/clang/Driver/Multilib.h
+++ b/clang/include/clang/Driver/Multilib.h
@@ -54,7 +54,7 @@ class Multilib {
// Some Multilib objects don't actually represent library directories you can
// select. Instead, they represent failures of multilib selection, of the
// form 'Sorry, we don't have any library compatible with these constraints'.
- std::optional<std::string> FatalError;
+ std::optional<std::string> Error;
public:
/// GCCSuffix, OSSuffix & IncludeSuffix will be appended directly to the
@@ -63,7 +63,7 @@ class Multilib {
Multilib(StringRef GCCSuffix = {}, StringRef OSSuffix = {},
StringRef IncludeSuffix = {}, const flags_list &Flags = flags_list(),
StringRef ExclusiveGroup = {},
- std::optional<StringRef> FatalError = std::nullopt);
+ std::optional<StringRef> Error = std::nullopt);
/// Get the detected GCC installation path suffix for the multi-arch
/// target variant. Always starts with a '/', unless empty
@@ -94,9 +94,9 @@ class Multilib {
bool operator==(const Multilib &Other) const;
- bool isFatalError() const { return FatalError.has_value(); }
+ bool isError() const { return Error.has_value(); }
- const std::string &getFatalError() const { return FatalError.value(); }
+ const std::string &getErrorMessage() const { return Error.value(); }
};
raw_ostream &operator<<(raw_ostream &OS, const Multilib &M);
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index fba6a8853c3960..b8536a706a8fa2 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -2314,7 +2314,7 @@ bool Driver::HandleImmediateArgs(Compilation &C) {
if (C.getArgs().hasArg(options::OPT_print_multi_lib)) {
for (const Multilib &Multilib : TC.getMultilibs())
- if (!Multilib.isFatalError())
+ if (!Multilib.isError())
llvm::outs() << Multilib << "\n";
return false;
}
diff --git a/clang/lib/Driver/Multilib.cpp b/clang/lib/Driver/Multilib.cpp
index e8a27fe9de473a..cc34794b7dac42 100644
--- a/clang/lib/Driver/Multilib.cpp
+++ b/clang/lib/Driver/Multilib.cpp
@@ -32,10 +32,9 @@ using namespace llvm::sys;
Multilib::Multilib(StringRef GCCSuffix, StringRef OSSuffix,
StringRef IncludeSuffix, const flags_list &Flags,
- StringRef ExclusiveGroup,
- std::optional<StringRef> FatalError)
+ StringRef ExclusiveGroup, std::optional<StringRef> Error)
: GCCSuffix(GCCSuffix), OSSuffix(OSSuffix), IncludeSuffix(IncludeSuffix),
- Flags(Flags), ExclusiveGroup(ExclusiveGroup), FatalError(FatalError) {
+ Flags(Flags), ExclusiveGroup(ExclusiveGroup), Error(Error) {
assert(GCCSuffix.empty() ||
(StringRef(GCCSuffix).front() == '/' && GCCSuffix.size() > 1));
assert(OSSuffix.empty() ||
@@ -126,8 +125,8 @@ bool MultilibSet::select(const Driver &D, const Multilib::flags_list &Flags,
// If this multilib is actually a placeholder containing a fatal
// error message written by the multilib.yaml author, display that
// error message, and return failure.
- if (M.isFatalError()) {
- D.Diag(clang::diag::err_drv_multilib_custom_error) << M.getFatalError();
+ if (M.isError()) {
+ D.Diag(clang::diag::err_drv_multilib_custom_error) << M.getErrorMessage();
return false;
}
@@ -173,7 +172,7 @@ static const VersionTuple MultilibVersionCurrent(1, 0);
struct MultilibSerialization {
std::string Dir; // if this record successfully selects a library dir
- std::string FatalError; // if this record reports a fatal error message
+ std::string Error; // if this record reports a fatal error message
std::vector<std::string> Flags;
std::string Group;
};
@@ -217,15 +216,15 @@ struct MultilibSetSerialization {
template <> struct llvm::yaml::MappingTraits<MultilibSerialization> {
static void mapping(llvm::yaml::IO &io, MultilibSerialization &V) {
io.mapOptional("Dir", V.Dir);
- io.mapOptional("FatalError", V.FatalError);
+ io.mapOptional("Error", V.Error);
io.mapRequired("Flags", V.Flags);
io.mapOptional("Group", V.Group);
}
static std::string validate(IO &io, MultilibSerialization &V) {
- if (V.Dir.empty() && V.FatalError.empty())
- return "one of the 'Dir' and 'FatalError' keys must be specified";
- if (!V.Dir.empty() && !V.FatalError.empty())
- return "the 'Dir' and 'FatalError' keys may not both be specified";
+ if (V.Dir.empty() && V.Error.empty())
+ return "one of the 'Dir' and 'atalError' keys must be specified";
+ if (!V.Dir.empty() && !V.Error.empty())
+ return "the 'Dir' and 'Error' keys may not both be specified";
if (StringRef(V.Dir).starts_with("/"))
return "paths must be relative but \"" + V.Dir + "\" starts with \"/\"";
return std::string{};
@@ -311,8 +310,8 @@ MultilibSet::parseYaml(llvm::MemoryBufferRef Input,
multilib_list Multilibs;
Multilibs.reserve(MS.Multilibs.size());
for (const auto &M : MS.Multilibs) {
- if (!M.FatalError.empty()) {
- Multilibs.emplace_back("", "", "", M.Flags, M.Group, M.FatalError);
+ if (!M.Error.empty()) {
+ Multilibs.emplace_back("", "", "", M.Flags, M.Group, M.Error);
} else {
std::string Dir;
if (M.Dir != ".")
diff --git a/clang/lib/Driver/ToolChains/BareMetal.cpp b/clang/lib/Driver/ToolChains/BareMetal.cpp
index 8aed9ed6e18817..d5c70f02b5e752 100644
--- a/clang/lib/Driver/ToolChains/BareMetal.cpp
+++ b/clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -187,7 +187,7 @@ static void findMultilibsFromYAML(const ToolChain &TC, const Driver &D,
D.Diag(clang::diag::warn_drv_missing_multilib) << llvm::join(Flags, " ");
std::stringstream ss;
for (const Multilib &Multilib : Result.Multilibs)
- if (!Multilib.isFatalError())
+ if (!Multilib.isError())
ss << "\n" << llvm::join(Multilib.flags(), " ");
D.Diag(clang::diag::note_drv_available_multilibs) << ss.str();
}
diff --git a/clang/test/Driver/baremetal-multilib-custom-error.yaml b/clang/test/Driver/baremetal-multilib-custom-error.yaml
index c006bb4072ce2f..761e595757f346 100644
--- a/clang/test/Driver/baremetal-multilib-custom-error.yaml
+++ b/clang/test/Driver/baremetal-multilib-custom-error.yaml
@@ -44,7 +44,7 @@ Variants:
Flags:
- --target=thumbv8.1m.main-unknown-none-eabi
-- FatalError: mve-softfloat is not supported
+- Error: mve-softfloat is not supported
Flags:
- --target=thumbv8.1m.main-unknown-none-eabi
- -march=thumbv8.1m.main+mve
diff --git a/clang/unittests/Driver/MultilibTest.cpp b/clang/unittests/Driver/MultilibTest.cpp
index dfeef7c2077c72..c03e117d993045 100644
--- a/clang/unittests/Driver/MultilibTest.cpp
+++ b/clang/unittests/Driver/MultilibTest.cpp
@@ -282,7 +282,7 @@ Variants: []
)"));
EXPECT_TRUE(
StringRef(Diagnostic)
- .contains("one of the 'Dir' and 'FatalError' keys must be specified"))
+ .contains("one of the 'Dir' and 'Error' keys must be specified"))
<< Diagnostic;
EXPECT_FALSE(parseYaml(MS, Diagnostic, YAML_PREAMBLE R"(
>From 9401c95c5183b460f0bdff92f65e14132191ae51 Mon Sep 17 00:00:00 2001
From: Simon Tatham <simon.tatham at arm.com>
Date: Wed, 2 Oct 2024 09:26:02 +0100
Subject: [PATCH 2/2] [clang][Driver] Improve multilib custom error reporting.
If `multilib.yaml` reports a custom error message for some unsupported
configuration, it's not very helpful to display that error message
_first_, and then follow it up with a huge list of all the multilib
configurations that _are_ supported.
In interactive use, the list is likely to scroll the most important
message off the top of the user's window, leaving them with just a
long list of available libraries, without a visible explanation of
_why_ clang just printed that long list. Also, in general, it makes
more intuitive sense to print the message last that shows why
compilation can't continue, because that's where users are most likely
to look for the reason why something stopped.
---
clang/lib/Driver/Multilib.cpp | 14 +++++++-------
clang/lib/Driver/ToolChains/BareMetal.cpp | 17 +++++++++++++++++
2 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/clang/lib/Driver/Multilib.cpp b/clang/lib/Driver/Multilib.cpp
index cc34794b7dac42..9d5cdde98c7972 100644
--- a/clang/lib/Driver/Multilib.cpp
+++ b/clang/lib/Driver/Multilib.cpp
@@ -99,6 +99,7 @@ bool MultilibSet::select(const Driver &D, const Multilib::flags_list &Flags,
llvm::SmallVectorImpl<Multilib> &Selected) const {
llvm::StringSet<> FlagSet(expandFlags(Flags));
Selected.clear();
+ bool AnyErrors = false;
// Decide which multilibs we're going to select at all.
llvm::DenseSet<StringRef> ExclusiveGroupsSelected;
@@ -123,12 +124,11 @@ bool MultilibSet::select(const Driver &D, const Multilib::flags_list &Flags,
}
// If this multilib is actually a placeholder containing a fatal
- // error message written by the multilib.yaml author, display that
- // error message, and return failure.
- if (M.isError()) {
- D.Diag(clang::diag::err_drv_multilib_custom_error) << M.getErrorMessage();
- return false;
- }
+ // error message written by the multilib.yaml author, then set a
+ // flag that will cause a failure return. Our caller will display
+ // the error message.
+ if (M.isError())
+ AnyErrors = true;
// Select this multilib.
Selected.push_back(M);
@@ -138,7 +138,7 @@ bool MultilibSet::select(const Driver &D, const Multilib::flags_list &Flags,
// round.
std::reverse(Selected.begin(), Selected.end());
- return !Selected.empty();
+ return !AnyErrors && !Selected.empty();
}
llvm::StringSet<>
diff --git a/clang/lib/Driver/ToolChains/BareMetal.cpp b/clang/lib/Driver/ToolChains/BareMetal.cpp
index d5c70f02b5e752..7e4cbe6b9b6561 100644
--- a/clang/lib/Driver/ToolChains/BareMetal.cpp
+++ b/clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -186,10 +186,27 @@ static void findMultilibsFromYAML(const ToolChain &TC, const Driver &D,
return;
D.Diag(clang::diag::warn_drv_missing_multilib) << llvm::join(Flags, " ");
std::stringstream ss;
+
+ // If multilib selection didn't complete successfully, report a list
+ // of all the configurations the user could have provided.
for (const Multilib &Multilib : Result.Multilibs)
if (!Multilib.isError())
ss << "\n" << llvm::join(Multilib.flags(), " ");
D.Diag(clang::diag::note_drv_available_multilibs) << ss.str();
+
+ // Now report any custom error messages requested by the YAML. We do
+ // this after displaying the list of available multilibs, because
+ // that list is probably large, and (in interactive use) risks
+ // scrolling the useful error message off the top of the user's
+ // terminal.
+ for (const Multilib &Multilib : Result.SelectedMultilibs)
+ if (Multilib.isError())
+ D.Diag(clang::diag::err_drv_multilib_custom_error)
+ << Multilib.getErrorMessage();
+
+ // If there was an error, clear the SelectedMultilibs vector, in
+ // case it contains partial data.
+ Result.SelectedMultilibs.clear();
}
static constexpr llvm::StringLiteral MultilibFilename = "multilib.yaml";
More information about the cfe-commits
mailing list