[PATCH] D143059: [NFC] Enable selecting multiple multilibs

[Peter Smith via Phabricator via cfe-commits]

peter.smith added a comment.

A couple of small suggestions but otherwise looks good to me.



================
Comment at: clang/lib/Driver/ToolChains/BareMetal.cpp:233
+  std::string Result = computeBaseSysRoot(getDriver(), getTriple());
+  if (!SelectedMultilibs.empty()) {
+    Result += SelectedMultilibs.back().osSuffix();
----------------
Nit: the braces can be omitted here. No strong opinion as it may be better to keep consistency with the rest of the file rather than LLVM as a whole.

https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements


================
Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:319
+    SelectedMultilibs.erase(SelectedMultilibs.begin(),
+                            SelectedMultilibs.end() - 1);
+
----------------
SelectedMultilibs.end() - 1 makes me a little nervous. This will work for the current container type (I think the standard requires it for vector and I don't think SmallVector would deviate from it. However in the unlikely event that the container changes and this isn't valid then this could break.

Perhaps use std::advance(SelectedMultilibs.end(), -1) which is more likely to break at compile time if this occurs.

Alternatively if this is just erasing all but the last element, maybe extract it, clear the container and reinsert.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143059/new/

https://reviews.llvm.org/D143059