[clang] [Driver] Fix detection of libc++ with empty sysroot. (PR #66947)

Shoaib Meenai via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 20 14:06:25 PDT 2023


smeenai wrote:

> Thanks for the quick response!
> 
> > The `sys::path::append` behavior here is surprising. I think it'd be better to change the computation of `SysRoot` in the function above (line 3102, which I can't comment because of GitHub, sigh) to default to `llvm::sys::path::get_separator()` if it's empty, to handle Windows slashes as well as be less confusing to read.
> 
> That sounds sensible to me in principle, but not trivial: presumably the overrides of `Toolchain::getSysroot()` should have a consistent contract. There are 6 overrides, where some explicitly return the empty string, and 25 callers some of which do things like `computeSysRoot() + "/include/c++/"` (RISCVToolchain). I feel it's likely I'll break something on a platform I don't understand before getting back to a good state :-)

I just meant to change https://github.com/llvm/llvm-project/blob/5eaa5312e7943e23155da4f0fbf07b55a200fc60/clang/lib/Driver/ToolChains/Gnu.cpp#L3102 to something like (untested):

```
std::string SysRoot = computeSysRoot();
if (SysRoot.empty())
  SysRoot = llvm::sys::path::get_separator();
```

It's a local fix, but all the code here is pretty inconsistent anyway, so I don't feel too bad about it.

https://github.com/llvm/llvm-project/pull/66947


More information about the cfe-commits mailing list