Skip to content

[Sema] Fix lifetime extension for temporaries in range-based for loops in C++23 #145164

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+61 −0
Open

[Sema] Fix lifetime extension for temporaries in range-based for loops in C++23 #145164

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
37c5713
[Sema] Fix lifetime extension for temporaries in range-based for loop…
mrcvtl Jun 21, 2025
d9a88d5
Address feedbacks
mrcvtl Jun 28, 2025
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
4 changes: 4 additions & 0 deletions clang/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,10 @@ Improvements to Clang's diagnostics
#GH142457, #GH139913, #GH138850, #GH137867, #GH137860, #GH107840, #GH93308,
#GH69470, #GH59391, #GH58172, #GH46215, #GH45915, #GH45891, #GH44490,
#GH36703, #GH32903, #GH23312, #GH69874.

- Clang no longer emits a spurious -Wdangling-gsl warning in C++23 when
iterating over an element of a temporary container in a range-based
for loop.(#GH109793, #GH145164)


Improvements to Clang's time-trace
Expand Down
18 changes: 18 additions & 0 deletions clang/include/clang/AST/Decl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1085,6 +1085,11 @@ class VarDecl : public DeclaratorDecl, public Redeclarable<VarDecl> {

LLVM_PREFERRED_TYPE(bool)
unsigned IsCXXCondDecl : 1;

/// Whether this variable is the implicit __range variable in a for-range
/// loop.
LLVM_PREFERRED_TYPE(bool)
unsigned IsCXXForRangeImplicitVar : 1;
};

union {
Expand Down Expand Up @@ -1584,6 +1589,19 @@ class VarDecl : public DeclaratorDecl, public Redeclarable<VarDecl> {
NonParmVarDeclBits.IsCXXCondDecl = true;
}

/// Whether this variable is the implicit '__range' variable in C++
/// range-based for loops.
bool isCXXForRangeImplicitVar() const {
return isa<ParmVarDecl>(this) ? false
: NonParmVarDeclBits.IsCXXForRangeImplicitVar;
}

void setCXXForRangeImplicitVar(bool FRV) {
assert(!isa<ParmVarDecl>(this) &&
"Cannot set IsCXXForRangeImplicitVar on ParmVarDecl");
NonParmVarDeclBits.IsCXXForRangeImplicitVar = FRV;
}

/// Determines if this variable's alignment is dependent.
bool hasDependentAlignment() const;

Expand Down
8 changes: 8 additions & 0 deletions clang/lib/Sema/CheckExprLifetime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1341,6 +1341,14 @@ checkExprLifetimeImpl(Sema &SemaRef, const InitializedEntity *InitEntity,
}

if (IsGslPtrValueFromGslTempOwner && DiagLoc.isValid()) {

if (SemaRef.getLangOpts().CPlusPlus23) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the analysis is done here, and we filter out the result. I think we can filter out this case before (in checkExprLifetimeImpl) running the analysis to save some cost.

Can you add a testcase (for the lifetimebound attr)? Make sure we don't warn on it

using size_t = decltype(sizeof(void *));

namespace my_ns {
template <typename T> struct vector {
  T &operator[](size_t I) [[clang::lifetimebound]];
};

struct string {
  const char *begin();
  const char *end();
};

} // namespace std

my_ns::vector<my_ns::string> getData();

void foo() {

  for (auto c : getData()[0]) {
    (void)c;
  }
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using [[clang::lifetimebound]] currently produces a warning, thanks for pointing it out!
Moving the check up to checkExprLifetimeImpl, as you suggested, also results in no warning for the test case you proposed.

However, after this change, running all Sema tests causes SemaCXX/attr-lifetimebound.cpp to fail, precisely because the warning is no longer emitted in the following case:

std::vector make_vector();
void use_reversed_range() {
  // FIXME: Don't expose the name of the internal range variable.
  for (auto x : reversed(make_vector())) {} // expected-warning {{temporary implicitly bound to local reference will be destroyed at the end of the full-expression}}
}

As this is seems exactly the test case you suggested, make sense to update this test instead of adding a new one?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only update this test seems make sense.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, just update the existing test. The test file attr-lifetimebound.cpp is compiled with -std=c++23, we should not diagnose this test.

if (const VarDecl *VD =
dyn_cast_if_present<VarDecl>(InitEntity->getDecl());
VD && VD->isCXXForRangeImplicitVar())
return false;
}

SemaRef.Diag(DiagLoc, diag::warn_dangling_lifetime_pointer)
<< DiagRange;
return false;
Expand Down
1 change: 1 addition & 0 deletions clang/lib/Sema/SemaStmt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2423,6 +2423,7 @@ VarDecl *BuildForRangeVarDecl(Sema &SemaRef, SourceLocation Loc,
VarDecl *Decl = VarDecl::Create(SemaRef.Context, DC, Loc, Loc, II, Type,
TInfo, SC_None);
Decl->setImplicit();
Decl->setCXXForRangeImplicitVar(true);
return Decl;
}

Expand Down
1 change: 1 addition & 0 deletions clang/lib/Serialization/ASTReaderDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1637,6 +1637,7 @@ RedeclarableResult ASTDeclReader::VisitVarDeclImpl(VarDecl *VD) {
VarDeclBits.getNextBits(/*Width*/ 3);

VD->NonParmVarDeclBits.ObjCForDecl = VarDeclBits.getNextBit();
VD->NonParmVarDeclBits.IsCXXForRangeImplicitVar = VarDeclBits.getNextBit();
}

// If this variable has a deduced type, defer reading that type until we are
Expand Down
2 changes: 2 additions & 0 deletions clang/lib/Serialization/ASTWriterDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1319,6 +1319,7 @@ void ASTDeclWriter::VisitVarDecl(VarDecl *D) {
VarDeclBits.addBits(0, /*Width=*/3);

VarDeclBits.addBit(D->isObjCForDecl());
VarDeclBits.addBit(D->isCXXForRangeImplicitVar());
}

Record.push_back(VarDeclBits);
Expand Down Expand Up @@ -2740,6 +2741,7 @@ void ASTWriter::WriteDeclAbbrevs() {
// isInline, isInlineSpecified, isConstexpr,
// isInitCapture, isPrevDeclInSameScope, hasInitWithSideEffects,
// EscapingByref, HasDeducedType, ImplicitParamKind, isObjCForDecl
// IsCXXForRangeImplicitVar
Abv->Add(BitCodeAbbrevOp(0)); // VarKind (local enum)
// Type Source Info
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));
Expand Down
27 changes: 27 additions & 0 deletions clang/test/SemaCXX/range-for-lifetime-cxx23.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// RUN: %clang_cc1 -std=c++23 -fsyntax-only -verify %s

using size_t = decltype(sizeof(void *));

namespace std {
template <typename T> struct vector {
T &operator[](size_t I);
};

struct string {
const char *begin();
const char *end();
};

} // namespace std

std::vector<std::string> getData();

void foo() {
// Verifies we don't trigger a diagnostic from -Wdangling-gsl
// when iterating over a temporary in C++23.
for (auto c : getData()[0]) {
(void)c;
}
}

// expected-no-diagnostics