Skip to content

Check @targetName when subtyping Refined Types #19081

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

Merged
merged 3 commits into from
Nov 30, 2023
Merged

Check @targetName when subtyping Refined Types #19081

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7596819
Check targetName when subtyping Refined Types
hamzaremmal Nov 25, 2023
582192d
Move targetName-refine from pos/ to neg/
hamzaremmal Nov 29, 2023
ea509f4
Add extend the check for targetName when checking matching members
hamzaremmal Nov 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/TypeComparer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2084,7 +2084,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
ExprType(info1.resType)
case info1 => info1

isSubInfo(info1, info2, m.symbol.info.orElse(info1))
(isSubInfo(info1, info2, m.symbol.info.orElse(info1)) && m.symbol.hasTargetName(m.symbol.name))
|| matchAbstractTypeMember(m.info)
|| (tp1.isStable && m.symbol.isStableMember && isSubType(TermRef(tp1, m.symbol), tp2.refinedInfo))
Copy link
Member

@sjrd sjrd Nov 29, 2023

Choose a reason for hiding this comment

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

Why add the target name test only in the first of the three alternatives, and not in all of them? Couldn't we have the same issue in other cases?

end qualifies
Expand Down
7 changes: 7 additions & 0 deletions tests/neg/i18922.check
View file
Open in desktop
Copy link
Member Author

@hamzaremmal hamzaremmal Nov 25, 2023

Choose a reason for hiding this comment

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

I don't like this error message as it does not explain exactly why the types didn't match. We will have to add some custom message probably

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
-- [E007] Type Mismatch Error: tests/neg/i18922.scala:11:27 ------------------------------------------------------------
11 |def test = doClose(Resource()) // error
| ^^^^^^^^^^
| Found: Resource
| Required: Object{def close(): Unit}
|
| longer explanation available when compiling with `-explain`
11 changes: 11 additions & 0 deletions tests/neg/i18922.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import scala.annotation.targetName

def doClose(closable: { def close(): Unit }): Unit =
import reflect.Selectable.reflectiveSelectable
closable.close()

class Resource:
@targetName("foo")
def close(): Unit = ???

def test = doClose(Resource()) // error