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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
