This is an archive of the discontinued LLVM Phabricator instance.

[ASTUnit] Fix a regression in cached completions
ClosedPublic

Authored by ilya-biryukov on Jul 18 2019, 6:32 AM.

Details

Summary

After r345152 cached completions started adding namespaces after
nested name specifiers, e.g. in some_name::^

The CCC_Symbol indicates the completed item cannot be a namespace (it is
described as being "a type, a function or a variable" in the comments).

Therefore, 'nested specifier' completions should only be added from cache
when the context is CCC_SymbolOrNewName (which roughly seems to indicate
that a nested name specifier is allowed).

Fixes https://bugs.llvm.org/show_bug.cgi?id=42646

Diff Detail

Repository
rL LLVM

Event Timeline

ilya-biryukov created this revision.Jul 18 2019, 6:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 18 2019, 6:32 AM
Herald added a subscriber: arphaman. · View Herald Transcript
sammccall accepted this revision.Jul 18 2019, 6:55 AM
sammccall added inline comments.
clang/test/Index/complete-qualified-with-preamble.cpp
7 ↗(On Diff #210548)

Nit: This (and the filename) seems like a nonstandard use of "preamble" - is there precedent for this in ASTUnit?

Else maybe these are just "cached global completions" or something

This revision is now accepted and ready to land.Jul 18 2019, 6:55 AM
kadircet accepted this revision.Jul 18 2019, 6:59 AM

LGTM from my side as well

ilya-biryukov marked an inline comment as done.
  • s/preamble/cached completions (in tests)
clang/test/Index/complete-qualified-with-preamble.cpp
7 ↗(On Diff #210548)

Good point, this is focused on cached completions, not preambles. Renamed the file and update the comments.

There seem to be other tests mentioning preamble in test/Index. Though I'm not sure myself CINDEXTEST_EDITING=1 affects only whether preambles are being used, my knowledge here is definitely not very deep.

sammccall added inline comments.Jul 18 2019, 8:13 AM
clang/test/Index/complete-qualified-with-preamble.cpp
7 ↗(On Diff #210548)

Thanks for the rename.

Looking at the implementation, CINDEXTEST_EDITING enables both preambles and completion caching, but they're separate features.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJul 18 2019, 8:24 AM