Suggest typo corrections for preprocessor directives

When a preprocessor directive is unknown outside of a skipped
conditional block, we give an error diagnostic because we don't know
how to proceed with preprocessing. But when the directive is in a
skipped conditional block, we would not diagnose it on the theory that
the directive may be known to an implementation other than Clang.

Now, for unknown directives inside a skipped conditional block, we
diagnose the unknown directive as a warning if it is sufficiently
similar to a directive specific to preprocessor conditional blocks. For
example, we'll warn about `#esle` and suggest `#else` but we won't warn
about `#progma` because it's not a directive specific to preprocessor
conditional blocks.

Fixes #51598

Differential Revision: https://reviews.llvm.org/D124726
This commit is contained in:
Ken Matsui 2022-05-13 09:09:32 -04:00 committed by Aaron Ballman
parent 92c645b5c1
commit a247ba9d15
5 changed files with 140 additions and 3 deletions

View File

@ -225,6 +225,13 @@ Improvements to Clang's diagnostics
- Clang now checks for stack resource exhaustion when recursively parsing
declarators in order to give a diagnostic before we run out of stack space.
This fixes `Issue 51642 <https://github.com/llvm/llvm-project/issues/51642>`_.
- Unknown preprocessor directives in a skipped conditional block are now given
a typo correction suggestion if the given directive is sufficiently similar
to another preprocessor conditional directive. For example, if ``#esle``
appears in a skipped block, we will warn about the unknown directive and
suggest ``#else`` as an alternative. ``#elifdef`` and ``#elifndef`` are only
suggested when in C2x or C++2b mode. Fixes
`Issue 51598 <https://github.com/llvm/llvm-project/issues/51598>`_.
Non-comprehensive list of changes in this release
-------------------------------------------------

View File

@ -430,8 +430,10 @@ def ext_pp_opencl_variadic_macros : Extension<
def ext_pp_gnu_line_directive : Extension<
"this style of line directive is a GNU extension">,
InGroup<GNULineMarker>;
def err_pp_invalid_directive : Error<"invalid preprocessing directive">;
def err_pp_invalid_directive : Error<
"invalid preprocessing directive%select{|, did you mean '#%1'?}0">;
def warn_pp_invalid_directive : Warning<
err_pp_invalid_directive.Text>, InGroup<DiagGroup<"unknown-directives">>;
def err_pp_directive_required : Error<
"%0 must be used within a preprocessing directive">;
def err_pp_file_not_found : Error<"'%0' file not found">, DefaultFatal;

View File

@ -2238,6 +2238,16 @@ private:
/// Return true if an error occurs parsing the arg list.
bool ReadMacroParameterList(MacroInfo *MI, Token& LastTok);
/// Provide a suggestion for a typoed directive. If there is no typo, then
/// just skip suggesting.
///
/// \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;
/// We just read a \#if or related directive and decided that the
/// subsequent tokens are in the \#if'd out portion of the
/// file. Lex the rest of the file, until we see an \#endif. If \p

View File

