[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL321587: Enable configuration files in clang (authored by 
sepavloff, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D24933

Files:
  cfe/trunk/docs/UsersManual.rst
  cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
  cfe/trunk/include/clang/Config/config.h.cmake
  cfe/trunk/include/clang/Driver/Driver.h
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/lib/Driver/Driver.cpp
  cfe/trunk/test/Driver/Inputs/config-1.cfg
  cfe/trunk/test/Driver/Inputs/config-2.cfg
  cfe/trunk/test/Driver/Inputs/config-2a.cfg
  cfe/trunk/test/Driver/Inputs/config-3.cfg
  cfe/trunk/test/Driver/Inputs/config-4.cfg
  cfe/trunk/test/Driver/Inputs/config-5.cfg
  cfe/trunk/test/Driver/Inputs/config-6.cfg
  cfe/trunk/test/Driver/Inputs/config/config-4.cfg
  cfe/trunk/test/Driver/Inputs/config/i386-qqq.cfg
  cfe/trunk/test/Driver/Inputs/config/i386-qqq3.cfg
  cfe/trunk/test/Driver/Inputs/config/x86_64-qqq.cfg
  cfe/trunk/test/Driver/Inputs/config/x86_64-qqq2.cfg
  cfe/trunk/test/Driver/Inputs/config/x86_64.cfg
  cfe/trunk/test/Driver/Inputs/config2/config-4.cfg
  cfe/trunk/test/Driver/Inputs/config2/i386.cfg
  cfe/trunk/test/Driver/config-file-errs.c
  cfe/trunk/test/Driver/config-file.c
  cfe/trunk/test/Driver/config-file2.c
  cfe/trunk/test/Driver/config-file3.c

Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -62,14 +62,16 @@
 #include "llvm/Option/OptSpecifier.h"
 #include "llvm/Option/OptTable.h"
 #include "llvm/Option/Option.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Support/StringSaver.h"
 #include 
 #include 
 #include 
@@ -92,7 +94,8 @@
   CCPrintHeadersFilename(nullptr), CCLogDiagnosticsFilename(nullptr),
   CCCPrintBindings(false), CCPrintHeaders(false), CCLogDiagnostics(false),
   CCGenDiagnostics(false), DefaultTargetTriple(DefaultTargetTriple),
-  CCCGenericGCCName(""), CheckInputsExist(true), CCCUsePCH(true),
+  CCCGenericGCCName(""), Saver(Alloc),
+  CheckInputsExist(true), CCCUsePCH(true),
   GenReproducer(false), SuppressMissingInputWarning(false) {
 
   // Provide a sane fallback if no VFS is specified.
@@ -103,6 +106,13 @@
   Dir = llvm::sys::path::parent_path(ClangExecutable);
   InstalledDir = Dir; // Provide a sensible default installed dir.
 
+#if defined(CLANG_CONFIG_FILE_SYSTEM_DIR)
+  SystemConfigDir = CLANG_CONFIG_FILE_SYSTEM_DIR;
+#endif
+#if defined(CLANG_CONFIG_FILE_USER_DIR)
+  UserConfigDir = CLANG_CONFIG_FILE_USER_DIR;
+#endif
+
   // Compute the path to the resource directory.
   StringRef ClangResourceDir(CLANG_RESOURCE_DIR);
   SmallString<128> P(Dir);
@@ -600,6 +610,216 @@
   //
 }
 
+/// Looks the given directories for the specified file.
+///
+/// \param[out] FilePath File path, if the file was found.
+/// \param[in]  Dirs Directories used for the search.
+/// \param[in]  FileName Name of the file to search for.
+/// \return True if file was found.
+///
+/// Looks for file specified by FileName sequentially in directories specified
+/// by Dirs.
+///
+static bool searchForFile(SmallVectorImpl ,
+  ArrayRef Dirs,
+  StringRef FileName) {
+  SmallString<128> WPath;
+  for (const StringRef  : Dirs) {
+if (Dir.empty())
+  continue;
+WPath.clear();
+llvm::sys::path::append(WPath, Dir, FileName);
+llvm::sys::path::native(WPath);
+if (llvm::sys::fs::is_regular_file(WPath)) {
+  FilePath = std::move(WPath);
+  return true;
+}
+  }
+  return false;
+}
+
+bool Driver::readConfigFile(StringRef FileName) {
+  // Try reading the given file.
+  SmallVector NewCfgArgs;
+  if (!llvm::cl::readConfigFile(FileName, Saver, NewCfgArgs)) {
+Diag(diag::err_drv_cannot_read_config_file) << FileName;
+return true;
+  }
+
+  // Read options from config file.
+  llvm::SmallString<128> CfgFileName(FileName);
+  llvm::sys::path::native(CfgFileName);
+  ConfigFile = CfgFileName.str();
+  bool ContainErrors;
+  CfgOptions = llvm::make_unique(
+  ParseArgStrings(NewCfgArgs, ContainErrors));
+  if (ContainErrors) {
+CfgOptions.reset();
+return true;
+  }
+
+  if (CfgOptions->hasArg(options::OPT_config)) {
+CfgOptions.reset();
+Diag(diag::err_drv_nested_config_file);
+return true;
+  }
+
+  // Claim all arguments that come from a configuration file so that the driver
+  // does not warn on any that is unused.
+  for (Arg *A : *CfgOptions)
+A->claim();
+  return false;
+}
+
+bool Driver::loadConfigFile() {
+  

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 127911.
sepavloff marked 2 inline comments as done.
sepavloff added a comment.

Small corrections to the patch

- Avoid quick return in the case of error, it may cause fails of clang tools.
- Fixed typo.


Repository:
  rC Clang

https://reviews.llvm.org/D24933

Files:
  docs/UsersManual.rst
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Config/config.h.cmake
  include/clang/Driver/Driver.h
  include/clang/Driver/Options.td
  lib/Driver/Driver.cpp
  test/Driver/Inputs/config-1.cfg
  test/Driver/Inputs/config-2.cfg
  test/Driver/Inputs/config-2a.cfg
  test/Driver/Inputs/config-3.cfg
  test/Driver/Inputs/config-4.cfg
  test/Driver/Inputs/config-5.cfg
  test/Driver/Inputs/config-6.cfg
  test/Driver/Inputs/config/config-4.cfg
  test/Driver/Inputs/config/i386-qqq.cfg
  test/Driver/Inputs/config/i386-qqq3.cfg
  test/Driver/Inputs/config/x86_64-qqq.cfg
  test/Driver/Inputs/config/x86_64-qqq2.cfg
  test/Driver/Inputs/config/x86_64.cfg
  test/Driver/Inputs/config2/config-4.cfg
  test/Driver/Inputs/config2/i386.cfg
  test/Driver/config-file-errs.c
  test/Driver/config-file.c
  test/Driver/config-file2.c
  test/Driver/config-file3.c

Index: test/Driver/config-file3.c
===
--- /dev/null
+++ test/Driver/config-file3.c
@@ -0,0 +1,98 @@
+// REQUIRES: shell
+// REQUIRES: x86-registered-target
+
+//--- If config file is specified by relative path (workdir/cfg-s2), it is searched for by that path.
+//
+// RUN: mkdir -p %T/workdir
+// RUN: echo "@subdir/cfg-s2" > %T/workdir/cfg-1
+// RUN: mkdir -p %T/workdir/subdir
+// RUN: echo "-Wundefined-var-template" > %T/workdir/subdir/cfg-s2
+//
+// RUN: ( cd %T && %clang --config workdir/cfg-1 -c %s -### 2>&1 | FileCheck %s -check-prefix CHECK-REL )
+//
+// CHECK-REL: Configuration file: {{.*}}/workdir/cfg-1
+// CHECK-REL: -Wundefined-var-template
+
+
+//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg first.
+//
+// RUN: mkdir -p %T/testdmode
+// RUN: [ ! -s %T/testdmode/qqq-clang-g++ ] || rm %T/testdmode/qqq-clang-g++
+// RUN: ln -s %clang %T/testdmode/qqq-clang-g++
+// RUN: echo "-Wundefined-func-template" > %T/testdmode/qqq-clang-g++.cfg
+// RUN: echo "-Werror" > %T/testdmode/qqq.cfg
+// RUN: %T/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir= -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix FULL-NAME
+//
+// FULL-NAME: Configuration file: {{.*}}/testdmode/qqq-clang-g++.cfg
+// FULL-NAME: -Wundefined-func-template
+// FULL-NAME-NOT: -Werror
+//
+//--- 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
+//
+// CHECK-EXPLICIT: Configuration file: {{.*}}/Inputs/config/i386-qqq.cfg
+//
+//--- Invocation qqq-clang-g++ tries to find config file qqq.cfg if qqq-clang-g++.cfg is not found.
+//
+// RUN: rm %T/testdmode/qqq-clang-g++.cfg
+// RUN: %T/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir= -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix SHORT-NAME
+//
+// SHORT-NAME: Configuration file: {{.*}}/testdmode/qqq.cfg
+// SHORT-NAME: -Werror
+// SHORT-NAME-NOT: -Wundefined-func-template
+
+
+//--- Config files are searched for in binary directory as well.
+//
+// RUN: mkdir -p %T/testbin
+// RUN: [ ! -s %T/testbin/clang ] || rm %T/testbin/clang
+// RUN: ln -s %clang %T/testbin/clang
+// RUN: echo "-Werror" > %T/testbin/aaa.cfg
+// RUN: %T/testbin/clang --config-system-dir= --config-user-dir= --config aaa.cfg -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix CHECK-BIN
+//
+// CHECK-BIN: Configuration file: {{.*}}/testbin/aaa.cfg
+// CHECK-BIN: -Werror
+
+
+//--- If command line contains options that change triple (for instance, -m32), clang tries
+//reloading config file.
+
+//--- When reloading config file, x86_64-clang-g++ tries to find config i386-clang-g++.cfg first.
+//
+// RUN: mkdir -p %T/testreload
+// RUN: [ ! -s %T/testreload/x86_64-clang-g++ ] || rm %T/testreload/x86_64-clang-g++
+// 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: %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
+
+//--- 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 

[PATCH] D24933: Enable configuration files in clang

[Hal Finkel via Phabricator via cfe-commits]

hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: lib/Driver/Driver.cpp:1508
+if (!SystemConfigDir.empty())
+  llvm::errs() << "System configuration file directory: "
+   << SystemConfigDir << "\n";

configuration file directory -> configuration-file directory



Comment at: lib/Driver/Driver.cpp:1511
+if (!UserConfigDir.empty())
+  llvm::errs() << "User configuration file directory: "
+   << UserConfigDir << "\n";

configuration file directory -> configuration-file directory


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff marked 2 inline comments as done.
sepavloff added inline comments.



Comment at: include/clang/Config/config.h.cmake:40
+#cmakedefine CLANG_CONFIG_FILE_SYSTEM_DIR ${CLANG_CONFIG_FILE_SYSTEM_DIR}
+#cmakedefine CLANG_CONFIG_FILE_USER_DIR ${CLANG_CONFIG_FILE_USER_DIR}
+

hintonda wrote:
> These need to be in quotes since you assign them to a std::string, i.e.:
> 
> ```
> #cmakedefine CLANG_CONFIG_FILE_SYSTEM_DIR "${CLANG_CONFIG_FILE_SYSTEM_DIR}"
> #cmakedefine CLANG_CONFIG_FILE_USER_DIR "${CLANG_CONFIG_FILE_USER_DIR}"
> ```
Fixed, thank you.



Comment at: lib/Driver/Driver.cpp:641
+
+static std::string getAbsolutePath(StringRef P) {
+  if (P.empty())

hfinkel wrote:
> Use llvm::sys::fs::make_absolute instead of this.
Removed `getAbsolutePath`.


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 127109.
sepavloff added a comment.

Updated patch

Fixed cmake config file definition.
Use llvm::sys::fs::make_absolute to get absolute path.


https://reviews.llvm.org/D24933

Files:
  docs/UsersManual.rst
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Config/config.h.cmake
  include/clang/Driver/Driver.h
  include/clang/Driver/Options.td
  lib/Driver/Driver.cpp
  test/Driver/Inputs/config-1.cfg
  test/Driver/Inputs/config-2.cfg
  test/Driver/Inputs/config-2a.cfg
  test/Driver/Inputs/config-3.cfg
  test/Driver/Inputs/config-4.cfg
  test/Driver/Inputs/config-5.cfg
  test/Driver/Inputs/config-6.cfg
  test/Driver/Inputs/config/config-4.cfg
  test/Driver/Inputs/config/i386-qqq.cfg
  test/Driver/Inputs/config/i386-qqq3.cfg
  test/Driver/Inputs/config/x86_64-qqq.cfg
  test/Driver/Inputs/config/x86_64-qqq2.cfg
  test/Driver/Inputs/config/x86_64.cfg
  test/Driver/Inputs/config2/config-4.cfg
  test/Driver/Inputs/config2/i386.cfg
  test/Driver/config-file-errs.c
  test/Driver/config-file.c
  test/Driver/config-file2.c
  test/Driver/config-file3.c

Index: test/Driver/config-file3.c
===
--- /dev/null
+++ test/Driver/config-file3.c
@@ -0,0 +1,98 @@
+// REQUIRES: shell
+// REQUIRES: x86-registered-target
+
+//--- If config file is specified by relative path (workdir/cfg-s2), it is searched for by that path.
+//
+// RUN: mkdir -p %T/workdir
+// RUN: echo "@subdir/cfg-s2" > %T/workdir/cfg-1
+// RUN: mkdir -p %T/workdir/subdir
+// RUN: echo "-Wundefined-var-template" > %T/workdir/subdir/cfg-s2
+//
+// RUN: ( cd %T && %clang --config workdir/cfg-1 -c %s -### 2>&1 | FileCheck %s -check-prefix CHECK-REL )
+//
+// CHECK-REL: Configuration file: {{.*}}/workdir/cfg-1
+// CHECK-REL: -Wundefined-var-template
+
+
+//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg first.
+//
+// RUN: mkdir -p %T/testdmode
+// RUN: [ ! -s %T/testdmode/qqq-clang-g++ ] || rm %T/testdmode/qqq-clang-g++
+// RUN: ln -s %clang %T/testdmode/qqq-clang-g++
+// RUN: echo "-Wundefined-func-template" > %T/testdmode/qqq-clang-g++.cfg
+// RUN: echo "-Werror" > %T/testdmode/qqq.cfg
+// RUN: %T/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir= -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix FULL-NAME
+//
+// FULL-NAME: Configuration file: {{.*}}/testdmode/qqq-clang-g++.cfg
+// FULL-NAME: -Wundefined-func-template
+// FULL-NAME-NOT: -Werror
+//
+//--- 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
+//
+// CHECK-EXPLICIT: Configuration file: {{.*}}/Inputs/config/i386-qqq.cfg
+//
+//--- Invocation qqq-clang-g++ tries to find config file qqq.cfg if qqq-clang-g++.cfg is not found.
+//
+// RUN: rm %T/testdmode/qqq-clang-g++.cfg
+// RUN: %T/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir= -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix SHORT-NAME
+//
+// SHORT-NAME: Configuration file: {{.*}}/testdmode/qqq.cfg
+// SHORT-NAME: -Werror
+// SHORT-NAME-NOT: -Wundefined-func-template
+
+
+//--- Config files are searched for in binary directory as well.
+//
+// RUN: mkdir -p %T/testbin
+// RUN: [ ! -s %T/testbin/clang ] || rm %T/testbin/clang
+// RUN: ln -s %clang %T/testbin/clang
+// RUN: echo "-Werror" > %T/testbin/aaa.cfg
+// RUN: %T/testbin/clang --config-system-dir= --config-user-dir= --config aaa.cfg -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix CHECK-BIN
+//
+// CHECK-BIN: Configuration file: {{.*}}/testbin/aaa.cfg
+// CHECK-BIN: -Werror
+
+
+//--- If command line contains options that change triple (for instance, -m32), clang tries
+//reloading config file.
+
+//--- When reloading config file, x86_64-clang-g++ tries to find config i386-clang-g++.cfg first.
+//
+// RUN: mkdir -p %T/testreload
+// RUN: [ ! -s %T/testreload/x86_64-clang-g++ ] || rm %T/testreload/x86_64-clang-g++
+// 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: %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
+
+//--- 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: 

[PATCH] D24933: Enable configuration files in clang

[Don Hinton via Phabricator via cfe-commits]

hintonda added inline comments.



Comment at: include/clang/Config/config.h.cmake:40
+#cmakedefine CLANG_CONFIG_FILE_SYSTEM_DIR ${CLANG_CONFIG_FILE_SYSTEM_DIR}
+#cmakedefine CLANG_CONFIG_FILE_USER_DIR ${CLANG_CONFIG_FILE_USER_DIR}
+

These need to be in quotes since you assign them to a std::string, i.e.:

```
#cmakedefine CLANG_CONFIG_FILE_SYSTEM_DIR "${CLANG_CONFIG_FILE_SYSTEM_DIR}"
#cmakedefine CLANG_CONFIG_FILE_USER_DIR "${CLANG_CONFIG_FILE_USER_DIR}"
```


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added inline comments.



Comment at: lib/Driver/Driver.cpp:641
+
+static std::string getAbsolutePath(StringRef P) {
+  if (P.empty())

Use llvm::sys::fs::make_absolute instead of this.


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 123503.
sepavloff added a comment.

Updated patch

- Added command line option for setting directories where config files are 
searched for,
- Fixed architecture calculation,
- Option --config in config file is now diagnosed,
- Output made by option -v contains config file search directories,
- Test are changed so that they do not fail on non-x86 architectures,
- Added new tests.


https://reviews.llvm.org/D24933

Files:
  docs/UsersManual.rst
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Config/config.h.cmake
  include/clang/Driver/Driver.h
  include/clang/Driver/Options.td
  lib/Driver/Driver.cpp
  test/Driver/Inputs/config-1.cfg
  test/Driver/Inputs/config-2.cfg
  test/Driver/Inputs/config-2a.cfg
  test/Driver/Inputs/config-3.cfg
  test/Driver/Inputs/config-4.cfg
  test/Driver/Inputs/config-5.cfg
  test/Driver/Inputs/config-6.cfg
  test/Driver/Inputs/config/config-4.cfg
  test/Driver/Inputs/config/i386-qqq.cfg
  test/Driver/Inputs/config/i386-qqq3.cfg
  test/Driver/Inputs/config/x86_64-qqq.cfg
  test/Driver/Inputs/config/x86_64-qqq2.cfg
  test/Driver/Inputs/config/x86_64.cfg
  test/Driver/Inputs/config2/config-4.cfg
  test/Driver/Inputs/config2/i386.cfg
  test/Driver/config-file-errs.c
  test/Driver/config-file.c
  test/Driver/config-file2.c
  test/Driver/config-file3.c

Index: test/Driver/config-file3.c
===
--- /dev/null
+++ test/Driver/config-file3.c
@@ -0,0 +1,98 @@
+// REQUIRES: shell
+// REQUIRES: x86-registered-target
+
+//--- If config file is specified by relative path (workdir/cfg-s2), it is searched for by that path.
+//
+// RUN: mkdir -p %T/workdir
+// RUN: echo "@subdir/cfg-s2" > %T/workdir/cfg-1
+// RUN: mkdir -p %T/workdir/subdir
+// RUN: echo "-Wundefined-var-template" > %T/workdir/subdir/cfg-s2
+//
+// RUN: ( cd %T && %clang --config workdir/cfg-1 -c %s -### 2>&1 | FileCheck %s -check-prefix CHECK-REL )
+//
+// CHECK-REL: Configuration file: {{.*}}/workdir/cfg-1
+// CHECK-REL: -Wundefined-var-template
+
+
+//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg first.
+//
+// RUN: mkdir -p %T/testdmode
+// RUN: [ ! -s %T/testdmode/qqq-clang-g++ ] || rm %T/testdmode/qqq-clang-g++
+// RUN: ln -s %clang %T/testdmode/qqq-clang-g++
+// RUN: echo "-Wundefined-func-template" > %T/testdmode/qqq-clang-g++.cfg
+// RUN: echo "-Werror" > %T/testdmode/qqq.cfg
+// RUN: %T/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir= -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix FULL-NAME
+//
+// FULL-NAME: Configuration file: {{.*}}/testdmode/qqq-clang-g++.cfg
+// FULL-NAME: -Wundefined-func-template
+// FULL-NAME-NOT: -Werror
+//
+//--- 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
+//
+// CHECK-EXPLICIT: Configuration file: {{.*}}/Inputs/config/i386-qqq.cfg
+//
+//--- Invocation qqq-clang-g++ tries to find config file qqq.cfg if qqq-clang-g++.cfg is not found.
+//
+// RUN: rm %T/testdmode/qqq-clang-g++.cfg
+// RUN: %T/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir= -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix SHORT-NAME
+//
+// SHORT-NAME: Configuration file: {{.*}}/testdmode/qqq.cfg
+// SHORT-NAME: -Werror
+// SHORT-NAME-NOT: -Wundefined-func-template
+
+
+//--- Config files are searched for in binary directory as well.
+//
+// RUN: mkdir -p %T/testbin
+// RUN: [ ! -s %T/testbin/clang ] || rm %T/testbin/clang
+// RUN: ln -s %clang %T/testbin/clang
+// RUN: echo "-Werror" > %T/testbin/aaa.cfg
+// RUN: %T/testbin/clang --config-system-dir= --config-user-dir= --config aaa.cfg -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix CHECK-BIN
+//
+// CHECK-BIN: Configuration file: {{.*}}/testbin/aaa.cfg
+// CHECK-BIN: -Werror
+
+
+//--- If command line contains options that change triple (for instance, -m32), clang tries
+//reloading config file.
+
+//--- When reloading config file, x86_64-clang-g++ tries to find config i386-clang-g++.cfg first.
+//
+// RUN: mkdir -p %T/testreload
+// RUN: [ ! -s %T/testreload/x86_64-clang-g++ ] || rm %T/testreload/x86_64-clang-g++
+// 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: %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
+
+//--- If config file is specified by --config and its name does not start with architecture, it is used 

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff marked 3 inline comments as done.
sepavloff added inline comments.



Comment at: lib/Driver/Driver.cpp:739
+  // like: i386-clang.cfg -> x86_64-clang.cfg.
+  if (ArchPrefixLen < CfgFileName.size())
+FixedConfigFile += CfgFileName.substr(ArchPrefixLen);

hfinkel wrote:
> I don't see how this length check makes sense in all cases. If CfgFileName 
> came from a --config command-line options, whether or not it is longer or 
> shoter than ArchPrefixLen seems irrelevant. Do you need to do some prefix 
> check here?
Indeed, this feature require further elaboration. We are going to treat config 
file specified by `--config` with higher priority, but actually we can have 
even more complex combination, something like:
mips64-clang --config mips32 -target x86 -m32
Nevertheless such call must be consistently processed.

I will implement the relevant functionality two weeks later, after return from 
vacation.


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added inline comments.



Comment at: lib/Driver/Driver.cpp:706
+
+  // Determine architecture part of the file name, if it presents.
+  size_t ArchPrefixLen = 0;

if it presents. -> if it is present.



Comment at: lib/Driver/Driver.cpp:739
+  // like: i386-clang.cfg -> x86_64-clang.cfg.
+  if (ArchPrefixLen < CfgFileName.size())
+FixedConfigFile += CfgFileName.substr(ArchPrefixLen);

I don't see how this length check makes sense in all cases. If CfgFileName came 
from a --config command-line options, whether or not it is longer or shoter 
than ArchPrefixLen seems irrelevant. Do you need to do some prefix check here?


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff marked 4 inline comments as done.
sepavloff added inline comments.



Comment at: lib/Driver/Driver.cpp:637
+  }
+  FilePath.clear();
+  return false;

hfinkel wrote:
> Why do we do clear FilePath here?
The intent was to avoid copying file name buffer. It however makes code less 
readable. Used temporary buffer instead.



Comment at: lib/Driver/Driver.cpp:657
+  StringRef FileName) {
+  FilePath.clear();
+  if (searchDirectoriesForFile(FilePath, FileName, Dirs))

hfinkel wrote:
> Is this necessary? I'd rather have a contract for these functions be that 
> they always clear the FilePath if necessary, and then the caller will not 
> need to worry about it.
Agree. Changed accordingly.



Comment at: lib/Driver/Driver.cpp:661
+
+  // If not found, try searching the directory where executable resides.
+  FilePath.clear();

hfinkel wrote:
> hfinkel wrote:
> > Why not just add this directory to the end of the list of directories?
> (by which I mean, why not add BinDirectory to the list of directories instead 
> of having special code for it?)
Now BinDirectory is added to the list of directories.


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 120403.
sepavloff added a comment.

Addressed reviewer's notes

- Do not process binary directory separately but add it to the array of search 
directories.
- Do not use result file name buffer for temporary data.


https://reviews.llvm.org/D24933

Files:
  docs/UsersManual.rst
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Config/config.h.cmake
  include/clang/Driver/Driver.h
  include/clang/Driver/Options.td
  lib/Driver/Driver.cpp
  test/Driver/Inputs/config-1.cfg
  test/Driver/Inputs/config-2.cfg
  test/Driver/Inputs/config-2a.cfg
  test/Driver/Inputs/config-3.cfg
  test/Driver/Inputs/config-4.cfg
  test/Driver/Inputs/config-5.cfg
  test/Driver/Inputs/config/config-4.cfg
  test/Driver/config-file-errs.c
  test/Driver/config-file.c
  test/Driver/config-file.cpp

Index: test/Driver/config-file.cpp
===
--- /dev/null
+++ test/Driver/config-file.cpp
@@ -0,0 +1,65 @@
+// REQUIRES: shell
+
+//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg first ...
+//
+// RUN: mkdir -p %T/testdmode
+// RUN: [ ! -s %T/testdmode/qqq-clang-g++ ] || rm %T/testdmode/qqq-clang-g++
+// RUN: ln -s %clang %T/testdmode/qqq-clang-g++
+// RUN: echo "-Wundefined-func-template" > %T/testdmode/qqq-clang-g++.cfg
+// RUN: echo "-Werror" > %T/testdmode/qqq.cfg
+//
+// RUN: %T/testdmode/qqq-clang-g++ -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix FULL-NAME
+//
+// FULL-NAME: Configuration file: {{.*}}/testdmode/qqq-clang-g++.cfg
+// FULL-NAME: -Wundefined-func-template
+// FULL-NAME-NOT: -Werror
+//
+//--- ... and qqq.cfg if qqq-clang-g++.cfg is not found.
+//
+// RUN: rm %T/testdmode/qqq-clang-g++.cfg
+//
+// RUN: %T/testdmode/qqq-clang-g++ -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix SHORT-NAME
+//
+// SHORT-NAME: Configuration file: {{.*}}/testdmode/qqq.cfg
+// SHORT-NAME: -Werror
+// SHORT-NAME-NOT: -Wundefined-func-template
+
+
+//--- Config files are searched for in binary directory as well.
+//
+// RUN: [ ! -s %T/testbin/clang ] || rm %T/testbin/clang
+// RUN: ln -s %clang %T/testbin/clang
+// RUN: echo "-Werror" > %T/testbin/aaa.cfg
+//
+// RUN: %T/testbin/clang --config aaa.cfg -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix CHECK-BIN
+//
+// CHECK-BIN: Configuration file: {{.*}}/testbin/aaa.cfg
+// CHECK-BIN: -Werror
+
+
+//--- If command line contains options that change triple (for instance, -m32), clang tries
+//reloading config file.
+
+//--- When reloading config file, target-clang-g++ tries to find config target32-clang-g++.cfg first ...
+//
+// RUN: mkdir -p %T/testreload
+// RUN: [ ! -s %T/testreload/x86_64-clang-g++ ] || rm %T/testreload/x86_64-clang-g++
+// 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: %T/testreload/x86_64-clang-g++ -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
+
+//--- and target32.cfg if target32-g++.cfg is not found.
+//
+// RUN: rm %T/testreload/i386-clang-g++.cfg
+//
+// RUN: %T/testreload/x86_64-clang-g++ -c -m32 -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix CHECK-RELOAD2
+//
+// CHECK-RELOAD2: Configuration file: {{.*}}/testreload/i386.cfg
+// CHECK-RELOAD2: -Werror
+// CHECK-RELOAD2-NOT: -Wundefined-func-template
Index: test/Driver/config-file.c
===
--- /dev/null
+++ test/Driver/config-file.c
@@ -0,0 +1,50 @@
+//--- Config file (full path) in output of -###
+//
+// RUN: %clang --config %S/Inputs/config-1.cfg -c %s -### 2>&1 | FileCheck %s -check-prefix CHECK-HHH
+// CHECK-HHH: Target: x86_64-apple-darwin
+// CHECK-HHH: Configuration file: {{.*}}Inputs{{.}}config-1.cfg
+// CHECK-HHH: -Werror
+// CHECK-HHH: -std=c99
+
+
+//--- Nested config files
+//
+// RUN: %clang --config %S/Inputs/config-2.cfg -c %s -### 2>&1 | FileCheck %s -check-prefix CHECK-HHH2
+// CHECK-HHH2: Target: x86_64-unknown-linux
+// CHECK-HHH2: Configuration file: {{.*}}Inputs{{.}}config-2.cfg
+// CHECK-HHH2: -Wundefined-func-template
+//
+
+// RUN: %clang --config %S/Inputs/config-2a.cfg -c %s -### 2>&1 | FileCheck %s -check-prefix CHECK-HHH2a
+// CHECK-HHH2a: Target: x86_64-unknown-linux
+// CHECK-HHH2a: Configuration file: {{.*}}Inputs{{.}}config-2a.cfg
+// CHECK-HHH2a: -isysroot
+// CHECK-HHH2a-SAME: /opt/data
+
+
+//--- If config file isspecified by relative path (workdir/cfg-s2), it is searched for by that path.
+//
+// RUN: mkdir -p %T/workdir
+// RUN: echo "@subdir/cfg-s2" > %T/workdir/cfg-1
+// RUN: mkdir -p %T/workdir/subdir
+// RUN: echo "-Wundefined-var-template" > %T/workdir/subdir/cfg-s2
+//
+// 

[PATCH] D24933: Enable configuration files in clang

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added inline comments.



Comment at: lib/Driver/Driver.cpp:661
+
+  // If not found, try searching the directory where executable resides.
+  FilePath.clear();

hfinkel wrote:
> Why not just add this directory to the end of the list of directories?
(by which I mean, why not add BinDirectory to the list of directories instead 
of having special code for it?)


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added inline comments.



Comment at: lib/Driver/Driver.cpp:630
+  for (const char *Dir : Directories) {
+assert(Dir);
+FilePath.clear();

  assert(Dir && "Directory path should not be null");



Comment at: lib/Driver/Driver.cpp:637
+  }
+  FilePath.clear();
+  return false;

Why do we do clear FilePath here?



Comment at: lib/Driver/Driver.cpp:657
+  StringRef FileName) {
+  FilePath.clear();
+  if (searchDirectoriesForFile(FilePath, FileName, Dirs))

Is this necessary? I'd rather have a contract for these functions be that they 
always clear the FilePath if necessary, and then the caller will not need to 
worry about it.



Comment at: lib/Driver/Driver.cpp:661
+
+  // If not found, try searching the directory where executable resides.
+  FilePath.clear();

Why not just add this directory to the end of the list of directories?


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff marked 28 inline comments as done.
sepavloff added a comment.

@hfinkel Thank you for explanations!


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 118315.
sepavloff added a comment.

Updated patch according to reviewer's notes


https://reviews.llvm.org/D24933

Files:
  docs/UsersManual.rst
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Config/config.h.cmake
  include/clang/Driver/Driver.h
  include/clang/Driver/Options.td
  lib/Driver/Driver.cpp
  test/Driver/Inputs/config-1.cfg
  test/Driver/Inputs/config-2.cfg
  test/Driver/Inputs/config-2a.cfg
  test/Driver/Inputs/config-3.cfg
  test/Driver/Inputs/config-4.cfg
  test/Driver/Inputs/config-5.cfg
  test/Driver/Inputs/config/config-4.cfg
  test/Driver/config-file-errs.c
  test/Driver/config-file.c
  test/Driver/config-file.cpp

Index: test/Driver/config-file.cpp
===
--- /dev/null
+++ test/Driver/config-file.cpp
@@ -0,0 +1,65 @@
+// REQUIRES: shell
+
+//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg first ...
+//
+// RUN: mkdir -p %T/testdmode
+// RUN: [ ! -s %T/testdmode/qqq-clang-g++ ] || rm %T/testdmode/qqq-clang-g++
+// RUN: ln -s %clang %T/testdmode/qqq-clang-g++
+// RUN: echo "-Wundefined-func-template" > %T/testdmode/qqq-clang-g++.cfg
+// RUN: echo "-Werror" > %T/testdmode/qqq.cfg
+//
+// RUN: %T/testdmode/qqq-clang-g++ -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix FULL-NAME
+//
+// FULL-NAME: Configuration file: {{.*}}/testdmode/qqq-clang-g++.cfg
+// FULL-NAME: -Wundefined-func-template
+// FULL-NAME-NOT: -Werror
+//
+//--- ... and qqq.cfg if qqq-clang-g++.cfg is not found.
+//
+// RUN: rm %T/testdmode/qqq-clang-g++.cfg
+//
+// RUN: %T/testdmode/qqq-clang-g++ -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix SHORT-NAME
+//
+// SHORT-NAME: Configuration file: {{.*}}/testdmode/qqq.cfg
+// SHORT-NAME: -Werror
+// SHORT-NAME-NOT: -Wundefined-func-template
+
+
+//--- Config files are searched for in binary directory as well.
+//
+// RUN: [ ! -s %T/testbin/clang ] || rm %T/testbin/clang
+// RUN: ln -s %clang %T/testbin/clang
+// RUN: echo "-Werror" > %T/testbin/aaa.cfg
+//
+// RUN: %T/testbin/clang --config aaa.cfg -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix CHECK-BIN
+//
+// CHECK-BIN: Configuration file: {{.*}}/testbin/aaa.cfg
+// CHECK-BIN: -Werror
+
+
+//--- If command line contains options that change triple (for instance, -m32), clang tries
+//reloading config file.
+
+//--- When reloading config file, target-clang-g++ tries to find config target32-clang-g++.cfg first ...
+//
+// RUN: mkdir -p %T/testreload
+// RUN: [ ! -s %T/testreload/x86_64-clang-g++ ] || rm %T/testreload/x86_64-clang-g++
+// 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: %T/testreload/x86_64-clang-g++ -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
+
+//--- and target32.cfg if target32-g++.cfg is not found.
+//
+// RUN: rm %T/testreload/i386-clang-g++.cfg
+//
+// RUN: %T/testreload/x86_64-clang-g++ -c -m32 -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix CHECK-RELOAD2
+//
+// CHECK-RELOAD2: Configuration file: {{.*}}/testreload/i386.cfg
+// CHECK-RELOAD2: -Werror
+// CHECK-RELOAD2-NOT: -Wundefined-func-template
Index: test/Driver/config-file.c
===
--- /dev/null
+++ test/Driver/config-file.c
@@ -0,0 +1,50 @@
+//--- Config file (full path) in output of -###
+//
+// RUN: %clang --config %S/Inputs/config-1.cfg -c %s -### 2>&1 | FileCheck %s -check-prefix CHECK-HHH
+// CHECK-HHH: Target: x86_64-apple-darwin
+// CHECK-HHH: Configuration file: {{.*}}Inputs{{.}}config-1.cfg
+// CHECK-HHH: -Werror
+// CHECK-HHH: -std=c99
+
+
+//--- Nested config files
+//
+// RUN: %clang --config %S/Inputs/config-2.cfg -c %s -### 2>&1 | FileCheck %s -check-prefix CHECK-HHH2
+// CHECK-HHH2: Target: x86_64-unknown-linux
+// CHECK-HHH2: Configuration file: {{.*}}Inputs{{.}}config-2.cfg
+// CHECK-HHH2: -Wundefined-func-template
+//
+
+// RUN: %clang --config %S/Inputs/config-2a.cfg -c %s -### 2>&1 | FileCheck %s -check-prefix CHECK-HHH2a
+// CHECK-HHH2a: Target: x86_64-unknown-linux
+// CHECK-HHH2a: Configuration file: {{.*}}Inputs{{.}}config-2a.cfg
+// CHECK-HHH2a: -isysroot
+// CHECK-HHH2a-SAME: /opt/data
+
+
+//--- If config file isspecified by relative path (workdir/cfg-s2), it is searched for by that path.
+//
+// RUN: mkdir -p %T/workdir
+// RUN: echo "@subdir/cfg-s2" > %T/workdir/cfg-1
+// RUN: mkdir -p %T/workdir/subdir
+// RUN: echo "-Wundefined-var-template" > %T/workdir/subdir/cfg-s2
+//
+// RUN: ( cd %T && %clang --config workdir/cfg-1 -c %s -### 2>&1 | FileCheck %s -check-prefix CHECK-REL )
+//
+// CHECK-REL: Configuration 

[PATCH] D24933: Enable configuration files in clang

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

Some suggested improvements to the English in the documentation...




Comment at: docs/UsersManual.rst:700
+
+Configuration files group command line options and allow to specify all of
+them just by referencing the configuration file. They may be used, for

Configuration files group command-line options and allow all of them to be 
specified just by ...



Comment at: docs/UsersManual.rst:702
+them just by referencing the configuration file. They may be used, for
+instance, to collect options required to tune compilation for particular
+target, such as -L, -I, -l, --sysroot, codegen options etc.

instance -> example

"for instance" and "for example" are essentially the same, but for consistency, 
"for example" is much more common in this document, so we should standardize.



Comment at: docs/UsersManual.rst:703
+instance, to collect options required to tune compilation for particular
+target, such as -L, -I, -l, --sysroot, codegen options etc.
+

options, etc.



Comment at: docs/UsersManual.rst:706
+The command line option `--config` can be used to specify configuration
+file in a Clang invocation. For instance:
+

instance -> example



Comment at: docs/UsersManual.rst:714
+If the provided argument contains a directory separator, it is considered as
+a file path, options are read from that file. Otherwise the argument is treated
+as a file name and is searched for sequentially in the directories:

and options are



Comment at: docs/UsersManual.rst:719
+- the directory where Clang executable resides.
+Both user and system directory for configuration files are specified during
+clang build using cmake parameters, CLANG_CONFIG_FILE_USER_DIR and

directory -> directories



Comment at: docs/UsersManual.rst:720
+Both user and system directory for configuration files are specified during
+clang build using cmake parameters, CLANG_CONFIG_FILE_USER_DIR and
+CLANG_CONFIG_FILE_SYSTEM_DIR respectively. The first found file is used. It is

cmake -> CMake



Comment at: docs/UsersManual.rst:721
+clang build using cmake parameters, CLANG_CONFIG_FILE_USER_DIR and
+CLANG_CONFIG_FILE_SYSTEM_DIR respectively. The first found file is used. It is
+an error if the required file cannot be found.

The first file found is used.



Comment at: docs/UsersManual.rst:724
+
+Another way to specify configuration file is to encode it in executable name. 
For
+instance, if Clang executable is named `armv7l-clang` (it may be a symbolic 
link

specify a configuration file



Comment at: docs/UsersManual.rst:725
+Another way to specify configuration file is to encode it in executable name. 
For
+instance, if Clang executable is named `armv7l-clang` (it may be a symbolic 
link
+to `clang`), then Clang will search file `armv7l.cfg` in the directory where 
Clang

if the Clang executable



Comment at: docs/UsersManual.rst:726
+instance, if Clang executable is named `armv7l-clang` (it may be a symbolic 
link
+to `clang`), then Clang will search file `armv7l.cfg` in the directory where 
Clang
+resides.

will search for file



Comment at: docs/UsersManual.rst:729
+
+If driver mode is specified in invocation, Clang tries to find file specific 
for
+the specified mode. For instance, if executable file is `x86_64-clang-cl`, 
Clang

If a driver mode



Comment at: docs/UsersManual.rst:729
+
+If driver mode is specified in invocation, Clang tries to find file specific 
for
+the specified mode. For instance, if executable file is `x86_64-clang-cl`, 
Clang

hfinkel wrote:
> If a driver mode
to find a file



Comment at: docs/UsersManual.rst:730
+If driver mode is specified in invocation, Clang tries to find file specific 
for
+the specified mode. For instance, if executable file is `x86_64-clang-cl`, 
Clang
+first looks for `x86_64-cl.cfg` and if it is not found, looks for `x86_64.cfg'.

if the executable is named `x86_64-clang-cl`



Comment at: docs/UsersManual.rst:733
+
+If command line contains options that effectively changes target architecture
+(these are -m32, -EL and some other) and configuration file starts with 
architecture

If the command line



Comment at: docs/UsersManual.rst:733
+
+If command line contains options that effectively changes target architecture
+(these are -m32, -EL and some other) and configuration file starts with 
architecture

hfinkel wrote:
> If the command line
changes -> change



Comment at: docs/UsersManual.rst:734
+If command line contains options that 

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

Here is a list of design solutions used in this implementation of config files.

**How config file is specified**

There are two ways to specify config file:

- To encode it into executable file name, such as `foo-clang`,
- To pass config file in command line arguments.

There were no objections to the variant `foo-clang`. It can be considered as a 
natural extension of the existing mechanism, in which invocation of `foo-clang` 
is equivalent to specifying target in command line: `clang --target=foo`. 
Config file allows to specify more than one option and its name is not confined 
to the registered targets.

As for specifying config file in command line, there are two variants:

- Use existing construct `@foo`.
- Use special command line option `--config foo`.

Each way has own advantages.

Construct `@file` allows to reuse existing command line syntax. Indeed, config 
file is a collection of command line arguments and `@file` is just a way to 
inserts such arguments from a file. Config file may include other files and it 
uses `@file` with the exception that `file` is resolved relative to the 
including file, not to current directory. Config file could be considered as an 
extension of existing mechanism provided by `@file`.

Using `@file` creates compatibility issues, because existing use of `@file` 
must not be broken. Obviously the file in `@file` may be treated as 
configuration only if it cannot be treated according to the existing semantics. 
Possible solution is to try loading `file` as configuration if it does not 
contain path separator and is not found in current directory.

The drawback of this syntax is that the meaning of `@foo` in the invocation 
`clang @foo abc.cpp` depends on the content of current directory. If it 
contains file `foo`, `@foo` is an expansion of response file, otherwise it 
specifies a config file. This behavior causes error if current directory 
accidentally contains a file with the same name as the specified config file.

Using dedicated option to apply config file makes the intention explicit. It 
also allow to use config files from arbitrary places. For instance, invocation 
`clang --config ./foo` allows to treat file `foo` in current directory as 
config file.

Although config file contains command line arguments as conventional response 
file, it differs from the latter:

- It resolves nested `@file` constructs differently, relative to including 
file, not current directory.
- File is searched for in predefined set of directories, not in the current 
only.
- Config file is more like a part of working environment. For instance, clang 
based SDK supplier could deliver a set config files as a part of their product. 
Response file in contrast is more close to transient build data, often 
generated by some tool.
- Warning about unused options are suppressed in config files.
- There was a proposal to extend syntax of config file to enable comments and 
using trailing backslash to split long lines, although these extensions are 
useful for response files as well.

So, maybe, loading config file deserves a special option. This way has 
advantages:

- it expresses intentions explicitly and reduce risk of accidental errors,
- it allows using absolute paths to specify config file.

**Where config files reside**

There may be several places where config files can be kept:

- Directory where clang executable resides. It is convenient for SDK developers 
as it simplifies packaging. User can use several instances of clang at the same 
time, they still may use their own set of config files without conflicts.
- System directory (for instance, /etc/llvm), that keeps config files for use 
by several users. Such case is interesting for OS distribution maintainers and 
SDK developers.
- User directory (for instance, ~/.llvm). A user can collect config file that 
tune compiler for their tasks in this directory and use them to select 
particular option set.
- Config file can be specified by path,  as in `clang --config ./foo`. This is 
convenient for developers to ensure that particular configuration is selected.

For the sake of flexibility it make sense to enable all these locations as they 
are useful in different scenarios. Location of user and system directories are 
specified at configuration, by default they are absent. If user directory is 
specified, it should have higher priority over other places for search so that 
user could correct system supplied option sets.

**Driver mode**

If config file is encoded in executable name, such as `foo-clang`, there is 
concern of using different driver modes. What config file should be searched 
for if compiler is called as `foo-cpp`, `foo-cl` etc? These tools support 
different set of options, so a flexible solution should provide possibility to 
specify different config files for different driver modes.

Clang implements flexible scheme of tool naming, in which a tool name has 
components:

  --[-][]

The part 

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 110909.
sepavloff edited the summary of this revision.
sepavloff added a comment.

Updated patch

- Some functionality, not related to config files directly, is move to separate 
changes
- Cleanup of tests.

This change does not implement using construct @file to apply config
file yet.


https://reviews.llvm.org/D24933

Files:
  docs/UsersManual.rst
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Config/config.h.cmake
  include/clang/Driver/Driver.h
  include/clang/Driver/Options.td
  include/clang/Driver/ToolChain.h
  lib/Driver/Driver.cpp
  lib/Driver/ToolChain.cpp
  lib/Tooling/Tooling.cpp
  test/Driver/Inputs/config-1.cfg
  test/Driver/Inputs/config-2.cfg
  test/Driver/Inputs/config-2a.cfg
  test/Driver/Inputs/config-3.cfg
  test/Driver/Inputs/config-4.cfg
  test/Driver/Inputs/config-5.cfg
  test/Driver/Inputs/config/config-4.cfg
  test/Driver/config-file-errs.c
  test/Driver/config-file.c
  test/Driver/config-file.cpp
  tools/driver/driver.cpp
  unittests/Driver/CMakeLists.txt
  unittests/Driver/ToolChainTest.cpp

Index: unittests/Driver/ToolChainTest.cpp
===
--- unittests/Driver/ToolChainTest.cpp
+++ unittests/Driver/ToolChainTest.cpp
@@ -18,6 +18,8 @@
 #include "clang/Basic/VirtualFileSystem.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
+#include "llvm/Support/TargetRegistry.h"
+#include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/raw_ostream.h"
 #include "gtest/gtest.h"
 using namespace clang;
@@ -164,4 +166,98 @@
   EXPECT_TRUE(C->containsError());
 }
 
+TEST(ToolChainTest, ParsedClangName) {
+  ParsedClangName Empty;
+  EXPECT_TRUE(Empty.TargetPrefix.empty());
+  EXPECT_TRUE(Empty.ModeSuffix.empty());
+  EXPECT_TRUE(Empty.DriverMode == nullptr);
+  EXPECT_FALSE(Empty.TargetIsValid);
+
+  ParsedClangName DriverOnly("clang", nullptr);
+  EXPECT_TRUE(DriverOnly.TargetPrefix.empty());
+  EXPECT_TRUE(DriverOnly.ModeSuffix == "clang");
+  EXPECT_TRUE(DriverOnly.DriverMode == nullptr);
+  EXPECT_FALSE(DriverOnly.TargetIsValid);
+
+  ParsedClangName DriverOnly2("clang++", "--driver-mode=g++");
+  EXPECT_TRUE(DriverOnly2.TargetPrefix.empty());
+  EXPECT_TRUE(DriverOnly2.ModeSuffix == "clang++");
+  EXPECT_STREQ(DriverOnly2.DriverMode, "--driver-mode=g++");
+  EXPECT_FALSE(DriverOnly2.TargetIsValid);
+
+  ParsedClangName TargetAndMode("i386", "clang-g++", "--driver-mode=g++", true);
+  EXPECT_TRUE(TargetAndMode.TargetPrefix == "i386");
+  EXPECT_TRUE(TargetAndMode.ModeSuffix == "clang-g++");
+  EXPECT_STREQ(TargetAndMode.DriverMode, "--driver-mode=g++");
+  EXPECT_TRUE(TargetAndMode.TargetIsValid);
+}
+
+TEST(ToolChainTest, GetTargetAndMode) {
+  llvm::InitializeAllTargets();
+  std::string IgnoredError;
+  if (!llvm::TargetRegistry::lookupTarget("x86_64", IgnoredError))
+return;
+
+  ParsedClangName Res = ToolChain::getTargetAndModeFromProgramName("clang");
+  EXPECT_TRUE(Res.TargetPrefix.empty());
+  EXPECT_TRUE(Res.ModeSuffix == "clang");
+  EXPECT_TRUE(Res.DriverMode == nullptr);
+  EXPECT_FALSE(Res.TargetIsValid);
+
+  Res = ToolChain::getTargetAndModeFromProgramName("clang++");
+  EXPECT_TRUE(Res.TargetPrefix.empty());
+  EXPECT_TRUE(Res.ModeSuffix == "clang++");
+  EXPECT_STREQ(Res.DriverMode, "--driver-mode=g++");
+  EXPECT_FALSE(Res.TargetIsValid);
+
+  Res = ToolChain::getTargetAndModeFromProgramName("clang++6.0");
+  EXPECT_TRUE(Res.TargetPrefix.empty());
+  EXPECT_TRUE(Res.ModeSuffix == "clang++");
+  EXPECT_STREQ(Res.DriverMode, "--driver-mode=g++");
+  EXPECT_FALSE(Res.TargetIsValid);
+
+  Res = ToolChain::getTargetAndModeFromProgramName("clang++-release");
+  EXPECT_TRUE(Res.TargetPrefix.empty());
+  EXPECT_TRUE(Res.ModeSuffix == "clang++");
+  EXPECT_STREQ(Res.DriverMode, "--driver-mode=g++");
+  EXPECT_FALSE(Res.TargetIsValid);
+
+  Res = ToolChain::getTargetAndModeFromProgramName("x86_64-clang++");
+  EXPECT_TRUE(Res.TargetPrefix == "x86_64");
+  EXPECT_TRUE(Res.ModeSuffix == "clang++");
+  EXPECT_STREQ(Res.DriverMode, "--driver-mode=g++");
+  EXPECT_TRUE(Res.TargetIsValid);
+
+  Res = ToolChain::getTargetAndModeFromProgramName(
+  "x86_64-linux-gnu-clang-c++");
+  EXPECT_TRUE(Res.TargetPrefix == "x86_64-linux-gnu");
+  EXPECT_TRUE(Res.ModeSuffix == "clang-c++");
+  EXPECT_STREQ(Res.DriverMode, "--driver-mode=g++");
+  EXPECT_TRUE(Res.TargetIsValid);
+
+  Res = ToolChain::getTargetAndModeFromProgramName(
+  "x86_64-linux-gnu-clang-c++-tot");
+  EXPECT_TRUE(Res.TargetPrefix == "x86_64-linux-gnu");
+  EXPECT_TRUE(Res.ModeSuffix == "clang-c++");
+  EXPECT_STREQ(Res.DriverMode, "--driver-mode=g++");
+  EXPECT_TRUE(Res.TargetIsValid);
+
+  Res = ToolChain::getTargetAndModeFromProgramName("qqq");
+  EXPECT_TRUE(Res.TargetPrefix.empty());
+  EXPECT_TRUE(Res.ModeSuffix.empty());
+  EXPECT_TRUE(Res.DriverMode == nullptr);
+  EXPECT_FALSE(Res.TargetIsValid);
+
+  Res = ToolChain::getTargetAndModeFromProgramName("x86_64-qqq");
+  

Re: [PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via cfe-commits]

2017-08-07 1:46 GMT+07:00 Richard Smith :

> On 6 August 2017 at 11:15, Serge Pavlov via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> 2017-08-06 6:43 GMT+07:00 Hal Finkel :
>>
>>> On 07/24/2017 10:18 AM, Serge Pavlov wrote:
>>>
>>> I am thinking about reducing the patch further to leave only the ability
>>> to include config file when clang is called as `target-clang-drivermode`.
>>> It is still useful for cross compilation tasks because:
>>> - It is a convenient way to switch between supported targets,
>>> - SDK producer can ship compiler with a set of appropriate options or
>>> prepare them during installation.
>>> In this case if clang is called as `target-clang-drivermode`, it first
>>> tries to find file `target-drivermode.cfg` or `target.cfg` in a set of
>>> well-known directories, which in minimal case includes the directory where
>>> clang executable resides. If such file is found, options are  read from it,
>>> otherwise only option --target is added as clang does it now.
>>>
>>> This solution has obvious drawbacks:
>>> - User cannot specify config file in command line in the same way as he
>>> can choose a target: `clang --target `,
>>> - On Windows symlinks are implemented as file copy, the solution looks
>>> awkward.
>>> So more or less complete solution needs to allow specifying config file
>>> in command line.
>>>
>>>
>>> I'd rather not reduce the patch in this way, and you didn't describe why
>>> you're considering reducing the patch. Can you please elaborate?
>>>
>>
>> The only intent was to facilitate review process.
>>
>>>
>>> Using `@file` has some problems. Config file is merely a set of options,
>>> just as file included by `@file`. Different include file search is only a
>>> convenience and could be sacrificed. Comments and unused option warning
>>> suppression could be extended for all files included with `@file`. The real
>>> problem is the search path. To be useful, config files must be searched for
>>> in well-known directories, so that meaning of `clang @config_fille` does
>>> not depend on the current directory. So clang must have some rule to
>>> distinguish between config file and traditional use of `@file`. For
>>> instance, if file name ends with `.cfg` and there is a file with this name
>>> in config search directories, this is a config file and it is interpreted a
>>> bit differently. Of course, the file may be specified with full path, but
>>> this way is inconvenient.
>>>
>>>
>>> I see no reason why we can't unify the processing but have different
>>> search-path rules for @file vs. --config file.
>>>
>>
>> Now I think we can use @file without breaking compatibility.
>>
>> libiberty resolves `file` in `@file` always relative to current
>> directory. If such file is not found, it tries to open file with name
>> `@file`. We must keep this behavior for the sake of compatibility.
>>
>
> Do you know of actual software that depends on the fallback working this
> way? That seems very fragile to me, since a command line that uses @foo to
> name the file ./@foo would change meaning if a file named foo were created.
> Perhaps we should consider the fallback to be a mistake, and require files
> whose name starts with @ to be named as ./@filename, just like we do for
> files whose name starts with a hyphen.
>

Most likely if `@foo` is specified and `foo` is not found this is an error.
And indeed, it just `@foo` is needed, it still can be specified using
slightly modified file name.


>
> If after these steps `file` is not found and `file` does not contain
>> directory separator, clang could try to treat `file` as config file and
>> search it using special search path. If such solution is acceptable, we can
>> get rid of `--config`.
>>
>
> If we go this way, I think we should also deprecate the @file -> "open
> file with name ./@file" (warn on it for now, with the intent to remove it
> in a future version).
>

 This is https://reviews.llvm.org/D36420

But... I think the concern about @ vs --config is principally around having
> two different file formats, not about having two different command-line
> syntaxes to specify a file, so this may be addressing a non-issue. And I
> think the different use cases provide a decent argument for using different
> search paths (compiler configs should live with the compiler, @-files are
> expected to be generated by the user or the build system so should be found
> relative to the current directory). Keeping the two separate but with a
> unified format and internal mechanism seems like a good approach to me.
>

Format of both files is very simple. Features like comments and line
concatenation are not specific to config files and can be extended for all
@-file, as you proposed earlier (potential implementaion is in
https://reviews.llvm.org/D36045). In this case the only difference is
nested `@file`, in which `file` is resolved relative to containing config
file, not current directory.And yes, 

Re: [PATCH] D24933: Enable configuration files in clang

[Richard Smith via cfe-commits]

On 6 August 2017 at 11:15, Serge Pavlov via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> 2017-08-06 6:43 GMT+07:00 Hal Finkel :
>
>> On 07/24/2017 10:18 AM, Serge Pavlov wrote:
>>
>> I am thinking about reducing the patch further to leave only the ability
>> to include config file when clang is called as `target-clang-drivermode`.
>> It is still useful for cross compilation tasks because:
>> - It is a convenient way to switch between supported targets,
>> - SDK producer can ship compiler with a set of appropriate options or
>> prepare them during installation.
>> In this case if clang is called as `target-clang-drivermode`, it first
>> tries to find file `target-drivermode.cfg` or `target.cfg` in a set of
>> well-known directories, which in minimal case includes the directory where
>> clang executable resides. If such file is found, options are  read from it,
>> otherwise only option --target is added as clang does it now.
>>
>> This solution has obvious drawbacks:
>> - User cannot specify config file in command line in the same way as he
>> can choose a target: `clang --target `,
>> - On Windows symlinks are implemented as file copy, the solution looks
>> awkward.
>> So more or less complete solution needs to allow specifying config file
>> in command line.
>>
>>
>> I'd rather not reduce the patch in this way, and you didn't describe why
>> you're considering reducing the patch. Can you please elaborate?
>>
>
> The only intent was to facilitate review process.
>
>>
>> Using `@file` has some problems. Config file is merely a set of options,
>> just as file included by `@file`. Different include file search is only a
>> convenience and could be sacrificed. Comments and unused option warning
>> suppression could be extended for all files included with `@file`. The real
>> problem is the search path. To be useful, config files must be searched for
>> in well-known directories, so that meaning of `clang @config_fille` does
>> not depend on the current directory. So clang must have some rule to
>> distinguish between config file and traditional use of `@file`. For
>> instance, if file name ends with `.cfg` and there is a file with this name
>> in config search directories, this is a config file and it is interpreted a
>> bit differently. Of course, the file may be specified with full path, but
>> this way is inconvenient.
>>
>>
>> I see no reason why we can't unify the processing but have different
>> search-path rules for @file vs. --config file.
>>
>
> Now I think we can use @file without breaking compatibility.
>
> libiberty resolves `file` in `@file` always relative to current directory.
> If such file is not found, it tries to open file with name `@file`. We must
> keep this behavior for the sake of compatibility.
>

Do you know of actual software that depends on the fallback working this
way? That seems very fragile to me, since a command line that uses @foo to
name the file ./@foo would change meaning if a file named foo were created.
Perhaps we should consider the fallback to be a mistake, and require files
whose name starts with @ to be named as ./@filename, just like we do for
files whose name starts with a hyphen.

If after these steps `file` is not found and `file` does not contain
> directory separator, clang could try to treat `file` as config file and
> search it using special search path. If such solution is acceptable, we can
> get rid of `--config`.
>

If we go this way, I think we should also deprecate the @file -> "open file
with name ./@file" (warn on it for now, with the intent to remove it in a
future version).

But... I think the concern about @ vs --config is principally around having
two different file formats, not about having two different command-line
syntaxes to specify a file, so this may be addressing a non-issue. And I
think the different use cases provide a decent argument for using different
search paths (compiler configs should live with the compiler, @-files are
expected to be generated by the user or the build system so should be found
relative to the current directory). Keeping the two separate but with a
unified format and internal mechanism seems like a good approach to me.

> Another possible solution is to extend meaning of `--target` so that it
>> fully matches with the use of `target-clang-drivermode`, that is the option
>> `--target=hexagon` causes clang first to look for the file `hexagon.cfg` in
>> well-known directories and use it if found. In this case treatment of
>> `--target` is different if the option is specified in command line or in
>> the content of config file (in the latter case it is processed as target
>> name only), it may be confusing. Besides, use of config files is not
>> restricted to the choice of target.
>>
>>
>> I think we should do this, so long as the implementation is reasonable,
>> and the special case doesn't bother me in this regard. I don't view this as
>> a replacement for '--config file', however, because, as 

Re: [PATCH] D24933: Enable configuration files in clang

[Hal Finkel via cfe-commits]

On 08/06/2017 01:15 PM, Serge Pavlov wrote:
2017-08-06 6:43 GMT+07:00 Hal Finkel >:


On 07/24/2017 10:18 AM, Serge Pavlov wrote:


I am thinking about reducing the patch further to leave only the
ability to include config file when clang is called as
`target-clang-drivermode`. It is still useful for cross
compilation tasks because:
- It is a convenient way to switch between supported targets,
- SDK producer can ship compiler with a set of appropriate
options or prepare them during installation.
In this case if clang is called as `target-clang-drivermode`, it
first tries to find file `target-drivermode.cfg` or `target.cfg`
in a set of well-known directories, which in minimal case
includes the directory where clang executable resides. If such
file is found, options are  read from it, otherwise only option
--target is added as clang does it now.

This solution has obvious drawbacks:
- User cannot specify config file in command line in the same way
as he can choose a target: `clang --target `,
- On Windows symlinks are implemented as file copy, the solution
looks awkward.
So more or less complete solution needs to allow specifying
config file in command line.


I'd rather not reduce the patch in this way, and you didn't
describe why you're considering reducing the patch. Can you please
elaborate?


The only intent was to facilitate review process.


As someone who's worked on reviewing the patches, I don't think this 
makes things any easier or harder. Once we decide on what we want to do, 
the rest of the review process should be straightforward.




Using `@file` has some problems. Config file is merely a set of
options, just as file included by `@file`. Different include file
search is only a convenience and could be sacrificed. Comments
and unused option warning suppression could be extended for all
files included with `@file`. The real problem is the search path.
To be useful, config files must be searched for in well-known
directories, so that meaning of `clang @config_fille` does not
depend on the current directory. So clang must have some rule to
distinguish between config file and traditional use of `@file`.
For instance, if file name ends with `.cfg` and there is a file
with this name in config search directories, this is a config
file and it is interpreted a bit differently. Of course, the file
may be specified with full path, but this way is inconvenient.


I see no reason why we can't unify the processing but have
different search-path rules for @file vs. --config file.


Now I think we can use @file without breaking compatibility.

libiberty resolves `file` in `@file` always relative to current 
directory. If such file is not found, it tries to open file with name 
`@file`. We must keep this behavior for the sake of compatibility. If 
after these steps `file` is not found and `file` does not contain 
directory separator, clang could try to treat `file` as config file 
and search it using special search path. If such solution is 
acceptable, we can get rid of `--config`.


I think that I'd prefer --config to this scheme. For one thing, it means 
that if I have a wrapper script that adds --config foo, this will break 
if the user happens to have a file named foo in their directory. I think 
that unifying the implementation of @foo and --config foo is a good 
idea, but combining them all into the same interface is not obviously 
optimal.


Thanks again,
Hal




Another possible solution is to extend meaning of `--target` so
that it fully matches with the use of `target-clang-drivermode`,
that is the option `--target=hexagon` causes clang first to look
for the file `hexagon.cfg` in well-known directories and use it
if found. In this case treatment of `--target` is different if
the option is specified in command line or in the content of
config file (in the latter case it is processed as target name
only), it may be confusing. Besides, use of config files is not
restricted to the choice of target.


I think we should do this, so long as the implementation is
reasonable, and the special case doesn't bother me in this regard.
I don't view this as a replacement for '--config file', however,
because, as you mention, the config files need not be restricted
to target triples.


Different treatment of  `--target` in config file and in command line 
is still a concern, to do or not to do this depends on which is looks 
more intuitive. I would try implementing it is a separate patch.


Thanks,
--Serge


Thanks again,
Hal



Using special option for config files does not bring risk of
compatibility breakage and does not change meaning of existing
options.


Thanks,
--Serge

2017-05-10 11:25 GMT+07:00 Serge Pavlov 

Re: [PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via cfe-commits]

2017-08-06 6:43 GMT+07:00 Hal Finkel :

> On 07/24/2017 10:18 AM, Serge Pavlov wrote:
>
> I am thinking about reducing the patch further to leave only the ability
> to include config file when clang is called as `target-clang-drivermode`.
> It is still useful for cross compilation tasks because:
> - It is a convenient way to switch between supported targets,
> - SDK producer can ship compiler with a set of appropriate options or
> prepare them during installation.
> In this case if clang is called as `target-clang-drivermode`, it first
> tries to find file `target-drivermode.cfg` or `target.cfg` in a set of
> well-known directories, which in minimal case includes the directory where
> clang executable resides. If such file is found, options are  read from it,
> otherwise only option --target is added as clang does it now.
>
> This solution has obvious drawbacks:
> - User cannot specify config file in command line in the same way as he
> can choose a target: `clang --target `,
> - On Windows symlinks are implemented as file copy, the solution looks
> awkward.
> So more or less complete solution needs to allow specifying config file in
> command line.
>
>
> I'd rather not reduce the patch in this way, and you didn't describe why
> you're considering reducing the patch. Can you please elaborate?
>

The only intent was to facilitate review process.

>
> Using `@file` has some problems. Config file is merely a set of options,
> just as file included by `@file`. Different include file search is only a
> convenience and could be sacrificed. Comments and unused option warning
> suppression could be extended for all files included with `@file`. The real
> problem is the search path. To be useful, config files must be searched for
> in well-known directories, so that meaning of `clang @config_fille` does
> not depend on the current directory. So clang must have some rule to
> distinguish between config file and traditional use of `@file`. For
> instance, if file name ends with `.cfg` and there is a file with this name
> in config search directories, this is a config file and it is interpreted a
> bit differently. Of course, the file may be specified with full path, but
> this way is inconvenient.
>
>
> I see no reason why we can't unify the processing but have different
> search-path rules for @file vs. --config file.
>

Now I think we can use @file without breaking compatibility.

libiberty resolves `file` in `@file` always relative to current directory.
If such file is not found, it tries to open file with name `@file`. We must
keep this behavior for the sake of compatibility. If after these steps
`file` is not found and `file` does not contain directory separator, clang
could try to treat `file` as config file and search it using special search
path. If such solution is acceptable, we can get rid of `--config`.

Another possible solution is to extend meaning of `--target` so that it
> fully matches with the use of `target-clang-drivermode`, that is the option
> `--target=hexagon` causes clang first to look for the file `hexagon.cfg` in
> well-known directories and use it if found. In this case treatment of
> `--target` is different if the option is specified in command line or in
> the content of config file (in the latter case it is processed as target
> name only), it may be confusing. Besides, use of config files is not
> restricted to the choice of target.
>
>
> I think we should do this, so long as the implementation is reasonable,
> and the special case doesn't bother me in this regard. I don't view this as
> a replacement for '--config file', however, because, as you mention, the
> config files need not be restricted to target triples.
>

Different treatment of  `--target` in config file and in command line is
still a concern, to do or not to do this depends on which is looks more
intuitive. I would try implementing it is a separate patch.

Thanks,
--Serge



>
> Thanks again,
> Hal
>
>
> Using special option for config files does not bring risk of compatibility
> breakage and does not change meaning of existing options.
>
>
> Thanks,
> --Serge
>
> 2017-05-10 11:25 GMT+07:00 Serge Pavlov :
>
>> 2017-05-10 3:46 GMT+07:00 Richard Smith :
>>
>>> On 1 March 2017 at 02:50, Serge Pavlov via Phabricator <
>>> revi...@reviews.llvm.org> wrote:
>>>

 Format of configuration file is similar to file used in the construct
 `@file`, it is a set of options. Configuration file have advantage over
 this construct:

 - it is searched for in well-known places rather than in current
 directory,

>>>
>>> This (and suppressing unused-argument warnings) might well be sufficient
>>> to justify a different command-line syntax rather than @file...
>>>
>>
>> Construct `@file` in this implementation is used only to read parts of
>> config file inside containing file. Driver knows that it processes config
>> file and can adjust treatment of 

Re: [PATCH] D24933: Enable configuration files in clang

[Hal Finkel via cfe-commits]

On 07/24/2017 10:18 AM, Serge Pavlov wrote:
I am thinking about reducing the patch further to leave only the 
ability to include config file when clang is called as 
`target-clang-drivermode`. It is still useful for cross compilation 
tasks because:

- It is a convenient way to switch between supported targets,
- SDK producer can ship compiler with a set of appropriate options or 
prepare them during installation.
In this case if clang is called as `target-clang-drivermode`, it first 
tries to find file `target-drivermode.cfg` or `target.cfg` in a set of 
well-known directories, which in minimal case includes the directory 
where clang executable resides. If such file is found, options are 
 read from it, otherwise only option --target is added as clang does 
it now.


This solution has obvious drawbacks:
- User cannot specify config file in command line in the same way as 
he can choose a target: `clang --target `,
- On Windows symlinks are implemented as file copy, the solution looks 
awkward.
So more or less complete solution needs to allow specifying config 
file in command line.


I'd rather not reduce the patch in this way, and you didn't describe why 
you're considering reducing the patch. Can you please elaborate?




Using `@file` has some problems. Config file is merely a set of 
options, just as file included by `@file`. Different include file 
search is only a convenience and could be sacrificed. Comments and 
unused option warning suppression could be extended for all files 
included with `@file`. The real problem is the search path. To be 
useful, config files must be searched for in well-known directories, 
so that meaning of `clang @config_fille` does not depend on the 
current directory. So clang must have some rule to distinguish between 
config file and traditional use of `@file`. For instance, if file name 
ends with `.cfg` and there is a file with this name in config search 
directories, this is a config file and it is interpreted a bit 
differently. Of course, the file may be specified with full path, but 
this way is inconvenient.


I see no reason why we can't unify the processing but have different 
search-path rules for @file vs. --config file.




Another possible solution is to extend meaning of `--target` so that 
it fully matches with the use of `target-clang-drivermode`, that is 
the option `--target=hexagon` causes clang first to look for the file 
`hexagon.cfg` in well-known directories and use it if found. In this 
case treatment of `--target` is different if the option is specified 
in command line or in the content of config file (in the latter case 
it is processed as target name only), it may be confusing. Besides, 
use of config files is not restricted to the choice of target.


I think we should do this, so long as the implementation is reasonable, 
and the special case doesn't bother me in this regard. I don't view this 
as a replacement for '--config file', however, because, as you mention, 
the config files need not be restricted to target triples.


Thanks again,
Hal



Using special option for config files does not bring risk of 
compatibility breakage and does not change meaning of existing options.



Thanks,
--Serge

2017-05-10 11:25 GMT+07:00 Serge Pavlov >:


2017-05-10 3:46 GMT+07:00 Richard Smith >:

On 1 March 2017 at 02:50, Serge Pavlov via Phabricator
>
wrote:


Format of configuration file is similar to file used in
the construct `@file`, it is a set of options.
Configuration file have advantage over this construct:

- it is searched for in well-known places rather than in
current directory,


This (and suppressing unused-argument warnings) might well be
sufficient to justify a different command-line syntax rather
than @file...


Construct `@file` in this implementation is used only to read
parts of config file inside containing file. Driver knows that it
processes config file and can adjust treatment of `@file`. On the
other hand, driver might parse config files in a more complicated
way, for instance, it could treat line `# include(file_name)` as a
command to include another file.

- it may contain comments, long options may be split
between lines using trailing backslashes,
- other files may be included by `@file` and they will be
resolved relative to the including file,


... but I think we should just add these extensions to our
@file handling, and then use the exact same syntax and code to
handle config files and @file files. That is, the difference
between @ and --config would be that the latter looks in a
different directory and suppresses "unused argument" 

Re: [PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via cfe-commits]

I am thinking about reducing the patch further to leave only the ability to
include config file when clang is called as `target-clang-drivermode`. It
is still useful for cross compilation tasks because:
- It is a convenient way to switch between supported targets,
- SDK producer can ship compiler with a set of appropriate options or
prepare them during installation.
In this case if clang is called as `target-clang-drivermode`, it first
tries to find file `target-drivermode.cfg` or `target.cfg` in a set of
well-known directories, which in minimal case includes the directory where
clang executable resides. If such file is found, options are  read from it,
otherwise only option --target is added as clang does it now.

This solution has obvious drawbacks:
- User cannot specify config file in command line in the same way as he can
choose a target: `clang --target `,
- On Windows symlinks are implemented as file copy, the solution looks
awkward.
So more or less complete solution needs to allow specifying config file in
command line.

Using `@file` has some problems. Config file is merely a set of options,
just as file included by `@file`. Different include file search is only a
convenience and could be sacrificed. Comments and unused option warning
suppression could be extended for all files included with `@file`. The real
problem is the search path. To be useful, config files must be searched for
in well-known directories, so that meaning of `clang @config_fille` does
not depend on the current directory. So clang must have some rule to
distinguish between config file and traditional use of `@file`. For
instance, if file name ends with `.cfg` and there is a file with this name
in config search directories, this is a config file and it is interpreted a
bit differently. Of course, the file may be specified with full path, but
this way is inconvenient.

Another possible solution is to extend meaning of `--target` so that it
fully matches with the use of `target-clang-drivermode`, that is the option
`--target=hexagon` causes clang first to look for the file `hexagon.cfg` in
well-known directories and use it if found. In this case treatment of
`--target` is different if the option is specified in command line or in
the content of config file (in the latter case it is processed as target
name only), it may be confusing. Besides, use of config files is not
restricted to the choice of target.

Using special option for config files does not bring risk of compatibility
breakage and does not change meaning of existing options.


Thanks,
--Serge

2017-05-10 11:25 GMT+07:00 Serge Pavlov :

> 2017-05-10 3:46 GMT+07:00 Richard Smith :
>
>> On 1 March 2017 at 02:50, Serge Pavlov via Phabricator <
>> revi...@reviews.llvm.org> wrote:
>>
>>>
>>> Format of configuration file is similar to file used in the construct
>>> `@file`, it is a set of options. Configuration file have advantage over
>>> this construct:
>>>
>>> - it is searched for in well-known places rather than in current
>>> directory,
>>>
>>
>> This (and suppressing unused-argument warnings) might well be sufficient
>> to justify a different command-line syntax rather than @file...
>>
>
> Construct `@file` in this implementation is used only to read parts of
> config file inside containing file. Driver knows that it processes config
> file and can adjust treatment of `@file`. On the other hand, driver might
> parse config files in a more complicated way, for instance, it could treat
> line `# include(file_name)` as a command to include another file.
>
>
>>
>>> - it may contain comments, long options may be split between lines using
>>> trailing backslashes,
>>> - other files may be included by `@file` and they will be resolved
>>> relative to the including file,
>>>
>>
>> ... but I think we should just add these extensions to our @file
>> handling, and then use the exact same syntax and code to handle config
>> files and @file files. That is, the difference between @ and --config would
>> be that the latter looks in a different directory and suppresses "unused
>> argument" warnings, but they would otherwise be identical.
>>
>
> Changing treatment of `@file` can cause compatibility issues, in
> particular, both libiberty and cl resolves file name relative to current
> directory. So driver must deduce that `@file` is used to load config file
> rather than merely to organize arguments. Another difference is that
> `@file` inserts its content in the place where it occurs, while `--config`
> always puts arguments before user specified options. The following
> invocations:
>
> clang --config a.cfg -opt1 -opt2 file1.cpp
> clang -opt1 -opt2 file1.cpp --config a.cfg
>
> are equivalent, but variants with `@file` can have different effect.
>
>
>> - the file may be encoded in executable name,
>>> - unused options from configuration file do not produce warnings.
>>>
>>>
>>> https://reviews.llvm.org/D24933
>>>
>>>
>>>
>>>
>>
>

Re: [PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via cfe-commits]

2017-05-10 3:46 GMT+07:00 Richard Smith :

> On 1 March 2017 at 02:50, Serge Pavlov via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
>>
>> Format of configuration file is similar to file used in the construct
>> `@file`, it is a set of options. Configuration file have advantage over
>> this construct:
>>
>> - it is searched for in well-known places rather than in current
>> directory,
>>
>
> This (and suppressing unused-argument warnings) might well be sufficient
> to justify a different command-line syntax rather than @file...
>

Construct `@file` in this implementation is used only to read parts of
config file inside containing file. Driver knows that it processes config
file and can adjust treatment of `@file`. On the other hand, driver might
parse config files in a more complicated way, for instance, it could treat
line `# include(file_name)` as a command to include another file.


>
>> - it may contain comments, long options may be split between lines using
>> trailing backslashes,
>> - other files may be included by `@file` and they will be resolved
>> relative to the including file,
>>
>
> ... but I think we should just add these extensions to our @file handling,
> and then use the exact same syntax and code to handle config files and
> @file files. That is, the difference between @ and --config would be that
> the latter looks in a different directory and suppresses "unused argument"
> warnings, but they would otherwise be identical.
>

Changing treatment of `@file` can cause compatibility issues, in
particular, both libiberty and cl resolves file name relative to current
directory. So driver must deduce that `@file` is used to load config file
rather than merely to organize arguments. Another difference is that
`@file` inserts its content in the place where it occurs, while `--config`
always puts arguments before user specified options. The following
invocations:

clang --config a.cfg -opt1 -opt2 file1.cpp
clang -opt1 -opt2 file1.cpp --config a.cfg

are equivalent, but variants with `@file` can have different effect.


> - the file may be encoded in executable name,
>> - unused options from configuration file do not produce warnings.
>>
>>
>> https://reviews.llvm.org/D24933
>>
>>
>>
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24933: Enable configuration files in clang

[Richard Smith via cfe-commits]

On 1 March 2017 at 02:50, Serge Pavlov via Phabricator <
revi...@reviews.llvm.org> wrote:

> sepavloff added a comment.
>
> Glad to know that someone is interested in this feature!
> Below is actual proposal.
>
> **Adding named configuration files to clang driver**
>
> A configuration file is a collection of driver options, which are inserted
> into command line before other options specified in the clang invocation.
> It groups related options together and allows specifying them in simpler,
> more flexible and less error prone way than just listing the options
> somewhere in build scripts. Configuration file may be thought as a "macro"
> that names an option set and is expanded when the driver is called.  This
> feature must be helpful when a user need to specify many options, cross
> compilation is likely to be such case.
>
> Configuration file can be specified by either of two methods:
>
> - by command line option `--config `, or
> - by using special executable file names, such as `armv7l-clang`.
>
> If the option `--config` is used, its argument is treated as a path to
> configuration file if it contains a directory separator, otherwise the file
> is searched for in the set of directories described below. If option
> `--config` is absent and clang executable has name in the form
> `armv7l-clang`, driver will search for file `armv7l.cfg` in the same set of
> directories. Similar encoding is already used by clang to specify target.
>
> The set of directories where configuration files are searched for consists
> of at most three directories, checked in this order:
>
> - user directory (like `~/.llvm`),
> - system directory (like `/etc/llvm`),
> - the directory where clang executable resides.
>
> User and system directories are optional, they are reserved for
> distribution or SDK suppliers. By default they are absent, corresponding
> directories can be specified by cmake arguments
> `CLANG_CONFIG_FILE_SYSTEM_DIR` and `CLANG_CONFIG_FILE_USER_DIR`. The first
> found file is used.
>
> Format of configuration file is similar to file used in the construct
> `@file`, it is a set of options. Configuration file have advantage over
> this construct:
>
> - it is searched for in well-known places rather than in current directory,
>

This (and suppressing unused-argument warnings) might well be sufficient to
justify a different command-line syntax rather than @file...


> - it may contain comments, long options may be split between lines using
> trailing backslashes,
> - other files may be included by `@file` and they will be resolved
> relative to the including file,
>

... but I think we should just add these extensions to our @file handling,
and then use the exact same syntax and code to handle config files and
@file files. That is, the difference between @ and --config would be that
the latter looks in a different directory and suppresses "unused argument"
warnings, but they would otherwise be identical.

- the file may be encoded in executable name,
> - unused options from configuration file do not produce warnings.
>
>
> https://reviews.llvm.org/D24933
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 98330.
sepavloff added a comment.
Herald added subscribers: krytarowski, rengolin.

Updated patch


https://reviews.llvm.org/D24933

Files:
  docs/UsersManual.rst
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Config/config.h.cmake
  include/clang/Driver/Driver.h
  include/clang/Driver/Options.td
  include/clang/Driver/ToolChain.h
  lib/Driver/Driver.cpp
  lib/Driver/ToolChain.cpp
  lib/Tooling/Tooling.cpp
  test/Driver/Inputs/config-1.cfg
  test/Driver/Inputs/config-2.cfg
  test/Driver/Inputs/config-2a.cfg
  test/Driver/Inputs/config-3.cfg
  test/Driver/Inputs/config-4.cfg
  test/Driver/Inputs/config-5.cfg
  test/Driver/Inputs/config/config-4.cfg
  test/Driver/config-file-errs.c
  test/Driver/config-file.c
  test/Driver/config-file2.c
  tools/driver/driver.cpp

Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -199,23 +199,19 @@
 extern int cc1as_main(ArrayRef Argv, const char *Argv0,
   void *MainAddr);
 
-static void insertTargetAndModeArgs(StringRef Target, StringRef Mode,
+static void insertTargetAndModeArgs(const ParsedClangName ,
 SmallVectorImpl ,
 std::set ) {
-  if (!Mode.empty()) {
+  if (!NameParts.ModeSuffix.empty()) {
 // Add the mode flag to the arguments.
-auto it = ArgVector.begin();
-if (it != ArgVector.end())
-  ++it;
-ArgVector.insert(it, GetStableCStr(SavedStrings, Mode));
+ArgVector.insert(ArgVector.end(),
+ GetStableCStr(SavedStrings, NameParts.ModeSuffix));
   }
 
-  if (!Target.empty()) {
-auto it = ArgVector.begin();
-if (it != ArgVector.end())
-  ++it;
-const char *arr[] = {"-target", GetStableCStr(SavedStrings, Target)};
-ArgVector.insert(it, std::begin(arr), std::end(arr));
+  if (NameParts.TargetIsValid) {
+const char *arr[] = {"-target", GetStableCStr(SavedStrings,
+  NameParts.TargetPrefix)};
+ArgVector.insert(ArgVector.end(), std::begin(arr), std::end(arr));
   }
 }
 
@@ -323,9 +319,7 @@
   }
 
   llvm::InitializeAllTargets();
-  std::string ProgName = argv[0];
-  std::pair TargetAndMode =
-  ToolChain::getTargetAndModeFromProgramName(ProgName);
+  auto TargetAndMode = ToolChain::getTargetAndModeFromProgramName(argv[0]);
 
   llvm::BumpPtrAllocator A;
   llvm::StringSaver Saver(A);
@@ -338,7 +332,7 @@
   // Finally, our -cc1 tools don't care which tokenization mode we use because
   // response files written by clang will tokenize the same way in either mode.
   bool ClangCLMode = false;
-  if (TargetAndMode.second == "--driver-mode=cl" ||
+  if (TargetAndMode.ModeSuffix == "--driver-mode=cl" ||
   std::find_if(argv.begin(), argv.end(), [](const char *F) {
 return F && strcmp(F, "--driver-mode=cl") == 0;
   }) != argv.end()) {
@@ -447,9 +441,9 @@
 
   Driver TheDriver(Path, llvm::sys::getDefaultTargetTriple(), Diags);
   SetInstallDir(argv, TheDriver, CanonicalPrefixes);
+  TheDriver.setTargetAndMode(TargetAndMode);
 
-  insertTargetAndModeArgs(TargetAndMode.first, TargetAndMode.second, argv,
-  SavedStrings);
+  insertTargetAndModeArgs(TargetAndMode, argv, SavedStrings);
 
   SetBackdoorDriverOutputsFromEnvVars(TheDriver);
 
@@ -491,7 +485,6 @@
   }
 }
   }
-
   Diags.getClient()->finish();
 
   // If any timers were active but haven't been destroyed yet, print their
Index: test/Driver/config-file2.c
===
--- /dev/null
+++ test/Driver/config-file2.c
@@ -0,0 +1,107 @@
+// REQUIRES: shell
+
+//--- Invocation qqq-clang tries to find config file qqq.cfg
+//
+// RUN: mkdir -p %T/testbin
+// RUN: [ ! -s %T/testbin/qqq-clang ] || rm %T/testbin/qqq-clang
+// RUN: ln -s %clang %T/testbin/qqq-clang
+// RUN: echo "-Wundefined-func-template" > %T/testbin/qqq.cfg
+//
+// RUN: %T/testbin/qqq-clang -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s
+//
+// CHECK: Configuration file: {{.*}}/testbin/qqq.cfg
+// CHECK: -Wundefined-func-template
+
+
+//--- Config files are searched for in binary directory as well.
+//
+// RUN: [ ! -s %T/testbin/clang ] || rm %T/testbin/clang
+// RUN: ln -s %clang %T/testbin/clang
+// RUN: echo "-Werror" > %T/testbin/aaa.cfg
+//
+// RUN: %T/testbin/clang --config aaa.cfg -c -no-canonical-prefixes %s -### 2>&1 | FileCheck %s -check-prefix CHECK-BIN
+//
+// CHECK-BIN: Configuration file: {{.*}}/testbin/aaa.cfg
+// CHECK-BIN: -Werror
+
+
+//--- If config file is specified by relative path (workdir/cfg-s2), it is searched for by
+//that path.
+//
+// RUN: mkdir -p %T/workdir
+// RUN: echo "@subdir/cfg-s2" > %T/workdir/cfg-1
+// RUN: mkdir -p %T/workdir/subdir
+// RUN: echo "-Wundefined-var-template" > %T/workdir/subdir/cfg-s2
+//
+// RUN: ( cd %T 

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

Glad to know that someone is interested in this feature!
Below is actual proposal.

**Adding named configuration files to clang driver**

A configuration file is a collection of driver options, which are inserted into 
command line before other options specified in the clang invocation. It groups 
related options together and allows specifying them in simpler, more flexible 
and less error prone way than just listing the options somewhere in build 
scripts. Configuration file may be thought as a "macro" that names an option 
set and is expanded when the driver is called.  This feature must be helpful 
when a user need to specify many options, cross compilation is likely to be 
such case.

Configuration file can be specified by either of two methods:

- by command line option `--config `, or
- by using special executable file names, such as `armv7l-clang`.

If the option `--config` is used, its argument is treated as a path to 
configuration file if it contains a directory separator, otherwise the file is 
searched for in the set of directories described below. If option `--config` is 
absent and clang executable has name in the form `armv7l-clang`, driver will 
search for file `armv7l.cfg` in the same set of directories. Similar encoding 
is already used by clang to specify target.

The set of directories where configuration files are searched for consists of 
at most three directories, checked in this order:

- user directory (like `~/.llvm`),
- system directory (like `/etc/llvm`),
- the directory where clang executable resides.

User and system directories are optional, they are reserved for distribution or 
SDK suppliers. By default they are absent, corresponding directories can be 
specified by cmake arguments `CLANG_CONFIG_FILE_SYSTEM_DIR` and 
`CLANG_CONFIG_FILE_USER_DIR`. The first found file is used.

Format of configuration file is similar to file used in the construct `@file`, 
it is a set of options. Configuration file have advantage over this construct:

- it is searched for in well-known places rather than in current directory,
- it may contain comments, long options may be split between lines using 
trailing backslashes,
- other files may be included by `@file` and they will be resolved relative to 
the including file,
- the file may be encoded in executable name,
- unused options from configuration file do not produce warnings.


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

Hello,
I work on WebAssembly toolchains (including emscripten, which is more or less 
the "cross-compiler SDK" use case). There's a lot of history in this review and 
I haven't looked at the details yet, but does the current summary text reflect 
the current proposal and/or content of the review?
Thanks!


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff marked 2 inline comments as done.
sepavloff added inline comments.



Comment at: lib/Driver/ToolChain.cpp:183
   std::string Target;
-  if (llvm::TargetRegistry::lookupTarget(Prefix, IgnoredError)) {
+  if (!VerifyTarget || llvm::TargetRegistry::lookupTarget(Prefix, 
IgnoredError))
 Target = Prefix;

hfinkel wrote:
> I don't think that we can do it this way; it is a behavior change (we now 
> might try to set the target to some string which did not validate as a known 
> target, whereas we did not previously).
> 
> How about you always return the prefix, but also return a boolean indicating 
> whether or not the prefix is a valid target? Then, after processing the 
> config file, you can clear out the string if it was not a valid target.
Changed implementation of this function. Indeed using explicit flag looks more 
clear than conditional setting target name.



Comment at: tools/driver/driver.cpp:363
+   TargetAndMode.first + ".cfg");
+TargetAndMode.first.clear();
+  }

hfinkel wrote:
> I don't think that you can clear the string here. We might need it later to 
> call insertTargetAndModeArgs.
With new implementation of `ToolChain::getTargetAndModeFromProgramName` it is 
not needed anymore.


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 87229.
sepavloff added a comment.
Herald added a subscriber: klimek.

Updated patch


https://reviews.llvm.org/D24933

Files:
  docs/UsersManual.rst
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Config/config.h.cmake
  include/clang/Driver/Driver.h
  include/clang/Driver/ToolChain.h
  lib/Driver/Driver.cpp
  lib/Driver/ToolChain.cpp
  lib/Tooling/Tooling.cpp
  test/Driver/Inputs/config-1.cfg
  test/Driver/Inputs/config-2.cfg
  test/Driver/Inputs/config-2a.cfg
  test/Driver/Inputs/config-3.cfg
  test/Driver/Inputs/config-4.cfg
  test/Driver/Inputs/config-5.cfg
  test/Driver/Inputs/config/config-4.cfg
  test/Driver/config-file.c
  test/Driver/config-file2.c
  test/Driver/lit.local.cfg
  tools/driver/driver.cpp

Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -199,22 +199,23 @@
 extern int cc1as_main(ArrayRef Argv, const char *Argv0,
   void *MainAddr);
 
-static void insertTargetAndModeArgs(StringRef Target, StringRef Mode,
+static void insertTargetAndModeArgs(const ToolChain::DriverNameParts ,
 SmallVectorImpl ,
 std::set ) {
-  if (!Mode.empty()) {
+  if (!NameParts.ModeSuffix.empty()) {
 // Add the mode flag to the arguments.
 auto it = ArgVector.begin();
 if (it != ArgVector.end())
   ++it;
-ArgVector.insert(it, GetStableCStr(SavedStrings, Mode));
+ArgVector.insert(it, GetStableCStr(SavedStrings, NameParts.ModeSuffix));
   }
 
-  if (!Target.empty()) {
+  if (NameParts.TargetIsValid) {
 auto it = ArgVector.begin();
 if (it != ArgVector.end())
   ++it;
-const char *arr[] = {"-target", GetStableCStr(SavedStrings, Target)};
+const char *arr[] = {"-target", GetStableCStr(SavedStrings,
+  NameParts.TargetPrefix)};
 ArgVector.insert(it, std::begin(arr), std::end(arr));
   }
 }
@@ -305,6 +306,16 @@
   return 1;
 }
 
+// Directories searched for configuration specified by option '--config'.
+static const ArrayRef SearchDirs = {
+#if defined(CLANG_CONFIG_FILE_USER_DIR)
+  CLANG_CONFIG_FILE_USER_DIR,
+#endif
+#if defined(CLANG_CONFIG_FILE_SYSTEM_DIR)
+  CLANG_CONFIG_FILE_SYSTEM_DIR
+#endif
+};
+
 int main(int argc_, const char **argv_) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv_[0]);
   llvm::PrettyStackTraceProgram X(argc_, argv_);
@@ -324,21 +335,52 @@
 
   llvm::InitializeAllTargets();
   std::string ProgName = argv[0];
-  std::pair TargetAndMode =
-  ToolChain::getTargetAndModeFromProgramName(ProgName);
+  auto TargetAndMode = ToolChain::getTargetAndModeFromProgramName(ProgName);
 
   llvm::BumpPtrAllocator A;
   llvm::StringSaver Saver(A);
 
+  // Try reading options from configuration file.
+  llvm::SmallString<128> ConfigFile;
+  bool CfgFound;
+  std::string ErrText;
+
+  // First try config file specified in command line. It has higher priority
+  // than any other way to specify configuration.
+  CfgFound = llvm::cl::findConfigFile(ConfigFile, argv, SearchDirs, true,
+  ErrText);
+  if (!CfgFound && !ErrText.empty()) {
+llvm::errs() << ProgName << ": CommandLine Error :" << ErrText << '\n';
+return 1;
+  }
+
+  // If config file is not specified explicitly, try to determine configuration
+  // implicitly. First try to deduce configuration from executable name. For
+  // instance, a file 'armv7l-clang' applies config file 'armv7l.cfg'.
+  if (!CfgFound && !TargetAndMode.TargetPrefix.empty())
+CfgFound = llvm::cl::searchForFile(ConfigFile, SearchDirs, ProgName,
+   TargetAndMode.TargetPrefix + ".cfg");
+
+  // If config file is found, read options from it.
+  unsigned NumConfigOptions = 0;
+  if (CfgFound) {
+if (!llvm::cl::readConfigFile(ConfigFile, Saver, argv, NumConfigOptions)) {
+  llvm::errs() << ProgName <<
+": CommandLine Error : Cannot read configuration file '" <<
+ConfigFile.c_str() << "'\n";
+  return 1;
+}
+  }
+
   // Parse response files using the GNU syntax, unless we're in CL mode. There
   // are two ways to put clang in CL compatibility mode: argv[0] is either
   // clang-cl or cl, or --driver-mode=cl is on the command line. The normal
   // command line parsing can't happen until after response file parsing, so we
   // have to manually search for a --driver-mode=cl argument the hard way.
   // Finally, our -cc1 tools don't care which tokenization mode we use because
   // response files written by clang will tokenize the same way in either mode.
   bool ClangCLMode = false;
-  if (TargetAndMode.second == "--driver-mode=cl" ||
+  if (TargetAndMode.ModeSuffix == "--driver-mode=cl" ||
   std::find_if(argv.begin(), argv.end(), [](const char *F) {
 return F && strcmp(F, 

[PATCH] D24933: Enable configuration files in clang

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added inline comments.



Comment at: include/clang/Driver/Driver.h:219
+  /// This number may be smaller that \c NumConfigOptions if some options
+  /// requires separate arguments.
+  unsigned NumConfigArgs;

requires -> require



Comment at: lib/Driver/ToolChain.cpp:183
   std::string Target;
-  if (llvm::TargetRegistry::lookupTarget(Prefix, IgnoredError)) {
+  if (!VerifyTarget || llvm::TargetRegistry::lookupTarget(Prefix, 
IgnoredError))
 Target = Prefix;

I don't think that we can do it this way; it is a behavior change (we now might 
try to set the target to some string which did not validate as a known target, 
whereas we did not previously).

How about you always return the prefix, but also return a boolean indicating 
whether or not the prefix is a valid target? Then, after processing the config 
file, you can clear out the string if it was not a valid target.



Comment at: tools/driver/driver.cpp:363
+   TargetAndMode.first + ".cfg");
+TargetAndMode.first.clear();
+  }

I don't think that you can clear the string here. We might need it later to 
call insertTargetAndModeArgs.



Comment at: tools/driver/driver.cpp:449
+  // there are options read from config file, put the options from "CL"
+  // after them, - config file is considered as a "patch" to compiler
+  // defaults.

Replace
  after them, - config file
with
  after them because the config file



https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 86761.
sepavloff added a comment.
Herald added a subscriber: danalbert.

Updated patch

Use more robust algorithm to determine custom compiler prefix.
Bring the code in sync with changes in llvm patch.


https://reviews.llvm.org/D24933

Files:
  docs/UsersManual.rst
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Config/config.h.cmake
  include/clang/Driver/Driver.h
  include/clang/Driver/ToolChain.h
  lib/Driver/Driver.cpp
  lib/Driver/ToolChain.cpp
  test/Driver/Inputs/config-1.cfg
  test/Driver/Inputs/config-2.cfg
  test/Driver/Inputs/config-2a.cfg
  test/Driver/Inputs/config-3.cfg
  test/Driver/Inputs/config-4.cfg
  test/Driver/Inputs/config-5.cfg
  test/Driver/Inputs/config/config-4.cfg
  test/Driver/config-file.c
  test/Driver/config-file2.c
  test/Driver/lit.local.cfg
  tools/driver/driver.cpp

Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -305,6 +305,16 @@
   return 1;
 }
 
+// Directories searched for configuration specified by option '--config'.
+static const ArrayRef SearchDirs = {
+#if defined(CLANG_CONFIG_FILE_USER_DIR)
+  CLANG_CONFIG_FILE_USER_DIR,
+#endif
+#if defined(CLANG_CONFIG_FILE_SYSTEM_DIR)
+  CLANG_CONFIG_FILE_SYSTEM_DIR
+#endif
+};
+
 int main(int argc_, const char **argv_) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv_[0]);
   llvm::PrettyStackTraceProgram X(argc_, argv_);
@@ -325,11 +335,45 @@
   llvm::InitializeAllTargets();
   std::string ProgName = argv[0];
   std::pair TargetAndMode =
-  ToolChain::getTargetAndModeFromProgramName(ProgName);
+  ToolChain::getTargetAndModeFromProgramName(ProgName, false);
 
   llvm::BumpPtrAllocator A;
   llvm::StringSaver Saver(A);
 
+  // Try reading options from configuration file.
+  llvm::SmallString<128> ConfigFile;
+  bool CfgFound;
+  std::string ErrText;
+
+  // First try config file specified in command line. It has higher priority
+  // than any other way to specify configuration.
+  CfgFound = llvm::cl::findConfigFile(ConfigFile, argv, SearchDirs, true,
+  ErrText);
+  if (!CfgFound && !ErrText.empty()) {
+llvm::errs() << ProgName << ": CommandLine Error :" << ErrText << '\n';
+return 1;
+  }
+
+  // If config file is not specified explicitly, try to determine configuration
+  // implicitly. First try to deduce configuration from executable name. For
+  // instance, a file 'armv7l-clang' applies config file 'armv7l.cfg'.
+  if (!CfgFound && !TargetAndMode.first.empty()) {
+CfgFound = llvm::cl::searchForFile(ConfigFile, SearchDirs, ProgName,
+   TargetAndMode.first + ".cfg");
+TargetAndMode.first.clear();
+  }
+
+  // If config file is found, read options from it.
+  unsigned NumConfigOptions = 0;
+  if (CfgFound) {
+if (!llvm::cl::readConfigFile(ConfigFile, Saver, argv, NumConfigOptions)) {
+  llvm::errs() << ProgName <<
+": CommandLine Error : Cannot read configuration file '" <<
+ConfigFile.c_str() << "'\n";
+  return 1;
+}
+  }
+
   // Parse response files using the GNU syntax, unless we're in CL mode. There
   // are two ways to put clang in CL compatibility mode: argv[0] is either
   // clang-cl or cl, or --driver-mode=cl is on the command line. The normal
@@ -400,8 +444,12 @@
   SmallVector PrependedOpts;
   getCLEnvVarOptions(OptCL.getValue(), Saver, PrependedOpts);
 
-  // Insert right after the program name to prepend to the argument list.
-  argv.insert(argv.begin() + 1, PrependedOpts.begin(), PrependedOpts.end());
+  // Insert right after the program name to prepend to the argument list. If
+  // there are options read from config file, put the options from "CL"
+  // after them, - config file is considered as a "patch" to compiler
+  // defaults.
+  argv.insert(argv.begin() + 1 + NumConfigOptions,
+  PrependedOpts.begin(), PrependedOpts.end());
 }
 // Arguments in "_CL_" are appended.
 llvm::Optional Opt_CL_ = llvm::sys::Process::GetEnv("_CL_");
@@ -446,6 +494,8 @@
   ProcessWarningOptions(Diags, *DiagOpts, /*ReportDiags=*/false);
 
   Driver TheDriver(Path, llvm::sys::getDefaultTargetTriple(), Diags);
+  if (!ConfigFile.empty())
+TheDriver.setConfigFile(ConfigFile.str(), NumConfigOptions);
   SetInstallDir(argv, TheDriver, CanonicalPrefixes);
 
   insertTargetAndModeArgs(TargetAndMode.first, TargetAndMode.second, argv,
Index: test/Driver/lit.local.cfg
===
--- test/Driver/lit.local.cfg
+++ test/Driver/lit.local.cfg
@@ -4,6 +4,8 @@
 config.substitutions.insert(0,
 ('%clang_cc1',
  """*** Do not use 'clang -cc1' in Driver tests. ***""") )
+config.substitutions.append( ('%bindir',
+  os.path.dirname(config.clang)) )
 
 # Remove harmful 

[PATCH] D24933: Enable configuration files in clang

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added inline comments.



Comment at: lib/Driver/Driver.cpp:172
+NumConfigArgs = static_cast(NumCfgArgs);
+  }
+

sepavloff wrote:
> bruno wrote:
> > If `NumCfgArgs == 0` it would be nice to warn that the config file is empty?
> Not sure if it makes sense. Usually warning are used to attract attention to 
> the things that potentially may be harmful. An empty config file is strange 
> but does not look dangerous. 
I agree. The config files might be automatically generated by wrapper scripts, 
and adding special cases to avoid creating empty files seems like an 
unnecessary complication.



Comment at: lib/Driver/Driver.cpp:2695
 
+  // Clain all arguments that come from a configuration file so that the driver
+  // does not warn on any that are unused.

Clain -> Claim



Comment at: tools/driver/driver.cpp:332
+  StringRef PName = llvm::sys::path::stem(ProgramName);
+  size_t Pos = PName.find("-clang");
+  if (Pos == StringRef::npos)

Looking for "-clang" is too specific; our driver-name suffix processing allows 
more general naming than this. For one thing, it will not pick up armv7l-cpp 
correctly (which would be a problem because the config files likely contain 
options affecting preprocessing). I also have users which use symlinks to clang 
named fooclang.

I think that an easy way to do this is to use the result from 
ToolChain::getTargetAndModeFromProgramName, which we call from the caller of 
this function:

  std::string ProgName = argv[0];
  std::pair TargetAndMode =
  ToolChain::getTargetAndModeFromProgramName(ProgName);

We can use TargetAndMode.first here, instead of looking for the string before 
"-clang", and that should be consistent with the rest of our processing.



https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added inline comments.



Comment at: include/clang/Driver/Driver.h:287
+  const std::string () const { return ConfigFile; }
+  void setConfigFile(StringRef x, unsigned N) {
+ConfigFile = x;

bruno wrote:
> x -> FileName
Fixed.



Comment at: lib/Driver/Driver.cpp:172
+NumConfigArgs = static_cast(NumCfgArgs);
+  }
+

bruno wrote:
> If `NumCfgArgs == 0` it would be nice to warn that the config file is empty?
Not sure if it makes sense. Usually warning are used to attract attention to 
the things that potentially may be harmful. An empty config file is strange but 
does not look dangerous. 



Comment at: lib/Driver/Driver.cpp:2698
+  for (unsigned I = 0; I < NumConfigArgs; ++I)
+C.getArgs().getArgs()[I]->claim();
+

bruno wrote:
> Why shouldn't we warn about those? Should clang warn and point to the config 
> file instead?
If clang is called to compile files only, it will warn on options like 
`-L/usr/local/lib` as they are unused. We don't want such warnings for options 
in config file, as it is used for all invocations of clang.



Comment at: tools/driver/driver.cpp:318
+
+/// Deduce configuration name if it is encoded in the executable name.
+///

bruno wrote:
> Use  `\brief` here?
Doxygen is run with AUTO_BRIEF options turned on (enabled in r242485 - Doxygen: 
Enable autobrief feature, matching llvm config/coding standards).



Comment at: tools/driver/driver.cpp:333
+  size_t Pos = PName.find("-clang");
+  if (Pos != StringRef::npos) {
+ConfigFile.append(PName.begin(), PName.begin() + Pos);

bruno wrote:
> You can early `return false` here if `Pos == StringRef::npos`
Fixed.


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 81227.
sepavloff added a comment.

Addressed review notes.


https://reviews.llvm.org/D24933

Files:
  docs/UsersManual.rst
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Config/config.h.cmake
  include/clang/Driver/Driver.h
  lib/Driver/Driver.cpp
  test/Driver/Inputs/config-1.cfg
  test/Driver/Inputs/config-2.cfg
  test/Driver/Inputs/config-2a.cfg
  test/Driver/Inputs/config-3.cfg
  test/Driver/Inputs/config-4.cfg
  test/Driver/Inputs/config-5.cfg
  test/Driver/Inputs/config/config-4.cfg
  test/Driver/config-file.c
  test/Driver/config-file2.c
  test/Driver/lit.local.cfg
  tools/driver/driver.cpp

Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -305,6 +305,41 @@
   return 1;
 }
 
+// Directories searched for configuration specified by option '--config'.
+static const ArrayRef SearchDirs = {
+#if defined(CLANG_CONFIG_FILE_USER_DIR)
+  CLANG_CONFIG_FILE_USER_DIR,
+#endif
+#if defined(CLANG_CONFIG_FILE_SYSTEM_DIR)
+  CLANG_CONFIG_FILE_SYSTEM_DIR
+#endif
+};
+
+/// Deduce configuration name if it is encoded in the executable name.
+///
+/// \param ConfigFile [out] Is assigned configuration file path.
+/// \param ProgramName [in] clang executable path.
+/// \return True if configuration file was found.
+///
+/// If clang executable is named e.g. 'armv7l-clang' the function tries to
+/// find config file 'armv7l.cfg'. If it is found, its path is put into
+/// ConfigFile and the function returns true.
+///
+static bool findConfigFileFromProgramName(
+llvm::SmallVectorImpl , StringRef ProgramName) {
+  ConfigFile.clear();
+  StringRef PName = llvm::sys::path::stem(ProgramName);
+  size_t Pos = PName.find("-clang");
+  if (Pos == StringRef::npos)
+return false;
+
+  ConfigFile.append(PName.begin(), PName.begin() + Pos);
+  const StringRef Ext(".cfg");
+  ConfigFile.append(Ext.begin(), Ext.end());
+  std::string CName(ConfigFile.begin(), ConfigFile.size());
+  return llvm::cl::searchForFile(ConfigFile, SearchDirs, ProgramName, CName);
+}
+
 int main(int argc_, const char **argv_) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv_[0]);
   llvm::PrettyStackTraceProgram X(argc_, argv_);
@@ -330,6 +365,31 @@
   llvm::BumpPtrAllocator A;
   llvm::StringSaver Saver(A);
 
+  // Try reading options from configuration file.
+  llvm::SmallString<128> ConfigFile;
+  llvm::cl::SearchResult SRes;
+
+  // First try config file specified in command line. It has higher priority
+  // than any other way to specify configuration.
+  SRes = llvm::cl::findConfigFileFromArgs(ConfigFile, argv, SearchDirs, true);
+  if (llvm::cl::checkConfigFileSearchResult(SRes, ConfigFile, SearchDirs, true,
+ProgName))
+return 1;
+
+  // If config file is not specified explicitly, try to determine configuration
+  // implicitly. First try to deduce configuration from executable name. For
+  // instance, a file 'armv7l-clang' applies config file 'armv7l.cfg'. Second,
+  // try to find file 'clang.cfg'.
+  if (SRes == llvm::cl::SearchResult::NotSpecified) {
+if (findConfigFileFromProgramName(ConfigFile, ProgName))
+  SRes = llvm::cl::SearchResult::Successful;
+  }
+
+  // If config file is found, read options from it.
+  unsigned NumConfigOptions = 0;
+  if (SRes == llvm::cl::SearchResult::Successful)
+llvm::cl::readConfigFile(ConfigFile, Saver, argv, NumConfigOptions);
+
   // Parse response files using the GNU syntax, unless we're in CL mode. There
   // are two ways to put clang in CL compatibility mode: argv[0] is either
   // clang-cl or cl, or --driver-mode=cl is on the command line. The normal
@@ -400,8 +460,12 @@
   SmallVector PrependedOpts;
   getCLEnvVarOptions(OptCL.getValue(), Saver, PrependedOpts);
 
-  // Insert right after the program name to prepend to the argument list.
-  argv.insert(argv.begin() + 1, PrependedOpts.begin(), PrependedOpts.end());
+  // Insert right after the program name to prepend to the argument list. If
+  // there are options read from config file, put the options from "CL"
+  // after them, - config file is considered as a "patch" to compiler
+  // defaults.
+  argv.insert(argv.begin() + 1 + NumConfigOptions,
+  PrependedOpts.begin(), PrependedOpts.end());
 }
 // Arguments in "_CL_" are appended.
 llvm::Optional Opt_CL_ = llvm::sys::Process::GetEnv("_CL_");
@@ -446,6 +510,8 @@
   ProcessWarningOptions(Diags, *DiagOpts, /*ReportDiags=*/false);
 
   Driver TheDriver(Path, llvm::sys::getDefaultTargetTriple(), Diags);
+  if (!ConfigFile.empty())
+TheDriver.setConfigFile(ConfigFile.str(), NumConfigOptions);
   SetInstallDir(argv, TheDriver, CanonicalPrefixes);
 
   insertTargetAndModeArgs(TargetAndMode.first, TargetAndMode.second, argv,
Index: test/Driver/lit.local.cfg

[PATCH] D24933: Enable configuration files in clang

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added inline comments.



Comment at: include/clang/Driver/Driver.h:287
+  const std::string () const { return ConfigFile; }
+  void setConfigFile(StringRef x, unsigned N) {
+ConfigFile = x;

x -> FileName



Comment at: lib/Driver/Driver.cpp:172
+NumConfigArgs = static_cast(NumCfgArgs);
+  }
+

If `NumCfgArgs == 0` it would be nice to warn that the config file is empty?



Comment at: lib/Driver/Driver.cpp:2698
+  for (unsigned I = 0; I < NumConfigArgs; ++I)
+C.getArgs().getArgs()[I]->claim();
+

Why shouldn't we warn about those? Should clang warn and point to the config 
file instead?



Comment at: tools/driver/driver.cpp:318
+
+/// Deduce configuration name if it is encoded in the executable name.
+///

Use  `\brief` here?



Comment at: tools/driver/driver.cpp:333
+  size_t Pos = PName.find("-clang");
+  if (Pos != StringRef::npos) {
+ConfigFile.append(PName.begin(), PName.begin() + Pos);

You can early `return false` here if `Pos == StringRef::npos`


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 81057.
sepavloff added a comment.

Fixed grammar in comment, thanks to Hal Finkel.


https://reviews.llvm.org/D24933

Files:
  docs/UsersManual.rst
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Config/config.h.cmake
  include/clang/Driver/Driver.h
  lib/Driver/Driver.cpp
  test/Driver/Inputs/config-1.cfg
  test/Driver/Inputs/config-2.cfg
  test/Driver/Inputs/config-2a.cfg
  test/Driver/Inputs/config-3.cfg
  test/Driver/Inputs/config-4.cfg
  test/Driver/Inputs/config-5.cfg
  test/Driver/Inputs/config/config-4.cfg
  test/Driver/config-file.c
  test/Driver/config-file2.c
  test/Driver/lit.local.cfg
  tools/driver/driver.cpp

Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -305,6 +305,41 @@
   return 1;
 }
 
+// Directories searched for configuration specified by option '--config'.
+static const ArrayRef SearchDirs = {
+#if defined(CLANG_CONFIG_FILE_USER_DIR)
+  CLANG_CONFIG_FILE_USER_DIR,
+#endif
+#if defined(CLANG_CONFIG_FILE_SYSTEM_DIR)
+  CLANG_CONFIG_FILE_SYSTEM_DIR
+#endif
+};
+
+/// Deduce configuration name if it is encoded in the executable name.
+///
+/// \param ConfigFile [out] Is assigned configuration file path.
+/// \param ProgramName [in] clang executable path.
+/// \return True if configuration file was found.
+///
+/// If clang executable is named e.g. 'armv7l-clang' the function tries to
+/// find config file 'armv7l.cfg'. If it is found, its path is put into
+/// ConfigFile and the function returns true.
+///
+static bool findConfigFileFromProgramName(
+llvm::SmallVectorImpl , StringRef ProgramName) {
+  ConfigFile.clear();
+  StringRef PName = llvm::sys::path::stem(ProgramName);
+  size_t Pos = PName.find("-clang");
+  if (Pos != StringRef::npos) {
+ConfigFile.append(PName.begin(), PName.begin() + Pos);
+const StringRef Ext(".cfg");
+ConfigFile.append(Ext.begin(), Ext.end());
+std::string CName(ConfigFile.begin(), ConfigFile.size());
+return llvm::cl::searchForFile(ConfigFile, SearchDirs, ProgramName, CName);
+  }
+  return false;
+}
+
 int main(int argc_, const char **argv_) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv_[0]);
   llvm::PrettyStackTraceProgram X(argc_, argv_);
@@ -330,6 +365,31 @@
   llvm::BumpPtrAllocator A;
   llvm::StringSaver Saver(A);
 
+  // Try reading options from configuration file.
+  llvm::SmallString<128> ConfigFile;
+  llvm::cl::SearchResult SRes;
+
+  // First try config file specified in command line. It has higher priority
+  // than any other way to specify configuration.
+  SRes = llvm::cl::findConfigFileFromArgs(ConfigFile, argv, SearchDirs, true);
+  if (llvm::cl::checkConfigFileSearchResult(SRes, ConfigFile, SearchDirs, true,
+ProgName))
+return 1;
+
+  // If config file is not specified explicitly, try to determine configuration
+  // implicitly. First try to deduce configuration from executable name. For
+  // instance, a file 'armv7l-clang' applies config file 'armv7l.cfg'. Second,
+  // try to find file 'clang.cfg'.
+  if (SRes == llvm::cl::SearchResult::NotSpecified) {
+if (findConfigFileFromProgramName(ConfigFile, ProgName))
+  SRes = llvm::cl::SearchResult::Successful;
+  }
+
+  // If config file is found, read options from it.
+  unsigned NumConfigOptions = 0;
+  if (SRes == llvm::cl::SearchResult::Successful)
+llvm::cl::readConfigFile(ConfigFile, Saver, argv, NumConfigOptions);
+
   // Parse response files using the GNU syntax, unless we're in CL mode. There
   // are two ways to put clang in CL compatibility mode: argv[0] is either
   // clang-cl or cl, or --driver-mode=cl is on the command line. The normal
@@ -400,8 +460,12 @@
   SmallVector PrependedOpts;
   getCLEnvVarOptions(OptCL.getValue(), Saver, PrependedOpts);
 
-  // Insert right after the program name to prepend to the argument list.
-  argv.insert(argv.begin() + 1, PrependedOpts.begin(), PrependedOpts.end());
+  // Insert right after the program name to prepend to the argument list. If
+  // there are options read from config file, put the options from "CL"
+  // after them, - config file is considered as a "patch" to compiler
+  // defaults.
+  argv.insert(argv.begin() + 1 + NumConfigOptions,
+  PrependedOpts.begin(), PrependedOpts.end());
 }
 // Arguments in "_CL_" are appended.
 llvm::Optional Opt_CL_ = llvm::sys::Process::GetEnv("_CL_");
@@ -446,6 +510,8 @@
   ProcessWarningOptions(Diags, *DiagOpts, /*ReportDiags=*/false);
 
   Driver TheDriver(Path, llvm::sys::getDefaultTargetTriple(), Diags);
+  if (!ConfigFile.empty())
+TheDriver.setConfigFile(ConfigFile.str(), NumConfigOptions);
   SetInstallDir(argv, TheDriver, CanonicalPrefixes);
 
   insertTargetAndModeArgs(TargetAndMode.first, TargetAndMode.second, argv,
Index: test/Driver/lit.local.cfg

[PATCH] D24933: Enable configuration files in clang

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added inline comments.



Comment at: lib/Driver/Driver.cpp:2695
 
+  // Claim all arguments that come from configuration file, - driver must not
+  // warn about unused argument on them.

Grammar here is a bit odd, how about:

  // Claim all arguments that come from a configuration file so that the driver 
does not warn on any that are unused.


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 81055.
sepavloff marked 4 inline comments as done.
sepavloff added a comment.

Updated patch.

- Directories which are searched for config files are now defined during 
project configuration process using special cmake options.
- Driver do not warn on unused arguments that come from config file.


https://reviews.llvm.org/D24933

Files:
  docs/UsersManual.rst
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Config/config.h.cmake
  include/clang/Driver/Driver.h
  lib/Driver/Driver.cpp
  test/Driver/Inputs/config-1.cfg
  test/Driver/Inputs/config-2.cfg
  test/Driver/Inputs/config-2a.cfg
  test/Driver/Inputs/config-3.cfg
  test/Driver/Inputs/config-4.cfg
  test/Driver/Inputs/config-5.cfg
  test/Driver/Inputs/config/config-4.cfg
  test/Driver/config-file.c
  test/Driver/config-file2.c
  test/Driver/lit.local.cfg
  tools/driver/driver.cpp

Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -305,6 +305,41 @@
   return 1;
 }
 
+// Directories searched for configuration specified by option '--config'.
+static const ArrayRef SearchDirs = {
+#if defined(CLANG_CONFIG_FILE_USER_DIR)
+  CLANG_CONFIG_FILE_USER_DIR,
+#endif
+#if defined(CLANG_CONFIG_FILE_SYSTEM_DIR)
+  CLANG_CONFIG_FILE_SYSTEM_DIR
+#endif
+};
+
+/// Deduce configuration name if it is encoded in the executable name.
+///
+/// \param ConfigFile [out] Is assigned configuration file path.
+/// \param ProgramName [in] clang executable path.
+/// \return True if configuration file was found.
+///
+/// If clang executable is named e.g. 'armv7l-clang' the function tries to
+/// find config file 'armv7l.cfg'. If it is found, its path is put into
+/// ConfigFile and the function returns true.
+///
+static bool findConfigFileFromProgramName(
+llvm::SmallVectorImpl , StringRef ProgramName) {
+  ConfigFile.clear();
+  StringRef PName = llvm::sys::path::stem(ProgramName);
+  size_t Pos = PName.find("-clang");
+  if (Pos != StringRef::npos) {
+ConfigFile.append(PName.begin(), PName.begin() + Pos);
+const StringRef Ext(".cfg");
+ConfigFile.append(Ext.begin(), Ext.end());
+std::string CName(ConfigFile.begin(), ConfigFile.size());
+return llvm::cl::searchForFile(ConfigFile, SearchDirs, ProgramName, CName);
+  }
+  return false;
+}
+
 int main(int argc_, const char **argv_) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv_[0]);
   llvm::PrettyStackTraceProgram X(argc_, argv_);
@@ -330,6 +365,31 @@
   llvm::BumpPtrAllocator A;
   llvm::StringSaver Saver(A);
 
+  // Try reading options from configuration file.
+  llvm::SmallString<128> ConfigFile;
+  llvm::cl::SearchResult SRes;
+
+  // First try config file specified in command line. It has higher priority
+  // than any other way to specify configuration.
+  SRes = llvm::cl::findConfigFileFromArgs(ConfigFile, argv, SearchDirs, true);
+  if (llvm::cl::checkConfigFileSearchResult(SRes, ConfigFile, SearchDirs, true,
+ProgName))
+return 1;
+
+  // If config file is not specified explicitly, try to determine configuration
+  // implicitly. First try to deduce configuration from executable name. For
+  // instance, a file 'armv7l-clang' applies config file 'armv7l.cfg'. Second,
+  // try to find file 'clang.cfg'.
+  if (SRes == llvm::cl::SearchResult::NotSpecified) {
+if (findConfigFileFromProgramName(ConfigFile, ProgName))
+  SRes = llvm::cl::SearchResult::Successful;
+  }
+
+  // If config file is found, read options from it.
+  unsigned NumConfigOptions = 0;
+  if (SRes == llvm::cl::SearchResult::Successful)
+llvm::cl::readConfigFile(ConfigFile, Saver, argv, NumConfigOptions);
+
   // Parse response files using the GNU syntax, unless we're in CL mode. There
   // are two ways to put clang in CL compatibility mode: argv[0] is either
   // clang-cl or cl, or --driver-mode=cl is on the command line. The normal
@@ -400,8 +460,12 @@
   SmallVector PrependedOpts;
   getCLEnvVarOptions(OptCL.getValue(), Saver, PrependedOpts);
 
-  // Insert right after the program name to prepend to the argument list.
-  argv.insert(argv.begin() + 1, PrependedOpts.begin(), PrependedOpts.end());
+  // Insert right after the program name to prepend to the argument list. If
+  // there are options read from config file, put the options from "CL"
+  // after them, - config file is considered as a "patch" to compiler
+  // defaults.
+  argv.insert(argv.begin() + 1 + NumConfigOptions,
+  PrependedOpts.begin(), PrependedOpts.end());
 }
 // Arguments in "_CL_" are appended.
 llvm::Optional Opt_CL_ = llvm::sys::Process::GetEnv("_CL_");
@@ -446,6 +510,8 @@
   ProcessWarningOptions(Diags, *DiagOpts, /*ReportDiags=*/false);
 
   Driver TheDriver(Path, llvm::sys::getDefaultTargetTriple(), Diags);
+  if (!ConfigFile.empty())
+

[PATCH] D24933: Enable configuration files in clang

[Michał Górny via cfe-commits]

mgorny added a comment.

In https://reviews.llvm.org/D24933#600884, @sepavloff wrote:

> > Whoever makes the `blah-clang` symlink should get to control what the 
> > default configuration for `blah-clang` is, I think.
>
> The patch is changed so that config file for `blah-clang` is searched for 
> *only* in the directory where `blah-clang` resides. It prevents a user from 
> overwriting 'system' config files. The idea is that the files in the binary 
> directory are prepared by SDK suppliers who adapt clang for specific needs.


I think this killed my use case. Could this be extended to somehow allow 
providing configuration for the compiler installed in standard *nix layout? 
I.e. make it possible to make it search ../../etc/clang or something like that?

>> you can replace a blah-clang symlink with a shell script containing `exec 
>> clang @blah.cfg "@$"`
> 
> Due to intermediate shell the environment variables which were not exported 
> would be lost. This solution is OK for build system but from viewpoint of 
> compiler, which must work in any build system, it is too fragile.

I think you are wrong here, or I'm missing something. When clang is spawned, 
only exported variables are passed to it. An intermediate shell layer won't 
change this.

However, I don't think forcing an additional wrapper for all clang invocations 
is a neat idea.


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via cfe-commits]

sepavloff marked 5 inline comments as done.
sepavloff added a comment.

> Rather than inventing a new mechanism, could we extend our existing `@file` 
> facility to support comments and nested inclusion of further `@file`s?

Reusing `@file` is an attractive idea, but it cannot be implemented due to 
compatibility reason. The file in `@file` is resolved relative to current 
directory in both msvc and libiberty. Current directory may change during 
build, so use of `@file` to deliver set of flags to every invocation of clang 
becomes unrealistic. Config files instead are taken from well-known places and 
if a user set `CFLAGS='--config abc.cfg'` in call to make, it should work as 
intended.

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

I believe no, current directory is not reliable place for configurations, as it 
may change during build.

> Whoever makes the `blah-clang` symlink should get to control what the default 
> configuration for `blah-clang` is, I think.

The patch is changed so that config file for `blah-clang` is searched for 
*only* in the directory where `blah-clang` resides. It prevents a user from 
overwriting 'system' config files. The idea is that the files in the binary 
directory are prepared by SDK suppliers who adapt clang for specific needs.

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

The patch is changed so that `~/.llvm` is searched only of config files 
specified explicitly by `--config`. A user can specify full path to the config 
file: `clang --config ~/.llvm/abc.cfg`. By making the search in `~/.llvm` we 
make work with config files a bit more convenient.

> you can replace a blah-clang symlink with a shell script containing `exec 
> clang @blah.cfg "@$"`

Due to intermediate shell the environment variables which were not exported 
would be lost. This solution is OK for build system but from viewpoint of 
compiler, which must work in any build system, it is too fragile.




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.

srhines wrote:
> rsmith wrote:
> > hans wrote:
> > > 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`.
> > I'm also not keen on searching `~/.llvm`. Whoever makes the `blah-clang` 
> > symlink should get to control what the default configuration for 
> > `blah-clang` is, I think. But then this seems to immediately lead to the 
> > conclusion that we don't need this implicit-config-file feature at all, 
> > since you can replace a `blah-clang` symlink with a shell script containing 
> > `exec clang @blah.cfg "@$"` -- and it's better to handle it that way, since 
> > you get more control over where the config file lives.
> Android is essentially taking the shell script wrapper approach today 
> (although we are using python, and we don't do a lot with it just yet).
Agree, removed this feature.



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`.

hans wrote:
> I guess you mean directive `@os/linux.opts`?
Yes, thank you for the catch.



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:

hans wrote:
> This test will fail if I have a `~/.llvm/inexistent.cfg` file on my machine.
Changed the name to more long and awkward to reduce chance of clash.



Comment at: test/Driver/config-file.c:13
+// CHECK-INEX-NOSEARCH: ~/.llvm
+// CHECK-INEX-NOSEARCH: /etc/llvm
+

hans wrote:
> Where did `/etc/llvm` come from?
Remnants of the previous variant, removed.



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

hans wrote:
> There is no test checking the functionality that finds a config file based on 
> the clang executable name.
Added tests (file config-file2.c).


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D24933: Enable configuration files in clang

[Hal Finkel via cfe-commits]

hfinkel added a comment.

What happens with unused arguments in the configuration files? This feature 
looks potentially useful for me, but only if it suppresses unused-argument 
warnings. For example, if I put

  -L/usr/local/lib -stdlib=libc++

into a configuration file, I can't have these:

  clang-4.0: warning: argument unused during compilation: '-L/usr/local/lib' 
[-Wunused-command-line-argument]
  clang-4.0: warning: argument unused during compilation: '-stdlib=libc++' 
[-Wunused-command-line-argument]

pop up when compiling all over the place.


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via cfe-commits]

sepavloff updated this revision to Diff 78652.
sepavloff added a comment.
Herald added a subscriber: aemerson.

Updated patch.

- Restrict the search for config files specified by mangled clang name to 
binary directory only,
- Allow the search for config files specified by `--config` in binary directory 
also,
- Added tests for using mangled clang names and nested file inclusion.


https://reviews.llvm.org/D24933

Files:
  docs/UsersManual.rst
  include/clang/Driver/Driver.h
  lib/Driver/Driver.cpp
  test/Driver/Inputs/config-1.cfg
  test/Driver/Inputs/config-2.cfg
  test/Driver/Inputs/config-3.cfg
  test/Driver/Inputs/config/config-4.cfg
  test/Driver/config-file.c
  test/Driver/config-file2.c
  test/Driver/lit.local.cfg
  tools/driver/driver.cpp

Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -305,6 +305,35 @@
   return 1;
 }
 
+// Directories searched for configuration specified by option '--config'.
+static const char * const SearchDirs[] = { "~/.llvm" };
+
+/// Deduce configuration name if it is encoded in the executable name.
+///
+/// \param ConfigFile [out] Is assigned configuration file path.
+/// \param ProgramName [in] clang executable path.
+/// \return True if configuration file was found.
+///
+/// If clang executable is named e.g. 'armv7l-clang' the function tries to
+/// find config file 'armv7l.cfg'. If it is found, its path is put into
+/// ConfigFile and the function returns true.
+///
+static bool findConfigFileFromProgramName(
+llvm::SmallVectorImpl , StringRef ProgramName) {
+  ConfigFile.clear();
+  StringRef PName = llvm::sys::path::stem(ProgramName);
+  size_t Pos = PName.find("-clang");
+  if (Pos != StringRef::npos) {
+ConfigFile.append(PName.begin(), PName.begin() + Pos);
+const StringRef Ext(".cfg");
+ConfigFile.append(Ext.begin(), Ext.end());
+std::string CName(ConfigFile.begin(), ConfigFile.size());
+return llvm::cl::searchForFile(ConfigFile, ArrayRef(),
+   ProgramName, CName);
+  }
+  return false;
+}
+
 int main(int argc_, const char **argv_) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv_[0]);
   llvm::PrettyStackTraceProgram X(argc_, argv_);
@@ -330,6 +359,31 @@
   llvm::BumpPtrAllocator A;
   llvm::StringSaver Saver(A);
 
+  // Try reading options from configuration file.
+  llvm::SmallString<128> ConfigFile;
+  llvm::cl::SearchResult SRes;
+
+  // First try config file specified in command line. It has higher priority
+  // than any other way to specify configuration.
+  SRes = llvm::cl::findConfigFileFromArgs(ConfigFile, argv, SearchDirs, true);
+  if (llvm::cl::checkConfigFileSearchResult(SRes, ConfigFile, SearchDirs, true,
+ProgName))
+return 1;
+
+  // If config file is not specified explicitly, try to determine configuration
+  // implicitly. First try to deduce configuration from executable name. For
+  // instance, a file 'armv7l-clang' applies config file 'armv7l.cfg'. Second,
+  // try to find file 'clang.cfg'.
+  if (SRes == llvm::cl::SearchResult::NotSpecified) {
+if (findConfigFileFromProgramName(ConfigFile, ProgName))
+  SRes = llvm::cl::SearchResult::Successful;
+  }
+
+  if (SRes == llvm::cl::SearchResult::Successful) {
+unsigned NumOpts;
+llvm::cl::readConfigFile(ConfigFile, Saver, argv, NumOpts);
+  }
+
   // Parse response files using the GNU syntax, unless we're in CL mode. There
   // are two ways to put clang in CL compatibility mode: argv[0] is either
   // clang-cl or cl, or --driver-mode=cl is on the command line. The normal
@@ -446,6 +500,8 @@
   ProcessWarningOptions(Diags, *DiagOpts, /*ReportDiags=*/false);
 
   Driver TheDriver(Path, llvm::sys::getDefaultTargetTriple(), Diags);
+  if (!ConfigFile.empty())
+TheDriver.setConfigFile(ConfigFile.str());
   SetInstallDir(argv, TheDriver, CanonicalPrefixes);
 
   insertTargetAndModeArgs(TargetAndMode.first, TargetAndMode.second, argv,
Index: test/Driver/lit.local.cfg
===
--- test/Driver/lit.local.cfg
+++ test/Driver/lit.local.cfg
@@ -4,6 +4,8 @@
 config.substitutions.insert(0,
 ('%clang_cc1',
  """*** Do not use 'clang -cc1' in Driver tests. ***""") )
+config.substitutions.append( ('%bindir',
+  os.path.dirname(config.clang)) )
 
 # Remove harmful environmental variables for clang Driver tests.
 # Some might be useful for other tests so they are only removed here.
Index: test/Driver/config-file2.c
===
--- /dev/null
+++ test/Driver/config-file2.c
@@ -0,0 +1,29 @@
+// REQUIRES: shell
+
+//--- Invocation qqq-clang tries to find config file qqq.cfg
+//
+// RUN: mkdir -p %T/testbin
+// RUN: [ ! -s %T/testbin/qqq-clang ] || rm %T/testbin/qqq-clang
+// RUN: ln -s %clang %T/testbin/qqq-clang
+// RUN: echo 

[PATCH] D24933: Enable configuration files in clang

[Stephen Hines via cfe-commits]

srhines added a comment.

Thanks to Hans for notifying me about this. Cross-compilation flags are a big 
challenge in Android, but I have similar concerns about looking for magic 
files/dirs. Richard's suggestion about improving @file is really interesting, 
and would satisfy a large part of our needs in Android. Can you comment on 
whether you think that would work for your cases as well?




Comment at: docs/UsersManual.rst:655-656
+
+Command line option `--config` can be used to specify configuration file in
+a clang invocation. For instance:
+

rsmith wrote:
> Rather than inventing a new mechanism, could we extend our existing `@file` 
> facility to support comments and nested inclusion of further `@file`s?
This is a very interesting idea, and seems like something that would be very 
interesting for large systems that do a lot of cross-compilation. For instance, 
in Android, I would love to have a set of generic flags to add to all of our 
cross-compiles, and then a set of per-ABI flags for each of our major targets. 
If I could nest @file directives, that would get me most of this with 
minimal/no duplication of flags.



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.

rsmith wrote:
> hans wrote:
> > 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`.
> I'm also not keen on searching `~/.llvm`. Whoever makes the `blah-clang` 
> symlink should get to control what the default configuration for `blah-clang` 
> is, I think. But then this seems to immediately lead to the conclusion that 
> we don't need this implicit-config-file feature at all, since you can replace 
> a `blah-clang` symlink with a shell script containing `exec clang @blah.cfg 
> "@$"` -- and it's better to handle it that way, since you get more control 
> over where the config file lives.
Android is essentially taking the shell script wrapper approach today (although 
we are using python, and we don't do a lot with it just yet).


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Richard Smith via cfe-commits]

rsmith added inline comments.



Comment at: docs/UsersManual.rst:655-656
+
+Command line option `--config` can be used to specify configuration file in
+a clang invocation. For instance:
+

Rather than inventing a new mechanism, could we extend our existing `@file` 
facility to support comments and nested inclusion of further `@file`s?



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.

hans wrote:
> 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`.
I'm also not keen on searching `~/.llvm`. Whoever makes the `blah-clang` 
symlink should get to control what the default configuration for `blah-clang` 
is, I think. But then this seems to immediately lead to the conclusion that we 
don't need this implicit-config-file feature at all, since you can replace a 
`blah-clang` symlink with a shell script containing `exec clang @blah.cfg "@$"` 
-- and it's better to handle it that way, since you get more control over where 
the config file lives.


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Hans Wennborg via cfe-commits]

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

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via cfe-commits]

sepavloff updated this revision to Diff 78167.
sepavloff added a comment.

Limit the patch by named configuration files only

Follow advice of @mgorny and removed all features from the patch except
named config files. These are files specified explicitly by option
`--config` or encoded in executable name, such as `armv7l-clang`. This
kind of configuration files did not cause objections except that "this
functionality is already available with `@file`". In contrast to `@file`
configuration files have advantages:

- they is searched in well-known places rather than in current directory,
- they may contain comments,
- they may include other files by `@file` and these files are resolved relative 
to the including file.
- they may be encoded in executable name; such encoding is already used by 
clang to specify targets.

Also added section about config files to the user manual.


https://reviews.llvm.org/D24933

Files:
  docs/UsersManual.rst
  include/clang/Driver/Driver.h
  lib/Driver/Driver.cpp
  test/Driver/Inputs/config-1.cfg
  test/Driver/Inputs/config-2.cfg
  test/Driver/config-file.c
  tools/driver/driver.cpp

Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -305,6 +305,34 @@
   return 1;
 }
 
+// Directories searched for configuration specified by option '--config'.
+static const char * const SearchDirs[] = { "~/.llvm" };
+
+/// Deduce configuration name if it is encoded in the executable name.
+///
+/// \param ConfigFile [out] Is assigned configuration file path.
+/// \param ProgramName [in] clang executable path.
+/// \return True if configuration file was found.
+///
+/// If clang executable is named e.g. 'armv7l-clang' the function tries to
+/// find config file 'armv7l.cfg'. If it is found, its path is put into
+/// ConfigFile and the function returns true.
+///
+static bool findConfigFileFromProgramName(
+llvm::SmallVectorImpl , StringRef ProgramName) {
+  ConfigFile.clear();
+  StringRef PName = llvm::sys::path::stem(ProgramName);
+  size_t Pos = PName.find("-clang");
+  if (Pos != StringRef::npos) {
+ConfigFile.append(PName.begin(), PName.begin() + Pos);
+const StringRef Ext(".cfg");
+ConfigFile.append(Ext.begin(), Ext.end());
+StringRef CName(ConfigFile.begin(), ConfigFile.size());
+return llvm::cl::searchForFile(ConfigFile, SearchDirs, ProgramName, CName);
+  }
+  return false;
+}
+
 int main(int argc_, const char **argv_) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv_[0]);
   llvm::PrettyStackTraceProgram X(argc_, argv_);
@@ -330,6 +358,31 @@
   llvm::BumpPtrAllocator A;
   llvm::StringSaver Saver(A);
 
+  // Try reading options from configuration file.
+  llvm::SmallString<128> ConfigFile;
+  llvm::cl::SearchResult SRes;
+
+  // First try config file specified in command line. It has higher priority
+  // than any other way to specify configuration.
+  SRes = llvm::cl::findConfigFileFromArgs(ConfigFile, argv, SearchDirs, true);
+  if (llvm::cl::checkConfigFileSearchResult(SRes, ConfigFile, SearchDirs,
+ProgName))
+return 1;
+
+  // If config file is not specified explicitly, try to determine configuration
+  // implicitly. First try to deduce configuration from executable name. For
+  // instance, a file 'armv7l-clang' applies config file 'armv7l.cfg'. Second,
+  // try to find file 'clang.cfg'.
+  if (SRes == llvm::cl::SearchResult::NotSpecified) {
+if (findConfigFileFromProgramName(ConfigFile, ProgName))
+  SRes = llvm::cl::SearchResult::Successful;
+  }
+
+  if (SRes == llvm::cl::SearchResult::Successful) {
+unsigned NumOpts;
+llvm::cl::readConfigFile(ConfigFile, Saver, argv, NumOpts);
+  }
+
   // Parse response files using the GNU syntax, unless we're in CL mode. There
   // are two ways to put clang in CL compatibility mode: argv[0] is either
   // clang-cl or cl, or --driver-mode=cl is on the command line. The normal
@@ -446,6 +499,8 @@
   ProcessWarningOptions(Diags, *DiagOpts, /*ReportDiags=*/false);
 
   Driver TheDriver(Path, llvm::sys::getDefaultTargetTriple(), Diags);
+  if (!ConfigFile.empty())
+TheDriver.setConfigFile(ConfigFile.str());
   SetInstallDir(argv, TheDriver, CanonicalPrefixes);
 
   insertTargetAndModeArgs(TargetAndMode.first, TargetAndMode.second, argv,
Index: test/Driver/config-file.c
===
--- /dev/null
+++ test/Driver/config-file.c
@@ -0,0 +1,22 @@
+// RUN: %clang --config %S/Inputs/config-1.cfg -c %s -### 2>&1 | FileCheck %s -check-prefix CHECK-HHH
+// CHECK-HHH: Target: x86_64-apple-darwin
+// CHECK-HHH: Configuration file: {{.*}}/Inputs/config-1.cfg
+// CHECK-HHH: -Werror
+// CHECK-HHH: -std=c99
+
+// RUN: not %clang --config %S/Inputs/inexistent.cfg -c %s -### 2>&1 | FileCheck %s -check-prefix CHECK-INEX
+// CHECK-INEX: Configuration file {{.*}}/Inputs/inexistent.cfg' specified by option 

[PATCH] D24933: Enable configuration files in clang

[Michał Górny via cfe-commits]

mgorny added a comment.

@sepavloff, my suggestion would be to rework the patch to the minimal, 
acceptable feature (i.e. executable-relative config), and then possibly send 
more patches for the additional features that require additional discussion. 
This way, we'll at least get something done in a reasonable time ;-). I'm still 
looking forward for the opportunity to let users switch default stdlib/rtlib 
without rebuilding clang.


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via cfe-commits]

sepavloff added a comment.

>> Config file set by environment variable is similar to default config file as 
>> user does not specify anything in command line. But the user must explicitly 
>> specify the file, it is not made automatically.
> 
> Sorry, I don't understand this part.

If a user specifies `clang foo.c`, the compiler driver "magically" gets 
additional options in both cases, whether default config file is used or the 
config is specified by environment variable. So both scenarios are evil to some 
extent. But the latter case is the lesser evil, as the user must previously 
define proper variable. Sorry for unclear wording.

>> Ability to append compiler options via environment variable is important 
>> feature, I think, because:
>> ...
>> 
>> - Build systems provided by software components are not perfect. Some do not 
>> honor CFLAGS, some require setting options inside makefiles etc. Changing 
>> compiler options might be a challenge. Environment variable solves this 
>> problem easily and effectively.
> 
> But now you seem to be serving the goal about letting the user set flags 
> without using the build system, which I thought wasn't a goal anymore?

Strictly speaking, you are right. Setting config by environment variable has 
the same effect as using default config file, with the exception that compiler 
user deliberately sets the variable and can chose what options to set in a 
particular case. By allowing this "lesser evil" we provide the user with a way 
to work around complex cases.


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Hans Wennborg via cfe-commits]

hans added a comment.

In https://reviews.llvm.org/D24933#591943, @sepavloff wrote:

> >> Default config file is searched for only in the directory where clang 
> >> executable resides. It allows SDK suppliers to customize compiler, for 
> >> instance by turning off unneeded warnings, by providing their own 
> >> clang.cfg. On the other hand developers cannot by accident override 
> >> default setting because clang.cfg is placed in system directory.
> > 
> > I thought one of the stated goals for this patch would be to allow users to 
> > set default flags without touching the project's build system.
>
> Yes, that's true. Initially it was supposed that a user can override compiler 
> defaults by putting `clang.cfg` into well-knows places, including `~/llvm`, 
> in particular. Then this idea was rejected as a result of feedback, and 
> default config file now is allowed only in the directory where `clang` 
> executable resides. It still allows to override default settings by SDK 
> suppliers but limit this possibility to powerful users.


I see. The SDK use case seems quite reasonable to me.

>> What about this line?
>> 
>>   static const ArrayRef SearchDirs({ "~/.llvm", "/etc/llvm" });
>> 
>> That sounds like you'll be searching the home directory for a default config 
>> file.
> 
> These are directories where config files specified by option `--config` are 
> searched for. There is a big difference between default config files and 
> those specified explicitly. Default file adds option silently, user does not 
> specify anything in the command line. Due to this point it can represent a 
> source of troubles, such as problems with bug reports. In contrast the named 
> config files are specified explicitly in clang invocation. A user may set the 
> same set of option by listing them in command line. Such config file does not 
> represents any dander, it is only a convenience feature.

Oh, I misunderstood then. Thanks for clarifying.

>> And even if we ignore that, the CLANGCFG environment variable presents the 
>> same problem.
>> 
>> Let's say a developer uses that to point to a config file with their 
>> favourite compiler flags for compiling on their x86_64 machine.
>> 
>> But then they want to build something using the Android NDK, whose Clang 
>> will now pull in that config file and start passing flags which doesn't 
>> apply to the target, e.g. ARM. The user might not even be invoking the NDK 
>> directly; it might be through Android Studio which certainly doesn't expect 
>> other flags to be used than the ones it passes.
> 
> If a user forget to update LIBRARY_PATH or C_INCLUDE_PATH the result would be 
> the same.

I don't like those either :-)

> Config file set by environment variable is similar to default config file as 
> user does not specify anything in command line. But the user must explicitly 
> specify the file, it is not made automatically.

Sorry, I don't understand this part.

> Ability to append compiler options via environment variable is important 
> feature, I think, because:
> 
> - This feature implemented by Intel compiles has proven its convenience.
> - Build systems provided by software components are not perfect. Some do not 
> honor CFLAGS, some require setting options inside makefiles etc. Changing 
> compiler options might be a challenge. Environment variable solves this 
> problem easily and effectively.

But now you seem to be serving the goal about letting the user set flags 
without using the build system, which I thought wasn't a goal anymore?

Again, allowing SDKs to configure the compiler (such as setting the default 
target) without actually changing it in the source seems somewhat reasonable, 
but this other part worries me.


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Hans Wennborg via cfe-commits]

hans added a comment.

In https://reviews.llvm.org/D24933#590493, @sepavloff wrote:

> > For Chromium, our build system provides a specific Clang version close to 
> > ToT, and obviously what flags to use when invoking it. (Developers can of 
> > course override the flags when configuring if they want.) Now maybe a 
> > developer has a ~/clang.cfg because they want certain flags on by default 
> > when they build *other* projects, those flags would now be applied to all 
> > clangs, including the one we provide, and potentially wreak havoc.
>
> Default config file is searched for only in the directory where clang 
> executable resides. It allows SDK suppliers to customize compiler, for 
> instance by turning off unneeded warnings, by providing their own 
> `clang.cfg`. On the other hand developers cannot by accident override default 
> setting because `clang.cfg` is placed in system directory.


I thought one of the stated goals for this patch would be to allow users to set 
default flags without touching the project's build system.

What about this line?

  static const ArrayRef SearchDirs({ "~/.llvm", "/etc/llvm" });

That sounds like you'll be searching the home directory for a default config 
file.

And even if we ignore that, the CLANGCFG environment variable presents the same 
problem.

Let's say a developer uses that to point to a config file with their favourite 
compiler flags for compiling on their x86_64 machine.

But then they want to build something using the Android NDK, whose Clang will 
now pull in that config file and start passing flags which doesn't apply to the 
target, e.g. ARM. The user might not even be invoking the NDK directly; it 
might be through Android Studio which certainly doesn't expect other flags to 
be used than the ones it passes.


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Michał Górny via cfe-commits]

mgorny added a comment.

I don't agree with the argumentation of @hans; however, I have my own concerns. 
I personally dislike the idea of reusing command-line option format for this. 
While I can see this is the simplest solution, it brings at least a few 
problems and questions. I'm afraid that this would result in a poor solution 
that gradually gets worse as people try to workaround its limitations.

Maybe it'd be better to take a step back and attempt to create a regular, 
.ini/.conf-style configuration file. One particular advantage of that is that 
we can precisely decide which options to support and how to support them. I 
think this could be better both for opponents of 'free configuration' (since it 
will be less powerful and more controlled), and for proponents of nice 
configuration files (such as me).

For example, the advantage of this is that `-std=` will no longer magically 
apply to all source types. If we decide to have a default -std= control via 
system configuration, we can add separate options for each language (which 
wouldn't be really useful as command-line options).


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Hans Wennborg via cfe-commits]

hans added a comment.

Apologies for seeming so negative here. I can see that this would possibly be 
beneficial for some users. This would be a powerful feature, which is what 
worries me.

Here's a specific scenario I'm worried about.

For Chromium, our build system provides a specific Clang version close to ToT, 
and obviously what flags to use when invoking it. (Developers can of course 
override the flags when configuring if they want.) Now maybe a developer has a 
~/clang.cfg because they want certain flags on by default when they build 
*other* projects, those flags would now be applied to all clangs, including the 
one we provide, and potentially wreak havoc.

As for compiler bugs getting harder to reproduce, we already get a lot of bug 
reports where only the driver invocation is provided, e.g. "clang -O2 foo.c". 
If the user has a config file that sets -fsome-other-flag, we wouldn't see it. 
Even worse, what if distros start trying to be helpful and provide default 
config files?

Other random thoughts:
Do we want to get the same config file for "clang", "clang++" and "clang-cl"? 
They accept different flags.
If we are going to move forward with this, I think the patch should also 
include an update to docs/UsersManual.rst, explaining exactly how it works 
(after that's been figured out of course).


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via cfe-commits]

sepavloff added a comment.

Summary of proposal and discussion in mail list.

**Problems**

The two main problems this feature addresses are:

1. Ability to modify default compiler settings.

