From d85873e84dcdd40ab527e46f26e71b0afce8969f Mon Sep 17 00:00:00 2001 From: David Tarditi Date: Tue, 19 Jul 2016 17:20:11 -0700 Subject: [PATCH 1/2] Parse bounds declarations for function return values. This extends parsing of function declarators to parse bounds expressions for function return values. A return bounds expression is declared by following a parameter list with ':' bounds-expression. The return bounds information is propagated to the representation of function declarations in clang. It is not propagated yet to function types because there is no way to represent it in function types. I have opened issue #20 to track taht and added a few TODO's to the code code flagging where the information will need to be added. Testing: - Add feature tests for parsing return bounds. The tests are in in the file return_bounds.c, which will be committed separately to the checkedc Github repo. - Passes existing clang regression test baseline. --- include/clang/AST/Decl.h | 9 ++++++++- include/clang/Sema/DeclSpec.h | 16 ++++++++++++++++ include/clang/Sema/Sema.h | 1 + lib/Parse/ParseDecl.cpp | 21 ++++++++++++++++++++- lib/Parse/ParseExpr.cpp | 2 ++ lib/Parse/ParseExprCXX.cpp | 10 ++++++++-- lib/Sema/DeclSpec.cpp | 4 ++++ lib/Sema/SemaDecl.cpp | 22 +++++++++++++++++++--- lib/Sema/SemaType.cpp | 9 ++++++++- 9 files changed, 86 insertions(+), 8 deletions(-) diff --git a/include/clang/AST/Decl.h b/include/clang/AST/Decl.h index 05256ed36051..a152bfcff8a4 100644 --- a/include/clang/AST/Decl.h +++ b/include/clang/AST/Decl.h @@ -1584,6 +1584,8 @@ class FunctionDecl : public DeclaratorDecl, public DeclContext, /// 'enum Y' in 'void f(enum Y {AA} x) {}'. ArrayRef DeclsInPrototypeScope; + BoundsExpr *ReturnBoundsExpr; + LazyDeclStmtPtr Body; // FIXME: This can be packed into the bitfields in DeclContext. @@ -1687,7 +1689,7 @@ class FunctionDecl : public DeclaratorDecl, public DeclContext, StartLoc), DeclContext(DK), redeclarable_base(C), - ParamInfo(nullptr), Body(), + ParamInfo(nullptr), ReturnBoundsExpr(nullptr), Body(), SClass(S), IsInline(isInlineSpecified), IsInlineSpecified(isInlineSpecified), IsVirtualAsWritten(false), IsPure(false), HasInheritedPrototype(false), @@ -2065,6 +2067,11 @@ 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; } diff --git a/include/clang/Sema/DeclSpec.h b/include/clang/Sema/DeclSpec.h index 6163924a4380..44c2da9fddc3 100644 --- a/include/clang/Sema/DeclSpec.h +++ b/include/clang/Sema/DeclSpec.h @@ -1287,11 +1287,18 @@ struct DeclaratorChunk { /// \brief The end location of the exception specification, if any. unsigned ExceptionSpecLocEnd; + /// The location of the ':' for the return bounds expression in the source + unsigned ReturnBoundsColon; + /// Params - This is a pointer to a new[]'d array of ParamInfo objects that /// describe the parameters specified by this function declarator. null if /// there are no parameters specified. ParamInfo *Params; + /// The bounds for the value returned by the function. Null if there is + /// no bounds specified. + BoundsExpr *ReturnBounds; + union { /// \brief Pointer to a new[]'d array of TypeAndRange objects that /// contain the types in the function's dynamic exception specification @@ -1352,6 +1359,10 @@ struct DeclaratorChunk { return SourceLocation::getFromRawEncoding(RParenLoc); } + SourceLocation getReturnBoundsColon() const { + return SourceLocation::getFromRawEncoding(ReturnBoundsColon); + } + SourceLocation getExceptionSpecLocBeg() const { return SourceLocation::getFromRawEncoding(ExceptionSpecLocBeg); } @@ -1408,6 +1419,9 @@ struct DeclaratorChunk { /// \brief Get the trailing-return-type for this function declarator. ParsedType getTrailingReturnType() const { return TrailingReturnType; } + + /// \brief The bounds expression for the return value + BoundsExpr *getReturnBounds() const { return ReturnBounds; } }; struct BlockPointerTypeInfo : TypeInfoCommon { @@ -1552,6 +1566,8 @@ struct DeclaratorChunk { CachedTokens *ExceptionSpecTokens, SourceLocation LocalRangeBegin, SourceLocation LocalRangeEnd, + SourceLocation ReturnBoundsColonLoc, + BoundsExpr *ReturnBoundsExpr, Declarator &TheDeclarator, TypeResult TrailingReturnType = TypeResult()); diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index 609de62ac47c..632a6e7ac33b 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -4116,6 +4116,7 @@ class Sema { void ActOnBoundsExpr(VarDecl *D, BoundsExpr *Expr); void ActOnInvalidBoundsExpr(VarDecl *D); + BoundsExpr *CreateInvalidBoundsExpr(); //===---------------------------- Clang Extensions ----------------------===// diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index 555ae6fefab0..3a1b35bde91d 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -5586,6 +5586,10 @@ void Parser::ParseParenDeclarator(Declarator &D) { /// dynamic-exception-specification /// noexcept-specification /// +/// For Checked C, after the parameter-list, it also parses the optional return +/// bounds expression for the function declarator: +/// [Checked C] ':' bounds-expression +//// void Parser::ParseFunctionDeclarator(Declarator &D, ParsedAttributes &FirstArgAttrs, BalancedDelimiterTracker &Tracker, @@ -5626,6 +5630,8 @@ void Parser::ParseFunctionDeclarator(Declarator &D, SourceLocation LParenLoc, RParenLoc; LParenLoc = Tracker.getOpenLocation(); StartLoc = LParenLoc; + SourceLocation BoundsColonLoc; + BoundsExpr *ReturnBoundsExpr = nullptr; if (isFunctionDeclaratorIdentifierList()) { if (RequiresArg) @@ -5741,6 +5747,17 @@ void Parser::ParseFunctionDeclarator(Declarator &D, } } + if (getLangOpts().CheckedC && Tok.is(tok::colon)) { + BoundsColonLoc = Tok.getLocation(); + ConsumeToken(); + ExprResult BoundsExprResult = ParseBoundsExpression(); + if (BoundsExprResult.isInvalid()) { + ReturnBoundsExpr = Actions.CreateInvalidBoundsExpr(); + SkipUntil(tok::l_brace, SkipUntilFlags::StopAtSemi | SkipUntilFlags::StopBeforeMatch); + } else + ReturnBoundsExpr = cast(BoundsExprResult.get()); + } + // Remember that we parsed a function type, and remember the attributes. D.AddTypeInfo(DeclaratorChunk::getFunction(HasProto, IsAmbiguous, @@ -5760,7 +5777,9 @@ void Parser::ParseFunctionDeclarator(Declarator &D, NoexceptExpr.isUsable() ? NoexceptExpr.get() : nullptr, ExceptionSpecTokens, - StartLoc, LocalEndLoc, D, + StartLoc, LocalEndLoc, + BoundsColonLoc, ReturnBoundsExpr, + D, TrailingReturnType), FnAttrs, EndLoc); } diff --git a/lib/Parse/ParseExpr.cpp b/lib/Parse/ParseExpr.cpp index beabe8b850ce..33fd22fac209 100644 --- a/lib/Parse/ParseExpr.cpp +++ b/lib/Parse/ParseExpr.cpp @@ -2971,6 +2971,8 @@ ExprResult Parser::ParseBlockLiteralExpression() { /*NoexceptExpr=*/nullptr, /*ExceptionSpecTokens=*/nullptr, CaretLoc, CaretLoc, + /*ReturnBoundsColon=*/NoLoc, + /*ReturnBoundsExpr=*/nullptr, ParamInfo), attrs, CaretLoc); diff --git a/lib/Parse/ParseExprCXX.cpp b/lib/Parse/ParseExprCXX.cpp index 63ece81b949a..df2fe2343b6c 100644 --- a/lib/Parse/ParseExprCXX.cpp +++ b/lib/Parse/ParseExprCXX.cpp @@ -1221,7 +1221,10 @@ ExprResult Parser::ParseLambdaExpressionAfterIntroducer( NoexceptExpr.isUsable() ? NoexceptExpr.get() : nullptr, /*ExceptionSpecTokens*/nullptr, - LParenLoc, FunLocalRangeEnd, D, + LParenLoc, FunLocalRangeEnd, + /*ReturnBoundsColon=*/NoLoc, + /*ReturnBoundsExpr=*/nullptr, + D, TrailingReturnType), Attr, DeclEndLoc); } else if (Tok.isOneOf(tok::kw_mutable, tok::arrow, tok::kw___attribute, @@ -1290,7 +1293,10 @@ ExprResult Parser::ParseLambdaExpressionAfterIntroducer( /*NumExceptions=*/0, /*NoexceptExpr=*/nullptr, /*ExceptionSpecTokens=*/nullptr, - DeclLoc, DeclEndLoc, D, + DeclLoc, DeclEndLoc, + /*ReturnBoundsColon=*/NoLoc, + /*ReturnBoundsExpr=*/nullptr, + D, TrailingReturnType), Attr, DeclEndLoc); } diff --git a/lib/Sema/DeclSpec.cpp b/lib/Sema/DeclSpec.cpp index cbe19939807b..69c43633976d 100644 --- a/lib/Sema/DeclSpec.cpp +++ b/lib/Sema/DeclSpec.cpp @@ -175,6 +175,8 @@ DeclaratorChunk DeclaratorChunk::getFunction(bool hasProto, CachedTokens *ExceptionSpecTokens, SourceLocation LocalRangeBegin, SourceLocation LocalRangeEnd, + SourceLocation ReturnBoundsColonLoc, + BoundsExpr *ReturnBoundsExpr, Declarator &TheDeclarator, TypeResult TrailingReturnType) { assert(!(TypeQuals & DeclSpec::TQ_atomic) && @@ -210,6 +212,8 @@ DeclaratorChunk DeclaratorChunk::getFunction(bool hasProto, I.Fun.HasTrailingReturnType = TrailingReturnType.isUsable() || TrailingReturnType.isInvalid(); I.Fun.TrailingReturnType = TrailingReturnType.get(); + I.Fun.ReturnBoundsColon = ReturnBoundsColonLoc.getRawEncoding(); + I.Fun.ReturnBounds = ReturnBoundsExpr; assert(I.Fun.TypeQuals == TypeQuals && "bitfield overflow"); assert(I.Fun.ExceptionSpecType == ESpecType && "bitfield overflow"); diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index 1965265130bd..844ace62abd3 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -7973,6 +7973,14 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC, // Finally, we know we have the right number of parameters, install them. NewFD->setParams(Params); + // TODO: clang-checked-issue #20. Issue #20 is attaching bounds information + // to function types. After it is address, we need to make sure that bounds + // information is propgated to newFD from the FunctionPrototype instance. + if (D.isFunctionDeclarator()) { + DeclaratorChunk::FunctionTypeInfo &FTI = D.getFunctionTypeInfo(); + NewFD->setReturnBoundsExpr(FTI.getReturnBounds()); + } + // Find all anonymous symbols defined during the declaration of this function // and add to NewFD. This lets us track decls such 'enum Y' in: // @@ -10880,13 +10888,19 @@ void Sema::ActOnInvalidBoundsExpr(VarDecl *D) { if (!D) return; + BoundsExpr *InvalidExpr = CreateInvalidBoundsExpr(); + D->setBoundsExpr(InvalidExpr); +} + +BoundsExpr *Sema::CreateInvalidBoundsExpr() { ExprResult Result = ActOnNullaryBoundsExpr(SourceLocation(), NullaryBoundsExpr::Kind::Invalid, SourceLocation()); - if (!Result.isInvalid()) - D->setBoundsExpr(cast(Result.get())); + return cast(Result.get()); + else + return nullptr; } Decl * @@ -11613,7 +11627,9 @@ NamedDecl *Sema::ImplicitlyDefineFunction(SourceLocation Loc, /*NumExceptions=*/0, /*NoexceptExpr=*/nullptr, /*ExceptionSpecTokens=*/nullptr, - Loc, Loc, D), + Loc, Loc, + /*ReturnBoundsColon=*/NoLoc, + /*ReturnBoundsExpr=*/nullptr, D), DS.getAttributes(), SourceLocation()); D.SetIdentifier(&II, Loc); diff --git a/lib/Sema/SemaType.cpp b/lib/Sema/SemaType.cpp index 6b9951bc842b..1c4604d0dac7 100644 --- a/lib/Sema/SemaType.cpp +++ b/lib/Sema/SemaType.cpp @@ -718,7 +718,10 @@ static void maybeSynthesizeBlockSignature(TypeProcessingState &state, /*NumExceptions=*/0, /*NoexceptExpr=*/nullptr, /*ExceptionSpecTokens=*/nullptr, - loc, loc, declarator)); + loc, loc, + /*ReturnBoundsColon=*/NoLoc, + /*ReturnBoundsExpr=*/nullptr, + declarator)); // For consistency, make sure the state still has us as processing // the decl spec. @@ -4238,6 +4241,10 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state, SmallVector ParamTys; ParamTys.reserve(FTI.NumParams); + // TODO: checkedc-clang issue #20: represent bounds information + // in function types. The code below will need to be updated + // to add the bounds information to the FunctionType. + SmallVector ExtParameterInfos(FTI.NumParams); bool HasAnyInterestingExtParameterInfos = false; From f65ff0e97d836456be177b13dab6a98bc6a802a5 Mon Sep 17 00:00:00 2001 From: David Tarditi Date: Fri, 22 Jul 2016 09:52:01 -0700 Subject: [PATCH 2/2] Address code review feedback and improve an error message. This change addresses code review feedback to rename some variables and add a comment. It also improves the error message for bounds(a,b) when a and b are omitted. We would get two error messages at the same location, one saying that an expression was expected and the other saying that a ',' was expected. The improvement is to print the error message for the ',' only if nothing went wrong trying to parse the lower bound. --- include/clang/Sema/DeclSpec.h | 6 +++--- lib/Parse/ParseDecl.cpp | 2 ++ lib/Parse/ParseExpr.cpp | 15 +++++++++++---- lib/Parse/ParseExprCXX.cpp | 4 ++-- lib/Sema/DeclSpec.cpp | 2 +- lib/Sema/SemaDecl.cpp | 2 +- 6 files changed, 20 insertions(+), 11 deletions(-) diff --git a/include/clang/Sema/DeclSpec.h b/include/clang/Sema/DeclSpec.h index 44c2da9fddc3..9673f2c40ccd 100644 --- a/include/clang/Sema/DeclSpec.h +++ b/include/clang/Sema/DeclSpec.h @@ -1288,7 +1288,7 @@ struct DeclaratorChunk { unsigned ExceptionSpecLocEnd; /// The location of the ':' for the return bounds expression in the source - unsigned ReturnBoundsColon; + unsigned ReturnBoundsColonLoc; /// Params - This is a pointer to a new[]'d array of ParamInfo objects that /// describe the parameters specified by this function declarator. null if @@ -1359,8 +1359,8 @@ struct DeclaratorChunk { return SourceLocation::getFromRawEncoding(RParenLoc); } - SourceLocation getReturnBoundsColon() const { - return SourceLocation::getFromRawEncoding(ReturnBoundsColon); + SourceLocation getReturnBoundsColonLoc() const { + return SourceLocation::getFromRawEncoding(ReturnBoundsColonLoc); } SourceLocation getExceptionSpecLocBeg() const { diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index 3a1b35bde91d..aeacdfd806b9 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -5747,6 +5747,8 @@ void Parser::ParseFunctionDeclarator(Declarator &D, } } + // Parse optional Checked C bounds expression with the form: + // ':' bounds-expression. if (getLangOpts().CheckedC && Tok.is(tok::colon)) { BoundsColonLoc = Tok.getLocation(); ConsumeToken(); diff --git a/lib/Parse/ParseExpr.cpp b/lib/Parse/ParseExpr.cpp index 33fd22fac209..c122cdfbf9c1 100644 --- a/lib/Parse/ParseExpr.cpp +++ b/lib/Parse/ParseExpr.cpp @@ -2795,10 +2795,8 @@ ExprResult Parser::ParseBoundsExpression() { ExprResult LowerBound = Actions.CorrectDelayedTyposInExpr(ParseAssignmentExpression()); - if (ExpectAndConsume(tok::comma)) - // We didn't find a comma, so don't try to parse the upper bounds expression. - Result = ExprError(); - else { + if (Tok.getKind() == tok::comma) { + ConsumeToken(); ExprResult UpperBound = Actions.CorrectDelayedTyposInExpr(ParseAssignmentExpression()); if (LowerBound.isInvalid() || UpperBound.isInvalid()) @@ -2807,6 +2805,15 @@ ExprResult Parser::ParseBoundsExpression() { Result = Actions.ActOnRangeBoundsExpr(BoundsKWLoc, LowerBound.get(), UpperBound.get(), Tok.getLocation()); + } else { + // We didn't find a comma. Only issue an error message if the + // LowerBound expression is valid. Otherwise, we already issued an + // error message when parsing the lower bound. Emitting an error + // message here could be spurious or confusing. + if (!LowerBound.isInvalid()) { + Diag(Tok, diag::err_expected) << tok::comma; + } + Result = ExprError(); } } // if (!FoundNullaryOperator) } else { diff --git a/lib/Parse/ParseExprCXX.cpp b/lib/Parse/ParseExprCXX.cpp index df2fe2343b6c..8f03b7197d57 100644 --- a/lib/Parse/ParseExprCXX.cpp +++ b/lib/Parse/ParseExprCXX.cpp @@ -1222,7 +1222,7 @@ ExprResult Parser::ParseLambdaExpressionAfterIntroducer( NoexceptExpr.get() : nullptr, /*ExceptionSpecTokens*/nullptr, LParenLoc, FunLocalRangeEnd, - /*ReturnBoundsColon=*/NoLoc, + /*ReturnBoundsColonLoc=*/NoLoc, /*ReturnBoundsExpr=*/nullptr, D, TrailingReturnType), @@ -1294,7 +1294,7 @@ ExprResult Parser::ParseLambdaExpressionAfterIntroducer( /*NoexceptExpr=*/nullptr, /*ExceptionSpecTokens=*/nullptr, DeclLoc, DeclEndLoc, - /*ReturnBoundsColon=*/NoLoc, + /*ReturnBoundsColonLoc=*/NoLoc, /*ReturnBoundsExpr=*/nullptr, D, TrailingReturnType), diff --git a/lib/Sema/DeclSpec.cpp b/lib/Sema/DeclSpec.cpp index 69c43633976d..ad9e4df588fc 100644 --- a/lib/Sema/DeclSpec.cpp +++ b/lib/Sema/DeclSpec.cpp @@ -212,7 +212,7 @@ DeclaratorChunk DeclaratorChunk::getFunction(bool hasProto, I.Fun.HasTrailingReturnType = TrailingReturnType.isUsable() || TrailingReturnType.isInvalid(); I.Fun.TrailingReturnType = TrailingReturnType.get(); - I.Fun.ReturnBoundsColon = ReturnBoundsColonLoc.getRawEncoding(); + I.Fun.ReturnBoundsColonLoc = ReturnBoundsColonLoc.getRawEncoding(); I.Fun.ReturnBounds = ReturnBoundsExpr; assert(I.Fun.TypeQuals == TypeQuals && "bitfield overflow"); diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index 844ace62abd3..3646b9e654cd 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -11628,7 +11628,7 @@ NamedDecl *Sema::ImplicitlyDefineFunction(SourceLocation Loc, /*NoexceptExpr=*/nullptr, /*ExceptionSpecTokens=*/nullptr, Loc, Loc, - /*ReturnBoundsColon=*/NoLoc, + /*ReturnBoundsColonLoc=*/NoLoc, /*ReturnBoundsExpr=*/nullptr, D), DS.getAttributes(), SourceLocation());