sam-mccall 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 :-) > This is tricky to test because it requires no sysroot to be passed, but we > might be able to do so with a VFS overlay. I'm okay adding the test > afterwards though so we get the breakage fixed first, and I can also look > into the test myself if you'd like. I can try to add something to the unittest. Looks like in the current form I also need to change the lit test back to hard-coding `/` for this one separator. (I'm out of time tonight but will pick it up tomorrow) https://github.com/llvm/llvm-project/pull/66947 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
