Skip to content
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.

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

[Clang] Refactor implementation of "Lifetime extension in range-based for loops" #87930

Merged
merged 1 commit into from
Apr 15, 2024

Conversation

yronglin
Copy link
Contributor

@yronglin yronglin commented Apr 7, 2024

This PR remove InMaterializeTemporaryObjectContext , because it's redundant, materialize non-cv void prvalue temporaries in discarded expressions can only appear under lifetime-extension context.

… for loops"

Signed-off-by: yronglin <yronglin777@gmail.com>
@yronglin yronglin requested a review from Endilll as a code owner April 7, 2024 17:07
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Apr 7, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 7, 2024

@llvm/pr-subscribers-clang

Author: None (yronglin)

Changes

This PR remove InMaterializeTemporaryObjectContext , because it's redundant, materialize non-cv void prvalue temporaries in discarded expressions can only appear under lifetime-extension context.


Full diff: https://github.com/llvm/llvm-project/pull/87930.diff

6 Files Affected:

  • (modified) clang/include/clang/Sema/Sema.h (-47)
  • (modified) clang/lib/Parse/ParseDecl.cpp (-4)
  • (modified) clang/lib/Sema/SemaExpr.cpp (+3-4)
  • (modified) clang/lib/Sema/SemaExprCXX.cpp (+1-1)
  • (modified) clang/lib/Sema/SemaTemplateInstantiateDecl.cpp (-1)
  • (modified) clang/lib/Sema/TreeTransform.h (-5)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 56d66a4486e0e7..ba9624b0085c48 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -5075,34 +5075,6 @@ class Sema final : public SemaBase {
     /// example, in a for-range initializer).
     bool InLifetimeExtendingContext = false;
 
-    /// Whether we are currently in a context in which all temporaries must be
-    /// materialized.
-    ///
-    /// [class.temporary]/p2:
-    /// The materialization of a temporary object is generally delayed as long
-    /// as possible in order to avoid creating unnecessary temporary objects.
-    ///
-    /// Temporary objects are materialized:
-    ///   (2.1) when binding a reference to a prvalue ([dcl.init.ref],
-    ///   [expr.type.conv], [expr.dynamic.cast], [expr.static.cast],
-    ///   [expr.const.cast], [expr.cast]),
-    ///
-    ///   (2.2) when performing member access on a class prvalue ([expr.ref],
-    ///   [expr.mptr.oper]),
-    ///
-    ///   (2.3) when performing an array-to-pointer conversion or subscripting
-    ///   on an array prvalue ([conv.array], [expr.sub]),
-    ///
-    ///   (2.4) when initializing an object of type
-    ///   std​::​initializer_list<T> from a braced-init-list
-    ///   ([dcl.init.list]),
-    ///
-    ///   (2.5) for certain unevaluated operands ([expr.typeid], [expr.sizeof])
-    ///
-    ///   (2.6) when a prvalue that has type other than cv void appears as a
-    ///   discarded-value expression ([expr.context]).
-    bool InMaterializeTemporaryObjectContext = false;
-
     // When evaluating immediate functions in the initializer of a default
     // argument or default member initializer, this is the declaration whose
     // default initializer is being evaluated and the location of the call
@@ -6383,19 +6355,6 @@ class Sema final : public SemaBase {
     }
   }
 
-  /// keepInMaterializeTemporaryObjectContext - Pull down
-  /// InMaterializeTemporaryObjectContext flag from previous context.
-  void keepInMaterializeTemporaryObjectContext() {
-    if (ExprEvalContexts.size() > 2 &&
-        ExprEvalContexts[ExprEvalContexts.size() - 2]
-            .InMaterializeTemporaryObjectContext) {
-      auto &LastRecord = ExprEvalContexts.back();
-      auto &PrevRecord = ExprEvalContexts[ExprEvalContexts.size() - 2];
-      LastRecord.InMaterializeTemporaryObjectContext =
-          PrevRecord.InMaterializeTemporaryObjectContext;
-    }
-  }
-
   DefaultedComparisonKind getDefaultedComparisonKind(const FunctionDecl *FD) {
     return getDefaultedFunctionKind(FD).asComparison();
   }
@@ -6539,12 +6498,6 @@ class Sema final : public SemaBase {
   /// used in initializer of the field.
   llvm::MapVector<FieldDecl *, DeleteLocs> DeleteExprs;
 
-  bool isInMaterializeTemporaryObjectContext() const {
-    assert(!ExprEvalContexts.empty() &&
-           "Must be in an expression evaluation context");
-    return ExprEvalContexts.back().InMaterializeTemporaryObjectContext;
-  }
-
   ParsedType getInheritingConstructorName(CXXScopeSpec &SS,
                                           SourceLocation NameLoc,
                                           IdentifierInfo &Name);
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 0aa14b0510746b..ae5d2cae06e536 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -2379,10 +2379,6 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
       if (getLangOpts().CPlusPlus23) {
         auto &LastRecord = Actions.ExprEvalContexts.back();
         LastRecord.InLifetimeExtendingContext = true;
-
-        // Materialize non-`cv void` prvalue temporaries in discarded
-        // expressions. These materialized temporaries may be lifetime-extented.
-        LastRecord.InMaterializeTemporaryObjectContext = true;
       }
 
       if (getLangOpts().OpenMP)
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 8db4fffeecfe35..3d6c649d8df862 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -6331,7 +6331,6 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
       // Pass down lifetime extending flag, and collect temporaries in
       // CreateMaterializeTemporaryExpr when we rewrite the call argument.
       keepInLifetimeExtendingContext();
-      keepInMaterializeTemporaryObjectContext();
       EnsureImmediateInvocationInDefaultArgs Immediate(*this);
       ExprResult Res;
       runWithSufficientStackSpace(CallLoc, [&] {
@@ -18695,9 +18694,9 @@ void Sema::PopExpressionEvaluationContext() {
   // Append the collected materialized temporaries into previous context before
   // exit if the previous also is a lifetime extending context.
   auto &PrevRecord = ExprEvalContexts[ExprEvalContexts.size() - 2];
-  if (getLangOpts().CPlusPlus23 && isInLifetimeExtendingContext() &&
-      PrevRecord.InLifetimeExtendingContext && !ExprEvalContexts.empty()) {
-    auto &PrevRecord = ExprEvalContexts[ExprEvalContexts.size() - 2];
+  if (getLangOpts().CPlusPlus23 && Rec.InLifetimeExtendingContext &&
+      PrevRecord.InLifetimeExtendingContext &&
+      !Rec.ForRangeLifetimeExtendTemps.empty()) {
     PrevRecord.ForRangeLifetimeExtendTemps.append(
         Rec.ForRangeLifetimeExtendTemps);
   }
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index cfb5c6b6f28337..4c97e72d6a0a1a 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -8360,7 +8360,7 @@ ExprResult Sema::IgnoredValueConversions(Expr *E) {
     // unnecessary temporary objects. If we skip this step, IR generation is
     // able to synthesize the storage for itself in the aggregate case, and
     // adding the extra node to the AST is just clutter.
-    if (isInMaterializeTemporaryObjectContext() && getLangOpts().CPlusPlus17 &&
+    if (isInLifetimeExtendingContext() && getLangOpts().CPlusPlus17 &&
         E->isPRValue() && !E->getType()->isVoidType()) {
       ExprResult Res = TemporaryMaterializationConversion(E);
       if (Res.isInvalid())
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 1cb071e4eb7d1c..15d0b8a69bcd2c 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5477,7 +5477,6 @@ void Sema::InstantiateVariableInitializer(
         *this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated, Var);
 
     keepInLifetimeExtendingContext();
-    keepInMaterializeTemporaryObjectContext();
     // Instantiate the initializer.
     ExprResult Init;
 
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 66c2c7dd6be8f9..7cb76b4c6d21c8 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -4173,7 +4173,6 @@ ExprResult TreeTransform<Derived>::TransformInitializer(Expr *Init,
       getSema(), EnterExpressionEvaluationContext::InitList,
       Construct->isListInitialization());
 
-  getSema().keepInLifetimeExtendingContext();
   getSema().keepInLifetimeExtendingContext();
   SmallVector<Expr*, 8> NewArgs;
   bool ArgChanged = false;
@@ -8748,10 +8747,6 @@ TreeTransform<Derived>::TransformCXXForRangeStmt(CXXForRangeStmt *S) {
   if (getSema().getLangOpts().CPlusPlus23) {
     auto &LastRecord = getSema().ExprEvalContexts.back();
     LastRecord.InLifetimeExtendingContext = true;
-
-    // Materialize non-`cv void` prvalue temporaries in discarded
-    // expressions. These materialized temporaries may be lifetime-extented.
-    LastRecord.InMaterializeTemporaryObjectContext = true;
   }
   StmtResult Init =
       S->getInit() ? getDerived().TransformStmt(S->getInit()) : StmtResult();

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

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

Give @hubert-reinterpretcast a few days to possibly review, otherwise LGTM

@yronglin
Copy link
Contributor Author

yronglin commented Apr 9, 2024

Give @hubert-reinterpretcast a few days to possibly review, otherwise LGTM

Thanks for your review!

@yronglin
Copy link
Contributor Author

friendly ping~ @hubert-reinterpretcast

@cor3ntin
Copy link
Contributor

@yronglin fee free to merge. Thanks!

@yronglin
Copy link
Contributor Author

@yronglin fee free to merge. Thanks!

Thanks for your confirmation!

@yronglin yronglin merged commit 7b9b28d into llvm:main Apr 15, 2024
7 checks passed
aniplcc pushed a commit to aniplcc/llvm-project that referenced this pull request Apr 15, 2024
… for loops" (llvm#87930)

This PR remove `InMaterializeTemporaryObjectContext` , because it's
redundant, materialize non-cv void prvalue temporaries in discarded
expressions can only appear under lifetime-extension context.

Signed-off-by: yronglin <yronglin777@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants