tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.
LGTM with few nits.
================
Comment at: clang/lib/Driver/Driver.cpp:4191
/// or CUDA architecture.
-static StringRef getCanonicalArchString(Compilation &C,
- const llvm::opt::DerivedArgList &Args,
- StringRef ArchStr,
- const llvm::Triple &Triple) {
+static Optional<StringRef>
+getCanonicalArchString(Compilation &C, const llvm::opt::DerivedArgList &Args,
----------------
Nit. Optional is fine, but we could've just checked returned value for
Arch.empty().
================
Comment at: clang/lib/Driver/Driver.cpp:4288
for (StringRef Arch : llvm::split(Arg->getValue(), ",")) {
- if (Arch == StringRef("all"))
+ if (Arch == StringRef("all")) {
Archs.clear();
----------------
Nit: do we need explicit `StringRef()` here? If implicit type conversion does
not work, we could use `Arch.equals("all")`.
The current code is fine, but it does stick out a bit and makes me wonder if
there's something interesting going on there.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135791/new/
https://reviews.llvm.org/D135791
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
