r360531 - Revert rL360499 and rL360464 from cfe/trunk:

Simon Pilgrim via cfe-commits cfe-commits at lists.llvm.org
Sat May 11 13:22:00 PDT 2019


Author: rksimon
Date: Sat May 11 13:21:59 2019
New Revision: 360531

URL: http://llvm.org/viewvc/llvm-project?rev=360531&view=rev
Log:
Revert rL360499 and rL360464 from cfe/trunk:
Reject attempts to call non-static member functions on objects outside
their lifetime in constant expressions.

This is undefined behavior per [class.cdtor]p2.

We continue to allow this for objects whose values are not visible
within the constant evaluation, because there's no way we can tell
whether the access is defined or not, existing code relies on the
ability to make such calls, and every other compiler allows such
calls.
........
Fix handling of objects under construction during constant expression
evaluation.

It's not enough to just track the LValueBase that we're evaluating, we
need to also track the path to the objects whose constructors are
running.
........
Fixes windows buildbots

Modified:
    cfe/trunk/include/clang/AST/APValue.h
    cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
    cfe/trunk/lib/AST/APValue.cpp
    cfe/trunk/lib/AST/ExprConstant.cpp
    cfe/trunk/test/CXX/expr/expr.const/p2-0x.cpp
    cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp
    cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp

Modified: cfe/trunk/include/clang/AST/APValue.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/APValue.h?rev=360531&r1=360530&r2=360531&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/APValue.h (original)
+++ cfe/trunk/include/clang/AST/APValue.h Sat May 11 13:21:59 2019
@@ -96,14 +96,10 @@ public:
       return Version;
     }
 
-    friend bool operator==(const LValueBase &LHS, const LValueBase &RHS) {
-      return LHS.Ptr == RHS.Ptr && LHS.CallIndex == RHS.CallIndex &&
-             LHS.Version == RHS.Version;
+    bool operator==(const LValueBase &Other) const {
+      return Ptr == Other.Ptr && CallIndex == Other.CallIndex &&
+             Version == Other.Version;
     }
-    friend bool operator!=(const LValueBase &LHS, const LValueBase &RHS) {
-      return !(LHS == RHS);
-    }
-    friend llvm::hash_code hash_value(const LValueBase &Base);
 
   private:
     PtrTy Ptr;

Modified: cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td?rev=360531&r1=360530&r2=360531&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td Sat May 11 13:21:59 2019
@@ -67,13 +67,13 @@ def note_constexpr_past_end : Note<
   "%select{temporary|%2}1 is not a constant expression">;
 def note_constexpr_past_end_subobject : Note<
   "cannot %select{access base class of|access derived class of|access field of|"
-  "access array element of|ERROR|"
+  "access array element of|ERROR|call member function on|"
   "access real component of|access imaginary component of}0 "
   "pointer past the end of object">;
 def note_constexpr_null_subobject : Note<
   "cannot %select{access base class of|access derived class of|access field of|"
   "access array element of|perform pointer arithmetic on|"
-  "access real component of|"
+  "call member function on|access real component of|"
   "access imaginary component of}0 null pointer">;
 def note_constexpr_var_init_non_constant : Note<
   "initializer of %0 is not a constant expression">;
@@ -96,10 +96,10 @@ def note_constexpr_this : Note<
   "%select{|implicit }0use of 'this' pointer is only allowed within the "
   "evaluation of a call to a 'constexpr' member function">;
 def note_constexpr_lifetime_ended : Note<
-  "%select{read of|assignment to|increment of|decrement of|member call on}0 "
+  "%select{read of|assignment to|increment of|decrement of}0 "
   "%select{temporary|variable}1 whose lifetime has ended">;
 def note_constexpr_access_uninit : Note<
-  "%select{read of|assignment to|increment of|decrement of|member call on}0 "
+  "%select{read of|assignment to|increment of|decrement of}0 "
   "object outside its lifetime is not allowed in a constant expression">;
 def note_constexpr_use_uninit_reference : Note<
   "use of reference outside its lifetime "
@@ -108,14 +108,12 @@ def note_constexpr_modify_const_type : N
   "modification of object of const-qualified type %0 is not allowed "
   "in a constant expression">;
 def note_constexpr_access_volatile_type : Note<
-  "%select{read of|assignment to|increment of|decrement of|<ERROR>}0 "
+  "%select{read of|assignment to|increment of|decrement of}0 "
   "volatile-qualified type %1 is not allowed in a constant expression">;
 def note_constexpr_access_volatile_obj : Note<
-  "%select{read of|assignment to|increment of|decrement of|<ERROR>}0 volatile "
+  "%select{read of|assignment to|increment of|decrement of}0 volatile "
   "%select{temporary|object %2|member %2}1 is not allowed in "
   "a constant expression">;
-def note_constexpr_volatile_here : Note<
-  "volatile %select{temporary created|object declared|member declared}0 here">;
 def note_constexpr_ltor_mutable : Note<
   "read of mutable member %0 is not allowed in a constant expression">;
 def note_constexpr_ltor_non_const_int : Note<
@@ -125,21 +123,21 @@ def note_constexpr_ltor_non_constexpr :
 def note_constexpr_ltor_incomplete_type : Note<
   "read of incomplete type %0 is not allowed in a constant expression">;
 def note_constexpr_access_null : Note<
-  "%select{read of|assignment to|increment of|decrement of|member call on}0 "
+  "%select{read of|assignment to|increment of|decrement of}0 "
   "dereferenced null pointer is not allowed in a constant expression">;
 def note_constexpr_access_past_end : Note<
-  "%select{read of|assignment to|increment of|decrement of|member call on}0 "
+  "%select{read of|assignment to|increment of|decrement of}0 "
   "dereferenced one-past-the-end pointer is not allowed in a constant expression">;
 def note_constexpr_access_unsized_array : Note<
-  "%select{read of|assignment to|increment of|decrement of|member call on}0 "
-  "element of array without known bound "
+  "%select{read of|assignment to|increment of|decrement of}0 "
+  "pointer to element of array without known bound "
   "is not allowed in a constant expression">;
 def note_constexpr_access_inactive_union_member : Note<
-  "%select{read of|assignment to|increment of|decrement of|member call on}0 "
+  "%select{read of|assignment to|increment of|decrement of}0 "
   "member %1 of union with %select{active member %3|no active member}2 "
   "is not allowed in a constant expression">;
 def note_constexpr_access_static_temporary : Note<
-  "%select{read of|assignment to|increment of|decrement of|<ERROR>}0 temporary "
+  "%select{read of|assignment to|increment of|decrement of}0 temporary "
   "is not allowed in a constant expression outside the expression that "
   "created the temporary">;
 def note_constexpr_modify_global : Note<

Modified: cfe/trunk/lib/AST/APValue.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/APValue.cpp?rev=360531&r1=360530&r2=360531&view=diff
==============================================================================
--- cfe/trunk/lib/AST/APValue.cpp (original)
+++ cfe/trunk/lib/AST/APValue.cpp Sat May 11 13:21:59 2019
@@ -58,16 +58,13 @@ llvm::DenseMapInfo<clang::APValue::LValu
       DenseMapInfo<unsigned>::getTombstoneKey());
 }
 
