Page MenuHomePhabricator

[clangd] Serve comments for headers decls from dynamic index only
ClosedPublic

Authored by ilya-biryukov on May 23 2018, 11:47 AM.

Details

Summary

To fix a crash in code completion that occurrs when reading doc
comments from files that were updated after the preamble was
computed. In that case, the files on disk could've been changed and we
can't rely on finding the comment text with the same range anymore.

The current workaround is to not provide comments from the headers at
all and rely on the dynamic index instead.

A more principled solution would be to store contents of the files
read inside the preamble, but it is way harder to implement properly,
given that it would definitely increase the sizes of the preamble.

Together with D47272, this should all preamble-related crashes we're
aware of.

Diff Detail

Repository
rL LLVM

Event Timeline

ilya-biryukov created this revision.May 23 2018, 11:47 AM
sammccall accepted this revision.May 24 2018, 6:10 AM
sammccall added inline comments.
clangd/CodeCompletionStrings.h
46 ↗(On Diff #148266)

this invites double negation.
Also it doesn't seem like a great default, violates principle of least surprise.

prefer bool CommentsFromHeaders = true, or with no default.
(or even consider an enum)

This revision is now accepted and ready to land.May 24 2018, 6:10 AM
ilya-biryukov marked an inline comment as done.
  • Invert flag, remove the default.
clangd/CodeCompletionStrings.h
46 ↗(On Diff #148266)

Thanks for spotting this!
I opted for a positive bool flag with no defaults.

This revision was automatically updated to reflect the committed changes.