[clang] [clang][deps] Fix `__has_include` behavior with umbrella headers (PR #70144)
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 25 14:20:51 PDT 2023
https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/70144
>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 1/2] [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: }
>From 6203f653cc2b81bd8bdab494488f607b2a58f9a3 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Wed, 25 Oct 2023 14:20:19 -0700
Subject: [PATCH 2/2] Improve test
---
.../modules-has-include-umbrella-header.c | 63 ++++++-------------
1 file changed, 20 insertions(+), 43 deletions(-)
diff --git a/clang/test/ClangScanDeps/modules-has-include-umbrella-header.c b/clang/test/ClangScanDeps/modules-has-include-umbrella-header.c
index 45ff2bd3b9cd24e..e9363b2e14b07ab 100644
--- a/clang/test/ClangScanDeps/modules-has-include-umbrella-header.c
+++ b/clang/test/ClangScanDeps/modules-has-include-umbrella-header.c
@@ -19,58 +19,39 @@ framework module FW_Private {
//--- frameworks/FW.framework/PrivateHeaders/A.h
#include <FW/B.h>
//--- frameworks/FW.framework/PrivateHeaders/B.h
-struct B {};
+#include "dependency.h"
//--- modules/module.modulemap
-module Foo { header "foo.h" }
-//--- modules/foo.h
+module Poison { header "poison.h" }
+module Import { header "import.h" }
+module Dependency { header "dependency.h" }
+//--- modules/poison.h
#if __has_include(<FW/B.h>)
#define HAS_B 1
#else
#define HAS_B 0
#endif
+//--- modules/import.h
+#include <FW/B.h>
+//--- modules/dependency.h
//--- tu.c
-#include "foo.h"
+#include "poison.h"
+
+#if __has_include(<FW/B.h>)
+#endif
+
+#include "import.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
+// RUN: cat %t/deps.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t
+// Let's check that the TU actually depends on `FW_Private` (and does not treat FW/B.h as textual).
// 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: "translation-units": [
// CHECK-NEXT: {
// CHECK-NEXT: "commands": [
// CHECK-NEXT: {
@@ -79,12 +60,8 @@ module Foo { header "foo.h" }
// 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: ],
// CHECK-NEXT: "command-line": [
// CHECK: ],
// CHECK-NEXT: "executable": "clang",
@@ -92,7 +69,7 @@ module Foo { header "foo.h" }
// CHECK-NEXT: "[[PREFIX]]/tu.c"
// CHECK-NEXT: ],
// CHECK-NEXT: "input-file": "[[PREFIX]]/tu.c"
-// CHECK: }
+// CHECK-NEXT: }
// CHECK: ]
// CHECK: }
// CHECK: ]
More information about the cfe-commits
mailing list