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

[Macros] Reproduce issue with peer+extension macro extension's methods not being checked as witnesses #71717

Merged
merged 2 commits into from Feb 20, 2024

Conversation

ktoso
Copy link
Member

@ktoso ktoso commented Feb 19, 2024

This narrows down and reproduces rdar://123012943

This is blocking the @_DistributedProtocol macro from working entirely since the methods introduced by the extension macro are not seen as witnesses.

It happens when a macro is both peer and extension;

  • the extension
    • the role's names must be names: arbitrary because we don't know statically what method names we'll introduce -- it depends on the protocol we're attached to
  • the peer role
    • would want to be names: prefixed('$') ... that seems to break things and the methods in the extension don't get "noticed" as witnesses
    • perhaps the system assumes they're going to be $ prefixed -- even though the extension roles' names are arbitrary?
    • we cannot make peer role's names "arbitrary" because 'peer' macros are not allowed to introduce arbitrary names at global scope

So this is somewhat blocked -- this looks like a bug in handling the names in the extension macro role, and not looking for the witnesses.

If a macro is made to be just extension the macro's introduced methods do properly witness the requirement.

@ktoso ktoso marked this pull request as draft February 19, 2024 06:21
ktoso and others added 2 commits February 19, 2024 13:19
…etect extension macros

Both `PotentialMacroExpansionsInContextRequest` and
`populateLookupTableEntryFromMacroExpansions` expected
extension macros only on extensions synthesized by macros.

That is not the same in practice and such attached macros
could appear on types as well.
@xedin xedin force-pushed the wip-reproduce-macros-witness-issue branch from 28136f6 to a6d53aa Compare February 19, 2024 23:04
@ktoso
Copy link
Member Author

ktoso commented Feb 19, 2024

@swift-ci please smoke test

nominal = container.getBaseNominal();

if (nominal)
forEachPotentialAttachedMacro(nominal, MacroRole::Extension, nameTracker);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Awesome, thanks Pavel :)

Copy link
Member Author

@ktoso ktoso left a comment

Choose a reason for hiding this comment

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

Awesome, thank you @xedin !

@ktoso ktoso marked this pull request as ready for review February 20, 2024 00:08
@ktoso
Copy link
Member Author

ktoso commented Feb 20, 2024

resolves rdar://123012943

@ktoso
Copy link
Member Author

ktoso commented Feb 20, 2024

@swift-ci please smoke test Linux

@ktoso ktoso added the distributed Feature → concurrency: distributed actor label Feb 20, 2024
@ktoso ktoso changed the title Reproduce issue with peer+extension macro extension's methods not being checked as witnesses [Macros] Reproduce issue with peer+extension macro extension's methods not being checked as witnesses Feb 20, 2024
@ktoso ktoso added the swift macro Feature → declarations: Swift `macro` declarations label Feb 20, 2024
@ktoso
Copy link
Member Author

ktoso commented Feb 20, 2024

@swift-ci please smoke test Linux

@ktoso ktoso enabled auto-merge (squash) February 20, 2024 06:48
@ktoso ktoso merged commit f03ddf7 into apple:main Feb 20, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
distributed Feature → concurrency: distributed actor swift macro Feature → declarations: Swift `macro` declarations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants