Page MenuHomePhabricator

[clang] [Driver] Fall back to default.cfg when calling clang w/o prefix
AbandonedPublic

Authored by mgorny on Sep 10 2021, 11:45 AM.

Details

Summary

Complement the configuration file search logic to use
default-<mode-suffix>.cfg or default.cfg if clang is called
via the executable without a prefix specifying target. This makes it
possible for vendors to supply a full set of configuration files
matching all installed executables.

Diff Detail

Event Timeline

asymptotically requested review of this revision.Sep 10 2021, 11:45 AM
asymptotically created this revision.
asymptotically edited the summary of this revision. (Show Details)Sep 10 2021, 11:47 AM
Arfrever added inline comments.
clang/docs/UsersManual.rst
877

This says armv7l.cfg...

clang/lib/Driver/Driver.cpp
906

But this says armv7l-clang.cfg.
Could you also fix this inconsistency?

Updated users' manual following Arfrever's feedback.

Herald added a project: Restricted Project. · View Herald TranscriptJun 18 2022, 12:50 AM
Herald added a subscriber: MaskRay. · View Herald Transcript
mgorny added a subscriber: mgorny.Wed, Sep 14, 3:19 AM

As @sepavloff points out in the linked discussion, we probably need an option to disable loading default configuration files too, and we should use it in the test suite (via modifying the %clang substitutions, I guess), as otherwise someone enabling config file support is going to see random test failures.

On this topic, it would be great if we could pick up a per-target default config file too, if clang is invoked with clang -target <triple>. Currently this is done automatically if clang is invoked as <triple>-clang, but not with an explicit -target parameter.

mgorny commandeered this revision.Fri, Sep 16, 10:19 AM
mgorny added a reviewer: asymptotically.

After discussing privately with @asymptotically, I'm going to try to finish this.

mgorny planned changes to this revision.Fri, Sep 16, 10:20 AM
mgorny added a reviewer: MaskRay.

So far my plan is to go for default-{ModeSuffix}.cfg instead (e.g. default-clang.cfg, default-clang++.cfg, etc.). If I'm not mistaken, this will trigger the existing fallback logic stripping mode suffix and trying default.cfg if suffixed config is not found. Also add tests ;-).

MaskRay added inline comments.Fri, Sep 16, 1:07 PM
clang/docs/UsersManual.rst
876–877

These single backsticks probably should be double backsticks.

MaskRay added a subscriber: joerg.Fri, Sep 16, 1:19 PM

Thanks for taking over the patch!

I've CCed (in https://discourse.llvm.org/t/rfc-adding-a-default-file-location-to-config-file-support/63606/33) some folks who had strong opinions in the initial support patch.
(Cc @joerg manually whom I cannot find a discourse username for)

mgorny updated this revision to Diff 460877.Fri, Sep 16, 1:25 PM
mgorny retitled this revision from [clang][Driver] Default to loading clang.cfg if config file not specified to [clang] [Driver] Fall back to default.cfg when calling clang w/o prefix.
mgorny edited the summary of this revision. (Show Details)

Changed the base config filename to default.cfg, and included looking up default-<mode-suffix>.cfg first. Added tests.

clang/docs/UsersManual.rst
876–877

To be honest, I was wondering why it was done like that.

As an example, we are currently installing the following set of executables + symlinks for clang in Gentoo on amd64 with full multilib:

