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