Author: Jinjie Huang Date: 2025-12-03T14:08:20+08:00 New Revision: 689b3cc7c700b1687cf4aaaf4ef2c81a4e988917
URL: https://github.com/llvm/llvm-project/commit/689b3cc7c700b1687cf4aaaf4ef2c81a4e988917 DIFF: https://github.com/llvm/llvm-project/commit/689b3cc7c700b1687cf4aaaf4ef2c81a4e988917.diff LOG: [clang] Support header shadowing diagnostics in Clang header search (#162491) When including a header file, multiple files with the same name may exist across different search paths, like:   |-- main.cpp   |-- **header.h**   |-- include   |  └── **header.h** The compiler usually picks the first match it finds (typically following MSVC rules for current/include-chain paths first, then regular -I paths), which may not be the user’s intended header. This silent behavior can lead to subtle runtime API mismatches or increase the cost of resolving errors such as “error: use of undeclared identifier”, especially in large projects. Therefore, this patch tries to provide a diagnostic message without changing the current header selection. It does this by performing an additional search for duplicate filenames across all search paths (both MSVC rules and standard paths). This informs the user about a potential "header shadowing" issue and clarifies which header path was actually used. Since header searching is much cheaper than file loading, the added overhead should be within an acceptable range -- assuming the diagnostic message is valuable. Added: clang/test/Preprocessor/header-shadowing.c Modified: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/DiagnosticGroups.td clang/include/clang/Basic/DiagnosticLexKinds.td clang/include/clang/Lex/HeaderSearch.h clang/lib/Lex/HeaderSearch.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 3526ffb40f350..aa172ce402f8f 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -449,6 +449,9 @@ Improvements to Clang's diagnostics comparison operators when mixed with bitwise operators in enum value initializers. This can be locally disabled by explicitly casting the initializer value. +- A new warning ``-Wshadow-header`` has been added to detect when a header file + is found in multiple search directories (excluding system paths). + Improvements to Clang's time-trace ---------------------------------- diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 063957e7b18ae..80fc12caa1d24 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -790,6 +790,8 @@ def ShadowFieldInConstructor : DiagGroup<"shadow-field-in-constructor", def ShadowIvar : DiagGroup<"shadow-ivar">; def ShadowUncapturedLocal : DiagGroup<"shadow-uncaptured-local">; +def ShadowHeader : DiagGroup<"shadow-header">; + // -Wshadow-all is a catch-all for all shadowing. -Wshadow is just the // shadowing that we think is unsafe. def Shadow : DiagGroup<"shadow", [ShadowFieldInConstructorModified, diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td index 417187222e448..a72d3f37b1b72 100644 --- a/clang/include/clang/Basic/DiagnosticLexKinds.td +++ b/clang/include/clang/Basic/DiagnosticLexKinds.td @@ -959,6 +959,10 @@ def warn_quoted_include_in_framework_header : Warning< def warn_framework_include_private_from_public : Warning< "public framework header includes private framework header '%0'" >, InGroup<FrameworkIncludePrivateFromPublic>; +def warn_header_shadowing : Warning< + "multiple candidates for header '%0' found; " + "directory '%1' chosen, ignoring others including '%2'">, + InGroup<ShadowHeader>, DefaultIgnore; def warn_deprecated_module_dot_map : Warning< "'%0' as a module map name is deprecated, rename it to %select{module.modulemap|module.private.modulemap}1%select{| in the 'Modules' directory of the framework}2">, InGroup<DeprecatedModuleDotMap>; diff --git a/clang/include/clang/Lex/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h index 850aea41c4c3b..5369c872ac1cd 100644 --- a/clang/include/clang/Lex/HeaderSearch.h +++ b/clang/include/clang/Lex/HeaderSearch.h @@ -465,6 +465,12 @@ class HeaderSearch { ExternalSource = ES; } + void diagnoseHeaderShadowing( + StringRef Filename, OptionalFileEntryRef FE, bool &DiagnosedShadowing, + SourceLocation IncludeLoc, ConstSearchDirIterator FromDir, + ArrayRef<std::pair<OptionalFileEntryRef, DirectoryEntryRef>> Includers, + bool isAngled, int IncluderLoopIndex, ConstSearchDirIterator MainLoopIt); + /// Set the target information for the header search, if not /// already known. void setTarget(const TargetInfo &Target); diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp index f05c28fd7a123..b2ed24f765dab 100644 --- a/clang/lib/Lex/HeaderSearch.cpp +++ b/clang/lib/Lex/HeaderSearch.cpp @@ -881,6 +881,66 @@ diagnoseFrameworkInclude(DiagnosticsEngine &Diags, SourceLocation IncludeLoc, << IncludeFilename; } +void HeaderSearch::diagnoseHeaderShadowing( + StringRef Filename, OptionalFileEntryRef FE, bool &DiagnosedShadowing, + SourceLocation IncludeLoc, ConstSearchDirIterator FromDir, + ArrayRef<std::pair<OptionalFileEntryRef, DirectoryEntryRef>> Includers, + bool isAngled, int IncluderLoopIndex, ConstSearchDirIterator MainLoopIt) { + + if (Diags.isIgnored(diag::warn_header_shadowing, IncludeLoc) || + DiagnosedShadowing) + return; + // Ignore diagnostics from system headers. + if (MainLoopIt && MainLoopIt->isSystemHeaderDirectory()) + return; + + DiagnosedShadowing = true; + + // Indicates that file is first found in the includer's directory + if (!MainLoopIt) { + for (size_t i = IncluderLoopIndex + 1; i < Includers.size(); ++i) { + const auto &IncluderAndDir = Includers[i]; + SmallString<1024> TmpDir = IncluderAndDir.second.getName(); + llvm::sys::path::append(TmpDir, Filename); + if (auto File = getFileMgr().getFileRef(TmpDir, false, false)) { + if (&File->getFileEntry() == *FE) + continue; + Diags.Report(IncludeLoc, diag::warn_header_shadowing) + << Filename << (*FE).getDir().getName() + << IncluderAndDir.second.getName(); + return; + } else { + llvm::errorToErrorCode(File.takeError()); + } + } + } + + // Continue searching in the regular search paths + ConstSearchDirIterator It = + isAngled ? angled_dir_begin() : search_dir_begin(); + if (MainLoopIt) { + It = std::next(MainLoopIt); + } else if (FromDir) { + It = FromDir; + } + for (; It != search_dir_end(); ++It) { + // Suppress check for system headers, as duplicates are often intentional. + if (It->getDirCharacteristic() != SrcMgr::C_User) + continue; + SmallString<1024> TmpPath = It->getName(); + llvm::sys::path::append(TmpPath, Filename); + if (auto File = getFileMgr().getFileRef(TmpPath, false, false)) { + if (&File->getFileEntry() == *FE) + continue; + Diags.Report(IncludeLoc, diag::warn_header_shadowing) + << Filename << (*FE).getDir().getName() << It->getName(); + return; + } else { + llvm::errorToErrorCode(File.takeError()); + } + } +} + /// LookupFile - Given a "foo" or \<foo> reference, look up the indicated file, /// return null on failure. isAngled indicates whether the file reference is /// for system \#include's or not (i.e. using <> instead of ""). Includers, if @@ -930,6 +990,7 @@ OptionalFileEntryRef HeaderSearch::LookupFile( // This is the header that MSVC's header search would have found. ModuleMap::KnownHeader MSSuggestedModule; OptionalFileEntryRef MSFE; + bool DiagnosedShadowing = false; // Check to see if the file is in the #includer's directory. This cannot be // based on CurDir, because each includer could be a #include of a @@ -963,6 +1024,9 @@ OptionalFileEntryRef HeaderSearch::LookupFile( if (OptionalFileEntryRef FE = getFileAndSuggestModule( TmpDir, IncludeLoc, IncluderAndDir.second, IncluderIsSystemHeader, RequestingModule, SuggestedModule)) { + diagnoseHeaderShadowing(Filename, FE, DiagnosedShadowing, IncludeLoc, + FromDir, Includers, isAngled, + &IncluderAndDir - Includers.begin(), nullptr); if (!Includer) { assert(First && "only first includer can have no file"); return FE; @@ -1097,6 +1161,9 @@ OptionalFileEntryRef HeaderSearch::LookupFile( if (!File) continue; + diagnoseHeaderShadowing(Filename, File, DiagnosedShadowing, IncludeLoc, + FromDir, Includers, isAngled, -1, It); + CurDir = It; IncludeNames[*File] = Filename; diff --git a/clang/test/Preprocessor/header-shadowing.c b/clang/test/Preprocessor/header-shadowing.c new file mode 100644 index 0000000000000..c6d90d6f7760e --- /dev/null +++ b/clang/test/Preprocessor/header-shadowing.c @@ -0,0 +1,57 @@ +// RUN: rm -rf %t +// RUN: split-file %s %t + +/// Check that: +/// - Quoted includes ("...") trigger the diagnostic. +/// - System headers are ignored. +/// - #include_next does not cause a duplicate warning. +// RUN: %clang_cc1 -Wshadow-header -Eonly %t/main.c -I %t/include1 -I %t/include2 \ +// RUN: -isystem %t/system1 -isystem %t/system2 2>&1 | FileCheck %s --check-prefix=SHADOWING + +// SHADOWING: {{.*}} warning: multiple candidates for header 'header.h' found; directory '{{.*}}include1' chosen, ignoring others including '{{.*}}include2' [-Wshadow-header] +// SHADOWING: warning: include1/header.h included! +// SHADOWING-NOT: {{.*}} warning: multiple candidates for header 'header.h' found; directory '{{.*}}include2' chosen, ignoring others including '{{.*}}include1' [-Wshadow-header] +// SHADOWING: warning: include2/header.h included! +// SHADOWING-NOT: {{.*}} warning: multiple candidates for header 'stdio.h' found; directory '{{.*}}system1' chosen, ignoring others including '{{.*}}system2' [-Wshadow-header] +// SHADOWING: warning: system1/stdio.h included! + +/// Check that the diagnostic is only performed once in MSVC compatibility mode. +// RUN: %clang_cc1 -fms-compatibility -Wshadow-header -Eonly %t/t.c 2>&1 | FileCheck %s --check-prefix=SHADOWING-MS + +// SHADOWING-MS: {{.*}} warning: multiple candidates for header 't3.h' found; directory '{{.*}}foo' chosen, ignoring others including '{{.*}}' [-Wshadow-header] +// SHADOWING-MS-NOT: {{.*}} warning: multiple candidates for header 't3.h' found; directory '{{.*}}' chosen, ignoring others including '{{.*}}foo' [-Wshadow-header] +// SHADOWING-MS: warning: Found foo/t3.h. + +//--- main.c +#include "header.h" +#include <stdio.h> + +//--- include1/header.h +#warning include1/header.h included! +#include_next "header.h" + +//--- include2/header.h +#warning include2/header.h included! + +//--- system1/stdio.h +#warning system1/stdio.h included! + +//--- system2/stdio.h +#warning system2/stdio.h included! + + +/// Used to test when running in MSVC compatibility +//--- t.c +#include "foo/t1.h" + +//--- foo/t1.h +#include "bar/t2.h" + +//--- foo/bar/t2.h +#include "t3.h" + +//--- foo/t3.h +#warning Found foo/t3.h. + +//--- t3.h +#warning Found t3.h. _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
