Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show access specifiers as part of the Hover information #382

Closed
danielmartin opened this issue May 9, 2020 · 3 comments
Closed

Show access specifiers as part of the Hover information #382

danielmartin opened this issue May 9, 2020 · 3 comments
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@danielmartin
Copy link

One piece of information about a method definition that it's sometimes needed is if its member access is public, private, etc. This can be useful, for example, if you are inside a .cpp file and want to know quickly if a member is accessible outside of the class or not. Knowing if a field is protected can also be useful if we consider them a code smell and would like to refactor them.

I propose this can be shown as part of the hover Markdown contents. For example, instead of

instance-method HoverInfo::present

...
// In ...

Clangd would show

public instance-method HoverInfo::present

...
// In ...

Any thoughts? As far as I know, visibility, linkage, member access is not trivially computable in Clang, but maybe if you restrict it to a couple of easy and useful cases (like member method accessibility), the feature can be reliable and useful to programmers, without much impact on the information that is already available in the hover popup. Thanks.

@danielmartin danielmartin added the enhancement New feature or request label May 9, 2020
@hokein
Copy link
Contributor

hokein commented May 14, 2020

this sounds a useful feature to me.

And sometimes I'd like to know whether a member function is overriding a virtual member from a base class, would be nice to show it in hover as well.

@sam-mccall sam-mccall added good first issue Good for newcomers help wanted Extra attention is needed labels May 18, 2020
kadircet pushed a commit to llvm/llvm-project that referenced this issue May 27, 2020
Summary:
For clangd/clangd#382

This commit adds access specifier information to the hover
contents. For example, the hover information of a class field or
member function will now indicate if the field or member is private,
public, or protected. This can be particularly useful when a developer
is in the implementation file and wants to know if a particular member
definition is public or private.

Reviewers: kadircet

Reviewed By: kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D80472
@Trass3r
Copy link

Trass3r commented Jun 26, 2020

Looks like this is implemented now.

@danielmartin
Copy link
Author

Yes, I implemented this in llvm/llvm-project@6407aa9 I'll close the issue.

arichardson pushed a commit to arichardson/llvm-project that referenced this issue Jul 3, 2020
Summary:
For clangd/clangd#382

This commit adds access specifier information to the hover
contents. For example, the hover information of a class field or
member function will now indicate if the field or member is private,
public, or protected. This can be particularly useful when a developer
is in the implementation file and wants to know if a particular member
definition is public or private.

Reviewers: kadircet

Reviewed By: kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D80472
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants