[PATCH] D135791: [Clang] Do not crash when an invalid offload architecture is set

[Artem Belevich via Phabricator via cfe-commits]

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