r256609 - [ptr-traits] Stop using two bogus types as stand-ins for flags

Chandler Carruth via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 29 18:51:00 PST 2015


Author: chandlerc
Date: Tue Dec 29 20:51:00 2015
New Revision: 256609

URL: http://llvm.org/viewvc/llvm-project?rev=256609&view=rev
Log:
[ptr-traits] Stop using two bogus types as stand-ins for flags
indicating the nature of the default argument in a ParmVarDecl.

Instead, this adds a proper enum stored exclusively in the ParmVarDecl
bits (which we have plenty of) for this. This even allows us to track
a previously unrepresented state in Clang when we parse a function
declaration with a default argument on a parameter but we cannot even
form an invalid expression node (for example, it is an invalid token).
Now, we can model this state in the AST at least, and potentially
improve recovery in this area in the future.

I've also cleaned up the functions managing both variable initializer
expressions and parameter default argument expresssions as much as
possible. I've left some comments about further improvements based on
a discussion with Richard Smith. Lots of credit to him for walking me
through exactly which of the *many* tradeoffs here he felt was the best
fit.

Should be NFC for now. I've tried my best to preserve existing behavior.

This is part of a series of patches to allow LLVM to check for complete
pointee types when computing its pointer traits. This is absolutely
necessary to get correct (or reproducible) results for things like how
many low bits are guaranteed to be zero.

Modified:
    cfe/trunk/include/clang/AST/Decl.h
    cfe/trunk/lib/AST/Decl.cpp
    cfe/trunk/lib/AST/StmtIterator.cpp

Modified: cfe/trunk/include/clang/AST/Decl.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=256609&r1=256608&r2=256609&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Tue Dec 29 20:51:00 2015
@@ -729,17 +729,14 @@ public:
   };
 
 protected:
-  /// \brief Placeholder type used in Init to denote an unparsed C++ default
-  /// argument.
-  struct UnparsedDefaultArgument;
-
-  /// \brief Placeholder type used in Init to denote an uninstantiated C++
-  /// default argument.
-  struct UninstantiatedDefaultArgument;
-
-  typedef llvm::PointerUnion4<Stmt *, EvaluatedStmt *,
-                              UnparsedDefaultArgument *,
-                              UninstantiatedDefaultArgument *> InitType;
+  // A pointer union of Stmt * and EvaluatedStmt *. When an EvaluatedStmt, we
+  // have allocated the auxilliary struct of information there.
+  //
+  // TODO: It is a bit unfortunate to use a PointerUnion inside the VarDecl for
+  // this as *many* VarDecls are ParmVarDecls that don't have default
+  // arguments. We could save some space by moving this pointer union to be
+  // allocated in trailing space when necessary.
+  typedef llvm::PointerUnion<Stmt *, EvaluatedStmt *> InitType;
 
   /// \brief The initializer for this variable or, for a ParmVarDecl, the
   /// C++ default argument.
@@ -763,6 +760,13 @@ private:
 protected:
   enum { NumParameterIndexBits = 8 };
 
+  enum DefaultArgKind {
+    DAK_None,
+    DAK_Unparsed,
+    DAK_Uninstantiated,
+    DAK_Normal
+  };
+
   class ParmVarDeclBitfields {
     friend class ParmVarDecl;
     friend class ASTDeclReader;
@@ -773,6 +777,12 @@ protected:
     /// prior declaration.
     unsigned HasInheritedDefaultArg : 1;
 
+    /// Describes the kind of default argument for this parameter. By default
+    /// this is none. If this is normal, then the default argument is stored in
+    /// the \c VarDecl initalizer expression unless we were unble to parse
+    /// (even an invalid) expression for the default argument.
+    unsigned DefaultArgKind : 2;
+
     /// Whether this parameter undergoes K&R argument promotion.
     unsigned IsKNRPromoted : 1;
 
@@ -1066,47 +1076,14 @@ public:
   /// declaration it is attached to. Also get that declaration.
   const Expr *getAnyInitializer(const VarDecl *&D) const;
 
-  bool hasInit() const {
-    return !Init.isNull() && (Init.is<Stmt *>() || Init.is<EvaluatedStmt *>());
-  }
+  bool hasInit() const;
   const Expr *getInit() const {
-    if (Init.isNull())
-      return nullptr;
-
-    const Stmt *S = Init.dyn_cast<Stmt *>();
-    if (!S) {
-      if (EvaluatedStmt *ES = Init.dyn_cast<EvaluatedStmt*>())
-        S = ES->Value;
-    }
-    return (const Expr*) S;
-  }
-  Expr *getInit() {
-    if (Init.isNull())
-      return nullptr;
-
-    Stmt *S = Init.dyn_cast<Stmt *>();
-    if (!S) {
-      if (EvaluatedStmt *ES = Init.dyn_cast<EvaluatedStmt*>())
-        S = ES->Value;
-    }
-
-    return (Expr*) S;
+    return const_cast<VarDecl *>(this)->getInit();
   }
+  Expr *getInit();
 
   /// \brief Retrieve the address of the initializer expression.
-  Stmt **getInitAddress() {
-    if (EvaluatedStmt *ES = Init.dyn_cast<EvaluatedStmt*>())
-      return &ES->Value;
-
-    // This union hack tip-toes around strict-aliasing rules.
-    union {
-      InitType *InitPtr;
-      Stmt **StmtPtr;
-    };
-
-    InitPtr = &Init;
-    return StmtPtr;
-  }
+  Stmt **getInitAddress();
 
   void setInit(Expr *I);
 
@@ -1128,33 +1105,18 @@ public:
   /// \brief Return the already-evaluated value of this variable's
   /// initializer, or NULL if the value is not yet known. Returns pointer
   /// to untyped APValue if the value could not be evaluated.
-  APValue *getEvaluatedValue() const {
-    if (EvaluatedStmt *Eval = Init.dyn_cast<EvaluatedStmt *>())
-      if (Eval->WasEvaluated)
-        return &Eval->Evaluated;
-
-    return nullptr;
-  }
+  APValue *getEvaluatedValue() const;
 
   /// \brief Determines whether it is already known whether the
   /// initializer is an integral constant expression or not.
-  bool isInitKnownICE() const {
-    if (EvaluatedStmt *Eval = Init.dyn_cast<EvaluatedStmt *>())
-      return Eval->CheckedICE;
-
-    return false;
-  }
+  bool isInitKnownICE() const;
 
   /// \brief Determines whether the initializer is an integral constant
   /// expression, or in C++11, whether the initializer is a constant
   /// expression.
   ///
   /// \pre isInitKnownICE()
-  bool isInitICE() const {
-    assert(isInitKnownICE() &&
-           "Check whether we already know that the initializer is an ICE");
-    return Init.get<EvaluatedStmt *>()->IsICE;
-  }
+  bool isInitICE() const;
 
   /// \brief Determine whether the value of the initializer attached to this
   /// declaration is an integral constant expression.
@@ -1355,6 +1317,7 @@ protected:
               TypeSourceInfo *TInfo, StorageClass S, Expr *DefArg)
       : VarDecl(DK, C, DC, StartLoc, IdLoc, Id, T, TInfo, S) {
     assert(ParmVarDeclBits.HasInheritedDefaultArg == false);
+    assert(ParmVarDeclBits.DefaultArgKind == DAK_None);
     assert(ParmVarDeclBits.IsKNRPromoted == false);
     assert(ParmVarDeclBits.IsObjCMethodParam == false);
     setDefaultArg(DefArg);
@@ -1429,29 +1392,20 @@ public:
     return const_cast<ParmVarDecl *>(this)->getDefaultArg();
   }
 
-  void setDefaultArg(Expr *defarg) {
-    Init = reinterpret_cast<Stmt *>(defarg);
-  }
+  void setDefaultArg(Expr *defarg);
 
   /// \brief Retrieve the source range that covers the entire default
   /// argument.
   SourceRange getDefaultArgRange() const;
