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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to