[clang] [clang][deps] Fix `__has_include` behavior with umbrella headers (PR #70144)

[Jan Svoboda via cfe-commits]

https://github.com/jansvoboda11 created https://github.com/llvm/llvm-project/pull/70144

Previously, Clang wouldn't try to resolve the module for the header being checked via `__has_include`. This meant that such header was considered textual (a.k.a. part of the module Clang is currently compiling).


>From 4199b80e5cb0a8873f63c356e4c4304833d6fffa Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Tue, 24 Oct 2023 16:30:22 -0700
Subject: [PATCH] [clang][deps] Fix `__has_include` behavior with umbrella
 headers

Previously, Clang wouldn't try to resolve the module for the header being checked via `__has_include`. This meant that such header was considered textual (a.k.a. part of the module Clang is currently compiling).
---
 clang/lib/Lex/PPMacroExpansion.cpp            |  7 +-
 .../modules-has-include-umbrella-header.c     | 99 +++++++++++++++++++
 2 files changed, 105 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/ClangScanDeps/modules-has-include-umbrella-header.c

diff --git a/clang/lib/Lex/PPMacroExpansion.cpp b/clang/lib/Lex/PPMacroExpansion.cpp
index b371f8cf7a9c072..30c4abdbad8aa44 100644
--- a/clang/lib/Lex/PPMacroExpansion.cpp
+++ b/clang/lib/Lex/PPMacroExpansion.cpp
@@ -1248,10 +1248,15 @@ static bool EvaluateHasIncludeCommon(Token &Tok, IdentifierInfo *II,
   if (Filename.empty())
     return false;
 
+  // Passing this to LookupFile forces header search to check whether the found
+  // file belongs to a module. Skipping that check could incorrectly mark
+  // modular header as textual, causing issues down the line.
+  ModuleMap::KnownHeader KH;
+
   // Search include directories.
   OptionalFileEntryRef File =
       PP.LookupFile(FilenameLoc, Filename, isAngled, LookupFrom, LookupFromFile,
-                    nullptr, nullptr, nullptr, nullptr, nullptr, nullptr);
+                    nullptr, nullptr, nullptr, &KH, nullptr, nullptr);
 
   if (PPCallbacks *Callbacks = PP.getPPCallbacks()) {
     SrcMgr::CharacteristicKind FileType = SrcMgr::C_User;
diff --git a/clang/test/ClangScanDeps/modules-has-include-umbrella-header.c b/clang/test/ClangScanDeps/modules-has-include-umbrella-header.c
new file mode 100644
index 000000000000000..45ff2bd3b9cd24e
--- /dev/null
+++ b/clang/test/ClangScanDeps/modules-has-include-umbrella-header.c
@@ -0,0 +1,99 @@
+// This test checks that __has_include(<FW/PrivateHeader.h>) in a module does
+// not clobber #include <FW/PrivateHeader.h> in importers of said module.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+//--- cdb.json.template
+[{
+  "file": "DIR/tu.c",
+  "directory": "DIR",
+  "command": "clang DIR/tu.c -fmodules -fmodules-cache-path=DIR/cache -I DIR/modules -F DIR/frameworks -o DIR/tu.o"
+}]
+
+//--- frameworks/FW.framework/Modules/module.private.modulemap
+framework module FW_Private {
+  umbrella header "A.h"
+  module * { export * }
+}
+//--- frameworks/FW.framework/PrivateHeaders/A.h
+#include <FW/B.h>
+//--- frameworks/FW.framework/PrivateHeaders/B.h
+struct B {};
+
+//--- modules/module.modulemap
+module Foo { header "foo.h" }
+//--- modules/foo.h
+#if __has_include(<FW/B.h>)
+#define HAS_B 1
+#else
+#define HAS_B 0
+#endif
+
+//--- tu.c
+#include "foo.h"
+#include <FW/B.h>
+
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full > %t/deps.json
+// RUN: cat %t/deps.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%t
+
+// CHECK:      {
+// CHECK-NEXT:   "modules": [
+// CHECK-NEXT:     {
+// CHECK-NEXT:       "clang-module-deps": [],
+// CHECK-NEXT:       "clang-modulemap-file": "[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap",
+// CHECK-NEXT:       "command-line": [
+// CHECK:            ],
+// CHECK-NEXT:       "context-hash": "{{.*}}",
+// CHECK-NEXT:       "file-deps": [
+// CHECK-NEXT:         "[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap",
+// CHECK-NEXT:         "[[PREFIX]]/frameworks/FW.framework/PrivateHeaders/A.h",
+// CHECK-NEXT:         "[[PREFIX]]/frameworks/FW.framework/PrivateHeaders/B.h"
+// CHECK-NEXT:       ],
+// CHECK-NEXT:       "name": "FW_Private"
+// CHECK-NEXT:     },
+// CHECK-NEXT:     {
+// CHECK-NEXT:       "clang-module-deps": [],
+// CHECK-NEXT:       "clang-modulemap-file": "[[PREFIX]]/modules/module.modulemap",
+// CHECK-NEXT:       "command-line": [
+// CHECK:              "-fmodule-map-file=[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap",
+// CHECK:            ],
+// CHECK-NEXT:       "context-hash": "{{.*}}",
+// CHECK-NEXT:       "file-deps": [
+// FIXME: The frameworks/FW.framework/PrivateHeaders/B.h header never makes it into SourceManager,
+//        so we don't track it as a file dependency (even though we should).
+// CHECK-NEXT:         "[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap",
+// CHECK-NEXT:         "[[PREFIX]]/modules/foo.h",
+// CHECK-NEXT:         "[[PREFIX]]/modules/module.modulemap"
+// CHECK-NEXT:       ],
+// CHECK-NEXT:       "name": "Foo"
+// CHECK-NEXT:     }
+// CHECK-NEXT:   ],
+// CHECK-NEXT:   "translation-units": [
+// CHECK-NEXT:     {
+// CHECK-NEXT:       "commands": [
+// CHECK-NEXT:         {
+// CHECK-NEXT:           "clang-context-hash": "{{.*}}",
+// CHECK-NEXT:           "clang-module-deps": [
+// CHECK-NEXT:             {
+// CHECK-NEXT:               "context-hash": "{{.*}}",
+// CHECK-NEXT:               "module-name": "FW_Private"
+// CHECK-NEXT:             },
+// CHECK-NEXT:             {
+// CHECK-NEXT:               "context-hash": "{{.*}}",
+// CHECK-NEXT:               "module-name": "Foo"
+// CHECK-NEXT:             }
+// CHECK-NEXT:           ],
+// CHECK-NEXT:           "command-line": [
+// CHECK:                ],
+// CHECK-NEXT:           "executable": "clang",
+// CHECK-NEXT:           "file-deps": [
+// CHECK-NEXT:             "[[PREFIX]]/tu.c"
+// CHECK-NEXT:           ],
+// CHECK-NEXT:           "input-file": "[[PREFIX]]/tu.c"
+// CHECK:              }
+// CHECK:            ]
+// CHECK:          }
+// CHECK:        ]
+// CHECK:      }