Skip to content

test: add in regression test for #7445 #17475

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.

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 1 commit into from
May 15, 2023

Conversation

ckipp01
Copy link
Member

@ckipp01 ckipp01 commented May 12, 2023

So this adds in a test that shows that the minimized version of the original issue is fixed, but the original report now fails the pickling tests. I've gone ahead and added a test for that as well, but added it to the excludes. I'll reopen the issue and mention this update.

@nicolasstucki
Copy link
Contributor

We should also include the original example as a regression test.

type O1[A] = {
  type OutInner[Ts] <: Tuple = Ts match {
    case Unit   => Unit
    case h *: t => h *: OutInner[t]
  }
  
  type Out = OutInner[A]
}

def f1: O1[(Int, Int)]#Out = ???

@ckipp01
Copy link
Member Author

ckipp01 commented May 12, 2023

Huh, actually trying out the originally report produces a different error than the original report is about.

Compiling project (Scala 3.3.1-RC1-bin-20230511-5c4e597-NIGHTLY, JVM)
[error] ./example.scala:5:20
[error] Found:    Unit
[error] Required: Tuple
[error]     case Unit   => Unit
[error]                    ^^^^
[warn] ./example.scala:6:25
[warn] forward reference in refinement is deprecated
[warn]     case h *: t => h *: OutInner[t]
[warn]                         ^^^^^^^^
Error compiling project (Scala 3.3.1-RC1-bin-20230511-5c4e597-NIGHTLY, JVM)
Compilation failed

To be honest, i'm not 100% sure on the intended behavior here. Is this indeed illustrating the original issue, another issue, or should this compile?

@nicolasstucki
Copy link
Contributor

That makes sense. The code should be updated to

type O1[A] = {
  type OutInner[Ts] <: Tuple = Ts match {
    case EmptyTuple => EmptyTuple
    case h *: t => h *: OutInner[t]
  }
  
  type Out = OutInner[A]
}

def f1: O1[(Int, Int)]#Out = ???

@ckipp01 ckipp01 marked this pull request as draft May 12, 2023 08:02
@ckipp01 ckipp01 marked this pull request as draft May 12, 2023 08:02
@ckipp01 ckipp01 force-pushed the 7445-regression branch from f780750 to 661f42f Compare May 12, 2023 09:30
@ckipp01
Copy link
Member Author

ckipp01 commented May 12, 2023

That makes sense. The code should be updated to

type O1[A] = {
  type OutInner[Ts] <: Tuple = Ts match {
    case EmptyTuple => EmptyTuple
    case h *: t => h *: OutInner[t]
  }
  
  type Out = OutInner[A]
}

def f1: O1[(Int, Int)]#Out = ???

Alright, thanks. So this doesn't show the original issue, but it does still fail a test with pickling. I've updated the original description to reflect this and I'll update the original issue as well.

@ckipp01 ckipp01 marked this pull request as ready for review May 12, 2023 09:32
@ckipp01 ckipp01 requested a review from nicolasstucki May 12, 2023 09:33
@nicolasstucki nicolasstucki self-assigned this May 12, 2023
@ckipp01
Copy link
Member Author

ckipp01 commented May 12, 2023

Weird so running testCompilation i7445b locally correctly skips this, but when I run the full testOnly dotty.tools.dotc.CompilationTests it does run this one and fails:

26 suites passed, 1 failed, 27 total
tests/pos/i7445b.scala failed

Am I misunderstanding how this .exludes file is working? Shouldn't it skip this?

Nevermind, I realized I should have been using the other excludes for pickling.

This adds in a regression test for the minimized issue, but the original
one now has a different issue with pickling. I've added this in as well,
but added it to the excludes.
@ckipp01 ckipp01 force-pushed the 7445-regression branch from 661f42f to 9310fc8 Compare May 14, 2023 08:14
@nicolasstucki nicolasstucki merged commit 7d6c835 into scala:main May 15, 2023
@ckipp01 ckipp01 deleted the 7445-regression branch May 15, 2023 07:20
Sign up for free to 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.

2 participants