xazax.hun created this revision.
xazax.hun added reviewers: gribozavr, rsmith, mgehre.
xazax.hun added a project: clang.
Herald added subscribers: cfe-commits, Charusso, gamesh411, Szelethus, dkrupp,
rnkovacs.
xazax.hun added a parent revision: D65120: More warnings regarding gsl::Pointer
and gsl::Owner attributes.
This patch introduces even more warnings some of them are very compelling. Look
at the loop example, it supposed to catch a lot of errors early.
This patch has not been tested on real world code yet, I plan to update this
revision with the findings.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D65127
Files:
clang/lib/Sema/SemaInit.cpp
clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
Index: clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
===================================================================
--- clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -116,13 +116,21 @@
namespace std {
template <typename T>
-struct basic_iterator {};
+struct basic_iterator {
+ basic_iterator operator++();
+ T& operator*();
+};
+
+template<typename T>
+bool operator!=(basic_iterator<T>, basic_iterator<T>);
template <typename T>
struct vector {
typedef basic_iterator<T> iterator;
iterator begin();
+ iterator end();
T *data();
+ T &at(int n);
};
template<typename T>
@@ -134,6 +142,13 @@
struct unique_ptr {
T *get() const;
};
+
+template<typename T>
+struct optional {
+ optional();
+ optional(const T&);
+ T &operator*();
+};
}
void modelIterators() {
@@ -162,4 +177,20 @@
int *danglingUniquePtrFromTemp2() {
return std::unique_ptr<int>().get(); // expected-warning {{returning address of local temporary object}}
+}
+
+void danglingReferenceFromTempOwner() {
+ int &r = *std::optional<int>(); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+ int &r2 = *std::optional<int>(5); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+ int &r3 = std::vector<int>().at(3); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+}
+
+std::vector<int> getTempVec();
+std::optional<std::vector<int>> getTempOptVec();
+
+void testLoops() {
+ for (auto i : getTempVec()) // ok
+ ;
+ for (auto i : *getTempOptVec()) // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+ ;
}
\ No newline at end of file
Index: clang/lib/Sema/SemaInit.cpp
===================================================================
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -6560,21 +6560,34 @@
return false;
}
-static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee,
- const CXXMemberCallExpr *MCE) {
+static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) {
if (auto *Conv = dyn_cast_or_null<CXXConversionDecl>(Callee))
if (isRecordWithAttr<PointerAttr>(Conv->getConversionType()))
return true;
- if (!Callee->getParent()->isInStdNamespace() || !Callee->getIdentifier())
+ if (!Callee->getParent()->isInStdNamespace())
return false;
- if (!isRecordWithAttr<PointerAttr>(Callee->getReturnType()) &&
- !Callee->getReturnType()->isPointerType())
- return false;
- return llvm::StringSwitch<bool>(Callee->getName())
- .Cases("begin", "rbegin", "cbegin", "crbegin", true)
- .Cases("end", "rend", "cend", "crend", true)
- .Cases("c_str", "data", "get", true)
- .Default(false);
+ if (Callee->getReturnType()->isPointerType() ||
+ isRecordWithAttr<PointerAttr>(Callee->getReturnType())) {
+ if (!Callee->getIdentifier())
+ return false;
+ return llvm::StringSwitch<bool>(Callee->getName())
+ .Cases("begin", "rbegin", "cbegin", "crbegin", true)
+ .Cases("end", "rend", "cend", "crend", true)
+ .Cases("c_str", "data", "get", true)
+ // Map and set types.
+ .Cases("find", "equal_range", "lower_bound", "upper_bound", true)
+ .Default(false);
+ } else if (Callee->getReturnType()->isReferenceType()) {
+ if (!Callee->getIdentifier()) {
+ auto OO = Callee->getOverloadedOperator();
+ return OO == OverloadedOperatorKind::OO_Subscript ||
+ OO == OverloadedOperatorKind::OO_Star;
+ }
+ return llvm::StringSwitch<bool>(Callee->getName())
+ .Cases("front", "back", "at", true)
+ .Default(false);
+ }
+ return false;
}
static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr *Call,
@@ -6605,15 +6618,21 @@
if (auto *MCE = dyn_cast<CXXMemberCallExpr>(Call)) {
const auto *MD = cast<CXXMethodDecl>(Callee);
- if (shouldTrackImplicitObjectArg(MD, MCE))
+ if (shouldTrackImplicitObjectArg(MD))
VisitPointerArg(MD, MCE->getImplicitObjectArgument());
return;
+ } else if (auto *OCE = dyn_cast<CXXOperatorCallExpr>(Call)) {
+ if (Callee->isCXXInstanceMember() &&
+ shouldTrackImplicitObjectArg(cast<CXXMethodDecl>(Callee)))
+ VisitPointerArg(Callee, OCE->getArg(0));
+ return;
}
if (auto *CCE = dyn_cast<CXXConstructExpr>(Call)) {
const auto *Ctor = CCE->getConstructor();
const CXXRecordDecl *RD = Ctor->getParent()->getCanonicalDecl();
- if (RD->hasAttr<OwnerAttr>() && isa<CXXTemporaryObjectExpr>(Call)) {
+ if (RD->hasAttr<OwnerAttr>() && isa<CXXConstructExpr>(Call) &&
+ !Ctor->isCopyOrMoveConstructor()) {
Path.push_back({IndirectLocalPathEntry::GslOwnerTemporaryInit, Call, RD});
Visit(Path, Call, RK_ReferenceBinding);
Path.pop_back();
@@ -7117,11 +7136,11 @@
llvm_unreachable("already handled this");
case LK_Extended: {
- auto *MTE = dyn_cast<MaterializeTemporaryExpr>(L);
if (IsGslPtrInitWithGslTempOwner) {
Diag(DiagLoc, diag::warn_dangling_lifetime_pointer) << DiagRange;
return false;
}
+ auto *MTE = dyn_cast<MaterializeTemporaryExpr>(L);
if (!MTE) {
// The initialized entity has lifetime beyond the full-expression,
// and the local entity does too, so don't warn.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
