[PATCH] D68114: Fix for expanding __pragmas in macro arguments
This revision was automatically updated to reflect the committed changes. Closed by commit rL373950: Fix for expanding __pragmas in macro arguments (authored by akhuang, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D68114?vs=223255=223754#toc Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68114/new/ https://reviews.llvm.org/D68114 Files: cfe/trunk/lib/Lex/Pragma.cpp cfe/trunk/test/Preprocessor/pragma_microsoft.c Index: cfe/trunk/test/Preprocessor/pragma_microsoft.c === --- cfe/trunk/test/Preprocessor/pragma_microsoft.c +++ cfe/trunk/test/Preprocessor/pragma_microsoft.c @@ -51,6 +51,8 @@ __pragma(warning(pop)); \ } +#define PRAGMA_IN_ARGS(p) p + void f() { __pragma() // expected-warning{{unknown pragma ignored}} @@ -64,8 +66,16 @@ // CHECK: #pragma warning(disable: 1) // CHECK: ; 1 + (2 > 3) ? 4 : 5; // CHECK: #pragma warning(pop) -} + // Check that macro arguments can contain __pragma. + PRAGMA_IN_ARGS(MACRO_WITH__PRAGMA) // expected-warning {{lower precedence}} \ + // expected-note 2 {{place parentheses}} \ + // expected-warning {{expression result unused}} +// CHECK: #pragma warning(push) +// CHECK: #pragma warning(disable: 1) +// CHECK: ; 1 + (2 > 3) ? 4 : 5; +// CHECK: #pragma warning(pop) +} // This should include macro_arg_directive even though the include // is looking for test.h This allows us to assign to "n" Index: cfe/trunk/lib/Lex/Pragma.cpp === --- cfe/trunk/lib/Lex/Pragma.cpp +++ cfe/trunk/lib/Lex/Pragma.cpp @@ -121,6 +121,40 @@ // Preprocessor Pragma Directive Handling. //===--===// +namespace { +// TokenCollector provides the option to collect tokens that were "read" +// and return them to the stream to be read later. +// Currently used when reading _Pragma/__pragma directives. +struct TokenCollector { + Preprocessor + bool Collect; + SmallVector Tokens; + Token + + void lex() { +if (Collect) + Tokens.push_back(Tok); +Self.Lex(Tok); + } + + void revert() { +assert(Collect && "did not collect tokens"); +assert(!Tokens.empty() && "collected unexpected number of tokens"); + +// Push the ( "string" ) tokens into the token stream. +auto Toks = std::make_unique(Tokens.size()); +std::copy(Tokens.begin() + 1, Tokens.end(), Toks.get()); +Toks[Tokens.size() - 1] = Tok; +Self.EnterTokenStream(std::move(Toks), Tokens.size(), + /*DisableMacroExpansion*/ true, + /*IsReinject*/ true); + +// ... and return the pragma token unchanged. +Tok = *Tokens.begin(); + } +}; +} // namespace + /// HandlePragmaDirective - The "\#pragma" directive has been parsed. Lex the /// rest of the pragma, passing it to the registered pragma handlers. void Preprocessor::HandlePragmaDirective(PragmaIntroducer Introducer) { @@ -166,35 +200,6 @@ // In Case #2, we check the syntax now, but then put the tokens back into the // token stream for later consumption. - struct TokenCollector { -Preprocessor -bool Collect; -SmallVector Tokens; -Token - -void lex() { - if (Collect) -Tokens.push_back(Tok); - Self.Lex(Tok); -} - -void revert() { - assert(Collect && "did not collect tokens"); - assert(!Tokens.empty() && "collected unexpected number of tokens"); - - // Push the ( "string" ) tokens into the token stream. - auto Toks = std::make_unique(Tokens.size()); - std::copy(Tokens.begin() + 1, Tokens.end(), Toks.get()); - Toks[Tokens.size() - 1] = Tok; - Self.EnterTokenStream(std::move(Toks), Tokens.size(), -/*DisableMacroExpansion*/ true, -/*IsReinject*/ true); - - // ... and return the _Pragma token unchanged. - Tok = *Tokens.begin(); -} - }; - TokenCollector Toks = {*this, InMacroArgPreExpansion, {}, Tok}; // Remember the pragma token location. @@ -328,11 +333,15 @@ /// HandleMicrosoft__pragma - Like Handle_Pragma except the pragma text /// is not enclosed within a string literal. void Preprocessor::HandleMicrosoft__pragma(Token ) { + // During macro pre-expansion, check the syntax now but put the tokens back + // into the token stream for later consumption. Same as Handle_Pragma. + TokenCollector Toks = {*this, InMacroArgPreExpansion, {}, Tok}; + // Remember the pragma token location. SourceLocation PragmaLoc = Tok.getLocation(); // Read the '('. - Lex(Tok); + Toks.lex(); if (Tok.isNot(tok::l_paren)) { Diag(PragmaLoc, diag::err__Pragma_malformed); return; @@ -341,14 +350,14 @@ // Get the tokens
[PATCH] D68114: Fix for expanding __pragmas in macro arguments
rnk added a subscriber: Bigcheese. rnk added a comment. + @Bigcheese, for amusement. We discussed some interesting behavior of _Pragma and C++ raw string literals last night. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68114/new/ https://reviews.llvm.org/D68114 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D68114: Fix for expanding __pragmas in macro arguments
akhuang updated this revision to Diff 223255. akhuang added a comment. - move TokenCollector out of function Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68114/new/ https://reviews.llvm.org/D68114 Files: clang/lib/Lex/Pragma.cpp clang/test/Preprocessor/pragma_microsoft.c Index: clang/test/Preprocessor/pragma_microsoft.c === --- clang/test/Preprocessor/pragma_microsoft.c +++ clang/test/Preprocessor/pragma_microsoft.c @@ -51,6 +51,8 @@ __pragma(warning(pop)); \ } +#define PRAGMA_IN_ARGS(p) p + void f() { __pragma() // expected-warning{{unknown pragma ignored}} @@ -64,8 +66,16 @@ // CHECK: #pragma warning(disable: 1) // CHECK: ; 1 + (2 > 3) ? 4 : 5; // CHECK: #pragma warning(pop) -} + // Check that macro arguments can contain __pragma. + PRAGMA_IN_ARGS(MACRO_WITH__PRAGMA) // expected-warning {{lower precedence}} \ + // expected-note 2 {{place parentheses}} \ + // expected-warning {{expression result unused}} +// CHECK: #pragma warning(push) +// CHECK: #pragma warning(disable: 1) +// CHECK: ; 1 + (2 > 3) ? 4 : 5; +// CHECK: #pragma warning(pop) +} // This should include macro_arg_directive even though the include // is looking for test.h This allows us to assign to "n" Index: clang/lib/Lex/Pragma.cpp === --- clang/lib/Lex/Pragma.cpp +++ clang/lib/Lex/Pragma.cpp @@ -121,6 +121,40 @@ // Preprocessor Pragma Directive Handling. //===--===// +namespace { +// TokenCollector provides the option to collect tokens that were "read" +// and return them to the stream to be read later. +// Currently used when reading _Pragma/__pragma directives. +struct TokenCollector { + Preprocessor + bool Collect; + SmallVector Tokens; + Token + + void lex() { +if (Collect) + Tokens.push_back(Tok); +Self.Lex(Tok); + } + + void revert() { +assert(Collect && "did not collect tokens"); +assert(!Tokens.empty() && "collected unexpected number of tokens"); + +// Push the ( "string" ) tokens into the token stream. +auto Toks = std::make_unique(Tokens.size()); +std::copy(Tokens.begin() + 1, Tokens.end(), Toks.get()); +Toks[Tokens.size() - 1] = Tok; +Self.EnterTokenStream(std::move(Toks), Tokens.size(), + /*DisableMacroExpansion*/ true, + /*IsReinject*/ true); + +// ... and return the pragma token unchanged. +Tok = *Tokens.begin(); + } +}; +} // namespace + /// HandlePragmaDirective - The "\#pragma" directive has been parsed. Lex the /// rest of the pragma, passing it to the registered pragma handlers. void Preprocessor::HandlePragmaDirective(PragmaIntroducer Introducer) { @@ -166,35 +200,6 @@ // In Case #2, we check the syntax now, but then put the tokens back into the // token stream for later consumption. - struct TokenCollector { -Preprocessor -bool Collect; -SmallVector Tokens; -Token - -void lex() { - if (Collect) -Tokens.push_back(Tok); - Self.Lex(Tok); -} - -void revert() { - assert(Collect && "did not collect tokens"); - assert(!Tokens.empty() && "collected unexpected number of tokens"); - - // Push the ( "string" ) tokens into the token stream. - auto Toks = std::make_unique(Tokens.size()); - std::copy(Tokens.begin() + 1, Tokens.end(), Toks.get()); - Toks[Tokens.size() - 1] = Tok; - Self.EnterTokenStream(std::move(Toks), Tokens.size(), -/*DisableMacroExpansion*/ true, -/*IsReinject*/ true); - - // ... and return the _Pragma token unchanged. - Tok = *Tokens.begin(); -} - }; - TokenCollector Toks = {*this, InMacroArgPreExpansion, {}, Tok}; // Remember the pragma token location. @@ -328,11 +333,15 @@ /// HandleMicrosoft__pragma - Like Handle_Pragma except the pragma text /// is not enclosed within a string literal. void Preprocessor::HandleMicrosoft__pragma(Token ) { + // During macro pre-expansion, check the syntax now but put the tokens back + // into the token stream for later consumption. Same as Handle_Pragma. + TokenCollector Toks = {*this, InMacroArgPreExpansion, {}, Tok}; + // Remember the pragma token location. SourceLocation PragmaLoc = Tok.getLocation(); // Read the '('. - Lex(Tok); + Toks.lex(); if (Tok.isNot(tok::l_paren)) { Diag(PragmaLoc, diag::err__Pragma_malformed); return; @@ -341,14 +350,14 @@ // Get the tokens enclosed within the __pragma(), as well as the final ')'. SmallVector PragmaToks; int NumParens = 0; - Lex(Tok); + Toks.lex(); while (Tok.isNot(tok::eof)) { PragmaToks.push_back(Tok); if (Tok.is(tok::l_paren))
[PATCH] D68114: Fix for expanding __pragmas in macro arguments
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Nice, the solution was right in front of us the whole time. :) lgtm, but please share the TokenCollector class. Comment at: clang/lib/Lex/Pragma.cpp:169 struct TokenCollector { Preprocessor Let's pull this class out to file scope (and put it in an anonymous namespace) so we can reuse it instead of copying it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68114/new/ https://reviews.llvm.org/D68114 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D68114: Fix for expanding __pragmas in macro arguments
akhuang updated this revision to Diff 222464. akhuang added a comment. comments/whitespace/test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68114/new/ https://reviews.llvm.org/D68114 Files: clang/lib/Lex/Pragma.cpp clang/test/Preprocessor/pragma_microsoft.c Index: clang/test/Preprocessor/pragma_microsoft.c === --- clang/test/Preprocessor/pragma_microsoft.c +++ clang/test/Preprocessor/pragma_microsoft.c @@ -51,6 +51,8 @@ __pragma(warning(pop)); \ } +#define PRAGMA_IN_ARGS(p) p + void f() { __pragma() // expected-warning{{unknown pragma ignored}} @@ -64,8 +66,16 @@ // CHECK: #pragma warning(disable: 1) // CHECK: ; 1 + (2 > 3) ? 4 : 5; // CHECK: #pragma warning(pop) -} + // Check that macro arguments can contain __pragma. + PRAGMA_IN_ARGS(MACRO_WITH__PRAGMA) // expected-warning {{lower precedence}} \ + // expected-note 2 {{place parentheses}} \ + // expected-warning {{expression result unused}} +// CHECK: #pragma warning(push) +// CHECK: #pragma warning(disable: 1) +// CHECK: ; 1 + (2 > 3) ? 4 : 5; +// CHECK: #pragma warning(pop) +} // This should include macro_arg_directive even though the include // is looking for test.h This allows us to assign to "n" Index: clang/lib/Lex/Pragma.cpp === --- clang/lib/Lex/Pragma.cpp +++ clang/lib/Lex/Pragma.cpp @@ -328,11 +328,43 @@ /// HandleMicrosoft__pragma - Like Handle_Pragma except the pragma text /// is not enclosed within a string literal. void Preprocessor::HandleMicrosoft__pragma(Token ) { + // During macro pre-expansion, check the syntax now but put the tokens back + // into the token stream for later consumption. Same as Handle_Pragma. + struct TokenCollector { +Preprocessor +bool Collect; +SmallVector Tokens; +Token + +void lex() { + if (Collect) +Tokens.push_back(Tok); + Self.Lex(Tok); +} + +void revert() { + assert(Collect && "did not collect tokens"); + assert(!Tokens.empty() && "collected unexpected number of tokens"); + + // Push the pragma content tokens into the token stream. + auto Toks = std::make_unique(Tokens.size()); + std::copy(Tokens.begin() + 1, Tokens.end(), Toks.get()); + Toks[Tokens.size() - 1] = Tok; + Self.EnterTokenStream(std::move(Toks), Tokens.size(), +/*DisableMacroExpansion*/ true, +/*IsReinject*/ true); + + // ... and return the _Pragma token unchanged. + Tok = *Tokens.begin(); +} + }; + TokenCollector Toks = {*this, InMacroArgPreExpansion, {}, Tok}; + // Remember the pragma token location. SourceLocation PragmaLoc = Tok.getLocation(); // Read the '('. - Lex(Tok); + Toks.lex(); if (Tok.isNot(tok::l_paren)) { Diag(PragmaLoc, diag::err__Pragma_malformed); return; @@ -341,14 +373,14 @@ // Get the tokens enclosed within the __pragma(), as well as the final ')'. SmallVector PragmaToks; int NumParens = 0; - Lex(Tok); + Toks.lex(); while (Tok.isNot(tok::eof)) { PragmaToks.push_back(Tok); if (Tok.is(tok::l_paren)) NumParens++; else if (Tok.is(tok::r_paren) && NumParens-- == 0) break; -Lex(Tok); +Toks.lex(); } if (Tok.is(tok::eof)) { @@ -356,6 +388,12 @@ return; } + // If we're expanding a macro argument, put the tokens back. + if (InMacroArgPreExpansion) { +Toks.revert(); +return; + } + PragmaToks.front().setFlag(Token::LeadingSpace); // Replace the ')' with an EOD to mark the end of the pragma. ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D68114: Fix for expanding __pragmas in macro arguments
akhuang created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Avoid parsing __pragma into an annotation token when macro arguments are pre-expanded. This is what clang currently does when parsing _Pragmas. Fixes https://bugs.llvm.org/show_bug.cgi?id=41128, where clang crashed when trying to get the length of an annotation token. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D68114 Files: clang/lib/Lex/Pragma.cpp clang/test/Preprocessor/pragma_microsoft.c Index: clang/test/Preprocessor/pragma_microsoft.c === --- clang/test/Preprocessor/pragma_microsoft.c +++ clang/test/Preprocessor/pragma_microsoft.c @@ -51,6 +51,8 @@ __pragma(warning(pop)); \ } +#define __PRAGMA_IN_ARG(p) p + void f() { __pragma() // expected-warning{{unknown pragma ignored}} @@ -64,6 +66,10 @@ // CHECK: #pragma warning(disable: 1) // CHECK: ; 1 + (2 > 3) ? 4 : 5; // CHECK: #pragma warning(pop) + + // Check that __pragma can be in a macro argument. + __PRAGMA_IN_ARG(__pragma(pack())) +// CHECK: #pragma pack() } Index: clang/lib/Lex/Pragma.cpp === --- clang/lib/Lex/Pragma.cpp +++ clang/lib/Lex/Pragma.cpp @@ -165,7 +165,6 @@ // // In Case #2, we check the syntax now, but then put the tokens back into the // token stream for later consumption. - struct TokenCollector { Preprocessor bool Collect; @@ -194,7 +193,6 @@ Tok = *Tokens.begin(); } }; - TokenCollector Toks = {*this, InMacroArgPreExpansion, {}, Tok}; // Remember the pragma token location. @@ -328,11 +326,41 @@ /// HandleMicrosoft__pragma - Like Handle_Pragma except the pragma text /// is not enclosed within a string literal. void Preprocessor::HandleMicrosoft__pragma(Token ) { + struct TokenCollector { +Preprocessor +bool Collect; +SmallVector Tokens; +Token + +void lex() { + if (Collect) +Tokens.push_back(Tok); + Self.Lex(Tok); +} + +void revert() { + assert(Collect && "did not collect tokens"); + assert(!Tokens.empty() && "collected unexpected number of tokens"); + + // Push the pragma content tokens into the token stream. + auto Toks = std::make_unique(Tokens.size()); + std::copy(Tokens.begin() + 1, Tokens.end(), Toks.get()); + Toks[Tokens.size() - 1] = Tok; + Self.EnterTokenStream(std::move(Toks), Tokens.size(), +/*DisableMacroExpansion*/ true, +/*IsReinject*/ true); + + // ... and return the _Pragma token unchanged. + Tok = *Tokens.begin(); +} + }; + TokenCollector Toks = {*this, InMacroArgPreExpansion, {}, Tok}; + // Remember the pragma token location. SourceLocation PragmaLoc = Tok.getLocation(); // Read the '('. - Lex(Tok); + Toks.lex(); if (Tok.isNot(tok::l_paren)) { Diag(PragmaLoc, diag::err__Pragma_malformed); return; @@ -341,14 +369,14 @@ // Get the tokens enclosed within the __pragma(), as well as the final ')'. SmallVector PragmaToks; int NumParens = 0; - Lex(Tok); + Toks.lex(); while (Tok.isNot(tok::eof)) { PragmaToks.push_back(Tok); if (Tok.is(tok::l_paren)) NumParens++; else if (Tok.is(tok::r_paren) && NumParens-- == 0) break; -Lex(Tok); +Toks.lex(); } if (Tok.is(tok::eof)) { @@ -356,6 +384,12 @@ return; } + // If we're expanding a macro argument, put the tokens back. + if (InMacroArgPreExpansion) { +Toks.revert(); +return; + } + PragmaToks.front().setFlag(Token::LeadingSpace); // Replace the ')' with an EOD to mark the end of the pragma. ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits