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

Signature of nested type with generic type parameter #15259

Merged
merged 9 commits into from
Jun 5, 2023

Conversation

nojaf
Copy link
Contributor

@nojaf nojaf commented May 24, 2023

When trying to create a signature for:

open System.Collections.Immutable

type ImmutableArrayViaBuilder<'T>(builder: ImmutableArray<'T>.Builder) = class end

the compiler generates:

type ImmutableArrayViaBuilder<'T> =
  new: builder: System.Collections.Immutable.ImmutableArray`1.Builder<'T> ->
         ImmutableArrayViaBuilder<'T>

System.Collections.Immutable.ImmutableArray`1.Builder<'T> is the problematic bit,
it should be System.Collections.Immutable.ImmutableArray<'T>.Builder.

I'm guessing Builder is a nested class?

My initial fix is not the way to go. And I could use a tip on how to tackle this in a nicer way.
I'm all hears 😇.

PS: @T-Gro I think my test will only work on net7.0, any pointers on how to address that?

@nojaf nojaf force-pushed the immutable-array-builder-sig branch from 5e644e4 to d99b563 Compare May 24, 2023 13:12
@vzarytovskii
Copy link
Member

vzarytovskii commented May 24, 2023

PS: @T-Gro I think my test will only work on net7.0, any pointers on how to address that?

#ifdef it, but will probably need to do the _OR_GREATER veresion. Or ignore it for non-net7

Or, even better, you probably can create this nested type and reference it in your test.

Update: oh, it just enumerates all files. In this case, it's probably easier to just manually add one separate test, which also references custom C# code, and compiles (withReferences function in framework).

@nojaf
Copy link
Contributor Author

nojaf commented May 24, 2023

A slightly better attempt by storing the generic parameters in ModuleOrType.
I'm most likely still missing some edge cases.

@nojaf nojaf marked this pull request as ready for review May 25, 2023 15:04
@nojaf nojaf requested a review from a team as a code owner May 25, 2023 15:04
@nojaf
Copy link
Contributor Author

nojaf commented May 25, 2023

This is ready.

@psfinaki psfinaki merged commit 97c650f into dotnet:main Jun 5, 2023
psfinaki pushed a commit that referenced this pull request Jun 6, 2023
* add review comment to sb files (#15288)

* add review comment to sb files

* add CODEOWNERS entry for source-build

* Don't show inline hint for arguments with same names as the parameters in DU (#15305)

* Signature of nested type with generic type parameter (#15259)

* Proof of concept

* Add generic parameter names to ModuleOrType.

* Revert ModuleOrType change

* Process ticks in demangledPath of TType_app.

* Only apply new logic when includeStaticParametersInTypeNames is active.

* Use FactForNETCOREAPP

* Fix build

---------

Co-authored-by: Tomas Grosup <[email protected]>

* Improve implied lambda and delegate argument names (#15277)

* Improve implied lambda and delegate argument names

* Fix

* Add tests

* Revert non-preview tests

* Sigh

* Re-revert

* Fix test

* Add testx

---------

Co-authored-by: Oleksandr Didyk <[email protected]>
Co-authored-by: Sudqi <[email protected]>
Co-authored-by: Florian Verdonck <[email protected]>
Co-authored-by: Tomas Grosup <[email protected]>
Co-authored-by: kerams <[email protected]>
T-Gro added a commit that referenced this pull request Jun 6, 2023
* add review comment to sb files (#15288)

* add review comment to sb files

* add CODEOWNERS entry for source-build

* Don't show inline hint for arguments with same names as the parameters in DU (#15305)

* Signature of nested type with generic type parameter (#15259)

* Proof of concept

* Add generic parameter names to ModuleOrType.

* Revert ModuleOrType change

* Process ticks in demangledPath of TType_app.

* Only apply new logic when includeStaticParametersInTypeNames is active.

* Use FactForNETCOREAPP

* Fix build

---------

Co-authored-by: Tomas Grosup <[email protected]>

* Improve implied lambda and delegate argument names (#15277)

* Improve implied lambda and delegate argument names

* Fix

* Add tests

* Revert non-preview tests

* Sigh

* Re-revert

* Fix test

* Add testx

---------

Co-authored-by: Oleksandr Didyk <[email protected]>
Co-authored-by: Sudqi <[email protected]>
Co-authored-by: Florian Verdonck <[email protected]>
Co-authored-by: Tomas Grosup <[email protected]>
Co-authored-by: kerams <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants