Skip to content

Commit cb1ee34

Browse files
dmpolukhindmpolukhin
committed
[clang-tidy] Optional inheritance of file configs from parent directories 
Summary: Without this patch clang-tidy stops finding file configs on the nearest .clang-tidy file. In some cases it is not very convenient because it results in common parts duplication into every child .clang-tidy file. This diff adds optional config inheritance from the parent directories config files. Test Plan: Added test cases in existing config test. Reviewers: alexfh, gribozavr2, klimek, hokein Subscribers: njames93, arphaman, xazax.hun, aheejin, cfe-commits Tags: #clang, #clang-tools-extra Differential Revision: https://reviews.llvm.org/D75184
1 parent d85b387 commit cb1ee34

File tree

12 files changed

+140
-57
lines changed

12 files changed

+140
-57
lines changed

‎clang-tools-extra/clang-tidy/ClangTidy.cpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,9 @@ static void setStaticAnalyzerCheckerOpts(const ClangTidyOptions &Opts,
328328
StringRef OptName(Opt.first);
329329
if (!OptName.startswith(AnalyzerPrefix))
330330
continue;
331-
AnalyzerOptions->Config[OptName.substr(AnalyzerPrefix.size())] = Opt.second;
331+
// Analyzer options are always local options so we can ignore priority.
332+
AnalyzerOptions->Config[OptName.substr(AnalyzerPrefix.size())] =
333+
Opt.second.Value;
332334
}
333335
}
334336

‎clang-tools-extra/clang-tidy/ClangTidyCheck.cpp

+22-23
Original file line numberDiff line numberDiff line change
@@ -72,19 +72,20 @@ llvm::Expected<std::string>
7272
ClangTidyCheck::OptionsView::get(StringRef LocalName) const {
7373
const auto &Iter = CheckOptions.find(NamePrefix + LocalName.str());
7474
if (Iter != CheckOptions.end())
75-
return Iter->second;
75+
return Iter->second.Value;
7676
return llvm::make_error<MissingOptionError>((NamePrefix + LocalName).str());
7777
}
7878

7979
llvm::Expected<std::string>
8080
ClangTidyCheck::OptionsView::getLocalOrGlobal(StringRef LocalName) const {
81-
auto Iter = CheckOptions.find(NamePrefix + LocalName.str());
82-
if (Iter != CheckOptions.end())
83-
return Iter->second;
84-
// Fallback to global setting, if present.
85-
Iter = CheckOptions.find(LocalName.str());
86-
if (Iter != CheckOptions.end())
87-
return Iter->second;
81+
auto IterLocal = CheckOptions.find(NamePrefix + LocalName.str());
82+
auto IterGlobal = CheckOptions.find(LocalName.str());
83+
if (IterLocal != CheckOptions.end() &&
84+
(IterGlobal == CheckOptions.end() ||
85+
IterLocal->second.Priority >= IterGlobal->second.Priority))
86+
return IterLocal->second.Value;
87+
if (IterGlobal != CheckOptions.end())
88+
return IterGlobal->second.Value;
8889
return llvm::make_error<MissingOptionError>((NamePrefix + LocalName).str());
8990
}
9091

