r197553 - [OPENMP] Fix for parsing OpenMP directives with extra braces, brackets and parens
Aaron Ballman
aaron at aaronballman.com
Wed Dec 18 05:19:33 PST 2013
On Wed, Dec 18, 2013 at 3:46 AM, Alexey Bataev <a.bataev at hotmail.com> wrote:
> Author: abataev
> Date: Wed Dec 18 02:46:25 2013
> New Revision: 197553
>
> URL: http://llvm.org/viewvc/llvm-project?rev=197553&view=rev
> Log:
> [OPENMP] Fix for parsing OpenMP directives with extra braces, brackets and parens
>
> Modified:
> cfe/trunk/include/clang/Parse/Parser.h
> cfe/trunk/lib/Parse/ParseOpenMP.cpp
> cfe/trunk/lib/Parse/Parser.cpp
> cfe/trunk/lib/Parse/RAIIObjectsForParser.h
> cfe/trunk/test/OpenMP/parallel_messages.cpp
> cfe/trunk/test/OpenMP/threadprivate_messages.cpp
>
> Modified: cfe/trunk/include/clang/Parse/Parser.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=197553&r1=197552&r2=197553&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Parse/Parser.h (original)
> +++ cfe/trunk/include/clang/Parse/Parser.h Wed Dec 18 02:46:25 2013
> @@ -684,9 +684,13 @@ private:
> /// If the input is malformed, this emits the specified diagnostic. Next, if
> /// SkipToTok is specified, it calls SkipUntil(SkipToTok). Finally, true is
> /// returned.
> + /// If NoCount is true, it ignores parens/brackets/braces as regular tokens
> + /// and does not count them. By default it recursively skips properly-nested
> + /// parens/brackets/braces.
> bool ExpectAndConsume(tok::TokenKind ExpectedTok, unsigned Diag,
> const char *DiagMsg = "",
> - tok::TokenKind SkipToTok = tok::unknown);
> + tok::TokenKind SkipToTok = tok::unknown,
> + bool NoCount = false);
>
> /// \brief The parser expects a semicolon and, if present, will consume it.
> ///
> @@ -789,7 +793,9 @@ public:
> StopAtSemi = 1 << 0, ///< Stop skipping at semicolon
> /// \brief Stop skipping at specified token, but don't skip the token itself
> StopBeforeMatch = 1 << 1,
> - StopAtCodeCompletion = 1 << 2 ///< Stop at code completion
> + StopAtCodeCompletion = 1 << 2, ///< Stop at code completion
> + NoBracketsCount = 1 << 3 /// \brief Don't count braces/brackets/parens
> + /// to skip balanced pairs
> };
>
> friend LLVM_CONSTEXPR SkipUntilFlags operator|(SkipUntilFlags L,
>
> Modified: cfe/trunk/lib/Parse/ParseOpenMP.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseOpenMP.cpp?rev=197553&r1=197552&r2=197553&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Parse/ParseOpenMP.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseOpenMP.cpp Wed Dec 18 02:46:25 2013
> @@ -48,7 +48,7 @@ Parser::DeclGroupPtrTy Parser::ParseOpen
> if (Tok.isNot(tok::annot_pragma_openmp_end)) {
> Diag(Tok, diag::warn_omp_extra_tokens_at_eol)
> << getOpenMPDirectiveName(OMPD_threadprivate);
> - SkipUntil(tok::annot_pragma_openmp_end, StopBeforeMatch);
> + SkipUntil(tok::annot_pragma_openmp_end, StopBeforeMatch | NoBracketsCount);
80-column violation.
> }
> // Skip the last annot_pragma_openmp_end.
> ConsumeToken();
> @@ -66,7 +66,7 @@ Parser::DeclGroupPtrTy Parser::ParseOpen
> << getOpenMPDirectiveName(DKind);
> break;
> }
> - SkipUntil(tok::annot_pragma_openmp_end);
> + SkipUntil(tok::annot_pragma_openmp_end, NoBracketsCount);
> return DeclGroupPtrTy();
> }
>
> @@ -105,14 +105,14 @@ StmtResult Parser::ParseOpenMPDeclarativ
> if (Tok.isNot(tok::annot_pragma_openmp_end)) {
> Diag(Tok, diag::warn_omp_extra_tokens_at_eol)
> << getOpenMPDirectiveName(OMPD_threadprivate);
> - SkipUntil(tok::annot_pragma_openmp_end, StopBeforeMatch);
> + SkipUntil(tok::annot_pragma_openmp_end, StopBeforeMatch | NoBracketsCount);
80-column violation.
> }
> DeclGroupPtrTy Res =
> Actions.ActOnOpenMPThreadprivateDirective(Loc,
> Identifiers);
> Directive = Actions.ActOnDeclStmt(Res, Loc, Tok.getLocation());
> }
> - SkipUntil(tok::annot_pragma_openmp_end);
> + SkipUntil(tok::annot_pragma_openmp_end, NoBracketsCount);
> break;
> case OMPD_parallel: {
> ConsumeToken();
> @@ -171,13 +171,13 @@ StmtResult Parser::ParseOpenMPDeclarativ
> break;
> case OMPD_unknown:
> Diag(Tok, diag::err_omp_unknown_directive);
> - SkipUntil(tok::annot_pragma_openmp_end);
> + SkipUntil(tok::annot_pragma_openmp_end, NoBracketsCount);
> break;
> case OMPD_task:
> case NUM_OPENMP_DIRECTIVES:
> Diag(Tok, diag::err_omp_unexpected_directive)
> << getOpenMPDirectiveName(DKind);
> - SkipUntil(tok::annot_pragma_openmp_end);
> + SkipUntil(tok::annot_pragma_openmp_end, NoBracketsCount);
> break;
> }
> return Directive;
> @@ -194,7 +194,8 @@ bool Parser::ParseOpenMPSimpleVarList(Op
> bool AllowScopeSpecifier) {
> VarList.clear();
> // Parse '('.
> - BalancedDelimiterTracker T(*this, tok::l_paren, tok::annot_pragma_openmp_end);
> + BalancedDelimiterTracker T(*this, tok::l_paren, tok::annot_pragma_openmp_end,
> + true);
> if (T.expectAndConsume(diag::err_expected_lparen_after,
> getOpenMPDirectiveName(Kind)))
> return true;
> @@ -214,17 +215,17 @@ bool Parser::ParseOpenMPSimpleVarList(Op
> ParseOptionalCXXScopeSpecifier(SS, ParsedType(), false)) {
> IsCorrect = false;
> SkipUntil(tok::comma, tok::r_paren, tok::annot_pragma_openmp_end,
> - StopBeforeMatch);
> + StopBeforeMatch | NoBracketsCount);
> } else if (ParseUnqualifiedId(SS, false, false, false, ParsedType(),
> TemplateKWLoc, Name)) {
> IsCorrect = false;
> SkipUntil(tok::comma, tok::r_paren, tok::annot_pragma_openmp_end,
> - StopBeforeMatch);
> + StopBeforeMatch | NoBracketsCount);
> } else if (Tok.isNot(tok::comma) && Tok.isNot(tok::r_paren) &&
> Tok.isNot(tok::annot_pragma_openmp_end)) {
> IsCorrect = false;
> SkipUntil(tok::comma, tok::r_paren, tok::annot_pragma_openmp_end,
> - StopBeforeMatch);
> + StopBeforeMatch | NoBracketsCount);
> Diag(PrevTok.getLocation(), diag::err_expected_ident)
> << SourceRange(PrevTok.getLocation(), PrevTokLocation);
> } else {
> @@ -287,13 +288,13 @@ OMPClause *Parser::ParseOpenMPClause(Ope
> case OMPC_unknown:
> Diag(Tok, diag::warn_omp_extra_tokens_at_eol)
> << getOpenMPDirectiveName(DKind);
> - SkipUntil(tok::annot_pragma_openmp_end, StopBeforeMatch);
> + SkipUntil(tok::annot_pragma_openmp_end, StopBeforeMatch | NoBracketsCount);
> break;
> case OMPC_threadprivate:
> case NUM_OPENMP_CLAUSES:
> Diag(Tok, diag::err_omp_unexpected_clause)
> << getOpenMPClauseName(CKind) << getOpenMPDirectiveName(DKind);
> - SkipUntil(tok::comma, tok::annot_pragma_openmp_end, StopBeforeMatch);
> + SkipUntil(tok::comma, tok::annot_pragma_openmp_end, StopBeforeMatch | NoBracketsCount);
80-column violation.
> break;
> }
> return ErrorFound ? 0 : Clause;
> @@ -308,7 +309,8 @@ OMPClause *Parser::ParseOpenMPSimpleClau
> SourceLocation Loc = Tok.getLocation();
> SourceLocation LOpen = ConsumeToken();
> // Parse '('.
> - BalancedDelimiterTracker T(*this, tok::l_paren, tok::annot_pragma_openmp_end);
> + BalancedDelimiterTracker T(*this, tok::l_paren, tok::annot_pragma_openmp_end,
> + true);
> if (T.expectAndConsume(diag::err_expected_lparen_after,
> getOpenMPClauseName(Kind)))
> return 0;
> @@ -342,7 +344,8 @@ OMPClause *Parser::ParseOpenMPVarListCla
> SourceLocation Loc = Tok.getLocation();
> SourceLocation LOpen = ConsumeToken();
> // Parse '('.
> - BalancedDelimiterTracker T(*this, tok::l_paren, tok::annot_pragma_openmp_end);
> + BalancedDelimiterTracker T(*this, tok::l_paren, tok::annot_pragma_openmp_end,
> + true);
> if (T.expectAndConsume(diag::err_expected_lparen_after,
> getOpenMPClauseName(Kind)))
> return 0;
> @@ -357,7 +360,7 @@ OMPClause *Parser::ParseOpenMPVarListCla
> Vars.push_back(VarExpr.take());
> } else {
> SkipUntil(tok::comma, tok::r_paren, tok::annot_pragma_openmp_end,
> - StopBeforeMatch);
> + StopBeforeMatch | NoBracketsCount);
> }
> // Skip ',' if any
> IsComma = Tok.is(tok::comma);
>
> Modified: cfe/trunk/lib/Parse/Parser.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/Parser.cpp?rev=197553&r1=197552&r2=197553&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Parse/Parser.cpp (original)
> +++ cfe/trunk/lib/Parse/Parser.cpp Wed Dec 18 02:46:25 2013
> @@ -159,7 +159,8 @@ static bool IsCommonTypo(tok::TokenKind
> /// SkipToTok is specified, it calls SkipUntil(SkipToTok). Finally, true is
> /// returned.
> bool Parser::ExpectAndConsume(tok::TokenKind ExpectedTok, unsigned DiagID,
> - const char *Msg, tok::TokenKind SkipToTok) {
> + const char *Msg, tok::TokenKind SkipToTok,
> + bool NoCount) {
> if (Tok.is(ExpectedTok) || Tok.is(tok::code_completion)) {
> ConsumeAnyToken();
> return false;
> @@ -189,8 +190,12 @@ bool Parser::ExpectAndConsume(tok::Token
> } else
> Diag(Tok, DiagID) << Msg;
>
> - if (SkipToTok != tok::unknown)
> - SkipUntil(SkipToTok, StopAtSemi);
> + if (SkipToTok != tok::unknown) {
> + SkipUntilFlags Flags = StopAtSemi;
> + if (NoCount)
> + Flags = Flags | NoBracketsCount;
> + SkipUntil(SkipToTok, Flags);
> + }
> return true;
> }
>
> @@ -314,26 +319,32 @@ bool Parser::SkipUntil(ArrayRef<tok::Tok
> case tok::l_paren:
> // Recursively skip properly-nested parens.
> ConsumeParen();
> - if (HasFlagsSet(Flags, StopAtCodeCompletion))
> - SkipUntil(tok::r_paren, StopAtCodeCompletion);
> - else
> - SkipUntil(tok::r_paren);
> + if (!HasFlagsSet(Flags, NoBracketsCount)) {
> + if (HasFlagsSet(Flags, StopAtCodeCompletion))
> + SkipUntil(tok::r_paren, StopAtCodeCompletion);
> + else
> + SkipUntil(tok::r_paren);
> + }
> break;
> case tok::l_square:
> // Recursively skip properly-nested square brackets.
> ConsumeBracket();
> - if (HasFlagsSet(Flags, StopAtCodeCompletion))
> - SkipUntil(tok::r_square, StopAtCodeCompletion);
> - else
> - SkipUntil(tok::r_square);
> + if (!HasFlagsSet(Flags, NoBracketsCount)) {
> + if (HasFlagsSet(Flags, StopAtCodeCompletion))
> + SkipUntil(tok::r_square, StopAtCodeCompletion);
> + else
> + SkipUntil(tok::r_square);
> + }
> break;
> case tok::l_brace:
> // Recursively skip properly-nested braces.
> ConsumeBrace();
> - if (HasFlagsSet(Flags, StopAtCodeCompletion))
> - SkipUntil(tok::r_brace, StopAtCodeCompletion);
> - else
> - SkipUntil(tok::r_brace);
> + if (!HasFlagsSet(Flags, NoBracketsCount)) {
> + if (HasFlagsSet(Flags, StopAtCodeCompletion))
> + SkipUntil(tok::r_brace, StopAtCodeCompletion);
> + else
> + SkipUntil(tok::r_brace);
> + }
This code is getting complex enough that it should probably be
factored into a helper method that takes the token to consume instead
of copy-pasting it several times over.
> break;
>
> // Okay, we found a ']' or '}' or ')', which we think should be balanced.
> @@ -342,17 +353,17 @@ bool Parser::SkipUntil(ArrayRef<tok::Tok
> // higher level, we will assume that this matches the unbalanced token
> // and return it. Otherwise, this is a spurious RHS token, which we skip.
> case tok::r_paren:
> - if (ParenCount && !isFirstTokenSkipped)
> + if (!HasFlagsSet(Flags, NoBracketsCount) && ParenCount && !isFirstTokenSkipped)
> return false; // Matches something.
> ConsumeParen();
> break;
> case tok::r_square:
> - if (BracketCount && !isFirstTokenSkipped)
> + if (!HasFlagsSet(Flags, NoBracketsCount) && BracketCount && !isFirstTokenSkipped)
> return false; // Matches something.
> ConsumeBracket();
> break;
> case tok::r_brace:
> - if (BraceCount && !isFirstTokenSkipped)
> + if (!HasFlagsSet(Flags, NoBracketsCount) && BraceCount && !isFirstTokenSkipped)
All of these are 80-column violations.
> return false; // Matches something.
> ConsumeBrace();
> break;
> @@ -2031,7 +2042,7 @@ bool BalancedDelimiterTracker::expectAnd
> const char *Msg,
> tok::TokenKind SkipToToc ) {
> LOpen = P.Tok.getLocation();
> - if (P.ExpectAndConsume(Kind, DiagID, Msg, SkipToToc))
> + if (P.ExpectAndConsume(Kind, DiagID, Msg, SkipToToc, NoCount))
> return true;
>
> if (getDepth() < MaxDepth)
> @@ -2056,16 +2067,21 @@ bool BalancedDelimiterTracker::diagnoseM
>
> // If we're not already at some kind of closing bracket, skip to our closing
> // token.
> + Parser::SkipUntilFlags Flags = Parser::StopAtSemi | Parser::StopBeforeMatch;
> + if (NoCount)
> + Flags = Flags | Parser::NoBracketsCount;
> if (P.Tok.isNot(tok::r_paren) && P.Tok.isNot(tok::r_brace) &&
> P.Tok.isNot(tok::r_square) &&
> - P.SkipUntil(Close, FinalToken,
> - Parser::StopAtSemi | Parser::StopBeforeMatch) &&
> + P.SkipUntil(Close, FinalToken, Flags) &&
> P.Tok.is(Close))
> LClose = P.ConsumeAnyToken();
> return true;
> }
>
> void BalancedDelimiterTracker::skipToEnd() {
> - P.SkipUntil(Close, Parser::StopBeforeMatch);
> + Parser::SkipUntilFlags Flags = Parser::StopBeforeMatch;
> + if (NoCount)
> + Flags = Flags | Parser::NoBracketsCount;
> + P.SkipUntil(Close, Flags);
> consumeClose();
> }
>
> Modified: cfe/trunk/lib/Parse/RAIIObjectsForParser.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/RAIIObjectsForParser.h?rev=197553&r1=197552&r2=197553&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Parse/RAIIObjectsForParser.h (original)
> +++ cfe/trunk/lib/Parse/RAIIObjectsForParser.h Wed Dec 18 02:46:25 2013
> @@ -361,6 +361,7 @@ namespace clang {
> tok::TokenKind Kind, Close, FinalToken;
> SourceLocation (Parser::*Consumer)();
> SourceLocation LOpen, LClose;
> + bool NoCount;
>
> unsigned short &getDepth() {
> switch (Kind) {
> @@ -378,9 +379,10 @@ namespace clang {
>
> public:
> BalancedDelimiterTracker(Parser& p, tok::TokenKind k,
> - tok::TokenKind FinalToken = tok::semi)
> + tok::TokenKind FinalToken = tok::semi,
> + bool NoCount = false)
> : GreaterThanIsOperatorScope(p.GreaterThanIsOperator, true),
> - P(p), Kind(k), FinalToken(FinalToken)
> + P(p), Kind(k), FinalToken(FinalToken), NoCount(NoCount)
> {
> switch (Kind) {
> default: llvm_unreachable("Unexpected balanced token");
>
> Modified: cfe/trunk/test/OpenMP/parallel_messages.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/parallel_messages.cpp?rev=197553&r1=197552&r2=197553&view=diff
> ==============================================================================
> --- cfe/trunk/test/OpenMP/parallel_messages.cpp (original)
> +++ cfe/trunk/test/OpenMP/parallel_messages.cpp Wed Dec 18 02:46:25 2013
> @@ -6,6 +6,18 @@ void foo() {
> #pragma omp parallel // expected-error {{unexpected OpenMP directive '#pragma omp parallel'}}
>
> int main(int argc, char **argv) {
> + #pragma omp parallel { // expected-warning {{extra tokens at the end of '#pragma omp parallel' are ignored}}
> + foo();
> + #pragma omp parallel ( // expected-warning {{extra tokens at the end of '#pragma omp parallel' are ignored}}
> + foo();
> + #pragma omp parallel [ // expected-warning {{extra tokens at the end of '#pragma omp parallel' are ignored}}
> + foo();
> + #pragma omp parallel ] // expected-warning {{extra tokens at the end of '#pragma omp parallel' are ignored}}
> + foo();
> + #pragma omp parallel ) // expected-warning {{extra tokens at the end of '#pragma omp parallel' are ignored}}
> + foo();
> + #pragma omp parallel } // expected-warning {{extra tokens at the end of '#pragma omp parallel' are ignored}}
> + foo();
> #pragma omp parallel
> #pragma omp parallel unknown() // expected-warning {{extra tokens at the end of '#pragma omp parallel' are ignored}}
> foo();
>
> Modified: cfe/trunk/test/OpenMP/threadprivate_messages.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/threadprivate_messages.cpp?rev=197553&r1=197552&r2=197553&view=diff
> ==============================================================================
> --- cfe/trunk/test/OpenMP/threadprivate_messages.cpp (original)
> +++ cfe/trunk/test/OpenMP/threadprivate_messages.cpp Wed Dec 18 02:46:25 2013
> @@ -24,6 +24,12 @@ int foo() { // expected-note {{declared
> return (a);
> }
>
> +#pragma omp threadprivate (a) ( // expected-error {{'#pragma omp threadprivate' must precede all references to variable 'a'}} expected-warning {{extra tokens at the end of '#pragma omp threadprivate' are ignored}}
> +#pragma omp threadprivate (a) [ // expected-error {{'#pragma omp threadprivate' must precede all references to variable 'a'}} expected-warning {{extra tokens at the end of '#pragma omp threadprivate' are ignored}}
> +#pragma omp threadprivate (a) { // expected-error {{'#pragma omp threadprivate' must precede all references to variable 'a'}} expected-warning {{extra tokens at the end of '#pragma omp threadprivate' are ignored}}
> +#pragma omp threadprivate (a) ) // expected-error {{'#pragma omp threadprivate' must precede all references to variable 'a'}} expected-warning {{extra tokens at the end of '#pragma omp threadprivate' are ignored}}
> +#pragma omp threadprivate (a) ] // expected-error {{'#pragma omp threadprivate' must precede all references to variable 'a'}} expected-warning {{extra tokens at the end of '#pragma omp threadprivate' are ignored}}
> +#pragma omp threadprivate (a) } // expected-error {{'#pragma omp threadprivate' must precede all references to variable 'a'}} expected-warning {{extra tokens at the end of '#pragma omp threadprivate' are ignored}}
> #pragma omp threadprivate a // expected-error {{expected '(' after 'threadprivate'}}
> #pragma omp threadprivate(d // expected-error {{expected ')'}} expected-note {{to match this '('}} expected-error {{'#pragma omp threadprivate' must precede all references to variable 'd'}}
> #pragma omp threadprivate(d)) // expected-error {{'#pragma omp threadprivate' must precede all references to variable 'd'}} expected-warning {{extra tokens at the end of '#pragma omp threadprivate' are ignored}}
Thanks!
~Aaron
More information about the cfe-commits
mailing list