[cfe-commits] r152761 - /cfe/trunk/lib/AST/ExprConstant.cpp

Daniel Dunbar daniel at zuster.org
Wed Mar 14 19:57:50 PDT 2012


Hey Richard,

This broke bootstrap, at least on Darwin.

addDiag() is getting called when EvalStatus.Diag is null, which == boom.

Can you investigate?

 - Daniel

On Wed, Mar 14, 2012 at 5:41 PM, Richard Smith
<richard-llvm at metafoo.co.uk> wrote:
> Author: rsmith
> Date: Wed Mar 14 19:41:48 2012
> New Revision: 152761
>
> URL: http://llvm.org/viewvc/llvm-project?rev=152761&view=rev
> Log:
> Minor optimization to constant evaluation: don't bother computing expr source
> locations for diagnostics we're not going to emit, and don't track the subobject
> designator outside C++11 (since we're not going to use it anyway).
>
> This seems to give about a 0.5% speedup on 403.gcc/combine.c, but the results
> were sufficiently noisy that I can't reject the null hypothesis.
>
> Modified:
>    cfe/trunk/lib/AST/ExprConstant.cpp
>
> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=152761&r1=152760&r2=152761&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
> +++ cfe/trunk/lib/AST/ExprConstant.cpp Wed Mar 14 19:41:48 2012
> @@ -480,9 +480,18 @@
>       return OptionalDiagnostic();
>     }
>
> +    OptionalDiagnostic Diag(const Expr *E, diag::kind DiagId
> +                              = diag::note_invalid_subexpr_in_const_expr,
> +                            unsigned ExtraNotes = 0) {
> +      if (EvalStatus.Diag)
> +        return Diag(E->getExprLoc(), DiagId, ExtraNotes);
> +      return OptionalDiagnostic();
> +    }
> +
>     /// Diagnose that the evaluation does not produce a C++11 core constant
>     /// expression.
> -    OptionalDiagnostic CCEDiag(SourceLocation Loc, diag::kind DiagId
> +    template<typename LocArg>
> +    OptionalDiagnostic CCEDiag(LocArg Loc, diag::kind DiagId
>                                  = diag::note_invalid_subexpr_in_const_expr,
>                                unsigned ExtraNotes = 0) {
>       // Don't override a previous diagnostic.
> @@ -556,7 +565,7 @@
>   if (Invalid)
>     return false;
>   if (isOnePastTheEnd()) {
> -    Info.CCEDiag(E->getExprLoc(), diag::note_constexpr_past_end_subobject)
> +    Info.CCEDiag(E, diag::note_constexpr_past_end_subobject)
>       << CSK;
>     setInvalid();
>     return false;
> @@ -567,11 +576,11 @@
>  void SubobjectDesignator::diagnosePointerArithmetic(EvalInfo &Info,
>                                                     const Expr *E, uint64_t N) {
>   if (MostDerivedPathLength == Entries.size() && MostDerivedArraySize)
> -    Info.CCEDiag(E->getExprLoc(), diag::note_constexpr_array_index)
> +    Info.CCEDiag(E, diag::note_constexpr_array_index)
>       << static_cast<int>(N) << /*array*/ 0
>       << static_cast<unsigned>(MostDerivedArraySize);
>   else
> -    Info.CCEDiag(E->getExprLoc(), diag::note_constexpr_array_index)
> +    Info.CCEDiag(E, diag::note_constexpr_array_index)
>       << static_cast<int>(N) << /*non-array*/ 1;
>   setInvalid();
>  }
> @@ -730,7 +739,7 @@
>       if (Designator.Invalid)
>         return false;
>       if (!Base) {
> -        Info.CCEDiag(E->getExprLoc(), diag::note_constexpr_null_subobject)
> +        Info.CCEDiag(E, diag::note_constexpr_null_subobject)
>           << CSK;
>         Designator.setInvalid();
>         return false;
> @@ -741,27 +750,30 @@
>     // Check this LValue refers to an object. If not, set the designator to be
>     // invalid and emit a diagnostic.
>     bool checkSubobject(EvalInfo &Info, const Expr *E, CheckSubobjectKind CSK) {
> +      // Outside C++11, do not build a designator referring to a subobject of
> +      // any object: we won't use such a designator for anything.
> +      if (!Info.getLangOpts().CPlusPlus0x)
> +        Designator.setInvalid();
>       return checkNullPointer(Info, E, CSK) &&
>              Designator.checkSubobject(Info, E, CSK);
>     }
>
>     void addDecl(EvalInfo &Info, const Expr *E,
>                  const Decl *D, bool Virtual = false) {
> -      checkSubobject(Info, E, isa<FieldDecl>(D) ? CSK_Field : CSK_Base);
> -      Designator.addDeclUnchecked(D, Virtual);
> +      if (checkSubobject(Info, E, isa<FieldDecl>(D) ? CSK_Field : CSK_Base))
> +        Designator.addDeclUnchecked(D, Virtual);
>     }
>     void addArray(EvalInfo &Info, const Expr *E, const ConstantArrayType *CAT) {
> -      checkSubobject(Info, E, CSK_ArrayToPointer);
> -      Designator.addArrayUnchecked(CAT);
> +      if (checkSubobject(Info, E, CSK_ArrayToPointer))
> +        Designator.addArrayUnchecked(CAT);
>     }
>     void addComplex(EvalInfo &Info, const Expr *E, QualType EltTy, bool Imag) {
> -      checkSubobject(Info, E, Imag ? CSK_Imag : CSK_Real);
> -      Designator.addComplexUnchecked(EltTy, Imag);
> +      if (checkSubobject(Info, E, Imag ? CSK_Imag : CSK_Real))
> +        Designator.addComplexUnchecked(EltTy, Imag);
>     }
>     void adjustIndex(EvalInfo &Info, const Expr *E, uint64_t N) {
> -      if (!checkNullPointer(Info, E, CSK_ArrayIndex))
> -        return;
> -      Designator.adjustIndex(Info, E, N);
> +      if (checkNullPointer(Info, E, CSK_ArrayIndex))
> +        Designator.adjustIndex(Info, E, N);
>     }
>   };
>
> @@ -1020,10 +1032,10 @@
>
>   // Prvalue constant expressions must be of literal types.
>   if (Info.getLangOpts().CPlusPlus0x)
> -    Info.Diag(E->getExprLoc(), diag::note_constexpr_nonliteral)
> +    Info.Diag(E, diag::note_constexpr_nonliteral)
>       << E->getType();
>   else
> -    Info.Diag(E->getExprLoc(), diag::note_invalid_subexpr_in_const_expr);
> +    Info.Diag(E, diag::note_invalid_subexpr_in_const_expr);
>   return false;
>  }
>
> @@ -1155,7 +1167,7 @@
>  template<typename T>
>  static bool HandleOverflow(EvalInfo &Info, const Expr *E,
>                            const T &SrcValue, QualType DestType) {
> -  Info.Diag(E->getExprLoc(), diag::note_constexpr_overflow)
> +  Info.Diag(E, diag::note_constexpr_overflow)
>     << SrcValue << DestType;
>   return false;
>  }
> @@ -1240,7 +1252,7 @@
>       } else {
>         // Don't try to handle vectors of anything other than int or float
>         // (not sure if it's possible to hit this case).
> -        Info.Diag(E->getExprLoc(), diag::note_invalid_subexpr_in_const_expr);
> +        Info.Diag(E, diag::note_invalid_subexpr_in_const_expr);
>         return false;
>       }
>       unsigned BaseEltSize = EltAsInt.getBitWidth();
> @@ -1253,7 +1265,7 @@
>   }
>   // Give up if the input isn't an int, float, or vector.  For example, we
>   // reject "(v4i16)(intptr_t)&a".
> -  Info.Diag(E->getExprLoc(), diag::note_invalid_subexpr_in_const_expr);
> +  Info.Diag(E, diag::note_invalid_subexpr_in_const_expr);
>   return false;
>  }
>
> @@ -1414,7 +1426,7 @@
>     if (Info.CheckingPotentialConstantExpression)
>       return false;
>     if (!Frame || !Frame->Arguments) {
> -      Info.Diag(E->getExprLoc(), diag::note_invalid_subexpr_in_const_expr);
> +      Info.Diag(E, diag::note_invalid_subexpr_in_const_expr);
>       return false;
>     }
>     Result = Frame->Arguments[PVD->getFunctionScopeIndex()];
> @@ -1427,7 +1439,7 @@
>     // If we're checking a potential constant expression, the variable could be
>     // initialized later.
>     if (!Info.CheckingPotentialConstantExpression)
> -      Info.Diag(E->getExprLoc(), diag::note_invalid_subexpr_in_const_expr);
> +      Info.Diag(E, diag::note_invalid_subexpr_in_const_expr);
>     return false;
>   }
>
> @@ -1441,7 +1453,7 @@
>   // Never evaluate the initializer of a weak variable. We can't be sure that
>   // this is the definition which will be used.
>   if (VD->isWeak()) {
> -    Info.Diag(E->getExprLoc(), diag::note_invalid_subexpr_in_const_expr);
> +    Info.Diag(E, diag::note_invalid_subexpr_in_const_expr);
>     return false;
>   }
>
> @@ -1449,13 +1461,13 @@
>   // this in the cases where it matters for conformance.
>   llvm::SmallVector<PartialDiagnosticAt, 8> Notes;
>   if (!VD->evaluateValue(Notes)) {
> -    Info.Diag(E->getExprLoc(), diag::note_constexpr_var_init_non_constant,
> +    Info.Diag(E, diag::note_constexpr_var_init_non_constant,
>               Notes.size() + 1) << VD;
>     Info.Note(VD->getLocation(), diag::note_declared_at);
>     Info.addNotes(Notes);
>     return false;
>   } else if (!VD->checkInitIsICE()) {
> -    Info.CCEDiag(E->getExprLoc(), diag::note_constexpr_var_init_non_constant,
> +    Info.CCEDiag(E, diag::note_constexpr_var_init_non_constant,
>                  Notes.size() + 1) << VD;
>     Info.Note(VD->getLocation(), diag::note_declared_at);
>     Info.addNotes(Notes);
> @@ -1507,7 +1519,7 @@
>     // A diagnostic will have already been produced.
>     return false;
>   if (Sub.isOnePastTheEnd()) {
> -    Info.Diag(E->getExprLoc(), Info.getLangOpts().CPlusPlus0x ?
> +    Info.Diag(E, Info.getLangOpts().CPlusPlus0x ?
>                 (unsigned)diag::note_constexpr_read_past_end :
>                 (unsigned)diag::note_invalid_subexpr_in_const_expr);
>     return false;
> @@ -1529,7 +1541,7 @@
>       if (CAT->getSize().ule(Index)) {
>         // Note, it should not be possible to form a pointer with a valid
>         // designator which points more than one past the end of the array.
> -        Info.Diag(E->getExprLoc(), Info.getLangOpts().CPlusPlus0x ?
> +        Info.Diag(E, Info.getLangOpts().CPlusPlus0x ?
>                     (unsigned)diag::note_constexpr_read_past_end :
>                     (unsigned)diag::note_invalid_subexpr_in_const_expr);
>         return false;
> @@ -1551,7 +1563,7 @@
>       // Next subobject is a complex number.
>       uint64_t Index = Sub.Entries[I].ArrayIndex;
>       if (Index > 1) {
> -        Info.Diag(E->getExprLoc(), Info.getLangOpts().CPlusPlus0x ?
> +        Info.Diag(E, Info.getLangOpts().CPlusPlus0x ?
>                     (unsigned)diag::note_constexpr_read_past_end :
>                     (unsigned)diag::note_invalid_subexpr_in_const_expr);
>         return false;
> @@ -1568,7 +1580,7 @@
>       return true;
>     } else if (const FieldDecl *Field = getAsField(Sub.Entries[I])) {
>       if (Field->isMutable()) {
> -        Info.Diag(E->getExprLoc(), diag::note_constexpr_ltor_mutable, 1)
> +        Info.Diag(E, diag::note_constexpr_ltor_mutable, 1)
>           << Field;
>         Info.Note(Field->getLocation(), diag::note_declared_at);
>         return false;
> @@ -1580,8 +1592,7 @@
>         const FieldDecl *UnionField = O->getUnionField();
>         if (!UnionField ||
>             UnionField->getCanonicalDecl() != Field->getCanonicalDecl()) {
> -          Info.Diag(E->getExprLoc(),
> -                    diag::note_constexpr_read_inactive_union_member)
> +          Info.Diag(E, diag::note_constexpr_read_inactive_union_member)
>             << Field << !UnionField << UnionField;
>           return false;
>         }
> @@ -1593,11 +1604,11 @@
>       if (ObjType.isVolatileQualified()) {
>         if (Info.getLangOpts().CPlusPlus) {
>           // FIXME: Include a description of the path to the volatile subobject.
> -          Info.Diag(E->getExprLoc(), diag::note_constexpr_ltor_volatile_obj, 1)
> +          Info.Diag(E, diag::note_constexpr_ltor_volatile_obj, 1)
>             << 2 << Field;
>           Info.Note(Field->getLocation(), diag::note_declared_at);
>         } else {
> -          Info.Diag(E->getExprLoc(), diag::note_invalid_subexpr_in_const_expr);
> +          Info.Diag(E, diag::note_invalid_subexpr_in_const_expr);
>         }
>         return false;
>       }
> @@ -1611,7 +1622,7 @@
>
>     if (O->isUninit()) {
>       if (!Info.CheckingPotentialConstantExpression)
> -        Info.Diag(E->getExprLoc(), diag::note_constexpr_read_uninit);
> +        Info.Diag(E, diag::note_constexpr_read_uninit);
>       return false;
>     }
>   }
> @@ -1702,11 +1713,10 @@
>     return false;
>
>   const Expr *Base = LVal.Base.dyn_cast<const Expr*>();
> -  SourceLocation Loc = Conv->getExprLoc();
>
>   if (!LVal.Base) {
>     // FIXME: Indirection through a null pointer deserves a specific diagnostic.
> -    Info.Diag(Loc, diag::note_invalid_subexpr_in_const_expr);
> +    Info.Diag(Conv, diag::note_invalid_subexpr_in_const_expr);
>     return false;
>   }
>
> @@ -1714,7 +1724,7 @@
>   if (LVal.CallIndex) {
>     Frame = Info.getCallFrame(LVal.CallIndex);
>     if (!Frame) {
> -      Info.Diag(Loc, diag::note_constexpr_lifetime_ended, 1) << !Base;
> +      Info.Diag(Conv, diag::note_constexpr_lifetime_ended, 1) << !Base;
>       NoteLValueLocation(Info, LVal.Base);
>       return false;
>     }
> @@ -1726,9 +1736,9 @@
>   // semantics.
>   if (Type.isVolatileQualified()) {
>     if (Info.getLangOpts().CPlusPlus)
> -      Info.Diag(Loc, diag::note_constexpr_ltor_volatile_type) << Type;
> +      Info.Diag(Conv, diag::note_constexpr_ltor_volatile_type) << Type;
>     else
> -      Info.Diag(Loc);
> +      Info.Diag(Conv);
>     return false;
>   }
>
> @@ -1742,7 +1752,7 @@
>     if (const VarDecl *VDef = VD->getDefinition(Info.Ctx))
>       VD = VDef;
>     if (!VD || VD->isInvalidDecl()) {
> -      Info.Diag(Loc);
> +      Info.Diag(Conv);
>       return false;
>     }
>
> @@ -1751,10 +1761,10 @@
>     QualType VT = VD->getType();
>     if (VT.isVolatileQualified()) {
>       if (Info.getLangOpts().CPlusPlus) {
> -        Info.Diag(Loc, diag::note_constexpr_ltor_volatile_obj, 1) << 1 << VD;
> +        Info.Diag(Conv, diag::note_constexpr_ltor_volatile_obj, 1) << 1 << VD;
>         Info.Note(VD->getLocation(), diag::note_declared_at);
>       } else {
> -        Info.Diag(Loc);
> +        Info.Diag(Conv);
>       }
>       return false;
>     }
> @@ -1765,10 +1775,10 @@
>       } else if (VT->isIntegralOrEnumerationType()) {
>         if (!VT.isConstQualified()) {
>           if (Info.getLangOpts().CPlusPlus) {
> -            Info.Diag(Loc, diag::note_constexpr_ltor_non_const_int, 1) << VD;
> +            Info.Diag(Conv, diag::note_constexpr_ltor_non_const_int, 1) << VD;
>             Info.Note(VD->getLocation(), diag::note_declared_at);
>           } else {
> -            Info.Diag(Loc);
> +            Info.Diag(Conv);
>           }
>           return false;
>         }
> @@ -1777,18 +1787,18 @@
>         // static const data members of such types (supported as an extension)
>         // more useful.
>         if (Info.getLangOpts().CPlusPlus0x) {
> -          Info.CCEDiag(Loc, diag::note_constexpr_ltor_non_constexpr, 1) << VD;
> +          Info.CCEDiag(Conv, diag::note_constexpr_ltor_non_constexpr, 1) << VD;
>           Info.Note(VD->getLocation(), diag::note_declared_at);
>         } else {
> -          Info.CCEDiag(Loc);
> +          Info.CCEDiag(Conv);
>         }
>       } else {
>         // FIXME: Allow folding of values of any literal type in all languages.
>         if (Info.getLangOpts().CPlusPlus0x) {
> -          Info.Diag(Loc, diag::note_constexpr_ltor_non_constexpr, 1) << VD;
> +          Info.Diag(Conv, diag::note_constexpr_ltor_non_constexpr, 1) << VD;
>           Info.Note(VD->getLocation(), diag::note_declared_at);
>         } else {
> -          Info.Diag(Loc);
> +          Info.Diag(Conv);
>         }
>         return false;
>       }
> @@ -1812,7 +1822,7 @@
>     if (unsigned CallIndex = RVal.getLValueCallIndex()) {
>       Frame = Info.getCallFrame(CallIndex);
>       if (!Frame) {
> -        Info.Diag(Loc, diag::note_constexpr_lifetime_ended, 1) << !Base;
> +        Info.Diag(Conv, diag::note_constexpr_lifetime_ended, 1) << !Base;
>         NoteLValueLocation(Info, RVal.getLValueBase());
>         return false;
>       }
> @@ -1824,10 +1834,10 @@
>   // Volatile temporary objects cannot be read in constant expressions.
>   if (Base->getType().isVolatileQualified()) {
>     if (Info.getLangOpts().CPlusPlus) {
> -      Info.Diag(Loc, diag::note_constexpr_ltor_volatile_obj, 1) << 0;
> +      Info.Diag(Conv, diag::note_constexpr_ltor_volatile_obj, 1) << 0;
>       Info.Note(Base->getExprLoc(), diag::note_constexpr_temporary_here);
>     } else {
> -      Info.Diag(Loc);
> +      Info.Diag(Conv);
>     }
>     return false;
>   }
> @@ -1850,7 +1860,7 @@
>     // FIXME: Support PredefinedExpr, ObjCEncodeExpr, MakeStringConstant
>     RVal = APValue(Base, CharUnits::Zero(), APValue::NoLValuePath(), 0);
>   } else {
> -    Info.Diag(Conv->getExprLoc(), diag::note_invalid_subexpr_in_const_expr);
> +    Info.Diag(Conv, diag::note_invalid_subexpr_in_const_expr);
>     return false;
>   }
>
> @@ -1976,7 +1986,7 @@
>
>   // Check this cast lands within the final derived-to-base subobject path.
>   if (D.MostDerivedPathLength + E->path_size() > D.Entries.size()) {
> -    Info.CCEDiag(E->getExprLoc(), diag::note_constexpr_invalid_downcast)
> +    Info.CCEDiag(E, diag::note_constexpr_invalid_downcast)
>       << D.MostDerivedType << TargetQT;
>     return false;
>   }
> @@ -1991,7 +2001,7 @@
>   else
>     FinalType = getAsBaseClass(D.Entries[NewEntriesSize - 1]);
>   if (FinalType->getCanonicalDecl() != TargetType->getCanonicalDecl()) {
> -    Info.CCEDiag(E->getExprLoc(), diag::note_constexpr_invalid_downcast)
> +    Info.CCEDiag(E, diag::note_constexpr_invalid_downcast)
>       << D.MostDerivedType << TargetQT;
>     return false;
>   }
> @@ -2420,13 +2430,13 @@
>   typedef ExprEvaluatorBase ExprEvaluatorBaseTy;
>
>   OptionalDiagnostic CCEDiag(const Expr *E, diag::kind D) {
> -    return Info.CCEDiag(E->getExprLoc(), D);
> +    return Info.CCEDiag(E, D);
>   }
>
>   /// Report an evaluation error. This should only be called when an error is
>   /// first discovered. When propagating an error, just return false.
>   bool Error(const Expr *E, diag::kind D) {
> -    Info.Diag(E->getExprLoc(), D);
> +    Info.Diag(E, D);
>     return false;
>   }
>   bool Error(const Expr *E) {
> @@ -2957,7 +2967,7 @@
>     return Success(E);
>   CXXRecordDecl *RD = E->getExprOperand()->getType()->getAsCXXRecordDecl();
>   if (RD && RD->isPolymorphic()) {
> -    Info.Diag(E->getExprLoc(), diag::note_constexpr_typeid_polymorphic)
> +    Info.Diag(E, diag::note_constexpr_typeid_polymorphic)
>       << E->getExprOperand()->getType()
>       << E->getExprOperand()->getSourceRange();
>     return false;
> @@ -3405,7 +3415,7 @@
>   }
>
>   if (isa<CXXRecordDecl>(RD) && cast<CXXRecordDecl>(RD)->getNumVBases()) {
> -    Info.Diag(E->getExprLoc(), diag::note_constexpr_virtual_base) << RD;
> +    Info.Diag(E, diag::note_constexpr_virtual_base) << RD;
>     return false;
>   }
>
> @@ -4115,7 +4125,7 @@
>   if (!Val.isInt()) {
>     // FIXME: It would be better to produce the diagnostic for casting
>     //        a pointer to an integer.
> -    Info.Diag(E->getExprLoc(), diag::note_invalid_subexpr_in_const_expr);
> +    Info.Diag(E, diag::note_invalid_subexpr_in_const_expr);
>     return false;
>   }
>   Result = Val.getInt();
> @@ -4338,10 +4348,10 @@
>   case Builtin::BIstrlen:
>     // A call to strlen is not a constant expression.
>     if (Info.getLangOpts().CPlusPlus0x)
> -      Info.CCEDiag(E->getExprLoc(), diag::note_constexpr_invalid_function)
> +      Info.CCEDiag(E, diag::note_constexpr_invalid_function)
>         << /*isConstexpr*/0 << /*isConstructor*/0 << "'strlen'";
>     else
> -      Info.CCEDiag(E->getExprLoc(), diag::note_invalid_subexpr_in_const_expr);
> +      Info.CCEDiag(E, diag::note_invalid_subexpr_in_const_expr);
>     // Fall through.
>   case Builtin::BI__builtin_strlen:
>     // As an extension, we support strlen() and __builtin_strlen() as constant
> @@ -5981,17 +5991,17 @@
>     Result = Info.CurrentCall->Temporaries[E];
>   } else if (E->getType()->isVoidType()) {
>     if (Info.getLangOpts().CPlusPlus0x)
> -      Info.CCEDiag(E->getExprLoc(), diag::note_constexpr_nonliteral)
> +      Info.CCEDiag(E, diag::note_constexpr_nonliteral)
>         << E->getType();
>     else
> -      Info.CCEDiag(E->getExprLoc(), diag::note_invalid_subexpr_in_const_expr);
> +      Info.CCEDiag(E, diag::note_invalid_subexpr_in_const_expr);
>     if (!EvaluateVoid(E, Info))
>       return false;
>   } else if (Info.getLangOpts().CPlusPlus0x) {
> -    Info.Diag(E->getExprLoc(), diag::note_constexpr_nonliteral) << E->getType();
> +    Info.Diag(E, diag::note_constexpr_nonliteral) << E->getType();
>     return false;
>   } else {
> -    Info.Diag(E->getExprLoc(), diag::note_invalid_subexpr_in_const_expr);
> +    Info.Diag(E, diag::note_invalid_subexpr_in_const_expr);
>     return false;
>   }
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits




More information about the cfe-commits mailing list