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 StringMarshalling error message and actually copy attributes from methods to shadows #86731

Merged
merged 4 commits into from May 26, 2023

Conversation

jtschuster
Copy link
Member

Just a couple small changes to make the diagnostics represent the actual attribute in question, and fix a bug that attributes weren't actually being carried over to the shadowing methods.

Copy link
Member

@jkoritzinsky jkoritzinsky left a comment

Choose a reason for hiding this comment

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

One fix then LGTM

@ghost ghost assigned jtschuster May 24, 2023
@ghost
Copy link

ghost commented May 24, 2023

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

Issue Details

Just a couple small changes to make the diagnostics represent the actual attribute in question, and fix a bug that attributes weren't actually being carried over to the shadowing methods.

Author: jtschuster
Assignees: -
Labels:

area-System.Runtime.InteropServices

Milestone: -

…Generator/LibraryImportGeneratorHelpers.cs

Co-authored-by: Jeremy Koritzinsky <jkoritzinsky@gmail.com>
@jtschuster jtschuster merged commit 4177d2d into dotnet:main May 26, 2023
104 of 111 checks passed
jtschuster added a commit that referenced this pull request May 30, 2023
…r attributes

The issue dotnet/source-build#3483 looks similar to the issues I found before fixing in #86731. The relevant changes were in ComMethodContext.cs. If it's the same issue I was hitting, the attribute syntax was being copied over without adding 'using' statements or changing the attribute name to be fully qualified. I haven't validated yet, but this should fix it by just not copying the attributes for the parameters since they're not strictly necessary.
carlossanlop pushed a commit that referenced this pull request May 30, 2023
…and don't copy parameter attributes (#86899)

* Use fully qualified type names for parameters and don't copy parameter attributes

The issue dotnet/source-build#3483 looks similar to the issues I found before fixing in #86731. The relevant changes were in ComMethodContext.cs. If it's the same issue I was hitting, the attribute syntax was being copied over without adding 'using' statements or changing the attribute name to be fully qualified. I haven't validated yet, but this should fix it by just not copying the attributes for the parameters since they're not strictly necessary.

* Add test for change
@ghost ghost locked as resolved and limited conversation to collaborators Jun 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants