Skip to content

Commit bcda54b

Browse files
krasimirggkrasimirgg
committedApr 21, 2017
[clang-format] Replace IncompleteFormat by a struct with Line
Summary: This patch replaces the boolean IncompleteFormat that is used to notify the client if an unrecoverable syntax error occurred by a struct that also contains a line number. Reviewers: djasper Reviewed By: djasper Subscribers: cfe-commits, klimek Differential Revision: https://reviews.llvm.org/D32298 llvm-svn: 300985
1 parent d631b9e commit bcda54b

File tree

11 files changed

+110
-64
lines changed

11 files changed

+110
-64
lines changed
 

‎clang/include/clang/Format/Format.h

+23-4
Original file line numberDiff line numberDiff line change
@@ -1512,6 +1512,18 @@ llvm::Expected<tooling::Replacements>
15121512
cleanupAroundReplacements(StringRef Code, const tooling::Replacements &Replaces,
15131513
const FormatStyle &Style);
15141514

1515+
/// \brief Represents the status of a formatting attempt.
1516+
struct FormattingAttemptStatus {
1517+
/// \brief A value of ``false`` means that any of the affected ranges were not
1518+
/// formatted due to a non-recoverable syntax error.
1519+
bool FormatComplete = true;
1520+
1521+
/// \brief If ``FormatComplete`` is false, ``Line`` records a one-based
1522+
/// original line number at which a syntax error might have occurred. This is
1523+
/// based on a best-effort analysis and could be imprecise.
1524+
unsigned Line = 0;
1525+
};
1526+
15151527
/// \brief Reformats the given \p Ranges in \p Code.
15161528
///
15171529
/// Each range is extended on either end to its next bigger logic unit, i.e.
@@ -1521,13 +1533,20 @@ cleanupAroundReplacements(StringRef Code, const tooling::Replacements &Replaces,
15211533
/// Returns the ``Replacements`` necessary to make all \p Ranges comply with
15221534
/// \p Style.
15231535
///
1524-
/// If ``IncompleteFormat`` is non-null, its value will be set to true if any
1525-
/// of the affected ranges were not formatted due to a non-recoverable syntax
1526-
/// error.
1536+
/// If ``Status`` is non-null, its value will be populated with the status of
1537+
/// this formatting attempt. See \c FormattingAttemptStatus.
15271538
tooling::Replacements reformat(const FormatStyle &Style, StringRef Code,
15281539
ArrayRef<tooling::Range> Ranges,
15291540
StringRef FileName = "<stdin>",
1530-
bool *IncompleteFormat = nullptr);
1541+
FormattingAttemptStatus *Status = nullptr);
1542+
1543+
/// \brief Same as above, except if ``IncompleteFormat`` is non-null, its value
1544+
/// will be set to true if any of the affected ranges were not formatted due to
1545+
/// a non-recoverable syntax error.
1546+
tooling::Replacements reformat(const FormatStyle &Style, StringRef Code,
1547+
ArrayRef<tooling::Range> Ranges,
1548+
StringRef FileName,
1549+
bool *IncompleteFormat);
15311550

15321551
/// \brief Clean up any erroneous/redundant code in the given \p Ranges in \p
15331552
/// Code.

‎clang/lib/Format/Format.cpp

+19-8
Original file line numberDiff line numberDiff line change
@@ -908,8 +908,8 @@ class JavaScriptRequoter : public TokenAnalyzer {
908908
class Formatter : public TokenAnalyzer {
909909
public:
910910
Formatter(const Environment &Env, const FormatStyle &Style,
911-
bool *IncompleteFormat)
912-
: TokenAnalyzer(Env, Style), IncompleteFormat(IncompleteFormat) {}
911+
FormattingAttemptStatus *Status)
912+
: TokenAnalyzer(Env, Style), Status(Status) {}
913913

914914
tooling::Replacements
915915
analyze(TokenAnnotator &Annotator,
@@ -931,7 +931,7 @@ class Formatter : public TokenAnalyzer {
931931
Env.getSourceManager(), Whitespaces, Encoding,
932932
BinPackInconclusiveFunctions);
933933
UnwrappedLineFormatter(&Indenter, &Whitespaces, Style, Tokens.getKeywords(),
934-
IncompleteFormat)
934+
Env.getSourceManager(), Status)
935935
.format(AnnotatedLines);
936936
for (const auto &R : Whitespaces.generateReplacements())
937937
if (Result.add(R))
@@ -1013,7 +1013,7 @@ class Formatter : public TokenAnalyzer {
10131013
}
10141014

10151015
bool BinPackInconclusiveFunctions;
1016-
bool *IncompleteFormat;
1016+
FormattingAttemptStatus *Status;
10171017
};
10181018

