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