| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| |
|
|
| |
see https://github.com/scala/scala-dev/issues/72 for details
|
| |\ |
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
I imagine these date back to old Subversion days and are probably the
result of inadvertent commits from Windows users with vcs client
configs.
having the bit set isn't really harmful most of the time,
but it's just not right, and it makes the files stand out in directory
listings for no reason
|
| |/
|
|
|
|
|
|
|
|
|
|
|
| |
I'm pretty sure the `isSynthetic` call added in 854de25ee6 should
instead be `isArtifact`, so that's what I've implemented here.
`isSynthetic` used to also filter out error symbols, which are
created with the flags `SYNTHETIC | IS_ERROR`. I've added an addition
test for `isError`, which was needed to keep the output of
`presentation/scope-completion-import` unchanged.
The checkfile for `presentation/callcc-interpreter` is modified to
add the additional completion proposals: synthetic companion objects.
|
| | |
|
| |
|
|
|
|
|
| |
The checkfile of the tests added in the last commit offered
a type member from `RichInt` in the completions for the type
`Int`. However, only term members can be extension methods;
type members cannot.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Three items of background are needed to understand this bug.
1. When typechecking an application like `qual.m({stats; expr})`, the
argument is typechecked using the formal parameter type of `m` as the
expected type. If this fails with a type error located within in
`expr`, the typer instead re-typechecks under `ContextMode.ReTyping`
without an expected type, and then searches for an implicit adaptation
to enable `view(qual).m(args)`. Under this mode, `Typer#typed1`
clears the type of incoming trees.
2. The presentation compiler performs targetted operations like
type completions by:
- typechecking the enclosing tree, registering all typechecker
`Context`s created in the process (`registerContext`)
- finding the smallest enclosing `Context` around the target
position (`doLocateContext`)
- Using this context to perform implicit search, which can
contribute members to the completion. (`applicableViews` within
`interactive.Global#typeMembers`)
3. When verifiying whether or not a candidate implicit is applicable
as a view from `F => T`, implicit search typechecks a dummy call
of the form `q"candiate(${Ident("<argument>").setType(typeOf[F])})".
Now, picture yourself at the nexus of these three storms.
In the enclosed test case, we search for completions at:
x + 1.<caret>
1. Because the code is incomplete, the application of `Int#+`
doesn't typecheck, and the typer also tries to adapt `x` to a
method applicable to the re-typechecked argument.
2. This process registers a context with `retypechecking` set to
true. (If multiple contexts at the same position are registered,
the last one wins.)
3. Implicit search uses this context to typecheck
`Predef.Ensuring(<argument>.setType(Int))`, but the argument
is promptly stripped of its type and retypechecking fails
as there is no definition named `<argument>` in scope.
As such, we missed out on extension methods, like `ensuring` in the
list of completions.
This commit changes the presentation compiler to turn off retyping
mode in the context before starting to work with it. (Are the other
modes that might cause similar bugs?)
Once I made that change, I noticed that the results the enclosed test
was not stable. I tracked this down to the use of a `HashMap` to
carry the applicable implicit views, together with the way that
the presentation compiler removes duplicates. This commit
switched to a `LinkedHashMap`.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
This commit corrects many typos found in scaladocs, comments and
documentation. It should reduce a bit number of PRs which fix one
typo.
There are no changes in the 'real' code except one corrected name of
a JUnit test method and some error messages in exceptions. In the case
of typos in other method or field names etc., I just skipped them.
Obviously this commit doesn't fix all existing typos. I just generated
in IntelliJ the list of potential typos and looked through it quickly.
|
| |\
| |
| | |
SI-8943 Handle non-public case fields in pres. compiler
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
When a case class is type checked, synthetic methods are added,
such as the `hashCode`/`equals`, implementations of the `Product`
interface. At the same time, a case accessor method is added for
each non-public constructor parameter. This the accessor for a
parameter named `x` is named `x$n`, where `n` is a fresh suffix.
This is all done to retain universal pattern-matchability of
case classes, irrespective of access. What is the point of allowing
non-public parameters if pattern matching can subvert access? I
believe it is to enables private setters:
```
case class C(private var x: String)
scala> val x = new C("")
x: C = C()
scala> val c = new C("")
c: C = C()
scala> val C(x) = c
x: String = ""
scala> c.x
<console>:11: error: variable x in class C cannot be accessed in C
c.x
^
scala> c.x = ""
<console>:13: error: variable x in class C cannot be accessed in C
val $ires2 = c.x
^
<console>:10: error: variable x in class C cannot be accessed in C
c.x = ""
^
```
Perhaps I'm missing additional motivations.
If you think scheme sounds like a binary compatiblity nightmare,
you're right: https://issues.scala-lang.org/browse/SI-8944
`caseFieldAccessors` uses the naming convention to find the right
accessor; this in turn is used in pattern match translation.
The accessors are also needed in the synthetic `unapply` method
in the companion object. Here, we must tread lightly to avoid
triggering a typechecking cycles before; the synthesis of that method
is not allowed to force the info of the case class.
Instead, it uses a back channel, `renamedCaseAccessors` to see
which parameters have corresonding accessors.
This is pretty flaky: if the companion object is typechecked
before the case class, it uses the private param accessor directly,
which it happends to have access to, and which duly gets an
expanded name to allow JVM level access. If the companion
appears afterwards, it uses the case accessor method.
In the presentation compiler, it is possible to typecheck a source
file more than once, in which case we can redefine a case class. This
uses the same `Symbol` with a new type completer. Synthetics must
be re-added to its type.
The reported bug occurs when, during the second typecheck, an entry
in `renamedCaseAccessors` directs the unapply method to use `x$1`
before it has been added to the type of the case class symbol.
This commit clears corresponding entries from that map when we
detect that we are redefining a class symbol.
Case accessors are in need of a larger scale refactoring. But I'm
leaving that for SI-8944.
|
| |\ \
| | |
| | | |
SI-8941 Idempotent presentation compilation of implicit classes
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
A retrospective test case which covers typechecking idempotency which
was introduced in 0b78a0196 / 148736c3df. It also tests the
implicit class handling, which was fixed in the previous commit.
It is difficult to test this using existing presentation compiler
testing infrastructure, as one can't control at which point during
the first typechecking the subesquent work item will be noticed.
Instead, I've created a test with a custom subclass of
`interactive.Global` that allows precise, deterministic control
of when this happens. It overrides `signalDone`, which is called
after each tree is typechecked, and watches for a defintion with
a well known name. At that point, it triggers a targetted typecheck
of the tree marked with a special comment.
It is likely that this approach can be generalized to a reusable
base class down the track. In particular, I expect that some of
the nasty interactive ScalaDoc bugs could use this single-threaded
approach to testing the presentation compiler.
|
| | |/
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
When we name an implicit class, `enterImplicitWrapper` is called,
which enters the symbol for the factory method into the
owning scope. The tree defining this factory method is stowed into
`unit.synthetics`, from whence it will be retrieved and incorporated
into the enclosing tree during typechecking (`addDerivedTrees`).
The entry in `unit.synthetics` is removed at that point.
However, in the presentation compiler, we can typecheck a unit
more than once in a single run. For example, if, as happens
in the enclosed test, a call to ask for a type at a given
position interrupts type checking of the entire unit, we
can get into a situation whereby the first type checking
invocation has consumed the entry from `unit.synthetics`,
and the second will crash when it can't find an entry.
Similar problems have been solved in the past in
`enterExistingSym` in the presentation compiler. This method
is called when the namer encounters a tree that already has
a symbol attached. See 0b78a0196 / 148736c3df.
This commit takes a two pronged approach.
First, `enterExistingSym` is extended to handle implicit classes.
Any previous factory method in the owning scope is removed, and
`enterImplicitWrapper` is called to place a new tree for the factory
into `unit.synthetics` and to enter its symbol into the owning scope.
Second, the assertions that could be tripped in `addDerivedTrees`
and in `ImplicitClassWrapper#derivedSym` have been converted to
positioned errors.
The first change is sufficient to fix this bug, but the second
is also enough to make the enclosed test pass, and has been retained
as an extra layer of defence.
|
| |/
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The code that "aligns" patterns and extractors assumes that it can
look at the type of the unapply method to figure the arity of the
extractor.
However, the result type of a whitebox macro does not tell the
whole story, only after expanding an application of that macro do
we know the result type.
During regular compilation, this isn't a problem, as the macro
application is expanded to a call to a synthetic unapply:
{
class anon$1 {
def unapply(tree: Any): Option[(Tree, List[Treed])]
}
new anon$1
}.unapply(<unapply selector>)
In the presentation compiler, however, we now use
`-Ymacro-expand:discard`, which expands macros only to compute
the type of the application (and to allow the macro to issue
warnings/errors). The original application is retained in the
typechecked tree, modified only by attributing the potentially-sharper
type taken from the expanded macro.
This was done to improve hyperlinking support in the IDE.
This commit passes `sel.tpe` (which is the type computed by the
macro expansion) to `unapplyMethodTypes`, rather than using
the `finalResultType` of the unapply method.
This is tested with a presentation compiler test (which closely
mimics the reported bug), and with a pos test that also exercises
`-Ymacro-expand:discard`. Prior to this patch, they used to fail with:
too many patterns for trait api: expected 1, found 2
|
| |
|
|
|
|
|
| |
This reverts commit 8986ee4fd56c53d563165d992185c6c532f35790.
Scaladoc seems to work reliably for 2.11.x. We are using it in the IDE builds and haven't noticed any
flakiness, so we'd like to get reinstate this test.
|
| |
|
|
|
|
| |
The mentioned issue is a presentation compiler issue, but its root cause is a bug in the parser which incorrectly assigned positions to incomplete selection trees (i.e. selections that lack an indentifier after dot and have some whitespace instead).
In detail: for such incomplete selection trees, the "point" of the position should be immediately after the dot but instead was at the start of next token after the dot. For range positions, this caused a pathological situation where the "point" was greater than the "end" of the position. This position is later used by the typechecker during resolution of dynamic calls and causes it to crash. Of course, because a syntactically incorrect code is required for the bug to manifest, it only happens in the presentation compiler.
|
| |\
| |
| | |
Java 8 agnostism for our test suite
|
| | |
| |
| |
| |
| |
| |
| |
| | |
Under Java 8, the output contains newly added methods in
j.u.Iterator.
I've created cut down, test-local versions of the relevant types
to decouple.
|
| |\ \
| |/
|/|
| |
| |
| |
| | |
merge/2.10.x-to-2.11.x-20140604
Conflicts:
src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Regressed in SI-7915 / 3009a525b5
We should be deriving the position of the synthetic `Select`
from `basefun1`, rather than `basefun`. In the new, enclosed
test, the difference amounts to:
new Container().typeParamAndDefaultArg[Any]()
`------------ basefun1 --------------'
`----------------- basefun ---------------'
For monomorphic methods, these are one and the same, which is
why `presentation/t7915` was working. I've extended that test
to a polymorphic method to check that hyperlink resolution works.
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
It turns out `findMembers` has been a bit sloppy in recent
years and has returned private members from *anywhere* up the
base class sequence. Access checks usually pick up the slack
and eliminate the unwanted privates.
But, in concert with the "concrete beats abstract" rule in
`findMember`, the following mishap appeared:
scala> :paste
// Entering paste mode (ctrl-D to finish)
trait T { def a: Int }
trait B { private def a: Int = 0 }
trait C extends T with B { a }
// Exiting paste mode, now interpreting.
<console>:9: error: method a in trait B cannot be accessed in C
trait C extends T with B { a }
^
I noticed this when compiling Akka against JDK 8; a new private method
in the bowels of the JDK was enough to break the build!
It turns out that some finesse in needed to interpret SLS 5.2:
> The private modifier can be used with any definition or declaration
> in a template. They are not inherited by subclasses [...]
So, can we simply exclude privates from all but the first base class?
No, as that might be a refinement class! The following must be
allowed:
trait A { private def foo = 0; trait T { self: A => this.foo } }
This commit:
- tracks when the walk up the base class sequence passes the
first non-refinement class, and excludes private members
- ... except, if we are at a direct parent of a refinement
class itself
- Makes a corresponding change to OverridingPairs, to only consider
private members if they are owned by the `base` Symbol under
consideration. We don't need to deal with the subtleties of
refinements there as that code is only used for bona-fide classes.
- replaces use of `hasTransOwner` when considering whether a
private[this] symbol is a member.
The last condition was not grounded in the spec at all. The change
is visible in cases like:
// Old
scala> trait A { private[this] val x = 0; class B extends A { this.x } }
<console>:7: error: value x in trait A cannot be accessed in A.this.B
trait A { private[this] val x = 0; class B extends A { this.x } }
^
// New
scala> trait A { private[this] val x = 0; class B extends A { this.x } }
<console>:8: error: value x is not a member of A.this.B
trait A { private[this] val x = 0; class B extends A { this.x } }
^
Furthermore, we no longer give a `private[this]` member a free pass
if it is sourced from the very first base class.
trait Cake extends Slice {
private[this] val bippy = ()
}
trait Slice { self: Cake =>
bippy // BCS: Cake, Slice, AnyRef, Any
}
The different handling between `private` and `private[this]`
still seems a bit dubious.
The spec says:
> An different form of qualification is private[this]. A member M
> marked with this modifier can be accessed only from within the
> object in which it is defined. That is, a selection p.M is only
> legal if the prefix is this or O.this, for some class O enclosing
> the reference. In addition, the restrictions for unqualified
> private apply.
This sounds like a question of access, not membership. If so, we
should admit `private[this]` members from parents of refined types
in `FindMember`.
AFAICT, not too much rests on the distinction: do we get a
"no such member", or "member foo inaccessible" error? I welcome
scrutinee of the checkfile of `neg/t7475f.scala` to help put
this last piece into the puzzle.
One more thing: findMember does not have *any* code the corresponds
to the last sentence of:
> SLS 5.2 The modifier can be qualified with an identifier C
> (e.g. private[C]) that must denote a class or package enclosing
> the definition. Members labeled with such a modifier are accessible
> respectively only from code inside the package C or only from code
> inside the class C and its companion module (§5.4).
> Such members are also inherited only from templates inside C.
When I showed Martin this, he suggested it was an error in the
spec, and we should leave the access checking to callers of
that inherited qualified-private member.
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Swathes of important logic are duplicated between `findMember`
and `findMembers` after they separated on grounds of irreconcilable
differences about how fast they should run:
d905558 Variation #10 to optimze findMember
fcb0c01 Attempt #9 to opimize findMember.
71d2ceb Attempt #8 to opimize findMember.
77e5692 Attempty #7 to optimize findMember
275115e Fixing problem that caused fingerprints to fail in
e94252e Attemmpt #6 to optimize findMember
73e61b8 Attempt #5 to optimize findMember.
04f0b65 Attempt #4 to optimize findMember
0e3c70f Attempt #3 to optimize findMember
41f4497 Attempt #2 to optimize findMember
1a73aa0 Attempt #1 to optimize findMember
This didn't actually bear fruit, and the intervening years have
seen the implementations drift.
Now is the time to reunite them under the banner of `FindMemberBase`.
Each has a separate subclass to customise the behaviour. This is
primarily used by `findMember` to cache member types and to assemble
the resulting list of symbols in an low-allocation manner.
While there I have introduced some polymorphic calls, the call sites
are only bi-morphic, and our typical pattern of compilation involves
far more `findMember` calls, so I expect that JIT will keep the
virtual call cost to an absolute minimum.
Test results have been updated now that `findMembers` correctly
excludes constructors and doesn't inherit privates.
Coming up next: we can actually fix SI-7475!
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
And the same for !=
If we tried to declare these signatures in non-fictional classes,
we would be chastised about collapsing into the "same signature after
erasure".
This will have an influence of typing, as the typechecking of
arguments is sensitive to overloading: if multiple variants are
feasible, the argument will be typechecked with a wildcard expected
type. So people inspecting the types of the arguments to `==` before
this change might have seen an interesting type for
`if (true) x else y`, but now the `If` will have type `Any`, as we
don't need to calculate the LUB.
I've left a TODO to note that we should really make `Any#{==, !=}`
non-final and include a final override in `AnyVal`. But I don't think
that is particularly urgent.
|
| |\|
| |
| |
| |
| |
| |
| |
| |
| | |
Check files updated: test/files/presentation/t8085*.check
Conflicts:
build.xml
src/compiler/scala/tools/nsc/ast/parser/Parsers.scala
src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
A source file like:
import foo.bar
package object baz
Is parsed into:
package <empty> {
import foo.bar
package baz {
object `package`
}
}
A special case in Namers compensates by adjusting the owner of
`baz` to be `<root>`, rather than `<empty>`.
This wasn't being accounted for in `BrowserTraverser`, which
underpins `-sourcepath`, and allows the presentation compiler to
load top level symbols from sources outside those passes as
the list of sources to compile.
This bug did not appear in sources like:
package p1
package object p2 { ... }
... because the parser does not wrap this in the `package <empty> {}`
This goes some way to explaining why it has gone unnoticed for
so long.
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
* The presentation compiler sourcepath is now correctly set-up.
* Amazingly enough (for me at least), the outer import in the
package object seem to be responsible of the faulty behavior.
In fact, if you move the import clause *inside* the package object,
the test succeed!
The test's output does provide the correct hint of this:
```
% diff /Users/mirco/Projects/ide/scala/test/files/presentation/t8085-presentation.log /Users/mirco/Projects/ide/scala/test/files/presentation/t8085.check
@@ -1,3 +1,2 @@
reload: NodeScalaSuite.scala
-prefixes differ: <empty>.nodescala,nodescala
-value always is not a member of object scala.concurrent.Future
+Test OK
```
Notice the ``-prefixes differ: <empty>.nodescala,nodescala``. And compare
it with the output when the import clause is placed inside the package
object:
```
% diff /Users/mirco/Projects/ide/scala/test/files/presentation/t8085-presentation.log /Users/mirco/Projects/ide/scala/test/files/presentation/t8085.check
@@ -1,4 +1,2 @@
reload: NodeScalaSuite.scala
-reload: NodeScalaSuite.scala
-open package module: package object nodescala
Test OK
```
Notice now the ``-open package module: package object nodescala``!
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
This PR (https://github.com/scala/scala/pull/3275#issuecomment-31986434)
demonstrates that the test is flaky.
The disabled test was introduced with the intent of preventing a
regression (here is the commit
https://github.com/scala/scala/commit/ccacb06c4928fd6aebc2c2539d7565cb079dc625).
It looks like there is a race condition when `askTypeAt(pos)` is called
on `implicitly[Foo[A]].foo` where `pos` is matches the end point of the
former expression. The issue is that the returned Tree is unattributed,
which is why the error "No symbol is associated with tree
implicitly[Foo[A]].foo" is reported.
|
| |\ \
| | |
| | | |
Presentation compiler friendliness for macros
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
The presentation compiler is primarily interested in trees that
represent the code that one sees in the IDE, not the expansion of
macros.
This commit continues to expand macros, but adds a hook in which
the presentation compiler discards the expansion, retaining instead
the expandee. The expandee is attributed with the type of the
expansion, which allows white box macros to work. In addition,
any domain specific errors and warnings issued by the macro will
still be reported, as a side-effect of the expansion.
The failing test from the last commit now correctly resolves
hyperlinks in macro arguments.
Related IDE ticket:
https://www.assembla.com/spaces/scala-ide/tickets/1001449#
This facility is configured as follows:
// expand macros as per normal
-Ymacro-expand:normal
// don't expand the macro, takes the place of -Ymacro-no-expand
-Ymacro-expand:none
// expand macros to compute type and emit warnings,
// but retain expandee. Set automatically be the presentation
// compiler
-Ymacro-expand:discard
This leaves to door ajar for a new option:
// Don't expand blackbox macros; expand whitebox
// but retain expandee
-Ymacro-expand:discard-whitebox-only
The existing test for SI-6812 has been duplicated. One copy exercises
the now-deprecated -Ymacro-no-expand, and the other uses the new
option.
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Currently, the presentation compiler sees the expansion of macros;
this no longer contains the trees corresponding to the macro arguments,
and hyperlink requests result incorrect results.
https://www.assembla.com/spaces/scala-ide/tickets/1001449#
|
| |\ \ \
| |/ /
|/| | |
Issue/si 4287
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
* Default arguments are always retained on the <init> method (i.e.,
the class' constructor). Therefore, when the <init> parameters are
created, we need to use `duplicateAndKeepPositions` to make sure that
if a default argument is present, its opaque position is retained as
well. This is necessary because when parameter accessors (i.e.,
`fieldDefs`) are created, all default arguments are discared ( as you
can see in the code, the right-hand-side of a `field` is always an
`EmptyTree`) - see changes in TreeGen.scala
* When constructing the `fieldDefs`, it is important to adapt their
range position to avoid overlappings with the positions of default
arguments. It is worth noting that updating the field's end position
to `vd.rhs.pos.start` would be incorrect, because `askTypeAt(pos)`
could return the incorrect tree when the position is equal to
`vd.rhs.pos.start` (because two nodes including that point position
would exist in the tree, and `CompilerControl.locateTree(pos)` would
return the first tree that includes the passed `pos`). This is why
`1` is subtracted to `vd.rhs.pos.start`. Alternatively, we could have
used `vd.tpt.pos.end` with similar results. However the logic would
have become slightly more complex as we would need to handle the case
where `vd.tpt` doesn't have a range position (for instance, this can
happen if `-Yinfer-argument-types` is enabled). Therefore, subtracting
`1` from `vd.rhs.pos.start` seemed the cleanest solution at the
moment. - see changes in TreeGen.scala.
* If the synthetic constructor contains trees with an opaque range
position (see point above), it must have a transparent position.
This can only happen if the constructor's parameters' positions are
considered, which is why we are now passing `vparamss1` to
`wrappingPos` - see changes in TreeGen.scala.
* The derived primary constructor should have a transparent position
as it may contain trees with an opaque range position. Hence, the
`primaryCtor` position is considered for computing the position of the
derived constructor - see change in Typers.scala.
Finally, below follows the printing of the tree for test t4287, which
you should compare with the one attached with the previous commit
message:
```
[[syntax trees at end of typer]] // Foo.scala
[0:63]package [0:0]<empty> {
[0:37]class Baz extends [9:37][39]scala.AnyRef {
[10:20]<paramaccessor> private[this] val f: [14]Int = _;
[14]<stable> <accessor> <paramaccessor> def f: [14]Int = [14][14]Baz.this.f;
<10:31>def <init>(<10:31>f: [17]<type: [17]scala.Int> = [23:31]B.a): [9]Baz = <10:31>{
<10:31><10:31><10:31>Baz.super.<init>();
<10:31>()
}
};
[6]<synthetic> object Baz extends [6][6]AnyRef {
[6]def <init>(): [9]Baz.type = [6]{
[6][6][6]Baz.super.<init>();
[9]()
};
[14]<synthetic> def <init>$default$1: [14]Int = [30]B.a
};
[39:63]object B extends [48:63][63]scala.AnyRef {
[63]def <init>(): [48]B.type = [63]{
[63][63][63]B.super.<init>();
[48]()
};
[52:61]private[this] val a: [56]Int = [60:61]2;
[56]<stable> <accessor> def a: [56]Int = [56][56]B.this.a
}
}
```
You should notice that the default arg of `Baz` constructor now has a
range position. And that explains why the associated test now returns
the right tree when asking hyperlinking at the location of the default
argument.
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
As demonstrated by the attached test, hyperlinking on constructor's
default arg doesn't work (the returned tree is the parameter tree,
i.e., `f`, which is of course wrong).
Printing the tree reveals the issue:
``
[[syntax trees at end of typer]] // Foo.scala
[0:63]package [0:0]<empty> {
[0:37]class Baz extends [9:37][39]scala.AnyRef {
[10:31]<paramaccessor> private[this] val f: [14]Int = _;
[14]<stable> <accessor> <paramaccessor> def f: [14]Int = [14][14]Baz.this.f;
[39]def <init>([14]f: [17]<type: [17]scala.Int> = [30]B.a): [9]Baz = [39]{
[39][39][39]Baz.super.<init>();
[9]()
}
};
[6]<synthetic> object Baz extends [6][6]AnyRef {
[6]def <init>(): [9]Baz.type = [6]{
[6][6][6]Baz.super.<init>();
[9]()
};
[14]<synthetic> def <init>$default$1: [14]Int = [30]B.a
};
[39:63]object B extends [48:63][63]scala.AnyRef {
[63]def <init>(): [48]B.type = [63]{
[63][63][63]B.super.<init>();
[48]()
};
[52:61]private[this] val a: [56]Int = [60:61]2;
[56]<stable> <accessor> def a: [56]Int = [56][56]B.this.a
}
}
``
In short, the default argument in `<init>` (the constructor) has an
offset position, while we would expect it to have a range position.
Therefore, when locating a tree for any position between (start=) 10
and (end=) 31, the paramaccessor tree `f` is returned.
The next commit will correct the problem.
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
This test was inspired by `test/files/run/t5603`, whose output (.check file)
will need to be updated in the upcoming commit that fixes SI-4287, because
the positions associated to the tree have slightly changed.
The additional test should demonstrate that the change in positions isn't
relevant, and it doesn't affect functionality. In fact, hyperlinking to a
constructor's argument work as expected before and after fixing SI-4287.
|
| | | |
| | |
| | |
| | |
| | | |
Added test to the presentation compiler regression suite to exercise
hyperlinking on context bounds.
|
| |\ \ \
| | | |
| | | | |
SI-8030 force symbols on presentation compiler initialization
|
| | |/ /
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
This commit forces a number of built-in symbols in presentation
compiler to prevent them from being entered during parsing.
The property “parsing doesn’t enter new symbols” is tested on
a rich source file that contains significant number of variations
of Scala syntax.
|
| |\ \ \
| |/ /
|/| /
| |/
| |
| |
| |
| |
| |
| |
| |
| | |
Conflicts:
bincompat-forward.whitelist.conf
src/interactive/scala/tools/nsc/interactive/Global.scala
test/files/presentation/scope-completion-2.check
test/files/presentation/scope-completion-3.check
test/files/presentation/scope-completion-import.check
Conflicts in the scope completion tests handled with the help
of @skyluc in https://github.com/scala/scala/pull/3264
|
| | |\
| | |
| | | |
SI-7995 completion imported vars and vals
|
| | | |
| | |
| | |
| | |
| | |
| | | |
Imported member vals and vars were always marked inaccessible, even
if referencing them at the location of the completion is valid in code.
The accessible flag is now set accordingly to the accessibility of the getter.
|
| | |\ \
| | |/
| |/| |
make askLoadedType unload arguments out of the PC by default,
|
| |\ \ \
| | |/
| |/|
| | |
| | | |
Conflicts:
src/interactive/scala/tools/nsc/interactive/CompilerControl.scala
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
The rationale for not keeping units loaded by default is that the more
units are loaded, the slower is background compilation. For instance, in
the Scala IDE for Eclipse (which uses the presentation compiler),
typechecking occurs every time the reconciler kicks-in (~500millis after
you stop typing), hence it is important that units are not kept loaded
unless strictly necessary (for some extra information about this, see
https://www.assembla.com/spaces/scala-ide/tickets/1001388)
While I agree that using a boolean argument (`keepLoaded`) for deciding
if a unit should be loaded isn't a great design, other methods in
`CompilerControl` also have a keepLoaded parameter, so at least we have
some consistency. For the future, I'm thinking we should be able to
remove the `keepLoaded` flag altogether, and change the implementation
of `askLoadedType` to preserve the same units loaded in the presentation
compiler before and after its execution. Basically, if you want a unit
to be kept loaded, you should call `askReload` first, and then
`askLoadedType`. However, to reduce impact, I think the changes carried
by this commit will help us estimate if the solution I just outlined is
viable (because `askLoadeType` won't be keeping units loaded by default,
which wasn't the case with the former implementation).
(While the patch was mostly contributed by @huitseeker, @dotta has edited the
commit message to preserve the comments in the PR
https://github.com/scala/scala/pull/3209)
|
| |\ \ \
| | |/
| |/|
| | |
| | | |
Conflicts:
src/compiler/scala/tools/nsc/interactive/Global.scala
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Otherwise we can think that `+` in `1 + BigInt(2)` refers
to a method in `Int`.
In general, this protects the IDE from observing results from
"exploratory" typing which is discarded as the compiler backtracks
to another possibility.
This protection subsumes the condition that checked for overloaded
types: presentation/t7458 now passes without this.
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
We shouldn't observe tree types under silent mode.
The enclosed test is a standalone version of `1 + BigInt(2)`,
a standard example of exploratory typing in Scala. Once we determine
that none of the `+` methods in `Int` accepts (possibly implicitly
coerced `BigInt`), we have to backtrack and look for a view from
`Int => { +(_: BigInt): ? }`.
The next commit will correct the problem.
|
| | |/
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
targeted type-checked
When asking for targeted typecheck, the located tree may have overloaded types
is the source isn't yet fully typechecked (e.g., a select tree for an
overloaded method). This is problematic as it can lead to unknown 'hovers',
broken hyperlinking that suddenly starts working, unresolved ScalaDoc comments,
and similar, in the Scala IDE.
With this commit we are hardening the contract of `askTypeAt` to return the
same type whether the file was fully type-checked or targeted type-checked.
This is done by preventing the typechecker to stop too early if the `located`
tree has an overloaded type. Furthermore, I'm assuming that if `located.tpe`
is of type `OverloadedType`, by letting the compiler carry-on the typechecking,
the `located.tpe` will eventually be resolved to a non-overloaded type. Said
otherwise, I expect the targeted typechecking will always terminate (if my
reasoning isn't sound, please say so).
The test provided with this commit demonstrates the new behavior (the position
used to execute the test is resolved to the `foo` method's call). In fact,
before this commit, executing the test returned the following:
(x: Int, y: String)Unit <and> (x: String)Unit <and> (x: Int)Unit
Showing that the tree's type is an overloaded type. The ambiguity is fixed by
this commit, and in fact the test's output is now:
(x: Int)Unit
|
| | |\
| | |
| | | |
Backport of
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Updates localeContext() to return the best context possible when there are none directly
associated with the given position. It happens when an expression cannot be
successfully typed, as no precise ContextTree covers the expression location, or if the
position is not inside any expression.
Adds corresponding tests
(cherry picked from commit 3028327e2a2b553b12ee45519413515c8aa0865f)
|
| | | |
| | |
| | |
| | | |
(cherry picked from commit 3d55fe723f1af91f4d2db421f0e0965c583346dc)
|
