[clang] [Clang] Add warning for non-portable include paths with trailing whitespace or dots (PR #190610)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 6 07:14:45 PDT 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Amelia Jochna (ameliajochna)
<details>
<summary>Changes</summary>
This patch extends -Wnonportable-include-path to detect and warn about trailing whitespace and dots in #include directives. Such paths are non-portable and can lead to build failures on different operating systems.
The warning is triggered when an include filename ends with a space or a dot, which is common when copy-pasting paths or due to typos.
---
Patch is 39.18 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/190610.diff
4 Files Affected:
- (modified) clang/include/clang/Basic/DiagnosticGroups.td (+2)
- (modified) clang/include/clang/Basic/DiagnosticLexKinds.td (+11-3)
- (modified) clang/lib/Lex/PPDirectives.cpp (+152-128)
- (added) clang/test/Preprocessor/nonportable-trailing-whitespace.c (+15)
``````````diff
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index dc7280c66ea79..e4491603df76c 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -944,6 +944,8 @@ def MemsetTransposedArgs : DiagGroup<"memset-transposed-args">;
def DynamicClassMemaccess : DiagGroup<"dynamic-class-memaccess">;
def NonTrivialMemcall : DiagGroup<"nontrivial-memcall">;
def NonTrivialMemaccess : DiagGroup<"nontrivial-memaccess", [NonTrivialMemcall]>;
+def NonportableIncludePath : DiagGroup<"nonportable-include-path">;
+def NonportableSystemIncludePath : DiagGroup<"nonportable-system-include-path">;
def SuspiciousBzero : DiagGroup<"suspicious-bzero">;
def SuspiciousMemaccess : DiagGroup<"suspicious-memaccess",
[SizeofPointerMemaccess, DynamicClassMemaccess,
diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td
index bea0aafac98cf..deb1111b46134 100644
--- a/clang/include/clang/Basic/DiagnosticLexKinds.td
+++ b/clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -373,13 +373,21 @@ def ext_missing_whitespace_after_macro_name : ExtWarn<
def warn_missing_whitespace_after_macro_name : Warning<
"whitespace recommended after macro name">;
-class NonportablePath : Warning<
+class NonportablePath : Warning<
"non-portable path to file '%0'; specified path differs in case from file"
" name on disk">;
def pp_nonportable_path : NonportablePath,
- InGroup<DiagGroup<"nonportable-include-path">>;
+ InGroup<NonportableIncludePath>;
def pp_nonportable_system_path : NonportablePath, DefaultIgnore,
- InGroup<DiagGroup<"nonportable-system-include-path">>;
+ InGroup<NonportableSystemIncludePath>;
+
+class NonportablePathTrailing : Warning<
+ "non-portable path to file '%0'; specified path contains trailing"
+ " %select{whitespace|dots}1">;
+def pp_nonportable_path_trailing : NonportablePathTrailing,
+ InGroup<NonportableIncludePath>;
+def pp_nonportable_system_path_trailing : NonportablePathTrailing, DefaultIgnore,
+ InGroup<NonportableSystemIncludePath>;
def pp_pragma_once_in_main_file : Warning<"#pragma once in main file">,
InGroup<DiagGroup<"pragma-once-outside-header">>;
diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index b90c04776ff9e..036e6f2556529 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -111,11 +111,7 @@ enum MacroDiag {
/// Enumerates possible %select values for the pp_err_elif_after_else and
/// pp_err_elif_without_if diagnostics.
-enum PPElifDiag {
- PED_Elif,
- PED_Elifdef,
- PED_Elifndef
-};
+enum PPElifDiag { PED_Elif, PED_Elifdef, PED_Elifndef };
static bool isFeatureTestMacro(StringRef MacroName) {
// list from:
@@ -227,13 +223,14 @@ static MacroDiag shouldWarnOnMacroUndef(Preprocessor &PP, IdentifierInfo *II) {
// and Boost headers. Improper case for these #includes is a
// potential portability issue.
static bool warnByDefaultOnWrongCase(StringRef Include) {
- // If the first component of the path is "boost", treat this like a standard header
- // for the purposes of diagnostics.
+ // If the first component of the path is "boost", treat this like a standard
+ // header for the purposes of diagnostics.
if (::llvm::sys::path::begin(Include)->equals_insensitive("boost"))
return true;
// "condition_variable" is the longest standard header name at 18 characters.
- // If the include file name is longer than that, it can't be a standard header.
+ // If the include file name is longer than that, it can't be a standard
+ // header.
static const size_t MaxStdHeaderNameLen = 18u;
if (Include.size() > MaxStdHeaderNameLen)
return false;
@@ -403,8 +400,7 @@ bool Preprocessor::CheckMacroName(Token &MacroNameTok, MacroUse isDefineUndef,
MacroDiag D = MD_NoWarn;
if (isDefineUndef == MU_Define) {
D = shouldWarnOnMacroDef(*this, II);
- }
- else if (isDefineUndef == MU_Undef)
+ } else if (isDefineUndef == MU_Undef)
D = shouldWarnOnMacroUndef(*this, II);
if (D == MD_KeywordDef) {
// We do not want to warn on some patterns widely used in configuration
@@ -484,7 +480,7 @@ Preprocessor::CheckEndOfDirective(StringRef DirType, bool EnableMacros,
// There should be no tokens after the directive, but we allow them as an
// extension.
- while (Tmp.is(tok::comment)) // Skip comments in -C mode.
+ while (Tmp.is(tok::comment)) // Skip comments in -C mode.
ReadNextTok(&Preprocessor::LexUnexpandedToken);
if (Tmp.is(tok::eod))
@@ -497,7 +493,7 @@ Preprocessor::CheckEndOfDirective(StringRef DirType, bool EnableMacros,
FixItHint Hint;
if ((LangOpts.GNUMode || LangOpts.C99 || LangOpts.CPlusPlus) &&
!CurTokenLexer)
- Hint = FixItHint::CreateInsertion(Tmp.getLocation(),"//");
+ Hint = FixItHint::CreateInsertion(Tmp.getLocation(), "//");
unsigned DiagID = diag::ext_pp_extra_tokens_at_eol;
// C++20 import or module directive has no '#' prefix.
@@ -513,11 +509,11 @@ void Preprocessor::SuggestTypoedDirective(const Token &Tok,
StringRef Directive) const {
// If this is a `.S` file, treat unknown # directives as non-preprocessor
// directives.
- if (getLangOpts().AsmPreprocessor) return;
+ if (getLangOpts().AsmPreprocessor)
+ return;
- std::vector<StringRef> Candidates = {
- "if", "ifdef", "ifndef", "elif", "else", "endif"
- };
+ std::vector<StringRef> Candidates = {"if", "ifdef", "ifndef",
+ "elif", "else", "endif"};
if (LangOpts.C23 || LangOpts.CPlusPlus23)
Candidates.insert(Candidates.end(), {"elifdef", "elifndef"});
@@ -708,7 +704,8 @@ void Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashTokenLoc,
// directive mode. Tell the lexer this so any newlines we see will be
// converted into an EOD token (this terminates the macro).
CurPPLexer->ParsingPreprocessorDirective = true;
- if (CurLexer) CurLexer->SetKeepWhitespaceMode(false);
+ if (CurLexer)
+ CurLexer->SetKeepWhitespaceMode(false);
assert(Tok.is(tok::hash));
const char *Hashptr = CurLexer->getBufferLocation() - Tok.getLength();
@@ -722,7 +719,8 @@ void Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashTokenLoc,
if (Tok.isNot(tok::raw_identifier)) {
CurPPLexer->ParsingPreprocessorDirective = false;
// Restore comment saving mode.
- if (CurLexer) CurLexer->resetExtendedTokenMode();
+ if (CurLexer)
+ CurLexer->resetExtendedTokenMode();
continue;
}
@@ -734,11 +732,12 @@ void Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashTokenLoc,
StringRef RI = Tok.getRawIdentifier();
char FirstChar = RI[0];
- if (FirstChar >= 'a' && FirstChar <= 'z' &&
- FirstChar != 'i' && FirstChar != 'e') {
+ if (FirstChar >= 'a' && FirstChar <= 'z' && FirstChar != 'i' &&
+ FirstChar != 'e') {
CurPPLexer->ParsingPreprocessorDirective = false;
// Restore comment saving mode.
- if (CurLexer) CurLexer->resetExtendedTokenMode();
+ if (CurLexer)
+ CurLexer->resetExtendedTokenMode();
continue;
}
@@ -755,7 +754,8 @@ void Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashTokenLoc,
if (IdLen >= 20) {
CurPPLexer->ParsingPreprocessorDirective = false;
// Restore comment saving mode.
- if (CurLexer) CurLexer->resetExtendedTokenMode();
+ if (CurLexer)
+ CurLexer->resetExtendedTokenMode();
continue;
}
memcpy(DirectiveBuf, &DirectiveStr[0], IdLen);
@@ -765,24 +765,25 @@ void Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashTokenLoc,
if (Directive.starts_with("if")) {
StringRef Sub = Directive.substr(2);
if (Sub.empty() || // "if"
- Sub == "def" || // "ifdef"
- Sub == "ndef") { // "ifndef"
+ Sub == "def" || // "ifdef"
+ Sub == "ndef") { // "ifndef"
// We know the entire #if/#ifdef/#ifndef block will be skipped, don't
// bother parsing the condition.
DiscardUntilEndOfDirective();
- CurPPLexer->pushConditionalLevel(Tok.getLocation(), /*wasskipping*/true,
- /*foundnonskip*/false,
- /*foundelse*/false);
+ CurPPLexer->pushConditionalLevel(Tok.getLocation(),
+ /*wasskipping*/ true,
+ /*foundnonskip*/ false,
+ /*foundelse*/ false);
} else {
SuggestTypoedDirective(Tok, Directive);
}
} else if (Directive[0] == 'e') {
StringRef Sub = Directive.substr(1);
- if (Sub == "ndif") { // "endif"
+ if (Sub == "ndif") { // "endif"
PPConditionalInfo CondInfo;
CondInfo.WasSkipping = true; // Silence bogus warning.
bool InCond = CurPPLexer->popConditionalLevel(CondInfo);
- (void)InCond; // Silence warning in no-asserts mode.
+ (void)InCond; // Silence warning in no-asserts mode.
assert(!InCond && "Can't be skipping if not in a conditional!");
// If we popped the outermost skipping block, we're done skipping!
@@ -828,9 +829,9 @@ void Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashTokenLoc,
Callbacks->Else(Tok.getLocation(), CondInfo.IfLoc);
break;
} else {
- DiscardUntilEndOfDirective(); // C99 6.10p4.
+ DiscardUntilEndOfDirective(); // C99 6.10p4.
}
- } else if (Sub == "lif") { // "elif".
+ } else if (Sub == "lif") { // "elif".
PPConditionalInfo &CondInfo = CurPPLexer->peekConditionalLevel();
if (!CondInfo.WasSkipping)
@@ -954,7 +955,8 @@ void Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashTokenLoc,
CurPPLexer->ParsingPreprocessorDirective = false;
// Restore comment saving mode.
- if (CurLexer) CurLexer->resetExtendedTokenMode();
+ if (CurLexer)
+ CurLexer->resetExtendedTokenMode();
}
// Finally, if we are out of the conditional (saw an #endif or ran off the end
@@ -1268,14 +1270,12 @@ OptionalFileEntryRef Preprocessor::LookupEmbedFile(StringRef Filename,
class Preprocessor::ResetMacroExpansionHelper {
public:
ResetMacroExpansionHelper(Preprocessor *pp)
- : PP(pp), save(pp->DisableMacroExpansion) {
+ : PP(pp), save(pp->DisableMacroExpansion) {
if (pp->MacroExpansionInDirectivesOverride)
pp->DisableMacroExpansion = false;
}
- ~ResetMacroExpansionHelper() {
- PP->DisableMacroExpansion = save;
- }
+ ~ResetMacroExpansionHelper() { PP->DisableMacroExpansion = save; }
private:
Preprocessor *PP;
@@ -1320,7 +1320,8 @@ void Preprocessor::HandleDirective(Token &Result) {
// mode. Tell the lexer this so any newlines we see will be converted into an
// EOD token (which terminates the directive).
CurPPLexer->ParsingPreprocessorDirective = true;
- if (CurLexer) CurLexer->SetKeepWhitespaceMode(false);
+ if (CurLexer)
+ CurLexer->SetKeepWhitespaceMode(false);
bool ImmediatelyAfterTopLevelIfndef =
CurPPLexer->MIOpt.getImmediatelyAfterTopLevelIfndef();
@@ -1331,7 +1332,8 @@ void Preprocessor::HandleDirective(Token &Result) {
// We are about to read a token. For the multiple-include optimization FA to
// work, we have to remember if we had read any tokens *before* this
// pp-directive.
- bool ReadAnyTokensBeforeDirective =CurPPLexer->MIOpt.getHasReadAnyTokensVal();
+ bool ReadAnyTokensBeforeDirective =
+ CurPPLexer->MIOpt.getHasReadAnyTokensVal();
// Save the directive-introducing token('#' and import/module in C++20) in
// case we need to return it later.
@@ -1392,14 +1394,14 @@ void Preprocessor::HandleDirective(Token &Result) {
// optimization, i.e. allow the null directive to appear outside of the
// include guard and still enable the multiple-include optimization.
CurPPLexer->MIOpt.SetReadToken(ReadAnyTokensBeforeDirective);
- return; // null directive.
+ return; // null directive.
case tok::code_completion:
setCodeCompletionReached();
if (CodeComplete)
CodeComplete->CodeCompleteDirective(
- CurPPLexer->getConditionalStackDepth() > 0);
+ CurPPLexer->getConditionalStackDepth() > 0);
return;
- case tok::numeric_constant: // # 7 GNU line marker directive.
+ case tok::numeric_constant: // # 7 GNU line marker directive.
// In a .S file "# 4" may be a comment so don't treat it as a preprocessor
// directive. However do permit it in the predefines file, as we use line
// markers to mark the builtin macros as being in a system header.
@@ -1409,11 +1411,13 @@ void Preprocessor::HandleDirective(Token &Result) {
return HandleDigitDirective(Result);
default:
IdentifierInfo *II = Result.getIdentifierInfo();
- if (!II) break; // Not an identifier.
+ if (!II)
+ break; // Not an identifier.
// Ask what the preprocessor keyword ID is.
switch (II->getPPKeywordID()) {
- default: break;
+ default:
+ break;
// C99 6.10.1 - Conditional Inclusion.
case tok::pp_if:
return HandleIfDirective(Result, Introducer,
@@ -1494,10 +1498,10 @@ void Preprocessor::HandleDirective(Token &Result) {
case tok::pp_embed:
return HandleEmbedDirective(Introducer.getLocation(), Result);
case tok::pp_assert:
- //isExtension = true; // FIXME: implement #assert
+ // isExtension = true; // FIXME: implement #assert
break;
case tok::pp_unassert:
- //isExtension = true; // FIXME: implement #unassert
+ // isExtension = true; // FIXME: implement #unassert
break;
case tok::pp___public_macro:
@@ -1531,7 +1535,7 @@ void Preprocessor::HandleDirective(Token &Result) {
// Enter this token stream so that we re-lex the tokens. Make sure to
// enable macro expansion, in case the token after the # is an identifier
// that is expanded.
- EnterTokenStream(std::move(Toks), 2, false, /*IsReinject*/false);
+ EnterTokenStream(std::move(Toks), 2, false, /*IsReinject*/ false);
return;
}
@@ -1547,9 +1551,8 @@ void Preprocessor::HandleDirective(Token &Result) {
/// GetLineValue - Convert a numeric token into an unsigned value, emitting
/// Diagnostic DiagID if it is invalid, and returning the value in Val.
-static bool GetLineValue(Token &DigitTok, unsigned &Val,
- unsigned DiagID, Preprocessor &PP,
- bool IsGNULineDirective=false) {
+static bool GetLineValue(Token &DigitTok, unsigned &Val, unsigned DiagID,
+ Preprocessor &PP, bool IsGNULineDirective = false) {
if (DigitTok.isNot(tok::numeric_constant)) {
PP.Diag(DigitTok, DiagID);
@@ -1578,12 +1581,13 @@ static bool GetLineValue(Token &DigitTok, unsigned &Val,
if (!isDigit(DigitTokBegin[i])) {
PP.Diag(PP.AdvanceToTokenCharacter(DigitTok.getLocation(), i),
- diag::err_pp_line_digit_sequence) << IsGNULineDirective;
+ diag::err_pp_line_digit_sequence)
+ << IsGNULineDirective;
PP.DiscardUntilEndOfDirective();
return true;
}
- unsigned NextVal = Val*10+(DigitTokBegin[i]-'0');
+ unsigned NextVal = Val * 10 + (DigitTokBegin[i] - '0');
if (NextVal < Val) { // overflow.
PP.Diag(DigitTok, DiagID);
PP.DiscardUntilEndOfDirective();
@@ -1594,7 +1598,7 @@ static bool GetLineValue(Token &DigitTok, unsigned &Val,
if (DigitTokBegin[0] == '0' && Val)
PP.Diag(DigitTok.getLocation(), diag::warn_pp_line_decimal)
- << IsGNULineDirective;
+ << IsGNULineDirective;
return false;
}
@@ -1614,7 +1618,7 @@ void Preprocessor::HandleLineDirective() {
// Validate the number and convert it to an unsigned.
unsigned LineNo;
- if (GetLineValue(DigitTok, LineNo, diag::err_pp_line_requires_integer,*this))
+ if (GetLineValue(DigitTok, LineNo, diag::err_pp_line_requires_integer, *this))
return;
if (LineNo == 0)
@@ -1690,7 +1694,8 @@ static bool ReadLineMarkerFlags(bool &IsFileEntry, bool &IsFileExit,
unsigned FlagVal;
Token FlagTok;
PP.Lex(FlagTok);
- if (FlagTok.is(tok::eod)) return false;
+ if (FlagTok.is(tok::eod))
+ return false;
if (GetLineValue(FlagTok, FlagVal, diag::err_pp_linemarker_invalid_flag, PP))
return true;
@@ -1698,8 +1703,10 @@ static bool ReadLineMarkerFlags(bool &IsFileEntry, bool &IsFileExit,
IsFileEntry = true;
PP.Lex(FlagTok);
- if (FlagTok.is(tok::eod)) return false;
- if (GetLineValue(FlagTok, FlagVal, diag::err_pp_linemarker_invalid_flag,PP))
+ if (FlagTok.is(tok::eod))
+ return false;
+ if (GetLineValue(FlagTok, FlagVal, diag::err_pp_linemarker_invalid_flag,
+ PP))
return true;
} else if (FlagVal == 2) {
IsFileExit = true;
@@ -1708,7 +1715,7 @@ static bool ReadLineMarkerFlags(bool &IsFileEntry, bool &IsFileExit,
// If we are leaving the current presumed file, check to make sure the
// presumed include stack isn't empty!
FileID CurFileID =
- SM.getDecomposedExpansionLoc(FlagTok.getLocation()).first;
+ SM.getDecomposedExpansionLoc(FlagTok.getLocation()).first;
PresumedLoc PLoc = SM.getPresumedLoc(FlagTok.getLocation());
if (PLoc.isInvalid())
return true;
@@ -1724,8 +1731,10 @@ static bool ReadLineMarkerFlags(bool &IsFileEntry, bool &IsFileExit,
}
PP.Lex(FlagTok);
- if (FlagTok.is(tok::eod)) return false;
- if (GetLineValue(FlagTok, FlagVal, diag::err_pp_linemarker_invalid_flag,PP))
+ if (FlagTok.is(tok::eod))
+ return false;
+ if (GetLineValue(FlagTok, FlagVal, diag::err_pp_linemarker_invalid_flag,
+ PP))
return true;
}
@@ -1739,7 +1748,8 @@ static bool ReadLineMarkerFlags(bool &IsFileEntry, bool &IsFileExit,
FileKind = SrcMgr::C_System;
PP.Lex(FlagTok);
- if (FlagTok.is(tok::eod)) return false;
+ if (FlagTok.is(tok::eod))
+ return false;
if (GetLineValue(FlagTok, FlagVal, diag::err_pp_linemarker_invalid_flag, PP))
return true;
@@ -1753,7 +1763,8 @@ static bool ReadLineMarkerFlags(bool &IsFileEntry, bool &IsFileExit,
FileKind = SrcMgr::C_ExternCSystem;
PP.Lex(FlagTok);
- if (FlagTok.is(tok::eod)) return false;
+ if (FlagTok.is(tok::eod))
+ return false;
// There are no more valid flags here.
PP.Diag(FlagTok, diag::err_pp_linemarker_invalid_flag);
@@ -1843,8 +1854,7 @@ void Preprocessor::HandleDigitDirective(Token &DigitTok) {
/// HandleUserDiagnosticDirective - Handle a #warning or #error directive.
///
-void Preprocessor::HandleUserDiagnosticDirective(Token &Tok,
- bool isWarning) {
+void Preprocessor::HandleUserDiagnosticDirective(Token &Tok, bool isWarning) {
// Read the rest of the line raw. We do this because we don't want macros
// to be expanded and we don't require that the tokens be valid preprocessing
// tokens. For example, this is allowed: "#warning ` 'foo". GCC does
@@ -1923,7 +1933,7 @@ void Preprocessor::HandleMacroPublicDirective(Token &Tok) {
// Note that this macro has now been exported.
appendMacroDirective(II, AllocateVisibilityMacroDirective(
- MacroNameTok.getLocation(), /*isPublic=*/true));
+ MacroNameTok.getLocation(), /*isPublic=*/true));
}
/// Handle a #private directive.
@@ -2006,13 +2016,12 @@ bool Preprocessor::GetIncludeFilenameSpelling(SourceLocation Loc,
}
// Skip the brackets.
- Buffer = Buffer.substr(1, Buffer.size()-2);
+ Buffer = Buffer.substr(1, Buffer.size() - 2);
return isAngled;
}
/// Push a token onto the token stream containing an annotation.
-void Preprocessor::EnterAnnotationToken(SourceRange Range,
- tok::TokenKind Kind,
+void Preprocessor::EnterAnnotationToken(SourceRange Range, tok::TokenKind Kind,
void *AnnotationVal) {
// FIXME: Produce this as the current token directly, rather than
// allocating a new token for it.
@@ -2214,8 +2223,8 @@ void Prepr...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/190610
More information about the cfe-commits
mailing list