Skip to content

Disallow calling abstract methods directly on interfaces #17021

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
Merged
merged 17 commits into from
Apr 16, 2024
Merged

Disallow calling abstract methods directly on interfaces #17021

merged 17 commits into from
Apr 16, 2024

Conversation

@edgarfgp
Copy link
Contributor

@edgarfgp edgarfgp commented Apr 10, 2024 •
edited
Loading

Description

Fixes #14012
Fixes #16299

  • Check introduced to ensure we can call on virtual members, can't call abstracts.
open System.Numerics
IAdditionOperators.op_Addition (3, 6) // System.BadImageFormatException: Bad IL format.

IAdditionOperators.op_CheckedAddition (3, 6) // Compiles and run as this a static virtual

See:
https://source.dot.net/#System.Private.CoreLib/src/libraries/System.Private.CoreLib/src/System/Numerics/IAdditionOperators.cs,5b77e6b404819054

Before

System.BadImageFormatException: Bad IL format.

After

(Error 3866, Line 15, Col 82, Line 15, Col 129, "A static abstract interface member can be accessed only on a type parameter.")

Rationale:

  • Added an specific error message to make it clear that is not allowed calling static abstract members on an interface.

Checklist

  • Test cases added
  • Release notes entry updated:

@edgarfgp edgarfgp mentioned this pull request Apr 10, 2024
Closed
@github-actions
Copy link
Contributor

github-actions bot commented Apr 10, 2024 •
edited
Loading

❗ Release notes required

✅ Found changes and release notes in following paths:

Change path Release notes path Description
src/Compiler docs/release-notes/.FSharp.Compiler.Service/8.0.400.md

@edgarfgp edgarfgp marked this pull request as ready for review April 10, 2024 19:21
@edgarfgp edgarfgp requested a review from a team as a code owner April 10, 2024 19:21
T-Gro
T-Gro reviewed Apr 11, 2024
View reviewed changes
psfinaki
psfinaki reviewed Apr 12, 2024
View reviewed changes
psfinaki
psfinaki reviewed Apr 12, 2024
View reviewed changes
@edgarfgp
Copy link
Contributor Author

This is ready :)

brianrourkeboll
brianrourkeboll reviewed Apr 15, 2024
View reviewed changes
brianrourkeboll
brianrourkeboll reviewed Apr 15, 2024
View reviewed changes
@edgarfgp
Copy link
Contributor Author

PR to update the IWASM design notes fsharp/fslang-design#767

vzarytovskii
vzarytovskii reviewed Apr 16, 2024
View reviewed changes
@edgarfgp
Copy link
Contributor Author

##[error]The job running on agent Azure Pipelines 68 ran longer than the maximum time of 60 minutes. For more information, see https://go.microsoft.com/fwlink/?linkid=2077134

@edgarfgp edgarfgp closed this Apr 16, 2024
@edgarfgp edgarfgp reopened this Apr 16, 2024
psfinaki
psfinaki approved these changes Apr 16, 2024
View reviewed changes
Copy link
Contributor

@psfinaki psfinaki left a comment

Choose a reason for hiding this comment

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

Great stuff @edgarfgp and thanks @brianrourkeboll for the thorough review and suggestions.

@psfinaki psfinaki merged commit f6ae54b into dotnet:main Apr 16, 2024
@edgarfgp
Copy link
Contributor Author

One less internal error :). Lets go to the next one

psfinaki pushed a commit that referenced this pull request Apr 16, 2024
@dotnet-bot @edgarfgp
@T-Gro @brianrourkeboll
Disallow calling abstract methods directly on interfaces (#17021) (#1…
d11b46f 
…7053)

* Disallow calling abstract methods directly on interfaces

* More tests

* IWSAMs are not supported by NET472

* Update src/Compiler/Checking/ConstraintSolver.fs



* fix typos

* looking for the right check

* Add comments

* move release notes

* Add a new error number and message

* Update docs/release-notes/.FSharp.Compiler.Service/8.0.400.md



* Update docs/release-notes/.FSharp.Compiler.Service/8.0.400.md



* Improve error message

---------

Co-authored-by: Edgar Gonzalez <[email protected]>
Co-authored-by: Tomas Grosup <[email protected]>
Co-authored-by: Brian Rourke Boll <[email protected]>
psfinaki added a commit that referenced this pull request Apr 17, 2024
@dotnet-bot @edgarfgp
@T-Gro @brianrourkeboll @psfinaki @KevinRansom
Merge main to release/dev17.11 (#17056)
ade0697 
* Disallow calling abstract methods directly on interfaces (#17021)

* Disallow calling abstract methods directly on interfaces

* More tests

* IWSAMs are not supported by NET472

* Update src/Compiler/Checking/ConstraintSolver.fs

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

* fix typos

* looking for the right check

* Add comments

* move release notes

* Add a new error number and message

* Update docs/release-notes/.FSharp.Compiler.Service/8.0.400.md

Co-authored-by: Brian Rourke Boll <[email protected]>

* Update docs/release-notes/.FSharp.Compiler.Service/8.0.400.md

Co-authored-by: Brian Rourke Boll <[email protected]>

* Improve error message

---------

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

* Always use `typeEquivAux EraseMeasures` (for  integral range optimizations) (#17048)

* Always use `typeEquivAux EraseMeasures`

* Update release notes

* Update baselines

---------

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

---------

Co-authored-by: Edgar Gonzalez <[email protected]>
Co-authored-by: Tomas Grosup <[email protected]>
Co-authored-by: Brian Rourke Boll <[email protected]>
Co-authored-by: Petr <[email protected]>
Co-authored-by: Kevin Ransom (msft) <[email protected]>
psfinaki added a commit that referenced this pull request Apr 17, 2024
@dotnet-bot @edgarfgp
@T-Gro @brianrourkeboll @psfinaki @vzarytovskii
Merge main to release/dev17.11 (#17059)
6b6bbf0 
* Disallow calling abstract methods directly on interfaces (#17021)

* Disallow calling abstract methods directly on interfaces

* More tests

* IWSAMs are not supported by NET472

* Update src/Compiler/Checking/ConstraintSolver.fs

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

* fix typos

* looking for the right check

* Add comments

* move release notes

* Add a new error number and message

* Update docs/release-notes/.FSharp.Compiler.Service/8.0.400.md

Co-authored-by: Brian Rourke Boll <[email protected]>

* Update docs/release-notes/.FSharp.Compiler.Service/8.0.400.md

Co-authored-by: Brian Rourke Boll <[email protected]>

* Improve error message

---------

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

* Always use `typeEquivAux EraseMeasures` (for  integral range optimizations) (#17048)

* Always use `typeEquivAux EraseMeasures`

* Update release notes

* Update baselines

---------

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

---------

Co-authored-by: Edgar Gonzalez <[email protected]>
Co-authored-by: Tomas Grosup <[email protected]>
Co-authored-by: Brian Rourke Boll <[email protected]>
Co-authored-by: Petr <[email protected]>
Co-authored-by: Vlad Zarytovskii <[email protected]>
psfinaki added a commit that referenced this pull request Apr 18, 2024
@dotnet-bot @edgarfgp
@T-Gro @brianrourkeboll @psfinaki @vzarytovskii @github-actions @KevinRansom
Merge main to release/dev17.11 (#17064)
27efc28 
* Disallow calling abstract methods directly on interfaces (#17021)

* Disallow calling abstract methods directly on interfaces

* More tests

* IWSAMs are not supported by NET472

* Update src/Compiler/Checking/ConstraintSolver.fs

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

* fix typos

* looking for the right check

* Add comments

* move release notes

* Add a new error number and message

* Update docs/release-notes/.FSharp.Compiler.Service/8.0.400.md

Co-authored-by: Brian Rourke Boll <[email protected]>

* Update docs/release-notes/.FSharp.Compiler.Service/8.0.400.md

Co-authored-by: Brian Rourke Boll <[email protected]>

* Improve error message

---------

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

* Always use `typeEquivAux EraseMeasures` (for  integral range optimizations) (#17048)

* Always use `typeEquivAux EraseMeasures`

* Update release notes

* Update baselines

---------

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

* Error message that explicitly disallowed static abstract members in classes. (#17055)

* WIP

* Error message that explicitly disallowed static abstract methods in abstract classes

* release notes

* SynTypeDefnKind.Class

* Fix #16761 (#17047)

* Fix #16761

* Fully async version + ignore cancellation on external navigation

* Automated command ran: fantomas

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

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

---------

Co-authored-by: Edgar Gonzalez <[email protected]>
Co-authored-by: Tomas Grosup <[email protected]>
Co-authored-by: Brian Rourke Boll <[email protected]>
Co-authored-by: Petr <[email protected]>
Co-authored-by: Vlad Zarytovskii <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Kevin Ransom (msft) <[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 left review comments

@vzarytovskii vzarytovskii vzarytovskii approved these changes

+2 more reviewers

@brianrourkeboll brianrourkeboll brianrourkeboll approved these changes

@psfinaki psfinaki psfinaki approved these changes

Reviewers whose approvals may not affect merge requirements

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.

Accessing to IWSAM static abstract member produces Bad IL format Bad image when calling op_Addition on interface

5 participants

@edgarfgp @vzarytovskii @psfinaki @brianrourkeboll @T-Gro