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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