lrwxrwxrwx 1 root root    8 Sep 17 12:31 /usr/lib/llvm/16/bin/clang -> clang-16
lrwxrwxrwx 1 root root   10 Sep 17 12:31 /usr/lib/llvm/16/bin/clang++ -> clang++-16
lrwxrwxrwx 1 root root    8 Sep 17 12:31 /usr/lib/llvm/16/bin/clang++-16 -> clang-16
-rwxr-xr-x 1 root root 170K Sep 17 12:31 /usr/lib/llvm/16/bin/clang-16
lrwxrwxrwx 1 root root   11 Sep 17 12:31 /usr/lib/llvm/16/bin/clang-cl -> clang-cl-16
lrwxrwxrwx 1 root root    8 Sep 17 12:31 /usr/lib/llvm/16/bin/clang-cl-16 -> clang-16
lrwxrwxrwx 1 root root   12 Sep 17 12:31 /usr/lib/llvm/16/bin/clang-cpp -> clang-cpp-16
lrwxrwxrwx 1 root root    8 Sep 17 12:31 /usr/lib/llvm/16/bin/clang-cpp-16 -> clang-16
lrwxrwxrwx 1 root root   26 Sep 17 12:31 /usr/lib/llvm/16/bin/i686-pc-linux-gnu-clang -> i686-pc-linux-gnu-clang-16
lrwxrwxrwx 1 root root   28 Sep 17 12:31 /usr/lib/llvm/16/bin/i686-pc-linux-gnu-clang++ -> i686-pc-linux-gnu-clang++-16
lrwxrwxrwx 1 root root   10 Sep 17 12:31 /usr/lib/llvm/16/bin/i686-pc-linux-gnu-clang++-16 -> clang++-16
lrwxrwxrwx 1 root root    8 Sep 17 12:31 /usr/lib/llvm/16/bin/i686-pc-linux-gnu-clang-16 -> clang-16
lrwxrwxrwx 1 root root   29 Sep 17 12:31 /usr/lib/llvm/16/bin/i686-pc-linux-gnu-clang-cl -> i686-pc-linux-gnu-clang-cl-16
lrwxrwxrwx 1 root root   11 Sep 17 12:31 /usr/lib/llvm/16/bin/i686-pc-linux-gnu-clang-cl-16 -> clang-cl-16
lrwxrwxrwx 1 root root   30 Sep 17 12:31 /usr/lib/llvm/16/bin/i686-pc-linux-gnu-clang-cpp -> i686-pc-linux-gnu-clang-cpp-16
lrwxrwxrwx 1 root root   12 Sep 17 12:31 /usr/lib/llvm/16/bin/i686-pc-linux-gnu-clang-cpp-16 -> clang-cpp-16
lrwxrwxrwx 1 root root   28 Sep 17 12:31 /usr/lib/llvm/16/bin/x86_64-pc-linux-gnu-clang -> x86_64-pc-linux-gnu-clang-16
lrwxrwxrwx 1 root root   30 Sep 17 12:31 /usr/lib/llvm/16/bin/x86_64-pc-linux-gnu-clang++ -> x86_64-pc-linux-gnu-clang++-16
lrwxrwxrwx 1 root root   10 Sep 17 12:31 /usr/lib/llvm/16/bin/x86_64-pc-linux-gnu-clang++-16 -> clang++-16
lrwxrwxrwx 1 root root    8 Sep 17 12:31 /usr/lib/llvm/16/bin/x86_64-pc-linux-gnu-clang-16 -> clang-16
lrwxrwxrwx 1 root root   31 Sep 17 12:31 /usr/lib/llvm/16/bin/x86_64-pc-linux-gnu-clang-cl -> x86_64-pc-linux-gnu-clang-cl-16
lrwxrwxrwx 1 root root   11 Sep 17 12:31 /usr/lib/llvm/16/bin/x86_64-pc-linux-gnu-clang-cl-16 -> clang-cl-16
lrwxrwxrwx 1 root root   32 Sep 17 12:31 /usr/lib/llvm/16/bin/x86_64-pc-linux-gnu-clang-cpp -> x86_64-pc-linux-gnu-clang-cpp-16
lrwxrwxrwx 1 root root   12 Sep 17 12:31 /usr/lib/llvm/16/bin/x86_64-pc-linux-gnu-clang-cpp-16 -> clang-cpp-16
lrwxrwxrwx 1 root root   31 Sep 17 12:31 /usr/lib/llvm/16/bin/x86_64-pc-linux-gnux32-clang -> x86_64-pc-linux-gnux32-clang-16
lrwxrwxrwx 1 root root   33 Sep 17 12:31 /usr/lib/llvm/16/bin/x86_64-pc-linux-gnux32-clang++ -> x86_64-pc-linux-gnux32-clang++-16
lrwxrwxrwx 1 root root   10 Sep 17 12:31 /usr/lib/llvm/16/bin/x86_64-pc-linux-gnux32-clang++-16 -> clang++-16
lrwxrwxrwx 1 root root    8 Sep 17 12:31 /usr/lib/llvm/16/bin/x86_64-pc-linux-gnux32-clang-16 -> clang-16
lrwxrwxrwx 1 root root   34 Sep 17 12:31 /usr/lib/llvm/16/bin/x86_64-pc-linux-gnux32-clang-cl -> x86_64-pc-linux-gnux32-clang-cl-16
lrwxrwxrwx 1 root root   11 Sep 17 12:31 /usr/lib/llvm/16/bin/x86_64-pc-linux-gnux32-clang-cl-16 -> clang-cl-16
lrwxrwxrwx 1 root root   35 Sep 17 12:31 /usr/lib/llvm/16/bin/x86_64-pc-linux-gnux32-clang-cpp -> x86_64-pc-linux-gnux32-clang-cpp-16
lrwxrwxrwx 1 root root   12 Sep 17 12:31 /usr/lib/llvm/16/bin/x86_64-pc-linux-gnux32-clang-cpp-16 -> clang-cpp-16

