From fbaa8b9ae5f3c6637be7d4dae6adaab4be811625 Mon Sep 17 00:00:00 2001 From: Argyrios Kyrtzidis Date: Tue, 7 Jun 2022 13:35:17 -0700 Subject: [PATCH] [Lex] Fix `fixits` for typo-corrections of preprocessing directives within skipped blocks The `EndLoc` parameter was always unset so no fixit was emitted. But it is also unnecessary for determining the range so we can remove it. Differential Revision: https://reviews.llvm.org/D127251 --- clang/include/clang/Lex/Preprocessor.h | 5 +---- clang/lib/Lex/PPDirectives.cpp | 22 ++++++++++++---------- clang/test/Preprocessor/suggest-typoed-directive.c | 17 +++++++++++++++-- 3 files changed, 28 insertions(+), 16 deletions(-) diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h index 6d8f03e..81d1481 100644 --- a/clang/include/clang/Lex/Preprocessor.h +++ b/clang/include/clang/Lex/Preprocessor.h @@ -2243,10 +2243,7 @@ private: /// /// \param Tok - Token that represents the directive /// \param Directive - String reference for the directive name - /// \param EndLoc - End location for fixit - void SuggestTypoedDirective(const Token &Tok, - StringRef Directive, - const SourceLocation &EndLoc) const; + void SuggestTypoedDirective(const Token &Tok, StringRef Directive) const; /// We just read a \#if or related directive and decided that the /// subsequent tokens are in the \#if'd out portion of the diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp index 97d7466..1356dc0 100644 --- a/clang/lib/Lex/PPDirectives.cpp +++ b/clang/lib/Lex/PPDirectives.cpp @@ -444,8 +444,7 @@ SourceLocation Preprocessor::CheckEndOfDirective(const char *DirType, } void Preprocessor::SuggestTypoedDirective(const Token &Tok, - StringRef Directive, - const SourceLocation &EndLoc) const { + StringRef Directive) const { // If this is a `.S` file, treat unknown # directives as non-preprocessor // directives. if (getLangOpts().AsmPreprocessor) return; @@ -457,11 +456,14 @@ void Preprocessor::SuggestTypoedDirective(const Token &Tok, Candidates.insert(Candidates.end(), {"elifdef", "elifndef"}); if (Optional Sugg = findSimilarStr(Directive, Candidates)) { - CharSourceRange DirectiveRange = - CharSourceRange::getCharRange(Tok.getLocation(), EndLoc); - std::string SuggValue = Sugg.getValue().str(); - - auto Hint = FixItHint::CreateReplacement(DirectiveRange, "#" + SuggValue); + // Directive cannot be coming from macro. + assert(Tok.getLocation().isFileID()); + CharSourceRange DirectiveRange = CharSourceRange::getCharRange( + Tok.getLocation(), + Tok.getLocation().getLocWithOffset(Directive.size())); + StringRef SuggValue = Sugg.getValue(); + + auto Hint = FixItHint::CreateReplacement(DirectiveRange, SuggValue); Diag(Tok, diag::warn_pp_invalid_directive) << 1 << SuggValue << Hint; } } @@ -596,7 +598,7 @@ void Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashTokenLoc, /*foundnonskip*/false, /*foundelse*/false); } else { - SuggestTypoedDirective(Tok, Directive, endLoc); + SuggestTypoedDirective(Tok, Directive); } } else if (Directive[0] == 'e') { StringRef Sub = Directive.substr(1); @@ -758,10 +760,10 @@ void Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashTokenLoc, } } } else { - SuggestTypoedDirective(Tok, Directive, endLoc); + SuggestTypoedDirective(Tok, Directive); } } else { - SuggestTypoedDirective(Tok, Directive, endLoc); + SuggestTypoedDirective(Tok, Directive); } CurPPLexer->ParsingPreprocessorDirective = false; diff --git a/clang/test/Preprocessor/suggest-typoed-directive.c b/clang/test/Preprocessor/suggest-typoed-directive.c index 8dd7ef4..cc15efe 100644 --- a/clang/test/Preprocessor/suggest-typoed-directive.c +++ b/clang/test/Preprocessor/suggest-typoed-directive.c @@ -1,6 +1,7 @@ // RUN: %clang_cc1 -fsyntax-only -verify=pre-c2x-cpp2b %s // RUN: %clang_cc1 -std=c2x -fsyntax-only -verify=c2x-cpp2b %s // RUN: %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify=c2x-cpp2b %s +// RUN: %clang_cc1 -x c++ -std=c++2b -fsyntax-only %s -fdiagnostics-parseable-fixits 2>&1 | FileCheck %s // id: pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#if'?}} // ifd: pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#if'?}} @@ -17,8 +18,8 @@ #id #ifd #ifde -#elf -#elsif +# elf +# elsif #elseif #elfidef #elfindef @@ -38,6 +39,18 @@ // els: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#else'?}} // endi: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#endif'?}} +// CHECK: fix-it:{{.*}}:{[[@LINE-24]]:2-[[@LINE-24]]:4}:"if" +// CHECK: fix-it:{{.*}}:{[[@LINE-24]]:2-[[@LINE-24]]:5}:"if" +// CHECK: fix-it:{{.*}}:{[[@LINE-24]]:2-[[@LINE-24]]:6}:"ifdef" +// CHECK: fix-it:{{.*}}:{[[@LINE-24]]:3-[[@LINE-24]]:6}:"elif" +// CHECK: fix-it:{{.*}}:{[[@LINE-24]]:4-[[@LINE-24]]:9}:"elif" +// CHECK: fix-it:{{.*}}:{[[@LINE-24]]:2-[[@LINE-24]]:8}:"elif" +// CHECK: fix-it:{{.*}}:{[[@LINE-24]]:2-[[@LINE-24]]:9}:"elifdef" +// CHECK: fix-it:{{.*}}:{[[@LINE-24]]:2-[[@LINE-24]]:10}:"elifdef" +// CHECK: fix-it:{{.*}}:{[[@LINE-24]]:2-[[@LINE-24]]:11}:"elifndef" +// CHECK: fix-it:{{.*}}:{[[@LINE-24]]:2-[[@LINE-24]]:5}:"else" +// CHECK: fix-it:{{.*}}:{[[@LINE-24]]:2-[[@LINE-24]]:6}:"endif" + #ifdef UNDEFINED #i // no diagnostic #endif -- 2.7.4