[clang] Support header shadowing diagnostics in Clang header search

clang/docs/ReleaseNotes.rst

@@ -404,6 +404,9 @@ 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
+ is found in multiple search directories (excluding system paths).
+
 Improvements to Clang's time-trace
 ----------------------------------
 

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,

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>;

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);

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;

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.