This is an archive of the discontinued LLVM Phabricator instance.

Add support for querying the visibility of a cursor
ClosedPublic

Authored by michaelwu on Oct 2 2015, 9:10 AM.

Details

Summary

This patch implements clang_getCursorVisibility which provides access to NamedDecl::getVisibility. It's been very useful for me when generating bindings.

Diff Detail

Event Timeline

michaelwu updated this revision to Diff 36370.Oct 2 2015, 9:10 AM
michaelwu retitled this revision from to Add support for querying the visibility of a cursor.
michaelwu updated this object.
michaelwu added a subscriber: cfe-commits.
klimek added a subscriber: klimek.

This LG, looping in Milian for a second opinion / to find more reviewers :)

milianw edited edge metadata.Oct 13 2015, 4:16 AM

Yep, looks good to me as well - thanks!

skalinichev edited edge metadata.Oct 17 2015, 1:45 AM

Isn't r246931 what you're looking for?

Index: expose visibility attribute

Expose the previously unexposed visibility attribute via the python and C
bindings.

Can't you use/improve that API instead?

Visibility can be set via command line args without any attributes in the AST, so I don't think that's sufficient.

Sergey, I think you raised concerns whether this is necessary. What are your thoughts?

Well, I think it's ok then. Still there is no test for visibility set from command line case.
Also adding comment to clang_getCursorVisibility explaining that it also works with visibility set from command line would be very useful.

test/Index/symbol-visibility.c
8

Are you sure that this is correct?
Looking at r246931, it seems like the protected visibility is not supported on all platforms.

michaelwu added inline comments.Nov 4 2015, 8:51 PM
test/Index/symbol-visibility.c
8

Not sure about this. For most purposes, default and hidden is all you need so I don't think there's much lost by cutting this part of this test. I was on the fence about whether to test for protected.

michaelwu updated this revision to Diff 40100.Nov 12 2015, 5:14 PM
michaelwu edited edge metadata.

This removes the test for protected visibility and improves documentation for clang_getCursorVisibility.

milianw accepted this revision.Nov 14 2015, 9:05 AM
milianw edited edge metadata.

From my POV, this looks good.

This revision is now accepted and ready to land.Nov 14 2015, 9:05 AM

Anyone mind landing this for me? I don't have commit access.

ehsan accepted this revision.Nov 23 2015, 11:59 AM
ehsan added a reviewer: ehsan.

Landed in r253909.

milianw closed this revision.Feb 20 2016, 5:29 PM

closing then, since this has been landed