[PATCH] D24933: Enable configuration files in clang

Hans Wennborg via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 16 11:44:34 PST 2016


hans added a subscriber: srhines.
hans added a comment.

I'm still skeptical, but I think this is an improvement over the previous patch.

I think your best bet to get this landed is to find someone from the cfe-dev thread who is in favour of config files and have them review it.

I'm also cc'ing srhines who might be interested in this since he works on the Android toolchains and this patch has configuring cross-compilers as one of its motivations.



================
Comment at: docs/UsersManual.rst:667
+extension `cfg` if it is not specified yet, and the obtained file name is searched
+for in the directories: `~/.llvm` and the directory where clang executable resides.
+The first found file is used. It is an error if the required file cannot be found.
----------------
The "add .cfg extension" magic seems a little awkward. It seems this is mixing the possibility of taking a filename and taking some other name.

For `clang --config myfile.cfg`, should it also search the current working directory?

I'm not keen on it searching in `~/.llvm`.


================
Comment at: docs/UsersManual.rst:694
+the including file. For instance if a config file `~/.llvm/target.cfg` contains
+directive `os/linux.opts`, the file `linux.opts` is searched for in the directory
+`~/.llvm/os`.
----------------
I guess you mean directive `@os/linux.opts`?


================
Comment at: test/Driver/config-file.c:10
+
+// RUN: not %clang --config inexistent.cfg -c %s -### 2>&1 | FileCheck %s -check-prefix CHECK-INEX-NOSEARCH
+// CHECK-INEX-NOSEARCH: Configuration {{.*}}inexistent.cfg' specified by option '--config' cannot be found in directories:
----------------
This test will fail if I have a `~/.llvm/inexistent.cfg` file on my machine.


================
Comment at: test/Driver/config-file.c:13
+// CHECK-INEX-NOSEARCH: ~/.llvm
+// CHECK-INEX-NOSEARCH: /etc/llvm
+
----------------
Where did `/etc/llvm` come from?


================
Comment at: test/Driver/config-file.c:14
+// CHECK-INEX-NOSEARCH: /etc/llvm
+
+// RUN: %clang --config %S/Inputs/config-2.cfg -### 2>&1 | FileCheck %s -check-prefix CHECK-HHH-NOFILE
----------------
There is no test checking the functionality that finds a config file based on the clang executable name.


https://reviews.llvm.org/D24933





More information about the cfe-commits mailing list