jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, JonChesterfield, ronlieb, MyDeveloperDay, owenpan, HazardyKnusperkeks, curdeius, wanders. Herald added a project: All. jhuber6 requested review of this revision. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: clang.
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. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D136100 Files: 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
Index: clang/unittests/Format/FormatTest.cpp =================================================================== --- clang/unittests/Format/FormatTest.cpp +++ clang/unittests/Format/FormatTest.cpp @@ -5172,7 +5172,7 @@ 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"); @@ -19934,6 +19934,27 @@ "(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"); Index: clang/lib/Format/UnwrappedLineParser.h =================================================================== --- clang/lib/Format/UnwrappedLineParser.h +++ clang/lib/Format/UnwrappedLineParser.h @@ -46,6 +46,8 @@ /// 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 @@ void parsePPElIf(); void parsePPElse(); void parsePPEndIf(); + void parsePPPragma(); void parsePPUnknown(); void readTokenWithJavaScriptASI(); void parseStructuralElement(bool IsTopLevel = false, @@ -355,8 +358,9 @@ }; 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 Index: clang/lib/Format/UnwrappedLineParser.cpp =================================================================== --- clang/lib/Format/UnwrappedLineParser.cpp +++ clang/lib/Format/UnwrappedLineParser.cpp @@ -1118,6 +1118,9 @@ case tok::pp_endif: parsePPEndIf(); break; + case tok::pp_pragma: + parsePPPragma(); + break; default: parsePPUnknown(); break; @@ -1280,6 +1283,11 @@ parseFile(); } +void UnwrappedLineParser::parsePPPragma() { + Line->InPragmaDirective = true; + parsePPUnknown(); +} + void UnwrappedLineParser::parsePPUnknown() { do { nextToken(); Index: clang/lib/Format/TokenAnnotator.h =================================================================== --- clang/lib/Format/TokenAnnotator.h +++ clang/lib/Format/TokenAnnotator.h @@ -40,7 +40,9 @@ : 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 @@ size_t MatchingOpeningBlockLineIndex; size_t MatchingClosingBlockLineIndex; bool InPPDirective; + bool InPragmaDirective; bool InMacroBody; bool MustBeDeclaration; bool MightBeFunctionDecl; Index: clang/lib/Format/TokenAnnotator.cpp =================================================================== --- clang/lib/Format/TokenAnnotator.cpp +++ clang/lib/Format/TokenAnnotator.cpp @@ -765,7 +765,7 @@ // 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); @@ -1047,7 +1047,8 @@ // 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; Index: clang/lib/Format/ContinuationIndenter.cpp =================================================================== --- clang/lib/Format/ContinuationIndenter.cpp +++ clang/lib/Format/ContinuationIndenter.cpp @@ -1248,6 +1248,9 @@ 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 &&
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits