Fix compile in Scala 2.10.1+ #1509

Merged
merged 1 commit into from Jan 16, 2014
Jump to file or symbol
Failed to load files and symbols.
+6 −6
Diff settings

Always

Just for now

@@ -360,7 +360,7 @@ trait AbstractScreen extends Factory {
case AVal(v: (T => List[FieldError])) => v
},
stuff.toList.collect {
- case AFilter(v) => v
+ case AFilter(v) => v.asInstanceOf[T => T]

This comment has been minimized.

Show comment Hide comment
@farmdawgnation

farmdawgnation Dec 20, 2013

Member

So, do we want to add an if isInstanceOf[T => T] to these case statements as well? As such, this will throw an exception if it can't fit into the type. May not be a horrible idea to log a warning or something as well...

@farmdawgnation

farmdawgnation Dec 20, 2013

Member

So, do we want to add an if isInstanceOf[T => T] to these case statements as well? As such, this will throw an exception if it can't fit into the type. May not be a horrible idea to log a warning or something as well...

This comment has been minimized.

Show comment Hide comment
@Shadowfiend

Shadowfiend Dec 20, 2013

Member

I'm down for the isInstanceOf check, though it's worth mentioning that AFilter should already be providing a compiler check for the type. I think the only real concern here is reflection or something similarly weird that would ignore the compiler and run in a type-erased context could theoretically send in something other than what's expected.

@Shadowfiend

Shadowfiend Dec 20, 2013

Member

I'm down for the isInstanceOf check, though it's worth mentioning that AFilter should already be providing a compiler check for the type. I think the only real concern here is reflection or something similarly weird that would ignore the compiler and run in a type-erased context could theoretically send in something other than what's expected.

This comment has been minimized.

Show comment Hide comment
@pbrant

pbrant Dec 20, 2013

Member

The compiler verification on AFilter construction isn't sufficient. 2.10.0
and earlier didn't complain and happily assumed T => T. 2.10.1+ does
complain. See https://issues.scala-lang.org/browse/SI-5189 . The cast is
basically restoring the pre-2.10.1 behavior (which is wrong, but mostly
good enough).

On Fri, Dec 20, 2013 at 3:17 PM, Antonio Salazar Cardozo <
notifications@github.com> wrote:

In web/webkit/src/main/scala/net/liftweb/http/LiftScreen.scala:

@@ -360,7 +360,7 @@ trait AbstractScreen extends Factory {
case AVal(v: (T => List[FieldError])) => v
},
stuff.toList.collect {

  •    case AFilter(v) => v
    
  •    case AFilter(v) => v.asInstanceOf[T => T]
    

I'm down for the isInstanceOf check, though it's worth mentioning that
AFilter should already be providing a compiler check for the type. I
think the only real concern here is reflection or something similarly weird
that would ignore the compiler and run in a type-erased context could
theoretically send in something other than what's expected.


Reply to this email directly or view it on GitHubhttps://github.com/lift/framework/pull/1509/files#r8500039
.

@pbrant

pbrant Dec 20, 2013

Member

The compiler verification on AFilter construction isn't sufficient. 2.10.0
and earlier didn't complain and happily assumed T => T. 2.10.1+ does
complain. See https://issues.scala-lang.org/browse/SI-5189 . The cast is
basically restoring the pre-2.10.1 behavior (which is wrong, but mostly
good enough).

On Fri, Dec 20, 2013 at 3:17 PM, Antonio Salazar Cardozo <
notifications@github.com> wrote:

In web/webkit/src/main/scala/net/liftweb/http/LiftScreen.scala:

@@ -360,7 +360,7 @@ trait AbstractScreen extends Factory {
case AVal(v: (T => List[FieldError])) => v
},
stuff.toList.collect {

  •    case AFilter(v) => v
    
  •    case AFilter(v) => v.asInstanceOf[T => T]
    

I'm down for the isInstanceOf check, though it's worth mentioning that
AFilter should already be providing a compiler check for the type. I
think the only real concern here is reflection or something similarly weird
that would ignore the compiler and run in a type-erased context could
theoretically send in something other than what's expected.


Reply to this email directly or view it on GitHubhttps://github.com/lift/framework/pull/1509/files#r8500039
.

This comment has been minimized.

Show comment Hide comment
@Shadowfiend

Shadowfiend Dec 20, 2013

Member

I realize that. I'm not saying asInstanceOf isn't necessary, I'm saying isInstanceOf might be too much, depending on whether we view this as dealing with something we know but the compiler doesn't, or whether we view this as dealing with the potential for someone to get around the compiler to give us bad data :)

@Shadowfiend

Shadowfiend Dec 20, 2013

Member

I realize that. I'm not saying asInstanceOf isn't necessary, I'm saying isInstanceOf might be too much, depending on whether we view this as dealing with something we know but the compiler doesn't, or whether we view this as dealing with the potential for someone to get around the compiler to give us bad data :)

This comment has been minimized.

Show comment Hide comment
@fmpwizard

fmpwizard Dec 22, 2013

Member

I'll let you guys decide on this one, once you all agree, just let me know and I'll press the green merge button (or any of you could do it too :) )

@fmpwizard

fmpwizard Dec 22, 2013

Member

I'll let you guys decide on this one, once you all agree, just let me know and I'll press the green merge button (or any of you could do it too :) )

},
stuff)
}
@@ -484,7 +484,7 @@ trait AbstractScreen extends Factory {
}.toList
override def setFilter = stuff.collect {
- case AFilter(f) => f
+ case AFilter(f) => f.asInstanceOf[ValueType => ValueType]
}.toList
override def is = underlying.get
@@ -586,7 +586,7 @@ trait AbstractScreen extends Factory {
}.toList
override def setFilter = stuff.collect {
- case AFilter(f) => f
+ case AFilter(f) => f.asInstanceOf[ValueType => ValueType]
}.toList
override def is = underlying.openOrThrowException("Legacy code").get
@@ -617,7 +617,7 @@ trait AbstractScreen extends Factory {
case AVal(v: (T => List[FieldError])) => List(v)
case _ => Nil
}, stuff.toList.flatMap {
- case AFilter(v) => List(v)
+ case AFilter(v) => List(v.asInstanceOf[T => T])
case _ => Nil
}, stuff).make
@@ -755,7 +755,7 @@ trait AbstractScreen extends Factory {
override lazy val formElemAttrs: Seq[SHtml.ElemAttr] = grabParams(stuff)
override val setFilter = stuff.flatMap {
- case AFilter(f) => List(f)
+ case AFilter(f) => List(f.asInstanceOf[ValueType => ValueType])
case _ => Nil
}.toList
override val validations = stuff.flatMap {
@@ -792,7 +792,7 @@ trait AbstractScreen extends Factory {
override lazy val formElemAttrs: Seq[SHtml.ElemAttr] = grabParams(stuff)
override val setFilter = stuff.flatMap {
- case AFilter(f) => List(f)
+ case AFilter(f) => List(f.asInstanceOf[ValueType => ValueType])
case _ => Nil
}.toList
override val validations = stuff.flatMap {