This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Compute scopes eagerly in IncludeFixer
ClosedPublic

Authored by ilya-biryukov on Aug 6 2019, 2:48 AM.

Details

Summary

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.

Event Timeline

ilya-biryukov created this revision.Aug 6 2019, 2:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 6 2019, 2:48 AM
  • Make sure the test actually runs IncludeFixer.cpp
sammccall accepted this revision.Aug 6 2019, 3:31 AM
This revision is now accepted and ready to land.Aug 6 2019, 3:31 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptAug 6 2019, 4:37 AM