…and we can cover all of them using four config files:

-rw-r--r-- 1 root root  0 Sep 17 13:30 default.cfg
lrwxrwxrwx 1 root root 11 Sep 17 12:31 i686-pc-linux-gnu.cfg -> default.cfg
lrwxrwxrwx 1 root root 11 Sep 17 12:31 x86_64-pc-linux-gnu.cfg -> default.cfg
lrwxrwxrwx 1 root root 11 Sep 17 12:31 x86_64-pc-linux-gnux32.cfg -> default.cfg
mgorny updated this revision to Diff 461054.Sun, Sep 18, 1:16 AM
mgorny marked an inline comment as done.

Fix documentation formatting. While at it, fix a doc mistake that referenced x86_64-cl.cfg instead of the correct x86_64-clang-cl.cfg.

sepavloff added inline comments.Mon, Sep 19, 3:01 AM
clang/docs/UsersManual.rst
881

What about using default config based on driver mode (clang.cfg, gcc.cfg etc)? It looks like a more flexible solution, - different driver modes may need different default settings. Even if these setting are identical, the default config files may be links to one, say clang.cfg or just include that file with @file.

mgorny added inline comments.Mon, Sep 19, 3:07 AM
clang/docs/UsersManual.rst
881

It's actually described below — you can have default-clang.cfg, default-clang++.cfg, etc. Using this naming makes it possible to use the same logic for default as for prefixed execs.

On this topic, it would be great if we could pick up a per-target default config file too, if clang is invoked with clang -target <triple>. Currently this is done automatically if clang is invoked as <triple>-clang, but not with an explicit -target parameter.

Would enabling config files in such invocations be a solution for your use case?

On this topic, it would be great if we could pick up a per-target default config file too, if clang is invoked with clang -target <triple>. Currently this is done automatically if clang is invoked as <triple>-clang, but not with an explicit -target parameter.

Would enabling config files in such invocations be a solution for your use case?

Not really, we don't strictly need per-target config files at the moment. There might be some use for them in the future but right now our most dire need is to be able to ensure that our config file is included in all invocations (except for explicit --no-default-config).

With this patch, this is handled by having files for all installed prefixes + default* for unprefixed executables.

I suppose an alternative option would be to do a major refactoring to support a wider range of naming variants with fallbacks. Would you prefer if I tried to do that?

mgorny abandoned this revision.Wed, Sep 21, 11:27 AM

Abandoning in favor of D134337.