[clang] [Driver] Do not transform explicit --config filename

mgorny · mgorny · commit a7d2409bac7f · 2022-09-20T07:58:27.000+02:00
Disable transformations (e.g. attempting to replace target architecture) in the config filename that is passed explicitly via `--config`. This behavior is surprising and confusing -- if user passes an explicit config filename, Clang should use it as is. The transformations are still applied when the name is deduced from filename. Update the tests accordingly. This primarily ensures that full filename with .cfg suffix is passed to --config (appending `.cfg` implicitly is not documented, and would collide with use of filenames with other suffixes). The config-file2.c suite is removed entirely as it tested the transformations on the argument to --config. However, the aspects of that that were not tested as part of config-file3.c are now added there (based on config filename deduced from executable). This change streamlines the code in Driver::loadConfigFile(), opening the possibility of further changes, including support for handling multiple --config options and refactoring of filename deduction. Differential Revision: https://reviews.llvm.org/D134208
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
@@ -964,7 +964,6 @@ bool Driver::readConfigFile(StringRef FileName) {
 
 bool Driver::loadConfigFile() {
   std::string CfgFileName;
-  bool FileSpecifiedExplicitly = false;
 
   // Process options that change search path for config files.
   if (CLOptions) {
@@ -988,7 +987,11 @@ bool Driver::loadConfigFile() {
     }
   }
 
+  // Prepare list of directories where config file is searched for.
+  StringRef CfgFileSearchDirs[] = {UserConfigDir, SystemConfigDir, Dir};
+
   // First try to find config file specified in command line.
+  llvm::SmallString<128> CfgFilePath;
   if (CLOptions) {
     std::vector<std::string> ConfigFiles =
         CLOptions->getAllArgValues(options::OPT_config);
@@ -1020,7 +1023,18 @@ bool Driver::loadConfigFile() {
         return readConfigFile(CfgFilePath);
       }
 
-      FileSpecifiedExplicitly = true;
+      // Look for the configuration file in the usual locations.
+      if (searchForFile(CfgFilePath, CfgFileSearchDirs, CfgFileName, getVFS()))
+        return readConfigFile(CfgFilePath);
+
+      // Report error but only if config file was specified explicitly, by
+      // option --config. If it was deduced from executable name, it is not an
+      // error.
+      Diag(diag::err_drv_config_file_not_found) << CfgFileName;
+      for (const StringRef &SearchDir : CfgFileSearchDirs)
+        if (!SearchDir.empty())
+          Diag(diag::note_drv_config_file_searched_in) << SearchDir;
+      return true;
     }
   }
 
@@ -1059,8 +1073,8 @@ bool Driver::loadConfigFile() {
     // Get architecture name from config file name like 'i386.cfg' or
     // 'armv7l-clang.cfg'.
     // Check if command line options changes effective triple.
-    llvm::Triple EffectiveTriple = computeTargetTriple(*this,
-                                             CfgTriple.getTriple(), *CLOptions);
+    llvm::Triple EffectiveTriple =
+        computeTargetTriple(*this, CfgTriple.getTriple(), *CLOptions);
     if (CfgTriple.getArch() != EffectiveTriple.getArch()) {
       FixedConfigFile = EffectiveTriple.getArchName();
       FixedArchPrefixLen = FixedConfigFile.size();
@@ -1071,11 +1085,7 @@ bool Driver::loadConfigFile() {
     }
   }
 
-  // Prepare list of directories where config file is searched for.
-  StringRef CfgFileSearchDirs[] = {UserConfigDir, SystemConfigDir, Dir};
-
   // Try to find config file. First try file with corrected architecture.
-  llvm::SmallString<128> CfgFilePath;
   if (!FixedConfigFile.empty()) {
     if (searchForFile(CfgFilePath, CfgFileSearchDirs, FixedConfigFile,
                       getVFS()))
@@ -1101,16 +1111,8 @@ bool Driver::loadConfigFile() {
       return readConfigFile(CfgFilePath);
   }
 
-  // Report error but only if config file was specified explicitly, by option
-  // --config. If it was deduced from executable name, it is not an error.
-  if (FileSpecifiedExplicitly) {
-    Diag(diag::err_drv_config_file_not_found) << CfgFileName;
-    for (const StringRef &SearchDir : CfgFileSearchDirs)
-      if (!SearchDir.empty())
-        Diag(diag::note_drv_config_file_searched_in) << SearchDir;
-    return true;
-  }
-
+  // If we were unable to find a config file deduced from executable name,
+  // do not report an error.
   return false;
 }
 
diff --git a/clang/test/Driver/config-file-errs.c b/clang/test/Driver/config-file-errs.c
@@ -24,24 +24,24 @@
 
 //--- Argument of '--config' must exist somewhere in well-known directories, if it is specified by bare name.
 //
-// RUN: not %clang --config-system-dir= --config-user-dir= --config nonexistent-config-file 2>&1 | FileCheck %s -check-prefix CHECK-NOTFOUND0
+// RUN: not %clang --config-system-dir= --config-user-dir= --config nonexistent-config-file.cfg 2>&1 | FileCheck %s -check-prefix CHECK-NOTFOUND0
 // CHECK-NOTFOUND0: configuration file 'nonexistent-config-file.cfg' cannot be found
 // CHECK-NOTFOUND0-NEXT: was searched for in the directory:
 // CHECK-NOTFOUND0-NOT: was searched for in the directory:
 //
-// RUN: not %clang --config-system-dir= --config-user-dir=%S/Inputs/config2 --config nonexistent-config-file 2>&1 | FileCheck %s -check-prefix CHECK-NOTFOUND1
+// RUN: not %clang --config-system-dir= --config-user-dir=%S/Inputs/config2 --config nonexistent-config-file.cfg 2>&1 | FileCheck %s -check-prefix CHECK-NOTFOUND1
 // CHECK-NOTFOUND1: configuration file 'nonexistent-config-file.cfg' cannot be found
 // CHECK-NOTFOUND1-NEXT: was searched for in the directory: {{.*}}/Inputs/config2
 // CHECK-NOTFOUND1-NEXT: was searched for in the directory:
 // CHECK-NOTFOUND1-NOT: was searched for in the directory:
 //
-// RUN: not %clang --config-system-dir=%S/Inputs/config --config-user-dir= --config nonexistent-config-file 2>&1 | FileCheck %s -check-prefix CHECK-NOTFOUND2
+// RUN: not %clang --config-system-dir=%S/Inputs/config --config-user-dir= --config nonexistent-config-file.cfg 2>&1 | FileCheck %s -check-prefix CHECK-NOTFOUND2
 // CHECK-NOTFOUND2: configuration file 'nonexistent-config-file.cfg' cannot be found
 // CHECK-NOTFOUND2-NEXT: was searched for in the directory: {{.*}}/Inputs/config
 // CHECK-NOTFOUND2-NEXT: was searched for in the directory:
 // CHECK-NOTFOUND2-NOT: was searched for in the directory:
 //
-// RUN: not %clang --config-system-dir=%S/Inputs/config --config-user-dir=%S/Inputs/config2 --config nonexistent-config-file 2>&1 | FileCheck %s -check-prefix CHECK-NOTFOUND3
+// RUN: not %clang --config-system-dir=%S/Inputs/config --config-user-dir=%S/Inputs/config2 --config nonexistent-config-file.cfg 2>&1 | FileCheck %s -check-prefix CHECK-NOTFOUND3
 // CHECK-NOTFOUND3: configuration file 'nonexistent-config-file.cfg' cannot be found
 // CHECK-NOTFOUND3-NEXT: was searched for in the directory: {{.*}}/Inputs/config2
 // CHECK-NOTFOUND3-NEXT: was searched for in the directory: {{.*}}/Inputs/config
diff --git a/clang/test/Driver/config-file.c b/clang/test/Driver/config-file.c
@@ -68,11 +68,11 @@
 
 //--- User directory is searched first.
 //
-// RUN: %clang --config-system-dir=%S/Inputs/config --config-user-dir=%S/Inputs/config2 --config config-4 -S %s -o /dev/null -v 2>&1 | FileCheck %s -check-prefix CHECK-PRECEDENCE
+// RUN: %clang --config-system-dir=%S/Inputs/config --config-user-dir=%S/Inputs/config2 --config config-4.cfg -S %s -o /dev/null -v 2>&1 | FileCheck %s -check-prefix CHECK-PRECEDENCE
 // CHECK-PRECEDENCE: Configuration file: {{.*}}Inputs{{.}}config2{{.}}config-4.cfg
 // CHECK-PRECEDENCE: -Wall
 
 
 //--- Duplicate --config options are allowed if the value is the same
-// RUN: %clang --config-system-dir=%S/Inputs/config --config-user-dir=%S/Inputs/config2 --config config-4 --config config-4 -S %s -o /dev/null -v 2>&1 | FileCheck %s -check-prefix CHECK-SAME-CONFIG
+// RUN: %clang --config-system-dir=%S/Inputs/config --config-user-dir=%S/Inputs/config2 --config config-4.cfg --config config-4.cfg -S %s -o /dev/null -v 2>&1 | FileCheck %s -check-prefix CHECK-SAME-CONFIG
 // CHECK-SAME-CONFIG: Configuration file: {{.*}}Inputs{{.}}config2{{.}}config-4.cfg
diff --git a/clang/test/Driver/config-file2.c b/clang/test/Driver/config-file2.c
diff --git a/clang/test/Driver/config-file3.c b/clang/test/Driver/config-file3.c
@@ -37,7 +37,7 @@
 //
 //--- File specified by --config overrides config inferred from clang executable.
 //
-// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir=%S/Inputs/config --config-user-dir= --config i386-qqq -c -no-canonical-prefixes -### %s 2>&1 | FileCheck %s -check-prefix CHECK-EXPLICIT
+// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir=%S/Inputs/config --config-user-dir= --config i386-qqq.cfg -c -no-canonical-prefixes -### %s 2>&1 | FileCheck %s -check-prefix CHECK-EXPLICIT
 //
 // CHECK-EXPLICIT: Configuration file: {{.*}}/Inputs/config/i386-qqq.cfg
 
@@ -77,27 +77,24 @@
 // RUN: ln -s %clang %t/testreload/x86_64-clang-g++
 // RUN: echo "-Wundefined-func-template" > %t/testreload/i386-clang-g++.cfg
 // RUN: echo "-Werror" > %t/testreload/i386.cfg
+// RUN: echo "-Wall" > %t/testreload/x86_64-clang-g++.cfg
 // RUN: %t/testreload/x86_64-clang-g++ --config-system-dir= --config-user-dir= -c -m32 -no-canonical-prefixes -### %s 2>&1 | FileCheck %s -check-prefix CHECK-RELOAD
 //
 // CHECK-RELOAD: Configuration file: {{.*}}/testreload/i386-clang-g++.cfg
 // CHECK-RELOAD: -Wundefined-func-template
 // CHECK-RELOAD-NOT: -Werror
+// CHECK-RELOAD-NOT: -Wall
 
-//--- If config file is specified by --config and its name does not start with architecture, it is used without reloading.
-//
-// RUN: %t/testreload/x86_64-clang-g++ --config-system-dir=%S/Inputs --config-user-dir= --config config-3 -c -m32 -no-canonical-prefixes -### %s 2>&1 | FileCheck %s -check-prefix CHECK-RELOAD1a
-//
-// CHECK-RELOAD1a: Configuration file: {{.*}}/Inputs/config-3.cfg
-//
-// RUN: %t/testreload/x86_64-clang-g++ --config-system-dir=%S/Inputs --config-user-dir= --config config-3 -c --target=i386 -no-canonical-prefixes -### %s 2>&1 | FileCheck %s -check-prefix CHECK-RELOAD1b
-//
-// CHECK-RELOAD1b: Configuration file: {{.*}}/Inputs/config-3.cfg
+//--- Same for -target in place of -m32.
+// RUN: %t/testreload/x86_64-clang-g++ --config-system-dir= --config-user-dir= -c -target i386 -no-canonical-prefixes -### %s 2>&1 | FileCheck %s -check-prefix CHECK-RELOAD
 
-//--- If config file is specified by --config and its name starts with architecture, it is reloaded.
-//
-// RUN: %t/testreload/x86_64-clang-g++ --config-system-dir=%S/Inputs/config --config-user-dir= --config x86_64-qqq -c -m32 -no-canonical-prefixes -### %s 2>&1 | FileCheck %s -check-prefix CHECK-RELOAD1c
+//--- `-target i386 -m64` should load the 64-bit config.
+// RUN: %t/testreload/x86_64-clang-g++ --config-system-dir= --config-user-dir= -c -target i386 -m64 -no-canonical-prefixes -### %s 2>&1 | FileCheck %s -check-prefix CHECK-RELOAD1a
 //
-// CHECK-RELOAD1c: Configuration file: {{.*}}/Inputs/config/i386-qqq.cfg
+// CHECK-RELOAD1a: Configuration file: {{.*}}/testreload/x86_64-clang-g++.cfg
+// CHECK-RELOAD1a: -Wall
+// CHECK-RELOAD1a-NOT: -Werror
+// CHECK-RELOAD1a-NOT: -Wundefined-func-template
 
 //--- x86_64-clang-g++ tries to find config i386.cfg if i386-clang-g++.cfg is not found.
 //
@@ -107,4 +104,9 @@
 // CHECK-RELOAD1d: Configuration file: {{.*}}/testreload/i386.cfg
 // CHECK-RELOAD1d: -Werror
 // CHECK-RELOAD1d-NOT: -Wundefined-func-template
+// CHECK-RELOAD1d-NOT: -Wall
 
+//--- x86_64-clang-g++ uses x86_64-clang-g++.cfg if i386*.cfg are not found.
+//
+// RUN: rm %t/testreload/i386.cfg
+// RUN: %t/testreload/x86_64-clang-g++ --config-system-dir= --config-user-dir= -c -m32 -no-canonical-prefixes -### %s 2>&1 | FileCheck %s -check-prefix CHECK-RELOAD1a
