https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/216748
>From 776ab186091eefeb2f4394b23777dc6145df8b44 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Mon, 17 Aug 2026 16:25:12 +0200 Subject: [PATCH 1/2] [clang][lex] Deduplicate -Wheader-shadowing diags --- clang/include/clang/Lex/HeaderSearch.h | 11 +++++- clang/lib/Lex/HeaderSearch.cpp | 37 +++++++++++------- .../Preprocessor/header-shadowing-dedup.c | 39 +++++++++++++++++++ clang/test/Preprocessor/header-shadowing.c | 12 +++++- 4 files changed, 81 insertions(+), 18 deletions(-) create mode 100644 clang/test/Preprocessor/header-shadowing-dedup.c diff --git a/clang/include/clang/Lex/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h index 962787cc48a17..438540ffe3af2 100644 --- a/clang/include/clang/Lex/HeaderSearch.h +++ b/clang/include/clang/Lex/HeaderSearch.h @@ -21,6 +21,7 @@ #include "clang/Lex/ModuleMap.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/DenseSet.h" #include "llvm/ADT/MapVector.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringMap.h" @@ -319,6 +320,12 @@ class HeaderSearch { }; llvm::StringMap<LookupFileCacheInfo, llvm::BumpPtrAllocator> LookupFileCache; + /// The files that were already considered for the \c -Wshadow-header + /// diagnostic, keyed by the spelling of the include that resolved to them. + /// Since the set of shadowing candidates depends on the spelling, the same + /// file has to be considered once per spelling it was found under. + llvm::StringMap<llvm::DenseSet<const FileEntry *>> ShadowCheckedHeaders; + /// Collection mapping a framework or subframework /// name like "Carbon" to the Carbon.framework directory. llvm::StringMap<FrameworkCacheEntry, llvm::BumpPtrAllocator> FrameworkMap; @@ -515,8 +522,8 @@ class HeaderSearch { } void diagnoseHeaderShadowing( - StringRef Filename, OptionalFileEntryRef FE, bool &DiagnosedShadowing, - SourceLocation IncludeLoc, ConstSearchDirIterator FromDir, + StringRef Filename, FileEntryRef FE, SourceLocation IncludeLoc, + ConstSearchDirIterator FromDir, ArrayRef<std::pair<OptionalFileEntryRef, DirectoryEntryRef>> Includers, bool isAngled, int IncluderLoopIndex, ConstSearchDirIterator MainLoopIt); diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp index 911997a10eba5..824ed8936621a 100644 --- a/clang/lib/Lex/HeaderSearch.cpp +++ b/clang/lib/Lex/HeaderSearch.cpp @@ -887,19 +887,22 @@ diagnoseFrameworkInclude(DiagnosticsEngine &Diags, SourceLocation IncludeLoc, } void HeaderSearch::diagnoseHeaderShadowing( - StringRef Filename, OptionalFileEntryRef FE, bool &DiagnosedShadowing, - SourceLocation IncludeLoc, ConstSearchDirIterator FromDir, + StringRef Filename, FileEntryRef FE, 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) + if (Diags.isIgnored(diag::warn_header_shadowing, IncludeLoc)) return; // Ignore diagnostics from system headers. if (MainLoopIt && MainLoopIt->isSystemHeaderDirectory()) return; - DiagnosedShadowing = true; + // Only consider each file once per spelling it was found under. Note that + // this also suppresses the search below for files that turn out not to be + // shadowed at all. + if (!ShadowCheckedHeaders[Filename].insert(FE).second) + return; // Indicates that file is first found in the includer's directory if (!MainLoopIt) { @@ -908,10 +911,10 @@ void HeaderSearch::diagnoseHeaderShadowing( SmallString<1024> TmpDir = IncluderAndDir.second.getName(); llvm::sys::path::append(TmpDir, Filename); if (auto File = getFileMgr().getOptionalFileRef(TmpDir)) { - if (&File->getFileEntry() == *FE) + if (*File == FE) continue; Diags.Report(IncludeLoc, diag::warn_header_shadowing) - << Filename << (*FE).getDir().getName() + << Filename << FE.getDir().getName() << IncluderAndDir.second.getName(); return; } @@ -933,10 +936,10 @@ void HeaderSearch::diagnoseHeaderShadowing( SmallString<1024> TmpPath = It->getName(); llvm::sys::path::append(TmpPath, Filename); if (auto File = getFileMgr().getOptionalFileRef(TmpPath)) { - if (&File->getFileEntry() == *FE) + if (*File == FE) continue; Diags.Report(IncludeLoc, diag::warn_header_shadowing) - << Filename << (*FE).getDir().getName() << It->getName(); + << Filename << FE.getDir().getName() << It->getName(); return; } } @@ -991,7 +994,6 @@ 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 @@ -1025,9 +1027,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); + diagnoseHeaderShadowing(Filename, *FE, IncludeLoc, FromDir, Includers, + isAngled, &IncluderAndDir - Includers.begin(), + nullptr); if (!Includer) { assert(First && "only first includer can have no file"); return FE; @@ -1162,8 +1164,13 @@ OptionalFileEntryRef HeaderSearch::LookupFile( if (!File) continue; - diagnoseHeaderShadowing(Filename, File, DiagnosedShadowing, IncludeLoc, - FromDir, Includers, isAngled, -1, It); + // In MSVC compatibility mode we may have already found the file in one of + // the includers' directories. That file is the one that ends up being used + // (see checkMSVCHeaderSearch() below), so reporting this one as the chosen + // candidate would be wrong. + if (!MSFE) + diagnoseHeaderShadowing(Filename, *File, IncludeLoc, FromDir, Includers, + isAngled, -1, It); CurDir = It; diff --git a/clang/test/Preprocessor/header-shadowing-dedup.c b/clang/test/Preprocessor/header-shadowing-dedup.c new file mode 100644 index 0000000000000..4d28811adbdd1 --- /dev/null +++ b/clang/test/Preprocessor/header-shadowing-dedup.c @@ -0,0 +1,39 @@ +// This test checks that -Wshadow-header reports each shadowed header only +// once, no matter how many times it is looked up. + +// RUN: rm -rf %t +// RUN: split-file %s %t +// RUN: %clang_cc1 -Eonly %t/tu.c -I %t/include1 -I %t/include2 \ +// RUN: -Wshadow-header -verify + +//--- tu.c +// expected-warning-re@+1 {{multiple candidates for header 'shadowed1.h' found; directory '{{.*}}include1' chosen, ignoring others including '{{.*}}include2'}} +#include "shadowed1.h" +// Including the same header again must not report the shadowing again. +#include "shadowed1.h" +// Neither must looking it up from a different includer. +#include "sub/reinclude.h" +// A different shadowed header is still reported. +// expected-warning-re@+1 {{multiple candidates for header 'shadowed2.h' found; directory '{{.*}}include1' chosen, ignoring others including '{{.*}}include2'}} +#include "shadowed2.h" +// The candidates depend on the spelling of the include, so the same file is +// still reported once per spelling it is found under: this one is shadowed by +// 'include2/a/spelled.h', the one in 'include1/a/reinclude.h' by +// 'include2/spelled.h'. +// expected-warning-re@+1 {{multiple candidates for header 'a/spelled.h' found; directory '{{.*}}include1{{.*}}a' chosen, ignoring others including '{{.*}}include2'}} +#include "a/spelled.h" +#include "a/reinclude.h" + +//--- sub/reinclude.h +#include "shadowed1.h" + +//--- include1/shadowed1.h +//--- include2/shadowed1.h +//--- include1/shadowed2.h +//--- include2/shadowed2.h +//--- include1/a/spelled.h +//--- include2/a/spelled.h +//--- include2/spelled.h +//--- include1/a/reinclude.h +// expected-warning-re@+1 {{multiple candidates for header 'spelled.h' found; directory '{{.*}}include1{{.*}}a' chosen, ignoring others including '{{.*}}include2'}} +#include "spelled.h" diff --git a/clang/test/Preprocessor/header-shadowing.c b/clang/test/Preprocessor/header-shadowing.c index c6d90d6f7760e..d1f71572bda35 100644 --- a/clang/test/Preprocessor/header-shadowing.c +++ b/clang/test/Preprocessor/header-shadowing.c @@ -16,7 +16,11 @@ // 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 +/// The file found in the includer's directory is the one that ends up being +/// used, so the candidates found in the search paths must not be reported. +// RUN: %clang_cc1 -fms-compatibility -Wshadow-header -Eonly %t/t.c \ +// RUN: -I %t/ms_include1 -I %t/ms_include2 2>&1 | FileCheck %s \ +// RUN: --check-prefix=SHADOWING-MS --implicit-check-not="multiple candidates" // 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] @@ -55,3 +59,9 @@ //--- t3.h #warning Found t3.h. + +//--- ms_include1/t3.h +#warning Found ms_include1/t3.h. + +//--- ms_include2/t3.h +#warning Found ms_include2/t3.h. >From 95d3cc32a9ef78bcbac97a7718d27306645d8b71 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Tue, 18 Aug 2026 20:16:41 +0200 Subject: [PATCH 2/2] DenseSet -> SmallPtrSet --- clang/include/clang/Lex/HeaderSearch.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/include/clang/Lex/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h index 438540ffe3af2..adaa126024c3c 100644 --- a/clang/include/clang/Lex/HeaderSearch.h +++ b/clang/include/clang/Lex/HeaderSearch.h @@ -21,8 +21,8 @@ #include "clang/Lex/ModuleMap.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/DenseMap.h" -#include "llvm/ADT/DenseSet.h" #include "llvm/ADT/MapVector.h" +#include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" @@ -324,7 +324,7 @@ class HeaderSearch { /// diagnostic, keyed by the spelling of the include that resolved to them. /// Since the set of shadowing candidates depends on the spelling, the same /// file has to be considered once per spelling it was found under. - llvm::StringMap<llvm::DenseSet<const FileEntry *>> ShadowCheckedHeaders; + llvm::StringMap<llvm::SmallPtrSet<const FileEntry *, 1>> ShadowCheckedHeaders; /// Collection mapping a framework or subframework /// name like "Carbon" to the Carbon.framework directory. _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
