krasimir added inline comments.
================
Comment at: clangd/DocumentStore.h:42
/// Delete a document from the store.
- void removeDocument(StringRef Uri) { Docs.erase(Uri); }
+ void removeDocument(StringRef Uri) { Docs.erase(Uri);
+ for (const auto &Listener : Listeners)
----------------
cierpuchaw wrote:
> cierpuchaw wrote:
> > Shouldn't this be called under a lock_guard?
> I should've reworded my comment before submitting it. By 'this' I'm referring
> to the 'Docs.erase()' part, not the whole function.
>
Even under a guard, it may potentially still be unsafe since workers may
operate on erased/modified StringRef-s.
================
Comment at: clangd/DocumentStore.h:51
auto I = Docs.find(Uri);
return I == Docs.end() ? StringRef("") : StringRef(I->second);
}
----------------
StringRef doesn't own the underlying data, right? What if I erase it in the
meantime?
================
Comment at: clangd/DocumentStore.h:63
+ AllDocs.emplace_back(P.first(), P.second);
+ return AllDocs;
+ }
----------------
Same thing, shouldn't these be strings?
https://reviews.llvm.org/D29886
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
