compiler things that make you go "hmmm"

On Fri, Oct 15, 2010 at 10:56 PM, Paul Phillips <paulp@improving.org> wrote:

Here is a test performed in Namers:

 name.endsWith(nme.OUTER, nme.OUTER.length)

I noticed there is an optimization available:

 name.startsWith(nme.OUTER)

Though it seems more likely it's not testing what's intended.  I believe
it's one of the "zombie conditions" we find in various large boolean
expression whose falsity does not impair a general state of workingness
(but does make things kind of harder to understand.)

I think it was just a missing refactoring. Probably the index was something else before, because the naming of outer fields was different, and then it got changed, and the opportunity for replacing the call was missed.

I don't think it's a zombie condition at all. It says that outer pointers do not get getters which makes eminent sense to me.

Generally, one needs to be very careful with assuming zombie conditions. In my experience, often the most tricky code handles very tricky situations, which are overlooked by casual inspection. We need to document better what these situations are, when we come across code like this. Sometimes, of course, the code is redundant or wrong, but the probabilities tend to point in the other direction.

Cheers

 -- Martin

Epilogue: that is the only usage of the two-argument form of endsWith
anywhere in the compiler.

--
Paul Phillips      | The important thing here is that the music is not in
Stickler           | the piano.  And knowledge and edification is not in the
Empiricist         | computer.  The computer is simply an instrument whose
slap pi uphill!    | music is ideas.  -- Alan Kay

On Sat, Oct 16, 2010 at 11:52:45AM +0200, martin odersky wrote:
> I think it was just a missing refactoring. Probably the index was
> something else before, because the naming of outer fields was
> different, and then it got changed, and the opportunity for replacing
> the call was missed.
>
> I don't think it's a zombie condition at all. It says that outer
> pointers do not get getters which makes eminent sense to me.

I don't always reveal everything I know, but when I encounter something
which I don't understand I usually resort to logging what's happening.
So I already knew the test doesn't do anything, unless it doesn't come
up in the whole distribution. Here is the whole list of names
encountered at that test, and you will notice there are no synthetic
names unless you count lexically determined $bang and friends as
synthetic.

http://pastie.org/1225739

And the test is in the exact form it was introduced, passing the length
of the first argument as the second argument: given the implementation
of endsWith it was and is equivalent to "startsWith" no matter what the
outer naming scheme was.

> Generally, one needs to be very careful with assuming zombie
> conditions. In my experience, often the most tricky code handles very
> tricky situations, which are overlooked by casual inspection.

My inspections are never casual. (Also, I haven't yet touched it.) The
trickier the situation, the more important we get dead/wrong code out of
there, so if you don't want me to pull it perhaps you could take a look.

On Sat, Oct 16, 2010 at 4:59 PM, Paul Phillips <paulp@improving.org> wrote:

On Sat, Oct 16, 2010 at 11:52:45AM +0200, martin odersky wrote:
> I think it was just a missing refactoring. Probably the index was
> something else before, because the naming of outer fields was
> different, and then it got changed, and the opportunity for replacing
> the call was missed.
>
> I don't think it's a zombie condition at all. It says that outer
> pointers do not get getters which makes eminent sense to me.

I don't always reveal everything I know, but when I encounter something
which I don't understand I usually resort to logging what's happening.
So I already knew the test doesn't do anything, unless it doesn't come
up in the whole distribution.  Here is the whole list of names
encountered at that test, and you will notice there are no synthetic
names unless you count lexically determined $bang and friends as
synthetic.

 http://pastie.org/1225739

I think that's because outer fields happen to be never entered in the current compiler structiure. But I would argue that the test is useful nevertheless. If some later transformation decides to generate code with outer fields and let the typechecker take care of entering, it will do the right thing.
 

And the test is in the exact form it was introduced, passing the length
of the first argument as the second argument: given the implementation
of endsWith it was and is equivalent to "startsWith" no matter what the
outer naming scheme was.

It was committed that way, but my guess is that it was still a refactoring from some internal version, which had a different end index. Otherwise it would simly make no sense.
 

> Generally, one needs to be very careful with assuming zombie
> conditions. In my experience, often the most tricky code handles very
> tricky situations, which are overlooked by casual inspection.

My inspections are never casual.  (Also, I haven't yet touched it.) The
trickier the situation, the more important we get dead/wrong code out of
there, so if you don't want me to pull it perhaps you could take a look.

I agree. But how do you find out what code is dead or redundant?

Cheers

 -- Martin

On Sat, Oct 16, 2010 at 05:40:04PM +0200, martin odersky wrote:
> I think that's because outer fields happen to be never entered in the
> current compiler structiure. But I would argue that the test is useful
> nevertheless. If some later transformation decides to generate code
> with outer fields and let the typechecker take care of entering, it
> will do the right thing.

I have no objection to this sort of thinking, it's just terribly
difficult to infer when there is no documentation, no test where it
makes any difference, and the original commit message is "added test for
outer in Namers.scala" which is actually one of the better checkin
comments when I think of all of them.

I can't default to assuming that everything which seems insensible would
make sense if I knew what someone else was thinking, or I couldn't have
done 95% of the work I've done in the last two years. And I can't ask
about everything: I mean I can, but I wouldn't get enough answers and
everyone would get tired of it really quickly. (If we want to go that
route I'll start by reviving a selection from my "unanswered archives"
which will keep us busy for a while.)

> I agree. But how do you find out what code is dead or redundant?

All code should meet one of these tests:

1) if you change it, something will change and preferably break OR
2) it is documented as to why 1) is not true