-namespace clang {
-llvm::hash_code hash_value(const APValue::LValueBase &Base) {
-  return llvm::hash_combine(Base.getOpaqueValue(), Base.getCallIndex(),
-                            Base.getVersion());
-}
-}
-
 unsigned llvm::DenseMapInfo<clang::APValue::LValueBase>::getHashValue(
     const clang::APValue::LValueBase &Base) {
-  return hash_value(Base);
+  llvm::FoldingSetNodeID ID;
+  ID.AddPointer(Base.getOpaqueValue());
+  ID.AddInteger(Base.getCallIndex());
+  ID.AddInteger(Base.getVersion());
+  return ID.ComputeHash();
 }
 
 bool llvm::DenseMapInfo<clang::APValue::LValueBase>::isEqual(

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=360531&r1=360530&r2=360531&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Sat May 11 13:21:59 2019
@@ -213,7 +213,7 @@ namespace {
   // The order of this enum is important for diagnostics.
   enum CheckSubobjectKind {
     CSK_Base, CSK_Derived, CSK_Field, CSK_ArrayToPointer, CSK_ArrayIndex,
-    CSK_Real, CSK_Imag
+    CSK_This, CSK_Real, CSK_Imag
   };
 
   /// A path from a glvalue to a subobject of that glvalue.
@@ -622,40 +622,6 @@ namespace {
     }
   };
 
-  /// A reference to an object whose construction we are currently evaluating.
-  struct ObjectUnderConstruction {
-    APValue::LValueBase Base;
-    ArrayRef<APValue::LValuePathEntry> Path;
-    friend bool operator==(const ObjectUnderConstruction &LHS,
-                           const ObjectUnderConstruction &RHS) {
-      return LHS.Base == RHS.Base && LHS.Path == RHS.Path;
-    }
-    friend llvm::hash_code hash_value(const ObjectUnderConstruction &Obj) {
-      return llvm::hash_combine(Obj.Base, Obj.Path);
-    }
-  };
-  enum class ConstructionPhase { None, Bases, AfterBases };
-}
-
-namespace llvm {
-template<> struct DenseMapInfo<ObjectUnderConstruction> {
-  using Base = DenseMapInfo<APValue::LValueBase>;
-  static ObjectUnderConstruction getEmptyKey() {
-    return {Base::getEmptyKey(), {}}; }
-  static ObjectUnderConstruction getTombstoneKey() {
-    return {Base::getTombstoneKey(), {}};
-  }
-  static unsigned getHashValue(const ObjectUnderConstruction &Object) {
-    return hash_value(Object);
-  }
-  static bool isEqual(const ObjectUnderConstruction &LHS,
-                      const ObjectUnderConstruction &RHS) {
-    return LHS == RHS;
-  }
-};
-}
-
-namespace {
   /// EvalInfo - This is a private struct used by the evaluator to capture
   /// information about a subexpression as it is folded.  It retains information
   /// about the AST context, but also maintains information about the folded
@@ -706,35 +672,32 @@ namespace {
     /// declaration whose initializer is being evaluated, if any.
     APValue *EvaluatingDeclValue;
 
-    /// Set of objects that are currently being constructed.
-    llvm::DenseMap<ObjectUnderConstruction, ConstructionPhase>
-        ObjectsUnderConstruction;
+    /// EvaluatingObject - Pair of the AST node that an lvalue represents and
+    /// the call index that that lvalue was allocated in.
+    typedef std::pair<APValue::LValueBase, std::pair<unsigned, unsigned>>
+        EvaluatingObject;
+
+    /// EvaluatingConstructors - Set of objects that are currently being
+    /// constructed.
+    llvm::DenseSet<EvaluatingObject> EvaluatingConstructors;
 
     struct EvaluatingConstructorRAII {
       EvalInfo &EI;
-      ObjectUnderConstruction Object;
+      EvaluatingObject Object;
       bool DidInsert;
-      EvaluatingConstructorRAII(EvalInfo &EI, ObjectUnderConstruction Object,
-                                bool HasBases)
+      EvaluatingConstructorRAII(EvalInfo &EI, EvaluatingObject Object)
           : EI(EI), Object(Object) {
-        DidInsert =
-            EI.ObjectsUnderConstruction
-                .insert({Object, HasBases ? ConstructionPhase::Bases
-                                          : ConstructionPhase::AfterBases})
-                .second;
-      }
-      void finishedConstructingBases() {
-        EI.ObjectsUnderConstruction[Object] = ConstructionPhase::AfterBases;
+        DidInsert = EI.EvaluatingConstructors.insert(Object).second;
       }
       ~EvaluatingConstructorRAII() {
-        if (DidInsert) EI.ObjectsUnderConstruction.erase(Object);
+        if (DidInsert) EI.EvaluatingConstructors.erase(Object);
       }
     };
 
-    ConstructionPhase
-    isEvaluatingConstructor(APValue::LValueBase Base,
-                            ArrayRef<APValue::LValuePathEntry> Path) {
-      return ObjectsUnderConstruction.lookup({Base, Path});
+    bool isEvaluatingConstructor(APValue::LValueBase Decl, unsigned CallIndex,
+                                 unsigned Version) {
+      return EvaluatingConstructors.count(
+          EvaluatingObject(Decl, {CallIndex, Version}));
     }
 
     /// If we're currently speculatively evaluating, the outermost call stack
@@ -823,6 +786,7 @@ namespace {
     void setEvaluatingDecl(APValue::LValueBase Base, APValue &Value) {
       EvaluatingDecl = Base;
       EvaluatingDeclValue = &Value;
+      EvaluatingConstructors.insert({Base, {0, 0}});
     }
 
     const LangOptions &getLangOpts() const { return Ctx.getLangOpts(); }
@@ -1326,22 +1290,14 @@ void EvalInfo::addCallStack(unsigned Lim
   }
 }
 
-/// Kinds of access we can perform on an object, for diagnostics. Note that
-/// we consider a member function call to be a kind of access, even though
-/// it is not formally an access of the object, because it has (largely) the
-/// same set of semantic restrictions.
+/// Kinds of access we can perform on an object, for diagnostics.
 enum AccessKinds {
   AK_Read,
   AK_Assign,
   AK_Increment,
-  AK_Decrement,
-  AK_MemberCall,
+  AK_Decrement
 };
 
-static bool isModification(AccessKinds AK) {
-  return AK != AK_Read && AK != AK_MemberCall;
-}
-
 namespace {
   struct ComplexValue {
   private:
@@ -2828,73 +2784,28 @@ static bool diagnoseUnreadableFields(Eva
   return false;
 }
 
-static bool lifetimeStartedInEvaluation(EvalInfo &Info,
-                                        APValue::LValueBase Base) {
-  // A temporary we created.
-  if (Base.getCallIndex())
-    return true;
-
-  auto *Evaluating = Info.EvaluatingDecl.dyn_cast<const ValueDecl*>();
-  if (!Evaluating)
-    return false;
-
-  // The variable whose initializer we're evaluating.
-  if (auto *BaseD = Base.dyn_cast<const ValueDecl*>())
-    if (declaresSameEntity(Evaluating, BaseD))
-      return true;
-
-  // A temporary lifetime-extended by the variable whose initializer we're
-  // evaluating.
-  if (auto *BaseE = Base.dyn_cast<const Expr *>())
-    if (auto *BaseMTE = dyn_cast<MaterializeTemporaryExpr>(BaseE))
-      if (declaresSameEntity(BaseMTE->getExtendingDecl(), Evaluating))
-        return true;
-
-  return false;
-}
-
 namespace {
 /// A handle to a complete object (an object that is not a subobject of
 /// another object).
 struct CompleteObject {
-  /// The identity of the object.
-  APValue::LValueBase Base;
   /// The value of the complete object.
   APValue *Value;
   /// The type of the complete object.
   QualType Type;
+  bool LifetimeStartedInEvaluation;
 
   CompleteObject() : Value(nullptr) {}
-  CompleteObject(APValue::LValueBase Base, APValue *Value, QualType Type)
-      : Base(Base), Value(Value), Type(Type) {}
-
-  bool mayReadMutableMembers(EvalInfo &Info) const {
-    // In C++14 onwards, it is permitted to read a mutable member whose
-    // lifetime began within the evaluation.
-    // FIXME: Should we also allow this in C++11?
-    if (!Info.getLangOpts().CPlusPlus14)
-      return false;
-    return lifetimeStartedInEvaluation(Info, Base);
+  CompleteObject(APValue *Value, QualType Type,
+                 bool LifetimeStartedInEvaluation)
+      : Value(Value), Type(Type),
+        LifetimeStartedInEvaluation(LifetimeStartedInEvaluation) {
+    assert(Value && "missing value for complete object");
   }
 
-  explicit operator bool() const { return !Type.isNull(); }
+  explicit operator bool() const { return Value; }
 };
 } // end anonymous namespace
 
-static QualType getSubobjectType(QualType ObjType, QualType SubobjType,
-                                 bool IsMutable = false) {
-  // C++ [basic.type.qualifier]p1:
-  // - A const object is an object of type const T or a non-mutable subobject
-  //   of a const object.
-  if (ObjType.isConstQualified() && !IsMutable)
-    SubobjType.addConst();
-  // - A volatile object is an object of type const T or a subobject of a
-  //   volatile object.
-  if (ObjType.isVolatileQualified())
-    SubobjType.addVolatile();
-  return SubobjType;
-}
-
 /// Find the designated sub-object of an rvalue.
 template<typename SubobjectHandler>
 typename SubobjectHandler::result_type
@@ -2917,78 +2828,31 @@ findSubobject(EvalInfo &Info, const Expr
   APValue *O = Obj.Value;
   QualType ObjType = Obj.Type;
   const FieldDecl *LastField = nullptr;
-  const FieldDecl *VolatileField = nullptr;
+  const bool MayReadMutableMembers =
+      Obj.LifetimeStartedInEvaluation && Info.getLangOpts().CPlusPlus14;
 
   // Walk the designator's path to find the subobject.
   for (unsigned I = 0, N = Sub.Entries.size(); /**/; ++I) {
     if (O->isUninit()) {
       if (!Info.checkingPotentialConstantExpression())
-        Info.FFDiag(E, diag::note_constexpr_access_uninit)
-            << handler.AccessKind;
+        Info.FFDiag(E, diag::note_constexpr_access_uninit) << handler.AccessKind;
       return handler.failed();
     }
 
-    // C++ [class.ctor]p5:
-    //    const and volatile semantics are not applied on an object under
-    //    construction.
-    if ((ObjType.isConstQualified() || ObjType.isVolatileQualified()) &&
-        ObjType->isRecordType() &&
-        Info.isEvaluatingConstructor(
-            Obj.Base, llvm::makeArrayRef(Sub.Entries.begin(),
-                                         Sub.Entries.begin() + I)) !=
-                          ConstructionPhase::None) {
-      ObjType = Info.Ctx.getCanonicalType(ObjType);
-      ObjType.removeLocalConst();
-      ObjType.removeLocalVolatile();
-    }
-
-    // If this is our last pass, check that the final object type is OK.
-    if (I == N || (I == N - 1 && ObjType->isAnyComplexType())) {
-      // Accesses to volatile objects are prohibited.
-      if (ObjType.isVolatileQualified() &&
-          handler.AccessKind != AK_MemberCall) {
-        if (Info.getLangOpts().CPlusPlus) {
-          int DiagKind;
-          SourceLocation Loc;
-          const NamedDecl *Decl = nullptr;
-          if (VolatileField) {
-            DiagKind = 2;
-            Loc = VolatileField->getLocation();
-            Decl = VolatileField;
-          } else if (auto *VD = Obj.Base.dyn_cast<const ValueDecl*>()) {
-            DiagKind = 1;
-            Loc = VD->getLocation();
-            Decl = VD;
-          } else {
-            DiagKind = 0;
-            if (auto *E = Obj.Base.dyn_cast<const Expr *>())
-              Loc = E->getExprLoc();
-          }
-          Info.FFDiag(E, diag::note_constexpr_access_volatile_obj, 1)
-              << handler.AccessKind << DiagKind << Decl;
-          Info.Note(Loc, diag::note_constexpr_volatile_here) << DiagKind;
-        } else {
-          Info.FFDiag(E, diag::note_invalid_subexpr_in_const_expr);
-        }
-        return handler.failed();
-      }
-
+    if (I == N) {
       // If we are reading an object of class type, there may still be more
       // things we need to check: if there are any mutable subobjects, we
       // cannot perform this read. (This only happens when performing a trivial
       // copy or assignment.)
       if (ObjType->isRecordType() && handler.AccessKind == AK_Read &&
-          !Obj.mayReadMutableMembers(Info) &&
-          diagnoseUnreadableFields(Info, E, ObjType))
+          !MayReadMutableMembers && diagnoseUnreadableFields(Info, E, ObjType))
         return handler.failed();
-    }
 
-    if (I == N) {
       if (!handler.found(*O, ObjType))
         return false;
 
       // If we modified a bit-field, truncate it to the right width.
-      if (isModification(handler.AccessKind) &&
+      if (handler.AccessKind != AK_Read &&
           LastField && LastField->isBitField() &&
           !truncateBitfieldValue(Info, E, *O, LastField))
         return false;
@@ -3034,8 +2898,10 @@ findSubobject(EvalInfo &Info, const Expr
         return handler.failed();
       }
 
-      ObjType = getSubobjectType(
-          ObjType, ObjType->castAs<ComplexType>()->getElementType());
+      bool WasConstQualified = ObjType.isConstQualified();
+      ObjType = ObjType->castAs<ComplexType>()->getElementType();
+      if (WasConstQualified)
+        ObjType.addConst();
 
       assert(I == N - 1 && "extracting subobject of scalar?");
       if (O->isComplexInt()) {
@@ -3047,8 +2913,11 @@ findSubobject(EvalInfo &Info, const Expr
                                    : O->getComplexFloatReal(), ObjType);
       }
     } else if (const FieldDecl *Field = getAsField(Sub.Entries[I])) {
+      // In C++14 onwards, it is permitted to read a mutable member whose
+      // lifetime began within the evaluation.
+      // FIXME: Should we also allow this in C++11?
       if (Field->isMutable() && handler.AccessKind == AK_Read &&
-          !Obj.mayReadMutableMembers(Info)) {
+          !MayReadMutableMembers) {
         Info.FFDiag(E, diag::note_constexpr_ltor_mutable, 1)
           << Field;
         Info.Note(Field->getLocation(), diag::note_declared_at);
@@ -3069,17 +2938,34 @@ findSubobject(EvalInfo &Info, const Expr
       } else
         O = &O->getStructField(Field->getFieldIndex());
 
-      ObjType = getSubobjectType(ObjType, Field->getType(), Field->isMutable());
+      bool WasConstQualified = ObjType.isConstQualified();
+      ObjType = Field->getType();
+      if (WasConstQualified && !Field->isMutable())
+        ObjType.addConst();
+
+      if (ObjType.isVolatileQualified()) {
+        if (Info.getLangOpts().CPlusPlus) {
+          // FIXME: Include a description of the path to the volatile subobject.
+          Info.FFDiag(E, diag::note_constexpr_access_volatile_obj, 1)
+            << handler.AccessKind << 2 << Field;
+          Info.Note(Field->getLocation(), diag::note_declared_at);
+        } else {
+          Info.FFDiag(E, diag::note_invalid_subexpr_in_const_expr);
+        }
+        return handler.failed();
+      }
+
       LastField = Field;
-      if (Field->getType().isVolatileQualified())
-        VolatileField = Field;
     } else {
       // Next subobject is a base class.
       const CXXRecordDecl *Derived = ObjType->getAsCXXRecordDecl();
       const CXXRecordDecl *Base = getAsBaseClass(Sub.Entries[I]);
       O = &O->getStructBase(getBaseIndex(Derived, Base));
 
-      ObjType = getSubobjectType(ObjType, Info.Ctx.getRecordType(Base));
+      bool WasConstQualified = ObjType.isConstQualified();
+      ObjType = Info.Ctx.getRecordType(Base);
+      if (WasConstQualified)
+        ObjType.addConst();
     }
   }
 }
@@ -3260,7 +3146,7 @@ static CompleteObject findCompleteObject
   // is not a constant expression (even if the object is non-volatile). We also
   // apply this rule to C++98, in order to conform to the expected 'volatile'
   // semantics.
-  if (AK != AK_MemberCall && LValType.isVolatileQualified()) {
+  if (LValType.isVolatileQualified()) {
     if (Info.getLangOpts().CPlusPlus)
       Info.FFDiag(E, diag::note_constexpr_access_volatile_type)
         << AK << LValType;
@@ -3269,16 +3155,10 @@ static CompleteObject findCompleteObject
     return CompleteObject();
   }
 
-  // The wording is unclear on this, but for the purpose of determining the
-  // validity of a member function call, we assume that all objects whose
-  // lifetimes did not start within the constant evaluation are in fact within
-  // their lifetimes, so member calls on them are valid. (This simultaneously
-  // includes all members of a union!)
-  bool NeedValue = AK != AK_MemberCall;
-
   // Compute value storage location and type of base object.
   APValue *BaseVal = nullptr;
   QualType BaseType = getType(LVal.Base);
+  bool LifetimeStartedInEvaluation = Frame;
 
   if (const ValueDecl *D = LVal.Base.dyn_cast<const ValueDecl*>()) {
     // In C++98, const, non-volatile integers initialized with ICEs are ICEs.
@@ -3298,29 +3178,37 @@ static CompleteObject findCompleteObject
       return CompleteObject();
     }
 
+    // Accesses of volatile-qualified objects are not allowed.
+    if (BaseType.isVolatileQualified()) {
+      if (Info.getLangOpts().CPlusPlus) {
+        Info.FFDiag(E, diag::note_constexpr_access_volatile_obj, 1)
+          << AK << 1 << VD;
+        Info.Note(VD->getLocation(), diag::note_declared_at);
+      } else {
+        Info.FFDiag(E);
+      }
+      return CompleteObject();
+    }
+
     // Unless we're looking at a local variable or argument in a constexpr call,
     // the variable we're reading must be const.
     if (!Frame) {
       if (Info.getLangOpts().CPlusPlus14 &&
-          declaresSameEntity(
-              VD, Info.EvaluatingDecl.dyn_cast<const ValueDecl *>())) {
+          VD == Info.EvaluatingDecl.dyn_cast<const ValueDecl *>()) {
         // OK, we can read and modify an object if we're in the process of
         // evaluating its initializer, because its lifetime began in this
         // evaluation.
-      } else if (isModification(AK)) {
-        // All the remaining cases do not permit modification of the object.
+      } else if (AK != AK_Read) {
+        // All the remaining cases only permit reading.
         Info.FFDiag(E, diag::note_constexpr_modify_global);
         return CompleteObject();
       } else if (VD->isConstexpr()) {
         // OK, we can read this variable.
       } else if (BaseType->isIntegralOrEnumerationType()) {
-        // In OpenCL if a variable is in constant address space it is a const
-        // value.
+        // In OpenCL if a variable is in constant address space it is a const value.
         if (!(BaseType.isConstQualified() ||
               (Info.getLangOpts().OpenCL &&
                BaseType.getAddressSpace() == LangAS::opencl_constant))) {
-          if (!NeedValue)
-            return CompleteObject(LVal.getLValueBase(), nullptr, BaseType);
           if (Info.getLangOpts().CPlusPlus) {
             Info.FFDiag(E, diag::note_constexpr_ltor_non_const_int, 1) << VD;
             Info.Note(VD->getLocation(), diag::note_declared_at);
@@ -3329,8 +3217,6 @@ static CompleteObject findCompleteObject
           }
           return CompleteObject();
         }
-      } else if (!NeedValue) {
-        return CompleteObject(LVal.getLValueBase(), nullptr, BaseType);
       } else if (BaseType->isFloatingType() && BaseType.isConstQualified()) {
         // We support folding of const floating-point types, in order to make
         // static const data members of such types (supported as an extension)
@@ -3390,8 +3276,6 @@ static CompleteObject findCompleteObject
         if (!(BaseType.isConstQualified() &&
               BaseType->isIntegralOrEnumerationType()) &&
             !(VD && VD->getCanonicalDecl() == ED->getCanonicalDecl())) {
-          if (!NeedValue)
-            return CompleteObject(LVal.getLValueBase(), nullptr, BaseType);
           Info.FFDiag(E, diag::note_constexpr_access_static_temporary, 1) << AK;
           Info.Note(MTE->getExprLoc(), diag::note_constexpr_temporary_here);
           return CompleteObject();
@@ -3399,9 +3283,8 @@ static CompleteObject findCompleteObject
 
         BaseVal = Info.Ctx.getMaterializedTemporaryValue(MTE, false);
         assert(BaseVal && "got reference to unevaluated temporary");
+        LifetimeStartedInEvaluation = true;
       } else {
-        if (!NeedValue)
-          return CompleteObject(LVal.getLValueBase(), nullptr, BaseType);
         Info.FFDiag(E);
         return CompleteObject();
       }
@@ -3409,6 +3292,29 @@ static CompleteObject findCompleteObject
       BaseVal = Frame->getTemporary(Base, LVal.Base.getVersion());
       assert(BaseVal && "missing value for temporary");
     }
+
+    // Volatile temporary objects cannot be accessed in constant expressions.
+    if (BaseType.isVolatileQualified()) {
+      if (Info.getLangOpts().CPlusPlus) {
+        Info.FFDiag(E, diag::note_constexpr_access_volatile_obj, 1)
+          << AK << 0;
+        Info.Note(Base->getExprLoc(), diag::note_constexpr_temporary_here);
+      } else {
+        Info.FFDiag(E);
+      }
+      return CompleteObject();
+    }
+  }
+
+  // During the construction of an object, it is not yet 'const'.
+  // FIXME: This doesn't do quite the right thing for const subobjects of the
+  // object under construction.
+  if (Info.isEvaluatingConstructor(LVal.getLValueBase(),
+                                   LVal.getLValueCallIndex(),
+                                   LVal.getLValueVersion())) {
+    BaseType = Info.Ctx.getCanonicalType(BaseType);
+    BaseType.removeLocalConst();
+    LifetimeStartedInEvaluation = true;
   }
 
   // In C++14, we can't safely access any mutable state when we might be
@@ -3418,10 +3324,10 @@ static CompleteObject findCompleteObject
   // to be read here (but take care with 'mutable' fields).
   if ((Frame && Info.getLangOpts().CPlusPlus14 &&
        Info.EvalStatus.HasSideEffects) ||
-      (isModification(AK) && Depth < Info.SpeculativeEvaluationDepth))
+      (AK != AK_Read && Depth < Info.SpeculativeEvaluationDepth))
     return CompleteObject();
 
-  return CompleteObject(LVal.getLValueBase(), BaseVal, BaseType);
+  return CompleteObject(BaseVal, BaseType, LifetimeStartedInEvaluation);
 }
 
 /// Perform an lvalue-to-rvalue conversion on the given glvalue. This
@@ -3455,7 +3361,7 @@ static bool handleLValueToRValueConversi
       APValue Lit;
       if (!Evaluate(Lit, Info, CLE->getInitializer()))
         return false;
-      CompleteObject LitObj(LVal.Base, &Lit, Base->getType());
+      CompleteObject LitObj(&Lit, Base->getType(), false);
       return extractSubobject(Info, Conv, LitObj, LVal.Designator, RVal);
     } else if (isa<StringLiteral>(Base) || isa<PredefinedExpr>(Base)) {
       // Special-case character extraction so we don't have to construct an
@@ -4501,48 +4407,6 @@ static bool CheckConstexprFunction(EvalI
   return false;
 }
 
-namespace {
-struct CheckMemberCallThisPointerHandler {
-  static const AccessKinds AccessKind = AK_MemberCall;
-  typedef bool result_type;
-  bool failed() { return false; }
-  bool found(APValue &Subobj, QualType SubobjType) { return true; }
-  bool found(APSInt &Value, QualType SubobjType) { return true; }
-  bool found(APFloat &Value, QualType SubobjType) { return true; }
-};
-} // end anonymous namespace
-
-const AccessKinds CheckMemberCallThisPointerHandler::AccessKind;
-
-/// Check that the pointee of the 'this' pointer in a member function call is
-/// either within its lifetime or in its period of construction or destruction.
-static bool checkMemberCallThisPointer(EvalInfo &Info, const Expr *E,
-                                       const LValue &This) {
-  CompleteObject Obj =
-      findCompleteObject(Info, E, AK_MemberCall, This, QualType());
-
-  if (!Obj)
-    return false;
-
-  if (!Obj.Value) {
-    // The object is not usable in constant expressions, so we can't inspect
-    // its value to see if it's in-lifetime or what the active union members
-    // are. We can still check for a one-past-the-end lvalue.
-    if (This.Designator.isOnePastTheEnd() ||
-        This.Designator.isMostDerivedAnUnsizedArray()) {
-      Info.FFDiag(E, This.Designator.isOnePastTheEnd()
-                         ? diag::note_constexpr_access_past_end
-                         : diag::note_constexpr_access_unsized_array)
-          << AK_MemberCall;
-      return false;
-    }
-    return true;
-  }
-
-  CheckMemberCallThisPointerHandler Handler;
-  return Obj && findSubobject(Info, E, Obj, This.Designator, Handler);
-}
-
 /// Determine if a class has any fields that might need to be copied by a
 /// trivial copy or move operation.
 static bool hasFields(const CXXRecordDecl *RD) {
@@ -4656,9 +4520,8 @@ static bool HandleConstructorCall(const
   }
 
   EvalInfo::EvaluatingConstructorRAII EvalObj(
-      Info,
-      ObjectUnderConstruction{This.getLValueBase(), This.Designator.Entries},
-      RD->getNumBases());
+      Info, {This.getLValueBase(),
+             {This.getLValueCallIndex(), This.getLValueVersion()}});
   CallStackFrame Frame(Info, CallLoc, Definition, &This, ArgValues);
 
   // FIXME: Creating an APValue just to hold a nonexistent return value is
@@ -4732,11 +4595,6 @@ static bool HandleConstructorCall(const
                                   BaseType->getAsCXXRecordDecl(), &Layout))
         return false;
       Value = &Result.getStructBase(BasesSeen++);
-
-      // This is the point at which the dynamic type of the object becomes this
-      // class type.
-      if (BasesSeen == RD->getNumBases())
-        EvalObj.finishedConstructingBases();
     } else if ((FD = I->getMember())) {
       if (!HandleLValueMember(Info, I->getInit(), Subobject, FD, &Layout))
         return false;
@@ -5127,7 +4985,7 @@ public:
     } else
       return Error(E);
 
-    if (This && !checkMemberCallThisPointer(Info, E, *This))
+    if (This && !This->checkSubobject(Info, E, CSK_This))
       return false;
 
     const FunctionDecl *Definition = nullptr;
@@ -5163,8 +5021,6 @@ public:
 
   /// A member expression where the object is a prvalue is itself a prvalue.
   bool VisitMemberExpr(const MemberExpr *E) {
-    assert(!Info.Ctx.getLangOpts().CPlusPlus11 &&
-           "missing temporary materialization conversion");
     assert(!E->isArrow() && "missing call to bound member function?");
 
     APValue Val;
@@ -5179,10 +5035,7 @@ public:
     assert(BaseTy->castAs<RecordType>()->getDecl()->getCanonicalDecl() ==
            FD->getParent()->getCanonicalDecl() && "record / field mismatch");
 
-    // Note: there is no lvalue base here. But this case should only ever
-    // happen in C or in C++98, where we cannot be evaluating a constexpr
-    // constructor, which is the only case the base matters.
-    CompleteObject Obj(APValue::LValueBase(), &Val, BaseTy);
+    CompleteObject Obj(&Val, BaseTy, true);
     SubobjectDesignator Designator(BaseTy);
     Designator.addDeclUnchecked(FD);
 
@@ -6776,12 +6629,6 @@ bool RecordExprEvaluator::VisitInitListE
   const RecordDecl *RD = E->getType()->castAs<RecordType>()->getDecl();
   if (RD->isInvalidDecl()) return false;
   const ASTRecordLayout &Layout = Info.Ctx.getASTRecordLayout(RD);
-  auto *CXXRD = dyn_cast<CXXRecordDecl>(RD);
-
-  EvalInfo::EvaluatingConstructorRAII EvalObj(
-      Info,
-      ObjectUnderConstruction{This.getLValueBase(), This.Designator.Entries},
-      CXXRD && CXXRD->getNumBases());
 
   if (RD->isUnion()) {
     const FieldDecl *Field = E->getInitializedFieldInUnion();
@@ -6808,6 +6655,7 @@ bool RecordExprEvaluator::VisitInitListE
     return EvaluateInPlace(Result.getUnionValue(), Info, Subobject, InitExpr);
   }
 
+  auto *CXXRD = dyn_cast<CXXRecordDecl>(RD);
   if (Result.isUninit())
     Result = APValue(APValue::UninitStruct(), CXXRD ? CXXRD->getNumBases() : 0,
                      std::distance(RD->field_begin(), RD->field_end()));
@@ -6815,7 +6663,7 @@ bool RecordExprEvaluator::VisitInitListE
   bool Success = true;
 
   // Initialize base classes.
-  if (CXXRD && CXXRD->getNumBases()) {
+  if (CXXRD) {
     for (const auto &Base : CXXRD->bases()) {
       assert(ElementNo < E->getNumInits() && "missing init for base class");
       const Expr *Init = E->getInit(ElementNo);
@@ -6832,8 +6680,6 @@ bool RecordExprEvaluator::VisitInitListE
       }
       ++ElementNo;
     }
-
-    EvalObj.finishedConstructingBases();
   }
 
   // Initialize members.

Modified: cfe/trunk/test/CXX/expr/expr.const/p2-0x.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/expr/expr.const/p2-0x.cpp?rev=360531&r1=360530&r2=360531&view=diff
==============================================================================
--- cfe/trunk/test/CXX/expr/expr.const/p2-0x.cpp (original)
+++ cfe/trunk/test/CXX/expr/expr.const/p2-0x.cpp Sat May 11 13:21:59 2019
@@ -210,8 +210,8 @@ namespace UndefinedBehavior {
       constexpr int f() const { return 0; }
     } constexpr c = C();
     constexpr int k1 = c.f(); // ok
-    constexpr int k2 = ((C*)nullptr)->f(); // expected-error {{constant expression}} expected-note {{member call on dereferenced null pointer}}
-    constexpr int k3 = (&c)[1].f(); // expected-error {{constant expression}} expected-note {{member call on dereferenced one-past-the-end pointer}}
+    constexpr int k2 = ((C*)nullptr)->f(); // expected-error {{constant expression}} expected-note {{cannot call member function on null pointer}}
+    constexpr int k3 = (&c)[1].f(); // expected-error {{constant expression}} expected-note {{cannot call member function on pointer past the end of object}}
     C c2;
     constexpr int k4 = c2.f(); // ok!
 

Modified: cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp?rev=360531&r1=360530&r2=360531&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp (original)
+++ cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp Sat May 11 13:21:59 2019
@@ -192,25 +192,6 @@ namespace StaticMemberFunction {
   constexpr int (*sf1)(int) = &S::f;
   constexpr int (*sf2)(int) = &s.f;
   constexpr const int *sk = &s.k;
-
-  // Note, out_of_lifetime returns an invalid pointer value, but we don't do
-  // anything with it (other than copy it around), so there's no UB there.
-  constexpr S *out_of_lifetime(S s) { return &s; } // expected-warning {{address of stack}}
-  static_assert(out_of_lifetime({})->k == 42, "");
-  static_assert(out_of_lifetime({})->f(3) == 128, "");
-
-  // Similarly, using an inactive union member breaks no rules.
-  union U {
-    int n;
-    S s;
-  };
-  constexpr U u = {0};
-  static_assert(u.s.k == 42, "");
-  static_assert(u.s.f(1) == 44, "");
-
-  // And likewise for a past-the-end pointer.
-  static_assert((&s)[1].k == 42, "");
-  static_assert((&s)[1].f(1) == 44, "");
 }
 
 namespace ParameterScopes {
@@ -1748,10 +1729,19 @@ namespace PR14203 {
     constexpr duration() {}
     constexpr operator int() const { return 0; }
   };
-  // These are valid per P0859R0 (moved as DR).
   template<typename T> void f() {
+    // If we want to evaluate this at the point of the template definition, we
+    // need to trigger the implicit definition of the move constructor at that
+    // point.
+    // FIXME: C++ does not permit us to implicitly define it at the appropriate
+    // times, since it is only allowed to be implicitly defined when it is
+    // odr-used.
     constexpr duration d = duration();
   }
+  // FIXME: It's unclear whether this is valid. On the one hand, we're not
+  // allowed to generate a move constructor. On the other hand, if we did,
+  // this would be a constant expression. For now, we generate a move
+  // constructor here.
   int n = sizeof(short{duration(duration())});
 }
 
@@ -1912,52 +1902,6 @@ namespace Lifetime {
   };
   constexpr int k1 = S().t; // expected-error {{constant expression}} expected-note {{in call}}
   constexpr int k2 = S(0).t; // expected-error {{constant expression}} expected-note {{in call}}
-
-  struct Q {
-    int n = 0;
-    constexpr int f() const { return 0; }
-  };
-  constexpr Q *out_of_lifetime(Q q) { return &q; } // expected-warning {{address of stack}} expected-note 2{{declared here}}
-  constexpr int k3 = out_of_lifetime({})->n; // expected-error {{constant expression}} expected-note {{read of variable whose lifetime has ended}}
-  constexpr int k4 = out_of_lifetime({})->f(); // expected-error {{constant expression}} expected-note {{member call on variable whose lifetime has ended}}
-
-  constexpr int null = ((Q*)nullptr)->f(); // expected-error {{constant expression}} expected-note {{member call on dereferenced null pointer}}
-
-  Q q;
-  Q qa[3];
-  constexpr int pte0 = (&q)[0].f(); // ok
-  constexpr int pte1 = (&q)[1].f(); // expected-error {{constant expression}} expected-note {{member call on dereferenced one-past-the-end pointer}}
-  constexpr int pte2 = qa[2].f(); // ok
-  constexpr int pte3 = qa[3].f(); // expected-error {{constant expression}} expected-note {{member call on dereferenced one-past-the-end pointer}}
-
-  constexpr Q cq;
-  constexpr Q cqa[3];
-  constexpr int cpte0 = (&cq)[0].f(); // ok
-  constexpr int cpte1 = (&cq)[1].f(); // expected-error {{constant expression}} expected-note {{member call on dereferenced one-past-the-end pointer}}
-  constexpr int cpte2 = cqa[2].f(); // ok
-  constexpr int cpte3 = cqa[3].f(); // expected-error {{constant expression}} expected-note {{member call on dereferenced one-past-the-end pointer}}
-
-  // FIXME: There's no way if we can tell if the first call here is valid; it
-  // depends on the active union member. Should we reject for that reason?
-  union U {
-    int n;
-    Q q;
-  };
-  U u1 = {0};
-  constexpr U u2 = {0};
-  constexpr int union_member1 = u1.q.f();
-  constexpr int union_member2 = u2.q.f(); // expected-error {{constant expression}} expected-note {{member call on member 'q' of union with active member 'n'}}
-
-  struct R { // expected-note {{field init}}
-    struct Inner { constexpr int f() const { return 0; } };
-    int a = b.f(); // expected-warning {{uninitialized}} expected-note {{member call on object outside its lifetime}}
-    Inner b;
-  };
-  // FIXME: This should be rejected under DR2026.
-  constexpr R r; // expected-note {{default constructor}}
-  void rf() {
-    constexpr R r; // expected-error {{constant expression}} expected-note {{in call}}
-  }
 }
 
 namespace Bitfields {

Modified: cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp?rev=360531&r1=360530&r2=360531&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp (original)
+++ cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp Sat May 11 13:21:59 2019
@@ -1159,49 +1159,3 @@ enum InEnum2 : int {
 enum class InEnum3 {
   THREE = indirect_builtin_constant_p("abc")
 };
-
-// [class.ctor]p4:
-//   A constructor can be invoked for a const, volatile or const volatile
-//   object. const and volatile semantics are not applied on an object under
-//   construction. They come into effect when the constructor for the most
-//   derived object ends.
-namespace ObjectsUnderConstruction {
-  struct A {
-    int n;
-    constexpr A() : n(1) { n = 2; }
-  };
-  struct B {
-    const A a;
-    constexpr B(bool mutate) {
-      if (mutate)
-        const_cast<A &>(a).n = 3; // expected-note {{modification of object of const-qualified type 'const int'}}
-    }
-  };
-  constexpr B b(false);
-  static_assert(b.a.n == 2, "");
-  constexpr B bad(true); // expected-error {{must be initialized by a constant expression}} expected-note {{in call to 'B(true)'}}
-
-  struct C {
-    int n;
-    constexpr C() : n(1) { n = 2; }
-  };
-  constexpr int f(bool get) {
-    volatile C c; // expected-note {{here}}
-    return get ? const_cast<int&>(c.n) : 0; // expected-note {{read of volatile object 'c'}}
-  }
-  static_assert(f(false) == 0, ""); // ok, can modify volatile c.n during c's initialization: it's not volatile then
-  static_assert(f(true) == 2, ""); // expected-error {{constant}} expected-note {{in call}}
-
-  struct Aggregate {
-    int x = 0;
-    int y = ++x;
-  };
-  constexpr Aggregate aggr1;
-  static_assert(aggr1.x == 1 && aggr1.y == 1, "");
-  // FIXME: This is not specified by the standard, but sanity requires it.
-  constexpr Aggregate aggr2 = {};
-  static_assert(aggr2.x == 1 && aggr2.y == 1, "");
-
-  // The lifetime of 'n' begins at the initialization, not before.
-  constexpr int n = ++const_cast<int&>(n); // expected-error {{constant expression}} expected-note {{modification}}
-}




More information about the cfe-commits mailing list