Skip to content

Conversation

@oldnewthing
Copy link
Member

@oldnewthing oldnewthing commented Apr 20, 2022

We forgot to apply the auto trick to auto_revoke methods to catch missing header files at compile time. As a result, people who used auto_revoke didn't learn about missing header files until link time.

Apply the same technique to auto-revoke event handler registration.

(Story time: A customer was running into linker errors due to a missing header file when they used auto_revoke. One thing they tried was switching to non-auto_revoke events, and then they got the compile-time error. But their conclusion wasn't "Oh, that's my problem" but rather "Oh no, that made it worse, go back!")

We forgot to apply the `auto` trick to catch missing header
files at compile time. As a result, people who used `auto_revoke`
didn't learn about missing header files until link time.

Apply the same technique to auto-revoke event handler registeration.

(Story time: A customer was running into linker errors due
to a missing header file when they used `auto_revoke`. One
thing they tried was switching to non-`auto_revoke` events,
and then they got the compile-time error. But their conclusion
wasn't "Oh, there's my problem" but rather "That made it worse,
go back!")
@kennykerr
Copy link
Collaborator

FYI: #1117

@oldnewthing
Copy link
Member Author

Thanks for the heads-up. Will rework and return.

@oldnewthing oldnewthing reopened this Apr 20, 2022
Copy link
Collaborator

@kennykerr kennykerr left a comment

Choose a reason for hiding this comment

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

Much simpler. 😄

@kennykerr kennykerr merged commit 959ae02 into microsoft:master Apr 21, 2022
@Scottj1s
Copy link
Member

Scottj1s commented Jun 8, 2022

Introduced a regression with static event subscriptions. Repro:
RuntimeComponentStaticEvent/SubscribeToStaticEvent.cpp at master · Scottj1s/RuntimeComponentStaticEvent (github.com)

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