Hi rsmith,
Storing the locations of parentheses in the if/while/switch statements
makes it much more straightforward for the tools (e.g. clang-tidy checks) to
operate on these locations. On the other hand, this increases memory usage
somewhat. I'm not sure what the right tradeoff here is, but my gut feeling is
that memory becomes cheaper, while developers' time doesn't.
http://reviews.llvm.org/D5528
Files:
include/clang/AST/Stmt.h
include/clang/Parse/Parser.h
include/clang/Sema/Sema.h
lib/AST/Stmt.cpp
lib/Analysis/BodyFarm.cpp
lib/Parse/ParseStmt.cpp
lib/Sema/SemaStmt.cpp
lib/Sema/TreeTransform.h
lib/Serialization/ASTReaderStmt.cpp
lib/Serialization/ASTWriterStmt.cpp
Index: include/clang/AST/Stmt.h
===================================================================
--- include/clang/AST/Stmt.h
+++ include/clang/AST/Stmt.h
@@ -869,12 +869,13 @@
Stmt* SubExprs[END_EXPR];
SourceLocation IfLoc;
+ SourceLocation LParenLoc, RParenLoc;
SourceLocation ElseLoc;
public:
IfStmt(const ASTContext &C, SourceLocation IL, VarDecl *var, Expr *cond,
- Stmt *then, SourceLocation EL = SourceLocation(),
- Stmt *elsev = nullptr);
+ SourceLocation LP, SourceLocation RP, Stmt *then,
+ SourceLocation EL = SourceLocation(), Stmt *elsev = nullptr);
/// \brief Build an empty if/then/else statement
explicit IfStmt(EmptyShell Empty) : Stmt(IfStmtClass, Empty) { }
@@ -911,6 +912,10 @@
void setIfLoc(SourceLocation L) { IfLoc = L; }
SourceLocation getElseLoc() const { return ElseLoc; }
void setElseLoc(SourceLocation L) { ElseLoc = L; }
+ SourceLocation getLParenLoc() const { return LParenLoc; }
+ void setLParenLoc(SourceLocation L) { LParenLoc = L; }
+ SourceLocation getRParenLoc() const { return RParenLoc; }
+ void setRParenLoc(SourceLocation L) { RParenLoc = L; }
SourceLocation getLocStart() const LLVM_READONLY { return IfLoc; }
SourceLocation getLocEnd() const LLVM_READONLY {
@@ -939,14 +944,16 @@
// This points to a linked list of case and default statements.
SwitchCase *FirstCase;
SourceLocation SwitchLoc;
+ SourceLocation LParenLoc, RParenLoc;
/// If the SwitchStmt is a switch on an enum value, this records whether
/// all the enum values were covered by CaseStmts. This value is meant to
/// be a hint for possible clients.
unsigned AllEnumCasesCovered : 1;
public:
- SwitchStmt(const ASTContext &C, VarDecl *Var, Expr *cond);
+ SwitchStmt(const ASTContext &C, VarDecl *Var, Expr *cond, SourceLocation LP,
+ SourceLocation RP);
/// \brief Build a empty switch statement.
explicit SwitchStmt(EmptyShell Empty) : Stmt(SwitchStmtClass, Empty) { }
@@ -984,6 +991,10 @@
SourceLocation getSwitchLoc() const { return SwitchLoc; }
void setSwitchLoc(SourceLocation L) { SwitchLoc = L; }
+ SourceLocation getLParenLoc() const { return LParenLoc; }
+ void setLParenLoc(SourceLocation L) { LParenLoc = L; }
+ SourceLocation getRParenLoc() const { return RParenLoc; }
+ void setRParenLoc(SourceLocation L) { RParenLoc = L; }
void setBody(Stmt *S, SourceLocation SL) {
SubExprs[BODY] = S;
@@ -1030,9 +1041,10 @@
enum { VAR, COND, BODY, END_EXPR };
Stmt* SubExprs[END_EXPR];
SourceLocation WhileLoc;
+ SourceLocation LParenLoc, RParenLoc;
public:
WhileStmt(const ASTContext &C, VarDecl *Var, Expr *cond, Stmt *body,
- SourceLocation WL);
+ SourceLocation WL, SourceLocation LP, SourceLocation RP);
/// \brief Build an empty while statement.
explicit WhileStmt(EmptyShell Empty) : Stmt(WhileStmtClass, Empty) { }
@@ -1063,6 +1075,10 @@
SourceLocation getWhileLoc() const { return WhileLoc; }
void setWhileLoc(SourceLocation L) { WhileLoc = L; }
+ SourceLocation getLParenLoc() const { return LParenLoc; }
+ void setLParenLoc(SourceLocation L) { LParenLoc = L; }
+ SourceLocation getRParenLoc() const { return RParenLoc; }
+ void setRParenLoc(SourceLocation L) { RParenLoc = L; }
SourceLocation getLocStart() const LLVM_READONLY { return WhileLoc; }
SourceLocation getLocEnd() const LLVM_READONLY {
Index: include/clang/Parse/Parser.h
===================================================================
--- include/clang/Parse/Parser.h
+++ include/clang/Parse/Parser.h
@@ -1587,10 +1587,9 @@
unsigned ScopeFlags);
void ParseCompoundStatementLeadingPragmas();
StmtResult ParseCompoundStatementBody(bool isStmtExpr = false);
- bool ParseParenExprOrCondition(ExprResult &ExprResult,
- Decl *&DeclResult,
- SourceLocation Loc,
- bool ConvertToBoolean);
+ bool ParseParenExprOrCondition(BalancedDelimiterTracker &Tracker,
+ ExprResult &ExprResult, Decl *&DeclResult,
+ SourceLocation Loc, bool ConvertToBoolean);
StmtResult ParseIfStatement(SourceLocation *TrailingElseLoc);
StmtResult ParseSwitchStatement(SourceLocation *TrailingElseLoc);
StmtResult ParseWhileStatement(SourceLocation *TrailingElseLoc);
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -3056,18 +3056,18 @@
ArrayRef<const Attr*> Attrs,
Stmt *SubStmt);
- StmtResult ActOnIfStmt(SourceLocation IfLoc,
+ StmtResult ActOnIfStmt(SourceLocation IfLoc, SourceLocation CondLParen,
FullExprArg CondVal, Decl *CondVar,
- Stmt *ThenVal,
+ SourceLocation CondRParen, Stmt *ThenVal,
SourceLocation ElseLoc, Stmt *ElseVal);
StmtResult ActOnStartOfSwitchStmt(SourceLocation SwitchLoc,
- Expr *Cond,
- Decl *CondVar);
+ SourceLocation CondLParen, Expr *Cond,
+ Decl *CondVar, SourceLocation CondRParen);
StmtResult ActOnFinishSwitchStmt(SourceLocation SwitchLoc,
Stmt *Switch, Stmt *Body);
- StmtResult ActOnWhileStmt(SourceLocation WhileLoc,
- FullExprArg Cond,
- Decl *CondVar, Stmt *Body);
+ StmtResult ActOnWhileStmt(SourceLocation WhileLoc, SourceLocation CondLParen,
+ FullExprArg Cond, Decl *CondVar,
+ SourceLocation CondRParen, Stmt *Body);
StmtResult ActOnDoStmt(SourceLocation DoLoc, Stmt *Body,
SourceLocation WhileLoc,
SourceLocation CondLParen, Expr *Cond,
Index: lib/AST/Stmt.cpp
===================================================================
--- lib/AST/Stmt.cpp
+++ lib/AST/Stmt.cpp
@@ -854,9 +854,9 @@
}
IfStmt::IfStmt(const ASTContext &C, SourceLocation IL, VarDecl *var, Expr *cond,
- Stmt *then, SourceLocation EL, Stmt *elsev)
- : Stmt(IfStmtClass), IfLoc(IL), ElseLoc(EL)
-{
+ SourceLocation LP, SourceLocation RP, Stmt *then,
+ SourceLocation EL, Stmt *elsev)
+ : Stmt(IfStmtClass), IfLoc(IL), LParenLoc(LP), RParenLoc(RP), ElseLoc(EL) {
setConditionVariable(C, var);
SubExprs[COND] = cond;
SubExprs[THEN] = then;
@@ -913,9 +913,10 @@
VarRange.getEnd());
}
-SwitchStmt::SwitchStmt(const ASTContext &C, VarDecl *Var, Expr *cond)
- : Stmt(SwitchStmtClass), FirstCase(nullptr), AllEnumCasesCovered(0)
-{
+SwitchStmt::SwitchStmt(const ASTContext &C, VarDecl *Var, Expr *cond,
+ SourceLocation LP, SourceLocation RP)
+ : Stmt(SwitchStmtClass), FirstCase(nullptr), LParenLoc(LP), RParenLoc(RP),
+ AllEnumCasesCovered(0) {
setConditionVariable(C, Var);
SubExprs[COND] = cond;
SubExprs[BODY] = nullptr;
@@ -947,12 +948,11 @@
}
WhileStmt::WhileStmt(const ASTContext &C, VarDecl *Var, Expr *cond, Stmt *body,
- SourceLocation WL)
- : Stmt(WhileStmtClass) {
+ SourceLocation WL, SourceLocation LP, SourceLocation RP)
+ : Stmt(WhileStmtClass), WhileLoc(WL), LParenLoc(LP), RParenLoc(RP) {
setConditionVariable(C, Var);
SubExprs[COND] = cond;
SubExprs[BODY] = body;
- WhileLoc = WL;
}
VarDecl *WhileStmt::getConditionVariable() const {
Index: lib/Analysis/BodyFarm.cpp
===================================================================
--- lib/Analysis/BodyFarm.cpp
+++ lib/Analysis/BodyFarm.cpp
@@ -242,7 +242,8 @@
SourceLocation());
// (5) Create the 'if' statement.
- IfStmt *If = new (C) IfStmt(C, SourceLocation(), nullptr, UO, CS);
+ IfStmt *If = new (C) IfStmt(C, SourceLocation(), nullptr, UO,
+ SourceLocation(), SourceLocation(), CS);
return If;
}
@@ -346,8 +347,8 @@
/// Construct the If.
Stmt *If =
- new (C) IfStmt(C, SourceLocation(), nullptr, Comparison, Body,
- SourceLocation(), Else);
+ new (C) IfStmt(C, SourceLocation(), nullptr, Comparison, SourceLocation(),
+ SourceLocation(), Body, SourceLocation(), Else);
return If;
}
Index: lib/Parse/ParseStmt.cpp
===================================================================
--- lib/Parse/ParseStmt.cpp
+++ lib/Parse/ParseStmt.cpp
@@ -1004,12 +1004,11 @@
/// should try to recover harder. It returns false if the condition is
/// successfully parsed. Note that a successful parse can still have semantic
/// errors in the condition.
-bool Parser::ParseParenExprOrCondition(ExprResult &ExprResult,
- Decl *&DeclResult,
- SourceLocation Loc,
+bool Parser::ParseParenExprOrCondition(BalancedDelimiterTracker &Tracker,
+ ExprResult &ExprResult,
+ Decl *&DeclResult, SourceLocation Loc,
bool ConvertToBoolean) {
- BalancedDelimiterTracker T(*this, tok::l_paren);
- T.consumeOpen();
+ Tracker.consumeOpen();
if (getLangOpts().CPlusPlus)
ParseCXXCondition(ExprResult, DeclResult, Loc, ConvertToBoolean);
@@ -1035,7 +1034,7 @@
}
// Otherwise the condition is valid or the rparen is present.
- T.consumeClose();
+ Tracker.consumeClose();
// Check for extraneous ')'s to catch things like "if (foo())) {". We know
// that all callers are looking for a statement after the condition, so ")"
@@ -1086,7 +1085,8 @@
// Parse the condition.
ExprResult CondExp;
Decl *CondVar = nullptr;
- if (ParseParenExprOrCondition(CondExp, CondVar, IfLoc, true))
+ BalancedDelimiterTracker T(*this, tok::l_paren);
+ if (ParseParenExprOrCondition(T, CondExp, CondVar, IfLoc, true))
return StmtError();
FullExprArg FullCondExp(Actions.MakeFullExpr(CondExp.get(), IfLoc));
@@ -1173,8 +1173,9 @@
if (ElseStmt.isInvalid())
ElseStmt = Actions.ActOnNullStmt(ElseStmtLoc);
- return Actions.ActOnIfStmt(IfLoc, FullCondExp, CondVar, ThenStmt.get(),
- ElseLoc, ElseStmt.get());
+ return Actions.ActOnIfStmt(IfLoc, T.getOpenLocation(), FullCondExp, CondVar,
+ T.getCloseLocation(), ThenStmt.get(), ElseLoc,
+ ElseStmt.get());
}
/// ParseSwitchStatement
@@ -1213,11 +1214,13 @@
// Parse the condition.
ExprResult Cond;
Decl *CondVar = nullptr;
- if (ParseParenExprOrCondition(Cond, CondVar, SwitchLoc, false))
+ BalancedDelimiterTracker T(*this, tok::l_paren);
+ if (ParseParenExprOrCondition(T, Cond, CondVar, SwitchLoc, false))
return StmtError();
- StmtResult Switch
- = Actions.ActOnStartOfSwitchStmt(SwitchLoc, Cond.get(), CondVar);
+ StmtResult Switch =
+ Actions.ActOnStartOfSwitchStmt(SwitchLoc, T.getOpenLocation(), Cond.get(),
+ CondVar, T.getCloseLocation());
if (Switch.isInvalid()) {
// Skip the switch body.
@@ -1301,7 +1304,8 @@
// Parse the condition.
ExprResult Cond;
Decl *CondVar = nullptr;
- if (ParseParenExprOrCondition(Cond, CondVar, WhileLoc, true))
+ BalancedDelimiterTracker T(*this, tok::l_paren);
+ if (ParseParenExprOrCondition(T, Cond, CondVar, WhileLoc, true))
return StmtError();
FullExprArg FullCond(Actions.MakeFullExpr(Cond.get(), WhileLoc));
@@ -1329,7 +1333,8 @@
if ((Cond.isInvalid() && !CondVar) || Body.isInvalid())
return StmtError();
- return Actions.ActOnWhileStmt(WhileLoc, FullCond, CondVar, Body.get());
+ return Actions.ActOnWhileStmt(WhileLoc, T.getOpenLocation(), FullCond,
+ CondVar, T.getCloseLocation(), Body.get());
}
/// ParseDoStatement
Index: lib/Sema/SemaStmt.cpp
===================================================================
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -449,10 +449,10 @@
return LS;
}
-StmtResult
-Sema::ActOnIfStmt(SourceLocation IfLoc, FullExprArg CondVal, Decl *CondVar,
- Stmt *thenStmt, SourceLocation ElseLoc,
- Stmt *elseStmt) {
+StmtResult Sema::ActOnIfStmt(SourceLocation IfLoc, SourceLocation CondLParen,
+ FullExprArg CondVal, Decl *CondVar,
+ SourceLocation CondRParen, Stmt *thenStmt,
+ SourceLocation ElseLoc, Stmt *elseStmt) {
// If the condition was invalid, discard the if statement. We could recover
// better by replacing it with a valid expr, but don't do that yet.
if (!CondVal.get() && !CondVar) {
@@ -482,8 +482,9 @@
DiagnoseUnusedExprResult(elseStmt);
- return new (Context) IfStmt(Context, IfLoc, ConditionVar, ConditionExpr,
- thenStmt, ElseLoc, elseStmt);
+ return new (Context)
+ IfStmt(Context, IfLoc, ConditionVar, ConditionExpr, CondLParen,
+ CondRParen, thenStmt, ElseLoc, elseStmt);
}
namespace {
@@ -545,9 +546,10 @@
return expr->getType();
}
-StmtResult
-Sema::ActOnStartOfSwitchStmt(SourceLocation SwitchLoc, Expr *Cond,
- Decl *CondVar) {
+StmtResult Sema::ActOnStartOfSwitchStmt(SourceLocation SwitchLoc,
+ SourceLocation CondLParen, Expr *Cond,
+ Decl *CondVar,
+ SourceLocation CondRParen) {
ExprResult CondResult;
VarDecl *ConditionVar = nullptr;
@@ -629,7 +631,8 @@
getCurFunction()->setHasBranchIntoScope();
- SwitchStmt *SS = new (Context) SwitchStmt(Context, ConditionVar, Cond);
+ SwitchStmt *SS = new (Context)
+ SwitchStmt(Context, ConditionVar, Cond, CondLParen, CondRParen);
getCurFunction()->SwitchStack.push_back(SS);
return SS;
}
@@ -1195,9 +1198,10 @@
}
}
-StmtResult
-Sema::ActOnWhileStmt(SourceLocation WhileLoc, FullExprArg Cond,
- Decl *CondVar, Stmt *Body) {
+StmtResult Sema::ActOnWhileStmt(SourceLocation WhileLoc,
+ SourceLocation CondLParen, FullExprArg Cond,
+ Decl *CondVar, SourceLocation CondRParen,
+ Stmt *Body) {
ExprResult CondResult(Cond.release());
VarDecl *ConditionVar = nullptr;
@@ -1217,8 +1221,8 @@
if (isa<NullStmt>(Body))
getCurCompoundScope().setHasEmptyLoopBodies();
- return new (Context)
- WhileStmt(Context, ConditionVar, ConditionExpr, Body, WhileLoc);
+ return new (Context) WhileStmt(Context, ConditionVar, ConditionExpr, Body,
+ WhileLoc, CondLParen, CondRParen);
}
StmtResult
Index: lib/Sema/TreeTransform.h
===================================================================
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -1112,20 +1112,24 @@
///
/// By default, performs semantic analysis to build the new statement.
/// Subclasses may override this routine to provide different behavior.
- StmtResult RebuildIfStmt(SourceLocation IfLoc, Sema::FullExprArg Cond,
- VarDecl *CondVar, Stmt *Then,
+ StmtResult RebuildIfStmt(SourceLocation IfLoc, SourceLocation LParenLoc,
+ Sema::FullExprArg Cond, VarDecl *CondVar,
+ SourceLocation RParenLoc, Stmt *Then,
SourceLocation ElseLoc, Stmt *Else) {
- return getSema().ActOnIfStmt(IfLoc, Cond, CondVar, Then, ElseLoc, Else);
+ return getSema().ActOnIfStmt(IfLoc, LParenLoc, Cond, CondVar, RParenLoc,
+ Then, ElseLoc, Else);
}
/// \brief Start building a new switch statement.
///
/// By default, performs semantic analysis to build the new statement.
/// Subclasses may override this routine to provide different behavior.
StmtResult RebuildSwitchStmtStart(SourceLocation SwitchLoc,
- Expr *Cond, VarDecl *CondVar) {
- return getSema().ActOnStartOfSwitchStmt(SwitchLoc, Cond,
- CondVar);
+ SourceLocation LParenLoc, Expr *Cond,
+ VarDecl *CondVar,
+ SourceLocation RParenLoc) {
+ return getSema().ActOnStartOfSwitchStmt(SwitchLoc, LParenLoc, Cond, CondVar,
+ RParenLoc);
}
/// \brief Attach the body to the switch statement.
@@ -1141,9 +1145,11 @@
///
/// By default, performs semantic analysis to build the new statement.
/// Subclasses may override this routine to provide different behavior.
- StmtResult RebuildWhileStmt(SourceLocation WhileLoc, Sema::FullExprArg Cond,
- VarDecl *CondVar, Stmt *Body) {
- return getSema().ActOnWhileStmt(WhileLoc, Cond, CondVar, Body);
+ StmtResult RebuildWhileStmt(SourceLocation WhileLoc, SourceLocation LParenLoc,
+ Sema::FullExprArg Cond, VarDecl *CondVar,
+ SourceLocation RParenLoc, Stmt *Body) {
+ return getSema().ActOnWhileStmt(WhileLoc, LParenLoc, Cond, CondVar,
+ RParenLoc, Body);
}
/// \brief Build a new do-while statement.
@@ -5602,8 +5608,8 @@
Else.get() == S->getElse())
return S;
- return getDerived().RebuildIfStmt(S->getIfLoc(), FullCond, ConditionVar,
- Then.get(),
+ return getDerived().RebuildIfStmt(S->getIfLoc(), S->getLParenLoc(), FullCond,
+ ConditionVar, S->getRParenLoc(), Then.get(),
S->getElseLoc(), Else.get());
}
@@ -5629,9 +5635,9 @@
}
// Rebuild the switch statement.
- StmtResult Switch
- = getDerived().RebuildSwitchStmtStart(S->getSwitchLoc(), Cond.get(),
- ConditionVar);
+ StmtResult Switch = getDerived().RebuildSwitchStmtStart(
+ S->getSwitchLoc(), S->getLParenLoc(), Cond.get(), ConditionVar,
+ S->getRParenLoc());
if (Switch.isInvalid())
return StmtError();
@@ -5691,8 +5697,9 @@
Body.get() == S->getBody())
return Owned(S);
- return getDerived().RebuildWhileStmt(S->getWhileLoc(), FullCond,
- ConditionVar, Body.get());
+ return getDerived().RebuildWhileStmt(S->getWhileLoc(), S->getLParenLoc(),
+ FullCond, ConditionVar,
+ S->getRParenLoc(), Body.get());
}
template<typename Derived>
Index: lib/Serialization/ASTReaderStmt.cpp
===================================================================
--- lib/Serialization/ASTReaderStmt.cpp
+++ lib/Serialization/ASTReaderStmt.cpp
@@ -189,6 +189,8 @@
S->setThen(Reader.ReadSubStmt());
S->setElse(Reader.ReadSubStmt());
S->setIfLoc(ReadSourceLocation(Record, Idx));
+ S->setLParenLoc(ReadSourceLocation(Record, Idx));
+ S->setRParenLoc(ReadSourceLocation(Record, Idx));
S->setElseLoc(ReadSourceLocation(Record, Idx));
}
@@ -199,6 +201,8 @@
S->setCond(Reader.ReadSubExpr());
S->setBody(Reader.ReadSubStmt());
S->setSwitchLoc(ReadSourceLocation(Record, Idx));
+ S->setLParenLoc(ReadSourceLocation(Record, Idx));
+ S->setRParenLoc(ReadSourceLocation(Record, Idx));
if (Record[Idx++])
S->setAllEnumCasesCovered();
@@ -222,6 +226,8 @@
S->setCond(Reader.ReadSubExpr());
S->setBody(Reader.ReadSubStmt());
S->setWhileLoc(ReadSourceLocation(Record, Idx));
+ S->setLParenLoc(ReadSourceLocation(Record, Idx));
+ S->setRParenLoc(ReadSourceLocation(Record, Idx));
}
void ASTStmtReader::VisitDoStmt(DoStmt *S) {
Index: lib/Serialization/ASTWriterStmt.cpp
===================================================================
--- lib/Serialization/ASTWriterStmt.cpp
+++ lib/Serialization/ASTWriterStmt.cpp
@@ -124,6 +124,8 @@
Writer.AddStmt(S->getThen());
Writer.AddStmt(S->getElse());
Writer.AddSourceLocation(S->getIfLoc(), Record);
+ Writer.AddSourceLocation(S->getLParenLoc(), Record);
+ Writer.AddSourceLocation(S->getRParenLoc(), Record);
Writer.AddSourceLocation(S->getElseLoc(), Record);
Code = serialization::STMT_IF;
}
@@ -134,6 +136,8 @@
Writer.AddStmt(S->getCond());
Writer.AddStmt(S->getBody());
Writer.AddSourceLocation(S->getSwitchLoc(), Record);
+ Writer.AddSourceLocation(S->getLParenLoc(), Record);
+ Writer.AddSourceLocation(S->getRParenLoc(), Record);
Record.push_back(S->isAllEnumCasesCovered());
for (SwitchCase *SC = S->getSwitchCaseList(); SC;
SC = SC->getNextSwitchCase())
@@ -147,6 +151,8 @@
Writer.AddStmt(S->getCond());
Writer.AddStmt(S->getBody());
Writer.AddSourceLocation(S->getWhileLoc(), Record);
+ Writer.AddSourceLocation(S->getLParenLoc(), Record);
+ Writer.AddSourceLocation(S->getRParenLoc(), Record);
Code = serialization::STMT_WHILE;
}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
