Page MenuHomePhabricator

[libclang] Restore old clang_Cursor_isAnonymous behaviour
ClosedPublic

Authored by JornVernee on Apr 27 2019, 12:06 PM.

Details

Summary

https://reviews.llvm.org/D54996 Changed the behaviour of clang_Cursor_isAnonymous, but there is no alternative available to get the old behaviour in some cases, which is essential for determining if a record is syntactically accessible, e.g.

struct {
  int x;
  int y;
} foo;

struct {
  struct {
    int x;
    int y;
  };
} bar;

void fun(struct { int x; int y; } *param);

The only 'anonymous' struct here is the one nested in bar, since there is no way to reference the struct itself, only the fields within. Though the anonymity applies to the instance itself, not the type.

To avoid confusion, I have added a new function called clang_Cursor_isAnonymousRecordDecl which has the old behaviour of clang_Cursor_isAnonymous (and updated the doc for the latter as well, which was seemingly forgotten).

Diff Detail

Repository
rL LLVM

Event Timeline

JornVernee created this revision.Apr 27 2019, 12:06 PM

Also, I don't have commit access, so I would need some help with that if this gets accepted :)

Now we know that somebody else also uses libclang :)
I the mentioned change I only wanted to follow the same logic as in TypePrinter::printTag to cover more anonymity cases.

Adding the old one as an extra function seems totally fine to me. Can you please add some tests for it to clarify the purpose for future users?

@yvvan Hows that as a test?

I'm pretty new at this, and it took a while to figure out how to run the tests. I used something like: build\Release\bin\c-index-test.exe -test-print-type .\test\Index\print-type.c | ..\llvm\build\Release\bin\FileCheck .\test\Index\print-type.c, but there's probably an easier way?

Could you please also upload diff with the full context? It's -U9999 for git and quite similar for svn.
Otherwise looks ok.

test/Index/print-type.c
70 ↗(On Diff #196988)

The path will be different depending on the machine and OS. You can {{.*}}print-type.cpp instead (like in https://reviews.llvm.org/D54996).

JornVernee marked an inline comment as done.
yvvan added a comment.Apr 28 2019, 1:35 PM

Thank you! Sorry that I forgot about the required minor version increment.
I will run tests tomorrow and then I can commit it for you.

include/clang-c/Index.h
35 ↗(On Diff #197019)

We also need to increment the minor version because of the new function added.

Incremented minor version

JornVernee marked an inline comment as done.Apr 28 2019, 2:02 PM

No worries! I'm already glad there's someone who picked up the review for this so quickly ;)

yvvan accepted this revision.Apr 29 2019, 5:25 AM
This revision is now accepted and ready to land.Apr 29 2019, 5:25 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 29 2019, 6:45 AM