-
Notifications
You must be signed in to change notification settings - Fork 15.5k
[alpha.webkit.UncountedLocalVarsChecker] Ignore a VarDecl in "if" with trivial "then" #171764
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
Conversation
…h trivial "then" Don't emit a warning when a variable declaration appears within the "if" condition and if its "then" statement is trivial.
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-static-analyzer-1 Author: Ryosuke Niwa (rniwa) ChangesDon't emit a warning when a variable declaration appears within the "if" condition and if its "then" statement is trivial. Full diff: https://github.com/llvm/llvm-project/pull/171764.diff 4 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
index f2235e7c25ab2..404ec0783dd64 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
@@ -234,6 +234,10 @@ class RawPtrRefLocalVarsChecker
}
bool TraverseIfStmt(IfStmt *IS) override {
+ if (IS->getConditionVariable()) {
+ if (auto *Then = IS->getThen(); !Then || TFA.isTrivial(Then))
+ return true;
+ }
if (!TFA.isTrivial(IS))
return DynamicRecursiveASTVisitor::TraverseIfStmt(IS);
return true;
diff --git a/clang/test/Analysis/Checkers/WebKit/local-vars-checked-const-member.cpp b/clang/test/Analysis/Checkers/WebKit/local-vars-checked-const-member.cpp
index be04cf26be2e8..bf58e676f3bb0 100644
--- a/clang/test/Analysis/Checkers/WebKit/local-vars-checked-const-member.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/local-vars-checked-const-member.cpp
@@ -21,10 +21,8 @@ class Foo {
CheckedObj& ensureObj4() {
if (!m_obj4)
const_cast<CheckedPtr<CheckedObj>&>(m_obj4) = new CheckedObj;
- if (auto* next = m_obj4->next()) {
- // expected-warning@-1{{Local variable 'next' is unchecked and unsafe [alpha.webkit.UncheckedLocalVarsChecker]}}
+ if (auto* next = m_obj4->next())
return *next;
- }
return *m_obj4;
}
diff --git a/clang/test/Analysis/Checkers/WebKit/local-vars-counted-const-member.cpp b/clang/test/Analysis/Checkers/WebKit/local-vars-counted-const-member.cpp
index e12c9b900a423..5f35535bde3aa 100644
--- a/clang/test/Analysis/Checkers/WebKit/local-vars-counted-const-member.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/local-vars-counted-const-member.cpp
@@ -21,10 +21,8 @@ class Foo {
RefCountable& ensureObj4() {
if (!m_obj4)
const_cast<RefPtr<RefCountable>&>(m_obj4) = RefCountable::create();
- if (auto* next = m_obj4->next()) {
- // expected-warning@-1{{Local variable 'next' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
+ if (auto* next = m_obj4->next())
return *next;
- }
return *m_obj4;
}
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
index 3364637369799..bf093dcf3fdd3 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
@@ -492,4 +492,42 @@ namespace virtual_function {
auto* baz = &*obj;
// expected-warning@-1{{Local variable 'baz' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
}
-}
\ No newline at end of file
+}
+
+namespace vardecl_in_if_condition {
+ RefCountable* provide();
+
+ RefCountable* get() {
+ if (auto* obj = provide())
+ return obj; // no warning
+ return nullptr;
+ }
+
+ RefCountable* get_non_trivial_then() {
+ if (auto* obj = provide()) // expected-warning{{Local variable 'obj' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
+ return obj->next();
+ return nullptr;
+ }
+
+ RefCountable* get_non_trivial_else() {
+ if (auto* obj = provide())
+ return obj;
+ else
+ return provide()->next();
+ return nullptr;
+ }
+
+ RefCountable& get_ref() {
+ if (auto* obj = provide())
+ return *obj; // no warning
+ static Ref<RefCountable> empty = RefCountable::create();
+ return empty.get();
+ }
+
+ RefCountable* get_non_trivial_condition() {
+ if (auto* obj = provide(); obj && obj->next())
+ return obj; // expected-warning@-1{{Local variable 'obj' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
+ return nullptr;
+ }
+
+}
|
🪟 Windows x64 Test Results
✅ The build succeeded and all tests passed. |
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
…h trivial "then" (llvm#171764) Don't emit a warning when a variable declaration appears within the "if" condition and if its "then" statement is trivial. (cherry picked from commit 6f1e3c3)
…h trivial "then" (llvm#171764) Don't emit a warning when a variable declaration appears within the "if" condition and if its "then" statement is trivial.
…h trivial "then" (llvm#171764) Don't emit a warning when a variable declaration appears within the "if" condition and if its "then" statement is trivial. (cherry picked from commit 6f1e3c3)
https://bugs.webkit.org/show_bug.cgi?id=304203 Reviewed by NOBODY (OOPS!). Remove the unnecessary suppressions for a variable declaration within "if" statement now that the corresponding clang static analyzer fix has been merged and deployed: llvm/llvm-project#171764 No new tests since there should be no behavioral changes. * Source/WebCore/accessibility/AXCoreObject.cpp: (WebCore::AXCoreObject::nextSiblingIncludingIgnoredOrParent const): (WebCore::AXCoreObject::parentObjectUnignored const): * Source/WebCore/accessibility/AXObjectCacheInlines.h: (WebCore::AXObjectCache::getOrCreate): * Source/WebCore/accessibility/AccessibilityRenderObject.cpp: (WebCore::AccessibilityRenderObject::parentObject const): * Source/WebCore/dom/EventPath.h: (WebCore::EventPath::eventTargetRespectingTargetRules): * Source/WebCore/editing/TextIterator.cpp: (WebCore::firstNode): * Source/WebCore/loader/archive/ArchiveResourceCollection.cpp: (WebCore::ArchiveResourceCollection::archiveResourceForURL): * Source/WebCore/platform/graphics/Path.cpp: (WebCore::Path::ensureImpl): * Source/WebCore/platform/mac/PlatformScreenMac.mm: (WebCore::screen): * Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm: (WebKit::NetworkSessionCocoa::sessionSetForPage): (WebKit::NetworkSessionCocoa::sessionSetForPage const): * Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp: (WebKit::WebAutomationSessionProxy::getAccessibilityObjectForNode): (WebKit::containerElementForElement):
Don't emit a warning when a variable declaration appears within the "if" condition and if its "then" statement is trivial.