Substantially restructure function-like macro argument parsing.
Highlights: PP::isNextPPTokenLParen() no longer eats the (
when present. We now simplify slightly the logic parsing
macro arguments. We now handle PR3937 and other related cases
correctly.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@69411 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/Lex/PPMacroExpansion.cpp b/lib/Lex/PPMacroExpansion.cpp
index 797a5bf..a682e86 100644
--- a/lib/Lex/PPMacroExpansion.cpp
+++ b/lib/Lex/PPMacroExpansion.cpp
@@ -136,13 +136,7 @@
// Okay, if we know that the token is a '(', lex it and return. Otherwise we
// have found something that isn't a '(' or we found the end of the
// translation unit. In either case, return false.
- if (Val != 1)
- return false;
-
- Token Tok;
- LexUnexpandedToken(Tok);
- assert(Tok.is(tok::l_paren) && "Error computing l-paren-ness?");
- return true;
+ return Val == 1;
}
/// HandleMacroExpandedIdentifier - If an identifier token is read that is to be
@@ -174,8 +168,7 @@
// If this is a function-like macro, read the arguments.
if (MI->isFunctionLike()) {
// C99 6.10.3p10: If the preprocessing token immediately after the the macro
- // name isn't a '(', this macro should not be expanded. Otherwise, consume
- // it.
+ // name isn't a '(', this macro should not be expanded.
if (!isNextPPTokenLParen())
return true;
@@ -277,9 +270,10 @@
return false;
}
-/// ReadFunctionLikeMacroArgs - After reading "MACRO(", this method is
-/// invoked to read all of the actual arguments specified for the macro
-/// invocation. This returns null on error.
+/// ReadFunctionLikeMacroArgs - After reading "MACRO" and knowing that the next
+/// token is the '(' of the macro, this method is invoked to read all of the
+/// actual arguments specified for the macro invocation. This returns null on
+/// error.
MacroArgs *Preprocessor::ReadFunctionLikeMacroArgs(Token &MacroName,
MacroInfo *MI,
SourceLocation &MacroEnd) {
@@ -289,16 +283,25 @@
// Outer loop, while there are more arguments, keep reading them.
Token Tok;
- Tok.setKind(tok::comma);
- --NumFixedArgsLeft; // Start reading the first arg.
+ // Read arguments as unexpanded tokens. This avoids issues, e.g., where
+ // an argument value in a macro could expand to ',' or '(' or ')'.
+ LexUnexpandedToken(Tok);
+ assert(Tok.is(tok::l_paren) && "Error computing l-paren-ness?");
+
// ArgTokens - Build up a list of tokens that make up each argument. Each
// argument is separated by an EOF token. Use a SmallVector so we can avoid
// heap allocations in the common case.
llvm::SmallVector<Token, 64> ArgTokens;
unsigned NumActuals = 0;
- while (Tok.is(tok::comma)) {
+ while (Tok.isNot(tok::r_paren)) {
+ assert((Tok.is(tok::l_paren) || Tok.is(tok::comma)) &&
+ "only expect argument separators here");
+
+ unsigned ArgTokenStart = ArgTokens.size();
+ SourceLocation ArgStartLoc = Tok.getLocation();
+
// C99 6.10.3p11: Keep track of the number of l_parens we have seen. Note
// that we already consumed the first one.
unsigned NumParens = 0;
@@ -323,17 +326,11 @@
++NumParens;
} else if (Tok.is(tok::comma) && NumParens == 0) {
// Comma ends this argument if there are more fixed arguments expected.
- if (NumFixedArgsLeft)
+ // However, if this is a variadic macro, and this is part of the
+ // variadic part, then the comma is just an argument token.
+ if (!isVariadic) break;
+ if (NumFixedArgsLeft > 1)
break;
-
- // If this is not a variadic macro, too many args were specified.
- if (!isVariadic) {
- // Emit the diagnostic at the macro name in case there is a missing ).
- // Emitting it at the , could be far away from the macro name.
- Diag(MacroName, diag::err_too_many_args_in_macro_invoc);
- return 0;
- }
- // Otherwise, continue to add the tokens to this variable argument.
} else if (Tok.is(tok::comment) && !KeepMacroComments) {
// If this is a comment token in the argument list and we're just in
// -C mode (not -CC mode), discard the comment.
@@ -351,10 +348,27 @@
}
ArgTokens.push_back(Tok);
}
+
+ // If this was an empty argument list foo(), don't add this as an empty
+ // argument.
+ if (ArgTokens.empty() && Tok.getKind() == tok::r_paren)
+ break;
+ // If this is not a variadic macro, and too many args were specified, emit
+ // an error.
+ if (!isVariadic && NumFixedArgsLeft == 0) {
+ if (ArgTokens.size() != ArgTokenStart)
+ ArgStartLoc = ArgTokens[ArgTokenStart].getLocation();
+
+ // Emit the diagnostic at the macro name in case there is a missing ).
+ // Emitting it at the , could be far away from the macro name.
+ Diag(ArgStartLoc, diag::err_too_many_args_in_macro_invoc);
+ return 0;
+ }
+
// Empty arguments are standard in C99 and supported as an extension in
// other modes.
- if (ArgTokens.empty() && !Features.C99)
+ if (ArgTokens.size() == ArgTokenStart && !Features.C99)
Diag(Tok, diag::ext_empty_fnmacro_arg);
// Add a marker EOF token to the end of the token list for this argument.
@@ -365,6 +379,7 @@
EOFTok.setLength(0);
ArgTokens.push_back(EOFTok);
++NumActuals;
+ assert(NumFixedArgsLeft != 0 && "Too many arguments parsed");
--NumFixedArgsLeft;
}
@@ -383,12 +398,18 @@
// A("blah")
Diag(Tok, diag::ext_missing_varargs_arg);
- // Remember this occurred if this is a macro invocation with at least
- // one actual argument. This allows us to elide the comma when used for
+ // Remember this occurred, allowing us to elide the comma when used for
// cases like:
// #define A(x, foo...) blah(a, ## foo)
- // #define A(x, ...) blah(a, ## __VA_ARGS__)
- isVarargsElided = MI->getNumArgs() > 1;
+ // #define B(x, ...) blah(a, ## __VA_ARGS__)
+ // #define C(...) blah(a, ## __VA_ARGS__)
+ // A(x) B(x) C()
+ isVarargsElided = true; //MI->getNumArgs() > 1;
+ } else if (NumActuals == 0 && MinArgsExpected == 1) {
+ assert(!MI->isVariadic() && "Variadic should be handled by case above");
+ // If there is exactly one argument, and that argument is missing,
+ // then we have an empty "()" argument empty list. This is fine, even if
+ // the macro expects one argument (the argument is just empty).
} else {
// Otherwise, emit the error.
Diag(Tok, diag::err_too_few_args_in_macro_invoc);
@@ -402,12 +423,11 @@
Tok.setLocation(EndLoc);
Tok.setLength(0);
ArgTokens.push_back(Tok);
- } else if (NumActuals == 1 && ArgTokens.size() == 1) {
- // If there is exactly one argument, and that argument is just an EOF token,
- // then we have an empty "()" argument empty list. This is fine, even if
- // the macro expects one argument (the argument is just empty). However, if
- // the macro expects "...", then we need to know that it was elided.
- isVarargsElided = MinArgsExpected == 1 && MI->isVariadic();
+ } else if (NumActuals > MinArgsExpected && !MI->isVariadic()) {
+ // Emit the diagnostic at the macro name in case there is a missing ).
+ // Emitting it at the , could be far away from the macro name.
+ Diag(MacroName, diag::err_too_many_args_in_macro_invoc);
+ return 0;
}
return MacroArgs::create(MI, &ArgTokens[0], ArgTokens.size(),isVarargsElided);
diff --git a/test/Preprocessor/macro_fn.c b/test/Preprocessor/macro_fn.c
new file mode 100644
index 0000000..8dc8b77
--- /dev/null
+++ b/test/Preprocessor/macro_fn.c
@@ -0,0 +1,27 @@
+/* RUN: clang-cc %s -Eonly -std=c89 -pedantic -verify
+*/
+/* PR3937 */
+#define zero() 0
+#define one(x) 0
+#define two(x, y) 0
+
+zero()
+zero(1); /* expected-error {{too many arguments provided to function-like macro invocation}} */
+zero(1, 2, 3); /* expected-error {{too many arguments provided to function-like macro invocation}} */
+
+one() /* ok */
+one(a)
+one(a,) /* expected-error {{too many arguments provided to function-like macro invocation}} */
+one(a, b) /* expected-error {{too many arguments provided to function-like macro invocation}} */
+
+two() /* expected-error {{too few arguments provided to function-like macro invocation}} */
+two(a) /* expected-error {{too few arguments provided to function-like macro invocation}} */
+two(a,b)
+two(a, ) /* expected-warning {{empty macro arguments were standardized in C99}} */
+two(a,b,c) /* expected-error {{too many arguments provided to function-like macro invocation}} */
+two(
+ , /* expected-warning {{empty macro arguments were standardized in C99}} */
+ , /* expected-warning {{empty macro arguments were standardized in C99}} \
+ expected-error {{too many arguments provided to function-like macro invocation}} */
+ )
+two(,) /* expected-warning 2 {{empty macro arguments were standardized in C99}} */