[clang-tidy] adding "--config-file=<file-path>" to specify custom config file.

[Hiralo]

Summary

commit 653cb7912bbe4daabc8d6dd6dca71b2cf9a10365 (HEAD -> master)
Author: Hiral Oza <[e-mail]>
Date: Fri Oct 30 05:07:19 2020 -0400

clang-tidy: adding "--config-file=<file-path>" to specify custom config file.

Let clang-tidy to read config from specified file.

This option internally works exactly the same way as --config option
after reading specified config file.

This option works as helper for --config in following few cases:
 - when there is shell limitation to read file content to the command line,
 - when there is limitation to command line length,
 - setting right build dependencies, and
 - others

--config-file and --config options are made mutually exclusive.

Example usage:
 $ clang-tidy --config-file=/some/path/myTidyConfigFile --dump-config --
 ...this will read config from /some/path/myTidyConfigFile and
    avoid searching '.clang-tidy' in parent-dirs.

May speed-up tidy runtime since now it will just look-up <file-path>
instead of searching ".clang-tidy" in parent-dirs.

Reviewed By: DmitryPolukhin, njames93

Differential Revision: https://reviews.llvm.org/D89936

[DmitryPolukhin]

I'm not sure that we need additional option to read configuration from file but, if we do need, I think this diff needs some improvements + test for new option.

clang-tools-extra/clang-tidy/ClangTidyOptions.cpp

324 ↗

