This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Collect and store expected types in the index
ClosedPublic

Authored by ilya-biryukov on Sep 19 2018, 12:14 PM.

Details

Summary

And add a hidden option to control whether the types are collected.
For experiments, will be removed when expected types implementation
is stabilized.

The index size is almost unchanged, e.g. the YAML index for all clangd
sources increased from 53MB to 54MB.

Event Timeline

ilya-biryukov created this revision.Sep 19 2018, 12:14 PM
ilya-biryukov planned changes to this revision.Sep 19 2018, 12:15 PM

Need to store the types in RIFF format too

  • Rebase and update after dependent changes.
sammccall added inline comments.Nov 20 2018, 7:23 AM
clangd/index/Index.cpp
118 ↗(On Diff #173340)

why these changes?

clangd/index/SymbolCollector.h
80 ↗(On Diff #173340)

Why make this an option? Is it expensive in time/size?

clangd/index/YAMLSerialization.cpp
202

also need to update the binary serialization?

ilya-biryukov marked an inline comment as done.
  • Remove accidental changes
  • Add types to binary serialization
  • Remove another accidental change
ilya-biryukov added inline comments.Nov 23 2018, 3:14 AM
clangd/index/Index.cpp
118 ↗(On Diff #173340)

Sorry, leftovers from the times we had arrays of sha hashes.

clangd/index/SymbolCollector.h
80 ↗(On Diff #173340)

Was there for experiments. They're not expensive, removed it.

sammccall accepted this revision.Nov 26 2018, 6:37 AM
sammccall added inline comments.
clangd/index/Index.h
286

either call this OpaqueType or point at in in the comment?

I'd put this below the ReturnType string too, it seems out of place here. But up to you.

clangd/index/SymbolCollector.cpp
591

nit: like this?

if (indexed for code completion)
 if (auto T = ...)
   S.Type = T->raw();
This revision is now accepted and ready to land.Nov 26 2018, 6:37 AM
ilya-biryukov marked 3 inline comments as done.Nov 26 2018, 6:55 AM
ilya-biryukov added inline comments.
clangd/index/Index.h
286
  • Added a comment. OpaqueType would clash with the type name, something I'd prefer to avoid.
  • Moved to live after ReturnType. I initially thought it'd be nice to keep them apart, since ReturnType is for showing it to the users and Type is for scoring. Now that I think about it does not look like a good idea, since keeping them apart means the user might read the comment for one of them, but not the other and end up inferring the other one on their own.
ilya-biryukov marked an inline comment as done.
  • Bump the RIFF version number
  • Place the Type field after the ReturnType
  • Address other comments
  • Remove unused #include
This revision was automatically updated to reflect the committed changes.