[PATCH] D30427: Fix whitespace before token-paste of an argument.
This revision was automatically updated to reflect the committed changes. Closed by commit rL302195: Fix whitespace before token-paste of an argument. (authored by jyknight). Changed prior to commit: https://reviews.llvm.org/D30427?vs=89918=97881#toc Repository: rL LLVM https://reviews.llvm.org/D30427 Files: cfe/trunk/lib/Lex/TokenLexer.cpp cfe/trunk/test/Preprocessor/macro_paste_commaext.c cfe/trunk/test/Preprocessor/stringize_space.c Index: cfe/trunk/test/Preprocessor/macro_paste_commaext.c === --- cfe/trunk/test/Preprocessor/macro_paste_commaext.c +++ cfe/trunk/test/Preprocessor/macro_paste_commaext.c @@ -1,13 +1,20 @@ -// RUN: %clang_cc1 %s -E | grep 'V);' -// RUN: %clang_cc1 %s -E | grep 'W, 1, 2);' -// RUN: %clang_cc1 %s -E | grep 'X, 1, 2);' -// RUN: %clang_cc1 %s -E | grep 'Y,);' -// RUN: %clang_cc1 %s -E | grep 'Z,);' +// RUN: %clang_cc1 %s -E | FileCheck --strict-whitespace --match-full-lines %s + +// In the following tests, note that the output is sensitive to the +// whitespace *preceeding* the varargs argument, as well as to +// interior whitespace. AFAIK, this is the only case where whitespace +// preceeding an argument matters, and might be considered a bug in +// GCC. Nevertheless, since this feature is a GCC extension in the +// first place, we'll follow along. #define debug(format, ...) format, ## __VA_ARGS__) +// CHECK:V); debug(V); -debug(W, 1, 2); +// CHECK:W,1, 2); +debug(W,1, 2); +// CHECK:X, 1, 2); debug(X, 1, 2 ); +// CHECK:Y,); debug(Y, ); +// CHECK:Z,); debug(Z,); - Index: cfe/trunk/test/Preprocessor/stringize_space.c === --- cfe/trunk/test/Preprocessor/stringize_space.c +++ cfe/trunk/test/Preprocessor/stringize_space.c @@ -18,3 +18,14 @@ 1) // CHECK: {{^}}"-1" + +#define paste(a,b) str(a ".foo" not ". foo". if (i != 0 && !Tokens[i-1].is(tok::hashhash) && CurTok.hasLeadingSpace()) NextTokGetsSpace = true; @@ -317,14 +323,16 @@ const Token *ArgToks = ActualArgs->getUnexpArgument(ArgNo); unsigned NumToks = MacroArgs::getArgLength(ArgToks); if (NumToks) { // Not an empty argument? + bool VaArgsPseudoPaste = false; // If this is the GNU ", ## __VA_ARGS__" extension, and we just learned // that __VA_ARGS__ expands to multiple tokens, avoid a pasting error when // the expander trys to paste ',' with the first token of the __VA_ARGS__ // expansion. if (NonEmptyPasteBefore && ResultToks.size() >= 2 && ResultToks[ResultToks.size()-2].is(tok::comma) && (unsigned)ArgNo == Macro->getNumArgs()-1 && Macro->isVariadic()) { +VaArgsPseudoPaste = true; // Remove the paste operator, report use of the extension. PP.Diag(ResultToks.pop_back_val().getLocation(), diag::ext_paste_comma); } @@ -344,18 +352,16 @@ ResultToks.end()-NumToks, ResultToks.end()); } - // If this token (the macro argument) was supposed to get leading - // whitespace, transfer this information onto the first token of the - // expansion. - // - // Do not do this if the paste operator occurs before the macro argument, - // as in "A ## MACROARG". In valid code, the first token will get - // smooshed onto the preceding one anyway (forming AMACROARG). In - // assembler-with-cpp mode, invalid pastes are allowed through: in this - // case, we do not want the extra whitespace to be added. For example, - // we want ". ## foo" -> ".foo" not ". foo". - if (NextTokGetsSpace) -ResultToks[ResultToks.size()-NumToks].setFlag(Token::LeadingSpace); + // Transfer the leading whitespace information from the token + // (the macro argument) onto the first token of the + // expansion. Note that we don't do this for the GNU + // pseudo-paste extension ", ## __VA_ARGS__". + if (!VaArgsPseudoPaste) { +ResultToks[ResultToks.size() -
[PATCH] D30427: Fix whitespace before token-paste of an argument.
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. LGTM Comment at: test/Preprocessor/macro_paste_commaext.c:4 +// In the following tests, note that the output is sensitive to the +// whitespace *preceeding* the varargs argument, as well as to +// interior whitespace. AFAIK, this is the only case where whitespace Typo (too many 'e's in "preceding"). https://reviews.llvm.org/D30427 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30427: Fix whitespace before token-paste of an argument.
jyknight added a comment. Ping. https://reviews.llvm.org/D30427 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30427: Fix whitespace before token-paste of an argument.
jyknight added a comment. Ping https://reviews.llvm.org/D30427 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30427: Fix whitespace before token-paste of an argument.
jyknight created this revision. The whitespace should come from the argument name in the macro expansion, rather than from the token passed to the macro (same as it does when not pasting). Added a new test case for the change in behavior to stringize_space.c. FileCheck'ized macro_paste_commaext.c, tweaked the test case, and added a comment; no behavioral change to this test. https://reviews.llvm.org/D30427 Files: lib/Lex/TokenLexer.cpp test/Preprocessor/macro_paste_commaext.c test/Preprocessor/stringize_space.c Index: test/Preprocessor/stringize_space.c === --- test/Preprocessor/stringize_space.c +++ test/Preprocessor/stringize_space.c @@ -18,3 +18,14 @@ 1) // CHECK: {{^}}"-1" + +#define paste(a,b) str(a ".foo" not ". foo". if (i != 0 && !Tokens[i-1].is(tok::hashhash) && CurTok.hasLeadingSpace()) NextTokGetsSpace = true; @@ -317,14 +323,16 @@ const Token *ArgToks = ActualArgs->getUnexpArgument(ArgNo); unsigned NumToks = MacroArgs::getArgLength(ArgToks); if (NumToks) { // Not an empty argument? + bool VaArgsPseudoPaste = false; // If this is the GNU ", ## __VA_ARGS__" extension, and we just learned // that __VA_ARGS__ expands to multiple tokens, avoid a pasting error when // the expander trys to paste ',' with the first token of the __VA_ARGS__ // expansion. if (NonEmptyPasteBefore && ResultToks.size() >= 2 && ResultToks[ResultToks.size()-2].is(tok::comma) && (unsigned)ArgNo == Macro->getNumArgs()-1 && Macro->isVariadic()) { +VaArgsPseudoPaste = true; // Remove the paste operator, report use of the extension. PP.Diag(ResultToks.pop_back_val().getLocation(), diag::ext_paste_comma); } @@ -344,18 +352,16 @@ ResultToks.end()-NumToks, ResultToks.end()); } - // If this token (the macro argument) was supposed to get leading - // whitespace, transfer this information onto the first token of the - // expansion. - // - // Do not do this if the paste operator occurs before the macro argument, - // as in "A ## MACROARG". In valid code, the first token will get - // smooshed onto the preceding one anyway (forming AMACROARG). In - // assembler-with-cpp mode, invalid pastes are allowed through: in this - // case, we do not want the extra whitespace to be added. For example, - // we want ". ## foo" -> ".foo" not ". foo". - if (NextTokGetsSpace) -ResultToks[ResultToks.size()-NumToks].setFlag(Token::LeadingSpace); + // Transfer the leading whitespace information from the token + // (the macro argument) onto the first token of the + // expansion. Note that we don't do this for the GNU + // pseudo-paste extension ", ## __VA_ARGS__". + if (!VaArgsPseudoPaste) { +ResultToks[ResultToks.size()-NumToks].setFlagValue(Token::StartOfLine, +