This is a big part of the reasons tests are so important. It isn't
necessarily that you think tests will find new bugs, it's that they
provide a critical form of documentation (often the only form) for those
who might modify the source. I can't even imagine how much harder my
life would have been without the tests we have, but the compiler is
still only sparsely covered.

Any code which fails both tests is a bug, in the grander sense of being
an impediment to the reliability and improvement of the code. I
understand that there is an awful lot of code which meets neither of
these tests, so I'm very careful. But they're still all bugs, and they
need to be flushed out.

On Sat, Oct 16, 2010 at 6:15 PM, Paul Phillips <paulp@improving.org> wrote:

On Sat, Oct 16, 2010 at 05:40:04PM +0200, martin odersky wrote:
> I think that's because outer fields happen to be never entered in the
> current compiler structiure. But I would argue that the test is useful
> nevertheless. If some later transformation decides to generate code
> with outer fields and let the typechecker take care of entering, it
> will do the right thing.

I have no objection to this sort of thinking, it's just terribly
difficult to infer when there is no documentation, no test where it
makes any difference, and the original commit message is "added test for
outer in Namers.scala" which is actually one of the better checkin
comments when I think of all of them.

I can't default to assuming that everything which seems insensible would
make sense if I knew what someone else was thinking, or I couldn't have
done 95% of the work I've done in the last two years.  And I can't ask
about everything: I mean I can, but I wouldn't get enough answers and
everyone would get tired of it really quickly.  (If we want to go that
route I'll start by reviving a selection from my "unanswered archives"
which will keep us busy for a while.)

> I agree. But how do you find out what code is dead or redundant?

All code should meet one of these tests:

 1) if you change it, something will change and preferably break  OR
 2) it is documented as to why 1) is not true

I agree. But we also agree that that's not what the current state of the codebase is.
 

This is a big part of the reasons tests are so important.  It isn't
necessarily that you think tests will find new bugs, it's that they
provide a critical form of documentation (often the only form) for those
who might modify the source.  I can't even imagine how much harder my
life would have been without the tests we have, but the compiler is
still only sparsely covered.

Indeed. We need more tests. It would be interesting to see what it would take to get a systematic set of test inputs that approach 100% code path coverage. I believe that would be a pretty large set of tests, and I have no good idea how to get there systematically. But if we could do it, it would be worth it. We all know that unit tests won't cut it.

But even if we had this, we would still need to worry about unknown compiler plugins exercising the compiler in new ways.

Cheers

 -- Martin






 

Any code which fails both tests is a bug, in the grander sense of being
an impediment to the reliability and improvement of the code.  I
understand that there is an awful lot of code which meets neither of
these tests, so I'm very careful.  But they're still all bugs, and they
need to be flushed out.

--
Paul Phillips      | You think you know when you learn, are more sure
Moral Alien        | when you can write, even more when you can teach,
Empiricist         | but certain when you can program.  -- Alan Perlis
all hip pupils!    |----------* http://www.improving.org/paulp/ *----------

On Sat, Oct 16, 2010 at 06:27:04PM +0200, martin odersky wrote:
> Indeed. We need more tests. It would be interesting to see what it
> would take to get a systematic set of test inputs that approach 100%
> code path coverage. I believe that would be a pretty large set of
> tests, and I have no good idea how to get there systematically. But if
> we could do it, it would be worth it. We all know that unit tests
> won't cut it.

You have heard my ideas about this so you know I think we can do pretty
well (I'd never say "100%" but I do think it's eminently testable.)

> But even if we had this, we would still need to worry about unknown
> compiler plugins exercising the compiler in new ways.

Very much so, and the way we minimize the pain both for us and the
plugin writers is to be aggressive about stating what is specified
behavior and what is not. Since very little in the way of compiler
internals are specified, we're looking at specified-by-implementation,
which doesn't end well for anyone.

A stable plugin architecture is high on my wishlist. But to get there
from here I think we have to accept the current situation for what it
is, and there is no way we won't be breaking compiler plugins which rely
on unspecified implementation details no matter what we do. The best we
can do (and what I intend for us to do) is to move us toward a properly
versioned plugin API with a specification which could plausibly be used
to write plugins without simultaneously picking through the source.
Clearly this will take time, but most things worth doing do.

