Multilib error fixes

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]
 
 

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);

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;
   }

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() ||
@@ -100,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,13 +123,11 @@ bool MultilibSet::select(const Driver &D, const Multilib::flags_list &Flags,
         continue;
     }
 
-    // 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();
-      return false;
-    }
+    // If this multilib is actually a placeholder containing an 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);
@@ -139,7 +137,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<>
@@ -173,7 +171,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 +215,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 +309,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 != ".")

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.isFatalError())
+    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";

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

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"(