[PATCH] D68114: Fix for expanding __pragmas in macro arguments

2019-10-07 Thread Amy Huang via Phabricator via cfe-commits
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

2019-10-04 Thread Reid Kleckner via Phabricator via cfe-commits
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

2019-10-04 Thread Amy Huang via Phabricator via cfe-commits
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

2019-10-03 Thread Reid Kleckner via Phabricator via cfe-commits
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

2019-09-30 Thread Amy Huang via Phabricator via cfe-commits
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

2019-09-26 Thread Amy Huang via Phabricator via cfe-commits
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