Remove ValueDependent assertions on ICE checks.

[mattd]

Summary

In the case of ill-formed templates, a value dependent name can reach the integral constant expression (ICE) check. After an error occurs following an ill-formed template, an ICE check can be performed on a template definition. That definition might contain an expression, such as "const a = sizeof(T);" where T is a template type argument. In this case, the latter expression is ValueDependent, T is type dependent, and the sizeof expression is therefore considered Value Dependent.

The assertions will trigger, if the implementation of a templated member function occurs in a separate namespace from its definition (bad code). If such a case occurs, an ODR marking of a variable decl is performed, which happens to check that a particular variable is constant.

Additionally, ValueDependent items can be ICEs, but we never check that case, and the assertions assume we never see them in the first place. In general, I don't like removing assertions, for obvious reasons. But in this case, I am not sure they are necessary, and can be problematic as seen by the included test case.

[mattd]

ping

[mattd]

Abandoning as this is no longer a problem with release builds.

lib/AST/Decl.cpp

APValue *VarDecl::evaluateValue(
 
   // We only produce notes indicating why an initializer is non-constant the
   // first time it is evaluated. FIXME: The notes won't always be emitted the
   // first time we try evaluation, so might not be produced at all.
   if (Eval->WasEvaluated)
     return Eval->Evaluated.isUninit() ? nullptr : &Eval->Evaluated;
 
   const auto *Init = cast<Expr>(Eval->Value);
-  assert(!Init->isValueDependent());
 
   if (Eval->IsEvaluating) {
     // FIXME: Produce a diagnostic for self-initialization.
     Eval->CheckedICE = true;
     Eval->IsICE = false;
     return nullptr;
   }
 
bool VarDecl::checkInitIsICE() const {
 
   EvaluatedStmt *Eval = ensureEvaluatedStmt();
   if (Eval->CheckedICE)
     // We have already checked whether this subexpression is an
     // integral constant expression.
     return Eval->IsICE;
 
   const auto *Init = cast<Expr>(Eval->Value);
-  assert(!Init->isValueDependent());
 
   // In C++11, evaluate the initializer to check whether it's a constant
   // expression.
   if (getASTContext().getLangOpts().CPlusPlus11) {
     SmallVector<PartialDiagnosticAt, 8> Notes;
     evaluateValue(Notes);
     return Eval->IsICE;
   }

lib/AST/ExprConstant.cpp

static ICEDiag CheckEvalInICE(const Expr* E, const ASTContext &Ctx) {
   if (!E->EvaluateAsRValue(EVResult, Ctx) || EVResult.HasSideEffects ||
       !EVResult.Val.isInt())
     return ICEDiag(IK_NotICE, E->getLocStart());
 
   return NoDiag();
 }
 
 static ICEDiag CheckICE(const Expr* E, const ASTContext &Ctx) {
-  assert(!E->isValueDependent() && "Should not see value dependent exprs!");
   if (!E->getType()->isIntegralOrEnumerationType())
     return ICEDiag(IK_NotICE, E->getLocStart());
 
   switch (E->getStmtClass()) {
 #define ABSTRACT_STMT(Node)
 #define STMT(Node, Base) case Expr::Node##Class:
 #define EXPR(Node, Base)
 #include "clang/AST/StmtNodes.inc"

test/SemaTemplate/illformed-template-ice.cpp

+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+class A {
+public:
+  template <typename T> void B();
+};
+
+namespace {
+  template <typename T> void A::B() { // expected-error {{cannot define or redeclare 'B' here because namespace '' does not enclose namespace 'A'}}
+    const int a = sizeof(T);
+    int  b = a;
+  }
+}