Incidentally, Pavel and Alexander have built up a reasonably body of
unit tests in the IntelliJ Scala plugin covering, among other things,
binding resolution, method applicability, type conformance and type
inference. (Such is the burden of re-implementing the front-end of
scalac!)

Many of these are expressed in a way that could be reused to test
scalac itself. For example, a simple resolve test looks like:

object Test {
def foo(x: Boolean): Int = 1
def foo(x: Int): Int = 2

implicit def str2int(x: String): Int = x.length

/* line: 3 */foo("")
}

Negative tests can also be expressed:

class ProtectedThis {
class A {
protected[this] val a = 34
}

class B extends A {
val g = a
}

object B {
val b = new B
b./* accessible: false */a
}
}

Type conformance:

class A[T, U]
class B
class C extends B
val x: (A[T, U] forSome {type T <: B}) forSome {type U <: B} = new A[B, B]
//True

Type inference:

def main(args: Array[String]) {
val list = List("a", "b", "c")
/*start*/for (x <- 1 to 10) yield (x, list.headOption)/*end*/
}
//IndexedSeq[(Int, Option[String])]

So if and when someone wants to apply a more fine-grained set of tests
to scalac, this could provide a good starting point. Ideally, such a
test suite could be shared across scalac, IntelliJ, and any other
future implementations of the Scala spec.

-jason

http://git.jetbrains.org/?p=idea/scala-plugin.git;a=tree;f=testdata/reso...
http://git.jetbrains.org/?p=idea/scala-plugin.git;a=tree;f=testdata/type...
http://git.jetbrains.org/?p=idea/scala-plugin.git;a=tree;f=testdata/type...

On Sat, Oct 16, 2010 at 6:44 PM, Paul Phillips wrote:
> On Sat, Oct 16, 2010 at 06:27:04PM +0200, martin odersky wrote:
>> Indeed. We need more tests. It would be interesting to see what it
>> would take to get a systematic set of test inputs that approach 100%
>> code path coverage. I believe that would be a pretty large set of
>> tests, and I have no good idea how to get there systematically. But if
>> we could do it, it would be worth it. We all know that unit tests
>> won't cut it.
>
> You have heard my ideas about this so you know I think we can do pretty
> well (I'd never say "100%" but I do think it's eminently testable.)

I like this a lot! We should indeed try to make this available for all implementations.

Cheers

 -- Martin

On Sun, Oct 17, 2010 at 10:19 AM, Jason Zaugg <jzaugg@gmail.com> wrote:

Incidentally, Pavel and Alexander have built up a reasonably body of
unit tests in the IntelliJ Scala plugin covering, among other things,
binding resolution, method applicability, type conformance and type
inference. (Such is the burden of re-implementing the front-end of
scalac!)

Many of these are expressed in a way that could be reused to test
scalac itself. For example, a simple resolve test looks like:

 object Test {
   def foo(x: Boolean): Int = 1
   def foo(x: Int): Int = 2

   implicit def str2int(x: String): Int = x.length

   /* line: 3 */foo("")
 }

Negative tests can also be expressed:

 class ProtectedThis {
   class A {
     protected[this] val a = 34
   }

   class B extends A {
     val g = a
   }

   object B {
     val b = new B
     b./* accessible: false */a
   }
 }

Type conformance:

 class A[T, U]
 class B
 class C extends B
 val x: (A[T, U] forSome {type T <: B}) forSome {type U <: B} = new A[B, B]
 //True

Type inference:

 def main(args: Array[String]) {
   val list = List("a", "b", "c")
   /*start*/for (x <- 1 to 10) yield (x, list.headOption)/*end*/
 }
 //IndexedSeq[(Int, Option[String])]

So if and when someone wants to apply a more fine-grained set of tests
to scalac, this could provide a good starting point. Ideally, such a
test suite could be shared across scalac, IntelliJ, and any other
future implementations of the Scala spec.

-jason

http://git.jetbrains.org/?p=idea/scala-plugin.git;a=tree;f=testdata/resolve2
http://git.jetbrains.org/?p=idea/scala-plugin.git;a=tree;f=testdata/typeConformance
http://git.jetbrains.org/?p=idea/scala-plugin.git;a=tree;f=testdata/typeInference

On Sat, Oct 16, 2010 at 6:44 PM, Paul Phillips <paulp@improving.org> wrote:
> On Sat, Oct 16, 2010 at 06:27:04PM +0200, martin odersky wrote:
>> Indeed. We need more tests. It would be interesting to see what it
>> would take to get a systematic set of test inputs that approach 100%
>> code path coverage. I believe that would be a pretty large set of
>> tests, and I have no good idea how to get there systematically. But if
>> we could do it, it would be worth it. We all know that unit tests
>> won't cut it.
>
> You have heard my ideas about this so you know I think we can do pretty
> well (I'd never say "100%" but I do think it's eminently testable.)