This is an archive of the discontinued LLVM Phabricator instance.

[clang][Index] Visit UsingDecls and generate USRs for them
ClosedPublic

Authored by kadircet on Feb 18 2019, 2:16 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

kadircet created this revision.Feb 18 2019, 2:16 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 18 2019, 2:16 AM
ilya-biryukov added inline comments.Feb 18 2019, 2:39 AM
lib/Index/IndexDecl.cpp
588 ↗(On Diff #187201)

Any reason to add this to the middle of the function?
Could we move it to the end or to the start of the function instead?

So that the function logically reads as

visitDecl();
traverseSubdecls();
processReferences();
lib/Index/USRGeneration.cpp
115 ↗(On Diff #187201)

We need to add a unique tag character here, e.g. namespace aliases are tagged as "@NA@", "@UD@" should be a good fit for using-decls.

118 ↗(On Diff #187201)

This comment does not make sense for the using-decl, ParmDecl is not a using-decl.
If there are other cases where it may fail, let's put them into the comment.
If EmitDeclName should not fail for UsingDecl, let's add an assertion.

kadircet updated this revision to Diff 187208.Feb 18 2019, 3:03 AM
kadircet marked 3 inline comments as done.
  • Address comments
ilya-biryukov added inline comments.Feb 18 2019, 5:55 AM
lib/Index/IndexDecl.cpp
586 ↗(On Diff #187208)

NIT: maybe put it at the very first line of the function, before the DC and Parent decls?
To keep the variable declarations closer to their usages.

lib/Index/USRGeneration.cpp
118 ↗(On Diff #187208)

add (void)EmittedDeclName to avoid unused warning with assertions disabled.

ilya-biryukov accepted this revision.Feb 18 2019, 5:56 AM

LGTM, but we need to make sure the variable is used in configs without assertions, otherwise we'll break buildbots.

This revision is now accepted and ready to land.Feb 18 2019, 5:56 AM
kadircet marked 2 inline comments as done.Feb 18 2019, 6:09 AM
kadircet updated this revision to Diff 187236.Feb 18 2019, 6:10 AM
  • Address comments
kadircet updated this revision to Diff 188171.Feb 25 2019, 7:26 AM
  • Add SymbolSubKind for UsingDeclarations
ilya-biryukov added inline comments.Feb 26 2019, 2:11 AM
include/clang/Index/IndexSymbol.h
75 ↗(On Diff #188171)

We don't seem to use the subkind anywhere. Let's remove it.

unittests/Index/IndexTests.cpp
253 ↗(On Diff #188171)

Could we also test the kind is correct here?

kadircet updated this revision to Diff 188361.Feb 26 2019, 6:20 AM
kadircet marked an inline comment as done.
  • Revert SymbolSubKind change
  • Add test to check for SymbolKind
kadircet marked an inline comment as done.Feb 26 2019, 6:21 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 26 2019, 6:23 AM