Skip to content

[EH] Replace event with tag #3937

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

Merged
merged 2 commits into from
Jun 18, 2021
Merged

[EH] Replace event with tag #3937

merged 2 commits into from
Jun 18, 2021

Conversation

aheejin
Copy link
Member

@aheejin aheejin commented Jun 17, 2021

We recently decided to change 'event' to 'tag', and 'event section' to
'tag section', out of the rationale that the section contains a
generalized tag that references a type, which may be used for something
other than exceptions, and the name 'event' can be confusing in the web
context.

See

We recently decided to change 'event' to 'tag', and to 'event section'
to 'tag section', out of the rationale that the section contains a
generalized tag that references a type, which may be used for something
other than exceptions, and the name 'event' can be confusing in the web
context.

See
- WebAssembly/exception-handling#159 (comment)
- WebAssembly/exception-handling#161
@aheejin aheejin requested review from kripken, dcodeIO and tlively June 17, 2021 01:07
@aheejin
Copy link
Member Author

aheejin commented Jun 17, 2021

Don't know what's going on with clang-tidy... Anyone has a clue? https://github.com/WebAssembly/binaryen/pull/3937/checks?check_run_id=2844692867

@kripken
Copy link
Member

kripken commented Jun 17, 2021

No idea... that started happening in every PR that modifies the delegations headers. It's a false positive, but I don't know how to ignore it...

@tlively
Copy link
Member

tlively commented Jun 17, 2021

We explicitly #error out in that file if any of the DELEGATE_FIELD_* macros are not defined. In normal usage, we expect those macros to be defined by the including source file. Clang-tidy looks at the header in isolation, though, so it never has a chance to define the macros and our own errors are being triggered.

I haven't seen any configuration options in clang-tidy that could fix this, but if we wanted to get clang-tidy working correctly we could downgrade these #errors to #warnings. We could also do something more complicated like error out if any macros are missing only if at least one is supplied.

@aheejin
Copy link
Member Author

aheejin commented Jun 18, 2021

Uploaded #3941 to fix the clang-tidy issue. If that lands, I can rebase this PR onto that.

@aheejin aheejin merged commit 28e88b9 into WebAssembly:main Jun 18, 2021
@aheejin aheejin deleted the tag_section branch June 18, 2021 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants