[PATCH] [clang-tidy] Add a checker for long functions.
Alexander Kornienko
alexfh at google.com
Fri Sep 12 09:46:14 PDT 2014
================
Comment at: clang-tidy/misc/FunctionSize.cpp:21
@@ +20,3 @@
+ LineThreshold(Options.get("LineThreshold", -1U)),
+ StatementThreshold(Options.get("StatementThreshold", 800)),
+ BranchThreshold(Options.get("BranchThreshold", -1U)) {}
----------------
Please make it 800U to instantiate get<unsigned>().
================
Comment at: clang-tidy/tool/ClangTidyMain.cpp:108
@@ +107,3 @@
+ ConfigFile("config",
+ cl::desc("File to override the configuration. Specifing a\nfile "
+ "will disable the default configuration file\nsearch."),
----------------
1. Please move configuration-related changes to a separate patch.
2. Please break the strings after \n's (and maybe file a bug against clang-format to respect \n's located close to the column limit).
3. s/Specifing/Specifying/
4. It seems reasonable to allow inline configurations like in clang-format, if the -config= value starts with a '{':
clang-tidy -config='{CheckOptions: [{key: misc-function-size.LineThreshold, value: 0}, {key: misc-function-size.StatementThreshold, value: 0}]}' some-file.cpp
================
Comment at: clang-tidy/tool/ClangTidyMain.cpp:178
@@ +177,3 @@
+ if (std::error_code EC =
+ parseConfiguration((*Text)->getBuffer(), EffectiveOptions)) {
+ llvm::errs() << "Error reading config file: " << EC.message() << '\n';
----------------
EffectiveOptions is only used for -list-checks and -dump-config. ClangTidy itself uses OptionsProvider, so overriding configuration file needs to be done in FileOptionsProvider. Alternatively, you can do:
OptionsProvider.reset(new DefaultOptionsProvider(GlobalOptions, EffectiveOptions));
after this. I'd probably prefer the first way.
================
Comment at: clang-tidy/tool/ClangTidyMain.cpp:182
@@ +181,3 @@
+ }
+ EffectiveOptions.mergeWith(OverrideOptions);
+ } else {
----------------
This line doesn't do what you expect (mergeWith should probably have LLVM_ATTRIBUTE_UNUSED_RESULT).
http://reviews.llvm.org/D4986
More information about the cfe-commits
mailing list