Proof of concept fix for #36#37
Conversation
e1dab33 to
c70b3c7
Compare
|
I would like to take a look myself how the Clang AST looks like before I can determine if this is a good solution. |
|
On related topic : is there any pretty printer of raw C AST representation in dstep sources? I didn't spot anything similar, could be useful for debugging. |
|
No, there isn't. |
|
But Clang itself has an AST dumper: |
|
Awesome, thanks, that will help a lot |
dstep/translator/Record.d
Outdated
There was a problem hiding this comment.
I think it would be better to handle all this in Record.translate. It's these lines that will write a struct declaration inline. When the definition is already know, don't execute those lines.
There was a problem hiding this comment.
I am not sure I understand. Lines you have linked get called from writeRecord and are responsible for writing the fields (CXCursor_FieldDecl). Struct declarations get written from the return value of writeRecord (return context.data).
There was a problem hiding this comment.
The lines I've linked above are the lines responsible for translating the actual type declaration. The lines below are for the field declaration.
This is the flow of for translating a struct:
- All top level nodes are iterated and translated, here
- When a struct is seen, it will start translating that, here
writeRecordis called which will translate the identifier of the struct, herewriteRecordthen calls the delegate which will start iterating all the children nodes of the struct, here- When a field is seen it will check if the type is not exposed and has a declaration, here
- If the type is not exposed and has a declaration it will translate the declaration of the type here
- If the declaration is a struct it will start from the beginning at step 2
This code, at step 6, is there to handle a struct and field in the same declaration, i.e.
struct Foo
{
struct Bar
{
int x;
} bar;
};There was a problem hiding this comment.
Ok, thanks, I have missed step 6 where it went in recursion and was confused by seemingly broken iteration over struct members.
|
Could you please add a testcase. |
c70b3c7 to
6fd7326
Compare
|
Updated. |
|
Hm, I have just spotted that it reorders last two struct declaration in generated binding but can't spot obvious reason to do so, any ideas? |
dstep/translator/Record.d
Outdated
There was a problem hiding this comment.
Updated
Fixes jacob-carlborg#36 Adds static set that keeps track of already known struct definitions and uses it to check if nested field struct is of already emitted type. Adds test cases that shows the original issue.
6fd7326 to
37957c7
Compare
|
Looking good. |
…truct-vars Fix #36: Don't output duplicate struct definition.
|
Thanks. |
Not checked with a test suite and results in extra empty
new lines placed instead but showcases core idea. @jacob-carlborg would you suggest any better place where to put it in dstep sources?