@@ -123,17 +124,15 @@ bool ClangTidyCheck::OptionsView::get<bool>(StringRef LocalName,
123124
template <>
124125
llvm::Expected<bool>
125126
ClangTidyCheck::OptionsView::getLocalOrGlobal<bool>(StringRef LocalName) const {
126-
llvm::Expected<std::string> ValueOr = get(LocalName);
127-
bool IsGlobal = false;
128-
if (!ValueOr) {
129-
llvm::consumeError(ValueOr.takeError());
130-
ValueOr = getLocalOrGlobal(LocalName);
131-
IsGlobal = true;
132-
}
133-
if (!ValueOr)
134-
return ValueOr.takeError();
135-
return getAsBool(*ValueOr, IsGlobal ? llvm::Twine(LocalName)
136-
: (NamePrefix + LocalName));
127+
auto IterLocal = CheckOptions.find(NamePrefix + LocalName.str());
128+
auto IterGlobal = CheckOptions.find(LocalName.str());
129+
if (IterLocal != CheckOptions.end() &&
130+
(IterGlobal == CheckOptions.end() ||
131+
IterLocal->second.Priority >= IterGlobal->second.Priority))
132+
return getAsBool(IterLocal->second.Value, NamePrefix + LocalName);
133+
if (IterGlobal != CheckOptions.end())
134+
return getAsBool(IterGlobal->second.Value, llvm::Twine(LocalName));
135+
return llvm::make_error<MissingOptionError>((NamePrefix + LocalName).str());
137136
}
138137

139138
template <>
@@ -149,7 +148,7 @@ bool ClangTidyCheck::OptionsView::getLocalOrGlobal<bool>(StringRef LocalName,
149148
void ClangTidyCheck::OptionsView::store(ClangTidyOptions::OptionMap &Options,
150149
StringRef LocalName,
151150
StringRef Value) const {
152-
Options[NamePrefix + LocalName.str()] = std::string(Value);
151+
Options[NamePrefix + LocalName.str()] = Value;
153152
}
154153

155154
void ClangTidyCheck::OptionsView::store(ClangTidyOptions::OptionMap &Options,
@@ -167,7 +166,7 @@ llvm::Expected<int64_t> ClangTidyCheck::OptionsView::getEnumInt(
167166
if (Iter == CheckOptions.end())
168167
return llvm::make_error<MissingOptionError>((NamePrefix + LocalName).str());
169168

170-
StringRef Value = Iter->second;
169+
StringRef Value = Iter->second.Value;
171170
StringRef Closest;
172171
unsigned EditDistance = -1;
173172
for (const auto &NameAndEnum : Mapping) {
@@ -189,9 +188,9 @@ llvm::Expected<int64_t> ClangTidyCheck::OptionsView::getEnumInt(
189188
}
190189
if (EditDistance < 3)
191190
return llvm::make_error<UnparseableEnumOptionError>(
192-
Iter->first, Iter->second, std::string(Closest));
191+
Iter->first, Iter->second.Value, std::string(Closest));
193192
return llvm::make_error<UnparseableEnumOptionError>(Iter->first,
194-
Iter->second);
193+
Iter->second.Value);
195194
}
196195

197196
void ClangTidyCheck::OptionsView::logErrToStdErr(llvm::Error &&Err) {

‎clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ ClangTidyOptions ClangTidyContext::getOptionsForFile(StringRef File) const {
199199
// Merge options on top of getDefaults() as a safeguard against options with
200200
// unset values.
201201
return ClangTidyOptions::getDefaults().mergeWith(
202-
OptionsProvider->getOptions(File));
202+
OptionsProvider->getOptions(File), 0);
203203
}
204204

205205
void ClangTidyContext::setEnableProfiling(bool P) { Profile = P; }

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

+27-11
Original file line numberDiff line numberDiff line change
@@ -67,12 +67,15 @@ template <> struct MappingTraits<ClangTidyOptions::StringPair> {
6767

6868
struct NOptionMap {
6969
NOptionMap(IO &) {}
70-
NOptionMap(IO &, const ClangTidyOptions::OptionMap &OptionMap)
71-
: Options(OptionMap.begin(), OptionMap.end()) {}
70+
NOptionMap(IO &, const ClangTidyOptions::OptionMap &OptionMap) {
71+
Options.reserve(OptionMap.size());
72+
for (const auto &KeyValue : OptionMap)
73+
Options.emplace_back(KeyValue.first, KeyValue.second.Value);
74+
}
7275
ClangTidyOptions::OptionMap denormalize(IO &) {
7376
ClangTidyOptions::OptionMap Map;
7477
for (const auto &KeyValue : Options)
75-
Map[KeyValue.first] = KeyValue.second;
78+
Map[KeyValue.first] = ClangTidyOptions::ClangTidyValue(KeyValue.second);
7679
return Map;
7780
}
7881
std::vector<ClangTidyOptions::StringPair> Options;
@@ -92,6 +95,7 @@ template <> struct MappingTraits<ClangTidyOptions> {
9295
IO.mapOptional("CheckOptions", NOpts->Options);
9396
IO.mapOptional("ExtraArgs", Options.ExtraArgs);
9497
IO.mapOptional("ExtraArgsBefore", Options.ExtraArgsBefore);
98+
IO.mapOptional("InheritParentConfig", Options.InheritParentConfig);
9599
}
96100
};
97101

@@ -109,10 +113,12 @@ ClangTidyOptions ClangTidyOptions::getDefaults() {
109113
Options.SystemHeaders = false;
110114
Options.FormatStyle = "none";
111115
Options.User = llvm::None;
116+
unsigned Priority = 0;
112117
for (ClangTidyModuleRegistry::iterator I = ClangTidyModuleRegistry::begin(),
113118
E = ClangTidyModuleRegistry::end();
114119
I != E; ++I)
115-
Options = Options.mergeWith(I->instantiate()->getModuleOptions());
120+
Options =
121+
Options.mergeWith(I->instantiate()->getModuleOptions(), ++Priority);
116122
return Options;
117123
}
118124

@@ -138,8 +144,8 @@ static void overrideValue(Optional<T> &Dest, const Optional<T> &Src) {
138144
Dest = Src;
139145
}
140146

141-
ClangTidyOptions
142-
ClangTidyOptions::mergeWith(const ClangTidyOptions &Other) const {
147+
ClangTidyOptions ClangTidyOptions::mergeWith(const ClangTidyOptions &Other,
148+
unsigned Priority) const {
143149
ClangTidyOptions Result = *this;
144150

145151
mergeCommaSeparatedLists(Result.Checks, Other.Checks);
@@ -151,8 +157,10 @@ ClangTidyOptions::mergeWith(const ClangTidyOptions &Other) const {
151157
mergeVectors(Result.ExtraArgs, Other.ExtraArgs);
152158
mergeVectors(Result.ExtraArgsBefore, Other.ExtraArgsBefore);
153159

154-
for (const auto &KeyValue : Other.CheckOptions)
155-
Result.CheckOptions[KeyValue.first] = KeyValue.second;
160+
for (const auto &KeyValue : Other.CheckOptions) {
161+
Result.CheckOptions[KeyValue.first] = ClangTidyValue(
162+
KeyValue.second.Value, KeyValue.second.Priority + Priority);
163+
}
156164

157165
return Result;
158166
}
@@ -168,8 +176,9 @@ const char
168176
ClangTidyOptions
169177
ClangTidyOptionsProvider::getOptions(llvm::StringRef FileName) {
170178
ClangTidyOptions Result;
179+
unsigned Priority = 0;
171180
for (const auto &Source : getRawOptions(FileName))
172-
Result = Result.mergeWith(Source.first);
181+
Result = Result.mergeWith(Source.first, ++Priority);
173182
return Result;
174183
}
175184

@@ -237,6 +246,7 @@ FileOptionsProvider::getRawOptions(StringRef FileName) {
237246
DefaultOptionsProvider::getRawOptions(AbsoluteFilePath.str());
238247
OptionsSource CommandLineOptions(OverrideOptions,
239248
OptionsSourceTypeCheckCommandLineOption);
249+
size_t FirstFileConfig = RawOptions.size();
240250
// Look for a suitable configuration file in all parent directories of the
241251
// file. Start with the immediate parent directory and move up.
242252
StringRef Path = llvm::sys::path::parent_path(AbsoluteFilePath.str());
@@ -256,15 +266,21 @@ FileOptionsProvider::getRawOptions(StringRef FileName) {
256266
while (Path != CurrentPath) {
257267
LLVM_DEBUG(llvm::dbgs()
258268
<< "Caching configuration for path " << Path << ".\n");
259-
CachedOptions[Path] = *Result;
269+
if (!CachedOptions.count(Path))
270+
CachedOptions[Path] = *Result;
260271
Path = llvm::sys::path::parent_path(Path);
261272
}
262273
CachedOptions[Path] = *Result;
263274

264275
RawOptions.push_back(*Result);
265-
break;
276+
if (!Result->first.InheritParentConfig ||
277+
!*Result->first.InheritParentConfig)
278+
break;
266279
}
267280
}
281+
// Reverse order of file configs because closer configs should have higher
282+
// priority.
283+
std::reverse(RawOptions.begin() + FirstFileConfig, RawOptions.end());
268284
RawOptions.push_back(CommandLineOptions);
269285
return RawOptions;
270286
}

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

+22-2
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,9 @@ struct ClangTidyOptions {
5858

5959
/// Creates a new \c ClangTidyOptions instance combined from all fields
6060
/// of this instance overridden by the fields of \p Other that have a value.
61-
ClangTidyOptions mergeWith(const ClangTidyOptions &Other) const;
61+
/// \p Order specifies precedence of \p Other option.
62+
ClangTidyOptions mergeWith(const ClangTidyOptions &Other,
63+
unsigned Order) const;
6264

6365
/// Checks filter.
6466
llvm::Optional<std::string> Checks;
@@ -93,8 +95,20 @@ struct ClangTidyOptions {
9395
/// comments in the relevant check.
9496
llvm::Optional<std::string> User;
9597

98+
/// Helper structure for storing option value with priority of the value.
99+
struct ClangTidyValue {
100+
ClangTidyValue() : Value(), Priority(0) {}
101+
ClangTidyValue(const char *Value) : Value(Value), Priority(0) {}
102+
ClangTidyValue(llvm::StringRef Value, unsigned Priority = 0)
103+
: Value(Value), Priority(Priority) {}
104+
105+
std::string Value;
106+
/// Priority stores relative precedence of the value loaded from config
107+
/// files to disambigute local vs global value from different levels.
108+
unsigned Priority;
109+
};
96110
typedef std::pair<std::string, std::string> StringPair;
97-
typedef std::map<std::string, std::string> OptionMap;
111+
typedef std::map<std::string, ClangTidyValue> OptionMap;
98112

99113
/// Key-value mapping used to store check-specific options.
100114
OptionMap CheckOptions;
@@ -106,6 +120,12 @@ struct ClangTidyOptions {
106120

107121
/// Add extra compilation arguments to the start of the list.
108122
llvm::Optional<ArgList> ExtraArgsBefore;
123+
124+
/// Only used in the FileOptionsProvider. If true, FileOptionsProvider will
125+
/// take a configuration file in the parent directory (if any exists) and
126+
/// apply this config file on top of the parent one. If false or missing,
127+
/// only this configuration file will be used.
128+
llvm::Optional<bool> InheritParentConfig;
109129
};
110130

111131
/// Abstract interface for retrieving various ClangTidy options.

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

+13-10
Original file line numberDiff line numberDiff line change
@@ -36,17 +36,20 @@ static cl::extrahelp ClangTidyHelp(R"(
3636
Configuration files:
3737
clang-tidy attempts to read configuration for each source file from a
3838
.clang-tidy file located in the closest parent directory of the source
39-
file. If any configuration options have a corresponding command-line
40-
option, command-line option takes precedence. The effective
41-
configuration can be inspected using -dump-config:
39+
file. If InheritParentConfig is true in a config file, the configuration file
40+
in the parent directory (if any exists) will be taken and current config file
41+
will be applied on top of the parent one. If any configuration options have
42+
a corresponding command-line option, command-line option takes precedence.
43+
The effective configuration can be inspected using -dump-config:
4244
4345
$ clang-tidy -dump-config
4446
---
45-
Checks: '-*,some-check'
46-
WarningsAsErrors: ''
47-
HeaderFilterRegex: ''
48-
FormatStyle: none
49-
User: user
47+
Checks: '-*,some-check'
48+
WarningsAsErrors: ''
49+
HeaderFilterRegex: ''
50+
FormatStyle: none
51+
InheritParentConfig: true
52+
User: user
5053
CheckOptions:
5154
- key: some-check.SomeOption
5255
value: 'some value'
@@ -294,7 +297,7 @@ static std::unique_ptr<ClangTidyOptionsProvider> createOptionsProvider(
294297
parseConfiguration(Config)) {
295298
return std::make_unique<ConfigOptionsProvider>(
296299
GlobalOptions,
297-
ClangTidyOptions::getDefaults().mergeWith(DefaultOptions),
300+
ClangTidyOptions::getDefaults().mergeWith(DefaultOptions, 0),
298301
*ParsedConfig, OverrideOptions);
299302
} else {
300303
llvm::errs() << "Error: invalid configuration specified.\n"
@@ -406,7 +409,7 @@ int clangTidyMain(int argc, const char **argv) {
406409
getCheckOptions(EffectiveOptions, AllowEnablingAnalyzerAlphaCheckers);
407410
llvm::outs() << configurationAsText(
408411
ClangTidyOptions::getDefaults().mergeWith(
409-
EffectiveOptions))
412+
EffectiveOptions, 0))
410413
<< "\n";
411414
return 0;
412415
}

‎clang-tools-extra/docs/clang-tidy/index.rst

+11-8
Original file line numberDiff line numberDiff line change
@@ -247,17 +247,20 @@ An overview of all the command-line options:
247247
Configuration files:
248248
clang-tidy attempts to read configuration for each source file from a
249249
.clang-tidy file located in the closest parent directory of the source
250-
file. If any configuration options have a corresponding command-line
251-
option, command-line option takes precedence. The effective
252-
configuration can be inspected using -dump-config:
250+
file. If InheritParentConfig is true in a config file, the configuration file
251+
in the parent directory (if any exists) will be taken and current config file
252+
will be applied on top of the parent one. If any configuration options have
253+
a corresponding command-line option, command-line option takes precedence.
254+
The effective configuration can be inspected using -dump-config:
253255
254256
$ clang-tidy -dump-config
255257
---
256-
Checks: '-*,some-check'
257-
WarningsAsErrors: ''
258-
HeaderFilterRegex: ''
259-
FormatStyle: none
260-
User: user
258+
Checks: '-*,some-check'
259+
WarningsAsErrors: ''
260+
HeaderFilterRegex: ''
261+
FormatStyle: none
262+
InheritParentConfig: true
263+
User: user
261264
CheckOptions:
262265
- key: some-check.SomeOption
263266
value: 'some value'

‎clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/3/.clang-tidy

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
InheritParentConfig: true
2+
Checks: 'from-child3'
3+
HeaderFilterRegex: 'child3'

‎clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/4/.clang-tidy

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
Checks: '-*,modernize-loop-convert,modernize-use-using'
2+
CheckOptions:
3+
- key: modernize-loop-convert.MaxCopySize
4+
value: '10'
5+
- key: modernize-loop-convert.MinConfidence
6+
value: reasonable
7+
- key: modernize-use-using.IgnoreMacros
8+
value: 1

‎clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/4/44/.clang-tidy

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
InheritParentConfig: true
2+
Checks: 'llvm-qualified-auto'
3+
CheckOptions:
4+
- key: modernize-loop-convert.MaxCopySize
5+
value: '20'
6+
- key: llvm-qualified-auto.AddConstToQualified
7+
value: '1'
8+
- key: IgnoreMacros
9+
value: '0'

‎clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp

+20
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,26 @@
77
// RUN: clang-tidy -dump-config %S/Inputs/config-files/2/- -- | FileCheck %s -check-prefix=CHECK-CHILD2
88
// CHECK-CHILD2: Checks: {{.*}}from-parent
99
// CHECK-CHILD2: HeaderFilterRegex: parent
10+
// RUN: clang-tidy -dump-config %S/Inputs/config-files/3/- -- | FileCheck %s -check-prefix=CHECK-CHILD3
11+
// CHECK-CHILD3: Checks: {{.*}}from-parent,from-child3
12+
// CHECK-CHILD3: HeaderFilterRegex: child3
1013
// RUN: clang-tidy -dump-config -checks='from-command-line' -header-filter='from command line' %S/Inputs/config-files/- -- | FileCheck %s -check-prefix=CHECK-COMMAND-LINE
1114
// CHECK-COMMAND-LINE: Checks: {{.*}}from-parent,from-command-line
1215
// CHECK-COMMAND-LINE: HeaderFilterRegex: from command line
16+
17+
// For this test we have to use names of the real checks because otherwise values are ignored.
18+
// RUN: clang-tidy -dump-config %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-CHILD4
19+
// CHECK-CHILD4: Checks: {{.*}}modernize-loop-convert,modernize-use-using,llvm-qualified-auto
20+
// CHECK-CHILD4: - key: llvm-qualified-auto.AddConstToQualified
21+
// CHECK-CHILD4-NEXT: value: '1
22+
// CHECK-CHILD4: - key: modernize-loop-convert.MaxCopySize
23+
// CHECK-CHILD4-NEXT: value: '20'
24+
// CHECK-CHILD4: - key: modernize-loop-convert.MinConfidence
25+
// CHECK-CHILD4-NEXT: value: reasonable
26+
// CHECK-CHILD4: - key: modernize-use-using.IgnoreMacros
27+
// CHECK-CHILD4-NEXT: value: '0'
28+
29+
// RUN: clang-tidy --explain-config %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-EXPLAIN
30+
// CHECK-EXPLAIN: 'llvm-qualified-auto' is enabled in the {{.*}}/Inputs/config-files/4/44/.clang-tidy.
31+
// CHECK-EXPLAIN: 'modernize-loop-convert' is enabled in the {{.*}}/Inputs/config-files/4/.clang-tidy.
32+
// CHECK-EXPLAIN: 'modernize-use-using' is enabled in the {{.*}}/Inputs/config-files/4/.clang-tidy.

‎clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ TEST(ParseConfiguration, MergeConfigurations) {
8787
ExtraArgsBefore: ['arg-before3', 'arg-before4']
8888
)");
8989
ASSERT_TRUE(!!Options2);
90-
ClangTidyOptions Options = Options1->mergeWith(*Options2);
90+
ClangTidyOptions Options = Options1->mergeWith(*Options2, 0);
9191
EXPECT_EQ("check1,check2,check3,check4", *Options.Checks);
9292
EXPECT_EQ("filter2", *Options.HeaderFilterRegex);
9393
EXPECT_EQ("user2", *Options.User);

0 commit comments

Comments
 (0)