This is an archive of the discontinued LLVM Phabricator instance.

[clangd][c++20]Consider the constraint of a constrained auto in FindTarget.
Needs RevisionPublic

Authored by massberg on Jul 10 2023, 8:29 AM.

Details

Summary

This enables support of hover on concepts with auto types.

I'm not 100% sure if this is the correct fix, but it works for
the exmaples and in vscode.

Diff Detail

Event Timeline

massberg created this revision.Jul 10 2023, 8:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 10 2023, 8:29 AM
massberg requested review of this revision.Jul 10 2023, 8:29 AM
massberg updated this revision to Diff 538952.Jul 11 2023, 1:10 AM

clang-format code

Sorry about the delay here.

Making the whole AutoTypeLoc resolve to the concept doesn't seem right - the auto part does not refer to the concept, and in principle refers to another type entirely.

Really I think there should be a child node representing just the concept part, probably a ConceptReference.

Compared to the case we discussed where constrained auto is a template type param, this looks easier: all the information is there in AutoTypeLoc.
I think we need RecursiveASTVisitor to assemble it into a ConceptReference, and (to use it with SelectionTree) we need the ability to store it in a DynTypedNode.

clang-tools-extra/clangd/unittests/FindTargetTests.cpp
581

this is going to have the same behavior on the auto token, right?

This is my main practical concern, that go-to-definition, hover, find-refs, go-to-type etc on auto will now treat Fooable as their target.

(That said, I'm not sure exactly how common it is for auto to be constrained in a non-dependent context...)

nridge added a subscriber: nridge.Jul 16 2023, 12:54 PM
nridge added inline comments.Jul 16 2023, 1:04 PM
clang-tools-extra/clangd/unittests/FindTargetTests.cpp
581

That said, I'm not sure exactly how common it is for auto to be constrained in a non-dependent context...

I think it may be reasonably common. For example, in this hello world example for the C++23 asynchronous programming proposal we have things like scheduler auto and sender auto (where scheduler and sender are concepts).

So, do think we want auto linked to the concrete deduced type in situations like this, and only the concept name linked to the concept definition.

nridge added inline comments.Jul 16 2023, 1:08 PM
clang-tools-extra/clangd/unittests/FindTargetTests.cpp
581

in this hello world example for the C++23 asynchronous programming proposal

(Slight correction: this is a C++26 proposal. My point remains though.)

sammccall requested changes to this revision.Aug 16 2023, 6:50 AM

(This will look different/better after D155858, taking it off my radar until then)

This revision now requires changes to proceed.Aug 16 2023, 6:50 AM