As an example, the warning `-Wundefined-var-template` can be helpful for people 
doing module-enabled builds   (https://llvm.org/bugs/show_bug.cgi?id=24425), 
but makes headache for others, who even come with ideas to turn this warning 
off by default 
(http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20160808/167354.html). 
To cope with this problem clang need a mechanism that effectively turns off a 
warning as if it were disabled in compiler sources.

2. Support of cross builds.

Doing cross builds require to specify lots of options that set target, backend 
options, include and library directories etc. There is interest in making this 
action more convenient.

**Solution**

Configuration file groups related options together and allow to specify them in 
more simple and less error prone way than just listing them somewhere in build 
scripts. Configuration may be thought as a "macro" names option set and is 
expanded when compiler is called. There are two use patterns of configuration 
files:

- Default configuration file that is applied without need to specify anything 
in compiler invocation. It can be read from predefined location or be specified 
by environmental variable.
- Named configuration file that is specified explicitly by a user, either by 
command line option `--config`, or by using special executable file names, such 
as `armv7l-clang`.

Only one config file is used, if it is specified explicitly, default config 
file is not searched. Config file may contain comments, references to other 
config files using syntax `@file`. Default config file is searched in the 
directory where clang executable resides.

**Existing experience**

Configuration file is not a new concept, it has been used in various form in 
many compilers including clang.

1. Intel compiler supports default configuration files 
(https://software.intel.com/en-us/node/522780), the file is searched for in the 
directory where executable resides or can be specified by setting environmental 
variable. The feature is pretty convenient in practice.
2. Clang allows to specify target by using special executable names, like 
`armv7l-clang`. Only target name can be specified by this way, which is not 
sufficient to tune compiler for new target. Obvious question arise - why not 
extend this mechanism for other target specific options too.
3. Clang allows modifying options using environmental variable 
`CCC_OVERRIDE_OPTIONS`. This is undocumented way and is looks more like a hack 
for internal use. Its existence however indicates that such functionality is 
useful.
4. Cross compilation environment ELLCC (http://ellcc.org), based on clang, 
supports advanced config files in YAML format. It has proven to be very handy 
to support a wide variety of targets.
5. GCC has support of spec files 
(https://gcc.gnu.org/onlinedocs/gcc/Spec-Files.html), which can be taken from 
default location or be specified by command line option `--spec`. Spec files 
are more powerful tool than just config files as they not only set options but 
also program driver logic. Using custom spec files allows GCC to be tuned to 
particular target without need to specify lots of options in command line.

**Concerns**

Somewhat random set of concerns extracted from discussions.

1. "what flags to pass to the compiler should be configured in a project's 
build system"

A project may include dozens of components each come with own build systems, 
which are tuned differently. Retargeting such build system can be time 
consuming and error prone. On the other hand, setting compiler and target 
specific options is not a business of a component, ideally it may be unaware of 
using in cross compilation environment. Also, not all component build systems 
are error free.

2. "If a project wants to disable some warnings by default, add it to the 
Makefile".

The problem is that some users do not want modify their build files, probably 
because it is not simple process as it appears to us. They would prefer 
customized compiler in which some options are on/off by default.

3. "The idea of having the compiler search for config files in random places on 
the filesystem seems scary"

Most of elaborated environments allows search for libraries, packages, classes 
etc in some predefined places. Running simple command `gcc file.c` make 
compiler search set of various places, and the set may change depending on 
version and OS variant. Complex problems may require complicated tools for 
solving.

4. "config files can make difficult to reproduce failure conditions"

Actual set of compiler options is provided in error dump files, in output of 
`clang -v` and `clang -###`, these invocations also report used config file.

5. "Right now when you invoke clang without any additional options you know 

[PATCH] D24933: Enable configuration files in clang

[Hans Wennborg via cfe-commits]

hans added a comment.

I didn't follow the original thread too closely as I didn't really see the 
problem this was trying to address, but since I'm now cc'd I'll add my opinion.

I'm not in favour of this functionality. (But I guess I might be missing the 
main use case.)

The way I see it, what flags to pass to the compiler should be configured in a 
project's build system. If a project wants to disable some warnings by default, 
add it to the Makefile (or equivalent). I don't see the point in checking in 
another file next to it.

The idea of having the compiler search for config files in random places on the 
filesystem seems scary to me. If I want to override a project's compile flags, 
I'd like to pass them in when configuring the build. If I don't pass any flags, 
I want Clang to work the same everywhere.

Bundling up compiler flags in a file can be convenient, but we already have 
`@file` for that.

There is already a lot of magic in the driver to figure out how to invoke the 
tools. I fear that this would make things even more complicated.


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Reid Kleckner via cfe-commits]

rnk added a comment.

Added some people who might have opinions on the command line interface. Sorry, 
I don't have a lot of time for this right now.


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Michał Górny via cfe-commits]

mgorny resigned from this revision.
mgorny removed a reviewer: mgorny.
mgorny added a comment.

I have no further comments. However, I'm quite a fresh contributor, so I'm not 
really in a position to accept it.


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via cfe-commits]

sepavloff added inline comments.


> mgorny wrote in driver.cpp:314
> Please document what this function does, exactly. I see you've documented it 
> in call site but a doc here would be helpful as well.

The function is documented now.

> mgorny wrote in driver.cpp:376
> Are you sure about the name? I would rather see `TARGET-clang.cfg` than a 
> name that doesn't explicitly mention that the file is for clang.

You are right. Changed the comment.

https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via cfe-commits]

sepavloff updated this revision to Diff 73482.
sepavloff added a comment.

Updated comments.


https://reviews.llvm.org/D24933

Files:
  include/clang/Driver/Driver.h
  lib/Driver/Driver.cpp
  test/Driver/Inputs/config-1.cfg
  test/Driver/Inputs/config-2.cfg
  test/Driver/config-file.c
  test/Driver/config-file2.c
  tools/driver/driver.cpp

Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -305,6 +305,36 @@
   return 1;
 }
 
+// Directories searched for configuration specified by option '--config'.
+static const ArrayRef SearchDirs({ "~/.llvm", "/etc/llvm" });
+
+// Directories searched for default configuration.
+static const ArrayRef DefSearchDirs;
+
+/// \brief Tries to find config file based on executable name.
+///
+/// If clang executable has name like foo-clang, the function tries to find file
+/// foo.cfg. The search is made by same rules as for default config file.
+///
+/// \param ConfigName  [out] File name, if the search is successful.
+/// \param ProgramName [in]  Clang executable name.
+///
+static llvm::cl::SearchResult findConfigFileFromProgramName(
+llvm::SmallVectorImpl , StringRef ProgramName) {
+  ConfigName.clear();
+  StringRef PName = llvm::sys::path::stem(ProgramName);
+  size_t Pos = PName.find("-clang");
+  if (Pos != StringRef::npos) {
+ConfigName.append(PName.begin(), PName.begin() + Pos);
+const StringRef Ext(".cfg");
+ConfigName.append(Ext.begin(), Ext.end());
+std::string CName(ConfigName.begin(), ConfigName.size());
+return llvm::cl::findDefaultCfgFile(ConfigName, DefSearchDirs, ProgramName,
+CName);
+  }
+  return llvm::cl::SearchResult::NotSpecified;
+}
+
 int main(int argc_, const char **argv_) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv_[0]);
   llvm::PrettyStackTraceProgram X(argc_, argv_);
@@ -330,6 +360,47 @@
   llvm::BumpPtrAllocator A;
   llvm::StringSaver Saver(A);
 
+  // Try reading options from configuration file.
+  llvm::SmallString<128> ConfigFile;
+
+  // First try config file specified in command line. It has higher priority
+  // than any other way to specify configuration.
+  auto SRes = llvm::cl::findConfigFileFromArgs(ConfigFile, argv, SearchDirs);
+  if (llvm::cl::checkConfigFileSearchResult(SRes, ConfigFile, SearchDirs,
+ProgName))
+return 1;
+
+  // Environment variable has the next priority. It also specifies config file
+  // explicitly.
+  if (SRes == llvm::cl::SearchResult::NotSpecified) {
+SRes = llvm::cl::findConfigFileFromEnv(ConfigFile, "CLANGCFG");
+if (llvm::cl::checkConfigFileSearchResult(SRes, ConfigFile, SearchDirs,
+  ProgName))
+  return 1;
+  }
+
+  // If config file is not specified explicitly, try determine configuration
+  // implicitly. First try deduce configuration from executable. For instance,
+  // file 'foo-clang' applies config file 'foo.cfg'.
+  if (SRes == llvm::cl::SearchResult::NotSpecified) {
+SRes = findConfigFileFromProgramName(ConfigFile, ProgName);
+if (llvm::cl::checkConfigFileSearchResult(SRes, ConfigFile, DefSearchDirs,
+  ProgName))
+  return 1;
+  }
+
+  // Finally try to find file 'clang.cfg'.
+  if (SRes == llvm::cl::SearchResult::NotSpecified) {
+SRes = llvm::cl::findDefaultCfgFile(ConfigFile, DefSearchDirs, ProgName,
+"clang.cfg");
+if (llvm::cl::checkConfigFileSearchResult(SRes, ConfigFile, DefSearchDirs,
+  ProgName))
+  return 1;
+  }
+
+  if (SRes == llvm::cl::SearchResult::Successful)
+llvm::cl::readConfigFile(ConfigFile, Saver, argv);
+
   // Parse response files using the GNU syntax, unless we're in CL mode. There
   // are two ways to put clang in CL compatibility mode: argv[0] is either
   // clang-cl or cl, or --driver-mode=cl is on the command line. The normal
@@ -446,6 +517,8 @@
   ProcessWarningOptions(Diags, *DiagOpts, /*ReportDiags=*/false);
 
   Driver TheDriver(Path, llvm::sys::getDefaultTargetTriple(), Diags);
+  if (!ConfigFile.empty())
+TheDriver.setConfigFile(ConfigFile.str());
   SetInstallDir(argv, TheDriver, CanonicalPrefixes);
 
   insertTargetAndModeArgs(TargetAndMode.first, TargetAndMode.second, argv,
Index: test/Driver/config-file2.c
===
--- /dev/null
+++ test/Driver/config-file2.c
@@ -0,0 +1,13 @@
+// Due to ln -sf:
+// REQUIRES: shell
+
+// RUN: mkdir -p %t.cfgtest
+// RUN: PWD=`pwd`
+// RUN: cd %t.cfgtest
+// RUN: ln -sf %clang test123-clang
+// RUN: echo "-ferror-limit=666" > test123.cfg
+// RUN: cd $PWD
+// RUN: %t.cfgtest/test123-clang -v -c %s -o - 2>&1 | FileCheck %s
+
+// CHECK: Configuration file:{{.*}}test123.cfg
+// CHECK: -ferror-limit{{.*}}666
Index: 

[PATCH] D24933: Enable configuration files in clang

[Michał Górny via cfe-commits]

mgorny added inline comments.


> driver.cpp:314
> +
> +static llvm::cl::SearchResult findConfigFileFromProgramName(
> +llvm::SmallVectorImpl , StringRef ProgramName) {

Please document what this function does, exactly. I see you've documented it in 
call site but a doc here would be helpful as well.

> driver.cpp:376
> +  // implicitly. First try deduce configuration from executable. For 
> instance,
> +  // file 'armv7l-clang' applies config file 'armv7l.cfg'.
> +  if (SRes == llvm::cl::SearchResult::NotSpecified) {

Are you sure about the name? I would rather see `TARGET-clang.cfg` than a name 
that doesn't explicitly mention that the file is for clang.

https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via cfe-commits]

sepavloff added inline comments.


> mgorny wrote in driver.cpp:334
> 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 /.

It make sense, as it would establish uniform paths. However CommandLine is 
positioned as general purpose library, which can be used by any tool, not only 
llvm ones, so forcing particular paths seems to be too rigid. Besides, 
discussion in mail list demonstrated very diverse opinions, so I would prefer 
flexible solution.

Notion about configurable paths is definitely worth implementing.

> mgorny wrote in driver.cpp:336
> 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.

Updated, thank you.

https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via cfe-commits]

sepavloff updated this revision to Diff 73245.
sepavloff added a comment.

Updated path

The patch is aligned with corresponding changes in llvm repository 
(https://reviews.llvm.org/D24926).
Also support of configuration selection for executables line foo-clang is
added.


https://reviews.llvm.org/D24933

Files:
  include/clang/Driver/Driver.h
  lib/Driver/Driver.cpp
  test/Driver/Inputs/config-1.cfg
  test/Driver/Inputs/config-2.cfg
  test/Driver/config-file.c
  test/Driver/config-file2.c
  tools/driver/driver.cpp

Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -305,6 +305,28 @@
   return 1;
 }
 
+// Directories searched for configuration specified by option '--config'.
+static const ArrayRef SearchDirs({ "~/.llvm", "/etc/llvm" });
+
+// Directories searched for default configuration.
+static const ArrayRef DefSearchDirs;
+
+static llvm::cl::SearchResult findConfigFileFromProgramName(
+llvm::SmallVectorImpl , StringRef ProgramName) {
+  ConfigName.clear();
+  StringRef PName = llvm::sys::path::stem(ProgramName);
+  size_t Pos = PName.find("-clang");
+  if (Pos != StringRef::npos) {
+ConfigName.append(PName.begin(), PName.begin() + Pos);
+const StringRef Ext(".cfg");
+ConfigName.append(Ext.begin(), Ext.end());
+std::string CName(ConfigName.begin(), ConfigName.size());
+return llvm::cl::findDefaultCfgFile(ConfigName, DefSearchDirs, ProgramName,
+CName);
+  }
+  return llvm::cl::SearchResult::NotSpecified;
+}
+
 int main(int argc_, const char **argv_) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv_[0]);
   llvm::PrettyStackTraceProgram X(argc_, argv_);
@@ -330,6 +352,47 @@
   llvm::BumpPtrAllocator A;
   llvm::StringSaver Saver(A);
 
+  // Try reading options from configuration file.
+  llvm::SmallString<128> ConfigFile;
+
+  // First try config file specified in command line. It has higher priority
+  // than any other way to specify configuration.
+  auto SRes = llvm::cl::findConfigFileFromArgs(ConfigFile, argv, SearchDirs);
+  if (llvm::cl::checkConfigFileSearchResult(SRes, ConfigFile, SearchDirs,
+ProgName))
+return 1;
+
+  // Environment variable has the next priority. It also specifies config file
+  // explicitly.
+  if (SRes == llvm::cl::SearchResult::NotSpecified) {
+SRes = llvm::cl::findConfigFileFromEnv(ConfigFile, "CLANGCFG");
+if (llvm::cl::checkConfigFileSearchResult(SRes, ConfigFile, SearchDirs,
+  ProgName))
+  return 1;
+  }
+
+  // If config file is not specified explicitly, try determine configuration
+  // implicitly. First try deduce configuration from executable. For instance,
+  // file 'armv7l-clang' applies config file 'armv7l.cfg'.
+  if (SRes == llvm::cl::SearchResult::NotSpecified) {
+SRes = findConfigFileFromProgramName(ConfigFile, ProgName);
+if (llvm::cl::checkConfigFileSearchResult(SRes, ConfigFile, DefSearchDirs,
+  ProgName))
+  return 1;
+  }
+
+  // Finally try to find file 'clang.cfg'.
+  if (SRes == llvm::cl::SearchResult::NotSpecified) {
+SRes = llvm::cl::findDefaultCfgFile(ConfigFile, DefSearchDirs, ProgName,
+"clang.cfg");
+if (llvm::cl::checkConfigFileSearchResult(SRes, ConfigFile, DefSearchDirs,
+  ProgName))
+  return 1;
+  }
+
+  if (SRes == llvm::cl::SearchResult::Successful)
+llvm::cl::readConfigFile(ConfigFile, Saver, argv);
+
   // Parse response files using the GNU syntax, unless we're in CL mode. There
   // are two ways to put clang in CL compatibility mode: argv[0] is either
   // clang-cl or cl, or --driver-mode=cl is on the command line. The normal
@@ -446,6 +509,8 @@
   ProcessWarningOptions(Diags, *DiagOpts, /*ReportDiags=*/false);
 
   Driver TheDriver(Path, llvm::sys::getDefaultTargetTriple(), Diags);
+  if (!ConfigFile.empty())
+TheDriver.setConfigFile(ConfigFile.str());
   SetInstallDir(argv, TheDriver, CanonicalPrefixes);
 
   insertTargetAndModeArgs(TargetAndMode.first, TargetAndMode.second, argv,
Index: test/Driver/config-file2.c
===
--- /dev/null
+++ test/Driver/config-file2.c
@@ -0,0 +1,13 @@
+// Due to ln -sf:
+// REQUIRES: shell
+
+// RUN: mkdir -p %t.cfgtest
+// RUN: PWD=`pwd`
+// RUN: cd %t.cfgtest
+// RUN: ln -sf %clang test123-clang
+// RUN: echo "-ferror-limit=666" > test123.cfg
+// RUN: cd $PWD
+// RUN: %t.cfgtest/test123-clang -v -c %s -o - 2>&1 | FileCheck %s
+
+// CHECK: Configuration file:{{.*}}test123.cfg
+// CHECK: -ferror-limit{{.*}}666
Index: test/Driver/config-file.c
===
--- /dev/null
+++ test/Driver/config-file.c
@@ -0,0 +1,30 @@
+// RUN: %clang --config 

[PATCH] D24933: Enable configuration files in clang

[Michał Górny via cfe-commits]

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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24933: Enable configuration files in clang

[Daniel Dunbar via cfe-commits]

ddunbar added a comment.

I am too out of the loop on Clang development to be able to comment on the 
specific direction, but I will just say that I am highly in favor of adding new 
features in this direction. Thank you!


https://reviews.llvm.org/D24933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via cfe-commits]

sepavloff created this revision.
sepavloff added reviewers: rnk, ddunbar.
sepavloff added a subscriber: cfe-commits.

The implementation follows the discussion in mail list
(http://lists.llvm.org/pipermail/cfe-dev/2016-September/050776.html).
Main part of the config file support is implemented in llvm source tree,
in CommandLine library, see D24926 and D24917.

https://reviews.llvm.org/D24933

Files:
  include/clang/Driver/Driver.h
  lib/Driver/Driver.cpp
  test/Driver/Inputs/config-1.cfg
  test/Driver/Inputs/config-2.cfg
  test/Driver/config-file.c
  tools/driver/driver.cpp

Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -330,6 +330,14 @@
   llvm::BumpPtrAllocator A;
   llvm::StringSaver Saver(A);
 
+  // Try reading options from configuration file.
+  static const char * const SearchDirs[] = { "~/.llvm", "/etc/llvm" };
+  llvm::SmallString<128> ConfigFile;
+  auto SRes = llvm::cl::findConfigFile(ConfigFile, argv, SearchDirs, "clang");
+  llvm::cl::reportConfigFileSearchError(SRes, ConfigFile, SearchDirs, ProgName);
+  if (SRes == llvm::cl::CfgFileSearch::Successful)
+llvm::cl::readConfigFile(ConfigFile, Saver, argv);
+
   // Parse response files using the GNU syntax, unless we're in CL mode. There
   // are two ways to put clang in CL compatibility mode: argv[0] is either
   // clang-cl or cl, or --driver-mode=cl is on the command line. The normal
@@ -446,6 +454,8 @@
   ProcessWarningOptions(Diags, *DiagOpts, /*ReportDiags=*/false);
 
   Driver TheDriver(Path, llvm::sys::getDefaultTargetTriple(), Diags);
+  if (!ConfigFile.empty())
+TheDriver.setConfigFile(ConfigFile.str());
   SetInstallDir(argv, TheDriver, CanonicalPrefixes);
 
   insertTargetAndModeArgs(TargetAndMode.first, TargetAndMode.second, argv,
Index: test/Driver/config-file.c
===
--- /dev/null
+++ test/Driver/config-file.c
@@ -0,0 +1,30 @@
+// RUN: %clang --config %S/Inputs/config-1.cfg -c %s -### 2>&1 | FileCheck %s -check-prefix CHECK-HHH
+// CHECK-HHH: Target: x86_64-apple-darwin
+// CHECK-HHH: Configuration file: {{.*}}/Inputs/config-1.cfg
+// CHECK-HHH: -Werror
+// CHECK-HHH: -std=c99
+
+// RUN: not %clang --config %S/Inputs/inexistent.cfg -c %s -### 2>&1 | FileCheck %s -check-prefix CHECK-INEX
+// CHECK-INEX: Configuration file {{.*}}/Inputs/inexistent.cfg' specified by option '--config' cannot be found
+
+// 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:
+// CHECK-INEX-NOSEARCH: ~/.llvm
+// CHECK-INEX-NOSEARCH: /etc/llvm
+
+// RUN: %clang --config %S/Inputs/config-2.cfg -### 2>&1 | FileCheck %s -check-prefix CHECK-HHH-NOFILE
+// CHECK-HHH-NOFILE: Target: x86_64-unknown-linux
+// CHECK-HHH-NOFILE: Configuration file: {{.*}}/Inputs/config-2.cfg
+
+// RUN: %clang --config %S/Inputs/config-1.cfg -c %s -v 2>&1 | FileCheck %s -check-prefix CHECK-V
+// CHECK-V: Target: x86_64-apple-darwin
+// CHECK-V: Configuration file: {{.*}}/Inputs/config-1.cfg
+// CHECK-V: -triple{{.*}}x86_64-apple-
+
+// RUN: env CLANGCFG=%S/Inputs/config-1.cfg %clang -c %s -### 2>&1 | FileCheck %s -check-prefix CHECK-ENV
+// CHECK-ENV: Target: x86_64-apple-darwin
+// CHECK-ENV: Configuration file: {{.*}}/Inputs/config-1.cfg
+// CHECK-ENV: -Werror
+
+// RUN: env CLANGCFG=inexistent.cfg not %clang -c %s -### 2>&1 | FileCheck %s -check-prefix CHECK-ENV-INEX
+// CHECK-ENV-INEX: Configuration file 'inexistent.cfg' specified by environment variable cannot be found
Index: test/Driver/Inputs/config-2.cfg
===
--- /dev/null
+++ test/Driver/Inputs/config-2.cfg
@@ -0,0 +1,2 @@
+# target
+-target x86_64-unknown-linux-gnu
Index: test/Driver/Inputs/config-1.cfg
===
--- /dev/null
+++ test/Driver/Inputs/config-1.cfg
@@ -0,0 +1,7 @@
+
+
+# Empty lines and line started with # are ignored
+-Werror -std=\
+c99
+# Target
+-target x86_64-apple-darwin
\ No newline at end of file
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -895,6 +895,10 @@
 
   // Print out the install directory.
   OS << "InstalledDir: " << InstalledDir << '\n';
+
+  // If configuration file was used, print its path.
+  if (!ConfigFile.empty())
+OS << "Configuration file: " << ConfigFile << '\n';
 }
 
 /// PrintDiagnosticCategories - Implement the --print-diagnostic-categories
Index: include/clang/Driver/Driver.h
===
--- include/clang/Driver/Driver.h
+++ include/clang/Driver/Driver.h
@@ -187,6 +187,9 @@
   /// Name to use when invoking gcc/g++.