Page MenuHomePhabricator

Moved QualTypeNames.h from Tooling to AST.
ClosedPublic

Authored by ilya-biryukov on Oct 24 2017, 2:04 AM.

Details

Summary

For code reuse in SemaCodeComplete.
Note that the tests for QualTypeNames are still in Tooling as they use
Tooling's common testing code.

Diff Detail

Repository
rL LLVM

Event Timeline

ilya-biryukov created this revision.Oct 24 2017, 2:04 AM

The actual usage of QualTypeNames is in D38538.

rnk edited edge metadata.Oct 24 2017, 10:39 AM

Can you remind me why NamedDecl::printQualifiedName is not appropriate for your needs?

In D39224#905431, @rnk wrote:

Can you remind me why NamedDecl::printQualifiedName is not appropriate for your needs?

The use-case in code completion (see D38538, the interesting bit is in SemaCodeComplete.cpp) creates a QualType that is being pretty-printed later on.
There does not seem to be a way to rewrite it to use printQualifiedNamefor that particular case without messing up the code. I was initially planning to simply create ElaborateType with proper NestedNameSpecifier, but that's essentially what the code in QualTypeNames does already (it has logic to create both the NestedNameSpecifier and ElaboratedType out of it), so we should definitely reuse it.

In general, helpers like this look somewhat useful when writing features that present code to the user and the usage in SemaCodeComplete feels natural, at least from my perspective.

rnk added a comment.Oct 25 2017, 10:08 AM
In D39224#905431, @rnk wrote:

Can you remind me why NamedDecl::printQualifiedName is not appropriate for your needs?

The use-case in code completion (see D38538, the interesting bit is in SemaCodeComplete.cpp) creates a QualType that is being pretty-printed later on.
There does not seem to be a way to rewrite it to use printQualifiedNamefor that particular case without messing up the code. I was initially planning to simply create ElaborateType with proper NestedNameSpecifier, but that's essentially what the code in QualTypeNames does already (it has logic to create both the NestedNameSpecifier and ElaboratedType out of it), so we should definitely reuse it.

Got it, thanks. If we're moving this into AST, I suspect ASTContext would be a better home for it. That's where the rest of our random type manipulation code lives, anyway. :)

rnk accepted this revision.Oct 25 2017, 10:09 AM

Looks good

This revision is now accepted and ready to land.Oct 25 2017, 10:09 AM
This revision was automatically updated to reflect the committed changes.