replica
/
hanchenye-llvm-project
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Revamps structural error detection / handling. 

Previously we'd only detect structural errors on the very first level.
This leads to incorrectly balanced braces not being discovered, and thus
incorrect indentation.

This change fixes the problem by:
- changing the parser to use an error state that can be detected
  anywhere inside the productions, for example if we get an eof on
  SOME_MACRO({ some block <eof>
- previously we'd never break lines when we discovered a structural
  error; now we break even in the case of a structural error if there
  are two unwrapped lines within the same line; thus,
  void f() { while (true) { g(); y(); } }
  will still be re-formatted, even if there's missing braces somewhere
  in the file
- still exclude macro definitions from generating structural error;
  macro definitions are inbalanced snippets

llvm-svn: 179379
This commit is contained in:
Manuel Klimek 2013-04-12 14:13:36 +00:00
parent 9fb82357dc
commit 1a18c40468
4 changed files with 57 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
clang/lib/Format/Format.cpp
View File

@ -459,7 +459,7 @@ public:
UnwrappedLineFormatter(const FormatStyle &Style, SourceManager &SourceMgr,
const AnnotatedLine &Line, unsigned FirstIndent,
const AnnotatedToken &RootToken,
WhitespaceManager &Whitespaces, bool StructuralError)
WhitespaceManager &Whitespaces)
: Style(Style), SourceMgr(SourceMgr), Line(Line),
FirstIndent(FirstIndent), RootToken(RootToken),
Whitespaces(Whitespaces), Count(0) {}
@ -1381,7 +1381,7 @@ public:
tooling::Replacements format() {
LexerBasedFormatTokenSource Tokens(Lex, SourceMgr);
UnwrappedLineParser Parser(Diag, Style, Tokens, *this);
StructuralError = Parser.parse();
bool StructuralError = Parser.parse();
unsigned PreviousEndOfLineColumn = 0;
TokenAnnotator Annotator(Style, SourceMgr, Lex,
Tokens.getIdentTable().get("in"));
@ -1431,17 +1431,19 @@ public:
unsigned Indent = LevelIndent;
if (static_cast<int>(Indent) + Offset >= 0)
Indent += Offset;
if (!FirstTok.WhiteSpaceStart.isValid() || StructuralError) {
Indent = LevelIndent =
SourceMgr.getSpellingColumnNumber(FirstTok.Tok.getLocation()) - 1;
} else {
if (FirstTok.WhiteSpaceStart.isValid() &&
// Insert a break even if there is a structural error in case where
// we break apart a line consisting of multiple unwrapped lines.
(FirstTok.NewlinesBefore == 0 || !StructuralError)) {
formatFirstToken(TheLine.First, PreviousLineLastToken, Indent,
TheLine.InPPDirective, PreviousEndOfLineColumn);
} else {
Indent = LevelIndent =
SourceMgr.getSpellingColumnNumber(FirstTok.Tok.getLocation()) - 1;
}
tryFitMultipleLinesInOne(Indent, I, E);
UnwrappedLineFormatter Formatter(Style, SourceMgr, TheLine, Indent,
TheLine.First, Whitespaces,
StructuralError);
TheLine.First, Whitespaces);
PreviousEndOfLineColumn =
Formatter.format(I + 1 != E ? &*(I + 1) : NULL);
IndentForLevel[TheLine.Level] = LevelIndent;
@ -1742,7 +1744,6 @@ private:
WhitespaceManager Whitespaces;
std::vector<CharSourceRange> Ranges;
std::vector<AnnotatedLine> AnnotatedLines;
bool StructuralError;
};
tooling::Replacements

55
clang/lib/Format/UnwrappedLineParser.cpp
View File

@ -45,9 +45,11 @@ private:
class ScopedMacroState : public FormatTokenSource {
public:
ScopedMacroState(UnwrappedLine &Line, FormatTokenSource *&TokenSource,
FormatToken &ResetToken)
FormatToken &ResetToken, bool &StructuralError)
: Line(Line), TokenSource(TokenSource), ResetToken(ResetToken),
PreviousLineLevel(Line.Level), PreviousTokenSource(TokenSource) {
PreviousLineLevel(Line.Level), PreviousTokenSource(TokenSource),
StructuralError(StructuralError),
PreviousStructuralError(StructuralError) {
TokenSource = this;
Line.Level = 0;
Line.InPPDirective = true;
@ -58,6 +60,7 @@ public:
ResetToken = Token;
Line.InPPDirective = false;
Line.Level = PreviousLineLevel;
StructuralError = PreviousStructuralError;
}
virtual FormatToken getNextToken() {
@ -85,6 +88,8 @@ private:
FormatToken &ResetToken;
unsigned PreviousLineLevel;
FormatTokenSource *PreviousTokenSource;
bool &StructuralError;
bool PreviousStructuralError;
FormatToken Token;
};
@ -124,13 +129,13 @@ UnwrappedLineParser::UnwrappedLineParser(
clang::DiagnosticsEngine &Diag, const FormatStyle &Style,
FormatTokenSource &Tokens, UnwrappedLineConsumer &Callback)
: Line(new UnwrappedLine), MustBreakBeforeNextToken(false),
CurrentLines(&Lines), Diag(Diag), Style(Style), Tokens(&Tokens),
Callback(Callback) {}
CurrentLines(&Lines), StructuralError(false), Diag(Diag), Style(Style),
Tokens(&Tokens), Callback(Callback) {}
bool UnwrappedLineParser::parse() {
DEBUG(llvm::dbgs() << "----\n");
readToken();
bool Error = parseFile();
parseFile();
for (std::vector<UnwrappedLine>::iterator I = Lines.begin(), E = Lines.end();
I != E; ++I) {
Callback.consumeUnwrappedLine(*I);
@ -139,23 +144,20 @@ bool UnwrappedLineParser::parse() {
// Create line with eof token.
pushToken(FormatTok);
Callback.consumeUnwrappedLine(*Line);
return Error;
return StructuralError;
}
bool UnwrappedLineParser::parseFile() {
void UnwrappedLineParser::parseFile() {
ScopedDeclarationState DeclarationState(
*Line, DeclarationScopeStack,
/*MustBeDeclaration=*/ !Line->InPPDirective);
bool Error = parseLevel(/*HasOpeningBrace=*/ false);
parseLevel(/*HasOpeningBrace=*/ false);
// Make sure to format the remaining tokens.
flushComments(true);
addUnwrappedLine();
return Error;
}
bool UnwrappedLineParser::parseLevel(bool HasOpeningBrace) {
bool Error = false;
void UnwrappedLineParser::parseLevel(bool HasOpeningBrace) {
do {
switch (FormatTok.Tok.getKind()) {
case tok::comment:
@ -165,30 +167,27 @@ bool UnwrappedLineParser::parseLevel(bool HasOpeningBrace) {
case tok::l_brace:
// FIXME: Add parameter whether this can happen - if this happens, we must
// be in a non-declaration context.
Error |= parseBlock(/*MustBeDeclaration=*/ false);
parseBlock(/*MustBeDeclaration=*/ false);
addUnwrappedLine();
break;
case tok::r_brace:
if (HasOpeningBrace) {
return false;
} else {
Diag.Report(FormatTok.Tok.getLocation(),
Diag.getCustomDiagID(clang::DiagnosticsEngine::Error,
"unexpected '}'"));
Error = true;
nextToken();
addUnwrappedLine();
}
if (HasOpeningBrace)
return;
Diag.Report(FormatTok.Tok.getLocation(),
Diag.getCustomDiagID(clang::DiagnosticsEngine::Error,
"unexpected '}'"));
StructuralError = true;
nextToken();
addUnwrappedLine();
break;
default:
parseStructuralElement();
break;
}
} while (!eof());
return Error;
}
bool UnwrappedLineParser::parseBlock(bool MustBeDeclaration,
void UnwrappedLineParser::parseBlock(bool MustBeDeclaration,
unsigned AddLevels) {
assert(FormatTok.Tok.is(tok::l_brace) && "'{' expected");
nextToken();
@ -202,17 +201,17 @@ bool UnwrappedLineParser::parseBlock(bool MustBeDeclaration,
if (!FormatTok.Tok.is(tok::r_brace)) {
Line->Level -= AddLevels;
return true;
StructuralError = true;
return;
}
nextToken(); // Munch the closing brace.
Line->Level -= AddLevels;
return false;
}
void UnwrappedLineParser::parsePPDirective() {
assert(FormatTok.Tok.is(tok::hash) && "'#' expected");
ScopedMacroState MacroState(*Line, Tokens, FormatTok);
ScopedMacroState MacroState(*Line, Tokens, FormatTok, StructuralError);
nextToken();
if (FormatTok.Tok.getIdentifierInfo() == NULL) {

10
clang/lib/Format/UnwrappedLineParser.h
View File

@ -125,9 +125,9 @@ public:
bool parse();
private:
bool parseFile();
bool parseLevel(bool HasOpeningBrace);
bool parseBlock(bool MustBeDeclaration, unsigned AddLevels = 1);
void parseFile();
void parseLevel(bool HasOpeningBrace);
void parseBlock(bool MustBeDeclaration, unsigned AddLevels = 1);
void parsePPDirective();
void parsePPDefine();
void parsePPUnknown();
@ -187,6 +187,10 @@ private:
// whether we are in a compound statement or not.
std::vector<bool> DeclarationScopeStack;
// Will be true if we encounter an error that leads to possibily incorrect
// indentation levels.
bool StructuralError;
clang::DiagnosticsEngine &Diag;
const FormatStyle &Style;
FormatTokenSource *Tokens;

15
clang/unittests/Format/FormatTest.cpp
View File

@ -430,6 +430,7 @@ TEST_F(FormatTest, FormatsSwitchStatement) {
verifyFormat("switch (x) {\n"
"default: {\n"
" // Do nothing.\n"
"}\n"
"}");
verifyFormat("switch (x) {\n"
"// comment\n"
@ -3563,8 +3564,18 @@ TEST_F(FormatTest, ObjCLiterals) {
verifyFormat("@{ @\"one\" : @1 }");
verifyFormat("return @{ @\"one\" : @1 };");
verifyFormat("@{ @\"one\" : @1, }");
verifyFormat("@{ @\"one\" : @{ @2 : @1 } }");
verifyFormat("@{ @\"one\" : @{ @2 : @1 }, }");
// FIXME: Breaking in cases where we think there's a structural error
// showed that we're incorrectly parsing this code. We need to fix the
// parsing here.
verifyFormat("@{ @\"one\" : @\n"
"{ @2 : @1 }\n"
"}");
verifyFormat("@{ @\"one\" : @\n"
"{ @2 : @1 }\n"
",\n"
"}");
verifyFormat("@{ 1 > 2 ? @\"one\" : @\"two\" : 1 > 2 ? @1 : @2 }");
verifyFormat("[self setDict:@{}");
verifyFormat("[self setDict:@{ @1 : @2 }");