Skip to content

fix: Fix ENFORCE hit due to cache reuse across functions. #53

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
varungandhi-src merged 1 commit into from
Aug 5, 2022
Merged

fix: Fix ENFORCE hit due to cache reuse across functions. #53

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 21 additions & 12 deletions scip_indexer/SCIPIndexer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,10 +437,23 @@ enum class Emitted {
Earlier,
};

using OccurrenceCache = UnorderedMap<std::pair<core::LocOffsets, /*SymbolRole*/ int32_t>, uint32_t>;

class SCIPState {
string symbolScratchBuffer;
UnorderedMap<NamedSymbolRef, string> symbolStringCache;
UnorderedMap<uint32_t, core::TypePtr> localTypes;

// Cache of occurrences for locals that have been emitted in this function.
//
// Note that the SymbolRole is a part of the key too, because we can
// have a read-reference and write-reference at the same location
// (we don't merge those for now).
//
// The 'value' in the map is purely for sanity-checking. It's a bit
// cumbersome to conditionalize the type to be a set in non-debug and
// map in debug, so keeping it a map.
UnorderedMap<std::pair<core::LocOffsets, /*SymbolRole*/ int32_t>, uint32_t> localOccurrenceCache;

GemMetadata gemMetadata;

public:
Expand All @@ -454,16 +467,6 @@ class SCIPState {
UnorderedSet<std::pair<core::FileRef, std::string>> emittedSymbols;
UnorderedMap<core::FileRef, vector<scip::SymbolInformation>> symbolMap;

// Cache of occurrences for locals that have been emitted in this function.
//
// Note that the SymbolRole is a part of the key too, because we can
// have a read-reference and write-reference at the same location
// (we don't merge those for now).
//
// The 'value' in the map is purely for sanity-checking. It's a bit
// cumbersome to conditionalize the type to be a set in non-debug and
// map in debug, so keeping it a map.
UnorderedMap<std::pair<core::LocOffsets, /*SymbolRole*/ int32_t>, uint32_t> localOccurrenceCache;
vector<scip::Document> documents;
vector<scip::SymbolInformation> externalSymbols;

Expand All @@ -476,6 +479,10 @@ class SCIPState {
SCIPState(const SCIPState &) = delete;
SCIPState &operator=(const SCIPState &other) = delete;

void clearFunctionLocalCaches() {
this->localOccurrenceCache.clear();
}

// If the returned value is as success, the pointer is non-null.
//
// The argument symbol is used instead of recomputing from scratch if it is non-null.
Expand Down Expand Up @@ -1348,8 +1355,10 @@ class SCIPSemanticExtension : public SemanticExtension {
// specific to a range, so directly using that would lead to recomputing local variable
// information repeatedly for each occurrence.

sorbet::scip_indexer::CFGTraversal traversal(*scipState.get(), core::Context(gs, methodDef.symbol, file));
auto &scipStateRef = *scipState.get();
sorbet::scip_indexer::CFGTraversal traversal(scipStateRef, core::Context(gs, methodDef.symbol, file));
traversal.traverse(cfg);
scipStateRef.clearFunctionLocalCaches();
}

virtual unique_ptr<SemanticExtension> deepCopy(const core::GlobalState &from, core::GlobalState &to) override {
Expand Down