[PATCH] D24933: Enable configuration files in clang

Michał Górny via cfe-commits cfe-commits at lists.llvm.org
Sun Oct 2 09:30:55 PDT 2016


mgorny requested changes to this revision.
mgorny added a reviewer: mgorny.
mgorny added a comment.
This revision now requires changes to proceed.

Few minor nits. However, I'd like to say that I like the idea in general and see forward to deploy it in Gentoo.

One use case which doesn't seem to have been mentioned yes is setting the default -rtlib= and -stdlib=. Currently we're only able to set the defaults at compile-time but it generally sucks to rebuild the compiler to change the default. With this, we'd be able to set them via the system config file ;-).



> driver.cpp:334
> +  // Try reading options from configuration file.
> +  static const char * const SearchDirs[] = { "~/.llvm", "/etc/llvm" };
> +  llvm::SmallString<128> ConfigFile;

1. I'm not sure if others would agree with me but I think it would be better to move those default paths straight to LLVM. If others tools are to adopt those configuration files, it would only be reasonable to use the same search paths consistently and not repeat them in every tool.

2. I think the `/etc` prefix should be made configurable via CMake options. One case that would benefit from this is Gentoo Prefix where the Gentoo system root is located in a subdirectory of /.

> driver.cpp:336
> +  llvm::SmallString<128> ConfigFile;
> +  auto SRes = llvm::cl::findConfigFile(ConfigFile, argv, SearchDirs, "clang");
> +  llvm::cl::reportConfigFileSearchError(SRes, ConfigFile, SearchDirs, ProgName);

1. You need to update this, following your update on https://reviews.llvm.org/D24926.
2. I think you need to use `clang.cfg` here.

https://reviews.llvm.org/D24933





More information about the cfe-commits mailing list