Conversation
The open generic formatter is merely skipped at this point to avoid generating code with compilation errors. This will need to be revisited to include the formatter in the resolver in a better way.
The problem was when more than one custom formatter is discovered for a given data type, a non-deterministic selection is made as to which formatter will be used by the generated resolver. We fix this by producing an error in this condition, and suppressing generation of code that uses either formatter for the colliding data types.
They spew NREs all over the place.
neuecc
left a comment
There was a problem hiding this comment.
The changes are so extensive that I cannot carefully review the source code, but the results of running it seem to be very good!
By the way, what happened to the policy for supporting Generics that you commented on here?
#1739 (comment)
[MessagePackFormatter(typeof(MyCustomFormatter<string>))]
// ...
public class MyCustomFormatter<T> : IMessagePackFormatter<int>Currently, it generates invalid source code that cannot be compiled.
If possible, should we make it reject such cases with a Source Generator error?
[MessagePackFormatter(typeof(MyCustomFormatter<int>))]
// ...
public class MyCustomFormatter<T> : IMessagePackFormatter<T>This case seems to be supportable, but currently the formatter generation completely disappears.
The current changes give users the impression that custom formatters are automatically included.
Unless we either issue a warning or provide support, there is a possibility of confusing users.
As an interim policy, it may be fine to say that we don't care about Generics-related cases, but...
|
Thanks for reviewing. And ya, sorry about the size of the PR. I too am relying on tests overall to know that we're moving forward. And yes, we surely need to add more tests for better coverage as we go. Regarding your generics question, I still like my proposal that you linked to. I don't think it's fully implemented yet. Generics is a huge space for this and I think it'll have to come incrementally.
That's exactly right. The only case where |
|
I understand. No worries.
Oh, sorry, I meant to provide the following example but omitted it. [MessagePackObject]
public class MyClass
{
[Key(0)]
[MessagePackFormatter(typeof(MyCustomFormatter<int>))]
public int MyProperty { get; set; }
}
public class MyCustomFormatter<T> : IMessagePackFormatter<T>
{
//...snip
} |

This required a great deal of (worthwhile) refactoring and fixes.
Closes #1739