[PATCH] D24892: [clang-tidy] Add option "LiteralInitializers" to cppcoreguidelines-pro-type-member-init

[Aaron Ballman via Phabricator via cfe-commits]

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