Skip to content

Commit 432fd89

Browse files
daniel-grumbergdaniel-grumberg
committed
[clang][ExtractAPI] Ensure LocationFileChecker doesn't try to traverse VFS when determining file path (llvm#74071)
As part of https://reviews.llvm.org/D154130 the logic of LocationFileChecker changed slightly to try and get the absolute external file path instead of the name as requested when the file was openened which would be before VFS mappings in our usage. Ensure that we only check against the name as requested instead of trying to generate the external canonical file path. rdar://115195433
1 parent 0f77a37 commit 432fd89

File tree

2 files changed

+224
-8
lines changed

2 files changed

+224
-8
lines changed

‎clang/lib/ExtractAPI/ExtractAPIConsumer.cpp

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include "clang/AST/ASTContext.h"
1818
#include "clang/AST/DeclObjC.h"
1919
#include "clang/Basic/DiagnosticFrontend.h"
20+
#include "clang/Basic/FileEntry.h"
2021
#include "clang/Basic/SourceLocation.h"
2122
#include "clang/Basic/SourceManager.h"
2223
#include "clang/Basic/TargetInfo.h"
@@ -164,6 +165,12 @@ Optional<std::string> getRelativeIncludeName(const CompilerInstance &CI,
164165
return None;
165166
}
166167

168+
Optional<std::string> getRelativeIncludeName(const CompilerInstance &CI,
169+
FileEntryRef FE,
170+
bool *IsQuoted = nullptr) {
171+
return getRelativeIncludeName(CI, FE.getNameAsRequested(), IsQuoted);
172+
}
173+
167174
struct LocationFileChecker {
168175
bool operator()(SourceLocation Loc) {
169176
// If the loc refers to a macro expansion we need to first get the file
@@ -174,33 +181,31 @@ struct LocationFileChecker {
174181
if (FID.isInvalid())
175182
return false;
176183

177-
const auto *File = SM.getFileEntryForID(FID);
184+
const auto File = SM.getFileEntryRefForID(FID);
178185
if (!File)
179186
return false;
180187

181-
if (KnownFileEntries.count(File))
188+
if (KnownFileEntries.count(*File))
182189
return true;
183190

184-
if (ExternalFileEntries.count(File))
191+
if (ExternalFileEntries.count(*File))
185192
return false;
186193

187-
StringRef FileName = SM.getFileManager().getCanonicalName(File);
188-
189194
// Try to reduce the include name the same way we tried to include it.
190195
bool IsQuoted = false;
191-
if (auto IncludeName = getRelativeIncludeName(CI, FileName, &IsQuoted))
196+
if (auto IncludeName = getRelativeIncludeName(CI, *File, &IsQuoted))
192197
if (llvm::any_of(KnownFiles,
193198
[&IsQuoted, &IncludeName](const auto &KnownFile) {
194199
return KnownFile.first.equals(*IncludeName) &&
195200
KnownFile.second == IsQuoted;
196201
})) {
197-
KnownFileEntries.insert(File);
202+
KnownFileEntries.insert(*File);
198203
return true;
199204
}
200205

201206
// Record that the file was not found to avoid future reverse lookup for
202207
// the same file.
203-
ExternalFileEntries.insert(File);
208+
ExternalFileEntries.insert(*File);
204209
return false;
205210
}
206211

‎clang/test/ExtractAPI/vfs_redirected_include.m

Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,211 @@
1+
// RUN: rm -rf %t
2+
// RUN: split-file %s %t
3+
4+
// Setup framework root
5+
// RUN: mkdir -p %t/Frameworks/MyFramework.framework/Headers
6+
// RUN: cp %t/MyFramework.h %t/Frameworks/MyFramework.framework/Headers/
7+
// RUN: cp %t/MyHeader.h %t/Frameworks/MyFramework.framework/Headers/
8+
9+
// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
10+
// RUN: %t/reference.output.json.in >> %t/reference.output.json
11+
12+
// Create VFS overlay from framework headers to SRCROOT
13+
// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" -e "s@DSTROOT@%{/t:regex_replacement}@g" \
14+
// RUN: %t/vfsoverlay.yaml.in >> %t/vfsoverlay.yaml
15+
16+
// Input headers use paths to the framework root/DSTROOT
17+
// RUN: %clang_cc1 -extract-api -v --product-name=MyFramework \
18+
// RUN: -triple arm64-apple-macosx \
19+
// RUN: -iquote%t -ivfsoverlay %t/vfsoverlay.yaml -F%t/Frameworks \
20+
// RUN: -x objective-c-header \
21+
// RUN: %t/Frameworks/MyFramework.framework/Headers/MyFramework.h \
22+
// RUN: %t/Frameworks/MyFramework.framework/Headers/MyHeader.h \
23+
// RUN: %t/QuotedHeader.h \
24+
// RUN: -o %t/output.json 2>&1 -verify | FileCheck -allow-empty %s
25+
26+
// Generator version is not consistent across test runs, normalize it.
27+
// RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \
28+
// RUN: %t/output.json >> %t/output-normalized.json
29+
// RUN: diff %t/reference.output.json %t/output-normalized.json
30+
31+
// CHECK: <extract-api-includes>:
32+
// CHECK-NEXT: #import <MyFramework/MyFramework.h>
33+
// CHECK-NEXT: #import <MyFramework/MyHeader.h>
34+
// CHECK-NEXT: #import "QuotedHeader.h"
35+
36+
//--- vfsoverlay.yaml.in
37+
{
38+
"version": 0,
39+
"case-sensitive": "false",
40+
"roots": [
41+
{
42+
"contents": [
43+
{
44+
"external-contents": "SRCROOT/MyHeader.h",
45+
"name": "MyHeader.h",
46+
"type": "file"
47+
}
48+
],
49+
"name": "DSTROOT/Frameworks/MyFramework.framework/Headers",
50+
"type": "directory"
51+
}
52+
],
53+
}
54+
55+
//--- MyFramework.h
56+
// Umbrella for MyFramework
57+
#import <MyFramework/MyHeader.h>
58+
// expected-no-diagnostics
59+
60+
//--- MyHeader.h
61+
#import <OtherFramework/OtherHeader.h>
62+
int MyInt;
63+
// expected-no-diagnostics
64+
65+
//--- QuotedHeader.h
66+
char MyChar;
67+
// expected-no-diagnostics
68+
69+
//--- Frameworks/OtherFramework.framework/Headers/OtherHeader.h
70+
int OtherInt;
71+
// expected-no-diagnostics
72+
73+
//--- reference.output.json.in
74+
{
75+
"metadata": {
76+
"formatVersion": {
77+
"major": 0,
78+
"minor": 5,
79+
"patch": 3
80+
},
81+
"generator": "?"
82+
},
83+
"module": {
84+
"name": "MyFramework",
85+
"platform": {
86+
"architecture": "arm64",
87+
"operatingSystem": {
88+
"minimumVersion": {
89+
"major": 11,
90+
"minor": 0,
91+
"patch": 0
92+
},
93+
"name": "macosx"
94+
},
95+
"vendor": "apple"
96+
}
97+
},
98+
"relationships": [],
99+
"symbols": [
100+
{
101+
"accessLevel": "public",
102+
"declarationFragments": [
103+
{
104+
"kind": "typeIdentifier",
105+
"preciseIdentifier": "c:I",
106+
"spelling": "int"
107+
},
108+
{
109+
"kind": "text",
110+
"spelling": " "
111+
},
112+
{
113+
"kind": "identifier",
114+
"spelling": "MyInt"
115+
},
116+
{
117+
"kind": "text",
118+
"spelling": ";"
119+
}
120+
],
121+
"identifier": {
122+
"interfaceLanguage": "objective-c",
123+
"precise": "c:@MyInt"
124+
},
125+
"kind": {
126+
"displayName": "Global Variable",
127+
"identifier": "objective-c.var"
128+
},
129+
"location": {
130+
"position": {
131+
"character": 4,
132+
"line": 1
133+
},
134+
"uri": "file://SRCROOT/MyHeader.h"
135+
},
136+
"names": {
137+
"navigator": [
138+
{
139+
"kind": "identifier",
140+
"spelling": "MyInt"
141+
}
142+
],
143+
"subHeading": [
144+
{
145+
"kind": "identifier",
146+
"spelling": "MyInt"
147+
}
148+
],
149+
"title": "MyInt"
150+
},
151+
"pathComponents": [
152+
"MyInt"
153+
]
154+
},
155+
{
156+
"accessLevel": "public",
157+
"declarationFragments": [
158+
{
159+
"kind": "typeIdentifier",
160+
"preciseIdentifier": "c:C",
161+
"spelling": "char"
162+
},
163+
{
164+
"kind": "text",
165+
"spelling": " "
166+
},
167+
{
168+
"kind": "identifier",
169+
"spelling": "MyChar"
170+
},
171+
{
172+
"kind": "text",
173+
"spelling": ";"
174+
}
175+
],
176+
"identifier": {
177+
"interfaceLanguage": "objective-c",
178+
"precise": "c:@MyChar"
179+
},
180+
"kind": {
181+
"displayName": "Global Variable",
182+
"identifier": "objective-c.var"
183+
},
184+
"location": {
185+
"position": {
186+
"character": 5,
187+
"line": 0
188+
},
189+
"uri": "file://SRCROOT/QuotedHeader.h"
190+
},
191+
"names": {
192+
"navigator": [
193+
{
194+
"kind": "identifier",
195+
"spelling": "MyChar"
196+
}
197+
],
198+
"subHeading": [
199+
{
200+
"kind": "identifier",
201+
"spelling": "MyChar"
202+
}
203+
],
204+
"title": "MyChar"
205+
},
206+
"pathComponents": [
207+
"MyChar"
208+
]
209+
}
210+
]
211+
}

0 commit comments

Comments
 (0)