[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat May 30 10:03:12 PDT 2020


aaron.ballman added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:106-107
+void PreferMemberInitializerCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+    return;
+
----------------
This should now be done by overriding `bool isLanguageVersionSupported(const LangOptions &LangOpts) const override;`, see bugprone/ThrowKeywordMissingCheck.h for an example.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst:3
+
+cppcoreguidelines-prefer-member-initializer
+===========================================
----------------
You should probably link to the rule from somewhere in these docs.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-assignment.cpp:29
+public:
+  Simple2() : n (0) {
+    x = 0.0;
----------------
By my reading of the core guideline (https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c48-prefer-in-class-initializers-to-member-initializers-in-constructors-for-constant-initializers), it looks like `n` should also be diagnosed because all of the constructors in the class initialize the member to the same constant value. Is there a reason to deviate from the rule (or have I missed something)?

Also, I'd like to see a test case like:
```
class C {
  int n;
public:
  C() { n = 0; }
  explicit C(int) { n = 12; }
};
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71199/new/

https://reviews.llvm.org/D71199





More information about the cfe-commits mailing list