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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
