[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:102
+    const unsigned short Nesting; /// How deeply nested is Loc located?
+    const Criteria C : 3;         /// The criteria of the increment
+
----------------
lebedev.ri wrote:
> aaron.ballman wrote:
> > lebedev.ri wrote:
> > > aaron.ballman wrote:
> > > > Why is this turned into a bit-field? If that is important, it should be of type `unsigned` to prevent differences between compilers (MSVC treats bit-fields differently than GCC and Clang).
> > > The comment right after this member variable should have explained it:
> > > ```
> > > /// Criteria C is a bitfield. Even though Criteria is an unsigned char; and
> > > /// only using 3 bits will still result in padding, the fact that it is a
> > > /// bitfield is a reminder that it is important to min(sizeof(Detail))
> > > ```
> > > It is already `unsigned`:
> > > ```
> > > enum Criteria : unsigned char {
> > > ```
> > No, it's underlying type is `unsigned char`, but the type is `Criteria`. I meant the field itself needs to be `unsigned`. However, since that means you have to do more type casting, I think making the type `uint8_t` instead of a bit-field would be an improvement.
> Better?
Much, thank you!


================
Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:499
+      functionDecl(
+          allOf(isDefinition(), unless(anyOf(isInstantiated(), isImplicit()))))
+          .bind("func"),
----------------
Since we're restricting this, might as well add deleted and defaulted functions to the list of `unless` items (as I think those are still definitions).


Repository:
  rL LLVM

https://reviews.llvm.org/D36836