Skip to content

Commit 7c09198

Browse files
benlangmuirbenlangmuir
authored
Merge pull request #24542 from benlangmuir/cc-assert-51
[5.1] [code-completion] Avoid invalid member substitution for keypath dynamic lookup
2 parents 3dd0e4c + 15f2e38 commit 7c09198

16 files changed

+501
-214
lines changed

‎include/swift/AST/NameLookup.h

Lines changed: 54 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,51 @@ enum class DeclVisibilityKind {
205205
DynamicLookup,
206206
};
207207

208+
/// For Decls found with DeclVisibilityKind::DynamicLookup, contains details of
209+
/// how they were looked up. For example, the SubscriptDecl used to find a
210+
/// KeyPath dynamic member.
211+
class DynamicLookupInfo {
212+
public:
213+
enum Kind {
214+
None,
215+
AnyObject,
216+
KeyPathDynamicMember,
217+
};
218+
219+
struct KeyPathDynamicMemberInfo {
220+
/// The subscript(dynamicMember:) by which we found the declaration.
221+
SubscriptDecl *subscript = nullptr;
222+
223+
/// The type context of `subscript`, which may be different than the
224+
/// original base type of the lookup if this declaration was found by nested
225+
/// dynamic lookups.
226+
Type baseType = Type();
227+
228+
/// Visibility of the declaration itself without dynamic lookup.
229+
///
230+
/// For example, dynamic lookup for KeyPath<Derived, U>, might find
231+
/// Base::foo with originalVisibility == MemberOfSuper.
232+
DeclVisibilityKind originalVisibility = DeclVisibilityKind::DynamicLookup;
233+
};
234+
235+
Kind getKind() const { return kind; }
236+
237+
const KeyPathDynamicMemberInfo &getKeyPathDynamicMember() const;
238+
239+
DynamicLookupInfo() : kind(None) {}
240+
DynamicLookupInfo(Kind kind) : kind(kind) {
241+
assert(kind != KeyPathDynamicMember && "use KeyPathDynamicMemberInfo ctor");
242+
}
243+
244+
/// Construct for a KeyPath dynamic member lookup.
245+
DynamicLookupInfo(SubscriptDecl *subscript, Type baseType,
246+
DeclVisibilityKind originalVisibility);
247+
248+
private:
249+
Kind kind;
250+
KeyPathDynamicMemberInfo keypath = {};
251+
};
252+
208253
/// An abstract base class for a visitor that consumes declarations found within
209254
/// a given context.
210255
class VisibleDeclConsumer {
@@ -213,7 +258,8 @@ class VisibleDeclConsumer {
213258
virtual ~VisibleDeclConsumer() = default;
214259

215260
/// This method is called by findVisibleDecls() every time it finds a decl.
216-
virtual void foundDecl(ValueDecl *VD, DeclVisibilityKind Reason) = 0;
261+
virtual void foundDecl(ValueDecl *VD, DeclVisibilityKind Reason,
262+
DynamicLookupInfo dynamicLookupInfo = {}) = 0;
217263
};
218264

219265
/// An implementation of VisibleDeclConsumer that's built from a lambda.
@@ -223,7 +269,7 @@ class LambdaDeclConsumer : public VisibleDeclConsumer {
223269
public:
224270
LambdaDeclConsumer(Fn &&callback) : Callback(std::move(callback)) {}
225271

226-
void foundDecl(ValueDecl *VD, DeclVisibilityKind reason) {
272+
void foundDecl(ValueDecl *VD, DeclVisibilityKind reason, DynamicLookupInfo) {
227273
Callback(VD, reason);
228274
}
229275
};
@@ -240,7 +286,8 @@ class VectorDeclConsumer : public VisibleDeclConsumer {
240286
explicit VectorDeclConsumer(SmallVectorImpl<ValueDecl *> &decls)
241287
: results(decls) {}
242288

243-
virtual void foundDecl(ValueDecl *VD, DeclVisibilityKind Reason) override {
289+
virtual void foundDecl(ValueDecl *VD, DeclVisibilityKind Reason,
290+
DynamicLookupInfo) override {
244291
results.push_back(VD);
245292
}
246293
};
@@ -259,7 +306,8 @@ class NamedDeclConsumer : public VisibleDeclConsumer {
259306
bool isTypeLookup)
260307
: name(name), results(results), isTypeLookup(isTypeLookup) {}
261308

262-
virtual void foundDecl(ValueDecl *VD, DeclVisibilityKind Reason) override {
309+
virtual void foundDecl(ValueDecl *VD, DeclVisibilityKind Reason,
310+
DynamicLookupInfo dynamicLookupInfo = {}) override {
263311
// Give clients an opportunity to filter out non-type declarations early,
264312
// to avoid circular validation.
265313
if (isTypeLookup && !isa<TypeDecl>(VD))
@@ -280,7 +328,8 @@ class AccessFilteringDeclConsumer final : public VisibleDeclConsumer {
280328
VisibleDeclConsumer &consumer)
281329
: DC(DC), ChainedConsumer(consumer) {}
282330

283-
void foundDecl(ValueDecl *D, DeclVisibilityKind reason) override;
331+
void foundDecl(ValueDecl *D, DeclVisibilityKind reason,
332+
DynamicLookupInfo dynamicLookupInfo = {}) override;
284333
};
285334

286335
/// Remove any declarations in the given set that were overridden by

‎lib/AST/ASTDemangler.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1041,7 +1041,8 @@ ASTBuilder::findForeignTypeDecl(StringRef name,
10411041

10421042
explicit Consumer(Demangle::Node::Kind kind) : ExpectedKind(kind) {}
10431043

1044-
void foundDecl(ValueDecl *decl, DeclVisibilityKind reason) override {
1044+
void foundDecl(ValueDecl *decl, DeclVisibilityKind reason,
1045+
DynamicLookupInfo dynamicLookupInfo = {}) override {
10451046
if (HadError) return;
10461047
if (decl == Result) return;
10471048
if (!Result) {

‎lib/AST/ExperimentalDependenciesSourceFileDepGraphConstructor.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,8 @@ struct SourceFileDeclFinder {
278278
ConstPtrVec<ValueDecl> &classMembers;
279279
Collector(ConstPtrVec<ValueDecl> &classMembers)
280280
: classMembers(classMembers) {}
281-
void foundDecl(ValueDecl *VD, DeclVisibilityKind) override {
281+
void foundDecl(ValueDecl *VD, DeclVisibilityKind,
282+
DynamicLookupInfo) override {
282283
classMembers.push_back(VD);
283284
}
284285
} collector{classMembers};

‎lib/AST/Module.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,8 @@ void SourceLookupCache::lookupClassMembers(AccessPathTy accessPath,
298298
for (ValueDecl *vd : member.second) {
299299
auto *nominal = vd->getDeclContext()->getSelfNominalTypeDecl();
300300
if (nominal && nominal->getName() == accessPath.front().first)
301-
consumer.foundDecl(vd, DeclVisibilityKind::DynamicLookup);
301+
consumer.foundDecl(vd, DeclVisibilityKind::DynamicLookup,
302+
DynamicLookupInfo::AnyObject);
302303
}
303304
}
304305
return;
@@ -311,7 +312,8 @@ void SourceLookupCache::lookupClassMembers(AccessPathTy accessPath,
311312
continue;
312313

313314
for (ValueDecl *vd : member.second)
314-
consumer.foundDecl(vd, DeclVisibilityKind::DynamicLookup);
315+
consumer.foundDecl(vd, DeclVisibilityKind::DynamicLookup,
316+
DynamicLookupInfo::AnyObject);
315317
}
316318
}
317319

‎lib/AST/NameLookup.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,14 +63,15 @@ ValueDecl *LookupResultEntry::getBaseDecl() const {
6363

6464
void DebuggerClient::anchor() {}
6565

66-
void AccessFilteringDeclConsumer::foundDecl(ValueDecl *D,
67-
DeclVisibilityKind reason) {
66+
void AccessFilteringDeclConsumer::foundDecl(
67+
ValueDecl *D, DeclVisibilityKind reason,
68+
DynamicLookupInfo dynamicLookupInfo) {
6869
if (D->isInvalid())
6970
return;
7071
if (!D->isAccessibleFrom(DC))
7172
return;
7273

73-
ChainedConsumer.foundDecl(D, reason);
74+
ChainedConsumer.foundDecl(D, reason, dynamicLookupInfo);
7475
}
7576

7677

‎lib/ClangImporter/ClangImporter.cpp

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2226,9 +2226,10 @@ class FilteringVisibleDeclConsumer : public swift::VisibleDeclConsumer {
22262226
assert(CMU);
22272227
}
22282228

2229-
void foundDecl(ValueDecl *VD, DeclVisibilityKind Reason) override {
2229+
void foundDecl(ValueDecl *VD, DeclVisibilityKind Reason,
2230+
DynamicLookupInfo dynamicLookupInfo) override {
22302231
if (isVisibleFromModule(ModuleFilter, VD))
2231-
NextConsumer.foundDecl(VD, Reason);
2232+
NextConsumer.foundDecl(VD, Reason, dynamicLookupInfo);
22322233
}
22332234
};
22342235

@@ -2243,9 +2244,10 @@ class FilteringDeclaredDeclConsumer : public swift::VisibleDeclConsumer {
22432244
assert(CMU);
22442245
}
22452246

2246-
void foundDecl(ValueDecl *VD, DeclVisibilityKind Reason) override {
2247+
void foundDecl(ValueDecl *VD, DeclVisibilityKind Reason,
2248+
DynamicLookupInfo dynamicLookupInfo) override {
22472249
if (isDeclaredInModule(ModuleFilter, VD))
2248-
NextConsumer.foundDecl(VD, Reason);
2250+
NextConsumer.foundDecl(VD, Reason, dynamicLookupInfo);
22492251
}
22502252
};
22512253

@@ -2308,9 +2310,10 @@ class DarwinLegacyFilterDeclConsumer : public swift::VisibleDeclConsumer {
23082310
topLevelModule->Name == "CoreServices");
23092311
}
23102312

2311-
void foundDecl(ValueDecl *VD, DeclVisibilityKind Reason) override {
2313+
void foundDecl(ValueDecl *VD, DeclVisibilityKind Reason,
2314+
DynamicLookupInfo dynamicLookupInfo) override {
23122315
if (!shouldDiscard(VD))
2313-
NextConsumer.foundDecl(VD, Reason);
2316+
NextConsumer.foundDecl(VD, Reason, dynamicLookupInfo);
23142317
}
23152318
};
23162319

@@ -2552,7 +2555,8 @@ class VectorDeclPtrConsumer : public swift::VisibleDeclConsumer {
25522555
explicit VectorDeclPtrConsumer(SmallVectorImpl<Decl *> &Decls)
25532556
: Results(Decls) {}
25542557

2555-
void foundDecl(ValueDecl *VD, DeclVisibilityKind Reason) override {
2558+
void foundDecl(ValueDecl *VD, DeclVisibilityKind Reason,
2559+
DynamicLookupInfo) override {
25562560
Results.push_back(VD);
25572561
}
25582562
};
@@ -3527,14 +3531,16 @@ void ClangImporter::Implementation::lookupObjCMembers(
35273531
// FIXME: If we didn't need to check alternate decls here, we could avoid
35283532
// importing the member at all by checking importedName ahead of time.
35293533
if (decl->getFullName().matchesRef(name)) {
3530-
consumer.foundDecl(decl, DeclVisibilityKind::DynamicLookup);
3534+
consumer.foundDecl(decl, DeclVisibilityKind::DynamicLookup,
3535+
DynamicLookupInfo::AnyObject);
35313536
}
35323537

35333538
// Check for an alternate declaration; if its name matches,
35343539
// report it.
35353540
for (auto alternate : getAlternateDecls(decl)) {
35363541
if (alternate->getFullName().matchesRef(name)) {
3537-
consumer.foundDecl(alternate, DeclVisibilityKind::DynamicLookup);
3542+
consumer.foundDecl(alternate, DeclVisibilityKind::DynamicLookup,
3543+
DynamicLookupInfo::AnyObject);
35383544
}
35393545
}
35403546
return true;

‎lib/FrontendTool/ReferenceDependencies.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,8 @@ void ProvidesEmitter::emitDynamicLookupMembers() const {
437437
SmallVector<DeclBaseName, 16> names;
438438

439439
public:
440-
void foundDecl(ValueDecl *VD, DeclVisibilityKind Reason) override {
440+
void foundDecl(ValueDecl *VD, DeclVisibilityKind Reason,
441+
DynamicLookupInfo) override {
441442
names.push_back(VD->getBaseName());
442443
}
443444
ArrayRef<DeclBaseName> getNames() {

0 commit comments

Comments
 (0)