[clang] 037669d - [clang-format] Do not parse certain characters in pragma directives
Joseph Huber via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 18 14:38:27 PDT 2022
Author: Joseph Huber
Date: 2022-10-18T16:38:19-05:00
New Revision: 037669de8bdff42fc17b72460bf8784403c8d055
URL: https://github.com/llvm/llvm-project/commit/037669de8bdff42fc17b72460bf8784403c8d055
DIFF: https://github.com/llvm/llvm-project/commit/037669de8bdff42fc17b72460bf8784403c8d055.diff
LOG: [clang-format] Do not parse certain characters in pragma directives
Currently, we parse lines inside of a compiler `#pragma` the same way we
parse any other line. This is fine for some cases, like separating
expressions and adding proper spacing, but in others it causes some poor
results from miscategorizing some tokens.
For example, the OpenMP offloading uses certain clauses that contain
special characters like `map(tofrom : A[0:N])`. This will be formatted
poorly as it will be split between lines on the first colon.
Additionally the subscript notation will lead to poor spacing. This can
be seen in the OpenMP tests as the automatic clang formatting with
inevitably ruin the formatting.
For example, the following contrived example will be formatted poorly.
```
#pragma omp target teams distribute collapse(2) map(to: A[0 : M * K]) \
map(to: B[0:K * N]) map(tofrom:C[0:M*N]) firstprivate(Alpha) \
firstprivate(Beta) firstprivate(X) firstprivate(D) firstprivate(Y) \
firstprivate(E) firstprivate(Z) firstprivate(F)
```
This results in this when formatted, which is far from ideal.
```
#pragma omp target teams distribute collapse(2) map(to \
: A [0:M * K]) \
map(to \
: B [0:K * N]) map(tofrom \
: C [0:M * N]) firstprivate(Alpha) \
firstprivate(Beta) firstprivate(X) firstprivate(D) firstprivate(Y) \
firstprivate(E) firstprivate(Z) firstprivate(F)
```
This patch seeks to improve this by adding extra logic where the parsing goes
awry. This is primarily caused by the colon being parsed as an inline-asm
directive and the brackes an objective-C expressions. Also the line gets
indented every single time the line is dropped.
This doesn't implement true parsing handling for OpenMP statements.
Reviewed By: HazardyKnusperkeks
Differential Revision: https://reviews.llvm.org/D136100
Added:
Modified:
clang/lib/Format/ContinuationIndenter.cpp
clang/lib/Format/TokenAnnotator.cpp
clang/lib/Format/TokenAnnotator.h
clang/lib/Format/UnwrappedLineParser.cpp
clang/lib/Format/UnwrappedLineParser.h
clang/unittests/Format/FormatTest.cpp
Removed:
################################################################################
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index e4610064ff7df..00a9ae1063d18 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -1248,6 +1248,9 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
return ContinuationIndent;
}
+ if (State.Line->InPragmaDirective)
+ return CurrentState.Indent + Style.ContinuationIndentWidth;
+
// This ensure that we correctly format ObjC methods calls without inputs,
// i.e. where the last element isn't selector like: [callee method];
if (NextNonComment->is(tok::identifier) && NextNonComment->FakeRParens == 0 &&
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 8eef2be404d18..076ceb72dd330 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -766,7 +766,7 @@ class AnnotatingParser {
// Remember that this is a [[using ns: foo]] C++ attribute, so we
// don't add a space before the colon (unlike other colons).
CurrentToken->setType(TT_AttributeColon);
- } else if (!Style.isVerilog() &&
+ } else if (!Style.isVerilog() && !Line.InPragmaDirective &&
Left->isOneOf(TT_ArraySubscriptLSquare,
TT_DesignatedInitializerLSquare)) {
Left->setType(TT_ObjCMethodExpr);
@@ -1048,7 +1048,8 @@ class AnnotatingParser {
// This handles a special macro in ObjC code where selectors including
// the colon are passed as macro arguments.
Tok->setType(TT_ObjCMethodExpr);
- } else if (Contexts.back().ContextKind == tok::l_paren) {
+ } else if (Contexts.back().ContextKind == tok::l_paren &&
+ !Line.InPragmaDirective) {
Tok->setType(TT_InlineASMColon);
}
break;
diff --git a/clang/lib/Format/TokenAnnotator.h b/clang/lib/Format/TokenAnnotator.h
index b6f086d76728b..cea9ef78f1b28 100644
--- a/clang/lib/Format/TokenAnnotator.h
+++ b/clang/lib/Format/TokenAnnotator.h
@@ -40,7 +40,9 @@ class AnnotatedLine {
: First(Line.Tokens.front().Tok), Level(Line.Level),
MatchingOpeningBlockLineIndex(Line.MatchingOpeningBlockLineIndex),
MatchingClosingBlockLineIndex(Line.MatchingClosingBlockLineIndex),
- InPPDirective(Line.InPPDirective), InMacroBody(Line.InMacroBody),
+ InPPDirective(Line.InPPDirective),
+ InPragmaDirective(Line.InPragmaDirective),
+ InMacroBody(Line.InMacroBody),
MustBeDeclaration(Line.MustBeDeclaration), MightBeFunctionDecl(false),
IsMultiVariableDeclStmt(false), Affected(false),
LeadingEmptyLinesAffected(false), ChildrenAffected(false),
@@ -130,6 +132,7 @@ class AnnotatedLine {
size_t MatchingOpeningBlockLineIndex;
size_t MatchingClosingBlockLineIndex;
bool InPPDirective;
+ bool InPragmaDirective;
bool InMacroBody;
bool MustBeDeclaration;
bool MightBeFunctionDecl;
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 2145cfe06b835..25d9018fa1093 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -1118,6 +1118,9 @@ void UnwrappedLineParser::parsePPDirective() {
case tok::pp_endif:
parsePPEndIf();
break;
+ case tok::pp_pragma:
+ parsePPPragma();
+ break;
default:
parsePPUnknown();
break;
@@ -1280,6 +1283,11 @@ void UnwrappedLineParser::parsePPDefine() {
parseFile();
}
+void UnwrappedLineParser::parsePPPragma() {
+ Line->InPragmaDirective = true;
+ parsePPUnknown();
+}
+
void UnwrappedLineParser::parsePPUnknown() {
do {
nextToken();
diff --git a/clang/lib/Format/UnwrappedLineParser.h b/clang/lib/Format/UnwrappedLineParser.h
index ce51d17d5f9ce..b9b106bcc89a4 100644
--- a/clang/lib/Format/UnwrappedLineParser.h
+++ b/clang/lib/Format/UnwrappedLineParser.h
@@ -46,6 +46,8 @@ struct UnwrappedLine {
/// Whether this \c UnwrappedLine is part of a preprocessor directive.
bool InPPDirective;
+ /// Whether this \c UnwrappedLine is part of a pramga directive.
+ bool InPragmaDirective;
/// Whether it is part of a macro body.
bool InMacroBody;
@@ -120,6 +122,7 @@ class UnwrappedLineParser {
void parsePPElIf();
void parsePPElse();
void parsePPEndIf();
+ void parsePPPragma();
void parsePPUnknown();
void readTokenWithJavaScriptASI();
void parseStructuralElement(bool IsTopLevel = false,
@@ -355,8 +358,9 @@ struct UnwrappedLineNode {
};
inline UnwrappedLine::UnwrappedLine()
- : Level(0), InPPDirective(false), InMacroBody(false),
- MustBeDeclaration(false), MatchingOpeningBlockLineIndex(kInvalidIndex) {}
+ : Level(0), InPPDirective(false), InPragmaDirective(false),
+ InMacroBody(false), MustBeDeclaration(false),
+ MatchingOpeningBlockLineIndex(kInvalidIndex) {}
} // end namespace format
} // end namespace clang
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index e9f5d66d8c566..79e0847e02629 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -5172,7 +5172,7 @@ TEST_F(FormatTest, MacroDefinitionsWithIncompleteCode) {
verifyIncompleteFormat("#define STR(x) #x\n"
"f(STR(this_is_a_string_literal{));");
verifyFormat("#pragma omp threadprivate( \\\n"
- " y)), // expected-warning",
+ " y)), // expected-warning",
getLLVMStyleWithColumns(28));
verifyFormat("#d, = };");
verifyFormat("#if \"a");
@@ -19935,6 +19935,27 @@ TEST_F(FormatTest, UnderstandsPragmas) {
"(including parentheses)."));
}
+TEST_F(FormatTest, UnderstandsPragmaOmpTarget) {
+ verifyFormat("#pragma omp target map(to : var)");
+ verifyFormat("#pragma omp target map(to : var[ : N])");
+ verifyFormat("#pragma omp target map(to : var[0 : N])");
+ verifyFormat("#pragma omp target map(always, to : var[0 : N])");
+
+ EXPECT_EQ(
+ "#pragma omp target \\\n"
+ " reduction(+ : var) \\\n"
+ " map(to : A[0 : N]) \\\n"
+ " map(to : B[0 : N]) \\\n"
+ " map(from : C[0 : N]) \\\n"
+ " firstprivate(i) \\\n"
+ " firstprivate(j) \\\n"
+ " firstprivate(k)",
+ format(
+ "#pragma omp target reduction(+:var) map(to:A[0:N]) map(to:B[0:N]) "
+ "map(from:C[0:N]) firstprivate(i) firstprivate(j) firstprivate(k)",
+ getLLVMStyleWithColumns(26)));
+}
+
TEST_F(FormatTest, UnderstandPragmaOption) {
verifyFormat("#pragma option -C -A");
More information about the cfe-commits
mailing list