inducer / pycparserext

Extensions for Eli Bendersky's pycparser
http://pypi.python.org/pypi/pycparserext
Other
83 stars 28 forks source link

Add support for attribute in between pointer and declaration name #59

Closed DCNick3 closed 11 months ago

DCNick3 commented 3 years ago
DCNick3 commented 3 years ago

While trying to come up with more test cases for the round-trip tests I found myself staring at what generator produced for this code:

int __attribute__((stdcall)) func1(void);
int func2(void) __attribute__((stdcall));

generated code:

 __attribute__((stdcall)) int func1(void);
int func2(void) __attribute__((stdcall));

I was puzzled why the attributes were located at different places in the generated code, while I thought they were put in the same place. It appears that they do not, with the former line putting the attributes into the AttributeSpecifier and it to the funcspec of the Decl and the latter - to the attributes of FuncDeclExt. Not clear to me if the distinction between those two cases should be made (pretty sure gcc does not care when compiling).

Will probably brush up this PR and dive a little bit deeper into the topic to better understand what is happening and how it should happen, probably coming with more PRs.

DCNick3 commented 3 years ago

Should be ready to merge. I was not able to make the _round_trip_matches work (it tried to compare two AttributeSpecifier's and failed there. Replaced with a check against a hard-coded code string.

inducer commented 3 years ago

You could make an "abbreviated" _round_trip_matches (_round_trip_match_source_only?) that asserts that the parsed source gets regenerated.

Also, please fix the linter failures.

DCNick3 commented 3 years ago

the parsed source gets regenerated

Do you intend to compare the original source code to the generated? Or do you mean checking that after regenerating the code two times (src -> AST1 -> regen1 -> AST2 -> regen2) regen1 == regen2?

The former will fail because the AST does not store, for example, the exact attribute position and it will get moved.

The latter will work, but it would not catch all the cases that the current comparison does. For example, it might just discard the attribute information and it will pass the test, even though it is horribly wrong.

inducer commented 3 years ago

The former will fail because the AST does not store, for example, the exact attribute position and it will get moved.

But there might be a source representation that's exactly reproduced. We could parse and regenerate that. Then, additional strings could be provided that are supposed to result in equivalent ASTs.

DCNick3 commented 3 years ago

Yeah, it is possible, but I want to be able to test that attributes are parsed correctly in all possible places, not only in their "canonical" form. That's why it's problematic..

inducer commented 3 years ago

I want to be able to test that attributes are parsed correctly in all possible places, not only in their "canonical" form

I agree with that goal. That's where I was going with the proposal of additional strings that parse/regenerate to the same canonical AST.

DCNick3 commented 3 years ago

So, you want them in addition to what I already have? Probably can do that

inducer commented 3 years ago

So, you want them in addition to what I already have? Probably can do that

Yeah, if the "reproduces canonical form" thing is easy to do, I'd appreciate having it. If it's not easy, don't bother though.

DCNick3 commented 11 months ago

I won't be pursuing this in any near time, sorry