(On Diff #299882)

Is it actually required to check absolute path, link it or not, etc.? Why not just try reading file with provided filename and report error if it fails?

408 ↗

(On Diff #299882)

It seems like some debug prints.

clang-tools-extra/clang-tidy/ClangTidyOptions.h

68 ↗

(On Diff #299882)

I'm not sure that we need it here. I would reuse code path for --config options as much as possible and implement new option as a simple wrapper that reads content of the file and interpret it as --config option. Moreover I think it should not be possible to specify both options in command line.

233 ↗

(On Diff #299882)

It looks like the second argument was added only for overload resolution. But I think it is better to rename this function. After all it is not "try" anymore because it reports fatal error in case of errors.

[DmitryPolukhin]

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

76

I would call it something like --config-file

[Hiralo]

Thanks for your kind review.
Working on it...

[Hiralo]

clang-tools-extra/clang-tidy/ClangTidyOptions.h

68 ↗

(On Diff #299882)

it should not be possible to specify both options in command line.

Made local changes with...

$ ./bin/clang-tidy -config="{Checks: '*'}" --config-file=/some/path/config --
Error: --config-file and --config are mutually exclusive. Specify only one.

$ ./bin/clang-tidy --checks='*' --config-file=/some/path/config --
Error: --config-file and --checks are mutually exclusive. Specify only one.

233 ↗

(On Diff #299882)

Renamed function to 'readConfigFile'.

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

76

renamed

[Hiralo]

clang-tools-extra/clang-tidy/ClangTidyOptions.cpp

324 ↗

(On Diff #299882)

Tried it and seems we don't need this changes since down the line below code emits required error message...

if (std::error_code EC = Text.getError()) {
std::string Msg;
llvm::raw_string_ostream ErrStream(Msg);
ErrStream << " Can't read <" << Path << ">: " << EC.message() << "\n";
llvm::report_fatal_error(ErrStream.str());
}

[Hiralo]

Incorporated review comments and updated patch.
(a) renamed var 'ClangTidyConfig' with 'ConfigFile'
(b) renamed function 'tryReadConfigFile(AbsoluteFilePath, true);' to 'readConfigFile(ConfigPath)'
(c) renamed command-line option to '--config-file'
(d) made --config-file and --checks mutually exclusive
(e) make --config-file and --config mutually exclusive
(f) removed extra checks -- making absolute-path, isFile and isLink checks

[Hiralo]

In D89936#2353048, @DmitryPolukhin wrote:

...I think this diff needs some improvements

Thanks @DmitryPolukhin for your valuable comments/feedback.
Please review updated patch.

+ test for new option.

I am planning to add following test-case...
<diff>
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/read_file_config.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/read_file_config.cpp
.......
RUN: clang-tidy -checks="-*,clang-analyzer-*" %T/read-file-config/test.cpp | grep "warning: .*\[clang-analyzer-deadcode.DeadStores\]$"
+ RUN: clang-tidy -config-file=%T/read-file-config/.clang-tidy %T/read-file-config/test.cpp | grep "warning: .*\[clang-analyzer-deadcode.DeadStores\]$"
</diff>

[DmitryPolukhin]

@Hiralo, it looks like you uploaded wrong diff because your previous revision is shown as "base" version. Base revision should be clang-tidy sources without your changes.

Please add Lit test for the new option, see https://llvm.org/docs/TestingGuide.html for more details and existing clang-tidy tests.

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

314–315

I think you can make this option much simpler if you just read file content and use it or Config here. No changes in clang-tools-extra/clang-tidy/ClangTidyOptions.h/.cpp will be required.

[Hiralo]

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

314–315

Sorry, didn't get. Can you please elaborate/pseudocode?

[Hiralo]

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

314–315

I think you can make this option much simpler if you just read file content and use it or Config here. No changes in clang-tools-extra/clang-tidy/ClangTidyOptions.h/.cpp will be required.

Are you suggesting to...
(a) as per this patch have option '--config-file' (ClangTidyMain.cpp changes)
(b) read this file in ClangTidyMain.cpp itself and
(c) pass YAML o/p to code-flow-of-config option (again change only in ClangTidyMain.cpp).

if so...

Shouldn't at call to tryReadConfigFile() kind of call in ClangTidyMain.cpp?

Can you please point me to LLVM/Clang File IO calls/examples?

[Hiralo]

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

314–315

From code point of view... within ClangTidyMain.cpp itself... code-flow may look...

llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Text = llvm::MemoryBuffer::getFile(ConfigFile.c_str());
if (std::error_code EC = Text.getError()) {
  llvm::errs() << "Can't read " << ConfigFile << ": " << EC.message() << "\n";
}

and then

Configassign(ConfigFile);
from then onwards it will be considered as Config values

Let me try implementing and test...

[Hiralo]

With updated patch now changes are only in ClangTidyMain.cpp.
Thanks to @DmitryPolukhin for valuable suggestion :)

Added '--config-file' option to specify custom config file.
ClangTidyMain.cpp reads ConfigFile into string and then assigned read data to 'Config' i.e. makes like '--config' code flow internally.

Ref comments https://reviews.llvm.org/D89936#inline-839224

Thank you.
-Hiral

[DmitryPolukhin]

Yes, it is what I meant as a simpler solution. But please add Lit test even for such trivial option.

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

336

I suggest creating new local variable with text of the config from Config or ConfigFile file content i.e. avoid modifying Config itself.

[Hiralo]

Added Lit test for --config-file cmdline option.

[Hiralo]

Latest patch with lit test.

[Hiralo]

In D89936#2356583, @DmitryPolukhin wrote:

Yes, it is what I meant as a simpler solution. But please add Lit test even for such trivial option.

Please review latest patch and Lit test-case.

[DmitryPolukhin]

clang-tools-extra/test/clang-tidy/infrastructure/config-file.cpp

3

I think it is better to use some file in %S/Inputs like in https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp

[njames93]

If you plan on contributing quite a lot then it would be wise to upload your patches with arcanist - https://llvm.org/docs/Phabricator.html. It will help prevent issues with diffs being relative to previous revisions.
Personally I just create a branch from master for a feature. When I create the patch or update, I just need to use arc diff master and it will handle everything for me.
The other advantage of using arc for patches is the pre-merge bot is then able to build and check your patch to make sure all tests pass.

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

292

Given you've removed the references to ConfigFile in ClangTidyOptions.h this should surely result in a compiler error, same goes below.

[Hiralo]

Latest patch with Lit test-case.

[Hiralo]

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

292

Thanks for heads-up and references.

clang-tools-extra/test/clang-tidy/infrastructure/config-file.cpp

3

sure... in latest update added clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file

[Hiralo]

In D89936#2357088, @njames93 wrote:

If you plan on contributing quite a lot then it would be wise to upload your patches with arcanist - https://llvm.org/docs/Phabricator.html. It will help prevent issues with diffs being relative to previous revisions.
Personally I just create a branch from master for a feature. When I create the patch or update, I just need to use arc diff master and it will handle everything for me.
The other advantage of using arc for patches is the pre-merge bot is then able to build and check your patch to make sure all tests pass.

Sure thanks for info.

will there be any update in this review after build error or successful merge ?

[DmitryPolukhin]

I think this diff looks very close to what we need. I hope it will be the last iteration.

Please also update title and description with the new name of the option, etc.

clang-tools-extra/test/clang-tidy/infrastructure/config-file.cpp

3

Please use FileCheck as all other tests do instead of grep to make test more portable. Also with FileCheck it should be no need in REQUIRES: shell.

And I think it is better to use -dump-config instead of checking specific check on a source file. This way you will tests only functionality that you implemented, without dependency some check behavior. Also it could be just another tests case in clang-tidy/infrastructure/config-files.cpp.

[Hiralo]

clang-tools-extra/test/clang-tidy/infrastructure/config-file.cpp

3

Please use FileCheck as all other tests do instead of grep to make test more portable. Also with FileCheck it should be no need in REQUIRES: shell.

And I think it is better to use -dump-config instead of checking specific check on a source file. This way you will tests only functionality that you implemented, without dependency some check behavior. Also it could be just another tests case in clang-tidy/infrastructure/config-files.cpp.

Sure...

Tried following...

// RUN: clang-tidy -config-file=%S/Inputs/config-file/config-file -dump-config -- | FileCheck %s -check-           prefix=CHECK-BASE
// CHECK-BASE: Checks: {{.*}}hicpp-uppercase-literal-suffix
// RUN: clang-tidy -config-file=%S/Inputs/config-file/config-file %S/config-file.cpp -- | FileCheck %s -check-     prefix=CHECK-CHILD1
// CHECK-CHILD1: Checks: {{.*}}warning: integer literal has suffix 'ull', which is not uppercase [hicpp-uppercase- literal-suffix]  ########### This one fails!

Can you please suggest how to match with following o/p:

path/clang-tools-extra/test/clang-tidy/infrastructure/config-file.cpp:7:21: warning: integer literal has suffix 'ull', which is not uppercase [hicpp-uppercase-literal-suffix]
  unsigned long c = 100ull;
                    ^  ~~~
                       ULL

JFYI: also tried matching just 'ULL'!

[Hiralo]

clang-tools-extra/test/clang-tidy/infrastructure/config-file.cpp

3

Removed below test...
// RUN: clang-tidy -config-file=%S/Inputs/config-file/config-file %S/config-file.cpp -- | FileCheck %s -check- prefix=CHECK-CHILD1

[Hiralo]

Latest patch with Lit test-case.

Updated Title and Summary.

[Hiralo]

In D89936#2358572, @DmitryPolukhin wrote:

I think this diff looks very close to what we need. I hope it will be the last iteration.
Please also update title and description with the new name of the option, etc.

Updated latest patch :)

[DmitryPolukhin]

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

336

It doesn't compile and using local variable instead of changing command line option will make this code easier to read and understand:

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:334:12: error: no matching member function for call to 'assign'
    Config.assign((*Text)->getBuffer());
    ~~~~~~~^~~~~~

Could you please fix build and please upload diff with arc diff as @njames93 suggested so buildbot can test your changes?

[Hiralo]

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

336

I suggest creating new local variable with text of the config from Config or ConfigFile file content i.e. avoid modifying Config itself.

Sorry! probably I missed this comment!

std::string Temp = (*Text)->getBuffer();
Config.assign(Temp); // ???

if so how code will enter into below block... because we want to reuse code-path of 'Config' correct?

if (!Config.empty()) {
if (llvm::ErrorOr<ClangTidyOptions> ParsedConfig = parseConfiguration(Config)) {
...
}

?

[njames93]

It's also be nice if --config-file would also support being passed a directory. If a directory was passed it would append ".clang-tidy" to the path and load that file, WDYT?

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

322–326

I disagree with this check here, Config is not mutually exclusive with Checks, Checks gets applied atop of Config. So the same should happen when using ConfigFile with Checks

336

Could extract the configuration loading into a lambda

auto LoadConfig = [&](StringRef Configuration)
    -> std::unique_ptr<ClangTidyOptionsProvider> {
  llvm::ErrorOr<ClangTidyOptions> ParsedConfig =
      parseConfiguration(Configuration);
  if (ParsedConfig)
    return std::make_unique<ConfigOptionsProvider>(
        GlobalOptions,
        ClangTidyOptions::getDefaults().mergeWith(DefaultOptions, 0),
        *ParsedConfig, OverrideOptions, std::move(FS));
  llvm::errs() << "Error: invalid configuration specified.\n"
               << ParsedConfig.getError().message() << "\n";
  return nullptr;
};

Then further down

if (!ConfigFile.empty()) {
  ...
  llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Text =
        llvm::MemoryBuffer::getFile(ConfigFile.c_str());
    if (std::error_code EC = Text.getError()) {
      llvm::errs() << "Can't read config-file '" << ConfigFile
                   << "': " << EC.message() << "\n";
      return nullptr;
    }

    return LoadConfig((*Text)->getBuffer());
}

if (!Config.empty())
  return LoadConfig(Config);

This has the added bonus of not having to needlessly copy the buffer

[DmitryPolukhin]

In D89936#2361580, @njames93 wrote:

It's also be nice if --config-file would also support being passed a directory. If a directory was passed it would append ".clang-tidy" to the path and load that file, WDYT?

It might be misleading what "InheritParentConfig: true" in the file means in this case. Current implementation that it will use directory structure of the source file and I think it is right approach. I would prefer to keep things simple and straightforward: --config-file should be file and it is simple helper for --config for the cases when you cannot use shell power to read file content to the command line or you exceed limit on command line length.

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

322–326

+1

[DmitryPolukhin]

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

322–326

Clarify: +1 to @njames93 that we don't need this check.

[Hiralo]

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

322–326

I disagree with this check here, Config is not mutually exclusive with Checks, Checks gets applied atop of Config. So the same should happen when using ConfigFile with Checks

ok, will remove these two checks :)

336

It doesn't compile and using local variable instead of changing command line option will make this code easier to read and understand:

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:334:12: error: no matching member function for call to 'assign'
    Config.assign((*Text)->getBuffer());
    ~~~~~~~^~~~~~

Could you please fix build and please upload diff with arc diff as @njames93 suggested so buildbot can test your changes?

Let me try.

[DmitryPolukhin]

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

322–326

I think we need to keep the first check. There is no reason to specify both --config and --config-file or give on of them precedence.

[Hiralo]

clang-tidy: adding "--config-file=<file-path>" to specify custom config file.

Let clang-tidy to read config from specified file.

This option internally works exactly the same way as --config option
after reading specified config file.

This option works as helper for --config in following few cases:

• when there is shell limitation to read file content to the command line,
• when there is limitation to command line length,
• setting right build dependencies, and
• others

--config-file and --config options are made mutually exclusive.

Example usage:
$ clang-tidy --config-file=/some/path/myTidyConfigFile --dump-config --
...this will read config from /some/path/myTidyConfigFile and

avoid searching '.clang-tidy' in parent-dirs.

May speed-up tidy runtime since now it will just look-up <file-path>
instead of searching ".clang-tidy" in parent-dirs.

Reviewed By: DmitryPolukhin, njames93

Differential Revision: https://reviews.llvm.org/D89936

[Hiralo]

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

322–326

I think we need to keep the first check. There is no reason to specify both --config and --config-file or give on of them precedence.

Kept '--config-file' and '--config' mutually exclusive.

336

It doesn't compile and using local variable instead of changing command line option will make this code easier to read and understand:

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:334:12: error: no matching member function for call to 'assign'
    Config.assign((*Text)->getBuffer());
    ~~~~~~~^~~~~~

Could you please fix build and please upload diff with arc diff as @njames93 suggested so buildbot can test your changes?

Let me try.

(Finally could make setup build on top of master and make arc work...)

Please review updated patch. with 'arc diff' :)

336

Could extract the configuration loading into a lambda
This has the added bonus of not having to needlessly copy the buffer

Thanks njames93 for lambda fun :)

[njames93]

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

329–330

nit: Should we be using Config(File)?.getNumOccurances() > 0 here and below
If someone puts --config="" They are specifying a config(file)?, even if its empty.

[Hiralo]

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

329–330

nit: Should we be using Config(File)?.getNumOccurances() > 0 here and below
If someone puts --config="" They are specifying a config(file)?, even if its empty.

Sounds good.

What is expected o/p with -config="" ?

Currently it works as below...
$ ./myinstall/bin/clang-tidy -config="" --dump-config --
config_occurances =1
config_is_empty = yes

Checks:          'clang-diagnostic-*,clang-analyzer-*'

WarningsAsErrors: ''
HeaderFilterRegex: ''
AnalyzeTemporaryDtors: false
FormatStyle:     none
CheckOptions:
  - key:             llvm-else-after-return.WarnOnConditionVariables
...

[Hiralo]

clang-tidy: adding "--config-file=<file-path>" to specify custom config file.

Let clang-tidy to read config from specified file.

This option internally works exactly the same way as --config option
after reading specified config file.

This option works as helper for --config in following few cases:

• when there is shell limitation to read file content to the command line,
• when there is limitation to command line length,
• setting right build dependencies, and
• others

--config-file and --config options are made mutually exclusive.

Example usage:
$ clang-tidy --config-file=/some/path/myTidyConfigFile --dump-config --
...this will read config from /some/path/myTidyConfigFile and

avoid searching '.clang-tidy' in parent-dirs.

May speed-up tidy runtime since now it will just look-up <file-path>
instead of searching ".clang-tidy" in parent-dirs.

Reviewed By: DmitryPolukhin, njames93

Differential Revision: https://reviews.llvm.org/D89936

[Hiralo]

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

329–330

nit: Should we be using Config(File)?.getNumOccurances() > 0 here and below
If someone puts --config="" They are specifying a config(file)?, even if its empty.

Updated patch...

$ ./myinstall/bin/clang-tidy --config="" --config-file="" --dump-config --
Error: --config-file and --config are mutually exclusive. Specify only one.

Thanks,

[Hiralo]

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

329–330

nit: Should we be using Config(File)?.getNumOccurances() > 0 here and below
If someone puts --config="" They are specifying a config(file)?, even if its empty.

Updated patch...

$ ./myinstall/bin/clang-tidy --config="" --config-file="" --dump-config --
Error: --config-file and --config are mutually exclusive. Specify only one.

Thanks,

and

$ ./myinstall/bin/clang-tidy --config-file="" --dump-config --
Error: can't read config-file '': No such file or directory

[DmitryPolukhin]

Looks good to me.

[Hiralo]

In D89936#2364665, @DmitryPolukhin wrote:

Looks good to me.

Thanks @DmitryPolukhin and @njames93 for your help.

This revision is now accepted and ready to land.Fri, Oct 30, 8:57 PM

When this patch will be merged and available in master?

[DmitryPolukhin]

In D89936#2370163, @Hiralo wrote:

When this patch will be merged and available in master?

@Hiralo I can push this changes to master for you. Please let me know if you need it.

[Hiralo]

In D89936#2370712, @DmitryPolukhin wrote:

In D89936#2370163, @Hiralo wrote:

When this patch will be merged and available in master?

@Hiralo I can push this changes to master for you. Please let me know if you need it.

Yes please.

And just for confirmation, this patch will be part of the release 12.0.0 (i.e. Dec release), correct?

[Hiralo]

Hello @@DmitryPolukhin ,

When I submitted latest via 'arc diff' my commit-message was...
Can you please help to have following commit message? Below commit message is more clear and helpful.
Sorry for inconvenience caused!

commit 653cb7912bbe4daabc8d6dd6dca71b2cf9a10365 (HEAD -> master)
Author: Hiral Oza <[e-mail]>
Date: Fri Oct 30 05:07:19 2020 -0400

clang-tidy: adding "--config-file=<file-path>" to specify custom config file.

Let clang-tidy to read config from specified file.

This option internally works exactly the same way as --config option
after reading specified config file.

This option works as helper for --config in following few cases:
 - when there is shell limitation to read file content to the command line,
 - when there is limitation to command line length,
 - setting right build dependencies, and
 - others

--config-file and --config options are made mutually exclusive.

Example usage:
 $ clang-tidy --config-file=/some/path/myTidyConfigFile --dump-config --
 ...this will read config from /some/path/myTidyConfigFile and
    avoid searching '.clang-tidy' in parent-dirs.

May speed-up tidy runtime since now it will just look-up <file-path>
instead of searching ".clang-tidy" in parent-dirs.

Reviewed By: DmitryPolukhin, njames93

Differential Revision: https://reviews.llvm.org/D89936

[DmitryPolukhin]

Hello @@DmitryPolukhin ,

When I submitted latest via 'arc diff' my commit-message was...
Can you please help to have following commit message? Below commit message is more clear and helpful.
Sorry for inconvenience caused!

It is not possible to update commit message afterwords. I received commit message from arc patch your diff (https://llvm.org/docs/Phabricator.html#committing-someone-s-change-from-phabricator). The only thing that I did manually was removing extra spaces and second link to the same codereview. I think arc patch ignores upload messages and uses diff description from Phabricator. Upload message just describes what you have changed in comparison with previous upload so it is expected behaviour.

[Hiralo]

Hello @DmitryPolukhin ,
Sorry I missed to update 'SUMMARY' section of this review earlier! was not aware that that will be considered as commit message.
I have now updated the 'SUMMARY', it is possible to revert this commit and then re-submit it with updated commit message?
I think latest commit message reflects and provides details about usage and helpful to people.
Sorry for inconvenience caused!

[DmitryPolukhin]

Sorry, I don't think it's worth reverting and resubmitting the same changes in such a case. Commit attribution is correct and there is link to this code review.
After all, most clang-tidy users will never read commit messages and will only use --help for discovering this feature.

[Hiralo]

Got it. No worries :)
Thanks @DmitryPolukhin for your help.

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

 set, globs with the '-' prefix remove checks
 with matching names from the set of enabled
 checks. This option's value is appended to the
 value of the 'Checks' option in .clang-tidy
 file, if any.
 )"),
                                    cl::init(""), cl::cat(ClangTidyCategory));
 
 static cl::opt<std::string> WarningsAsErrors("warnings-as-errors", cl::desc(R"(
 Upgrades warnings to errors. Same format as
 '-checks'.
 This option's value is appended to the value of
 the 'WarningsAsErrors' option in .clang-tidy
 file, if any.
 )"),
                                              cl::init(""),
                                              cl::cat(ClangTidyCategory));
-config="{Checks: '*',
             CheckOptions: [{key: x,
                             value: y}]}"
 When the value is empty, clang-tidy will
 attempt to find a file named .clang-tidy for
 each source file in its parent directories.
 )"),
                                    cl::init(""), cl::cat(ClangTidyCategory));
 
+static cl::opt<std::string> ConfigFile("config-file", cl::desc(R"(
+Specify the path of .clang-tidy or custom config file:
+ e.g. --config-file=/some/path/myTidyConfigFile
+This option internally works exactly the same way as
+ --config option after reading specified config file.
+Use either --config-file or --config, not both.
+)"),
+                                       cl::init(""),
+                                       cl::cat(ClangTidyCategory));
+
 static cl::opt<bool> DumpConfig("dump-config", cl::desc(R"(
 Dumps configuration in the YAML format to
 stdout. This option can be used along with a
 file name (and '--' if the file is outside of a
 project with configured compilation database).
 The configuration used for this file will be
 printed.
 Use along with -checks=* to include
static std::unique_ptr<ClangTidyOptionsProvider> createOptionsProvider(
   if (std::error_code Err = parseLineFilter(LineFilter, GlobalOptions)) {
     llvm::errs() << "Invalid LineFilter: " << Err.message() << "\n\nUsage:\n";
     llvm::cl::PrintHelpMessage(/*Hidden=*/false, /*Categorized=*/true);
     return nullptr;
   }
 
   ClangTidyOptions DefaultOptions;
   DefaultOptions.Checks = DefaultChecks;
   DefaultOptions.WarningsAsErrors = "";
   DefaultOptions.HeaderFilterRegex = HeaderFilter;
   DefaultOptions.SystemHeaders = SystemHeaders;
   DefaultOptions.FormatStyle = FormatStyle;
   DefaultOptions.User = llvm::sys::Process::GetEnv("USER");
   // USERNAME is used on Windows.
   if (!DefaultOptions.User)
     DefaultOptions.User = llvm::sys::Process::GetEnv("USERNAME");
 
   ClangTidyOptions OverrideOptions;
   if (Checks.getNumOccurrences() > 0)
     OverrideOptions.Checks = Checks;
   if (WarningsAsErrors.getNumOccurrences() > 0)
     OverrideOptions.WarningsAsErrors = WarningsAsErrors;
   if (HeaderFilter.getNumOccurrences() > 0)
     OverrideOptions.HeaderFilterRegex = HeaderFilter;
   if (SystemHeaders.getNumOccurrences() > 0)
     OverrideOptions.SystemHeaders = SystemHeaders;
   if (FormatStyle.getNumOccurrences() > 0)
     OverrideOptions.FormatStyle = FormatStyle;
   if (UseColor.getNumOccurrences() > 0)
     OverrideOptions.UseColor = UseColor;
 
-  if (!Config.empty()) {
-    if (llvm::ErrorOr<ClangTidyOptions> ParsedConfig =
-            parseConfiguration(Config)) {
+  auto LoadConfig = [&](StringRef Configuration)
+      -> std::unique_ptr<ClangTidyOptionsProvider> {
+    llvm::ErrorOr<ClangTidyOptions> ParsedConfig =
+        parseConfiguration(Configuration);
+    if (ParsedConfig)
       return std::make_unique<ConfigOptionsProvider>(
           GlobalOptions,
           ClangTidyOptions::getDefaults().mergeWith(DefaultOptions, 0),
           *ParsedConfig, OverrideOptions, std::move(FS));
-    } else {
-      llvm::errs() << "Error: invalid configuration specified.\n"
-                   << ParsedConfig.getError().message() << "\n";
-      return nullptr;
+  };
+
+  if (ConfigFile.getNumOccurrences() > 0) {
+    if (Config.getNumOccurrences() > 0) {
+      llvm::errs() << "Error: --config-file and --config are "
+                      "mutually exclusive. Specify only one.\n";
+      return nullptr;
     }
+
+    llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Text =
+        llvm::MemoryBuffer::getFile(ConfigFile.c_str());
+    if (std::error_code EC = Text.getError()) {
+      llvm::errs() << "Error: can't read config-file '" << ConfigFile
+                   << "': " << EC.message() << "\n";
+      return nullptr;
+    }
+
+    return LoadConfig((*Text)->getBuffer());
   }
+
+  if (Config.getNumOccurrences() > 0)
+    return LoadConfig(Config);
+
   return std::make_unique<FileOptionsProvider>(GlobalOptions, DefaultOptions,
                                                 OverrideOptions, std::move(FS));
 }
 
 llvm::IntrusiveRefCntPtr<vfs::FileSystem>
 getVfsFromFile(const std::string &OverlayFile,
                llvm::IntrusiveRefCntPtr<vfs::FileSystem> BaseFS) {
   llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Buffer =

clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-file/config-file

+Checks: "-*,hicpp-uppercase-literal-suffix"

clang-tools-extra/test/clang-tidy/infrastructure/config-file.cpp

+// RUN: clang-tidy -config-file=%S/Inputs/config-file/config-file -dump-config -- | FileCheck %s -check-prefix=CHECK-BASE
+// CHECK-BASE: Checks: {{.*}}hicpp-uppercase-literal-suffix