| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| |\
| |
| | |
Don't exclude super calls to trait methods from inlining
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
In 8020cd6, the inliner was changed to make sure trait methods bodies
are not duplicated into the static super accessors, and from there into
mixin forwarders.
The check for mixin forwarders was too wide. In `def t = super.m`, where
`m` is a trait method annotated `@inline`, we want to inline `m`. Note
that `super.m` is translated to an `invokestatic T.m$`. The current
check incorrectly identifies `t` as a mixin forwarder, and skip
inlining.
|
| |\ \
| | |
| | | |
SI-10009 Fields survive untypecheck/retypecheck
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Some places in the compiler, and many places in macros, use
`untypecheck` (aka `resetAttrs`) to strip types and local symbols
from a tree before retypechecking it under some different context.
The refactoring of the desugaring of vals and vars in Scala 2.12.0
broke an assumption in this facility.
When a ValDef must be split into multiple members (e.g. a field and
a getter, or a perhaps also a setter), the ValDef that was parsed
assumes the role of the `field`, and the trees for other members are
stached by `Namer` to the `synthetics` map of the compilation unit,
in order to spliced into the right statement list by typechecking.
See `enterGetterSetter` for more details.
However, the parsed ValDef is now used verbatim, carrying the meaning
(ie, the symbol) of the `private[this]` field. This tree now had
an inconsistency between the flags in `tree.mods.flags` and
`tree.symbol.flags`. `tree.name` also differed from `tree.symbol.name`
(the latter was renamed to be a local name, ie one with a trailing space.)
When `ResetAttrs` stripped off the symbol and we retypechecked, we'd
end up with two symbols in scope with the same name.
In the first from the `run` test:
```
================================================================================
{
class a extends scala.AnyRef {
def <init>(): a = {
a.super.<init>();
()
};
private[this] val x: Int = 42;
<stable> <accessor> def x: Int = a.this.x
};
new a()
}
{
class a extends scala.AnyRef {
def <init>() = {
super.<init>();
()
};
val x = 42; // oops, the name is "x" rather than "x " and we've missing `private[this]`!
<stable> <accessor> def x: Int = a.this.x
};
new a()
}
scala.tools.reflect.ToolBoxError: reflective typecheck has failed: x is already defined as value x
```
This commit uses the flags and name of the symbol in `typedValDef`.
I've also had to modify the internals of `CodePrinter` to use the
implicit, override, and deferred flags from the modifiers of an
accessor when recovering pre-typer tree for a ValDef.
|
| |\ \ \
| | | |
| | | | |
Fix more compiler crashes with fields, refinement types
|
| | |/ /
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
In the same manner as scala/scala-dev#219, the placement of the fields
phase after uncurry is presenting some challenges in keeping our trees
type correct.
This commit whacks a few more moles by adding a casts in the body of
synthetic methods.
Fixes scala/scala-dev#268
|
| |\ \ \
| |_|/
|/| | |
Avoid name table pollution with fresh existentials
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
During large compilations runs, the large numbers of globally unique
fresh names for existentials captured from prefixes of `asSeenFrom`.
is a) somewhat wasteful (all these names are interned in the name table)
, and, b) form a pathological case for the current implementation of
`Names#hashValue`, which leads to overfull hash-buckets in the name table.
`hashValue` should probably be improved, but my attempts to do so have
shown a small performance degradation in some benchmarks. So this commit
starts by being more frugal with these names, only uniquely naming
within an `asSeenFrom` operation.
References scala/scala-dev#246
|
| |\ \ \
| | | |
| | | | |
SI-8779 Enable inlining of code within a REPL session
|
| | | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
The REPL has a long running instance of Global which outputs
classfiles by default to a VirtualDirectory. The inliner did not find
any of these class files when compiling calls to methods defined in
previous runs (ie, previous lines of input.)
This commit:
- Adds a hook to augment the classpath that the optimizer searches,
and uses this in the REPL to add the output directory
- Fixes the implementation of `findClassFile` in VirtualDirectory,
which doesn't seem to have been used in anger before. I've factored out
some common code into a new method on `AbstractFile`.
- Fixes a similar problem getSubDir reported by Li Haoyi
- Adds missing unit test coverage.
This also fixes a bug in REPL autocompletion for types defined
in packages >= 2 level deep (with the `:paste -raw` command).
I've added a test for this case.
|
| |\ \ \ \
| |/ / /
|/| | | |
Better inliner support for 2.12 trait encoding
|
| | | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
One line per inline request, nested inlines are indented. Log when a
rollback happens. Examples:
```
Inline into scala/collection/SeqLike$$anon$2.andThen: inlined scala/collection/SeqLike$$anon$2.andThen. Before: 8 ins, inlined: 8 ins.
inlined scala/PartialFunction.andThen$. Before: 20 ins, inlined: 8 ins.
inlined scala/PartialFunction.andThen. Before: 31 ins, inlined: 10 ins.
```
and
```
Inline into scala/collection/IterableLike$$anon$1.takeWhile: inlined scala/collection/IterableLike$$anon$1.takeWhile. Before: 8 ins, inlined: 8 ins.
inlined scala/collection/TraversableViewLike.takeWhile$. Before: 20 ins, inlined: 8 ins.
failed scala/collection/TraversableViewLike.takeWhile. [...] would cause IllegalAccessError [...]
rolling back, nested inline failed.
```
|
| | | | |
| | | |
| | | |
| | | |
| | | | |
Rename `undoLog.run` to `rollback`, use java ArrayList instead of
helper methods to copy to an array.
|
| | | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Some changes to the trait encoding came late in the 2.12 cycle, and the
inliner was not adapted to support it in the best possible way.
In 2.12.0 concrete trait methods are encoded as
interface T {
default int m() { return 1 }
static int m$(T $this) { <invokespecial $this.m()> }
}
class C implements T {
public int m() { return T.m$(this) }
}
If a trait method is selected for inlining, the 2.12.0 inliner would
copy its body into the static super accessor `T.m$`, and from there into
the mixin forwarder `C.m`.
This commit special-cases the inliner:
- We don't inline into static super accessors and mixin forwarders.
- Insted, when inlining an invocation of a mixin forwarder, the
inliner also follows through the two forwarders and inlines the
trait method body.
There was a difficulty implementing this: inlining the static static
super accessor would copy an `invokespecial` instruction into a
different classfile, which is not legal / may change semantics. That
`invokespecial` is supposed to disappear when inlining the actual
default method body. However, this last step may fail, for example
because the trait method body itself contains instructions that are not
legal in a different classfile.
It is very difficult to perform all necessary checks ahead of time. So
instead, this commit implements the ability to speculatively inline a
callsite and roll back if necessary.
The commit also cleans up the implementation of inliner warnings a
little. The previous code would always emit a warning when a method
annotated `@inline` was not picked by the heuristics - this was a
problem when the callsite in the static super accessor was no longer
chosen.
|
| |\ \ \ \
| |/ / /
|/| | | |
SI-10071 Separate compilation for varargs methods
|
| | | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Make sure that methods annotated with varargs are properly mixed-in. This commit
splits the transformation into an info transformer (that works on all symbols, whether
they come from source or binary) and a tree transformer.
The gist of this is that the symbol-creation part of the code was moved to the UnCurry
info transformer, while tree operations remained in the tree transformer. The newly
created symbol is attached to the original method so that the tree transformer can still
retrieve the symbol.
A few fall outs:
- I removed a local map that was identical to TypeParamsVarargsAttachment
- moved the said attachment to StdAttachments so it’s visible between reflect.internal
and nsc.transform
- a couple more comments in UnCurry to honour the boy-scout rule
|
| |\ \ \ \
| |/ / /
|/| | | |
SI-9814 Fix synchronized in specialized overrides
|
| | | |/
| |/|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Specialization creates a subclasses of a specializd class for each
type parameter combination. These contains copies of the methods from
the superclass.
However, before this transform, the pattern of self-synchronization
in a method body had been replace by flag Flag.SYNCHRONIZED on the
method symbol. This was not being propagated to the override, and
hence no locking occured.
This commit modifies the creation of the specialized overload symbol
to copy the SYNCHRONIZED flag, as was already done for ASBOVERRIDE.
I have also done the same for the `@strictfp` annotation.
|
| |/ /
| |
| |
| | |
Scaladoc was prone to warning about java imports.
|
| |\ \
| | |
| | | |
SI-9953 Any Any aborts warn on equals
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | | |
Don't warn about equals if any `Any` is involved. cf SI-8965
The condition for warning is that both types lub to a supertype
of Object.
|
| |\ \ \
| | | |
| | | | |
SI-9944 Scan after interp expr keeps CR
|
| | | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
In an interpolated expression `s"""${ e }"""`, the scanner
advances input past the RBRACE. If a multiline string as
shown, get the next raw char, because CR is significant.
|
| |\ \ \ \
| | | | |
| | | | | |
SI-10059 reset the `DEFERRED` flag for Java varargs forwarders
|
| | | |_|/
| |/| |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
When an abstract method is annotated `@varargs`, make sure that the
generated synthetic Java varargs method does not have the `DEFERRED`
flag (`ACC_ABSTRACT` in bytecode).
The flag lead to an NPE in the code generator, because the ASM framework
leaves certain fields `null` for abstract methods (`localVariables` in
this case).
Interestingly this did not crash in 2.11.x: the reason is that the test
whether to emit a method body or not has changed in the 2.12 backend
(in c8e6050).
val isAbstractMethod = [..] methSymbol.isDeferred [..] // 2.11
val isAbstractMethod = rhs == EmptyTree // 2.12
So in 2.11, the varargs forwarder method was actually left abstract in
bytecode, leading to an `AbstractMethodError: T.m([I)I` at run-time.
|
| |\ \ \ \
| | | | |
| | | | | |
SI-9915 Utf8_info are modified UTF8
|
| | | |_|/
| |/| |
| | | |
| | | |
| | | |
| | | | |
Use DataInputStream.readUTF to read CONSTANT_Utf8_info.
This fixes reading embedded null char and supplementary chars.
|
| | | | | |
|
| |\ \ \ \
| | | | |
| | | | | |
SI-6978 No linting of Java parens
|
| | |/ / /
| | | |
| | | |
| | | |
| | | | |
Don't lint overriding of nullary by non-nullary
when non-nullary is Java-defined. They can't help it.
|
| |\ \ \ \
| | | | |
| | | | | |
SI-6734 Synthesize companion near case class
|
| | | | | | |
|
| | | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
Tweak the "should I synthesize now" test for
case modules, so that the tree is inserted in
the same tree as the case class.
|
| | | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
When a return in a finalizer was reached through a return within the try
block, the backend ignored the return in the finalizer:
try {
try { return 1 }
finally { return 2 }
} finally { println() }
This expression should evaluate to 2 (it does in 2.11.8), but in 2.12.0
it the result is 1.
The Scala spec is currently incomplete, it does not say that a finalizer
should be exectuted if a return occurs within a try block, and it does
not specify what happens if also the finally block has a return.
So we follow the Java spec, which basically says: if the finally blocks
completes abruptly for reason S, then the entire try statement completes
abruptly with reason S. An abrupt termination of the try block for a
different reason R is discarded.
Abrupt completion is basically returning or throwing.
|
| | |_|/ /
|/| | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Return statements within `try` or `catch` blocks need special treatement
if there's also a `finally`
try { return 1 } finally { println() }
For the return, the code generator emits a store to a local and a jump
to a "cleanup" version of the finally block. There will be 3 versions
of the finally block:
- One reached through a handler, if the code in the try block
throws; re-throws at the end
- A "cleanup" version reached from returns within the try; reads the
local and returns the value at the end
- One reached for ordinary control flow, if there's no return and no
exception within the try
If there are multiple enclosing finally blocks, a "cleanup" version is
emitted for each of them. The nested ones jump to the enclosing ones,
the outermost one reads the local and returns.
A global variable `shouldEmitCleanup` stores whether cleanup versions
are required for the curren finally blocks. By mistake, this variable
was not reset to `false` when emitting a `try-finally` nested within a
`finally`:
try {
try { return 1 }
finally { println() } // need cleanup version
} finally { // need cleanup version
try { println() }
finally { println() } // no cleanup version needed!
}
In this commit we ensure that the variable is reset when emitting
nested `try-finally` blocks.
|
| |\ \ \ \
| | | | |
| | | | |
| | | | |
| | | | | |
No StackOverflowError in Java doc comment scanning
Fixes SI-10020 SI-10027
|
| | | | | | |
|
| | | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
Align the Scala and Java doc comment scanning methods a bit.
The Scala one especially had gotten a bit messy,
with regular block comments being kind of accumulated,
but never actually registered as DocComments.
|
| | | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
Avoid StackOverflow on big comments.
Simplify `ScaladocJavaUnitScanner` while in there.
TODO: Do same for `ScaladocUnitScanner`?
|
| |\ \ \ \ \
| | | | | |
| | | | | | |
Frontend fixes for scala-dev#248
|
| | | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | | |
Tighten some types (Symbol -> ClassSymbol / ModuleSymbol), use NonFatal
instead of catching Throwable.
Also don't run the classfile parser enteringPhase(phaseBeforeRefchecks)
anymore. This was added in 0ccdb15 but seems no longer required.
|
| | | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | | |
Skipping other annotations not only saves some cycles / GC, but also
prevents some spurious warnings / errors related to cyclic dependencies
when parsing annotation arguments refering to members of the class.
|
| | | | | | | |
|
| | | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | | |
In SymbolLoaders, when seeing a classfile `Foo.class`, we always
(unconditionally) create 3 symbols: a class, a module and a module
class. Some symbols get invalidated later (`.exists`).
Until now, the classfile parser (and unpickler) received the "root"
symbol as argument, which is the symbol whose type is being completed.
This is either the class symbol or the module symbol.
The classfile parser would then try to lookup the other symbol through
`root.companionClass` or `root.companionModule`. Howver, this lookup can
fail. One example is scala-dev#248: when a type alias (in a package
object) shadows a class symbol, `companionClass` will fail.
The implementations of the classfile parser / unpickler assume that
both the `clazz` and the `staticModule` symbols are available. This
change makes sure that they are always passed in explicitly.
Before this patch, in the example of scala-dev#248, the `classRoot` of
the unpickler was NoSymbol. This caused a bug when unpickling the
module class symbol, causing a second module class symbol to be created
mistakingly. The next commit cleans up this logic, more details there.
This second symbol would then cause the crash in the backend because it
doesn't have an `associatedFile`, therefore `isCoDefinedWith` would
spuriously return `true`.
|
| | | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | | |
One of the first entries in the classfile is the class name. The
classfile parser performs a cross-check by looking up the class sybmol
corresponding to that name and ensures it's the same as `clazz`, the
class symbol that the parser currently populates.
Since 322c980 ("Another massive IDE checkin"), if at the time of the
check `clazz` but the lookup returns some class, the `clazz` field is
assigned.
The commit following this one makes sure `clazz` is never NoSymbol, so
the assignment can safely be removed.
|
| | | |/ / /
| |/| | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
There was a piece of logic essentially duplicating getClassByName
in Mirrors (split up a fully qualified class name by ".", look up
pieces). That piece of code was added in 0ce0ad5 to fix one example in
SI-2464.
However, since 020053c (2012, 2.10) that code was broken: the line
ss = name.subName(0, start)
should be
ss = name.subName(start, name.length).toTypeName
As a result, the code would always create a stub symbol. Returning a
stub seems to be the right thing to do anyway, and the fact that we were
doing so during two major releases is a good proof.
|
| |\ \ \ \ \
| |/ / / /
|/| | | | |
SI-9750 scala.util.Properties.isJavaAtLeast works with JDK9
|
| | | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
Formatting suppressed exceptions required reflection for platform
compatibility. No longer, since Java 8 is assumed. Minor tidying.
|
| | | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
just in time for Halloween. "boostrap" is definitely the most
adorable typo evah -- and one of the most common, too. but we don't
want to scare anybody.
|
| | | | | |
| | | | |
| | | | |
| | | | | |
now that STARR includes the relevant fix
|
