llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Ryosuke Niwa (rniwa) <details> <summary>Changes</summary> Refactor RawPtrRefMemberChecker so that each subclass override isUnsafePtr like other WebKit checkers instead of overriding isPtrCompatible. --- Full diff: https://github.com/llvm/llvm-project/pull/137559.diff 1 Files Affected: - (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp (+45-53) ``````````diff diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp index 10b9749319a57..209073e3ccc3a 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp @@ -38,9 +38,7 @@ class RawPtrRefMemberChecker RawPtrRefMemberChecker(const char *description) : Bug(this, description, "WebKit coding guidelines") {} - virtual std::optional<bool> - isPtrCompatible(const clang::QualType, - const clang::CXXRecordDecl *R) const = 0; + virtual std::optional<bool> isUnsafePtr(QualType) const = 0; virtual const char *typeName() const = 0; virtual const char *invariant() const = 0; @@ -93,22 +91,30 @@ class RawPtrRefMemberChecker if (!MemberType) continue; - if (auto *MemberCXXRD = MemberType->getPointeeCXXRecordDecl()) { - std::optional<bool> IsCompatible = isPtrCompatible(QT, MemberCXXRD); - if (IsCompatible && *IsCompatible) - reportBug(Member, MemberType, MemberCXXRD, RD); - } else { - std::optional<bool> IsCompatible = isPtrCompatible(QT, nullptr); - auto *PointeeType = MemberType->getPointeeType().getTypePtrOrNull(); - if (IsCompatible && *IsCompatible) { - auto *Desugared = PointeeType->getUnqualifiedDesugaredType(); - if (auto *ObjCType = dyn_cast_or_null<ObjCInterfaceType>(Desugared)) - reportBug(Member, MemberType, ObjCType->getDecl(), RD); - } - } + auto IsUnsafePtr = isUnsafePtr(QT); + if (!IsUnsafePtr || !*IsUnsafePtr) + continue; + + if (auto *MemberCXXRD = MemberType->getPointeeCXXRecordDecl()) + reportBug(Member, MemberType, MemberCXXRD, RD); + else if (auto* ObjCDecl = getObjCDecl(MemberType)) + reportBug(Member, MemberType, ObjCDecl, RD); } } + ObjCInterfaceDecl* getObjCDecl(const Type *TypePtr) const { + auto *PointeeType = TypePtr->getPointeeType().getTypePtrOrNull(); + if (!PointeeType) + return nullptr; + auto *Desugared = PointeeType->getUnqualifiedDesugaredType(); + if (!Desugared) + return nullptr; + auto *ObjCType = dyn_cast<ObjCInterfaceType>(Desugared); + if (!ObjCType) + return nullptr; + return ObjCType->getDecl(); + } + void visitObjCDecl(const ObjCContainerDecl *CD) const { if (BR->getSourceManager().isInSystemHeader(CD->getLocation())) return; @@ -138,19 +144,15 @@ class RawPtrRefMemberChecker const Type *IvarType = QT.getTypePtrOrNull(); if (!IvarType) return; - if (auto *IvarCXXRD = IvarType->getPointeeCXXRecordDecl()) { - std::optional<bool> IsCompatible = isPtrCompatible(QT, IvarCXXRD); - if (IsCompatible && *IsCompatible) - reportBug(Ivar, IvarType, IvarCXXRD, CD); - } else { - std::optional<bool> IsCompatible = isPtrCompatible(QT, nullptr); - auto *PointeeType = IvarType->getPointeeType().getTypePtrOrNull(); - if (IsCompatible && *IsCompatible) { - auto *Desugared = PointeeType->getUnqualifiedDesugaredType(); - if (auto *ObjCType = dyn_cast_or_null<ObjCInterfaceType>(Desugared)) - reportBug(Ivar, IvarType, ObjCType->getDecl(), CD); - } - } + + auto IsUnsafePtr = isUnsafePtr(QT); + if (!IsUnsafePtr || !*IsUnsafePtr) + return; + + if (auto *MemberCXXRD = IvarType->getPointeeCXXRecordDecl()) + reportBug(Ivar, IvarType, MemberCXXRD, CD); + else if (auto* ObjCDecl = getObjCDecl(IvarType)) + reportBug(Ivar, IvarType, ObjCDecl, CD); } void visitObjCPropertyDecl(const ObjCContainerDecl *CD, @@ -161,19 +163,15 @@ class RawPtrRefMemberChecker const Type *PropType = QT.getTypePtrOrNull(); if (!PropType) return; - if (auto *PropCXXRD = PropType->getPointeeCXXRecordDecl()) { - std::optional<bool> IsCompatible = isPtrCompatible(QT, PropCXXRD); - if (IsCompatible && *IsCompatible) - reportBug(PD, PropType, PropCXXRD, CD); - } else { - std::optional<bool> IsCompatible = isPtrCompatible(QT, nullptr); - auto *PointeeType = PropType->getPointeeType().getTypePtrOrNull(); - if (IsCompatible && *IsCompatible) { - auto *Desugared = PointeeType->getUnqualifiedDesugaredType(); - if (auto *ObjCType = dyn_cast_or_null<ObjCInterfaceType>(Desugared)) - reportBug(PD, PropType, ObjCType->getDecl(), CD); - } - } + + auto IsUnsafePtr = isUnsafePtr(QT); + if (!IsUnsafePtr || !*IsUnsafePtr) + return; + + if (auto *MemberCXXRD = PropType->getPointeeCXXRecordDecl()) + reportBug(PD, PropType, MemberCXXRD, CD); + else if (auto* ObjCDecl = getObjCDecl(PropType)) + reportBug(PD, PropType, ObjCDecl, CD); } bool shouldSkipDecl(const RecordDecl *RD) const { @@ -263,10 +261,8 @@ class NoUncountedMemberChecker final : public RawPtrRefMemberChecker { : RawPtrRefMemberChecker("Member variable is a raw-pointer/reference to " "reference-countable type") {} - std::optional<bool> - isPtrCompatible(const clang::QualType, - const clang::CXXRecordDecl *R) const final { - return R ? isRefCountable(R) : std::nullopt; + std::optional<bool> isUnsafePtr(QualType QT) const final { + return isUncountedPtr(QT.getCanonicalType()); } const char *typeName() const final { return "ref-countable type"; } @@ -282,10 +278,8 @@ class NoUncheckedPtrMemberChecker final : public RawPtrRefMemberChecker { : RawPtrRefMemberChecker("Member variable is a raw-pointer/reference to " "checked-pointer capable type") {} - std::optional<bool> - isPtrCompatible(const clang::QualType, - const clang::CXXRecordDecl *R) const final { - return R ? isCheckedPtrCapable(R) : std::nullopt; + std::optional<bool> isUnsafePtr(QualType QT) const final { + return isUncheckedPtr(QT.getCanonicalType()); } const char *typeName() const final { return "CheckedPtr capable type"; } @@ -304,9 +298,7 @@ class NoUnretainedMemberChecker final : public RawPtrRefMemberChecker { RTC = RetainTypeChecker(); } - std::optional<bool> - isPtrCompatible(const clang::QualType QT, - const clang::CXXRecordDecl *) const final { + std::optional<bool> isUnsafePtr(QualType QT) const final { return RTC->isUnretained(QT); } `````````` </details> https://github.com/llvm/llvm-project/pull/137559 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