10191019
// This class clean up the erroneous/redundant code around the given ranges in
@@ -1830,7 +1830,8 @@ cleanupAroundReplacements(StringRef Code, const tooling::Replacements &Replaces,
18301830

18311831
tooling::Replacements reformat(const FormatStyle &Style, StringRef Code,
18321832
ArrayRef<tooling::Range> Ranges,
1833-
StringRef FileName, bool *IncompleteFormat) {
1833+
StringRef FileName,
1834+
FormattingAttemptStatus *Status) {
18341835
FormatStyle Expanded = expandPresets(Style);
18351836
if (Expanded.DisableFormat)
18361837
return tooling::Replacements();
@@ -1846,11 +1847,11 @@ tooling::Replacements reformat(const FormatStyle &Style, StringRef Code,
18461847
auto NewEnv = Environment::CreateVirtualEnvironment(
18471848
*NewCode, FileName,
18481849
tooling::calculateRangesAfterReplacements(Fixes, Ranges));
1849-
Formatter Format(*NewEnv, Expanded, IncompleteFormat);
1850+
Formatter Format(*NewEnv, Expanded, Status);
18501851
return Fixes.merge(Format.process());
18511852
}
18521853
}
1853-
Formatter Format(*Env, Expanded, IncompleteFormat);
1854+
Formatter Format(*Env, Expanded, Status);
18541855
return Format.process();
18551856
};
18561857

@@ -1866,7 +1867,7 @@ tooling::Replacements reformat(const FormatStyle &Style, StringRef Code,
18661867
return reformatAfterApplying(Requoter);
18671868
}
18681869

1869-
Formatter Format(*Env, Expanded, IncompleteFormat);
1870+
Formatter Format(*Env, Expanded, Status);
18701871
return Format.process();
18711872
}
18721873

@@ -1879,6 +1880,16 @@ tooling::Replacements cleanup(const FormatStyle &Style, StringRef Code,
18791880
return Clean.process();
18801881
}
18811882

1883+
tooling::Replacements reformat(const FormatStyle &Style, StringRef Code,
1884+
ArrayRef<tooling::Range> Ranges,
1885+
StringRef FileName, bool *IncompleteFormat) {
1886+
FormattingAttemptStatus Status;
1887+
auto Result = reformat(Style, Code, Ranges, FileName, &Status);
1888+
if (!Status.FormatComplete)
1889+
*IncompleteFormat = true;
1890+
return Result;
1891+
}
1892+
18821893
tooling::Replacements fixNamespaceEndComments(const FormatStyle &Style,
18831894
StringRef Code,
18841895
ArrayRef<tooling::Range> Ranges,

‎clang/lib/Format/UnwrappedLineFormatter.cpp

+5-2
Original file line numberDiff line numberDiff line change
@@ -835,8 +835,11 @@ UnwrappedLineFormatter::format(const SmallVectorImpl<AnnotatedLine *> &Lines,
835835
bool ShouldFormat = TheLine.Affected || FixIndentation;
836836
// We cannot format this line; if the reason is that the line had a
837837
// parsing error, remember that.
838-
if (ShouldFormat && TheLine.Type == LT_Invalid && IncompleteFormat)
839-
*IncompleteFormat = true;
838+
if (ShouldFormat && TheLine.Type == LT_Invalid && Status) {
839+
Status->FormatComplete = false;
840+
Status->Line =
841+
SourceMgr.getSpellingLineNumber(TheLine.First->Tok.getLocation());
842+
}
840843

841844
if (ShouldFormat && TheLine.Type != LT_Invalid) {
842845
if (!DryRun)

‎clang/lib/Format/UnwrappedLineFormatter.h

+6-3
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,11 @@ class UnwrappedLineFormatter {
3232
WhitespaceManager *Whitespaces,
3333
const FormatStyle &Style,
3434
const AdditionalKeywords &Keywords,
35-
bool *IncompleteFormat)
35+
const SourceManager &SourceMgr,
36+
FormattingAttemptStatus *Status)
3637
: Indenter(Indenter), Whitespaces(Whitespaces), Style(Style),
37-
Keywords(Keywords), IncompleteFormat(IncompleteFormat) {}
38+
Keywords(Keywords), SourceMgr(SourceMgr),
39+
Status(Status) {}
3840

3941
/// \brief Format the current block and return the penalty.
4042
unsigned format(const SmallVectorImpl<AnnotatedLine *> &Lines,
@@ -63,7 +65,8 @@ class UnwrappedLineFormatter {
6365
WhitespaceManager *Whitespaces;
6466
const FormatStyle &Style;
6567
const AdditionalKeywords &Keywords;
66-
bool *IncompleteFormat;
68+
const SourceManager &SourceMgr;
69+
FormattingAttemptStatus *Status;
6770
};
6871
} // end namespace format
6972
} // end namespace clang

‎clang/test/Format/incomplete.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// RUN: grep -Ev "// *[A-Z-]+:" %s | clang-format -style=LLVM -cursor=0 \
22
// RUN: | FileCheck -strict-whitespace %s
3-
// CHECK: {{"IncompleteFormat": true}}
3+
// CHECK: {{"IncompleteFormat": true, "Line": 2}}
44
// CHECK: {{^int\ \i;$}}
55
int i;
66
// CHECK: {{^f \( g \(;$}}

‎clang/tools/clang-format/ClangFormat.cpp

+12-5
Original file line numberDiff line numberDiff line change
@@ -276,14 +276,17 @@ static bool format(StringRef FileName) {
276276
}
277277
// Get new affected ranges after sorting `#includes`.
278278
Ranges = tooling::calculateRangesAfterReplacements(Replaces, Ranges);
279-
bool IncompleteFormat = false;
279+
FormattingAttemptStatus Status;
280280
Replacements FormatChanges = reformat(*FormatStyle, *ChangedCode, Ranges,
281-
AssumedFileName, &IncompleteFormat);
281+
AssumedFileName, &Status);
282282
Replaces = Replaces.merge(FormatChanges);
283283
if (OutputXML) {
284284
outs() << "<?xml version='1.0'?>\n<replacements "
285285
"xml:space='preserve' incomplete_format='"
286-
<< (IncompleteFormat ? "true" : "false") << "'>\n";
286+
<< (Status.FormatComplete ? "false" : "true") << "'";
287+
if (!Status.FormatComplete)
288+
outs() << " line=" << Status.Line;
289+
outs() << ">\n";
287290
if (Cursor.getNumOccurrences() != 0)
288291
outs() << "<cursor>"
289292
<< FormatChanges.getShiftedCodePosition(CursorPosition)
@@ -307,11 +310,15 @@ static bool format(StringRef FileName) {
307310
if (Rewrite.overwriteChangedFiles())
308311
return true;
309312
} else {
310-
if (Cursor.getNumOccurrences() != 0)
313+
if (Cursor.getNumOccurrences() != 0) {
311314
outs() << "{ \"Cursor\": "
312315
<< FormatChanges.getShiftedCodePosition(CursorPosition)
313316
<< ", \"IncompleteFormat\": "
314-
<< (IncompleteFormat ? "true" : "false") << " }\n";
317+
<< (Status.FormatComplete ? "false" : "true");
318+
if (!Status.FormatComplete)
319+
outs() << ", \"Line\": " << Status.Line;
320+
outs() << " }\n";
321+
}
315322
Rewrite.getEditBuffer(ID).write(outs());
316323
}
317324
}

‎clang/unittests/Format/FormatTest.cpp

+14-13
Original file line numberDiff line numberDiff line change
@@ -30,24 +30,25 @@ FormatStyle getGoogleStyle() { return getGoogleStyle(FormatStyle::LK_Cpp); }
3030