-  void setUninstantiatedDefaultArg(Expr *arg) {
-    Init = reinterpret_cast<UninstantiatedDefaultArgument *>(arg);
-  }
-  Expr *getUninstantiatedDefaultArg() {
-    return (Expr *)Init.get<UninstantiatedDefaultArgument *>();
-  }
+  void setUninstantiatedDefaultArg(Expr *arg);
+  Expr *getUninstantiatedDefaultArg();
   const Expr *getUninstantiatedDefaultArg() const {
-    return (const Expr *)Init.get<UninstantiatedDefaultArgument *>();
+    return const_cast<ParmVarDecl *>(this)->getUninstantiatedDefaultArg();
   }
 
   /// hasDefaultArg - Determines whether this parameter has a default argument,
   /// either parsed or not.
-  bool hasDefaultArg() const {
-    return getInit() || hasUnparsedDefaultArg() ||
-      hasUninstantiatedDefaultArg();
-  }
+  bool hasDefaultArg() const;
 
   /// hasUnparsedDefaultArg - Determines whether this parameter has a
   /// default argument that has not yet been parsed. This will occur
@@ -1464,11 +1418,11 @@ public:
   ///   }; // x has a regular default argument now
   /// @endcode
   bool hasUnparsedDefaultArg() const {
-    return Init.is<UnparsedDefaultArgument*>();
+    return ParmVarDeclBits.DefaultArgKind == DAK_Unparsed;
   }
 
   bool hasUninstantiatedDefaultArg() const {
-    return Init.is<UninstantiatedDefaultArgument*>();
+    return ParmVarDeclBits.DefaultArgKind == DAK_Uninstantiated;
   }
 
   /// setUnparsedDefaultArg - Specify that this parameter has an
@@ -1476,7 +1430,9 @@ public:
   /// real default argument via setDefaultArg when the class
   /// definition enclosing the function declaration that owns this
   /// default argument is completed.
