[clang] [Clang] Extend lifetime bound analysis to support assignments (PR #96475)

Haojian Wu via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 1 01:39:42 PDT 2024


https://github.com/hokein updated https://github.com/llvm/llvm-project/pull/96475

>From 97d1b80680112c3fa271501427a18273aed61dbe Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein.wu at gmail.com>
Date: Mon, 24 Jun 2024 10:58:53 +0200
Subject: [PATCH 1/4] [clang] Extend the existing lifetimebound check for
 assignments.

Currently we only detect the builtin pointer type.
---
 clang/include/clang/Basic/DiagnosticGroups.td |  4 +-
 .../clang/Basic/DiagnosticSemaKinds.td        |  5 ++
 clang/lib/Sema/CheckExprLifetime.cpp          | 60 ++++++++++++++-----
 clang/lib/Sema/CheckExprLifetime.h            | 18 +++++-
 clang/lib/Sema/SemaExpr.cpp                   |  4 ++
 clang/lib/Sema/SemaInit.cpp                   |  2 +-
 clang/test/Parser/compound_literal.c          |  5 +-
 clang/test/SemaCXX/attr-lifetimebound.cpp     |  6 ++
 clang/test/SemaCXX/warn-dangling-local.cpp    |  2 +
 9 files changed, 83 insertions(+), 23 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 1c4f305fb5d00..9ae579c2c2a11 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -430,6 +430,7 @@ def LogicalOpParentheses: DiagGroup<"logical-op-parentheses">;
 def LogicalNotParentheses: DiagGroup<"logical-not-parentheses">;
 def ShiftOpParentheses: DiagGroup<"shift-op-parentheses">;
 def OverloadedShiftOpParentheses: DiagGroup<"overloaded-shift-op-parentheses">;
+def DanglingAssignment: DiagGroup<"dangling-assignment">;
 def DanglingElse: DiagGroup<"dangling-else">;
 def DanglingField : DiagGroup<"dangling-field">;
 def DanglingInitializerList : DiagGroup<"dangling-initializer-list">;
@@ -437,7 +438,8 @@ def DanglingGsl : DiagGroup<"dangling-gsl">;
 def ReturnStackAddress : DiagGroup<"return-stack-address">;
 // Name of this warning in GCC
 def : DiagGroup<"return-local-addr", [ReturnStackAddress]>;
-def Dangling : DiagGroup<"dangling", [DanglingField,
+def Dangling : DiagGroup<"dangling", [DanglingAssignment,
+                                      DanglingField,
                                       DanglingInitializerList,
                                       DanglingGsl,
                                       ReturnStackAddress]>;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 33412e14897b6..f613243cb8460 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10120,6 +10120,11 @@ def warn_new_dangling_initializer_list : Warning<
   "the allocated initializer list}0 "
   "will be destroyed at the end of the full-expression">,
   InGroup<DanglingInitializerList>;
+def warn_dangling_pointer_assignment : Warning<
+   "object backing the pointer %0 "
+   "will be destroyed at the end of the full-expression">,
+   InGroup<DanglingAssignment>;
+
 def warn_unsupported_lifetime_extension : Warning<
   "lifetime extension of "
   "%select{temporary|backing array of initializer list}0 created "
diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index 54e2f1c22536d..67fbd5e449d4a 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -8,6 +8,8 @@
 
 #include "CheckExprLifetime.h"
 #include "clang/AST/Expr.h"
+#include "clang/Basic/DiagnosticSema.h"
+#include "clang/Sema/Initialization.h"
 #include "clang/Sema/Sema.h"
 #include "llvm/ADT/PointerIntPair.h"
 
@@ -964,11 +966,26 @@ static bool pathOnlyInitializesGslPointer(IndirectLocalPath &Path) {
   return false;
 }
 
-void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity,
+void checkExprLifetime(Sema &SemaRef, const CheckingEntity &CEntity,
                        Expr *Init) {
-  LifetimeResult LR = getEntityLifetime(&Entity);
-  LifetimeKind LK = LR.getInt();
-  const InitializedEntity *ExtendingEntity = LR.getPointer();
+  LifetimeKind LK = LK_FullExpression;
+
+  const AssignedEntity *AEntity = nullptr;
+  // Local variables for initialized entity.
+  const InitializedEntity *InitEntity = nullptr;
+  const InitializedEntity *ExtendingEntity = nullptr;
+  if (auto IEntityP = std::get_if<const InitializedEntity *>(&CEntity)) {
+    InitEntity = *IEntityP;
+    auto LTResult = getEntityLifetime(InitEntity);
+    LK = LTResult.getInt();
+    ExtendingEntity = LTResult.getPointer();
+  } else if (auto AEntityP = std::get_if<const AssignedEntity *>(&CEntity)) {
+    AEntity = *AEntityP;
+    if (AEntity->LHS->getType()->isPointerType()) // builtin pointer type
+      LK = LK_Extended;
+  } else {
+    llvm_unreachable("unexpected kind");
+  }
 
   // If this entity doesn't have an interesting lifetime, don't bother looking
   // for temporaries within its initializer.
@@ -1028,6 +1045,7 @@ void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity,
 
       switch (shouldLifetimeExtendThroughPath(Path)) {
       case PathLifetimeKind::Extend:
+        assert(InitEntity && "Expect only on initializing the entity");
         // Update the storage duration of the materialized temporary.
         // FIXME: Rebuild the expression instead of mutating it.
         MTE->setExtendingDecl(ExtendingEntity->getDecl(),
@@ -1036,6 +1054,7 @@ void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity,
         return true;
 
       case PathLifetimeKind::ShouldExtend:
+        assert(InitEntity && "Expect only on initializing the entity");
         // We're supposed to lifetime-extend the temporary along this path (per
         // the resolution of DR1815), but we don't support that yet.
         //
@@ -1053,16 +1072,23 @@ void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity,
         if (pathContainsInit(Path))
           return false;
 
-        SemaRef.Diag(DiagLoc, diag::warn_dangling_variable)
-            << RK << !Entity.getParent()
-            << ExtendingEntity->getDecl()->isImplicit()
-            << ExtendingEntity->getDecl() << Init->isGLValue() << DiagRange;
+        if (InitEntity) {
+          SemaRef.Diag(DiagLoc, diag::warn_dangling_variable)
+              << RK << !InitEntity->getParent()
+              << ExtendingEntity->getDecl()->isImplicit()
+              << ExtendingEntity->getDecl() << Init->isGLValue() << DiagRange;
+        } else {
+          SemaRef.Diag(DiagLoc, diag::warn_dangling_pointer_assignment)
+              << AEntity->LHS << DiagRange;
+        }
         break;
       }
       break;
     }
 
     case LK_MemInitializer: {
+      assert(InitEntity && "Expect only on initializing the entity");
+
       if (isa<MaterializeTemporaryExpr>(L)) {
         // Under C++ DR1696, if a mem-initializer (or a default member
         // initializer used by the absence of one) would lifetime-extend a
@@ -1077,7 +1103,7 @@ void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity,
                 << true;
             return false;
           }
-          bool IsSubobjectMember = ExtendingEntity != &Entity;
+          bool IsSubobjectMember = ExtendingEntity != InitEntity;
           SemaRef.Diag(DiagLoc, shouldLifetimeExtendThroughPath(Path) !=
                                         PathLifetimeKind::NoExtend
                                     ? diag::err_dangling_member
@@ -1137,6 +1163,7 @@ void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity,
     }
 
     case LK_New:
+      assert(InitEntity && "Expect only on initializing the entity");
       if (isa<MaterializeTemporaryExpr>(L)) {
         if (IsGslPtrInitWithGslTempOwner)
           SemaRef.Diag(DiagLoc, diag::warn_dangling_lifetime_pointer)
@@ -1145,7 +1172,7 @@ void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity,
           SemaRef.Diag(DiagLoc, RK == RK_ReferenceBinding
                                     ? diag::warn_new_dangling_reference
                                     : diag::warn_new_dangling_initializer_list)
-              << !Entity.getParent() << DiagRange;
+              << !InitEntity->getParent() << DiagRange;
       } else {
         // We can't determine if the allocation outlives the local declaration.
         return false;
@@ -1154,13 +1181,14 @@ void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity,
 
     case LK_Return:
     case LK_StmtExprResult:
+      assert(InitEntity && "Expect only on initializing the entity");
       if (auto *DRE = dyn_cast<DeclRefExpr>(L)) {
         // We can't determine if the local variable outlives the statement
         // expression.
         if (LK == LK_StmtExprResult)
           return false;
         SemaRef.Diag(DiagLoc, diag::warn_ret_stack_addr_ref)
-            << Entity.getType()->isReferenceType() << DRE->getDecl()
+            << InitEntity->getType()->isReferenceType() << DRE->getDecl()
             << isa<ParmVarDecl>(DRE->getDecl()) << DiagRange;
       } else if (isa<BlockExpr>(L)) {
         SemaRef.Diag(DiagLoc, diag::err_ret_local_block) << DiagRange;
@@ -1172,8 +1200,8 @@ void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity,
         SemaRef.Diag(DiagLoc, diag::warn_ret_addr_label) << DiagRange;
       } else if (auto *CLE = dyn_cast<CompoundLiteralExpr>(L)) {
         SemaRef.Diag(DiagLoc, diag::warn_ret_stack_addr_ref)
-            << Entity.getType()->isReferenceType() << CLE->getInitializer() << 2
-            << DiagRange;
+            << InitEntity->getType()->isReferenceType() << CLE->getInitializer()
+            << 2 << DiagRange;
       } else {
         // P2748R5: Disallow Binding a Returned Glvalue to a Temporary.
         // [stmt.return]/p6: In a function whose return type is a reference,
@@ -1181,12 +1209,12 @@ void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity,
         // a return statement that binds the returned reference to a temporary
         // expression ([class.temporary]) is ill-formed.
         if (SemaRef.getLangOpts().CPlusPlus26 &&
-            Entity.getType()->isReferenceType())
+            InitEntity->getType()->isReferenceType())
           SemaRef.Diag(DiagLoc, diag::err_ret_local_temp_ref)
-              << Entity.getType()->isReferenceType() << DiagRange;
+              << InitEntity->getType()->isReferenceType() << DiagRange;
         else
           SemaRef.Diag(DiagLoc, diag::warn_ret_local_temp_addr_ref)
-              << Entity.getType()->isReferenceType() << DiagRange;
+              << InitEntity->getType()->isReferenceType() << DiagRange;
       }
       break;
     }
diff --git a/clang/lib/Sema/CheckExprLifetime.h b/clang/lib/Sema/CheckExprLifetime.h
index f9cd3f55d3dbf..0882d41b35b6c 100644
--- a/clang/lib/Sema/CheckExprLifetime.h
+++ b/clang/lib/Sema/CheckExprLifetime.h
@@ -15,13 +15,25 @@
 #include "clang/AST/Expr.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Sema.h"
+#include <variant>
 
 namespace clang::sema {
 
+/// Describes an entity that is being assigned.
+struct AssignedEntity {
+  // The left-hand side expression of the assignment.
+  Expr *LHS = nullptr;
+};
+
+using CheckingEntity =
+    std::variant<const InitializedEntity *, const AssignedEntity *>;
+
 /// Check that the lifetime of the given expr (and its subobjects) is
-/// sufficient for initializing the entity, and perform lifetime extension
-/// (when permitted) if not.
-void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity,
+/// sufficient for initializing or assigning to the entity.
+///
+/// If the entity is being initialized and its lifetime is insufficient, perform
+/// lifetime extension (when permitted).
+void checkExprLifetime(Sema &SemaRef, const CheckingEntity &CEntity,
                        Expr *Init);
 
 } // namespace clang::sema
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index db44cfe1288b6..5c8b0682dcacc 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -10,6 +10,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "CheckExprLifetime.h"
 #include "TreeTransform.h"
 #include "UsedDeclVisitor.h"
 #include "clang/AST/ASTConsumer.h"
@@ -13778,6 +13779,9 @@ QualType Sema::CheckAssignmentOperands(Expr *LHSExpr, ExprResult &RHS,
 
   CheckForNullPointerDereference(*this, LHSExpr);
 
+  AssignedEntity AE{LHSExpr};
+  checkExprLifetime(*this, &AE, RHS.get());
+
   if (getLangOpts().CPlusPlus20 && LHSType.isVolatileQualified()) {
     if (CompoundType.isNull()) {
       // C++2a [expr.ass]p5:
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 26c65b34f4cc4..db2bacbad8a61 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -7287,7 +7287,7 @@ PerformConstructorInitialization(Sema &S,
 
 void Sema::checkInitializerLifetime(const InitializedEntity &Entity,
                                     Expr *Init) {
-  return sema::checkExprLifetime(*this, Entity, Init);
+  return sema::checkExprLifetime(*this, &Entity, Init);
 }
 
 static void DiagnoseNarrowingInInitList(Sema &S,
diff --git a/clang/test/Parser/compound_literal.c b/clang/test/Parser/compound_literal.c
index 808ca63f97d9e..281a6d7ada430 100644
--- a/clang/test/Parser/compound_literal.c
+++ b/clang/test/Parser/compound_literal.c
@@ -1,8 +1,9 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -verify -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -verify -x c++ -Wno-dangling-assignment %s
 // expected-no-diagnostics
 int main(void) {
   char *s;
-  s = (char []){"whatever"};
+  // In C++ mode, the cast creates a "char [4]" array temporary here.
+  s = (char []){"whatever"};  // dangling!
   s = (char(*)){s};
 }
diff --git a/clang/test/SemaCXX/attr-lifetimebound.cpp b/clang/test/SemaCXX/attr-lifetimebound.cpp
index 5fcda6f23dab7..7c572ecef52ba 100644
--- a/clang/test/SemaCXX/attr-lifetimebound.cpp
+++ b/clang/test/SemaCXX/attr-lifetimebound.cpp
@@ -40,6 +40,12 @@ namespace usage_ok {
   int *p = A().class_member(); // expected-warning {{temporary whose address is used as value of local variable 'p' will be destroyed at the end of the full-expression}}
   int *q = A(); // expected-warning {{temporary whose address is used as value of local variable 'q' will be destroyed at the end of the full-expression}}
   int *r = A(1); // expected-warning {{temporary whose address is used as value of local variable 'r' will be destroyed at the end of the full-expression}}
+
+  void test_assignment() {
+    p = A().class_member(); // expected-warning {{object backing the pointer p will be destroyed at the end of the full-expression}}
+    q = A(); // expected-warning {{object backing the pointer q will be destroyed at the end of the full-expression}}
+    r = A(1); // expected-warning {{object backing the pointer r will be destroyed at the end of the full-expression}}
+  }
 }
 
 # 1 "<std>" 1 3
diff --git a/clang/test/SemaCXX/warn-dangling-local.cpp b/clang/test/SemaCXX/warn-dangling-local.cpp
index 5c1d56972945c..a946b8a241a38 100644
--- a/clang/test/SemaCXX/warn-dangling-local.cpp
+++ b/clang/test/SemaCXX/warn-dangling-local.cpp
@@ -4,8 +4,10 @@ using T = int[];
 
 void f() {
   int *p = &(int&)(int&&)0; // expected-warning {{temporary whose address is used as value of local variable 'p' will be destroyed at the end of the full-expression}}
+  p =  &(int&)(int&&)0; // expected-warning {{object backing the pointer p will be destroyed at the end of the full-expression}}
 
   int *q = (int *const &)T{1, 2, 3}; // expected-warning {{temporary whose address is used as value of local variable 'q' will be destroyed at the end of the full-expression}}
+  q = (int *const &)T{1, 2, 3}; // expected-warning {{object backing the pointer q will be destroyed at the end of the full-expression}}
 
   // FIXME: We don't warn here because the 'int*' temporary is not const, but
   // it also can't have actually changed since it was created, so we could

>From 78485d8f62eb36ecf7176f37ad19c43e2eea6a09 Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein.wu at gmail.com>
Date: Fri, 28 Jun 2024 16:25:23 +0200
Subject: [PATCH 2/4] Address review comments.

---
 clang/lib/Sema/CheckExprLifetime.cpp      | 10 +++++++---
 clang/test/SemaCXX/attr-lifetimebound.cpp |  1 +
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index 67fbd5e449d4a..e50fa4facdff6 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -992,6 +992,8 @@ void checkExprLifetime(Sema &SemaRef, const CheckingEntity &CEntity,
   if (LK == LK_FullExpression)
     return;
 
+  // FIXME: consider moving the TemporaryVisitor and visitLocalsRetained*
+  // functions to a dedicated class.
   auto TemporaryVisitor = [&](IndirectLocalPath &Path, Local L,
                               ReferenceKind RK) -> bool {
     SourceRange DiagRange = nextPathEntryRange(Path, 0, L);
@@ -1089,7 +1091,7 @@ void checkExprLifetime(Sema &SemaRef, const CheckingEntity &CEntity,
     case LK_MemInitializer: {
       assert(InitEntity && "Expect only on initializing the entity");
 
-      if (isa<MaterializeTemporaryExpr>(L)) {
+      if (MTE) {
         // Under C++ DR1696, if a mem-initializer (or a default member
         // initializer used by the absence of one) would lifetime-extend a
         // temporary, the program is ill-formed.
@@ -1280,8 +1282,10 @@ void checkExprLifetime(Sema &SemaRef, const CheckingEntity &CEntity,
                                           TemporaryVisitor,
                                           EnableLifetimeWarnings);
   else
-    visitLocalsRetainedByInitializer(Path, Init, TemporaryVisitor, false,
-                                     EnableLifetimeWarnings);
+    visitLocalsRetainedByInitializer(
+        Path, Init, TemporaryVisitor,
+        // Don't revisit the sub inits for the intialization case.
+        /*RevisitSubinits=*/!InitEntity, EnableLifetimeWarnings);
 }
 
 } // namespace clang::sema
diff --git a/clang/test/SemaCXX/attr-lifetimebound.cpp b/clang/test/SemaCXX/attr-lifetimebound.cpp
index 7c572ecef52ba..70bc545c07bd9 100644
--- a/clang/test/SemaCXX/attr-lifetimebound.cpp
+++ b/clang/test/SemaCXX/attr-lifetimebound.cpp
@@ -43,6 +43,7 @@ namespace usage_ok {
 
   void test_assignment() {
     p = A().class_member(); // expected-warning {{object backing the pointer p will be destroyed at the end of the full-expression}}
+    p = {A().class_member()}; // expected-warning {{object backing the pointer p will be destroyed at the end of the full-expression}}
     q = A(); // expected-warning {{object backing the pointer q will be destroyed at the end of the full-expression}}
     r = A(1); // expected-warning {{object backing the pointer r will be destroyed at the end of the full-expression}}
   }

>From 1e5a113707ac6354677a394bf6798f76b77ba7f8 Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein.wu at gmail.com>
Date: Fri, 28 Jun 2024 16:37:22 +0200
Subject: [PATCH 3/4] Release notes

---
 clang/docs/ReleaseNotes.rst          | 4 ++++
 clang/lib/Sema/CheckExprLifetime.cpp | 6 ++++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index df579ae398c5e..03968d60d9f77 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -626,6 +626,10 @@ Improvements to Clang's diagnostics
   used rather than when they are needed for constant evaluation or when code is generated for them.
   The check is now stricter to prevent crashes for some unsupported declarations (Fixes #GH95495).
 
+- Clang now diagnoses dangling cases where a pointer is assigned to a temporary
+  that will be destroyed at the end of the full expression.
+  Fixes #GH54492.
+
 Improvements to Clang's time-trace
 ----------------------------------
 
diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index e50fa4facdff6..f321019710737 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -1047,7 +1047,8 @@ void checkExprLifetime(Sema &SemaRef, const CheckingEntity &CEntity,
 
       switch (shouldLifetimeExtendThroughPath(Path)) {
       case PathLifetimeKind::Extend:
-        assert(InitEntity && "Expect only on initializing the entity");
+        assert(InitEntity && "Lifetime extension should happen only for "
+                             "initialization and not assignment");
         // Update the storage duration of the materialized temporary.
         // FIXME: Rebuild the expression instead of mutating it.
         MTE->setExtendingDecl(ExtendingEntity->getDecl(),
@@ -1056,7 +1057,8 @@ void checkExprLifetime(Sema &SemaRef, const CheckingEntity &CEntity,
         return true;
 
       case PathLifetimeKind::ShouldExtend:
-        assert(InitEntity && "Expect only on initializing the entity");
+        assert(InitEntity && "Lifetime extension should happen only for "
+                             "initialization and not assignment");
         // We're supposed to lifetime-extend the temporary along this path (per
         // the resolution of DR1815), but we don't support that yet.
         //

>From 8f599bcc57d7f37f7d8391b96cdc5cf1b87984ee Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein.wu at gmail.com>
Date: Mon, 1 Jul 2024 10:20:39 +0200
Subject: [PATCH 4/4] Tweak the APIs

---
 clang/lib/Sema/CheckExprLifetime.cpp | 28 +++++++++++++++++++---------
 clang/lib/Sema/CheckExprLifetime.h   | 16 +++++++---------
 clang/lib/Sema/SemaExpr.cpp          |  2 +-
 clang/lib/Sema/SemaInit.cpp          |  2 +-
 4 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index f321019710737..be77949e8b547 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -966,27 +966,27 @@ static bool pathOnlyInitializesGslPointer(IndirectLocalPath &Path) {
   return false;
 }
 
-void checkExprLifetime(Sema &SemaRef, const CheckingEntity &CEntity,
-                       Expr *Init) {
+static void checkExprLifetimeImpl(
+    Sema &SemaRef,
+    llvm::PointerUnion<const InitializedEntity *, const AssignedEntity *>
+        CEntity,
+    Expr *Init) {
   LifetimeKind LK = LK_FullExpression;
 
   const AssignedEntity *AEntity = nullptr;
   // Local variables for initialized entity.
   const InitializedEntity *InitEntity = nullptr;
   const InitializedEntity *ExtendingEntity = nullptr;
-  if (auto IEntityP = std::get_if<const InitializedEntity *>(&CEntity)) {
-    InitEntity = *IEntityP;
+  if (CEntity.is<const InitializedEntity *>()) {
+    InitEntity = CEntity.get<const InitializedEntity *>();
     auto LTResult = getEntityLifetime(InitEntity);
     LK = LTResult.getInt();
     ExtendingEntity = LTResult.getPointer();
-  } else if (auto AEntityP = std::get_if<const AssignedEntity *>(&CEntity)) {
-    AEntity = *AEntityP;
+  } else {
+    AEntity = CEntity.get<const AssignedEntity *>();
     if (AEntity->LHS->getType()->isPointerType()) // builtin pointer type
       LK = LK_Extended;
-  } else {
-    llvm_unreachable("unexpected kind");
   }
-
   // If this entity doesn't have an interesting lifetime, don't bother looking
   // for temporaries within its initializer.
   if (LK == LK_FullExpression)
@@ -1290,4 +1290,14 @@ void checkExprLifetime(Sema &SemaRef, const CheckingEntity &CEntity,
         /*RevisitSubinits=*/!InitEntity, EnableLifetimeWarnings);
 }
 
+void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity,
+                       Expr *Init) {
+  checkExprLifetimeImpl(SemaRef, &Entity, Init);
+}
+
+void checkExprLifetime(Sema &SemaRef, const AssignedEntity &Entity,
+                       Expr *Init) {
+  checkExprLifetimeImpl(SemaRef, &Entity, Init);
+}
+
 } // namespace clang::sema
diff --git a/clang/lib/Sema/CheckExprLifetime.h b/clang/lib/Sema/CheckExprLifetime.h
index 0882d41b35b6c..af381fb96c4d6 100644
--- a/clang/lib/Sema/CheckExprLifetime.h
+++ b/clang/lib/Sema/CheckExprLifetime.h
@@ -15,7 +15,6 @@
 #include "clang/AST/Expr.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Sema.h"
-#include <variant>
 
 namespace clang::sema {
 
@@ -25,17 +24,16 @@ struct AssignedEntity {
   Expr *LHS = nullptr;
 };
 
-using CheckingEntity =
-    std::variant<const InitializedEntity *, const AssignedEntity *>;
-
 /// Check that the lifetime of the given expr (and its subobjects) is
-/// sufficient for initializing or assigning to the entity.
-///
-/// If the entity is being initialized and its lifetime is insufficient, perform
-/// lifetime extension (when permitted).
-void checkExprLifetime(Sema &SemaRef, const CheckingEntity &CEntity,
+/// sufficient for initializing the entity, and perform lifetime extension
+/// (when permitted) if not.
+void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity,
                        Expr *Init);
 
+/// Check that the lifetime of the given expr (and its subobjects) is
+/// sufficient for assigning to the entity.
+void checkExprLifetime(Sema &SemaRef, const AssignedEntity &Entity, Expr *Init);
+
 } // namespace clang::sema
 
 #endif // LLVM_CLANG_SEMA_CHECK_EXPR_LIFETIME_H
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 5c8b0682dcacc..94c0efe7ae011 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -13780,7 +13780,7 @@ QualType Sema::CheckAssignmentOperands(Expr *LHSExpr, ExprResult &RHS,
   CheckForNullPointerDereference(*this, LHSExpr);
 
   AssignedEntity AE{LHSExpr};
-  checkExprLifetime(*this, &AE, RHS.get());
+  checkExprLifetime(*this, AE, RHS.get());
 
   if (getLangOpts().CPlusPlus20 && LHSType.isVolatileQualified()) {
     if (CompoundType.isNull()) {
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index db2bacbad8a61..26c65b34f4cc4 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -7287,7 +7287,7 @@ PerformConstructorInitialization(Sema &S,
 
 void Sema::checkInitializerLifetime(const InitializedEntity &Entity,
                                     Expr *Init) {
-  return sema::checkExprLifetime(*this, &Entity, Init);
+  return sema::checkExprLifetime(*this, Entity, Init);
 }
 
 static void DiagnoseNarrowingInInitList(Sema &S,



More information about the cfe-commits mailing list