Skip to content

Parsing of member bounds declarations. #23

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 2, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 27 additions & 33 deletions include/clang/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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 {
Copy link

@reubeno reubeno Jul 29, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getBoundsExpr [](start = 20, length = 13)

Does clang have a general standard for how to indicate that certain members in base classes may not be applicable to certain derived classes? Would it, say, be appropriate to add an assertion on setBoundsExpr() that it's only being set on an object on which it makes sense? #Resolved

Copy link
Member Author

@dtarditi dtarditi Aug 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are only 3 subclasses of DeclaratorDecl and it makes sense to set the bounds on all of them.
#WontFix

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't there is a standard, but I've seen asserts using dynamic types that guard against the misuse of members for derived classes.


In reply to: 72876100 [](ancestors = 72876100)

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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -1584,8 +1585,6 @@ class FunctionDecl : public DeclaratorDecl, public DeclContext,
/// 'enum Y' in 'void f(enum Y {AA} x) {}'.
ArrayRef<NamedDecl *> DeclsInPrototypeScope;

Copy link

@reubeno reubeno Jul 29, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ReturnBoundsExpr [](start = 14, length = 16)

I'm not sure where it would go, but it might be helpful to have a comment somewhere explain the intended meaning of 'Bounds' for FunctionDecls--that it's intended to describe the return value of the function. #Resolved

Copy link
Member Author

@dtarditi dtarditi Aug 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are already comments on the getter functions in the superclass of FunctionDecl. These will be picked up the automatic doc generation and by IDE tool tips (for IDEs that understand /brief comments). I've add a comment to the setter function too. #Resolved

BoundsExpr *ReturnBoundsExpr;

LazyDeclStmtPtr Body;

// FIXME: This can be packed into the bitfields in DeclContext.
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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; }
Expand Down
4 changes: 4 additions & 0 deletions include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -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.
7 changes: 5 additions & 2 deletions include/clang/Parse/Parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down
31 changes: 27 additions & 4 deletions include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 ----------------------===//
Expand Down Expand Up @@ -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);
Expand Down
29 changes: 14 additions & 15 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -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.
}

Expand Down Expand Up @@ -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;

Expand Down
75 changes: 54 additions & 21 deletions lib/Parse/ParseDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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
Expand Down
Loading