C_INCLUDE_DIRS feature is messed up

[pogo59]

When doing some driver work a little while ago, I bumped into code handling C_INCLUDE_DIRS, which appears to be replicated across many driver toolchain classes. My first thought was to refactor all those duplicates into a common utility function, but on closer inspection they aren't all the same.

First off, C_INCLUDE_DIRS is a CMake variable that is a PATH-like string that lets you specify system include directories; the driver handling typically skips adding $sysroot/include (and maybe others, varies with target) and use what's in the C_INCLUDE_DIRS string instead. This appears to be legacy going back to the Autoconf days, with references to configure --with-c-include-dirs popping up. I have no idea whether anyone actually uses this.

Even so, there are issues.

1. Some targets prepend the sysroot to absolute paths from C_INCLUDE_DIRS, and leave relative paths along. Other targets do it the other way around. (This inconsistency stopped me from doing the refactor. The code for all targets is otherwise identical, except for the name of the variable containing the sysroot to use.)
2. Not all targets support this. I removed PS4/PS5 support it because it makes no sense for us (and it existed in the first place only because I was cargo-culting other targets). I don't know what the story is for other targets.
3. The separator is hard-coded to be a colon, which totally fails on a Windows hosted cross compiler. (Windows targets do not support C_INCLUDE_DIRS at all.) It should use llvm::sys::EnvPathSeparator instead. But, that means updating all the 8-10 places that do this bit of code, because factoring into a utility routine would change behavior (see item 1).

I know too little about how different environments and distros handle where they put things, so I have no opinion about what if anything should be done here. But item 1 in particular seems very weird. Personally I'd be inclined to rip it all out, but maybe it's useful to someone.

Tagging @MaskRay as the current Driver owner, and @echristo as the former Driver and Autoconf-stuff owner.

[llvmbot]

@llvm/issue-subscribers-clang-driver

[tstellar]

I would be in favor of removing all of the C_INCLUDE_DIRS usage. Recently, we've been encouraging distributors to use the config files for these kind of customization rather than CMake variables.

[brad0]

@MaskRay @echristo Ping.

[MaskRay]

Personally I'd be inclined to rip it all out, but maybe it's useful to someone.

+1. C_INCLUDE_DIRS is not recognized by GCC.

C_INCLUDE_DIRS is a popular substring. If I use https://sourcegraph.com/search?q=context:global+%5CbC_INCLUDE_DIRS%3D+-f:clang/&patternType=regexp&case=yes&sm=1&groupBy=repo to find the use cases, one project has a use like add_opt C_INCLUDE_DIRS=$C_INCLUDES, but the search patch seems likely redundant.

[brad0]

I posted a diff here for review to rip it all out.. https://reviews.llvm.org/D159054