worse or better? `tp.typeSymbol==XXX'

 @MAT means matthias zenger

I'll reply in more detail to your actual questions as soon as the current paper deadline has passed, but @M stands for Adriaan Moors  (it also means "At Martin" sometimes, esp. when the line ends in a '?')
AT means Alias Types (after adding tcpoly, i refactored type aliases to be expanded lazily instead of eagerly) 
@M

On Wed, Oct 21, 2009 at 07:00:00PM +0200, Adriaan Moors wrote:
> I'll reply in more detail to your actual questions as soon as the
> current paper deadline has passed, but @M stands for Adriaan Moors (it
> also means "At Martin" sometimes, esp. when the line ends in a '?')

That's great news all by itself! Anticipated quality of response quite a
bit higher when responder and commenter are one. Now to write some
comment expansion filters for autodecoding @M[AT].*[\?]$

So if it's OK with everyone I think I would like to make it a regular
endeavor to shine a light on a comment so we can clarify it, delete it,
or enshrine it as received wisdom.

Great idea! Bit rot is so much worse in comments because if you can't trust those, what's left...  

Here is one such, from Types.scala.

/* @MAT
whenever you see `tp.typeSymbol.isXXXX' and then act on tp based on that predicate, you're on thin ice,
as `typeSymbol' (and `prefix') automatically normalize, but the other inspectors don't.
In other words, even if `tp.normalize.sym.isXXX' is true, `tp.sym.isXXX' may be false (if sym were a public method to access the non-normalized typeSymbol)...

In retrospect, I think `tp.typeSymbol.isXXX' or (worse) `tp.typeSymbol==XXX' should be replaced by `val tp = tp0.asXXX'.
A type's typeSymbol should never be inspected directly.
*/

I may have been exaggerating a little, I forgot the concrete bugs that I ran into (should have put in comments about those too of course).
The discrepancy arises when `tp` is a reference to an alias type, of course. Now, tp.typeSymbol is probably different from tp.typeSymbolDirect, and there was some code that checked some property of tp.typeSymbol (such as number of type params, I think) and concluded stuff about tp, even though typeSymbol is the symbol of the type on the right hand side of the type alias (which may not take type params).
example: type GenericString[T] = String
currently, normalize on a type alias does eta-expansion, so that TypeRef(_, `GenericString`, List()) = PolyType(`T`, `String`), which ensures the number of type parameters is stable under normalisation
sorry I couldn't do better to elucidate this comment -- if my 'explanation' raised other questions, was inaccurate, ... let me know
cheersadriaan

On Fri, Oct 23, 2009 at 06:59:00PM +0200, Adriaan Moors wrote:
> sorry I couldn't do better to elucidate this comment -- if my
> 'explanation' raised other questions, was inaccurate, ... let me know

Thank you for the answer. I have to do some more study before I can ask
a decent followup question. In the meantime I have a different one...

For quite a while I've had a method sitting around as my sort of test
case for the seems-like-it-should-work task of accepting an arbitrary
traversable as a parameter and then returning a permutation of it in the
same kind of container. When the new collections came out, and
periodically thereafter, I tried everything imaginable to make this work
and could not do it in such a way that type parameters didn't have to be
supplied. I could only make it work as desired "from the inside", by
defining the method on Traversable.

From the outisde, shuffle[Int,List](List(1,2,3)) would work, but
shuffle(List(1,2,3)) would infer Nothing and yell at me. I've come back
to it periodically since then with no results, until unexpectedly
yesterday I got it to work, with a much simpler signature than I'd have
imagined.

def shuffle[T, CC[_] <: Traversable[_]](seq: CC[T])
(implicit bf: CanBuildFrom[CC[T], T, CC[T]]): CC[T] = { ...

So I assume this must be the result of your work. Great, thanks. There
is a wrinkle I don't understand, is it a bug or programmer failure? Here
are examples of it working:

scala> util.Random.shuffle(IndexedSeq(1,2,3))
res0: IndexedSeq[Int] = NewIndexedSeqImpl(3, 1, 2)

scala> util.Random.shuffle(Iterable(1,2,3))
res1: Iterable[Int] = List(3, 1, 2)

Great. Here are examples of me trying to break it by lying in the type
parameters:

scala> util.Random.shuffle[List[String],List](List(List(1)))
:5: error: type mismatch;
found : Int(1)
required: String

scala> util.Random.shuffle[Set[Int],IndexedSeq](IndexedSeq(Set(1L)))
:5: error: type mismatch;
found : Long(1L)
required: Int

So it seems pretty clear it knows exactly what the CC[T] is. So why is
this cast necessary?

val builder = bf(seq)
val buf = new ArrayBuffer[T]
seq foreach (x => buf += x.asInstanceOf[T])

Without the cast, it thinks the elements of Seq are Anys, even though on
some level it must know otherwise.

[scalacfork] found : Any
[scalacfork] required: T
[scalacfork] seq foreach (x => buf += x)
[scalacfork] ^

I tried throwing some Manifests at it (that was what brought me back to
it in the first place) to no effect, not that I thought it should help.

Hi Paul,

On Wed, Oct 28, 2009 at 1:54 PM, Paul Phillips <paulp@improving.org> wrote:

 def shuffle[T, CC[_] <: Traversable[_]](seq: CC[T])
   (implicit bf: CanBuildFrom[CC[T], T, CC[T]]): CC[T] = { ...

So I assume this must be the result of your work.  Great, thanks.  

It is and you're welcome. Thanks for putting it to good use and through its paces. 

There is a wrinkle I don't understand, is it a bug or programmer failure?

 the latter, though it's a common one (I never know whether that's supposed to be a consolation, but there you go)

Here are examples of it working:
So it seems pretty clear it knows exactly what the CC[T] is.  So why is
this cast necessary?

   val builder = bf(seq)
   val buf = new ArrayBuffer[T]
   seq foreach (x => buf += x.asInstanceOf[T])

This cast is necessary because you didn't give the type checker all the info it needs:
   the bound CC[_] <: Traversable[_] does not mean what I think you think it means
it says `CC is a type constructor with one argument, so that for any type argument X, CC[X] should be a subtype of Traversable[Y forSome {type Y}]` note how the former underscore stands for some unnamed higher-order type parameter, whereas the latter sneakily expands to an existential type. I wish I had known Martin was going to use the `_` for existentials (their introductions are at least one year apart) -- I would never have introduced it as a wildcard name for a higher-order type parameter (as they mean completely different things). 
It would be swell if someone could implement a warning that alerts the unsuspecting programmer. I wish I could deprecate _ for higher-order type parameters -- it's not worth it, IMO.
Anyway, all is well if you write `CC[x] <: Traversable[x]`, as this means `the CC type parameter stands for a type constructor with one argument that satisfies this condition: for all valid type arguments Y,CC[Y] is a subtype of Traversable[Y]`
cheers,adriaan

On Wed, Oct 28, 2009 at 02:55:14PM +0100, Adriaan Moors wrote:
> the latter, though it's a common one (I never know whether that's
> supposed to be a consolation, but there you go)

I can take consolation in anything.

> note how the former underscore stands for some unnamed higher-order
> type parameter, whereas the latter sneakily expands to an existential
> type.

Interesting, I knew this was exactly the example where underscore usages
overlapped dangerously, but clearly I was mentally parroting the sounds
without properly appreciating the danger.

Thanks! That's the best answer imaginable, easy to fix, easy to put to
work, and without any overly ominous implications.

As an innocent bystander, let me take the opportunity to wholly endorse the warning at least, or even the deprecation!

On Wed, Oct 28, 2009 at 12:19 PM, Paul Phillips <paulp@improving.org> wrote:

On Wed, Oct 28, 2009 at 02:55:14PM +0100, Adriaan Moors wrote:
>  the latter, though it's a common one (I never know whether that's
> supposed to be a consolation, but there you go)

I can take consolation in anything.

> note how the former underscore stands for some unnamed higher-order
> type parameter, whereas the latter sneakily expands to an existential
> type.

Interesting, I knew this was exactly the example where underscore usages
overlapped dangerously, but clearly I was mentally parroting the sounds
without properly appreciating the danger.

Thanks! That's the best answer imaginable, easy to fix, easy to put to
work, and without any overly ominous implications.

--
Paul Phillips      | Atheists dig the best foxholes.
In Theory          |
Empiricist         |
i pull his palp!   |----------* http://www.improving.org/paulp/ *----------

--
Daniel C. Sobral

Something I learned in academia: there are three kinds of academic reviews: review by name, review by reference and review by value.

Having been bitten by this a few times, I'd appreciate a deprecation.

--j

On Wed, Oct 28, 2009 at 10:20 AM, Daniel Sobral <dcsobral@gmail.com> wrote:

As an innocent bystander, let me take the opportunity to wholly endorse the warning at least, or even the deprecation!

