Skip to content

Report error diagnostic when a specific overload from a single template would be generated multiple times#2

Merged
InsanityCode merged 8 commits into
add-template-generatorfrom
diagnose-generating-overload-duplicates
Jun 2, 2024
Merged

Report error diagnostic when a specific overload from a single template would be generated multiple times#2
InsanityCode merged 8 commits into
add-template-generatorfrom
diagnose-generating-overload-duplicates

Conversation

@FramePerfection

Copy link
Copy Markdown
Member

This adds an error diagnostic when a single template delegate in combination with its respective TemplateGenerator attribute would generate the same overload multiple times.

For exmample, the following code would generate an error diagnostic:

[ChaosGenerators.TemplateGenerator(
    "=> new System.Tuple<T1, T2, T3>(v1, v2, v3[0]);",
    typeof(float), typeof(int), typeof(string),
    typeof(float), null, typeof(string),
    typeof(float), null, typeof(string), // this would generate the same overload as the line above
    null, typeof(int), null
    )]
internal delegate System.Tuple<T1, T2, T3> _Test<T1, T2, T3>(ref T1 v1, ref readonly T2 v2, params T3[] v3);

It may be better to use ChaosUtil's Array equality checks rather than the somewhat scuffed OverloadComparer solution, which would then operate on a TemplateParameter type (or similar in name) replacing the anonymous ((Regex, string), ITypeSymbol) type, hence the draft label.

@FramePerfection FramePerfection added draft This is a draft pull request. Do not merge (yet). enhancement New feature or request labels Jun 1, 2024

@InsanityCode InsanityCode left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The overall changes seem fine to me.

I implemented your suggestions regarding the elimination of anonymous types and using ChaosUtil.Primitives (see #4), so if these changes meet your expectations, please merge them into this PR.

As mentioned here, I think raising a warning would be enough, but changing the error to a warning can be done later.

…ad-duplicates-review

diagnose generating overload duplicates - review

@InsanityCode InsanityCode left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

@InsanityCode InsanityCode merged commit 805f60d into add-template-generator Jun 2, 2024
@InsanityCode InsanityCode deleted the diagnose-generating-overload-duplicates branch June 2, 2024 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

draft This is a draft pull request. Do not merge (yet). enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants