[PATCH] D76438: ConstantExpr cached APValues if present for constant evaluation

[Wyatt Childers via Phabricator via cfe-commits]

wchilders updated this revision to Diff 251732.
wchilders added a comment.

Updated to assume LValue `ConstantExpr`s have LValue `APValue`s, with verification in debug mode. Additionally, added a missing LValue -> RValue conversion for `VerifyIntegerConstantExpression`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76438/new/

https://reviews.llvm.org/D76438

Files:
  clang/include/clang/AST/Expr.h
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/SemaExpr.cpp


Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -15167,6 +15167,12 @@
     return ExprError();
   }
 
+  ExprResult RValueExpr = DefaultLvalueConversion(E);
+  if (RValueExpr.isInvalid())
+    return ExprError();
+
+  E = RValueExpr.get();
+
   // Circumvent ICE checking in C++11 to avoid evaluating the expression twice
   // in the non-ICE case.
   if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
Index: clang/lib/AST/ExprConstant.cpp
===================================================================
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -6751,8 +6751,13 @@
     return Error(E);
   }
 
-  bool VisitConstantExpr(const ConstantExpr *E)
-    { return StmtVisitorTy::Visit(E->getSubExpr()); }
+  bool VisitConstantExpr(const ConstantExpr *E) {
+    if (E->hasAPValueResult())
+      return DerivedSuccess(E->getAPValueResult(), E);
+
+    return StmtVisitorTy::Visit(E->getSubExpr());
+  }
+
   bool VisitParenExpr(const ParenExpr *E)
     { return StmtVisitorTy::Visit(E->getSubExpr()); }
   bool VisitUnaryExtension(const UnaryOperator *E)
@@ -7317,6 +7322,13 @@
     return true;
   }
 
+  // Override to perform additional checks to ensure the cached APValue
+  // is actually an LValue.
+  bool VisitConstantExpr(const ConstantExpr *E) {
+    assert(!E->hasAPValueResult() || E->getAPValueResult().isLValue());
+    return ExprEvaluatorBaseTy::VisitConstantExpr(E);
+  }
+
   bool VisitMemberExpr(const MemberExpr *E) {
     // Handle non-static data members.
     QualType BaseTy;
Index: clang/lib/AST/Expr.cpp
===================================================================
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -356,6 +356,8 @@
 }
 
 APValue ConstantExpr::getAPValueResult() const {
+  assert(hasAPValueResult());
+
   switch (ConstantExprBits.ResultKind) {
   case ConstantExpr::RSK_APValue:
     return APValueResult();
@@ -2870,9 +2872,6 @@
       return CE->getChosenSubExpr();
   }
 
-  else if (auto *CE = dyn_cast<ConstantExpr>(E))
-    return CE->getSubExpr();
-
   return E;
 }
 
Index: clang/include/clang/AST/Expr.h
===================================================================
--- clang/include/clang/AST/Expr.h
+++ clang/include/clang/AST/Expr.h
@@ -1063,6 +1063,9 @@
   bool isImmediateInvocation() const {
     return ConstantExprBits.IsImmediateInvocation;
   }
+  bool hasAPValueResult() const {
+    return ConstantExprBits.APValueKind != APValue::None;
+  }
   APValue getAPValueResult() const;
   APValue &getResultAsAPValue() const { return APValueResult(); }
   llvm::APSInt getResultAsAPSInt() const;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D76438.251732.patch
Type: text/x-patch
Size: 2745 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200320/389bd8ae/attachment-0001.bin>