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

GenAPI drops ref keyword when generating reference source for System.Memory/4.5.5 #38166

Closed
ViktorHofer opened this issue Jan 22, 2024 · 6 comments
Assignees

Comments

@ViktorHofer
Copy link
Member

image

Repro: `./generate.sh --package System.Memory,4.5.5
Observe changes in src/referencePackages/src/system.memory/4.5.5/lib/netstandard2.0/System.Memory.cs

GenAPI drops the ref keyword from structs like ReadOnlySpan<T>.

cc @ericstj

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Request triage from a team member label Jan 22, 2024
@ViktorHofer ViktorHofer added Bug and removed untriaged Request triage from a team member labels Jan 22, 2024
@ericstj
Copy link
Member

ericstj commented Jan 22, 2024

That's odd - it should treat it as ref - though in this case it looks like the assembly defined IsByRefLikeAttribute internally so that could be causing things to get tripped up.

@ericstj
Copy link
Member

ericstj commented Jan 22, 2024

Hmm, I see plenty of evidence that this never worked, but I'm at a loss for why not and why we didn't have an existing issue / discussion around it. I see the bug in roslyn that we need to fix to support it, so I'll give that a try and see if I run into any problems or rediscover past issues.

@ericstj
Copy link
Member

ericstj commented Jan 25, 2024

This was fixed in #38179. We'll be able to remove the workaround that added when we ingest the new roslyn with the fix.

@ericstj ericstj closed this as completed Jan 25, 2024
@ViktorHofer
Copy link
Member Author

Source-build might be building against the very latest roslyn. Do we need to adjust the code path for source-build only?

@ViktorHofer
Copy link
Member Author

Actually disregard. For GenAPI, source-build and non source-build both use the very latest roslyn so we shouldn't get into an inconsistent state between build variants.

@ericstj
Copy link
Member

ericstj commented Jan 25, 2024

I made this work so that it's OK if a newer roslyn meets up before we remove the assert, the assert should only pop during testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants