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.

Sign up for GitHub

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

Jump to bottom

Change ty1.Equals(ty2) to call static op_Equality #13028

Merged
merged 4 commits into from
Oct 12, 2022
Merged

Change ty1.Equals(ty2) to call static op_Equality #13028

merged 4 commits into from
Oct 12, 2022

Conversation

rstm-sf
Copy link
Contributor

@rstm-sf rstm-sf commented Apr 20, 2022

Closes #8447

@rstm-sf rstm-sf marked this pull request as ready for review April 21, 2022 18:59
@dsyme
Copy link
Contributor

dsyme commented May 19, 2022

@rstm-sf This needs to be merged with main, can you do that please? Thank you

@edgarfgp
Copy link
Contributor

@vzarytovskii This would a good improvement to have merged to main ?

@vzarytovskii vzarytovskii requested a review from dsyme September 27, 2022 10:07
T-Gro
T-Gro previously approved these changes Oct 7, 2022
View reviewed changes
@T-Gro
Copy link
Member

T-Gro commented Oct 7, 2022

@vzarytovskii This would a good improvement to have merged to main ?

Would be good (uses a more fitting overload, skipping type checks when they are known).
It has some conflicts since creation...

@T-Gro T-Gro requested review from vzarytovskii and 0101 October 7, 2022 12:42
@rstm-sf
Merge branch 'main' into feature/ty.Equals(typeof_'T_)_to_Type.op_Equ…
63b98a2 
…ality(typeof_'T_,_ty)

# Conflicts:
#	src/FSharp.Core/prim-types.fs
@tannergooding
Copy link
Member

tannergooding commented Oct 7, 2022 •
edited
Loading

Edit: Ah, I misread the issue. This is only for type equality, not value equality. Sorry for the noise.

It's worth noting that for float, double, System.Half, and potentially other types == and Equals are not the same.

In the case of the IEEE 754 floating-point types, == follows IEEE 754 semantics and so NaN != NaN, even for bitwise equivalent values. However, Equals follows .NET semantics and fits the needs around using a type in dictionaries, sets, and other collection types and so NaN.Equals(NaN), even for NaNs of differing signs or payloads.

@0101 0101 merged commit 889709d into dotnet:main Oct 12, 2022
@rstm-sf rstm-sf deleted the feature/ty.Equals(typeof_'T_)_to_Type.op_Equality(typeof_'T_,_ty) branch October 12, 2022 17:26
T-Gro pushed a commit that referenced this pull request Oct 13, 2022
@dotnet-bot @nojaf
@rstm-sf @vzarytovskii @dsyme
Merge main to release/dev17.5 (#14098)
b9053b2 
* Add SynType.Or. (#14058)

* Add SynType.Or for generic constrains in the form (^A or ^B):...

* Change ty1.Equals(ty2) to call static op_Equality (#13028)

Co-authored-by: Vlad Zarytovskii <[email protected]>
Co-authored-by: Don Syme <[email protected]>

Co-authored-by: Florian Verdonck <[email protected]>
Co-authored-by: Rustam <[email protected]>
Co-authored-by: Vlad Zarytovskii <[email protected]>
Co-authored-by: Don Syme <[email protected]>
@allisonchou allisonchou mentioned this pull request Oct 20, 2022
Closed
vzarytovskii added a commit that referenced this pull request Jan 11, 2023
@dotnet-bot @vzarytovskii
@dsyme @T-Gro @edgarfgp @auduchinok @baronfel @dotnet-maestro @nojaf @psfinaki @0101 @teo-tsirpanis @KevinRansom @rstm-sf @abonie @epananth @MattGal @github-actions @NinoFloris
Merge release/dev17.5 to main (#14577)
f1ec7d7 
* Localized file check-in by OneLocBuild Task: Build definition ID 499: Build ID 1997730 (#13925) (#14041)

Co-authored-by: Vlad Zarytovskii <[email protected]>

* Merge main to release/dev17.5 (#14043)

Co-authored-by: Vlad Zarytovskii <[email protected]>
Co-authored-by: Don Syme <[email protected]>
Co-authored-by: Tomas Grosup <[email protected]>
Co-authored-by: Edgar Gonzalez <[email protected]>
Co-authored-by: Eugene Auduchinok <[email protected]>
Co-authored-by: Chet Husk <[email protected]>
Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Florian Verdonck <[email protected]>
Co-authored-by: Petr <[email protected]>
Co-authored-by: Petr Pokorny <[email protected]>
Co-authored-by: Theodore Tsirpanis <[email protected]>

* Update FSharp.Editor.fsproj

I believe a bad merge happened, this line is not in main.  And the file does not exist in either branch.

* Localized file check-in by OneLocBuild Task: Build definition ID 499: Build ID 2014480 (#14049)

* Localized file check-in by OneLocBuild Task: Build definition ID 499: Build ID 2016907

* Localized file check-in by OneLocBuild Task: Build definition ID 499: Build ID 2016985

* XLF

* Localized file check-in by OneLocBuild Task: Build definition ID 499: Build ID 2017073

* Localized file check-in by OneLocBuild Task: Build definition ID 499: Build ID 2017391

* Localized file check-in by OneLocBuild Task: Build definition ID 499: Build ID 2017391

* Localized file check-in by OneLocBuild Task: Build definition ID 499: Build ID 2018131 (#14089)

* Bugfix: Ref assemblies contained .property definitions at the wrong type in generated IL (#14093)

* Bugfix: Ref assemblies contained property definitions at the wrong type

* Better comment

* Update versions for dev17.4 (#14102)

* Update versions for dev17.5 (#14100)

* Merge main to release/dev17.5 (#14098)

* Add SynType.Or. (#14058)

* Add SynType.Or for generic constrains in the form (^A or ^B):...

* Change ty1.Equals(ty2) to call static op_Equality (#13028)

Co-authored-by: Vlad Zarytovskii <[email protected]>
Co-authored-by: Don Syme <[email protected]>

Co-authored-by: Florian Verdonck <[email protected]>
Co-authored-by: Rustam <[email protected]>
Co-authored-by: Vlad Zarytovskii <[email protected]>
Co-authored-by: Don Syme <[email protected]>

* Localized file check-in by OneLocBuild Task: Build definition ID 499: Build ID 2023680 (#14133)

* Disable ref assemblies in e2e tests (#14135)

Disable reference assemblies for e2e test of type providers

Co-authored-by: Adam Boniecki <[email protected]>
Co-authored-by: Vlad Zarytovskii <[email protected]>

* Localized file check-in by OneLocBuild Task: Build definition ID 499: Build ID 2024009 (#14139)

* Downgrade SDK, rc2 is failing signing (#14146)

* Generate SBOM for Fsharp (#14029) (#14169)

* Generate Sbom

* pass ci flag

* update

* Sbom generation

* Fix for trimming tests: Added nuget.org source + explicit source mapping for FSharp.Core

* Update Build.ps1

Tweaks to handle useGlobalNugetCache

Co-authored-by: Kevin Ransom (msft) <[email protected]>
Co-authored-by: Vlad Zarytovskii <[email protected]>
Co-authored-by: Kevin Ransom (msft) <[email protected]>

Co-authored-by: Epsitha Ananth <[email protected]>
Co-authored-by: Kevin Ransom (msft) <[email protected]>
Co-authored-by: Vlad Zarytovskii <[email protected]>
Co-authored-by: Kevin Ransom (msft) <[email protected]>

* Update Arcade & Put automated pool provider usage functionality into Dev17.4 branch (similar to PR in main now but will not be backported here) (#14177)

* [release/dev17.4] fix metadata failure due to double integration of signature (#14190)

* fix metadata failure due to double duplication

* fix metadata failure due to double duplication

Co-authored-by: Don Syme <[email protected]>

* Global.json

* [release/dev17.4] Don't emit IsReadOnlyAttribute if not available. (#14281)

Co-authored-by: nojaf <[email protected]>

* Fixed package versions to publicly available (#14291)

* Fixed package versions to publicly available

* Update Versions.props

Microsoft.Build.* to 17.4.0

Co-authored-by: Kevin Ransom (msft) <[email protected]>

* Update Versions.props

* Prefer nullable over other conversions, fixes #14302

* Replace ROSpan for DateTimeOffset as op_Implicit target, ROSpan is not defined on all test TFMs

* [release/dev17.4] F# 7 fixes (#14322)

* WIP: Fix for calling init-only setter via srtp call + allow calling special-named functions via srtp
* Fix 14097

Co-authored-by: Vlad Zarytovskii <[email protected]>
Co-authored-by: Tomas Grosup <[email protected]>
Co-authored-by: Don Syme <[email protected]>

* Localized file check-in by OneLocBuild Task: Build definition ID 499: Build ID 2051937 (#14381)

* Deploy System.Diagnostics.DiagnosticSource to Tools folder (#14417)

* Localized file check-in by OneLocBuild Task: Build definition ID 499: Build ID 2059214 (#14427)

* Revert "IL: optimize attribute cluster reading (#13821)"

This reverts commit 179db4e.

* Localized file check-in by OneLocBuild Task: Build definition ID 499: Build ID 2067933 (#14472)

* Localized file check-in by OneLocBuild Task: Build definition ID 499: Build ID 2067933

* Localized file check-in by OneLocBuild Task: Build definition ID 499: Build ID 2067933

* Localized file check-in by OneLocBuild Task: Build definition ID 499: Build ID 2068561 (#14474)

* Localized file check-in by OneLocBuild Task: Build definition ID 499: Build ID 2068115

* Localized file check-in by OneLocBuild Task: Build definition ID 499: Build ID 2068115

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

* Localized file check-in by OneLocBuild Task: Build definition ID 499: Build ID 2077478 (#14533)

* Revert "Merge branch 'release/dev17.5' of https://github.com/dotnet/fsharp into release/dev17.5"

This reverts commit 4d222de, reversing
changes made to 2e92791.

* Update azure-pipelines.yml

* Merge main to release/dev17.5 (#14562)

Co-authored-by: Petr <[email protected]>
Co-authored-by: Vlad Zarytovskii <[email protected]>

Co-authored-by: Vlad Zarytovskii <[email protected]>
Co-authored-by: Don Syme <[email protected]>
Co-authored-by: Tomas Grosup <[email protected]>
Co-authored-by: Edgar Gonzalez <[email protected]>
Co-authored-by: Eugene Auduchinok <[email protected]>
Co-authored-by: Chet Husk <[email protected]>
Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Florian Verdonck <[email protected]>
Co-authored-by: Petr <[email protected]>
Co-authored-by: Petr Pokorny <[email protected]>
Co-authored-by: Theodore Tsirpanis <[email protected]>
Co-authored-by: Kevin Ransom (msft) <[email protected]>
Co-authored-by: Rustam <[email protected]>
Co-authored-by: Adam Boniecki <[email protected]>
Co-authored-by: Adam Boniecki <[email protected]>
Co-authored-by: Epsitha Ananth <[email protected]>
Co-authored-by: Kevin Ransom (msft) <[email protected]>
Co-authored-by: Matt Galbraith <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Nino Floris <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@T-Gro T-Gro T-Gro approved these changes

@0101 0101 0101 approved these changes

@dsyme dsyme Awaiting requested review from dsyme

@vzarytovskii vzarytovskii Awaiting requested review from vzarytovskii

Assignees
No one assigned
Labels
None yet
Projects
F# Compiler and Tooling
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Change ty.Equals(typeof<'T>) to Type.op_Equality(typeof<'T>, ty) to please RyuJIT
7 participants
@rstm-sf @dsyme @edgarfgp @T-Gro @tannergooding @0101 @vzarytovskii