extractors broken: "not enough arguments for method body%1"

[scabug]

Related to #2883, which was fixed by avoiding the use of extractors. Now I really need them in refactoring some stuff in the compiler, though...

class Type
class Symbol
case class PolyType(tps: List[Symbol], res: Type) extends Type

object NullaryMethodType {
  /** Creator for nullary method types.

    @note: for now, this returns a PolyType with an empty type-parameter list, 
       but this will eventually change to a NullaryMethodType
  */
  def apply(resTpe: Type): Type = PolyType(List(), resTpe)
    
  /** Extractor for nullary method types.

     @note: to ease transition to NullaryMethodType (instead of PolyType with empty tparam list)
   */
  def unapply(tp: Type): Option[(Type)] = tp match {
    case PolyType(List(), restpe) => Some((restpe))
    case _ => None
  }
}

object Test {
  def apply(tp: Type): Type = 
    tp match {
      case PolyType(params, PolyType(tparams, restpe @ PolyType(_, _))) => tp // couldn't find a simpler version that still crashes
      case NullaryMethodType(restpe) => tp
    }
}

/* error:
not enough arguments for method body%1: (val restpe: Type)Type.
Unspecified value parameter restpe.
*/

[scabug]

Imported From: https://issues.scala-lang.org/browse/SI-3136?orig=1
Reporter: @adriaanm

[scabug]

@paulp said:
I'm very glad to hear that you need them! Because there's no way I'm going to iron out the extractor issues on my own in the near future. I have already sunk soooo much time into it and when I weigh the likely outcomes of ways I can spend time, choosing between fixing fifty things for sure and extractors maybe, "extractors maybe" loses.

So if you want to make extractors "our problem" that would provide me with some new enthusiasm, but I can't do it right now when there is so much else that only I can (or will) do.

[scabug]

@adriaanm said:
Yes, I'm going to have a look right now. I certainly didn't mean to shove it down your throat -- I realise the pattern matcher can be somewhat of a black hole for time (let's call it structured procrastination).

[scabug]

@adriaanm said:
well, I spent all daying wading through the code (I'd rather go skinny dipping in a marsh filled with cranky gators, ya'll)
[disclaimer: Paul, I don't mean to shoot the refactorer, I know the origin of these problems is moar deeply rooted)

one revelation was triggered by overriding def isLabellable to be false like, all the time, and observe the broken code that is spat out... the condition that tests the result of the unapply is only generated for the final case, not in the else branches of the nested patterns -- this seems like a major clue

this leads me to believe applyRule() in Rep is broken (btw, mixture has been factored out to death, I'd say, bigger methods aren't always worse)

anyway, maybe we need some logic to deal with the case that an unapply label is going to be referenced before its time has come

then again, I've been moodswinging between this explanation and the observation that subpatterns cause insert2 to be called to insert extra rows, so that one would expect some kind of index-patching up code, but haven't been able to find it (so you want the body for index 1? in which list? what's the unit of that 1? do i really need to point you to the wikipedia page on the Mars Climate Orbiter?)

suggestions for future cleaner-upper of this code: get rid of those bloody integer indices -- objects have identity too, you know....

[scabug]

@adriaanm said:
I forgot to add my wadings can be followed here: http://github.com/adriaanm/scala/tree/ticket/3136
(though not much to see)

[scabug]

@paulp said:
Replying to [comment:3 moors]:

[disclaimer: Paul, I don't mean to shoot the refactorer, I know the origin of these problems is moar deeply rooted)

Oh yes. I will sometimes be protective of my code, but absolutely not in the pattern matcher. So much of it is the process of trying out many different often insensible things trying to untangle the machinery. There's still plenty I know is wrong, and the names of things in some cases are for sure wrong, which is frustrating, but as they were often in the past named things like "temp" and I'm only trying to eke them along.

I will take a look at the areas you mentioned: it is a huge help to have any kind of "this is wrong" lifeline to grab onto.

suggestions for future cleaner-upper of this code: get rid of those bloody integer indices -- objects have identity too, you know....

Yeah, don't I know it. I tried to do that at some point and it was one of many times I discovered that they were doing something beyond what I understood they were doing. Note the use of negative indices... there was something bizarre about it. I thought I'd duplicated it but things broke.

[scabug]

@paulp said:
Replying to [comment:3 moors]:

one revelation was triggered by overriding def isLabellable to be false like, all the time, and observe the broken code that is spat out... the condition that tests the result of the unapply is only generated for the final case, not in the else branches of the nested patterns -- this seems like a major clue

You need to read my email where I explained the origin of #1697 and #2337 if you haven't already.

http://article.gmane.org/gmane.comp.lang.scala.internals/2752

The problem is in MixTypes, because the unapply gets broken up into two tests which end up separating and breaking the logic.

It should be: if (x && y) body1 else body2
but it ends up like: if (x) { if (y) body1 else fail } else body2

However MixTypes has completely foiled my attempts to deconstruct it far enough to accomplish this. The entirety of Patterns.scala and all code which uses it is essentially my attempt at rewriting the whole thing to keep such tests together. By the way Patterns.scala should be a lot closer to code we want to use than the rest of it, but it still has many things I'm not proud because to use it I had to integrate with the rest of it.

[scabug]

@paulp said:
Replying to [comment:4 moors]:

I forgot to add my wadings can be followed here: http://github.com/adriaanm/scala/tree/ticket/3136
(though not much to see)

Also: for more on this, see http://github.com/paulp/scala/tree/bug1697

That contains the distillation of the case I am aware of where it generates incorrect conditional logic. It'll yell at you if it sees it.

[scabug]

@paulp said:
Replying to [comment:3 moors]:

well, I spent all daying wading through the code (I'd rather go skinny dipping in a marsh filled with cranky gators, ya'll)

I had no idea you were such a fountain of homespun wisdom.

OK, this is fixed and guarded with your test in r21081. I know other open tickets were caused by the same change. This sent SI-1697's test case back into pending. Now that I assume your immediate issue is gone, the temptation will be to forget all about the pattern matcher and its many victims. Although I would completely understand, that understanding wouldn't stop me from hunting you down and throwing you to the gators.

[scabug]

@acruise said:
Well, this is entertaining. At Paul's suggestion I'm using -Ypmat-debug to see if I can get more insight into just what's happening, and at the moment it looks like code that compiles just fine without -Ypmat-debug is failing with this (or a very similar) problem when -Ypmat-debug is enabled.

[scabug]

@acruise said:
And, just to capture a little more information about the problem...

Leaving -Ypmat-debug out of the picture, this line crashes due to this (or similar) bug:

case app @ Apply(x,y,z) => ...

When any of x, y or z are there, it crashes. But this doesn't:

case app @ Apply(_,_,_) => ...

And finally, this version avoids the extractor and therefore can't possibly fail:

case app: Apply[_] => ...