On Wed, Oct 28, 2009 at 12:19 PM, Paul Phillips <paulp@improving.org> wrote:

On Wed, Oct 28, 2009 at 02:55:14PM +0100, Adriaan Moors wrote:
>  the latter, though it's a common one (I never know whether that's
> supposed to be a consolation, but there you go)

I can take consolation in anything.

> note how the former underscore stands for some unnamed higher-order
> type parameter, whereas the latter sneakily expands to an existential
> type.

Interesting, I knew this was exactly the example where underscore usages
overlapped dangerously, but clearly I was mentally parroting the sounds
without properly appreciating the danger.

Thanks! That's the best answer imaginable, easy to fix, easy to put to
work, and without any overly ominous implications.

--
Paul Phillips      | Atheists dig the best foxholes.
In Theory          |
Empiricist         |
i pull his palp!   |----------* http://www.improving.org/paulp/ *----------

--
Daniel C. Sobral

Something I learned in academia: there are three kinds of academic reviews: review by name, review by reference and review by value.

Martin just fixed a bug that resulted from the discrepancy between looking at aliased types through normalizing glasses vs. with normalization-resistant ones: see http://lampsvn.epfl.ch/trac/scala/changeset/19818/scala/trunk

-      def mot(tp0: Type): Tree = tp0.normalize match {
-        case ThisType(_) | SingleType(_, _) =>
-          manifestFactoryCall("singleType", tp, gen.mkAttributedQualifier(tp0)) 
-        case ConstantType(value) =>
-          manifestOfType(tp0.deconst, full)
-        case TypeRef(pre, sym, args) =>
-          if (isValueClass(sym) || isPhantomClass(sym)) {
-            typed { atPos(tree.pos.focus) {
-              Select(gen.mkAttributedRef(FullManifestModule), sym.name.toString)
-            }}
-          } else if (sym == ArrayClass && args.length == 1) {
-            manifestFactoryCall("arrayType", args.head, findSubManifest(args.head))
-           } else if (sym.isClass) {
-            val suffix = gen.mkClassOf(tp0) :: (args map findSubManifest)
-            manifestFactoryCall(
-              "classType", tp, 
-              (if ((pre eq NoPrefix) || pre.typeSymbol.isStaticOwner) suffix
-               else findSubManifest(pre) :: suffix): _*)
-          } else if (sym.isAbstractType) {
-            if (sym.isExistential) 
-              EmptyTree // todo: change to existential parameter manifest
-            else if (sym.isTypeParameterOrSkolem)

...

-            else
-              manifestFactoryCall(
-                "abstractType", tp,
-                findSubManifest(pre) :: Literal(sym.name.toString) :: findManifest(tp0.bounds.hi) :: (args map findSubManifest): _*)

On Fri, Oct 23, 2009 at 5:59 PM, Adriaan Moors <adriaan.moors@cs.kuleuven.be> wrote:

So if it's OK with everyone I think I would like to make it a regular
endeavor to shine a light on a comment so we can clarify it, delete it,
or enshrine it as received wisdom.

Great idea! Bit rot is so much worse in comments because if you can't trust those, what's left...  

Here is one such, from Types.scala.

/* @MAT
whenever you see `tp.typeSymbol.isXXXX' and then act on tp based on that predicate, you're on thin ice,
as `typeSymbol' (and `prefix') automatically normalize, but the other inspectors don't.
In other words, even if `tp.normalize.sym.isXXX' is true, `tp.sym.isXXX' may be false (if sym were a public method to access the non-normalized typeSymbol)...

In retrospect, I think `tp.typeSymbol.isXXX' or (worse) `tp.typeSymbol==XXX' should be replaced by `val tp = tp0.asXXX'.
A type's typeSymbol should never be inspected directly.
*/

I may have been exaggerating a little, I forgot the concrete bugs that I ran into (should have put in comments about those too of course).
The discrepancy arises when `tp` is a reference to an alias type, of course. Now, tp.typeSymbol is probably different from tp.typeSymbolDirect, and there was some code that checked some property of tp.typeSymbol (such as number of type params, I think) and concluded stuff about tp, even though typeSymbol is the symbol of the type on the right hand side of the type alias (which may not take type params).
example: type GenericString[T] = String
currently, normalize on a type alias does eta-expansion, so that TypeRef(_, `GenericString`, List()) = PolyType(`T`, `String`), which ensures the number of type parameters is stable under normalisation
sorry I couldn't do better to elucidate this comment -- if my 'explanation' raised other questions, was inaccurate, ... let me know
cheersadriaan