From 4f97786e1606f185c0652dddfa702450f80bd799 Mon Sep 17 00:00:00 2001
From: Eli Friedman <eli.friedman@gmail.com>
Date: Tue, 17 Jan 2012 01:17:46 +0000
Subject: [PATCH] Change the behavior of the lvalue-to-rvalue conversion for
 varargs in PotentiallyPotentiallyEvaluated contexts so that we model it in a
 sane way in most cases, and give up for the edge case which hopefully doesn't
 matter too much.

In preparation for correctly treating sizeof() as a PotentiallyPotentiallyEvaluated context.

llvm-svn: 148271
---
 .../clang/Basic/DiagnosticSemaKinds.td        |  4 ++
 clang/lib/Sema/SemaExpr.cpp                   | 39 +++++++++++++------
 clang/test/SemaCXX/vararg-non-pod.cpp         |  3 +-
 3 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 049074d6e5b1..f8e58267ccd9 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4318,6 +4318,10 @@ def err_cannot_pass_objc_interface_to_vararg : Error<
   "cannot pass object with interface type %0 by-value through variadic "
   "%select{function|block|method}1">;
 
+def err_cannot_pass_non_pod_to_vararg_ppe : Error<
+  "passing object of type %0 to variadic call in a context "
+  "which is potentially evaluatable, but not obviously "
+  "potentially evaluatable, is not yet implemented">;
 def warn_cannot_pass_non_pod_arg_to_vararg : Warning<
   "cannot pass object of %select{non-POD|non-trivial}0 type %1 through variadic"
   " %select{function|block|method|constructor}2; call will abort at runtime">,
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 1655c9105fc0..33102ccc04ec 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -486,17 +486,34 @@ ExprResult Sema::DefaultArgumentPromotion(Expr *E) {
   //     has a class type, the conversion copy-initializes a temporary
   //     of type T from the glvalue and the result of the conversion
   //     is a prvalue for the temporary.
-  // FIXME: add some way to gate this entire thing for correctness in
-  // potentially potentially evaluated contexts.
-  if (getLangOptions().CPlusPlus && E->isGLValue() && 
-      ExprEvalContexts.back().Context != Unevaluated) {
-    ExprResult Temp = PerformCopyInitialization(
-                       InitializedEntity::InitializeTemporary(E->getType()),
-                                                E->getExprLoc(),
-                                                Owned(E));
-    if (Temp.isInvalid())
-      return ExprError();
-    E = Temp.get();
+  // This requirement has some strange effects for
+  // PotentiallyPotentiallyEvaluated contexts; specifically, doing precisely
+  // what the standard requires involves mutating the AST once we decide
+  // whether an expression is potentially evaluated. Rather than actually try and
+  // model this correctly, we just make sure to handle the important cases:
+  // for types with a trivial copy constructor/destructor, we build the AST
+  // as if it were potentially evaluated, and we give an error in other cases
+  // if the context turns out to be potentially evaluatable.
+  // FIXME: If anyone actually cares about this case, try to implement
+  // it correctly, or at least improve the diagnostic output a bit.
+  if (getLangOptions().CPlusPlus && E->isGLValue()) {
+    if (ExprEvalContexts.back().Context == PotentiallyPotentiallyEvaluated &&
+        E->getType()->isRecordType() &&
+        (!E->getType().isTriviallyCopyableType(Context) ||
+         E->getType().isDestructedType())) {
+      ExprEvalContexts.back()
+          .addDiagnostic(E->getExprLoc(),
+              PDiag(diag::err_cannot_pass_non_pod_to_vararg_ppe)
+                  << E->getType());
+    } else if (ExprEvalContexts.back().Context != Unevaluated) {
+      ExprResult Temp = PerformCopyInitialization(
+                         InitializedEntity::InitializeTemporary(E->getType()),
+                                                  E->getExprLoc(),
+                                                  Owned(E));
+      if (Temp.isInvalid())
+        return ExprError();
+      E = Temp.get();
+    }
   }
 
   return Owned(E);
diff --git a/clang/test/SemaCXX/vararg-non-pod.cpp b/clang/test/SemaCXX/vararg-non-pod.cpp
index 42c27fb30e1b..aaf6cc1191c7 100644
--- a/clang/test/SemaCXX/vararg-non-pod.cpp
+++ b/clang/test/SemaCXX/vararg-non-pod.cpp
@@ -88,7 +88,8 @@ Base &get_base(...);
 int eat_base(...);
 
 void test_typeid(Base &base) {
-  (void)typeid(get_base(base)); // expected-warning{{cannot pass object of non-POD type 'Base' through variadic function; call will abort at runtime}}
+  (void)typeid(get_base(base)); // expected-warning{{cannot pass object of non-POD type 'Base' through variadic function; call will abort at runtime}} \
+                                // expected-error {{potentially evaluatable, but not obviously potentially evaluatable}}
   (void)typeid(eat_base(base)); // okay
 }