3131
class FormatTest : public ::testing::Test {
3232
protected:
33-
enum IncompleteCheck {
34-
IC_ExpectComplete,
35-
IC_ExpectIncomplete,
36-
IC_DoNotCheck
33+
enum StatusCheck {
34+
SC_ExpectComplete,
35+
SC_ExpectIncomplete,
36+
SC_DoNotCheck
3737
};
3838

3939
std::string format(llvm::StringRef Code,
4040
const FormatStyle &Style = getLLVMStyle(),
41-
IncompleteCheck CheckIncomplete = IC_ExpectComplete) {
41+
StatusCheck CheckComplete = SC_ExpectComplete) {
4242
DEBUG(llvm::errs() << "---\n");
4343
DEBUG(llvm::errs() << Code << "\n\n");
4444
std::vector<tooling::Range> Ranges(1, tooling::Range(0, Code.size()));
45-
bool IncompleteFormat = false;
45+
FormattingAttemptStatus Status;
4646
tooling::Replacements Replaces =
47-
reformat(Style, Code, Ranges, "<stdin>", &IncompleteFormat);
48-
if (CheckIncomplete != IC_DoNotCheck) {
49-
bool ExpectedIncompleteFormat = CheckIncomplete == IC_ExpectIncomplete;
50-
EXPECT_EQ(ExpectedIncompleteFormat, IncompleteFormat) << Code << "\n\n";
47+
reformat(Style, Code, Ranges, "<stdin>", &Status);
48+
if (CheckComplete != SC_DoNotCheck) {
49+
bool ExpectedCompleteFormat = CheckComplete == SC_ExpectComplete;
50+
EXPECT_EQ(ExpectedCompleteFormat, Status.FormatComplete)
51+
<< Code << "\n\n";
5152
}
5253
ReplacementCount = Replaces.size();
5354
auto Result = applyAllReplacements(Code, Replaces);
@@ -83,7 +84,7 @@ class FormatTest : public ::testing::Test {
8384
void verifyIncompleteFormat(llvm::StringRef Code,
8485
const FormatStyle &Style = getLLVMStyle()) {
8586
EXPECT_EQ(Code.str(),
86-
format(test::messUp(Code), Style, IC_ExpectIncomplete));
87+
format(test::messUp(Code), Style, SC_ExpectIncomplete));
8788
}
8889

8990
void verifyGoogleFormat(llvm::StringRef Code) {
@@ -98,7 +99,7 @@ class FormatTest : public ::testing::Test {
9899
/// \brief Verify that clang-format does not crash on the given input.
99100
void verifyNoCrash(llvm::StringRef Code,
100101
const FormatStyle &Style = getLLVMStyle()) {
101-
format(Code, Style, IC_DoNotCheck);
102+
format(Code, Style, SC_DoNotCheck);
102103
}
103104

104105
int ReplacementCount;
@@ -6189,7 +6190,7 @@ TEST_F(FormatTest, SkipsDeeplyNestedLines) {
61896190
// Deeply nested part is untouched, rest is formatted.
61906191
EXPECT_EQ(std::string("int i;\n") + Code + "int j;\n",
61916192
format(std::string("int i;\n") + Code + "int j;\n",
6192-
getLLVMStyle(), IC_ExpectIncomplete));
6193+
getLLVMStyle(), SC_ExpectIncomplete));
61936194
}
61946195

61956196
//===----------------------------------------------------------------------===//

‎clang/unittests/Format/FormatTestComments.cpp

+12-11
Original file line numberDiff line numberDiff line change
@@ -29,24 +29,25 @@ FormatStyle getGoogleStyle() { return getGoogleStyle(FormatStyle::LK_Cpp); }
2929

3030
class FormatTestComments : public ::testing::Test {
3131
protected:
32-
enum IncompleteCheck {
33-
IC_ExpectComplete,
34-
IC_ExpectIncomplete,
35-
IC_DoNotCheck
32+
enum StatusCheck {
33+
SC_ExpectComplete,
34+
SC_ExpectIncomplete,
35+
SC_DoNotCheck
3636
};
3737

3838
std::string format(llvm::StringRef Code,
3939
const FormatStyle &Style = getLLVMStyle(),
40-
IncompleteCheck CheckIncomplete = IC_ExpectComplete) {
40+
StatusCheck CheckComplete = SC_ExpectComplete) {
4141
DEBUG(llvm::errs() << "---\n");
4242
DEBUG(llvm::errs() << Code << "\n\n");
4343
std::vector<tooling::Range> Ranges(1, tooling::Range(0, Code.size()));
44-
bool IncompleteFormat = false;
44+
FormattingAttemptStatus Status;
4545
tooling::Replacements Replaces =
46-
reformat(Style, Code, Ranges, "<stdin>", &IncompleteFormat);
47-
if (CheckIncomplete != IC_DoNotCheck) {
48-
bool ExpectedIncompleteFormat = CheckIncomplete == IC_ExpectIncomplete;
49-
EXPECT_EQ(ExpectedIncompleteFormat, IncompleteFormat) << Code << "\n\n";
46+
reformat(Style, Code, Ranges, "<stdin>", &Status);
47+
if (CheckComplete != SC_DoNotCheck) {
48+
bool ExpectedCompleteFormat = CheckComplete == SC_ExpectComplete;
49+
EXPECT_EQ(ExpectedCompleteFormat, Status.FormatComplete)
50+
<< Code << "\n\n";
5051
}
5152
ReplacementCount = Replaces.size();
5253
auto Result = applyAllReplacements(Code, Replaces);
@@ -73,7 +74,7 @@ class FormatTestComments : public ::testing::Test {
7374
/// \brief Verify that clang-format does not crash on the given input.
7475
void verifyNoCrash(llvm::StringRef Code,
7576
const FormatStyle &Style = getLLVMStyle()) {
76-
format(Code, Style, IC_DoNotCheck);
77+
format(Code, Style, SC_DoNotCheck);
7778
}
7879

7980
int ReplacementCount;

‎clang/unittests/Format/FormatTestJS.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@ class FormatTestJS : public ::testing::Test {
2424
DEBUG(llvm::errs() << "---\n");
2525
DEBUG(llvm::errs() << Code << "\n\n");
2626
std::vector<tooling::Range> Ranges(1, tooling::Range(Offset, Length));
27-
bool IncompleteFormat = false;
27+
FormattingAttemptStatus Status;
2828
tooling::Replacements Replaces =
29-
reformat(Style, Code, Ranges, "<stdin>", &IncompleteFormat);
30-
EXPECT_FALSE(IncompleteFormat);
29+
reformat(Style, Code, Ranges, "<stdin>", &Status);
30+
EXPECT_TRUE(Status.FormatComplete);
3131
auto Result = applyAllReplacements(Code, Replaces);
3232
EXPECT_TRUE(static_cast<bool>(Result));
3333
DEBUG(llvm::errs() << "\n" << *Result << "\n\n");

‎clang/unittests/Format/FormatTestObjC.cpp

+12-11
Original file line numberDiff line numberDiff line change
@@ -33,23 +33,24 @@ class FormatTestObjC : public ::testing::Test {
3333
Style.Language = FormatStyle::LK_ObjC;
3434
}
3535

36-
enum IncompleteCheck {
37-
IC_ExpectComplete,
38-
IC_ExpectIncomplete,
39-
IC_DoNotCheck
36+
enum StatusCheck {
37+
SC_ExpectComplete,
38+
SC_ExpectIncomplete,
39+
SC_DoNotCheck
4040
};
4141

4242
std::string format(llvm::StringRef Code,
43-
IncompleteCheck CheckIncomplete = IC_ExpectComplete) {
43+
StatusCheck CheckComplete = SC_ExpectComplete) {
4444
DEBUG(llvm::errs() << "---\n");
4545
DEBUG(llvm::errs() << Code << "\n\n");
4646
std::vector<tooling::Range> Ranges(1, tooling::Range(0, Code.size()));
47-
bool IncompleteFormat = false;
47+
FormattingAttemptStatus Status;
4848
tooling::Replacements Replaces =
49-
reformat(Style, Code, Ranges, "<stdin>", &IncompleteFormat);
50-
if (CheckIncomplete != IC_DoNotCheck) {
51-
bool ExpectedIncompleteFormat = CheckIncomplete == IC_ExpectIncomplete;
52-
EXPECT_EQ(ExpectedIncompleteFormat, IncompleteFormat) << Code << "\n\n";
49+
reformat(Style, Code, Ranges, "<stdin>", &Status);
50+
if (CheckComplete != SC_DoNotCheck) {
51+
bool ExpectedCompleteFormat = CheckComplete == SC_ExpectComplete;
52+
EXPECT_EQ(ExpectedCompleteFormat, Status.FormatComplete)
53+
<< Code << "\n\n";
5354
}
5455
auto Result = applyAllReplacements(Code, Replaces);
5556
EXPECT_TRUE(static_cast<bool>(Result));
@@ -62,7 +63,7 @@ class FormatTestObjC : public ::testing::Test {
6263
}
6364

6465
void verifyIncompleteFormat(StringRef Code) {
65-
EXPECT_EQ(Code.str(), format(test::messUp(Code), IC_ExpectIncomplete));
66+
EXPECT_EQ(Code.str(), format(test::messUp(Code), SC_ExpectIncomplete));
6667
}
6768

6869
FormatStyle Style;

‎clang/unittests/Format/FormatTestSelective.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@ class FormatTestSelective : public ::testing::Test {
2424
DEBUG(llvm::errs() << "---\n");
2525
DEBUG(llvm::errs() << Code << "\n\n");
2626
std::vector<tooling::Range> Ranges(1, tooling::Range(Offset, Length));
27-
bool IncompleteFormat = false;
27+
FormattingAttemptStatus Status;
2828
tooling::Replacements Replaces =
29-
reformat(Style, Code, Ranges, "<stdin>", &IncompleteFormat);
30-
EXPECT_FALSE(IncompleteFormat) << Code << "\n\n";
29+
reformat(Style, Code, Ranges, "<stdin>", &Status);
30+
EXPECT_TRUE(Status.FormatComplete) << Code << "\n\n";
3131
auto Result = applyAllReplacements(Code, Replaces);
3232
EXPECT_TRUE(static_cast<bool>(Result));
3333
DEBUG(llvm::errs() << "\n" << *Result << "\n\n");

0 commit comments

Comments
 (0)
Please sign in to comment.