Skip to content

Conversation

@jkotas
Copy link
Member

@jkotas jkotas commented Dec 17, 2021

String constructors implementation methods had a dummy this argument on CoreCLR, but not on other runtimes. It required ifdefs in the implementation. This change removes the ifdefs and makes the string constructors implementation methods uniform accross all runtimes. It is possible to do this cleanup now since we have just bumped R2R version band.

@ghost ghost assigned jkotas Dec 17, 2021
@ghost
Copy link

ghost commented Dec 17, 2021

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@jkotas jkotas force-pushed the string-ctors branch 3 times, most recently from 1ed4c99 to 0425ace Compare December 17, 2021 06:09
String constructors implementation methods had a dummy this argument on CoreCLR, but not on other runtimes.  It required ifdefs in the implementation. This change removes the ifdefs and makes the string constructors implementation methods uniform accross all runtimes. It is possible to do this cleanup now since we have just bumped R2R version band.
@jkotas jkotas marked this pull request as ready for review December 17, 2021 14:08
@jkotas jkotas requested a review from jkoritzinsky December 17, 2021 14:09
@jkotas
Copy link
Member Author

jkotas commented Dec 17, 2021

I am sure that there is one more place somewhere that will need to be fixed to account for this change...

@jkotas jkotas merged commit bcd3527 into dotnet:main Dec 17, 2021
@stephentoub
Copy link
Member

It is possible to do this cleanup now since we have just bumped R2R version band.

Are there other cleanups we've been holding off doing but can now do as a result?

@ghost ghost locked as resolved and limited conversation to collaborators Feb 2, 2022
@jkotas jkotas deleted the string-ctors branch February 9, 2022 02:59
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.

3 participants