-  void setUnparsedDefaultArg() { Init = (UnparsedDefaultArgument *)nullptr; }
+  void setUnparsedDefaultArg() {
+    ParmVarDeclBits.DefaultArgKind = DAK_Unparsed;
+  }
 
   bool hasInheritedDefaultArg() const {
     return ParmVarDeclBits.HasInheritedDefaultArg;

Modified: cfe/trunk/lib/AST/Decl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=256609&r1=256608&r2=256609&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Tue Dec 29 20:51:00 2015
@@ -2031,6 +2031,31 @@ const Expr *VarDecl::getAnyInitializer(c
   return nullptr;
 }
 
+bool VarDecl::hasInit() const {
+  if (auto *P = dyn_cast<ParmVarDecl>(this))
+    if (P->hasUnparsedDefaultArg() || P->hasUninstantiatedDefaultArg())
+      return false;
+
+  return !Init.isNull();
+}
+
+Expr *VarDecl::getInit() {
+  if (!hasInit())
+    return nullptr;
+
+  if (auto *S = Init.dyn_cast<Stmt *>())
+    return cast<Expr>(S);
+
+  return cast_or_null<Expr>(Init.get<EvaluatedStmt *>()->Value);
+}
+
+Stmt **VarDecl::getInitAddress() {
+  if (auto *ES = Init.dyn_cast<EvaluatedStmt *>())
+    return &ES->Value;
+
+  return Init.getAddrOfPtr1();
+}
+
 bool VarDecl::isOutOfLine() const {
   if (Decl::isOutOfLine())
     return true;
@@ -2101,13 +2126,12 @@ bool VarDecl::isUsableInConstantExpressi
 EvaluatedStmt *VarDecl::ensureEvaluatedStmt() const {
   auto *Eval = Init.dyn_cast<EvaluatedStmt *>();
   if (!Eval) {
-    auto *S = Init.get<Stmt *>();
     // Note: EvaluatedStmt contains an APValue, which usually holds
     // resources not allocated from the ASTContext.  We need to do some
     // work to avoid leaking those, but we do so in VarDecl::evaluateValue
     // where we can detect whether there's anything to clean up or not.
     Eval = new (getASTContext()) EvaluatedStmt;
-    Eval->Value = S;
+    Eval->Value = Init.get<Stmt *>();
     Init = Eval;
   }
   return Eval;
@@ -2171,6 +2195,27 @@ APValue *VarDecl::evaluateValue(
   return Result ? &Eval->Evaluated : nullptr;
 }
 
+APValue *VarDecl::getEvaluatedValue() const {
+  if (EvaluatedStmt *Eval = Init.dyn_cast<EvaluatedStmt *>())
+    if (Eval->WasEvaluated)
+      return &Eval->Evaluated;
+
+  return nullptr;
+}
+
+bool VarDecl::isInitKnownICE() const {
+  if (EvaluatedStmt *Eval = Init.dyn_cast<EvaluatedStmt *>())
+    return Eval->CheckedICE;
+
+  return false;
+}
+
+bool VarDecl::isInitICE() const {
+  assert(isInitKnownICE() &&
+         "Check whether we already know that the initializer is an ICE");
+  return Init.get<EvaluatedStmt *>()->IsICE;
+}
+
 bool VarDecl::checkInitIsICE() const {
   // Initializers of weak variables are never ICEs.
   if (isWeak())
@@ -2336,14 +2381,48 @@ Expr *ParmVarDecl::getDefaultArg() {
   return Arg;
 }
 
+void ParmVarDecl::setDefaultArg(Expr *defarg) {
+  ParmVarDeclBits.DefaultArgKind = DAK_Normal;
+  Init = defarg;
+}
+
 SourceRange ParmVarDecl::getDefaultArgRange() const {
-  if (const Expr *E = getInit())
-    return E->getSourceRange();
+  switch (ParmVarDeclBits.DefaultArgKind) {
+  case DAK_None:
+  case DAK_Unparsed:
+    // Nothing we can do here.
+    return SourceRange();
 
-  if (hasUninstantiatedDefaultArg())
+  case DAK_Uninstantiated:
     return getUninstantiatedDefaultArg()->getSourceRange();
 
-  return SourceRange();
+  case DAK_Normal:
+    if (const Expr *E = getInit())
+      return E->getSourceRange();
+
+    // Missing an actual expression, may be invalid.
+    return SourceRange();
+  }
+  llvm_unreachable("Invalid default argument kind.");
+}
+
+void ParmVarDecl::setUninstantiatedDefaultArg(Expr *arg) {
+  ParmVarDeclBits.DefaultArgKind = DAK_Uninstantiated;
+  Init = arg;
+}
+
+Expr *ParmVarDecl::getUninstantiatedDefaultArg() {
+  assert(hasUninstantiatedDefaultArg() &&
+         "Wrong kind of initialization expression!");
+  return cast_or_null<Expr>(Init.get<Stmt *>());
+}
+
+bool ParmVarDecl::hasDefaultArg() const {
+  // FIXME: We should just return false for DAK_None here once callers are
+  // prepared for the case that we encountered an invalid default argument and
+  // were unable to even build an invalid expression.
+  return hasUnparsedDefaultArg() || hasUninstantiatedDefaultArg() ||
+         !Init.isNull();
 }
 
 bool ParmVarDecl::isParameterPack() const {

Modified: cfe/trunk/lib/AST/StmtIterator.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/StmtIterator.cpp?rev=256609&r1=256608&r2=256609&view=diff
==============================================================================
--- cfe/trunk/lib/AST/StmtIterator.cpp (original)
+++ cfe/trunk/lib/AST/StmtIterator.cpp Tue Dec 29 20:51:00 2015
@@ -42,7 +42,7 @@ void StmtIteratorBase::NextVA() {
 
   if (inDeclGroup()) {
     if (VarDecl* VD = dyn_cast<VarDecl>(*DGI))
-      if (VD->Init)
+      if (VD->hasInit())
         return;
 
     NextDecl();




More information about the cfe-commits mailing list