Skip to content

Conversation

@grendello
Copy link
Contributor

Context: fb94d59

fb94d59 added code to collect all overriden method descriptors in
a list, so that the future marshal methods code generator can generate
correct code for them. However, not all gathered descriptors contained
name of the managed type which declares the overridden methods. This
commit fixes the problem.

Context: fb94d59

fb94d59 added code to collect all overriden method descriptors in
a list, so that the future marshal methods code generator can generate
correct code for them.  However, not all gathered descriptors contained
name of the managed type which declares the overridden methods.  This
commit fixes the problem.
Connector = parts[2];
if (parts.Length > 3) {
ManagedTypeName = parts[3];
ManagedTypeName = parts[3].Replace ('/', '+');
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that the reason for this isn't particularly clear, I think it would be helpful if we had a "nicely named method" "somewhere".

Question is what the method should be named, and where it should go.

Spending 2 seconds on it, I'd suggest putting it with TypeDefinitionRocks:

partial class TypeDefinitionRocks {
    public static string CecilTypeNameToReflectionTypeName (string  typeName) => typeName.Replace ('/', '+');
}

Yes, it's longer, but at least it makes it clear why we're doing this.

Should probably update TypeDefinitionRocks' use of .Replace('/', '+') as well.

@jonpryor jonpryor merged commit 02aa54e into dotnet:main May 19, 2022
@grendello grendello deleted the marshal-methods-fix branch May 19, 2022 16:18
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants