Skip to content

Specializing Operators.op_UnaryPlus for witness info - #9769

Merged
KevinRansom merged 8 commits into
dotnet:masterfrom
TIHan:fix-unary-plus-witness
Jul 25, 2020
Merged

Specializing Operators.op_UnaryPlus for witness info#9769
KevinRansom merged 8 commits into
dotnet:masterfrom
TIHan:fix-unary-plus-witness

Conversation

@TIHan

@TIHanTIHan commented Jul 24, 2020

Copy link
Copy Markdown
Contributor

I got an internal compiler error when trying to compile one of the "hello world" test type providers under the preview flag. The error described that op_UnaryPlus had no witness info.

Upon investigation, we didn't add an UnaryPlusDynamic function to handle the witness info creation in TcGlobals.
Two ways to handle it:

  1. Add UnaryPlusDynamic public API so it will be picked up when looking for built-in witness infos
    or
  2. Specialize op_UnaryPlus call to id function for built-in witness info

@TIHan
TIHan requested a review from dsymeJuly 24, 2020 02:33

@cartermpcartermp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The change looks good but there is still a failure in the type provider test.

@TIHan

Copy link
Copy Markdown
ContributorAuthor

I think adding UnaryPlusDynamic is the right thing to do as it follows the current pattern. But, it does seem a little odd to create a new public API that just returns itself. Maybe there is a better way to do this without introducing a new public API.

@TIHanTIHan changed the title Added UnaryPlusDynamic for witness infoSpecializing op_UnaryPlus for witness infoJul 24, 2020
@TIHan

Copy link
Copy Markdown
ContributorAuthor

Ok, I managed to figure it out without adding to the public API. Basically, op_UnaryPlus will choose Operators.id for the witness info. I like this better versus having to add something to the public API that isn't necessary.

@TIHanTIHan changed the title Specializing op_UnaryPlus for witness infoSpecializing Operators.op_UnaryPlus for witness infoJul 24, 2020
@KevinRansom
KevinRansom merged commit 87114ec into dotnet:masterJul 25, 2020
nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Feb 23, 2021
* Initial work to fix op_UnaryPlus witness info
* Added UnaryPlusDynamic
* update surface area
* Only enable type provider test on net472
* Removing UnaryPlusDynamic
* fixing build
* fix comment
* Update prim-types.fs
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@TIHan@KevinRansom@cartermp