aaron.ballman added inline comments.

================
Comment at: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp:348
 
+static const char *getInitializer(QualType type, bool LiteralInitializers) {
+  const char *DefaultInitializer = "{}";
----------------
`type` doesn't follow our usual naming conventions. I would recommend `QT`.


================
Comment at: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp:357
+  const BuiltinType *BT =
+      dyn_cast<BuiltinType>(type->getCanonicalTypeInternal());
+  if (!BT)
----------------
You shouldn't be calling an "internal" function here. Instead, you can do 
`dyn_cast<BuiltinType>(type.getCanonicalType().getTypePtr())`


================
Comment at: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp:369
+  case BuiltinType::LongDouble:
+    return " = 0.0l";
+  case BuiltinType::SChar:
----------------
Some users care deeply about `l` vs `L` because of how hard it is to 
distinguish depending on code fonts. I don't know that we need an option for 
that sort of thing, but my personal preference is to use capital letters rather 
than lowercase ones. (I originally read this as initializing to 0.01, FWIW).


================
Comment at: 
test/clang-tidy/cppcoreguidelines-pro-type-member-init-literal-initializers.cpp:1
+// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-member-init %t -- 
-config="{CheckOptions: [{key: 
"cppcoreguidelines-pro-type-member-init.LiteralInitializers", value: 1}]}" -- 
-std=c++11
+
----------------
mgehre wrote:
> hokein wrote:
> > `-std=c++11` is not needed. This extra compile argument is added by default 
> > when running check_clang_tidy.
> If I remove ``-std=c++11``, the behavior changes and 
> Context.getLangOpts().CPlusPlus11 is false.
That's because the extra args you specify disable the c++11 specification, so 
this is correct to do.


https://reviews.llvm.org/D24892



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

Reply via email to