[clang] 5a9e93f - [clang][Driver] Improve multilib custom error reporting (#110804)

[Simon Tatham via cfe-commits]

Author: Simon Tatham
Date: 2024-10-07T09:32:12+01:00
New Revision: 5a9e93f39cc78276a12852bbc4d639689fe73d5a

URL: https://github.com/llvm/llvm-project/commit/5a9e93f39cc78276a12852bbc4d639689fe73d5a
DIFF: https://github.com/llvm/llvm-project/commit/5a9e93f39cc78276a12852bbc4d639689fe73d5a.diff

LOG: [clang][Driver] Improve multilib custom error reporting (#110804)

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.

Added: 
    

Modified: 
    clang/lib/Driver/Multilib.cpp
    clang/lib/Driver/ToolChains/BareMetal.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Driver/Multilib.cpp b/clang/lib/Driver/Multilib.cpp
index 44bed82ee36bd4..c56417c6f6d0b0 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,10 @@ bool MultilibSet::select(const Driver &D, const Multilib::flags_list &Flags,
     }
 
     // If this multilib is actually a placeholder containing an 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;
-    }
+    // 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 +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<>

diff  --git a/clang/lib/Driver/ToolChains/BareMetal.cpp b/clang/lib/Driver/ToolChains/BareMetal.cpp
index 0094f575507637..f9a73f60973e4c 100644
--- a/clang/lib/Driver/ToolChains/BareMetal.cpp
+++ b/clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -200,10 +200,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";