Skip to content
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.

Sign up for GitHub

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

Fix UnnecessaryMutableChecker #47014

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Fix UnnecessaryMutableChecker #47014

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
739976f
Add missing check for access modifier in UnnecessaryMutableChecker; f…
iarspider Dec 19, 2024
41c8cf1
Update UnnecessaryMutableChecker.cpp
iarspider Dec 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 17 additions & 7 deletions Utilities/StaticAnalyzers/src/UnnecessaryMutableChecker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,15 @@ namespace clangcms {
if (!Field->isMutable()) {
continue;
}

// == Skip non-private mutables
// Public mutable members are forbidden and flagged by PublicMutableChecker
// Protected ones can be modified by derived classes, so it is bit hard to find if
// a `mutable` attribute is not necessary (i.e. not modified in any const method).
if (Field->getAccess() != clang::AS_private) {
return;
}

// == Skip atmoic mutables, these are thread-safe by design ==
if (support::isStdAtomic(Field)) {
return; // Skip if it's a mutable std::atomic
Expand All @@ -64,16 +73,17 @@ namespace clangcms {
if (!isMutableMemberModified(Field, RD)) {
clang::SourceLocation Loc = Field->getLocation();
if (Loc.isValid()) {
clang::ento::PathDiagnosticLocation FieldLoc(Loc, SM);
if (!BT) {
BT = std::make_unique<clang::ento::BugType>(this, "Unnecessarily Mutable Member", "Coding Practices");
}
BR.EmitBasicReport(
Field,
this,
"Useless mutable field",
"ConstThreadSafety",
"The mutable field '" + Field->getQualifiedNameAsString() + "' is not modified in any const methods",
PathLoc);
BR.EmitBasicReport(Field,
this,
"Useless mutable field",
"ConstThreadSafety",
"The mutable field '" + Field->getQualifiedNameAsString() +
"' is not modified in any public const methods",
FieldLoc);
}
}
}
Expand Down
6 changes: 2 additions & 4 deletions Utilities/StaticAnalyzers/test/reference.log
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
src/Utilities/StaticAnalyzers/test/class_checker.cpp:108:5: warning: 1st function call argument is an uninitialized value [core.CallAndMessage]
src/Utilities/StaticAnalyzers/test/class_checker.cpp:10:1: warning: Non-const variable 'int * g_ptr_static' is static and might be thread-unsafe [threadsafety.GlobalStatic]
src/Utilities/StaticAnalyzers/test/class_checker.cpp:12:1: warning: The mutable field 'ClassTest::m_pubMutable' is not modified in any const methods [cms.CodeRules.UnnecessaryMutableChecker]
src/Utilities/StaticAnalyzers/test/class_checker.cpp:161:10: warning: Value stored to 'ip' during its initialization is never read [deadcode.DeadStores]
src/Utilities/StaticAnalyzers/test/class_checker.cpp:162:10: warning: Value stored to 'cip' during its initialization is never read [deadcode.DeadStores]
src/Utilities/StaticAnalyzers/test/class_checker.cpp:167:5: warning: Non-const variable 'int & intRef' is static local or static member data and might be thread-unsafe [threadsafety.StaticLocal]
Expand All @@ -15,7 +14,6 @@ src/Utilities/StaticAnalyzers/test/global_static.cpp:10:1: warning: Non-const va
src/Utilities/StaticAnalyzers/test/global_static.cpp:11:1: warning: Non-const variable 'int * g_ptr_static' is static and might be thread-unsafe [threadsafety.GlobalStatic]
src/Utilities/StaticAnalyzers/test/global_static_edm.cpp:19:1: warning: Non-const variable 'int g_static' is static and might be thread-unsafe [threadsafety.GlobalStatic]
src/Utilities/StaticAnalyzers/test/global_static_edm.cpp:22:1: warning: Non-const variable 'edm::InputSourcePluginFactory g_static_edm_namespace' is static and might be thread-unsafe [threadsafety.GlobalStatic]
src/Utilities/StaticAnalyzers/test/mutable_member.cpp:12:1: warning: The mutable field 'Foo::badPublicMutable' is not modified in any const methods [cms.CodeRules.UnnecessaryMutableChecker]
src/Utilities/StaticAnalyzers/test/mutable_member.cpp:14:41: warning: Calling non-const method 'Bar::someMethod' of mutable member 'Foo::barMutableMember' in a const member function is potentially thread-unsafe [threadsafety.MutableMember]
src/Utilities/StaticAnalyzers/test/mutable_member.cpp:19:50: warning: Modifying mutable member 'Foo::privateMutable' in const member function is potentially thread-unsafe [threadsafety.MutableMember]
src/Utilities/StaticAnalyzers/test/mutable_member.cpp:20:50: warning: Modifying mutable member 'Foo::privateMutable' in const member function is potentially thread-unsafe [threadsafety.MutableMember]
Expand All @@ -27,7 +25,7 @@ src/Utilities/StaticAnalyzers/test/mutable_member.cpp:25:50: warning: Modifying
src/Utilities/StaticAnalyzers/test/mutable_member.cpp:29:15: warning: Class 'Foo' has publically-accessible mutable member 'Foo::badPublicMutable', this is not allowed [threadsafety.PublicMutableMember]
src/Utilities/StaticAnalyzers/test/static_local.cpp:8:5: warning: Non-const variable 'int & intRef' is static local or static member data and might be thread-unsafe [threadsafety.StaticLocal]
src/Utilities/StaticAnalyzers/test/static_local.cpp:9:5: warning: Non-const variable 'int * intPtr' is static local or static member data and might be thread-unsafe [threadsafety.StaticLocal]
src/Utilities/StaticAnalyzers/test/unnecessary_mutable_member.cpp:11:15: warning: The mutable field 'Foo::m_intMutable' is not modified in any public const methods [cms.CodeRules.UnnecessaryMutableChecker]
src/Utilities/StaticAnalyzers/test/unnecessary_mutable_member.cpp:12:23: warning: The mutable field 'Foo::m_strMutable' is not modified in any public const methods [cms.CodeRules.UnnecessaryMutableChecker]
src/Utilities/StaticAnalyzers/test/unnecessary_mutable_member.cpp:18:5: warning: Modifying mutable member 'Bar::m_intMutable' in const member function is potentially thread-unsafe [threadsafety.MutableMember]
src/Utilities/StaticAnalyzers/test/unnecessary_mutable_member.cpp:26:57: warning: Modifying mutable member 'Bar::m_strMutable' in const member function is potentially thread-unsafe [threadsafety.MutableMember]
src/Utilities/StaticAnalyzers/test/unnecessary_mutable_member.cpp:4:1: warning: The mutable field 'Foo::m_intMutable' is not modified in any const methods [cms.CodeRules.UnnecessaryMutableChecker]
src/Utilities/StaticAnalyzers/test/unnecessary_mutable_member.cpp:4:1: warning: The mutable field 'Foo::m_strMutable' is not modified in any const methods [cms.CodeRules.UnnecessaryMutableChecker]