-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[clang-tidy] Fix cppcoreguidelines-prefer-member-initializer
false positive for inherited members
#153941
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[clang-tidy] Fix cppcoreguidelines-prefer-member-initializer
false positive for inherited members
#153941
Conversation
@llvm/pr-subscribers-clang-tidy Author: None (v1nh1shungry) Changesstruct Base {
int m;
};
template <class T>
struct Derived : Base {
Derived() { m = 0; }
}; would previously generate the following output:
This patch fixes this false positive. Note that before this patch the checker won't give false positive for struct Derived : Base {
Derived() { m = 0; }
}; and the constructor's AST is
so llvm-project/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp Lines 118 to 119 in f0967fc
Full diff: https://github.com/llvm/llvm-project/pull/153941.diff 3 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
index 593a4f85d1309..79cd4bbcc9a60 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
@@ -191,6 +191,9 @@ void PreferMemberInitializerCheck::check(
if (!AssignmentToMember)
continue;
const FieldDecl *Field = AssignmentToMember->Field;
+ // Skip if the field is inherited from a base class.
+ if (Field->getParent() != Class)
+ continue;
const Expr *InitValue = AssignmentToMember->Init;
updateAssignmentLevel(Field, InitValue, Ctor, AssignedFields);
if (!canAdvanceAssignment(AssignedFields[Field]))
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 1553f461634d0..0d5dbd85df6a4 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -163,6 +163,10 @@ Changes in existing checks
an additional matcher that generalizes the copy-and-swap idiom pattern
detection.
+- Improved :doc:`cppcoreguidelines-prefer-member-initializer
+ <clang-tidy/checks/cppcoreguidelines/prefer-member-initializer>` check by
+ no longer giving false positives for inherited members.
+
- Improved :doc:`misc-header-include-cycle
<clang-tidy/checks/misc/header-include-cycle>` check performance.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-member-initializer.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-member-initializer.cpp
index 7d6164946fc3d..faff4882142f1 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-member-initializer.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-member-initializer.cpp
@@ -650,3 +650,20 @@ struct InitFromBindingDecl {
}
};
} // namespace GH82970
+
+namespace inherited_members {
+
+struct A {
+ int m;
+};
+
+struct B : A {
+ B() { m = 0; }
+};
+
+template <class T>
+struct C : A {
+ C() { m = 0; }
+};
+
+} // namespace inherited_members
|
@llvm/pr-subscribers-clang-tools-extra Author: None (v1nh1shungry) Changesstruct Base {
int m;
};
template <class T>
struct Derived : Base {
Derived() { m = 0; }
}; would previously generate the following output:
This patch fixes this false positive. Note that before this patch the checker won't give false positive for struct Derived : Base {
Derived() { m = 0; }
}; and the constructor's AST is
so llvm-project/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp Lines 118 to 119 in f0967fc
Full diff: https://github.com/llvm/llvm-project/pull/153941.diff 3 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
index 593a4f85d1309..79cd4bbcc9a60 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
@@ -191,6 +191,9 @@ void PreferMemberInitializerCheck::check(
if (!AssignmentToMember)
continue;
const FieldDecl *Field = AssignmentToMember->Field;
+ // Skip if the field is inherited from a base class.
+ if (Field->getParent() != Class)
+ continue;
const Expr *InitValue = AssignmentToMember->Init;
updateAssignmentLevel(Field, InitValue, Ctor, AssignedFields);
if (!canAdvanceAssignment(AssignedFields[Field]))
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 1553f461634d0..0d5dbd85df6a4 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -163,6 +163,10 @@ Changes in existing checks
an additional matcher that generalizes the copy-and-swap idiom pattern
detection.
+- Improved :doc:`cppcoreguidelines-prefer-member-initializer
+ <clang-tidy/checks/cppcoreguidelines/prefer-member-initializer>` check by
+ no longer giving false positives for inherited members.
+
- Improved :doc:`misc-header-include-cycle
<clang-tidy/checks/misc/header-include-cycle>` check performance.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-member-initializer.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-member-initializer.cpp
index 7d6164946fc3d..faff4882142f1 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-member-initializer.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-member-initializer.cpp
@@ -650,3 +650,20 @@ struct InitFromBindingDecl {
}
};
} // namespace GH82970
+
+namespace inherited_members {
+
+struct A {
+ int m;
+};
+
+struct B : A {
+ B() { m = 0; }
+};
+
+template <class T>
+struct C : A {
+ C() { m = 0; }
+};
+
+} // namespace inherited_members
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This should fix #104400 |
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-member-initializer.cpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, please fix the CI errors :)
6c6ceab
to
05f32d9
Compare
…positive for inherited members in class templates ```cpp struct Base { int m; }; template <class T> struct Derived : Base { Derived() { m = 0; } }; ``` would previously generate the following output: ``` <source>:7:15: warning: 'm' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer] 7 | Derived() { m = 0; } | ^~~~~~ | : m(0) ``` This patch fixes this false positive.
05f32d9
to
c139268
Compare
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/169/builds/14124 Here is the relevant piece of the build log for the reference
|
would previously generate the following output:
This patch fixes this false positive.
Note that before this patch the checker won't give false positive for
and the constructor's AST is
so
isAssignmentToMemberOf
would return empty due tollvm-project/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
Lines 118 to 119 in f0967fc
Fixes #104400