@ -266,6 +266,51 @@ static bool warnByDefaultOnWrongCase(StringRef Include) {
.Default(false);
}
/// Find a similar string in `Candidates`.
///
/// \param LHS a string for a similar string in `Candidates`
///
/// \param Candidates the candidates to find a similar string.
///
/// \returns a similar string if exists. If no similar string exists,
/// returns None.
static Optional<StringRef> findSimilarStr(
StringRef LHS, const std::vector<StringRef> &Candidates) {
// We need to check if `Candidates` has the exact case-insensitive string
// because the Levenshtein distance match does not care about it.
for (StringRef C : Candidates) {
if (LHS.equals_insensitive(C)) {
return C;
}
}
// Keep going with the Levenshtein distance match.
// If the LHS size is less than 3, use the LHS size minus 1 and if not,
// use the LHS size divided by 3.
size_t Length = LHS.size();
size_t MaxDist = Length < 3 ? Length - 1 : Length / 3;
Optional<std::pair<StringRef, size_t>> SimilarStr = None;
for (StringRef C : Candidates) {
size_t CurDist = LHS.edit_distance(C, true);
if (CurDist <= MaxDist) {
if (!SimilarStr.hasValue()) {
// The first similar string found.
SimilarStr = {C, CurDist};
} else if (CurDist < SimilarStr->second) {
// More similar string found.
SimilarStr = {C, CurDist};
}
}
}
if (SimilarStr.hasValue()) {
return SimilarStr->first;
} else {
return None;
}
}
bool Preprocessor::CheckMacroName(Token &MacroNameTok, MacroUse isDefineUndef,
bool *ShadowFlag) {
// Missing macro name?
@ -433,6 +478,25 @@ Optional<unsigned> Preprocessor::getSkippedRangeForExcludedConditionalBlock(
return BytesToSkip - LengthDiff;
}
void Preprocessor::SuggestTypoedDirective(const Token &Tok,
StringRef Directive,
const SourceLocation &EndLoc) const {
std::vector<StringRef> Candidates = {
"if", "ifdef", "ifndef", "elif", "else", "endif"
};
if (LangOpts.C2x || LangOpts.CPlusPlus2b)
Candidates.insert(Candidates.end(), {"elifdef", "elifndef"});
if (Optional<StringRef> Sugg = findSimilarStr(Directive, Candidates)) {
CharSourceRange DirectiveRange =
CharSourceRange::getCharRange(Tok.getLocation(), EndLoc);
std::string SuggValue = Sugg.getValue().str();
auto Hint = FixItHint::CreateReplacement(DirectiveRange, "#" + SuggValue);
Diag(Tok, diag::warn_pp_invalid_directive) << 1 << SuggValue << Hint;
}
}
/// SkipExcludedConditionalBlock - We just read a \#if or related directive and
/// decided that the subsequent tokens are in the \#if'd out portion of the
/// file. Lex the rest of the file, until we see an \#endif. If
@ -556,6 +620,8 @@ void Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashTokenLoc,
CurPPLexer->pushConditionalLevel(Tok.getLocation(), /*wasskipping*/true,
/*foundnonskip*/false,
/*foundelse*/false);
} else {
SuggestTypoedDirective(Tok, Directive, endLoc);
}
} else if (Directive[0] == 'e') {
StringRef Sub = Directive.substr(1);
@ -716,7 +782,11 @@ void Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashTokenLoc,
break;
}
}
} else {
SuggestTypoedDirective(Tok, Directive, endLoc);
}
} else {
SuggestTypoedDirective(Tok, Directive, endLoc);
}
CurPPLexer->ParsingPreprocessorDirective = false;
@ -1193,7 +1263,8 @@ void Preprocessor::HandleDirective(Token &Result) {
}
// If we reached here, the preprocessing token is not valid!
Diag(Result, diag::err_pp_invalid_directive);
// Start suggesting if a similar directive found.
Diag(Result, diag::err_pp_invalid_directive) << 0;
// Read the rest of the PP line.
DiscardUntilEndOfDirective();

View File

@ -0,0 +1,47 @@
// 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
// 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'?}}
// ifde: pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#ifdef'?}}
// elf: pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
// elsif: pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
// elseif: pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
// elfidef: not suggested to '#elifdef'
// elfindef: not suggested to '#elifdef'
// elfinndef: not suggested to '#elifndef'
// els: pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#else'?}}
// endi: pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#endif'?}}
#ifdef UNDEFINED
#id
#ifd
#ifde
#elf
#elsif
#elseif
#elfidef
#elfindef
#elfinndef
#els
#endi
#endif
// id: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#if'?}}
// ifd: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#if'?}}
// ifde: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#ifdef'?}}
// elf: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
// elsif: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
// elseif: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
// elfidef: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifdef'?}}
// elfindef: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifdef'?}}
// elfinndef: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifndef'?}}
// els: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#else'?}}
// endi: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#endif'?}}
#ifdef UNDEFINED
#i // no diagnostic
#endif
#if special_compiler
#special_compiler_directive // no diagnostic
#endif