Parsing of member bounds declarations. (#23)

This change adds parsing of member bounds declarations.  Member bounds
declarations use member bounds expressions, which can only use other
members of the structure.  The changes include:
- IR: no change is needed to represent member bounds expressions. Because
  clang handles C++, it already can represent members directly in
  the IR for expressions.  Bounds expressions are a subclass of expressions.

  We need to attach bounds expressions to member declarations.  We already
  have bounds expressions on function declarations and variable declarations,
  which have the same base class as member declarations.  Instead of having
  a 3rd copy of the code, we factor the common code for representing bounds
  expressions and move it to the common base.

- Parsing: we modify the parsing of member declarators that are followed
  by ':' to parse either a bounds expression or a constant-expression
  that is a bit-field size. We check whether the first token after the
  ':' is an identifier that is contextual keyword for bounds expressions.

  The parsing of bounds expressions is deferred until all members of the
  structure have been seen.  This is so that member bounds expressions may
  refer to any member of the structure.   Constant expressions for bitfield
  sizes continue to be eagerly parsed.

 A unique_ptr is used for the pointer to the CachedTokens object.  This 
 makes it clear when the CachedTokens object is deallocated.  Clean up
 existing code for deferred parsing of bounds expressions to use
 unique_ptrs for CachedTokens as well.

  The decision to just look for a bounds expression keyword can lead
  to several error messages from clang if there is a spelling
  error in a contextual keyword.   Clang interprets the misspelled keyword
  as an implicit function declaration and issues unknown identifier errors
  for uses of members.

  For now, we'll live this because this specific case is a fairly
  low priority to address.  The type for the identifier in the declarator
  hasn't been built yet at the time that we make the parsing decision.
  We could move that up or defer parsing of constant expressions.  Either
  change seems likely orthogonal to this change.

- Semantic processing: member bounds expressions are restricted to accessing
only members.  We add a state flag to Sema that changes the lookup of C
identifiers during expression parsing to look up identifiers in the
member namespace instead of the ordinary namespace.  The state flag is
protected by an RAII class.

Testing:
- Wrote a new feature tests of parsing member bounds
  declaration.  These wil be checked in separately in the Checked C
  repo in parsing\member_bounds.c.

- Passes existing clang test baseline.

Author: dtarditi

include/clang/AST/Decl.h

@@ -664,9 +664,12 @@ class DeclaratorDecl : public ValueDecl {
   DeclaratorDecl(Kind DK, DeclContext *DC, SourceLocation L,
                  DeclarationName N, QualType T, TypeSourceInfo *TInfo,
                  SourceLocation StartL)
-    : ValueDecl(DK, DC, L, N, T), DeclInfo(TInfo), InnerLocStart(StartL) {
+    : ValueDecl(DK, DC, L, N, T), DeclInfo(TInfo), InnerLocStart(StartL),
+      Bounds(nullptr) {
   }
 
+  BoundsExpr *Bounds;
+
 public:
   TypeSourceInfo *getTypeSourceInfo() const {
     return hasExtInfo()
@@ -731,6 +734,28 @@ class DeclaratorDecl : public ValueDecl {
 
   friend class ASTDeclReader;
   friend class ASTDeclWriter;
+
+  // Checked C bounds information
+  // For function declarations, this is the return bounds of the function.
+
+  /// \brief Return true if this declaration has bounds declared for it.
+  bool hasBoundsExpr() const;
+
+  /// \brief The declared bounds for this declaration. For function
+  /// declarations, this is the return bounds of the function. Null if no
+  /// bounds have been declared.
+  const BoundsExpr *getBoundsExpr() const {
+    return const_cast<DeclaratorDecl *>(this)->getBoundsExpr();
+  }
+
+  /// \brief The declared bounds for this declaration. For function
+  /// declarations, this is the return bounds of the function. Null if no
+  /// bounds have been declared.
+  BoundsExpr *getBoundsExpr();
+
+  /// \brief Set the declared bounds for this declaration. For function
+  /// declarations, this is the return bounds of the function.
+  void setBoundsExpr(BoundsExpr *E);
 };
 
 /// \brief Structure used to store a statement, the constant value to
@@ -801,15 +826,6 @@ class VarDecl : public DeclaratorDecl, public Redeclarable<VarDecl> {
   /// C++ default argument.
   mutable InitType Init;
 
-  // TODO: like the Init member above, it wastes space to have a pointer to a
-  // BoundsExpr in every VarDecl when many of them won't have bounds
-  // declarations. We could move the Init member and the Bounds to an
-  // "extension" object that is allocated on demand, at least not increasing
-  // the space usage of every single VarDecl.
-
-  /// \brief The bounds expression for the variable, if any.
-  BoundsExpr *Bounds;
-
 private:
   class VarDeclBitfields {
     friend class VarDecl;
@@ -1130,21 +1146,6 @@ class VarDecl : public DeclaratorDecl, public Redeclarable<VarDecl> {
     return false;
   }
 
-  /// \brief Return true if this variable has bounds declared for it.
-  bool hasBoundsExpr() const;
-
-  /// \brief The declared bounds for this variable.  Null if no
-  /// bounds have been declared.
-  const BoundsExpr *getBoundsExpr() const {
-    return const_cast<VarDecl *>(this)->getBoundsExpr();
-  }
-
-  /// \brief The declared bounds for this variable.  Null if no
-  /// bounds have been declared.
-  BoundsExpr *getBoundsExpr();
-  
-  void setBoundsExpr(BoundsExpr *E);
-
   /// getAnyInitializer - Get the initializer for this variable, no matter which
   /// declaration it is attached to.
   const Expr *getAnyInitializer() const {
@@ -1584,8 +1585,6 @@ class FunctionDecl : public DeclaratorDecl, public DeclContext,
   /// 'enum Y' in 'void f(enum Y {AA} x) {}'.
   ArrayRef<NamedDecl *> DeclsInPrototypeScope;
 
-  BoundsExpr *ReturnBoundsExpr;
-
   LazyDeclStmtPtr Body;
 
   // FIXME: This can be packed into the bitfields in DeclContext.
@@ -1689,7 +1688,7 @@ class FunctionDecl : public DeclaratorDecl, public DeclContext,
                      StartLoc),
       DeclContext(DK),
       redeclarable_base(C),
-      ParamInfo(nullptr), ReturnBoundsExpr(nullptr), Body(),
+      ParamInfo(nullptr), Body(),
       SClass(S),
       IsInline(isInlineSpecified), IsInlineSpecified(isInlineSpecified),
       IsVirtualAsWritten(false), IsPure(false), HasInheritedPrototype(false),
@@ -2067,11 +2066,6 @@ class FunctionDecl : public DeclaratorDecl, public DeclContext,
   /// computed linkage of symbol, see getLinkage.
   StorageClass getStorageClass() const { return StorageClass(SClass); }
 
-  /// \brief Returns the bounds expression for the return value of this function, if
-  /// any.
-  BoundsExpr *getReturnBoundsExpr() { return ReturnBoundsExpr; }
-  void setReturnBoundsExpr(BoundsExpr *E) { ReturnBoundsExpr = E; }
-
   /// \brief Determine whether the "inline" keyword was specified for this
   /// function.
   bool isInlineSpecified() const { return IsInlineSpecified; }

include/clang/Basic/DiagnosticSemaKinds.td

@@ -8430,4 +8430,8 @@ def err_checked_vla : Error<
 def err_checked_cplusplus : Error<
   "checked extension not supported for C++">;
 
+def err_undeclared_member_use : Error<"use of undeclared member %0">;
+
+def err_expected_bounds_expr_for_member : Error<
+  "expected bounds expression">;
 } // end of sema component.

include/clang/Parse/Parser.h

@@ -1632,9 +1632,12 @@ class Parser : public CodeCompletionHandler {
   //===--------------------------------------------------------------------===//
   // Checked C Expressions
 
+  /// \brief Return true if this token can start a bounds expression.
+  bool StartsBoundsExpression(Token &Tok);
   ExprResult ParseBoundsExpression();
   bool ConsumeAndStoreBoundsExpression(CachedTokens &Toks);
-  ExprResult DeferredParseBoundsExpression(CachedTokens *Toks);
+  ExprResult DeferredParseBoundsExpression(std::unique_ptr<CachedTokens> Toks);
+
 
   //===--------------------------------------------------------------------===//
   // clang Expressions
@@ -1895,7 +1898,7 @@ class Parser : public CodeCompletionHandler {
 
   void ParseStructDeclaration(
       ParsingDeclSpec &DS,
-      llvm::function_ref<void(ParsingFieldDeclarator &)> FieldsCallback);
+      llvm::function_ref<void(ParsingFieldDeclarator &, std::unique_ptr<CachedTokens>)> FieldsCallback);
 
   bool isDeclarationSpecifier(bool DisambiguatingWithExpression = false);
   bool isTypeSpecifierQualifier();

include/clang/Sema/Sema.h

@@ -420,6 +420,11 @@ class Sema {
   /// element type here is ExprWithCleanups::Object.
   SmallVector<BlockDecl*, 8> ExprCleanupObjects;
 
+  /// True if the current expression is a member bounds expression
+  /// for a structure.  Member bounds expressions can only reference
+  /// members and cannot reference variables.
+  bool IsMemberBoundsExpr;
+
   /// \brief Store a list of either DeclRefExprs or MemberExprs
   ///  that contain a reference to a variable (constant) that may or may not
   ///  be odr-used in this Expr, and we won't know until all lvalue-to-rvalue
@@ -1920,8 +1925,8 @@ class Sema {
   void ActOnDefs(Scope *S, Decl *TagD, SourceLocation DeclStart,
                  IdentifierInfo *ClassName,
                  SmallVectorImpl<Decl *> &Decls);
-  Decl *ActOnField(Scope *S, Decl *TagD, SourceLocation DeclStart,
-                   Declarator &D, Expr *BitfieldWidth);
+  FieldDecl *ActOnField(Scope *S, Decl *TagD, SourceLocation DeclStart,
+                        Declarator &D, Expr *BitfieldWidth);
 
   FieldDecl *HandleField(Scope *S, RecordDecl *TagD, SourceLocation DeclStart,
                          Declarator &D, Expr *BitfieldWidth,
@@ -4114,8 +4119,8 @@ class Sema {
   ExprResult ActOnRangeBoundsExpr(SourceLocation BoundsKWLoc, Expr *LowerBound,
                                   Expr *UpperBound, SourceLocation RParenLoc);
 
-  void ActOnBoundsExpr(VarDecl *D, BoundsExpr *Expr);
-  void ActOnInvalidBoundsExpr(VarDecl *D);
+  void ActOnBoundsExpr(DeclaratorDecl *D, BoundsExpr *Expr);
+  void ActOnInvalidBoundsExpr(DeclaratorDecl *D);
   BoundsExpr *CreateInvalidBoundsExpr();
 
   //===---------------------------- Clang Extensions ----------------------===//
@@ -9437,6 +9442,24 @@ class EnterExpressionEvaluationContext {
   }
 };
 
+/// \brief RAII object that handles state changes for processing a member
+// bounds expressions.
+class EnterMemberBoundsExprRAII {
+  Sema &S;
+  bool SavedMemberBounds;
+
+public:
+  EnterMemberBoundsExprRAII(Sema &S)
+    : S(S), SavedMemberBounds(S.IsMemberBoundsExpr)
+  {
+    S.IsMemberBoundsExpr = true;
+  }
+
+  ~EnterMemberBoundsExprRAII() {
+    S.IsMemberBoundsExpr = SavedMemberBounds;
+  }
+};
+
 DeductionFailureInfo
 MakeDeductionFailureInfo(ASTContext &Context, Sema::TemplateDeductionResult TDK,
                          sema::TemplateDeductionInfo &Info);

lib/AST/Decl.cpp

@@ -1764,6 +1764,20 @@ void QualifierInfo::setTemplateParameterListsInfo(
   }
 }
 
+// Checked C bounds information
+
+bool DeclaratorDecl::hasBoundsExpr() const {
+  return Bounds != nullptr;
+}
+
+BoundsExpr *DeclaratorDecl::getBoundsExpr() {
+  return Bounds;
+}
+
+void DeclaratorDecl::setBoundsExpr(BoundsExpr *E) {
+  Bounds = E;
+}
+
 //===----------------------------------------------------------------------===//
 // VarDecl Implementation
 //===----------------------------------------------------------------------===//
@@ -1795,7 +1809,6 @@ VarDecl::VarDecl(Kind DK, ASTContext &C, DeclContext *DC,
                 "NonParmVarDeclBitfields too large!");
   AllBits = 0;
   VarDeclBits.SClass = SC;
-  Bounds = nullptr;
   // Everything else is implicitly initialized to false.
 }
 
@@ -2010,20 +2023,6 @@ VarDecl *VarDecl::getDefinition(ASTContext &C) {
   return nullptr;
 }
 
-// Checked C bounds information
-
-bool VarDecl::hasBoundsExpr() const {
-  return Bounds != nullptr;
-}
-
-BoundsExpr *VarDecl::getBoundsExpr() {
-  return Bounds;
-}
-
-void VarDecl::setBoundsExpr(BoundsExpr *E) {
-  Bounds = E;
-}
-
 VarDecl::DefinitionKind VarDecl::hasDefinition(ASTContext &C) const {
   DefinitionKind Kind = DeclarationOnly;
 

lib/Parse/ParseDecl.cpp

@@ -3575,7 +3575,7 @@ void Parser::ParseDeclarationSpecifiers(DeclSpec &DS,
 ///
 void Parser::ParseStructDeclaration(
     ParsingDeclSpec &DS,
-    llvm::function_ref<void(ParsingFieldDeclarator &)> FieldsCallback) {
+    llvm::function_ref<void(ParsingFieldDeclarator &, std::unique_ptr<CachedTokens>)> FieldsCallback) {
 
   if (Tok.is(tok::kw___extension__)) {
     // __extension__ silences extension warnings in the subexpression.
@@ -3618,19 +3618,29 @@ void Parser::ParseStructDeclaration(
     } else
       DeclaratorInfo.D.SetIdentifier(nullptr, Tok.getLocation());
 
+    std::unique_ptr<CachedTokens> BoundsExprTokens;
     if (TryConsumeToken(tok::colon)) {
-      ExprResult Res(ParseConstantExpression());
-      if (Res.isInvalid())
-        SkipUntil(tok::semi, StopBeforeMatch);
-      else
-        DeclaratorInfo.BitfieldSize = Res.get();
+      if (getLangOpts().CheckedC && StartsBoundsExpression(Tok)) {
+         BoundsExprTokens.reset(new CachedTokens);
+         bool ParsingError = !ConsumeAndStoreBoundsExpression(*BoundsExprTokens);
+         if (ParsingError) {
+           SkipUntil(tok::semi, StopBeforeMatch);
+         }
+      } else {
+        ExprResult Res(ParseConstantExpression());
+        if (Res.isInvalid())
+          SkipUntil(tok::semi, StopBeforeMatch);
+        else
+          DeclaratorInfo.BitfieldSize = Res.get();
+      }
     }
 
     // If attributes exist after the declarator, parse them.
     MaybeParseGNUAttributes(DeclaratorInfo.D);
 
     // We're done with this declarator;  invoke the callback.
-    FieldsCallback(DeclaratorInfo);
+
+    FieldsCallback(DeclaratorInfo, std::move(BoundsExprTokens));
 
     // If we don't have a comma, it is either the end of the list (a ';')
     // or an error, bail out.
@@ -3666,6 +3676,16 @@ void Parser::ParseStructUnionBody(SourceLocation RecordLoc,
 
   SmallVector<Decl *, 32> FieldDecls;
 
+  // Delay parsing/semantic processing of member bounds expressions until after the
+  // member list is parsed. For each member with a bounds declaration,
+  // keep a list of tokens for the bounds expression.   Parsing/processing
+  // member bounds expressions is done in a scope that includes all
+  // the members in the member list.
+  typedef std::pair<FieldDecl *, std::unique_ptr<CachedTokens>> BoundsExprInfo;
+  // We are guessing that most structure declarations have 4 or fewer members
+  // with bounds expressions on them.
+  SmallVector<BoundsExprInfo, 4> deferredBoundsExpressions;
+
   // While we still have something to read, read the declarations in the struct.
   while (!tryParseMisplacedModuleImport() && Tok.isNot(tok::r_brace) &&
          Tok.isNot(tok::eof)) {
@@ -3703,14 +3723,17 @@ void Parser::ParseStructUnionBody(SourceLocation RecordLoc,
     }
 
     if (!Tok.is(tok::at)) {
-      auto CFieldCallback = [&](ParsingFieldDeclarator &FD) {
+      auto CFieldCallback = [&](ParsingFieldDeclarator &FD,
+                                std::unique_ptr<CachedTokens> BoundsExprTokens) {
         // Install the declarator into the current TagDecl.
-        Decl *Field =
-            Actions.ActOnField(getCurScope(), TagDecl,
-                               FD.D.getDeclSpec().getSourceRange().getBegin(),
-                               FD.D, FD.BitfieldSize);
+        FieldDecl *Field =
+          Actions.ActOnField(getCurScope(), TagDecl,
+                             FD.D.getDeclSpec().getSourceRange().getBegin(),
+                             FD.D, FD.BitfieldSize);
         FieldDecls.push_back(Field);
         FD.complete(Field);
+        if (BoundsExprTokens != nullptr)
+          deferredBoundsExpressions.emplace_back(Field, std::move(BoundsExprTokens));
       };
 
       // Parse all the comma separated declarators.
@@ -3763,6 +3786,17 @@ void Parser::ParseStructUnionBody(SourceLocation RecordLoc,
                       RecordLoc, TagDecl, FieldDecls,
                       T.getOpenLocation(), T.getCloseLocation(),
                       attrs.getList());
+  // Parse the deferred bounds expressions
+  for (auto &Pair : deferredBoundsExpressions) {
+    EnterMemberBoundsExprRAII MemberBoundsContext(Actions);
+    FieldDecl *FieldDecl = Pair.first;
+    std::unique_ptr<CachedTokens> Tokens = std::move(Pair.second);
+    ExprResult Bounds = DeferredParseBoundsExpression(std::move(Tokens));
+    if (Bounds.isInvalid())
+      Actions.ActOnInvalidBoundsExpr(FieldDecl);
+    else
+      Actions.ActOnBoundsExpr(FieldDecl, cast<BoundsExpr>(Bounds.get()));
+  }
   StructScope.Exit();
   Actions.ActOnTagFinishDefinition(getCurScope(), TagDecl,
                                    T.getCloseLocation());
@@ -5925,8 +5959,9 @@ void Parser::ParseParameterDeclarationClause(
   // keep a list of tokens for the bounds expression.   Parsing/checking
   // bounds expressions for parameters is done in a scope that includes all
   // the parameters in the parameter list.
-  typedef std::pair<ParmVarDecl *, CachedTokens *> BoundsExprInfo;
-  // We are guessing that most functions take 4 or fewer parameters.
+  typedef std::pair<ParmVarDecl *, std::unique_ptr<CachedTokens>> BoundsExprInfo;
+  // We are guessing that most functions take 4 or fewer parameters with
+  // bounds expressions on them.
   SmallVector<BoundsExprInfo, 4> deferredBoundsExpressions;
   do {
     // FIXME: Issue a diagnostic if we parsed an attribute-specifier-seq
@@ -6007,10 +6042,9 @@ void Parser::ParseParameterDeclarationClause(
         // error message.  This way the error messages from parsing of bounds
         // expressions will be the same or very similar regardless of whether
         // parsing is deferred or not.
-        // FIXME: Can we use a smart pointer for BoundsExprTokens?
-        CachedTokens *BoundsExprTokens = new CachedTokens;
+        std::unique_ptr<CachedTokens> BoundsExprTokens { new CachedTokens};
         bool ParsingError = !ConsumeAndStoreBoundsExpression(*BoundsExprTokens);
-        deferredBoundsExpressions.emplace_back(Param, BoundsExprTokens);
+        deferredBoundsExpressions.emplace_back(Param, std::move(BoundsExprTokens));
         if (ParsingError) {
           SkipUntil(tok::comma, tok::r_paren, StopAtSemi | StopBeforeMatch);
         }
@@ -6108,11 +6142,10 @@ void Parser::ParseParameterDeclarationClause(
   } while (TryConsumeToken(tok::comma));
 
   // Now parse the deferred bounds expressions
-  for (const auto &Pair : deferredBoundsExpressions) {
+  for (auto &Pair : deferredBoundsExpressions) {
     ParmVarDecl *Param = Pair.first;
-    CachedTokens *Tokens = Pair.second;
-    // DeferredParseBoundsExpression deletes Tokens
-    ExprResult Bounds = DeferredParseBoundsExpression(Tokens);
+    std::unique_ptr<CachedTokens> Tokens = std::move(Pair.second);
+    ExprResult Bounds = DeferredParseBoundsExpression(std::move(Tokens));
     if (Bounds.isInvalid())
       Actions.ActOnInvalidBoundsExpr(Param);
     else