From 2b596dae66948859a5a62b6d3c8771b96ed5fe3c Mon Sep 17 00:00:00 2001 From: Kim Grasman Date: Sun, 13 Jan 2019 19:35:12 +0100 Subject: [PATCH] Simplify handling of defined() operator Clang r350891 changed the raw lexer so it asserts for IWYU's use case. Instead of re-lexing to catch symbols inside defined(), use a PPCallbacks::Defined override to capture the tokens directly (presumably, Defined did not exist when the open-coded parser was added). This removes a lot of code, including the PPCallbacks for If and Elif, which are no longer necessary. No functional change. --- iwyu_lexer_utils.cc | 61 ----------------------------- iwyu_lexer_utils.h | 9 ----- iwyu_preprocessor.cc | 44 ++++++--------------- iwyu_preprocessor.h | 15 +++---- more_tests/iwyu_lexer_utils_test.cc | 61 ----------------------------- 5 files changed, 18 insertions(+), 172 deletions(-) diff --git a/iwyu_lexer_utils.cc b/iwyu_lexer_utils.cc index 9a3545f..0475600 100644 --- a/iwyu_lexer_utils.cc +++ b/iwyu_lexer_utils.cc @@ -10,28 +10,19 @@ #include "iwyu_lexer_utils.h" #include "iwyu_globals.h" -#include #include -#include -#include "iwyu_verrs.h" #include "port.h" -#include "llvm/Support/raw_ostream.h" -#include "clang/Basic/LangOptions.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" -#include "clang/Lex/Lexer.h" #include "clang/Lex/Token.h" -using clang::Lexer; -using clang::LangOptions; using clang::SourceLocation; using clang::SourceManager; using clang::SourceRange; using clang::Token; using llvm::StringRef; using std::string; -using std::vector; namespace include_what_you_use { @@ -101,56 +92,4 @@ string GetTokenText(const Token& token, return string(text, token.getLength()); } -// Given the range of an #if or #elif statement, determine the -// symbols which are arguments to "defined". This allows iwyu to -// treat these symbols as if #ifdef was used instead. -vector FindArgumentsToDefined( - SourceRange range, - const CharacterDataGetterInterface& data_getter) { - const char* text = data_getter.GetCharacterData(range.getBegin()); - const char* text_end = data_getter.GetCharacterData(range.getEnd()); - - // Ugh. The lexer wants the text to be nul-terminated. Make a copy. - const unsigned range_length = text_end - text; - const string range_str(text, range_length); - const char* range_cstr = range_str.c_str(); - - VERRS(8) << "Lexing: " << range_str << "\n"; - Lexer lexer(range.getBegin(), LangOptions(), range_cstr, range_cstr, - range_cstr + range_length); - - vector ret; - Token token; - enum { kLookingForDefined, - kExpectingLeftParenOrDefinedIdentifier, - kExpectingDefinedIdentifier } state = kLookingForDefined; - while (!lexer.LexFromRawLexer(token)) { - VERRS(8) << "Processing token \"" - << GetTokenText(token, data_getter) - << "\" of type " << token.getName() - << " in state " << state << "\n"; - switch (state) { - case kLookingForDefined: - if (token.getKind() == clang::tok::raw_identifier) { - if (GetTokenText(token, data_getter) == "defined") { - state = kExpectingLeftParenOrDefinedIdentifier; - } - } - break; - case kExpectingLeftParenOrDefinedIdentifier: - if (token.getKind() == clang::tok::l_paren) { - state = kExpectingDefinedIdentifier; - continue; - } - // Fall through. - case kExpectingDefinedIdentifier: - CHECK_(token.getKind() == clang::tok::raw_identifier); - ret.push_back(token); - state = kLookingForDefined; - break; - } - } - return ret; -} - } // namespace include_what_you_use diff --git a/iwyu_lexer_utils.h b/iwyu_lexer_utils.h index 872b5e5..4311508 100644 --- a/iwyu_lexer_utils.h +++ b/iwyu_lexer_utils.h @@ -11,7 +11,6 @@ #define INCLUDE_WHAT_YOU_USE_IWYU_LEXER_UTILS_H_ #include // for string -#include // for vector #include "clang/Basic/SourceLocation.h" @@ -23,7 +22,6 @@ class Token; namespace include_what_you_use { using std::string; -using std::vector; // For a particular source line that source_location points to, // returns true if the given text occurs on the line. @@ -71,13 +69,6 @@ string GetIncludeNameAsWritten( clang::SourceLocation include_loc, const CharacterDataGetterInterface& data_getter); -// Given the range of an #if or #elif statement, determine the -// symbols which are arguments to "defined". This allows iwyu to -// treat these symbols as if #ifdef was used instead. -vector FindArgumentsToDefined( - clang::SourceRange range, - const CharacterDataGetterInterface& data_getter); - // Get the text of a given token. string GetTokenText(const clang::Token& token, const CharacterDataGetterInterface& data_getter); diff --git a/iwyu_preprocessor.cc b/iwyu_preprocessor.cc index 91ea106..71610c1 100644 --- a/iwyu_preprocessor.cc +++ b/iwyu_preprocessor.cc @@ -609,24 +609,6 @@ void IwyuPreprocessorInfo::MacroDefined(const Token& id, } } -void IwyuPreprocessorInfo::If(SourceLocation loc, SourceRange condition_range, - ConditionValueKind condition_value) { - ERRSYM(GetFileEntry(condition_range.getBegin())) - << " [ #if ] " - << PrintableSourceRange(condition_range) << "\n"; - CheckIfOrElif(condition_range); -} - -void IwyuPreprocessorInfo::Elif(SourceLocation loc, - SourceRange condition_range, - ConditionValueKind condition_value, - SourceLocation if_loc) { - ERRSYM(GetFileEntry(condition_range.getBegin())) - << " [ #elif ] " - << PrintableSourceRange(condition_range) << "\n"; - CheckIfOrElif(condition_range); -} - void IwyuPreprocessorInfo::Ifdef(SourceLocation loc, const Token& id, const MacroDefinition& /*definition*/) { @@ -645,6 +627,18 @@ void IwyuPreprocessorInfo::Ifndef(SourceLocation loc, FindAndReportMacroUse(GetName(id), id.getLocation()); } +// Clang will give a MacroExpands() callback for all macro-tokens +// used inside an #if or #elif, *except* macro-tokens used within a +// 'defined' operator. They produce a Defined() callback. +void IwyuPreprocessorInfo::Defined(const Token& id, + const MacroDefinition& /*definition*/, + SourceRange /*range*/) { + ERRSYM(GetFileEntry(id.getLocation())) + << "[ #if defined ] " << PrintableLoc(id.getLocation()) + << ": " << GetName(id) << "\n"; + FindAndReportMacroUse(GetName(id), id.getLocation()); +} + void IwyuPreprocessorInfo::InclusionDirective( SourceLocation hash_loc, const Token& include_token, @@ -825,20 +819,6 @@ void IwyuPreprocessorInfo::FindAndReportMacroUse(const string& name, } } -// Clang will give an OnExpandMacro() callback for all macro-tokens -// used inside an #if or #elif, *except* macro-tokens used within a -// 'define': for '#if FOO || defined(BAR)', clang calls -// OnExpandMacro() for FOO, but not for BAR (since macros within -// defined() aren't expanded). We catch BAR-type uses here. -void IwyuPreprocessorInfo::CheckIfOrElif(SourceRange range) { - const vector defined_args = - FindArgumentsToDefined(range, DefaultDataGetter()); // in iwyu_lexer.h - for (const Token& token : defined_args) { - FindAndReportMacroUse(GetTokenText(token, DefaultDataGetter()), - token.getLocation()); - } -} - //------------------------------------------------------------ // Post-processing functions (done after all source is read). diff --git a/iwyu_preprocessor.h b/iwyu_preprocessor.h index 85c6e98..76eabef 100644 --- a/iwyu_preprocessor.h +++ b/iwyu_preprocessor.h @@ -177,18 +177,17 @@ class IwyuPreprocessorInfo : public clang::PPCallbacks, // Not needed for iwyu: // virtual void MacroUndefined(const clang::Token&, const clang::MacroInfo*); - void If(clang::SourceLocation loc, - clang::SourceRange condition_range, - ConditionValueKind condition_value) override; - void Elif(clang::SourceLocation loc, - clang::SourceRange condition_range, - ConditionValueKind condition_value, - clang::SourceLocation if_loc) override; void Ifdef(clang::SourceLocation loc, const clang::Token& id, const clang::MacroDefinition& definition) override; void Ifndef(clang::SourceLocation loc, const clang::Token& id, const clang::MacroDefinition& definition) override; + void Defined(const clang::Token& id, + const clang::MacroDefinition& definition, + clang::SourceRange range) override; + // Not needed for iwyu: + // virtual void If(); + // virtual void Elif(); // virtual void Else(); // virtual void Endif(); @@ -270,8 +269,6 @@ class IwyuPreprocessorInfo : public clang::PPCallbacks, // As above, but get the definition location from macros_definition_loc_. void FindAndReportMacroUse(const string& name, clang::SourceLocation loc); - void CheckIfOrElif(clang::SourceRange range); - // Final-processing routines done after all header files have been read. void DoFinalMacroChecks(); // Helper for PopulateIntendsToProvideMap(). diff --git a/more_tests/iwyu_lexer_utils_test.cc b/more_tests/iwyu_lexer_utils_test.cc index 2669af0..e38c392 100644 --- a/more_tests/iwyu_lexer_utils_test.cc +++ b/more_tests/iwyu_lexer_utils_test.cc @@ -34,7 +34,6 @@ using clang::Token; namespace iwyu = include_what_you_use; using iwyu::CharacterDataGetterInterface; -using iwyu::FindArgumentsToDefined; namespace { @@ -67,66 +66,6 @@ class StringCharacterDataGetter : public CharacterDataGetterInterface { string str_; }; -TEST(LexerTest, ClangLexer) { - // Not so much a test as an example of how to use the Lexer. - const char text[] = "#if defined(FOO)"; - Lexer lexer(SourceLocation(), LangOptions(), text, text, text + strlen(text)); - - Token token; - while (!lexer.LexFromRawLexer(token)) { - printf("Token: %s at %u length %u: \"%s\"\n", - token.getName(), token.getLocation().getRawEncoding(), - token.getLength(), - string(text + token.getLocation().getRawEncoding(), - token.getLength()).c_str()); - } -} - -// Common test code for testing FindArgumentsToDefined. The symbols -// should be the arguments to defined() in order. -void TestFindArgumentsToDefinedWithText(const string& text, - const vector& symbols) { - StringCharacterDataGetter data_getter(text); - SourceLocation begin_loc = data_getter.BeginningOfString(); - SourceLocation end_loc = - CreateSourceLocationFromOffset(begin_loc, text.size()); - - vector defined_tokens = FindArgumentsToDefined( - SourceRange(begin_loc, end_loc), data_getter); - EXPECT_EQ(symbols.size(), defined_tokens.size()); - for (int i = 0; i < symbols.size(); ++i) { - const string& symbol = symbols[i]; - const Token& token = defined_tokens[i]; - EXPECT_EQ(clang::tok::raw_identifier, token.getKind()); - SourceLocation expected_loc = - CreateSourceLocationFromOffset(begin_loc, text.find(symbol)); - EXPECT_EQ(expected_loc, token.getLocation()); - EXPECT_EQ(symbol.size(), token.getLength()); - EXPECT_EQ(symbol, GetTokenText(token, data_getter)); - } -} - -TEST(FindArgumentsToDefined, InParentheses) { - vector symbols; - symbols.push_back("FOO"); - TestFindArgumentsToDefinedWithText("#if defined(FOO)\n", symbols); -} - -TEST(FindArgumentsToDefined, NoParentheses) { - vector symbols; - symbols.push_back("FOO"); - TestFindArgumentsToDefinedWithText("#if defined FOO\n", symbols); -} - -TEST(FindArgumentsToDefined, MultipleArgs) { - vector symbols; - symbols.push_back("FOO"); - symbols.push_back("BAR"); - symbols.push_back("BAZ"); - TestFindArgumentsToDefinedWithText( - "#if defined FOO || defined(BAR) || !defined(BAZ)\n", symbols); -} - TEST(GetSourceTextUntilEndOfLine, FullLine) { const char text[] = "This is the full line.\n"; StringCharacterDataGetter data_getter(text);