Skip to content

Commit 0996342

Browse files
committed
[CodeCompletion] Fix an issue that causes an iterator to be invalidated while iterating
In `SourceLookupCache::lookupVisibleDecls`, copy the top level values before iterating them. If we have 'addinitstotoplevel' enabled, calling 'addConstructorCallsForType' can cause macros to get expanded, which can then cause new members ot get added to 'TopLevelValues', invalidating the current iterator. I have not been able to reduce this to a test case that doesn’t rely on the `Observation` module in the SDK but here is the test case with which I was able to reproduce the issue very reliably. ```swift import Foundation import Observation @observable class MyObject {} extension MyObject {} // RUN: ~/sbin/sourcekitd-test \ // RUN: -req=complete.open \ // RUN: -req-opts=addinitstotoplevel=1 \ // RUN: -pos=8:1 \ // RUN: %s \ // RUN: -- \ // RUN: %s \ // RUN: -Xfrontend \ // RUN: -load-plugin-library \ // RUN: -Xfrontend \ // RUN: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/swift/host/plugins/libObservationMacros.dylib \ // RUN: -sdk \ // RUN: /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.0.sdk ``` rdar://109202157
1 parent 4bacb7f commit 0996342

File tree

1 file changed

+48
-1
lines changed

1 file changed

+48
-1
lines changed

lib/IDE/CompletionLookup.cpp

+48-1
Original file line numberDiff line numberDiff line change
@@ -3214,16 +3214,61 @@ void CompletionLookup::getTypeCompletionsInDeclContext(SourceLoc Loc,
32143214
RequestedCachedResults.insert(RequestedResultsTy::topLevelResults(filter));
32153215
}
32163216

3217+
namespace {
3218+
3219+
/// A \c VisibleDeclConsumer that stores all decls that are found and is able
3220+
/// to forward the to another \c VisibleDeclConsumer later.
3221+
class StoringDeclConsumer : public VisibleDeclConsumer {
3222+
struct FoundDecl {
3223+
ValueDecl *VD;
3224+
DeclVisibilityKind Reason;
3225+
DynamicLookupInfo DynamicLookupInfo;
3226+
};
3227+
3228+
std::vector<FoundDecl> FoundDecls;
3229+
3230+
void foundDecl(ValueDecl *VD, DeclVisibilityKind Reason,
3231+
DynamicLookupInfo DynamicLookupInfo = {}) override {
3232+
FoundDecls.push_back({VD, Reason, DynamicLookupInfo});
3233+
}
3234+
3235+
public:
3236+
/// Call \c foundDecl for every declaration that this consumer has found.
3237+
void forward(VisibleDeclConsumer &Consumer) {
3238+
for (auto &FoundDecl : FoundDecls) {
3239+
Consumer.foundDecl(FoundDecl.VD, FoundDecl.Reason,
3240+
FoundDecl.DynamicLookupInfo);
3241+
}
3242+
}
3243+
};
3244+
3245+
} // namespace
3246+
32173247
void CompletionLookup::getToplevelCompletions(CodeCompletionFilter Filter) {
32183248
Kind = (Filter - CodeCompletionFilterFlag::Module)
32193249
.containsOnly(CodeCompletionFilterFlag::Type)
32203250
? LookupKind::TypeInDeclContext
32213251
: LookupKind::ValueInDeclContext;
32223252
NeedLeadingDot = false;
32233253

3254+
// If we have 'addinitstotoplevel' enabled, calling `foundDecl` on `this`
3255+
// can cause macros to get expanded, which can then cause new members ot get
3256+
// added to 'TopLevelValues', invalidating iterator over `TopLevelDecls` in
3257+
// `SourceLookupCache::lookupVisibleDecls`.
3258+
//
3259+
// Technically `foundDecl` should not expand macros or discover new top level
3260+
// members in any way because those newly discovered decls will not be added
3261+
// to the code completion results. However, it's preferrable to miss results
3262+
// than to silently invalidate a collection, resulting in hard-to-diagnose
3263+
// crashes.
3264+
// Thus, store all the decls found by `CurrModule->lookupVisibleDecls` in a
3265+
// vector first and only call `this->foundDecl` once we have left the
3266+
// iteration loop over `TopLevelDecls`.
3267+
StoringDeclConsumer StoringConsumer;
3268+
32243269
UsableFilteringDeclConsumer UsableFilteringConsumer(
32253270
Ctx.SourceMgr, CurrDeclContext, Ctx.SourceMgr.getIDEInspectionTargetLoc(),
3226-
*this);
3271+
StoringConsumer);
32273272
AccessFilteringDeclConsumer AccessFilteringConsumer(CurrDeclContext,
32283273
UsableFilteringConsumer);
32293274

@@ -3242,6 +3287,8 @@ void CompletionLookup::getToplevelCompletions(CodeCompletionFilter Filter) {
32423287

32433288
CurrModule->lookupVisibleDecls({}, FilteringConsumer,
32443289
NLKind::UnqualifiedLookup);
3290+
3291+
StoringConsumer.forward(*this);
32453292
}
32463293

32473294
void CompletionLookup::lookupExternalModuleDecls(

0 commit comments

Comments
 (0)