JonasToth added a comment. For my part the current state is ok. @alexfh and @aaron.ballman but should do their review before committing. I would be interested in a exampleoutput for any real project.
================ Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:102-114 +const std::array<const char *const, 4> CognitiveComplexity::Msgs = {{ + // B1 + B2 + B3 + "+%0, including nesting penalty of %1, nesting level increased to %2", + + // B1 + B2 + "+%0, nesting level increased to %2", + ---------------- lebedev.ri wrote: > JonasToth wrote: > > lebedev.ri wrote: > > > JonasToth wrote: > > > > could this initialization land in line 45? that would be directly close > > > > to the criteria. > > > It would be nice indeed, but if i do > > > ``` > > > // All the possible messages that can be outputed. The choice of the > > > message > > > // to use is based of the combination of the Criterias > > > static constexpr std::array<const char *const, 4> Msgs = {{ > > > // B1 + B2 + B3 > > > "+%0, including nesting penalty of %1, nesting level increased to > > > %2", > > > > > > // B1 + B2 > > > "+%0, nesting level increased to %2", > > > > > > // B1 > > > "+%0", > > > > > > // B2 > > > "nesting level increased to %2", > > > }}; > > > ``` > > > i get > > > ``` > > > $ ninja check-clang-tools -j1 > > > [1/5] Linking CXX executable bin/clang-tidy > > > FAILED: bin/clang-tidy > > > : && /usr/local/bin/clang++ -fPIC -fvisibility-inlines-hidden > > > -Werror=date-time -Werror=unguarded-availability-new -std=c++11 -Wall -W > > > -Wno-unused-parameter -Wwrite-strings -Wcast-qual > > > -Wmissing-field-initializers -pedantic -Wno-long-long > > > -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor > > > -Wstring-conversion -fcolor-diagnostics -ffunction-sections > > > -fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types > > > -O3 -DNDEBUG -Wl,-allow-shlib-undefined -Wl,-O3 -Wl,--gc-sections > > > tools/clang/tools/extra/clang-tidy/tool/CMakeFiles/clang-tidy.dir/ClangTidyMain.cpp.o > > > -o bin/clang-tidy -Wl,-rpath,"\$ORIGIN/../lib" lib/libLLVMSupport.a > > > -lpthread lib/libclangAST.a lib/libclangASTMatchers.a lib/libclangBasic.a > > > lib/libclangTidy.a lib/libclangTidyAndroidModule.a > > > lib/libclangTidyBoostModule.a lib/libclangTidyBugproneModule.a > > > lib/libclangTidyCERTModule.a lib/libclangTidyCppCoreGuidelinesModule.a > > > lib/libclangTidyGoogleModule.a lib/libclangTidyHICPPModule.a > > > lib/libclangTidyLLVMModule.a lib/libclangTidyMiscModule.a > > > lib/libclangTidyModernizeModule.a lib/libclangTidyMPIModule.a > > > lib/libclangTidyPerformanceModule.a lib/libclangTidyReadabilityModule.a > > > lib/libclangTooling.a lib/libclangToolingCore.a > > > lib/libclangTidyCppCoreGuidelinesModule.a lib/libclangTidyGoogleModule.a > > > lib/libclangTidyMiscModule.a lib/libclangTidyReadabilityModule.a > > > lib/libclangTidyUtils.a lib/libclangTidy.a lib/libclangTooling.a > > > lib/libclangFormat.a lib/libclangToolingCore.a > > > lib/libclangStaticAnalyzerFrontend.a lib/libclangFrontend.a > > > lib/libclangDriver.a lib/libLLVMOption.a lib/libclangParse.a > > > lib/libLLVMMCParser.a lib/libclangSerialization.a lib/libclangSema.a > > > lib/libclangEdit.a lib/libLLVMBitReader.a lib/libLLVMProfileData.a > > > lib/libclangStaticAnalyzerCheckers.a lib/libclangStaticAnalyzerCore.a > > > lib/libclangASTMatchers.a lib/libclangRewrite.a lib/libclangAnalysis.a > > > lib/libclangAST.a lib/libclangLex.a lib/libclangBasic.a lib/libLLVMCore.a > > > lib/libLLVMBinaryFormat.a lib/libLLVMMC.a lib/libLLVMSupport.a -lrt -ldl > > > -ltinfo -lpthread -lz -lm lib/libLLVMDemangle.a && : > > > lib/libclangTidyReadabilityModule.a(FunctionCognitiveComplexityCheck.cpp.o):FunctionCognitiveComplexityCheck.cpp:function > > > > > > clang::tidy::readability::FunctionCognitiveComplexityCheck::check(clang::ast_matchers::MatchFinder::MatchResult > > > const&): error: undefined reference to > > > 'clang::tidy::readability::(anonymous > > > namespace)::CognitiveComplexity::Msgs' > > > ``` > > > Same if `process()` returns `std::pair<unsigned, unsigned short>` and in > > > `FunctionCognitiveComplexityCheck::check()` i do `const char > > > *IncreaseMessage = Visitor.CC.Msgs[IncreaseMsgId];` > > might the `static` cause that linking error? > > Did you consider using `StringRef` instead of `const char*`? It is better > > behaved. > > > > ```constexpr std::array<StringRef, 4> Msgs = { /* messages */ };``` > Actually, found a combination that works, done. > FIXME: so should `const std::array<const StringRef, 4> > CognitiveComplexity::Msgs` be `static` and initialized out-of-line, or not > `static`, but initialized in-line? Since it is already in an anonymous namespace, static would be duplicated, wouldn't it? I like it now! ================ Comment at: docs/ReleaseNotes.rst:138 + + Checks function Cognitive Complexity metric, and flags the functions with + Cognitive Complexity exceeding the configured limit. ---------------- lebedev.ri wrote: > JonasToth wrote: > > lebedev.ri wrote: > > > JonasToth wrote: > > > > How about > > > > 'Applies the Cognitive Complexity metric to functions (classes?), > > > > flagging those exceeding the configured limit/threshold' > > > English is hard :P > > > Reworded, maybe better? > > I struggle with it as well ;D > > `Calculates the Cognitive Complexity metric for each function and flags > > those with a value exceeding the configured limit.` > > I am not sure about the `flags those ...` part. > > Swapping the beginning would make the sentence easier to read. > Reworded once more Sounds good. :) Repository: rL LLVM https://reviews.llvm.org/D36836 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits