JonasToth added a comment.
Looking up to the Visitor, i will do this next.
================
Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:27
+ // Here are all the possible reasons:
+ enum Criteria : unsigned char {
+ None = 0,
----------------
i think clarifying which language constructs relative to what criteria would
help here.
with the document next to me here its clear, but i think we shouldn't expect
that. a link/refernce to the page would be nice, too.
================
Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:51
+ const SourceLocation Loc; // what caused the increment?
+ const unsigned short Nesting; // how deeply nestedly is Loc located?
+ const Criteria C : 3; // the criteria of the increment
----------------
s/nestedly/nested/
and make the trailing comments doxygen style? might help, but not so important
i think.
================
Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:52
+ const unsigned short Nesting; // how deeply nestedly is Loc located?
+ const Criteria C : 3; // the criteria of the increment
+
----------------
is this a bitfield? How does it help here? A byte should already be started
anyway, so leaving 5 bits empty wouldn't have an impact on the size of `Detail`
================
Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:57
+
+ // To minimize the the sizeof(Detail), we only store the minimal info
there.
+ // This function is used to convert from the stored info into the usable
----------------
duplicated 'the'
================
Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:62
+ std::pair<const char *, unsigned short> Process() const {
+ assert(C != Criteria::None && "invalid criteria");
+
----------------
You acces `Critera` always scoped. I think you could declare `Criteria` to be a
`enum class`, not just a plain `enum`
================
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",
+
----------------
could this initialization land in line 45? that would be directly close to the
criteria.
================
Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:116-146
+// Criteria is a bitset, thus a few helpers are needed
+static CognitiveComplexity::Criteria
+operator|(CognitiveComplexity::Criteria lhs,
+ CognitiveComplexity::Criteria rhs) {
+ return static_cast<CognitiveComplexity::Criteria>(
+ static_cast<std::underlying_type<CognitiveComplexity::Criteria>::type>(
+ lhs) |
----------------
`Criteria` could be moved out of the `Detail` struct. That would allow closer
definiton of the helper code to the `enum`.
================
Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:154
+ Details.emplace_back(Loc, Nesting, C);
+ const Detail &d(Details.back());
+
----------------
s/d/D/ naming convention
================
Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:168
+ // Used to efficiently know the last type of binary sequence operator that
+ // was encoutered. It would make sense for the function call to start the
+ // new sequence, thus it is a stack.
----------------
s/encoutered/encountered/
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
