ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added a project: clang.
Preivously we would only discard it if we failed to parse parameter lists. If we do not consume the body, parser sees tokens inside directive. In turns, this leads to spurious diagnostics and crash in TokenBuffer, see the added tests. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D65517 Files: clang/lib/Lex/PPDirectives.cpp clang/test/Preprocessor/stringize_skipped.c clang/unittests/Tooling/Syntax/TokensTest.cpp Index: clang/unittests/Tooling/Syntax/TokensTest.cpp =================================================================== --- clang/unittests/Tooling/Syntax/TokensTest.cpp +++ clang/unittests/Tooling/Syntax/TokensTest.cpp @@ -298,6 +298,21 @@ spelled tokens: <empty> no mappings. +)"}, + // Should not crash on errors inside '#define' directives. Error is that + // stringification (#B) does not refer to a macro parameter. + { + R"cpp( +a +#define MACRO() A #B +)cpp", + R"(expanded tokens: + a +file './input.cpp' + spelled tokens: + a # define MACRO ( ) A # B + mappings: + ['#'_1, '<eof>'_9) => ['<eof>'_1, '<eof>'_1) )"}}; for (auto &Test : TestCases) EXPECT_EQ(collectAndDump(Test.first), Test.second) Index: clang/test/Preprocessor/stringize_skipped.c =================================================================== --- /dev/null +++ clang/test/Preprocessor/stringize_skipped.c @@ -0,0 +1,5 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s +// Ensure we see the error from PP and do not see errors from the parser. + +// expected-error@+1{{'#' is not followed by a macro parameter}} +#define INVALID() #B 10+10 Index: clang/lib/Lex/PPDirectives.cpp =================================================================== --- clang/lib/Lex/PPDirectives.cpp +++ clang/lib/Lex/PPDirectives.cpp @@ -33,6 +33,7 @@ #include "clang/Lex/Token.h" #include "clang/Lex/VariadicMacroSupport.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/STLExtras.h" @@ -2399,6 +2400,13 @@ Token Tok; LexUnexpandedToken(Tok); + // Ensure we consume the rest of the macro body if errors occur. + auto _ = llvm::make_scope_exit([&]() { + // The flag indicates if we have already reached 'eod'. + if (CurLexer->ParsingPreprocessorDirective) + DiscardUntilEndOfDirective(); + }); + // Used to un-poison and then re-poison identifiers of the __VA_ARGS__ ilk // within their appropriate context. VariadicMacroScopeGuard VariadicMacroScopeGuard(*this); @@ -2420,12 +2428,8 @@ } else if (Tok.is(tok::l_paren)) { // This is a function-like macro definition. Read the argument list. MI->setIsFunctionLike(); - if (ReadMacroParameterList(MI, LastTok)) { - // Throw away the rest of the line. - if (CurPPLexer->ParsingPreprocessorDirective) - DiscardUntilEndOfDirective(); + if (ReadMacroParameterList(MI, LastTok)) return nullptr; - } // If this is a definition of an ISO C/C++ variadic function-like macro (not // using the GNU named varargs extension) inform our variadic scope guard
Index: clang/unittests/Tooling/Syntax/TokensTest.cpp =================================================================== --- clang/unittests/Tooling/Syntax/TokensTest.cpp +++ clang/unittests/Tooling/Syntax/TokensTest.cpp @@ -298,6 +298,21 @@ spelled tokens: <empty> no mappings. +)"}, + // Should not crash on errors inside '#define' directives. Error is that + // stringification (#B) does not refer to a macro parameter. + { + R"cpp( +a +#define MACRO() A #B +)cpp", + R"(expanded tokens: + a +file './input.cpp' + spelled tokens: + a # define MACRO ( ) A # B + mappings: + ['#'_1, '<eof>'_9) => ['<eof>'_1, '<eof>'_1) )"}}; for (auto &Test : TestCases) EXPECT_EQ(collectAndDump(Test.first), Test.second) Index: clang/test/Preprocessor/stringize_skipped.c =================================================================== --- /dev/null +++ clang/test/Preprocessor/stringize_skipped.c @@ -0,0 +1,5 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s +// Ensure we see the error from PP and do not see errors from the parser. + +// expected-error@+1{{'#' is not followed by a macro parameter}} +#define INVALID() #B 10+10 Index: clang/lib/Lex/PPDirectives.cpp =================================================================== --- clang/lib/Lex/PPDirectives.cpp +++ clang/lib/Lex/PPDirectives.cpp @@ -33,6 +33,7 @@ #include "clang/Lex/Token.h" #include "clang/Lex/VariadicMacroSupport.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/STLExtras.h" @@ -2399,6 +2400,13 @@ Token Tok; LexUnexpandedToken(Tok); + // Ensure we consume the rest of the macro body if errors occur. + auto _ = llvm::make_scope_exit([&]() { + // The flag indicates if we have already reached 'eod'. + if (CurLexer->ParsingPreprocessorDirective) + DiscardUntilEndOfDirective(); + }); + // Used to un-poison and then re-poison identifiers of the __VA_ARGS__ ilk // within their appropriate context. VariadicMacroScopeGuard VariadicMacroScopeGuard(*this); @@ -2420,12 +2428,8 @@ } else if (Tok.is(tok::l_paren)) { // This is a function-like macro definition. Read the argument list. MI->setIsFunctionLike(); - if (ReadMacroParameterList(MI, LastTok)) { - // Throw away the rest of the line. - if (CurPPLexer->ParsingPreprocessorDirective) - DiscardUntilEndOfDirective(); + if (ReadMacroParameterList(MI, LastTok)) return nullptr; - } // If this is a definition of an ISO C/C++ variadic function-like macro (not // using the GNU named varargs extension) inform our variadic scope guard
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits