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

fix Issue 21937 - importC: Support parsing __attribute__ specifiers #12569

Merged
merged 1 commit into from
May 27, 2021

Conversation

ibuclaw
Copy link
Member

@ibuclaw ibuclaw commented May 23, 2021

Only parses __attribute__, ignored its contents for now. @WalterBright

@ibuclaw ibuclaw added Review:Walter Bright Feature:ImportC Pertaining to ImportC support labels May 23, 2021
@ibuclaw ibuclaw requested a review from WalterBright May 23, 2021 22:55
@dlang-bot
Copy link
Contributor

Thanks for your pull request, @ibuclaw!

Bugzilla references

Auto-close Bugzilla Severity Description
21937 major importC: Support parsing __attribute specifiers

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#12569"

@ibuclaw ibuclaw force-pushed the issue21937 branch 9 times, most recently from d24b1a7 to 8265212 Compare May 26, 2021 09:15
@ibuclaw
Copy link
Member Author

ibuclaw commented May 26, 2021

Alright, I think I've covered all obvious bases. Mind reviewing @thewilsonator / @RazvanN7? Doesn't need to be merged immediately, to allow @WalterBright at least 72 hours to check it first.

Copy link
Contributor

@thewilsonator thewilsonator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC we currently ignore the meaning of attributes that have direct d equivalents?

@thewilsonator thewilsonator added the Merge:72h no objection -> merge The PR will be merged if there are no objections raised. label May 26, 2021
@ibuclaw
Copy link
Member Author

ibuclaw commented May 26, 2021

IIUC we currently ignore the meaning of attributes that have direct d equivalents?

Yes, that is for issue 21938 to solve.

@@ -2644,6 +2681,15 @@ final class CParser(AST) : Parser!AST
const loc = token.loc;
nextToken();

/* GNU Extensions
* enum-specifier:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link to (the closest thing to a) specification from the horse's (gnu's?) mouth?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function cparseGnuAttributes has the link, I don't think it is worth repasting it everywhere.

For your review this is it.

https://gcc.gnu.org/onlinedocs/gcc/Attribute-Syntax.html

As you will see, it uses hand wavy words rather than clear, concise grammar to describe how it is parsed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ibuclaw Must've missed that my bad, thanks for the good work

@WalterBright WalterBright added Merge:auto-merge and removed Merge:72h no objection -> merge The PR will be merged if there are no objections raised. labels May 27, 2021
@dlang-bot dlang-bot merged commit 69eced3 into dlang:master May 27, 2021
@ibuclaw ibuclaw deleted the issue21937 branch May 27, 2021 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants