https://github.com/Jinjie-Huang updated https://github.com/llvm/llvm-project/pull/162491
>From 2928471bb58221e8eee08a1da92d8565fb0901ad Mon Sep 17 00:00:00 2001 From: huangjinjie <[email protected]> Date: Thu, 6 Nov 2025 19:38:53 +0800 Subject: [PATCH 1/4] header shadowing diagnostics --- clang/docs/ReleaseNotes.rst | 4 ++ clang/include/clang/Basic/DiagnosticGroups.td | 2 + .../include/clang/Basic/DiagnosticLexKinds.td | 4 ++ clang/include/clang/Lex/HeaderSearch.h | 6 ++ clang/lib/Lex/HeaderSearch.cpp | 61 +++++++++++++++++++ clang/test/Preprocessor/header-shadowing.c | 56 +++++++++++++++++ 6 files changed, 133 insertions(+) create mode 100644 clang/test/Preprocessor/header-shadowing.c diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index e8339fa13ffba..c4c88c3facdd3 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -404,6 +404,10 @@ Improvements to Clang's diagnostics - Clang now emits a diagnostic in case `vector_size` or `ext_vector_type` attributes are used with a negative size (#GH165463). +- A new warning, -Wshadow-header, has been added to detect when a header file + included via quotes (#include "...") is found in multiple distinct search + directories. + Improvements to Clang's time-trace ---------------------------------- diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 1e0321de3f4b6..279c45bb938c1 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..7be1d3b4f0660 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 '%2' and others">, + 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..a996366337c31 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..49d7d736c8ed3 100644 --- a/clang/lib/Lex/HeaderSearch.cpp +++ b/clang/lib/Lex/HeaderSearch.cpp @@ -881,6 +881,60 @@ 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) || isAngled || + DiagnosedShadowing) + 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) { + 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 +984,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 +1018,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 +1155,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..23211c07e1e2b --- /dev/null +++ b/clang/test/Preprocessor/header-shadowing.c @@ -0,0 +1,56 @@ +// RUN: rm -rf %t +// RUN: split-file %s %t + +/// Check that: +/// - Quoted includes ("...") trigger the diagnostic. +/// - Angled includes (<...>) 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 '{{.*}}include2' and others [-Wshadow-header] +// SHADOWING: warning: include1/header.h included! +// SHADOWING: warning: include2/header.h included! +// SHADOWING: warning: system1/stdio.h included! +// SHADOWING-NOT: {{.*}} warning: multiple candidates for header 'header.h' found; directory '{{.*}}include2' chosen, ignoring '{{.*}}include1' and others [-Wshadow-header] +// SHADOWING-NOT: {{.*}} warning: multiple candidates for header 'stdio.h' found; directory '{{.*}}system1' chosen, ignoring '{{.*}}system2' and others [-Wshadow-header] + +/// Check that the diagnostic is only performed once in MSVC compatibility mode. +// RUN: %clang_cc1 -fms-compatibility -Wshadow-header -Eonly %t/t.c -I %t -I %t/foo -I %t/foo/bar 2>&1 | FileCheck %s --check-prefix=SHADOWING-MS + +// SHADOWING-MS: {{.*}} warning: multiple candidates for header 't3.h' found; directory '{{.*}}foo' chosen, ignoring '{{.*}}' and others [-Wshadow-header] +// SHADOWING-MS-NOT: {{.*}} warning: multiple candidates for header 't3.h' found; directory '{{.*}}foo' chosen, ignoring '{{.*}}' and others [-Wshadow-header] + +//--- 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. >From b0d265d271829eb52ef681052855135fd09e44c0 Mon Sep 17 00:00:00 2001 From: huangjinjie <[email protected]> Date: Thu, 6 Nov 2025 20:02:24 +0800 Subject: [PATCH 2/4] update release note --- clang/docs/ReleaseNotes.rst | 5 ++--- clang/test/Preprocessor/header-shadowing.c | 1 + 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index c4c88c3facdd3..233c746b779f3 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -404,9 +404,8 @@ Improvements to Clang's diagnostics - Clang now emits a diagnostic in case `vector_size` or `ext_vector_type` attributes are used with a negative size (#GH165463). -- A new warning, -Wshadow-header, has been added to detect when a header file - included via quotes (#include "...") is found in multiple distinct search - directories. +- A new warning ``-Wshadow-header`` has been added to detect when a header file + included via quotes (#include "...") is found in multiple distinct search directories. Improvements to Clang's time-trace ---------------------------------- diff --git a/clang/test/Preprocessor/header-shadowing.c b/clang/test/Preprocessor/header-shadowing.c index 23211c07e1e2b..92fc0664adf0c 100644 --- a/clang/test/Preprocessor/header-shadowing.c +++ b/clang/test/Preprocessor/header-shadowing.c @@ -19,6 +19,7 @@ // RUN: %clang_cc1 -fms-compatibility -Wshadow-header -Eonly %t/t.c -I %t -I %t/foo -I %t/foo/bar 2>&1 | FileCheck %s --check-prefix=SHADOWING-MS // SHADOWING-MS: {{.*}} warning: multiple candidates for header 't3.h' found; directory '{{.*}}foo' chosen, ignoring '{{.*}}' and others [-Wshadow-header] +// SHADOWING-MS: warning: Found foo/t3.h. // SHADOWING-MS-NOT: {{.*}} warning: multiple candidates for header 't3.h' found; directory '{{.*}}foo' chosen, ignoring '{{.*}}' and others [-Wshadow-header] //--- main.c >From cde15fa643610803f30d016fbeb22826154c511b Mon Sep 17 00:00:00 2001 From: huangjinjie <[email protected]> Date: Fri, 7 Nov 2025 14:34:26 +0800 Subject: [PATCH 3/4] Suppress check for system headers --- clang/lib/Lex/HeaderSearch.cpp | 3 +++ clang/test/Preprocessor/header-shadowing.c | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp index 49d7d736c8ed3..1bd91a341ffdd 100644 --- a/clang/lib/Lex/HeaderSearch.cpp +++ b/clang/lib/Lex/HeaderSearch.cpp @@ -921,6 +921,9 @@ void HeaderSearch::DiagnoseHeaderShadowing( 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)) { diff --git a/clang/test/Preprocessor/header-shadowing.c b/clang/test/Preprocessor/header-shadowing.c index 92fc0664adf0c..cace321d5be13 100644 --- a/clang/test/Preprocessor/header-shadowing.c +++ b/clang/test/Preprocessor/header-shadowing.c @@ -10,10 +10,10 @@ // SHADOWING: {{.*}} warning: multiple candidates for header 'header.h' found; directory '{{.*}}include1' chosen, ignoring '{{.*}}include2' and others [-Wshadow-header] // SHADOWING: warning: include1/header.h included! -// SHADOWING: warning: include2/header.h included! -// SHADOWING: warning: system1/stdio.h included! // SHADOWING-NOT: {{.*}} warning: multiple candidates for header 'header.h' found; directory '{{.*}}include2' chosen, ignoring '{{.*}}include1' and others [-Wshadow-header] +// SHADOWING: warning: include2/header.h included! // SHADOWING-NOT: {{.*}} warning: multiple candidates for header 'stdio.h' found; directory '{{.*}}system1' chosen, ignoring '{{.*}}system2' and others [-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 -I %t -I %t/foo -I %t/foo/bar 2>&1 | FileCheck %s --check-prefix=SHADOWING-MS >From 375efdf0bd544ff932aac5e865ad21fc492534d0 Mon Sep 17 00:00:00 2001 From: huangjinjie <[email protected]> Date: Fri, 7 Nov 2025 19:41:11 +0800 Subject: [PATCH 4/4] Identify if a file is a system header by its path --- clang/lib/Lex/HeaderSearch.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp index 1bd91a341ffdd..ca850c6f58961 100644 --- a/clang/lib/Lex/HeaderSearch.cpp +++ b/clang/lib/Lex/HeaderSearch.cpp @@ -887,8 +887,8 @@ void HeaderSearch::DiagnoseHeaderShadowing( ArrayRef<std::pair<OptionalFileEntryRef, DirectoryEntryRef>> Includers, bool isAngled, int IncluderLoopIndex, ConstSearchDirIterator MainLoopIt) { - if (Diags.isIgnored(diag::warn_header_shadowing, IncludeLoc) || isAngled || - DiagnosedShadowing) + if (Diags.isIgnored(diag::warn_header_shadowing, IncludeLoc) || + DiagnosedShadowing || (getFileInfo(*FE).DirInfo != SrcMgr::C_User)) return; DiagnosedShadowing = true; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
