Add support for _Return_value expressions (#544)

A_Return_value expression allows programmers to name the value returned by a function in a return bounds declaration.  This changes extends the clang IR to represent _Return_value expressions and _Current_expr_value expressions. It also adds lexing, parsing, and type system support for _Return_value expressions.   This addresses the first part of issue #205.

We create a new expression class BoundsValueExpr to represent these expressions.  This class is modelled on CXXThisExpr.   Constructing a_Return_value expression requires knowing the return type of the function that the return bounds is associated with.  We must have the _Return_value expression constructed before we can build the function type, since function types incorporate the return bounds expressions.  Unfortunately, we don't know the function return type until the middle of GetFullTypeForDeclarator in SemaType.cpp.  Because clang intertwines AST building, type checking, and other static semantic checking, we have to "defer parse" return bounds expressions.   "Defer parse" means that the compiler recognize the tokens that make up a return bounds expression, saves them, and parses them later to build the IR for the return bounds expressions.

To allow parsing of the return bounds to happen in the middle of SemaType.cpp, we introduce a callback from semantic checking to the parser.  This lets us preserve the interface between the two.  We have to bring the parameters into scope, since they've already gone out of scope by the time GetFullTypeForDeclarator runs.   We aren't the first people to have to do this.  Other parsing phases such as parsing of attributes have to do this too.

After parsing the return bounds expression, we attach it to the declarator whose type is being constructed.    This lets us get the line number information right for error messages involving redeclarations with conflicting return bounds.  (Long explanation: memoization of function types also memoizes bounds expressions embedded in types.  It does this by ignoring line numbers and abstracting function parameters to parameter locations.   The end result is that line numbers for expressions embedded in types are meaningless.  Clang has a complicated mechanism for tracking source line information for types called TypeSourceInfo, but we haven't implemented support for embedded expressions in types and are not likely to anytime soon.

This change also contains some unrelated clean up:
1. Fix the line ending for the test case for bug526 to be line feeds.
2. Delete identifiers for _Dynamic_bounds_cast and _Assume_bounds_cast.  These are keywords, so we don't need identifiers.

Testing:
-  There will be a separate pull request to the Checked C repo for parsing and typechecking tests for_Return_value.
- Added clang tests for AST dumping and checking of bounds declarations.  The checking of bounds declarations involving _Return_value doesn't work yet, so the tests document this.
- Passes automated Checked C and clang testing with these changes.

Author: dtarditi

include/clang/AST/CanonBounds.h

@@ -99,6 +99,7 @@ namespace clang {
     Result CompareImpl(const PositionalParameterExpr *E1,
                    const PositionalParameterExpr *E2);
     Result CompareImpl(const BoundsCastExpr *E1, const BoundsCastExpr *E2);
+    Result CompareImpl(const BoundsValueExpr *E1, const BoundsValueExpr *E2);
     Result CompareImpl(const AtomicExpr *E1, const AtomicExpr *E2);
     Result CompareImpl(const BlockExpr *E1, const BlockExpr *E2);
 

include/clang/AST/Expr.h

@@ -5441,6 +5441,45 @@ class PositionalParameterExpr : public Expr {
     }
 };
 
+// \brief Represents the \c _Currrent_expr_value and _Return_value expressions
+// in Checked C.  These expressions can be used within bounds expressions.
+class BoundsValueExpr : public Expr {
+public:
+  enum Kind {
+    Current,
+    Return
+  };
+
+private:
+  SourceLocation Loc;
+  Kind ValueExprKind;
+
+public:
+  BoundsValueExpr(SourceLocation L, QualType Type, Kind K)
+    : Expr(BoundsValueExprClass, Type, VK_RValue, OK_Ordinary,
+           false, false, false, false), Loc(L), ValueExprKind(K) { }
+
+  BoundsValueExpr(EmptyShell Empty) : Expr(BoundsValueExprClass, Empty) {}
+
+  SourceLocation getLocation() const { return Loc; }
+  void setLocation(SourceLocation L) { Loc = L; }
+
+  SourceLocation getLocStart() const LLVM_READONLY { return Loc; }
+  SourceLocation getLocEnd() const LLVM_READONLY { return Loc; }
+
+  Kind getKind() const { return ValueExprKind; }
+  void setKind(Kind K) { ValueExprKind = K; }
+
+  static bool classof(const Stmt *T) {
+    return T->getStmtClass() == BoundsValueExprClass;
+  }
+
+  // Iterators
+  child_range children() {
+    return child_range(child_iterator(), child_iterator());
+  }
+};
+
 
 //===----------------------------------------------------------------------===//
 // Clang Extensions

include/clang/AST/RecursiveASTVisitor.h

@@ -2555,6 +2555,7 @@ DEF_TRAVERSE_STMT(NullaryBoundsExpr, {})
 DEF_TRAVERSE_STMT(RangeBoundsExpr, {})
 DEF_TRAVERSE_STMT(InteropTypeExpr, {})
 DEF_TRAVERSE_STMT(PositionalParameterExpr, {})
+DEF_TRAVERSE_STMT(BoundsValueExpr, {})
 
 // For coroutines expressions, traverse either the operand
 // as written or the implied calls, depending on what the

include/clang/Basic/DiagnosticSemaKinds.td

@@ -9548,6 +9548,9 @@ def err_bounds_type_annotation_lost_checking : Error<
     "out-of-scope variable for bounds in a function type (a function type can "
     "only reference parameters from its own parameter list)">;
 
+  def err_return_value_not_in_scope : Error<
+    "_Return_value can be used only in a return bounds expression">;
+
   def err_expected_bounds : Error<
     "expression has unknown bounds">;
 

include/clang/Basic/StmtNodes.td

@@ -183,6 +183,7 @@ def RangeBoundsExpr : DStmt<BoundsExpr>;
 def InteropTypeExpr : DStmt<Expr>;
 def PositionalParameterExpr : DStmt<Expr>;
 def BoundsCastExpr : DStmt<ExplicitCastExpr>;
+def BoundsValueExpr : DStmt<Expr>;
 
 // CUDA Expressions.
 def CUDAKernelCallExpr : DStmt<CallExpr>;

include/clang/Basic/TokenKinds.def

@@ -654,6 +654,8 @@ KEYWORD(_Assume_bounds_cast        , KEYCHECKEDC)
 KEYWORD(_Dynamic_bounds_cast       , KEYCHECKEDC)
 KEYWORD(_For_any                   , KEYCHECKEDC)
 KEYWORD(_Itype_for_any             , KEYCHECKEDC)
+KEYWORD(_Current_expr_value        , KEYCHECKEDC)
+KEYWORD(_Return_value              , KEYCHECKEDC)
 
 // Borland Extensions which should be disabled in strict conformance mode.
 ALIAS("_pascal"      , __pascal   , KEYBORLAND)

include/clang/Parse/Parser.h

@@ -179,12 +179,6 @@ class Parser : public CodeCompletionHandler {
   /// \brief Identifier for "rel_align_value"
   IdentifierInfo *Ident_rel_align_value;
 
-  /// \brief Identifier for "dynamic_bounds_cast"
-  IdentifierInfo *Ident_dynamic_bounds_cast;
-  
-  /// \brief Identifier for "assume_bounds_cast"
-  IdentifierInfo *Ident_assume_bounds_cast;
-
   enum CheckedScopeKind {
     /// '{'
     CSK_None,
@@ -1780,6 +1774,18 @@ class Parser : public CodeCompletionHandler {
                                      BoundsAnnotations &Result,
                                      const Declarator &D);
 
+  // Delay parse a return bounds expression in Toks.  Used to parse return
+  // bounds after the return type has been constructed.  Stores the bounds
+  // expression in Result.  Returns true if there was a parsing error.
+  static bool ParseBoundsCallback(void *P,
+                                  std::unique_ptr<CachedTokens> Toks,
+                                  ArrayRef<ParmVarDecl *> Params,
+                                  BoundsAnnotations &Result,
+                                  const Declarator &D);
+
+  ExprResult ParseReturnValueExpression();
+
+
   //===--------------------------------------------------------------------===//
   // clang Expressions
 

include/clang/Sema/DeclSpec.h

@@ -1394,11 +1394,11 @@ struct DeclaratorChunk {
     ParamInfo *Params;
 
     /// The annotations for the value returned by the function.  We store them
-    // as individual fields, not BoundsAnnotations, because BoundsAnnotations has
-    // a default constructor.  This is for good reason. For equally good reasons,
-    // DeclaratorChunk doesn't have a default constructor.  This means that using
-    // a default initialized instance of BoundsAnnotations is not allowed here.
-    BoundsExpr *ReturnBounds;
+    // as individual fields because the return bounds are deferred-parsed.
+    // Note: ReturnBounds is actually a unique_ptr. However unique_ptr requires
+    // a constructor and this struct can't have one, so we cast it to a
+    // a regular pointer type.
+    CachedTokens *ReturnBounds;
     InteropTypeExpr *ReturnInteropType;
 
     union {
@@ -1547,11 +1547,6 @@ struct DeclaratorChunk {
 
     /// \brief Get the trailing-return-type for this function declarator.
     ParsedType getTrailingReturnType() const { return TrailingReturnType; }
-
-    /// \brief The bounds annotations for the return value
-    BoundsAnnotations getReturnAnnots() const {
-      return BoundsAnnotations(ReturnBounds, ReturnInteropType);
-    }
   };
 
   struct BlockPointerTypeInfo : TypeInfoCommon {
@@ -1697,7 +1692,8 @@ struct DeclaratorChunk {
                                      SourceLocation LocalRangeBegin,
                                      SourceLocation LocalRangeEnd,
                                      SourceLocation ReturnAnnotsColonLoc,
-                                     BoundsAnnotations &ReturnAnnotsExpr,
+                                     InteropTypeExpr *ReturnInteorpTypeExpr,
+                                     std::unique_ptr<CachedTokens> ReturnBounds,
                                      Declarator &TheDeclarator,
                                      TypeResult TrailingReturnType =
                                                     TypeResult());
@@ -1906,6 +1902,8 @@ class Declarator {
   /// \brief The asm label, if specified.
   Expr *AsmLabel;
 
+  /// \brief The return bounds for a function declarator.
+  BoundsExpr *ReturnBounds;
 #ifndef _MSC_VER
   union {
 #endif
@@ -1935,7 +1933,8 @@ class Declarator {
         GroupingParens(false), FunctionDefinition(FDK_Declaration),
         Redeclaration(false), Extension(false), ObjCIvar(false),
         ObjCWeakProperty(false), InlineStorageUsed(false),
-        Attrs(ds.getAttributePool().getFactory()), AsmLabel(nullptr) {}
+        Attrs(ds.getAttributePool().getFactory()), AsmLabel(nullptr),
+        ReturnBounds(nullptr) {}
 
   ~Declarator() {
     clear();
@@ -2500,6 +2499,9 @@ class Declarator {
   void setAsmLabel(Expr *E) { AsmLabel = E; }
   Expr *getAsmLabel() const { return AsmLabel; }
 
+  void setReturnBounds(BoundsExpr *E) { ReturnBounds = E; }
+  BoundsExpr *getReturnBounds() const { return ReturnBounds; }
+
   void setExtension(bool Val = true) { Extension = Val; }
   bool getExtension() const { return Extension; }
 

include/clang/Sema/Sema.h

@@ -2344,6 +2344,9 @@ class Sema {
   void ActOnReenterFunctionContext(Scope* S, Decl* D);
   void ActOnExitFunctionContext();
 
+  /// Push the parameters listed in Params into scope.
+  void ActOnSetupParametersAgain(Scope* S, ArrayRef<ParmVarDecl *> Params);
+
   DeclContext *getFunctionLevelDeclContext();
 
   /// getCurFunctionDecl - If inside of a function body, this returns a pointer
@@ -4666,6 +4669,45 @@ class Sema {
   // \#pragma CHECKED_SCOPE.
   void ActOnPragmaCheckedScope(Scope *S, tok::OnOffSwitch OOS);
 
+  BoundsExpr *CreateInvalidBoundsExpr();
+  /// /brief Synthesize the interop type expression implied by the presence
+  /// of a bounds expression.  Ty is the original unchecked type.  Returns null
+  /// if none exists.
+  InteropTypeExpr *SynthesizeInteropTypeExpr(QualType Ty, bool IsParam);
+  BoundsExpr *CreateCountForArrayType(QualType QT);
+
+  // _Return_value in Checked C bounds expressions.
+  ExprResult ActOnReturnValueExpr(SourceLocation Loc);
+
+  /// \brief When non-NULL, the type of the '_Return_value' expression.
+  QualType BoundsExprReturnValue;
+
+  /// \brief RAII object used to temporarily set the the type of _Return_value
+  class CheckedCReturnValueRAII {
+    Sema &S;
+    QualType OldReturnValue;
+  public:
+    CheckedCReturnValueRAII(Sema &S, QualType ReturnVal) : S(S) {
+      OldReturnValue = S.BoundsExprReturnValue;
+      S.BoundsExprReturnValue = ReturnVal;
+    }
+
+    ~CheckedCReturnValueRAII() {
+      S.BoundsExprReturnValue = OldReturnValue;
+    }
+  };
+
+  typedef bool
+  (*ParseDeferredBoundsCallBackFn)(void *P,
+                                   std::unique_ptr<CachedTokens> Toks,
+                                   ArrayRef<ParmVarDecl *> Params,
+                                   BoundsAnnotations &Result,
+                                   const Declarator &D);
+  void SetDeferredBoundsCallBack(void *OpaqueData, ParseDeferredBoundsCallBackFn p);
+
+  ParseDeferredBoundsCallBackFn DeferredBoundsParser;
+  void *DeferredBoundsParserData;
+
   // Represents the context where an expression must be non-modifying.
   enum NonModifyingContext {
     NMC_Unknown,
@@ -4679,13 +4721,6 @@ class Sema {
                                  // parameter bounds.
   };
 
-  BoundsExpr *CreateInvalidBoundsExpr();
-  /// /brief Synthesize the interop type expression implied by the presence
-  /// of a bounds expression.  Ty is the original unchecked type.  Returns null
-  /// if none exists.
-  InteropTypeExpr *SynthesizeInteropTypeExpr(QualType Ty, bool IsParam);
-  BoundsExpr *CreateCountForArrayType(QualType QT);
-
   /// /brief Checks whether an expression is non-modifying
   /// (see Checked C Spec, 3.6.1).  Returns true if the expression is non-modifying,
   /// false otherwise.

include/clang/Serialization/ASTBitCodes.h

@@ -1495,6 +1495,7 @@ namespace clang {
       EXPR_INTEROPTYPE_BOUNDS_ANNOTATION,// InteropTypeBoundsAnnotation
       EXPR_POSITIONAL_PARAMETER_EXPR, // PositionalParameterExpr
       EXPR_BOUNDS_CAST,
+      EXPR_BOUNDS_VALUE_EXPR,      // BoundsValueExpr
       // OpenCL
       EXPR_ASTYPE,                 // AsTypeExpr
 

lib/AST/ASTDumper.cpp

@@ -619,6 +619,7 @@ namespace  {
     void dumpBoundsCheckKind(BoundsCheckKind kind);
     void VisitInteropTypeExpr(const InteropTypeExpr *Node);
     void VisitPositionalParameterExpr(const PositionalParameterExpr *Node);
+    void VisitBoundsValueExpr(const BoundsValueExpr *Node);
   };
 }
 
@@ -2844,6 +2845,12 @@ void ASTDumper::VisitPositionalParameterExpr(
   OS << Node->getIndex();
 }
 
+void ASTDumper::VisitBoundsValueExpr(const BoundsValueExpr *Node) {
+  VisitExpr(Node);
+  OS << (Node->getKind() == BoundsValueExpr::Kind::Current ?
+         " _Current_expr_value" : " _Return_value");
+}
+
 //===----------------------------------------------------------------------===//
 // Type method implementations
 //===----------------------------------------------------------------------===//

lib/AST/CanonBounds.cpp

@@ -382,6 +382,7 @@ Result Lexicographic::CompareExpr(const Expr *Arg1, const Expr *Arg2) {
      case Expr::InteropTypeExprClass: Cmp = Compare<InteropTypeExpr>(E1, E2); break;
      case Expr::PositionalParameterExprClass: Cmp = Compare<PositionalParameterExpr>(E1, E2); break;
      case Expr::BoundsCastExprClass: Cmp = Compare<BoundsCastExpr>(E1, E2); break;
+     case Expr::BoundsValueExprClass: Cmp = Compare<BoundsValueExpr>(E1, E2); break;
 
      // Clang extensions
      case Expr::ShuffleVectorExprClass: break;
@@ -744,6 +745,16 @@ Lexicographic::CompareImpl(const BoundsCastExpr *E1,
   return CompareType(E1->getType(), E2->getType());
 }
 
+Result
+Lexicographic::CompareImpl(const BoundsValueExpr *E1,
+                           const BoundsValueExpr *E2) {
+  Result Cmp = CompareInteger(E1->getKind(), E2->getKind());
+  if (Cmp != Result::Equal)
+    return Cmp;
+  return CompareType(E1->getType(), E2->getType());
+}
+
+
 Result
 Lexicographic::CompareImpl(const BlockExpr *E1, const BlockExpr *E2) {
   return Result::Equal;

lib/AST/Expr.cpp

@@ -3066,8 +3066,9 @@ bool Expr::HasSideEffects(const ASTContext &Ctx,
   case CountBoundsExprClass:
   case InteropTypeExprClass:
   case NullaryBoundsExprClass:
-  case PositionalParameterExprClass:
   case RangeBoundsExprClass:
+  case PositionalParameterExprClass:
+  case BoundsValueExprClass:
     // These never have a side-effect.
     return false;
 

lib/AST/ExprClassification.cpp

@@ -190,6 +190,7 @@ static Cl::Kinds ClassifyInternal(ASTContext &Ctx, const Expr *E) {
   case Expr::ArrayInitIndexExprClass:
   case Expr::NoInitExprClass:
   case Expr::DesignatedInitUpdateExprClass:
+  case Expr::BoundsValueExprClass:
     return Cl::CL_PRValue;
 
     // Next come the complicated cases.

lib/AST/ExprConstant.cpp

@@ -10345,6 +10345,7 @@ static ICEDiag CheckICE(const Expr* E, const ASTContext &Ctx) {
   case Expr::InteropTypeExprClass:
   case Expr::NullaryBoundsExprClass:
   case Expr::PositionalParameterExprClass:
+  case Expr::BoundsValueExprClass:
     // These are parameter variables and are never constants,
   case Expr::RangeBoundsExprClass:
     return ICEDiag(IK_NotICE, E->getLocStart());

lib/AST/ItaniumMangle.cpp

@@ -3478,6 +3478,7 @@ void CXXNameMangler::mangleExpression(const Expr *E, unsigned Arity) {
   case Expr::InteropTypeExprClass:
   case Expr::NullaryBoundsExprClass:
   case Expr::RangeBoundsExprClass:
+  case Expr::BoundsValueExprClass:
   {
     if (!NullOut) {
       // As bad as this diagnostic is, it's better than crashing.

lib/AST/StmtPrinter.cpp

@@ -2082,6 +2082,11 @@ void StmtPrinter::VisitPositionalParameterExpr(PositionalParameterExpr *E) {
   OS << (E->getIndex());
 }
 
+void StmtPrinter::VisitBoundsValueExpr(BoundsValueExpr *E) {
+  OS << ((E->getKind() == BoundsValueExpr::Current) ?
+         "_Current_expr_value" : "_Return_value");
+}
+
 // C++
 void StmtPrinter::VisitCXXOperatorCallExpr(CXXOperatorCallExpr *Node) {
   const char *OpStrings[NUM_OVERLOADED_OPERATORS] = {

lib/AST/StmtProfile.cpp

@@ -1261,6 +1261,13 @@ void StmtProfiler::VisitPositionalParameterExpr(
   ID.AddInteger(P->getIndex());
 }
 
+void StmtProfiler::VisitBoundsValueExpr(const BoundsValueExpr *S) {
+  VisitExpr(S);
+  VisitType(S->getType());
+  ID.AddInteger(S->getKind());
+}
+
+
 void StmtProfiler::VisitAtomicExpr(const AtomicExpr *S) {
   VisitExpr(S);
   ID.AddInteger(S->getOp());