This page is no longer maintained — Please continue to the home page at www.scala-lang.org

reflective calls in trunk

8 replies
extempore
Joined: 2008-12-17,
User offline. Last seen 35 weeks 3 days ago.
We should consider what to do about the abstract types in scala.reflect in this light as well as the pattern matching light.

quick.lib:    [mkdir] Created dir: /scratch/trunk4/build/quick/classes/library     [javac] Compiling 25 source files to /scratch/trunk4/build/quick/classes/library    [javac] Compiling 41 source files to /scratch/trunk4/build/quick/classes/library    [javac] Note: Some input files use unchecked or unsafe operations.     [javac] Note: Recompile with -Xlint:unchecked for details.[scalacfork] Compiling 686 files to /scratch/trunk4/build/quick/classes/library[scalacfork] /scratch/trunk4/src/library/scala/collection/parallel/mutable/ParHashMap.scala:200: warning: method invocation uses reflection [scalacfork]         for (elem <- buckets(i)) table.insertEntry(elem)[scalacfork]                                                   ^[scalacfork] warning: there were 1 deprecation warnings; re-run with -deprecation for details [scalacfork] warning: there were 1 unchecked warnings; re-run with -unchecked for details[scalacfork] three warnings found[scalacfork] Compiling 46 files to /scratch/trunk4/build/quick/classes/library [scalacfork] Compiling 66 files to /scratch/trunk4/build/quick/classes/library[scalacfork] Compiling 103 files to /scratch/trunk4/build/quick/classes/library[scalacfork] /scratch/trunk4/src/swing/scala/swing/Action.scala:38: warning: method invocation uses reflection [scalacfork]       def action_=(a: Action) { _action = a; peer.setAction(a.peer) }[scalacfork]                                                            ^[scalacfork] /scratch/trunk4/src/swing/scala/swing/Orientable.scala:15: warning: method invocation uses reflection [scalacfork]     def orientation_=(o: Orientation.Value) { peer.setOrientation(o.id) }[scalacfork]                                                                  ^[scalacfork] /scratch/trunk4/src/swing/scala/swing/Oriented.scala:25: warning: method invocation uses reflection [scalacfork]     def orientation: Orientation.Value = Orientation(peer.getOrientation)[scalacfork]                                                           ^[scalacfork] three warnings found [propertyfile] Creating new property file: /scratch/trunk4/build/quick/classes/library/library.properties[stopwatch] [quick.lib.timer: 2:21.626 sec]
quick.newlibs:
quick.libs:
quick.newforkjoin:
quick.forkjoin:
quick.pre-comp:
quick.comp:    [mkdir] Created dir: /scratch/trunk4/build/quick/classes/compiler [scalacfork] Compiling 453 files to /scratch/trunk4/build/quick/classes/compiler[scalacfork] /scratch/trunk4/src/compiler/scala/tools/nsc/typechecker/PatMatVirtualiser.scala:1382: warning: match is not exhaustive! [scalacfork] missing combination            Nil             *[scalacfork] [scalacfork]       override def runOrElse(scrut: Tree, matcher: Tree, scrutTp: Type, resTp: Type, hasDefault: Boolean) = matcher match { [scalacfork]                                                                                                             ^[scalacfork] /scratch/trunk4/src/compiler/scala/reflect/internal/SymbolTable.scala:217: warning: method invocation uses reflection [scalacfork]           else " has " + cache.size + " entries:\n" + stringOf(cache)[scalacfork]                                ^[scalacfork] /scratch/trunk4/src/compiler/scala/reflect/internal/SymbolTable.scala:232: warning: method invocation uses reflection [scalacfork]         val size = caches flatMap (ref => Option(ref.get)) map (_.size) sum;[scalacfork]                                                                   ^[scalacfork] /scratch/trunk4/src/compiler/scala/reflect/internal/SymbolTable.scala:240: warning: method invocation uses reflection [scalacfork]           cache.clear()[scalacfork]                      ^[scalacfork] /scratch/trunk4/src/compiler/scala/reflect/internal/Symbols.scala:1505: warning: method invocation uses reflection [scalacfork]           if (this.sourceFile.path != that.sourceFile.path) {[scalacfork]                               ^[scalacfork] /scratch/trunk4/src/compiler/scala/reflect/internal/Symbols.scala:1505: warning: method invocation uses reflection [scalacfork]           if (this.sourceFile.path != that.sourceFile.path) {[scalacfork]                                                       ^[scalacfork] /scratch/trunk4/src/compiler/scala/reflect/internal/Symbols.scala:1508: warning: method invocation uses reflection [scalacfork]             if (this.sourceFile.canonicalPath != that.sourceFile.canonicalPath)[scalacfork]                                 ^[scalacfork] /scratch/trunk4/src/compiler/scala/reflect/internal/Symbols.scala:1508: warning: method invocation uses reflection [scalacfork]             if (this.sourceFile.canonicalPath != that.sourceFile.canonicalPath)[scalacfork]                                                                  ^[scalacfork] /scratch/trunk4/src/compiler/scala/reflect/internal/Symbols.scala:2543: warning: method invocation uses reflection [scalacfork]     "  Found in " + sym1.sourceFile.canonicalPath + " and " + sym2.sourceFile.canonicalPath[scalacfork]                                     ^[scalacfork] /scratch/trunk4/src/compiler/scala/reflect/internal/Symbols.scala:2543: warning: method invocation uses reflection [scalacfork]     "  Found in " + sym1.sourceFile.canonicalPath + " and " + sym2.sourceFile.canonicalPath[scalacfork]                                                                               ^ [scalacfork] /scratch/trunk4/src/compiler/scala/reflect/internal/Types.scala:2435: warning: method invocation uses reflection[scalacfork]     for (tp <- tpe ; if tp.typeSymbol.isExistentiallyBound) yield [scalacfork]             ^[scalacfork] /scratch/trunk4/src/compiler/scala/tools/nsc/doc/model/IndexModelFactory.scala:46: warning: method invocation uses reflection[scalacfork]               result.addMember(tpl) [scalacfork]                               ^[scalacfork] /scratch/trunk4/src/compiler/scala/tools/nsc/doc/model/IndexModelFactory.scala:49: warning: method invocation uses reflection[scalacfork]               result.addMember(alias) [scalacfork]                               ^[scalacfork] /scratch/trunk4/src/compiler/scala/tools/nsc/doc/model/IndexModelFactory.scala:51: warning: method invocation uses reflection[scalacfork]               result.addMember(absType) [scalacfork]                               ^[scalacfork] /scratch/trunk4/src/compiler/scala/tools/nsc/doc/model/IndexModelFactory.scala:53: warning: method invocation uses reflection[scalacfork]               result.addMember(non) [scalacfork]                               ^[scalacfork] /scratch/trunk4/src/compiler/scala/tools/nsc/io/File.scala:35: warning: method invocation uses reflection[scalacfork]     try target.close() catch { case e: IOException => } [scalacfork]                     ^[scalacfork] /scratch/trunk4/src/compiler/scala/tools/reflect/SigParser.scala:27: warning: method invocation uses reflection[scalacfork]   def verifyClass(s: String)  = isParserAvailable && wrap(make() parseClassSig s) [scalacfork]                                                                  ^[scalacfork] /scratch/trunk4/src/compiler/scala/tools/reflect/SigParser.scala:28: warning: method invocation uses reflection [scalacfork]   def verifyMethod(s: String) = isParserAvailable && wrap(make() parseMethodSig s)[scalacfork]                                                                  ^[scalacfork] /scratch/trunk4/src/compiler/scala/tools/reflect/SigParser.scala:29: warning: method invocation uses reflection [scalacfork]   def verifyType(s: String)   = isParserAvailable && wrap(make() parseTypeSig s)[scalacfork]                                                                  ^[scalacfork] /scratch/trunk4/src/compiler/scala/tools/util/Javap.scala:128: warning: method invocation uses reflection [scalacfork]     def show() = value.asInstanceOf[{ def print(): Unit }].print()[scalacfork]                                                                 ^[scalacfork] warning: there were 10 deprecation warnings; re-run with -deprecation for details [scalacfork] 21 warnings found[propertyfile] Creating new property file: /scratch/trunk4/build/quick/classes/compiler/compiler.properties     [copy] Copying 57 files to /scratch/trunk4/build/quick/classes/compiler [stopwatch] [quick.comp.timer: 1:57.556 sec]
quick.pre-plugins:
quick.plugins:    [mkdir] Created dir: /scratch/trunk4/build/quick/classes/continuations-plugin [scalacfork] Compiling 5 files to /scratch/trunk4/build/quick/classes/continuations-plugin     [copy] Copying 1 file to /scratch/trunk4/build/quick/classes/continuations-plugin    [mkdir] Created dir: /scratch/trunk4/build/quick/misc/scala-devel/plugins       [jar] Building jar: /scratch/trunk4/build/quick/misc/scala-devel/plugins/continuations.jar[scalacfork] Compiling 2 files to /scratch/trunk4/build/quick/classes/library[stopwatch] [quick.plugins.timer: 18.605 sec]
quick.scalacheck:    [mkdir] Created dir: /scratch/trunk4/build/quick/classes/scalacheck[scalacfork] Compiling 14 files to /scratch/trunk4/build/quick/classes/scalacheck [scalacfork] /scratch/trunk4/src/scalacheck/org/scalacheck/Pretty.scala:45: warning: method invocation uses reflection[scalacfork]     else s.substring(0, length) / break(lead+s.substring(length), lead, length) [scalacfork]                                 ^[scalacfork] /scratch/trunk4/src/scalacheck/org/scalacheck/Pretty.scala:67: warning: method invocation uses reflection[scalacfork]     e.getClass.getName + ": " + e.getMessage / strs2.mkString("\n") [scalacfork]                                              ^[scalacfork] /scratch/trunk4/src/scalacheck/org/scalacheck/Pretty.scala:86: warning: method invocation uses reflection[scalacfork]       "> Collected test data: " / { [scalacfork]                                 ^[scalacfork] /scratch/trunk4/src/scalacheck/org/scalacheck/Pretty.scala:101: warning: method invocation uses reflection[scalacfork]       case Test.Proved(args) => "OK, proved property."/pretty(args,prms) [scalacfork]                                                       ^[scalacfork] /scratch/trunk4/src/scalacheck/org/scalacheck/Pretty.scala:104: warning: method invocation uses reflection[scalacfork]         "Falsified after "+res.succeeded+" passed tests."/labels(l)/pretty(args,prms) [scalacfork]                                                                    ^[scalacfork] /scratch/trunk4/src/scalacheck/org/scalacheck/Pretty.scala:104: warning: method invocation uses reflection [scalacfork]         "Falsified after "+res.succeeded+" passed tests."/labels(l)/pretty(args,prms)[scalacfork]                                                          ^[scalacfork] /scratch/trunk4/src/scalacheck/org/scalacheck/Pretty.scala:109: warning: method invocation uses reflection [scalacfork]         "Exception raised on property evaluation."/labels(l)/pretty(args,prms)/[scalacfork]                                                                               ^ [scalacfork] /scratch/trunk4/src/scalacheck/org/scalacheck/Pretty.scala:109: warning: method invocation uses reflection[scalacfork]         "Exception raised on property evaluation."/labels(l)/pretty(args,prms)/ [scalacfork]                                                             ^[scalacfork] /scratch/trunk4/src/scalacheck/org/scalacheck/Pretty.scala:109: warning: method invocation uses reflection[scalacfork]         "Exception raised on property evaluation."/labels(l)/pretty(args,prms)/ [scalacfork]                                                   ^[scalacfork] /scratch/trunk4/src/scalacheck/org/scalacheck/Pretty.scala:112: warning: method invocation uses reflection[scalacfork]         "Exception raised on argument generation."/ [scalacfork]                                                   ^[scalacfork] /scratch/trunk4/src/scalacheck/org/scalacheck/Pretty.scala:116: warning: method invocation uses reflection[scalacfork]     s/t/pretty(res.freqMap,prms) [scalacfork]        ^[scalacfork] /scratch/trunk4/src/scalacheck/org/scalacheck/Pretty.scala:116: warning: method invocation uses reflection[scalacfork]     s/t/pretty(res.freqMap,prms) [scalacfork]      ^[scalacfork] /scratch/trunk4/src/scalacheck/org/scalacheck/Pretty.scala:99: warning: method invocation uses reflection[scalacfork]       else "> Labels of failing property: " / ls.mkString("\n") [scalacfork]                                             ^[scalacfork] /scratch/trunk4/src/scalacheck/org/scalacheck/Prop.scala:48: warning: method invocation uses reflection[scalacfork]     Test.cmdLineParser.parseParams(args) match { [scalacfork]                                   ^[scalacfork] /scratch/trunk4/src/scalacheck/org/scalacheck/Properties.scala:60: warning: method invocation uses reflection[scalacfork]     Test.cmdLineParser.parseParams(args) match { [scalacfork]                                   ^[scalacfork] 15 warnings found
quick.pre-scalap:
quick.scalap:    [mkdir] Created dir: /scratch/trunk4/build/quick/classes/scalap [scalacfork] Compiling 28 files to /scratch/trunk4/build/quick/classes/scalap[scalacfork] /scratch/trunk4/src/scalap/scala/tools/scalap/scalax/rules/Memoisable.scala:22: warning: method invocation uses reflection [scalacfork]     from[In] { in => in.memo(key, rule(in)) }[scalacfork]              ^[scalacfork] /scratch/trunk4/src/scalap/scala/tools/scalap/scalax/rules/SeqRule.scala:54: warning: method invocation uses reflection [scalacfork]   def * = from[S] {[scalacfork]                   ^[scalacfork] /scratch/trunk4/src/scalap/scala/tools/scalap/scalax/rules/SeqRule.scala:84: warning: method invocation uses reflection [scalacfork]   def times(num : Int) : Rule[S, S, Seq[A], X] = from[S] {[scalacfork]                                                          ^[scalacfork] /scratch/trunk4/src/scalap/scala/tools/scalap/scalax/rules/scalasig/ScalaSigPrinter.scala:190: warning: method invocation uses reflection [scalacfork]   def genParamNames(t: {def paramTypes: Seq[Type]}): List[String] = t.paramTypes.toList.map(x => {[scalacfork]                                                                       ^ [scalacfork] /scratch/trunk4/src/scalap/scala/tools/scalap/scalax/rules/scalasig/ScalaSigPrinter.scala:204: warning: method invocation uses reflection[scalacfork]       val paramEntries = mt.paramSymbols.map({ [scalacfork]                             ^[scalacfork] /scratch/trunk4/src/scalap/scala/tools/scalap/scalax/rules/scalasig/ScalaSigPrinter.scala:208: warning: method invocation uses reflection[scalacfork]       val implicitWord = mt.paramSymbols.headOption match { [scalacfork]                             ^[scalacfork] /scratch/trunk4/src/scalap/scala/tools/scalap/scalax/rules/scalasig/ScalaSigPrinter.scala:217: warning: method invocation uses reflection[scalacfork]       mt.resultType match { [scalacfork]          ^[scalacfork] 7 warnings found[stopwatch] [quick.scalap.timer: 23.657 sec]
quick.pre-partest:
quick.partest:    [mkdir] Created dir: /scratch/trunk4/build/quick/classes/partest     [javac] Compiling 2 source files to /scratch/trunk4/build/quick/classes/partest[scalacfork] Compiling 26 files to /scratch/trunk4/build/quick/classes/partest[scalacfork] warning: there were 3 deprecation warnings; re-run with -deprecation for details [scalacfork] one warning found[propertyfile] Creating new property file: /scratch/trunk4/build/quick/classes/partest/partest.properties     [copy] Copying 1 file to /scratch/trunk4/build/quick/classes/partest [stopwatch] [quick.partest.timer: 20.817 sec]
quick.pre-bin:
quick.bin:    [mkdir] Created dir: /scratch/trunk4/build/quick/bin
quick.done:
odersky
Joined: 2008-07-29,
User offline. Last seen 45 weeks 6 days ago.
Re: reflective calls in trunk

On Tue, Dec 27, 2011 at 8:26 PM, Paul Phillips wrote:
> We should consider what to do about the abstract types in scala.reflect in
> this light as well as the pattern matching light.
>
Can you qickly explain what the problem is?

Thanks

adriaanm
Joined: 2010-02-08,
User offline. Last seen 31 weeks 4 days ago.
Re: reflective calls in trunk
for the reflective calls on targets with abstract types, I assume we're talking performance
for the pattern matches, I guess we're talking old patmat bugs, although I fixed one of those by generating smarter bridges to unapplies (we'll still keep running into the bugs related to mixing user-defined extractors and synthetic case class extractors: https://issues.scala-lang.org/browse/SI-1697 and https://issues.scala-lang.org/browse/SI-2337 -- of course these are fixed by virtpatmat)
cheersadriaan

On Wed, Dec 28, 2011 at 1:40 PM, martin odersky <martin.odersky@epfl.ch> wrote:
On Tue, Dec 27, 2011 at 8:26 PM, Paul Phillips <paulp@improving.org> wrote:
> We should consider what to do about the abstract types in scala.reflect in
> this light as well as the pattern matching light.
>
Can you qickly explain what the problem is?

Thanks

extempore
Joined: 2008-12-17,
User offline. Last seen 35 weeks 3 days ago.
Re: reflective calls in trunk


On Wed, Dec 28, 2011 at 4:40 AM, martin odersky <martin.odersky@epfl.ch> wrote:
Can you qickly explain what the problem is?

It is the usual thing that we should avoid reflection whenever possible.  It is only secondarily a matter of performance and primarily a matter of correctness, or the lack thereof.  There is just all kinds of stuff that goes slow, wrong, or unexpectedly when reflection is involved.  Environments where things don't work, behavior changes, optimizations can't be performed, programmer mental model gets harder.  Reflection is fine when one needs it, but in this case it is gratuitous because all of it is under our control.  I understand the appeal of defining types abstractly at the API level and having them defined concretely internally, but when this leads to a bunch of reflective calls  being generated, the price is too high.
This ticket increases in relevance - I ran into this long ago.
    https://issues.scala-lang.org/browse/SI-3394
    "concrete classes implementing structural types are pessimized"
    In Concrete2 we override the method with a syntactically identical version of the inherited method. The impact is profound.
    In Concrete1: a call to foobar forwards the target object and argument to Trait.foobar, where the integer is boxed and placed in an array, the method handle is looked up, the reflective call is issued, the result is unboxed, and it is returned to the calling class.
    In Concrete2: a call to foobar results in a direct unboxed call to foo.bar.
    But surely if manually overriding each method with an exact copy achieves this, the compiler could assume the responsibility.
I feel strongly we should either pursue that approach or get rid of this:

  type AbstractFileType >: Null <: {    def path: String    def canonicalPath: String   }
But that we should have no reflective calls in the standard library unless it's something which can be done no other way.
odersky
Joined: 2008-07-29,
User offline. Last seen 45 weeks 6 days ago.
Re: reflective calls in trunk

On Fri, Dec 30, 2011 at 8:55 PM, Paul Phillips wrote:
>
>
> On Wed, Dec 28, 2011 at 4:40 AM, martin odersky
> wrote:
>>
>> Can you qickly explain what the problem is?
>
>
> It is the usual thing that we should avoid reflection whenever possible.  It
> is only secondarily a matter of performance and primarily a matter of
> correctness, or the lack thereof.  There is just all kinds of stuff that
> goes slow, wrong, or unexpectedly when reflection is involved.

Of course. Sorry, I should have been clearer in my question. I meant:
How do abstract types in reflect.api cause structural types and
therefore reflection? Is that some detail that can be easily corrected
or something fundamental?

Cheers

extempore
Joined: 2008-12-17,
User offline. Last seen 35 weeks 3 days ago.
Re: reflective calls in trunk


On Fri, Dec 30, 2011 at 12:35 PM, martin odersky <martin.odersky@epfl.ch> wrote:
Of course. Sorry, I should have been clearer in my question. I meant:
How do abstract types in reflect.api cause structural types and
therefore reflection? Is that some detail that can be easily corrected
or something fundamental?

The abstract types are OK, but not abstract members with no backing interface.
  type AbstractFileType >: Null <: {     def path: String    def canonicalPath: String  }
It is nothing fundamental.  Something as simple as this would send it away.
  trait MinimalFile {
    def path: String    def canonicalPath: String
  }  type AbstractFileType <: MinimalFile
odersky
Joined: 2008-07-29,
User offline. Last seen 45 weeks 6 days ago.
Re: reflective calls in trunk

On Fri, Dec 30, 2011 at 10:33 PM, Paul Phillips wrote:
>
>
> On Fri, Dec 30, 2011 at 12:35 PM, martin odersky
> wrote:
>>
>> Of course. Sorry, I should have been clearer in my question. I meant:
>> How do abstract types in reflect.api cause structural types and
>> therefore reflection? Is that some detail that can be easily corrected
>> or something fundamental?
>
>
> The abstract types are OK, but not abstract members with no backing
> interface.
>
>   type AbstractFileType >: Null <: {
>     def path: String
>     def canonicalPath: String
>   }
>
> It is nothing fundamental.  Something as simple as this would send it away.
>
>   trait MinimalFile {
>     def path: String
>     def canonicalPath: String
>   }
>   type AbstractFileType <: MinimalFile
>

I see. Yes, we should do that.

milessabin
Joined: 2008-08-11,
User offline. Last seen 33 weeks 3 days ago.
Re: reflective calls in trunk

On Fri, Dec 30, 2011 at 9:33 PM, Paul Phillips wrote:
> The abstract types are OK, but not abstract members with no backing
> interface.
>
>   type AbstractFileType >: Null <: {
>     def path: String
>     def canonicalPath: String
>   }

As far as I can tell, the only thing that the ">: Null" excludes is an
ultimate concrete definition of the form "... with NotNull". What is
(or was) the rationale for that?

Cheers,

Miles

extempore
Joined: 2008-12-17,
User offline. Last seen 35 weeks 3 days ago.
Re: reflective calls in trunk


On Fri, Dec 30, 2011 at 3:04 PM, Miles Sabin <miles@milessabin.com> wrote:
As far as I can tell, the only thing that the ">: Null" excludes is an
ultimate concrete definition of the form "... with NotNull". What is
(or was) the rationale for that?

I don't know why it's done in this case, but this is usually why.
trait A {  type Foo <: AnyRef     var x: Foo = null}// // ./a.scala:4: error: type mismatch;//  found   : Null(null)//  required: A.this.Foo//   var x: Foo = null //                ^// one error found

Copyright © 2012 École Polytechnique Fédérale de Lausanne (EPFL), Lausanne, Switzerland