[PATCH] D25669: [Driver] Simplify ToolChain::GetCXXStdlibType (NFC)

[Jonas Hahnfeld via cfe-commits]

Hahnfeld added inline comments.


================
Comment at: lib/Driver/ToolChain.cpp:553-559
+  // "platform" is only used in tests to override CLANG_DEFAULT_CXX_STDLIB
+  if (LibName == "libc++")
+    return ToolChain::CST_Libcxx;
+  else if (LibName == "libstdc++")
+    return ToolChain::CST_Libstdcxx;
+  else if (LibName == "platform")
+    return GetDefaultCXXStdlibType();
----------------
ABataev wrote:
> Hahnfeld wrote:
> > mgorny wrote:
> > > ABataev wrote:
> > > > I believe you can use StringSwitch here
> > > I think you are changing the meaning of this comment. The original sounded like a request not to use this type anywhere else. Your version only explains where it's used (right now).
> > I'm uncertain how to apply it while also preserving the `Diag` for an invalid argument: I would probably need an additional `CST_Unknown` which I don't like more...
> I think you can try something like this:
> ```
> llvm::Optional<ToolChain::CXXStdlibType> Res = StringSwitch<ToolChain::CXXStdlibType, llvm::NoneType>(LibName).Case("libc++", ToolChain::CST_Libcxx).Case("libstdc++", ToolChain::CST_Libstdcxx).Case("platform", GetDefaultCXXStdlibType()).Default(llvm::None);
> if (Res)
>   return Res.getValue();
> ```
Does that really improve readability? `StringSwitch` is currently never used in `ToolChain.cpp` and the cascade with `if`s is one line shorter in this case...


https://reviews.llvm.org/D25669