isUncountedPtr should take QualType as an argument. #110213
Merged
+12
−28
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Make isUncountedPtr take QualType as an argument instead of Type*. This simplifies some code.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Ryosuke Niwa (rniwa) ChangesMake isUncountedPtr take QualType as an argument instead of Type*. This simplifies some code. Full diff: https://github.com/llvm/llvm-project/pull/110213.diff 5 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 54c99c3c1b37f9..38582e6d543cd4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -165,14 +165,11 @@ std::optional<bool> isUncounted(const CXXRecordDecl* Class)
return (*IsRefCountable);
}
-std::optional<bool> isUncountedPtr(const Type* T)
+std::optional<bool> isUncountedPtr(const QualType T)
{
- assert(T);
-
if (T->isPointerType() || T->isReferenceType()) {
- if (auto *CXXRD = T->getPointeeCXXRecordDecl()) {
+ if (auto *CXXRD = T->getPointeeCXXRecordDecl())
return isUncounted(CXXRD);
- }
}
return false;
}
@@ -196,12 +193,8 @@ std::optional<bool> isGetterOfRefCounted(const CXXMethodDecl* M)
// Ref<T> -> T conversion
// FIXME: Currently allowing any Ref<T> -> whatever cast.
if (isRefType(className)) {
- if (auto *maybeRefToRawOperator = dyn_cast<CXXConversionDecl>(M)) {
- if (auto *targetConversionType =
- maybeRefToRawOperator->getConversionType().getTypePtrOrNull()) {
- return isUncountedPtr(targetConversionType);
- }
- }
+ if (auto *maybeRefToRawOperator = dyn_cast<CXXConversionDecl>(M))
+ return isUncountedPtr(maybeRefToRawOperator->getConversionType());
}
}
return false;
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index e2d0342bebd52c..4988f604c52283 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -53,7 +53,7 @@ std::optional<bool> isUncounted(const clang::CXXRecordDecl* Class);
/// \returns true if \p T is either a raw pointer or reference to an uncounted
/// class, false if not, std::nullopt if inconclusive.
-std::optional<bool> isUncountedPtr(const clang::Type* T);
+std::optional<bool> isUncountedPtr(const clang::QualType T);
/// \returns true if Name is a RefPtr, Ref, or its variant, false if not.
bool isRefType(const std::string &Name);
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
index 31e9b3c4b9d412..8071b6f70f58dc 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
@@ -103,12 +103,8 @@ class UncountedCallArgsChecker
// continue;
QualType ArgType = (*P)->getType().getCanonicalType();
- const auto *TypePtr = ArgType.getTypePtrOrNull();
- if (!TypePtr)
- continue; // FIXME? Should we bail?
-
// FIXME: more complex types (arrays, references to raw pointers, etc)
- std::optional<bool> IsUncounted = isUncountedPtr(TypePtr);
+ std::optional<bool> IsUncounted = isUncountedPtr(ArgType);
if (!IsUncounted || !(*IsUncounted))
continue;
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
index a226a01ec0a579..998bd4ccee07db 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
@@ -59,11 +59,11 @@ class UncountedLambdaCapturesChecker
for (const LambdaCapture &C : L->captures()) {
if (C.capturesVariable()) {
ValueDecl *CapturedVar = C.getCapturedVar();
- if (auto *CapturedVarType = CapturedVar->getType().getTypePtrOrNull()) {
- std::optional<bool> IsUncountedPtr = isUncountedPtr(CapturedVarType);
- if (IsUncountedPtr && *IsUncountedPtr) {
- reportBug(C, CapturedVar, CapturedVarType);
- }
+ QualType CapturedVarQualType = CapturedVar->getType();
+ if (auto *CapturedVarType = CapturedVarQualType.getTypePtrOrNull()) {
+ auto IsUncountedPtr = isUncountedPtr(CapturedVarQualType);
+ if (IsUncountedPtr && *IsUncountedPtr)
+ reportBug(C, CapturedVar, CapturedVarType);
}
}
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
index 274da0baf2ce5c..3cc4e86b2abb1a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
@@ -190,11 +190,7 @@ class UncountedLocalVarsChecker
if (shouldSkipVarDecl(V))
return;
- const auto *ArgType = V->getType().getTypePtr();
- if (!ArgType)
- return;
-
- std::optional<bool> IsUncountedPtr = isUncountedPtr(ArgType);
+ std::optional<bool> IsUncountedPtr = isUncountedPtr(V->getType());
if (IsUncountedPtr && *IsUncountedPtr) {
if (tryToFindPtrOrigin(
Value, /*StopAtFirstRefCountedObj=*/false,
|
|
@llvm/pr-subscribers-clang-static-analyzer-1 Author: Ryosuke Niwa (rniwa) ChangesMake isUncountedPtr take QualType as an argument instead of Type*. This simplifies some code. Full diff: https://github.com/llvm/llvm-project/pull/110213.diff 5 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 54c99c3c1b37f9..38582e6d543cd4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -165,14 +165,11 @@ std::optional<bool> isUncounted(const CXXRecordDecl* Class)
return (*IsRefCountable);
}
-std::optional<bool> isUncountedPtr(const Type* T)
+std::optional<bool> isUncountedPtr(const QualType T)
{
- assert(T);
-
if (T->isPointerType() || T->isReferenceType()) {
- if (auto *CXXRD = T->getPointeeCXXRecordDecl()) {
+ if (auto *CXXRD = T->getPointeeCXXRecordDecl())
return isUncounted(CXXRD);
- }
}
return false;
}
@@ -196,12 +193,8 @@ std::optional<bool> isGetterOfRefCounted(const CXXMethodDecl* M)
// Ref<T> -> T conversion
// FIXME: Currently allowing any Ref<T> -> whatever cast.
if (isRefType(className)) {
- if (auto *maybeRefToRawOperator = dyn_cast<CXXConversionDecl>(M)) {
- if (auto *targetConversionType =
- maybeRefToRawOperator->getConversionType().getTypePtrOrNull()) {
- return isUncountedPtr(targetConversionType);
- }
- }
+ if (auto *maybeRefToRawOperator = dyn_cast<CXXConversionDecl>(M))
+ return isUncountedPtr(maybeRefToRawOperator->getConversionType());
}
}
return false;
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index e2d0342bebd52c..4988f604c52283 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -53,7 +53,7 @@ std::optional<bool> isUncounted(const clang::CXXRecordDecl* Class);
/// \returns true if \p T is either a raw pointer or reference to an uncounted
/// class, false if not, std::nullopt if inconclusive.
-std::optional<bool> isUncountedPtr(const clang::Type* T);
+std::optional<bool> isUncountedPtr(const clang::QualType T);
/// \returns true if Name is a RefPtr, Ref, or its variant, false if not.
bool isRefType(const std::string &Name);
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
index 31e9b3c4b9d412..8071b6f70f58dc 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
@@ -103,12 +103,8 @@ class UncountedCallArgsChecker
// continue;
QualType ArgType = (*P)->getType().getCanonicalType();
- const auto *TypePtr = ArgType.getTypePtrOrNull();
- if (!TypePtr)
- continue; // FIXME? Should we bail?
-
// FIXME: more complex types (arrays, references to raw pointers, etc)
- std::optional<bool> IsUncounted = isUncountedPtr(TypePtr);
+ std::optional<bool> IsUncounted = isUncountedPtr(ArgType);
if (!IsUncounted || !(*IsUncounted))
continue;
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
index a226a01ec0a579..998bd4ccee07db 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
@@ -59,11 +59,11 @@ class UncountedLambdaCapturesChecker
for (const LambdaCapture &C : L->captures()) {
if (C.capturesVariable()) {
ValueDecl *CapturedVar = C.getCapturedVar();
- if (auto *CapturedVarType = CapturedVar->getType().getTypePtrOrNull()) {
- std::optional<bool> IsUncountedPtr = isUncountedPtr(CapturedVarType);
- if (IsUncountedPtr && *IsUncountedPtr) {
- reportBug(C, CapturedVar, CapturedVarType);
- }
+ QualType CapturedVarQualType = CapturedVar->getType();
+ if (auto *CapturedVarType = CapturedVarQualType.getTypePtrOrNull()) {
+ auto IsUncountedPtr = isUncountedPtr(CapturedVarQualType);
+ if (IsUncountedPtr && *IsUncountedPtr)
+ reportBug(C, CapturedVar, CapturedVarType);
}
}
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
index 274da0baf2ce5c..3cc4e86b2abb1a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
@@ -190,11 +190,7 @@ class UncountedLocalVarsChecker
if (shouldSkipVarDecl(V))
return;
- const auto *ArgType = V->getType().getTypePtr();
- if (!ArgType)
- return;
-
- std::optional<bool> IsUncountedPtr = isUncountedPtr(ArgType);
+ std::optional<bool> IsUncountedPtr = isUncountedPtr(V->getType());
if (IsUncountedPtr && *IsUncountedPtr) {
if (tryToFindPtrOrigin(
Value, /*StopAtFirstRefCountedObj=*/false,
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
haoNoQ
reviewed
Oct 1, 2024
| @@ -190,11 +190,7 @@ class UncountedLocalVarsChecker | |||
| if (shouldSkipVarDecl(V)) | |||
| return; | |||
|
|
|||
| const auto *ArgType = V->getType().getTypePtr(); | |||
| if (!ArgType) | |||
There was a problem hiding this comment.
Some of these null checks may still be necessary (with QualType.isNull()).
There was a problem hiding this comment.
I was able to build the entire WebKit with this PR applied so it seems that we don't need these isNull checks in practice.
haoNoQ
approved these changes
Oct 9, 2024
| @@ -190,11 +190,7 @@ class UncountedLocalVarsChecker | |||
| if (shouldSkipVarDecl(V)) | |||
| return; | |||
|
|
|||
| const auto *ArgType = V->getType().getTypePtr(); | |||
| if (!ArgType) | |||
|
Thanks for reviews! |
ericastor
pushed a commit
to ericastor/llvm-project
that referenced
this pull request
Oct 10, 2024
Make isUncountedPtr take QualType as an argument instead of Type*. This simplifies some code.
DanielCChen
pushed a commit
to DanielCChen/llvm-project
that referenced
this pull request
Oct 16, 2024
Make isUncountedPtr take QualType as an argument instead of Type*. This simplifies some code.
rniwa
added a commit
to rniwa/llvm-project
that referenced
this pull request
Feb 3, 2025
Make isUncountedPtr take QualType as an argument instead of Type*. This simplifies some code.
devincoughlin
pushed a commit
to swiftlang/llvm-project
that referenced
this pull request
Feb 25, 2025
Make isUncountedPtr take QualType as an argument instead of Type*. This simplifies some code.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Make isUncountedPtr take QualType as an argument instead of Type*. This simplifies some code.