[clang][modules] Avoid modules diagnostics for __has_include()

[jansvoboda11]

After #70144 Clang started resolving module maps even for __has_include() expressions. This had the unintended consequence of emitting diagnostics around header misuse. These don't make sense if you actually don't bring contents of the header into the importer, so should be skipped for __has_include(). This patch moves emission of these diagnostics out of Preprocessor::LookupFile() up into Preprocessor::LookupHeaderIncludeOrImport().

[llvmbot]

@llvm/pr-subscribers-clang-modules

@llvm/pr-subscribers-clang

Author: Jan Svoboda (jansvoboda11)

Changes

After #70144 Clang started resolving module maps even for __has_include() expressions. The unintended consequence of this is that diagnostics related to module mis-use started trigerring. These diagnostics are clearly intended to catch actual usage of a header, so should not apply for __has_include() expressions that don't bring the contents of the header into the TU.

---

Full diff: https://github.com/llvm/llvm-project/pull/71450.diff

2 Files Affected:

• (modified) clang/lib/Lex/PPDirectives.cpp (+20-16)
• (added) clang/test/Modules/has_include_non_modular.c (+14)

diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index d97a103833c2fa6..956e2276f25b710 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -960,7 +960,6 @@ OptionalFileEntryRef Preprocessor::LookupFile(
 
   Module *RequestingModule = getModuleForLocation(
       FilenameLoc, LangOpts.ModulesValidateTextualHeaderIncludes);
-  bool RequestingModuleIsModuleInterface = !SourceMgr.isInMainFile(FilenameLoc);
 
   // If the header lookup mechanism may be relative to the current inclusion
   // stack, record the parent #includes.
@@ -1041,13 +1040,8 @@ OptionalFileEntryRef Preprocessor::LookupFile(
       Filename, FilenameLoc, isAngled, FromDir, &CurDir, Includers, SearchPath,
       RelativePath, RequestingModule, SuggestedModule, IsMapped,
       IsFrameworkFound, SkipCache, BuildSystemModule, OpenFile, CacheFailures);
-  if (FE) {
-    if (SuggestedModule && !LangOpts.AsmPreprocessor)
-      HeaderInfo.getModuleMap().diagnoseHeaderInclusion(
-          RequestingModule, RequestingModuleIsModuleInterface, FilenameLoc,
-          Filename, *FE);
+  if (FE)
     return FE;
-  }
 
   OptionalFileEntryRef CurFileEnt;
   // Otherwise, see if this is a subframework header.  If so, this is relative
@@ -1058,10 +1052,6 @@ OptionalFileEntryRef Preprocessor::LookupFile(
       if (OptionalFileEntryRef FE = HeaderInfo.LookupSubframeworkHeader(
               Filename, *CurFileEnt, SearchPath, RelativePath, RequestingModule,
               SuggestedModule)) {
-        if (SuggestedModule && !LangOpts.AsmPreprocessor)
-          HeaderInfo.getModuleMap().diagnoseHeaderInclusion(
-              RequestingModule, RequestingModuleIsModuleInterface, FilenameLoc,
-              Filename, *FE);
         return FE;
       }
     }
@@ -1073,10 +1063,6 @@ OptionalFileEntryRef Preprocessor::LookupFile(
         if (OptionalFileEntryRef FE = HeaderInfo.LookupSubframeworkHeader(
                 Filename, *CurFileEnt, SearchPath, RelativePath,
                 RequestingModule, SuggestedModule)) {
-          if (SuggestedModule && !LangOpts.AsmPreprocessor)
-            HeaderInfo.getModuleMap().diagnoseHeaderInclusion(
-                RequestingModule, RequestingModuleIsModuleInterface,
-                FilenameLoc, Filename, *FE);
           return FE;
         }
       }
@@ -2027,12 +2013,28 @@ OptionalFileEntryRef Preprocessor::LookupHeaderIncludeOrImport(
     const FileEntry *LookupFromFile, StringRef &LookupFilename,
     SmallVectorImpl<char> &RelativePath, SmallVectorImpl<char> &SearchPath,
     ModuleMap::KnownHeader &SuggestedModule, bool isAngled) {
+  auto DiagnoseHeaderInclusion = [&](FileEntryRef FE) {
+    if (LangOpts.AsmPreprocessor)
+      return;
+
+    Module *RequestingModule = getModuleForLocation(
+        FilenameLoc, LangOpts.ModulesValidateTextualHeaderIncludes);
+    bool RequestingModuleIsModuleInterface =
+        !SourceMgr.isInMainFile(FilenameLoc);
+
+    HeaderInfo.getModuleMap().diagnoseHeaderInclusion(
+        RequestingModule, RequestingModuleIsModuleInterface, FilenameLoc,
+        Filename, FE);
+  };
+
   OptionalFileEntryRef File = LookupFile(
       FilenameLoc, LookupFilename, isAngled, LookupFrom, LookupFromFile, CurDir,
       Callbacks ? &SearchPath : nullptr, Callbacks ? &RelativePath : nullptr,
       &SuggestedModule, &IsMapped, &IsFrameworkFound);
-  if (File)
+  if (File) {
+    DiagnoseHeaderInclusion(*File);
     return File;
+  }
 
   // Give the clients a chance to silently skip this include.
   if (Callbacks && Callbacks->FileNotFound(Filename))
@@ -2051,6 +2053,7 @@ OptionalFileEntryRef Preprocessor::LookupHeaderIncludeOrImport(
         &SuggestedModule, &IsMapped,
         /*IsFrameworkFound=*/nullptr);
     if (File) {
+      DiagnoseHeaderInclusion(*File);
       Diag(FilenameTok, diag::err_pp_file_not_found_angled_include_not_fatal)
           << Filename << IsImportDecl
           << FixItHint::CreateReplacement(FilenameRange,
@@ -2081,6 +2084,7 @@ OptionalFileEntryRef Preprocessor::LookupHeaderIncludeOrImport(
         Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped,
         /*IsFrameworkFound=*/nullptr);
     if (File) {
+      DiagnoseHeaderInclusion(*File);
       auto Hint =
           isAngled ? FixItHint::CreateReplacement(
                          FilenameRange, "<" + TypoCorrectionName.str() + ">")
diff --git a/clang/test/Modules/has_include_non_modular.c b/clang/test/Modules/has_include_non_modular.c
new file mode 100644
index 000000000000000..2e1a06bdc6e4a0d
--- /dev/null
+++ b/clang/test/Modules/has_include_non_modular.c
@@ -0,0 +1,14 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+//--- module.modulemap
+module Mod { header "mod.h" }
+//--- mod.h
+#if __has_include("textual.h")
+#endif
+//--- textual.h
+
+//--- tu.c
+#include "mod.h"
+
+// RUN: %clang -fsyntax-only %t/tu.c -fmodules -fmodules-cache-path=%t/cache -Werror=non-modular-include-in-module