[PATCH] D134018: [clang] [Driver] Add an option to disable default config filenames

[Michał Górny via Phabricator via cfe-commits]

mgorny added inline comments.


================
Comment at: clang/lib/Driver/Driver.cpp:1027
 
-  // If config file is not specified explicitly, try to deduce configuration
-  // from executable name. For instance, an executable 'armv7l-clang' will
-  // search for config file 'armv7l-clang.cfg'.
-  if (CfgFileName.empty() && !ClangNameParts.TargetPrefix.empty())
-    CfgFileName = ClangNameParts.TargetPrefix + '-' + ClangNameParts.ModeSuffix;
+  if (!CLOptions || !CLOptions->hasArg(options::OPT_no_default_config)) {
+    // If config file is not specified explicitly, try to deduce configuration
----------------
MaskRay wrote:
> mgorny wrote:
> > MaskRay wrote:
> > > `&&` ?
> > `&&` would mean dereferencing null pointer ;-).
> oh, i misread. sorry. the two `if` can be merged
I've deliberately left it like that to account for possibly more rules being added in the future (e.g. explicit `-target`, given the discourse discussion).

I've changed `!x || !y` to `!(x && y)`, perhaps that will be less confusing.


================
Comment at: clang/test/Driver/config-file3.c:49
+// NO-DEFAULT-CONFIG-NOT: Configuration file:
+//
 //--- Invocation qqq-clang-g++ tries to find config file qqq.cfg if qqq-clang-g++.cfg is not found.
----------------
MaskRay wrote:
> mgorny wrote:
> > MaskRay wrote:
> > > The convention doesn't use otherwise empty `// `. The file does not obey the convention but your new lines can drop the empty `//`
> > Do you mean the extra space or the entire empty line?
> I mean `^// *$`, which typically makes the test harder to browse.
> 
> (Think of `{` `}` in vim. If every line has `//`, `{` `}` isn't effective at all.)
Oh, so you mean to have empty lines between tests to make it easier to navigate between tests? Please let me know if I got this right.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134018/new/

https://reviews.llvm.org/D134018