[PATCH] D65796: [clangd] Compute scopes eagerly in IncludeFixer

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

Computing lazily leads to crashes. In particular, computing scopes may
produce diagnostics (from inside template instantiations) and we
currently do it when processing another diagnostic, which leads to
crashes.

Moreover, we remember and access 'Scope*' when computing scopes. This
might lead to invalid memory access if the Scope is deleted by the time
we run the delayed computation. We did not actually construct an example
when this happens, though.

>From the VCS and review history, it seems the optimization was
introduced in the initial version without a mention of any performance
benchmarks justifying the performance gains. This led me to a
conclusion that the optimization was premature, so removing it to avoid
crashes seems like the right trade-off at that point.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65796

Files:
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/IncludeFixer.h
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D65796.213554.patch
Type: text/x-patch
Size: 6365 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190806/72668be5/attachment-0001.bin>