| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| ... | |
| | | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
If a value class extends a specialized class, it can sprout
specialized members after the specialization info transformer has run.
However, we only install extension methods for class members we know
about at the extmethods phase.
This commit simply disables rewiring calls to these methods in
erasure to an extention method. This follows the approach taken
from super accessors.
Note: value class type parameters themselves currently are not
allowed to be specialized.
|
| |\ \ \ \ \ \ \ \
| |_|/ / / / / /
|/| | | | | | | |
GenBCode refactoring (remove Tracked) and fix InnerClass / EnclosingMethod attributes
|
| | | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
This commit seems bigger than it is. Most of it is tests, and moving
some code around. The actual changes are small, but a bit subtle.
The InnerClass and EnclosingMethod attributes should now be close to
the JVM spec (which is summarized in BTypes.scala). New tests make
sure that changes to these attributes, and changes to the way Java
reflection sees Scala classfiles, don't go unnoticed.
A new file, BCodeAsmCommon, holds code that's shared between the two
backend (it could hold more, future work).
In general, the difficulty with emitting InnerClass / EnclosingMethod
is that we need to find out source-level properties. We need to make
sure to do enough phase-travelling, and work around destructive
changes to the ownerchain in lambdalift (we use originalOwner a lot).
The change to JavaMirrors is prompted by the change to the
EnclosingMethod attribute, which changes Java reflection's answer to
getEnclosingMethod and getEnclosingConstructor. Classes defined in
field initializers no longer have an enclosing method, just an
enclosing class, which broke an assumption in JavaMirrors.
There's one change in erasure. Before this change, when an object
declaration implements / overrides a method, and a bridge is required,
then the bridge method was actually a ModuleSymbol (it would get the
lateMETHOD flag and be emitted as a method anyway). This is confusing,
when iterating through the members of a class, you can find two
modules with the same name, and one of them doesn't have a module
class. Now, such bridge methods will be MethodSymbols.
Removed Symbol.originalEnclosingMethod, that is a backend thing and
doesn't need to live in the symbol API.
|
| | | |_|_|/ / /
| |/| | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
Before this change, a ClassBType was just a name. The `exemplars` map
stored a Tracked instance for every ClassBType. Tracked stored
type information that is required later in the backend when things
may run concurrently. In particular: superclass, interfaces, flags
and information for emitting the InnerClass attribute. Now we put all
the information stored in Tracked directly in the ClassBType.
There is still one hash map: `classBTypeFromInternalNameMap` maps
a JVM internal class name (e.g. "scala/Predef$") to the corresponding
ClassBType. This map is used during bytecode generation, when the
ASM framework computes stack map frames.
In order to compute stack map frames, the ASM framework needs to be
able to get the LUB of two types. The default implementation uses
reflection to get type information, however that doesn't work in our
case: the classes we compile are not on the classpath. So instead,
the backend overwrites the method `getCommonSuperClass` of the
ClassWriter. This method receives two class internal names and
computes their LUB. This is done by looking up the ClassBTypes in the
`classBTypeFromInternalNameMap` and invoking `jvmWiseLUB`.
This commit was reviwed in https://github.com/scala/scala/pull/3855.
It consists of all commits labelled [squash-after-review], squashed
into one.
|
| |\ \ \ \ \ \ \
| | | | | | | |
| | | | | | | | |
SI-8627 make Stream.filterNot non-eager
|
| | |/ / / / / /
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
The obvious fix, overriding `filterNot` in Stream, is not binary
compatible, see https://github.com/scala/scala/pull/3925
Instead, this makes `filterImpl` in TaversableLike private[scala],
which allows overriding it in Stream. The corresponding mima-failures
can be whitelisted, as the changes are only to private[scala].
In 2.12.x we can remove the override of `filter` in Stream, but in
2.11.x this is not binary compatible.
Eventually we'd also like to make filter / filterNot in
TraversableLike final, but that's not source compatible, so it cannot
be done in 2.12.x.
|
| |\ \ \ \ \ \ \
| |_|/ / / / /
|/| | | | | | |
SI-5691 lint warning when a type parameter shadows an existing type.
|
| | | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
This adds a new lint warning for when a class/method/type-member's
type parameter shadows an existing type: `-Xlint:type-parameter-shadow`.
It excludes type parameters of synthetic methods (the user can't
rename or remove those anyway), otherwise, for example, every case class
triggers the warning.
Also fixes a test that contained wrong java sources (that didn't even
compile...), discovered thanks to the warning.
---
This kind of errors shows up every now and then on the mailing-list, on
stackoverflow, etc. so maybe a warning would be useful.
I was afraid this would yield too many warnings for libraries that are
heavy on type parameters, but no: running this on scalaz and shapeless
HEAD (`v7.1.0-RC1-41-g1cc0a96` and `v2.0.0-M1-225-g78426a0` respectively)
yields 44 warnings. None of them are false positives; they usually come
from:
- scalaz loving using `A` as type parameter, even several levels deep
of parametrized classes/methods
- or calling a type parameter that will hold a map `Map`, or similar,
thus shadowing an existing type
|
| |\ \ \ \ \ \ \
| | | | | | | |
| | | | | | | | |
SI-8793 Fix patmat regression with extractors, existentials
|
| | | |_|_|_|_|/
| |/| | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
In the same vein as SI-8128 / 3e9e2c65a, revert to the 2.10.x style
of determining the types of the product elements of an extractor
when using `TupleN`.
I believe we can discard the special casing for Option/Tuple/Seq
altogether with judicious application of `repackExistential` in
`unapplyMethodTypes`. That ought to also fix fix SI-8149. But I'll
target that work at 2.12.x.
|
| |\ \ \ \ \ \ \
| |_|_|_|_|_|/
|/| | | | | | |
part 2 of the big error reporting refactoring
|
| | | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
Reporting mode used to be governed by contextMode.
This logic is left in place by this commit, and the consistency
of the new and the old is checked. Will be removed in follow-up commit.
The main difference is that we no longer throw TypeErrors in buffering mode.
There was one instance of context.error in implicit search the exploited the
fact that implicit search runs in buffering (silent) mode and thus calls to
error(pos,msg) used to throw new TypeError(pos, msg) -- made this explicit,
and removed throwing behavior from the buffering context reporter.
|
| | | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
The two functional differences are:
- always add the diagnostics string
- check erroneousness in `issueAmbiguousTypeErrorUnlessErroneous`,
before even constructing the error message.
Consider this nugget:
```
- def issueAmbiguousError(pre: Type, sym1: Symbol, sym2: Symbol, err: AbsTypeError) {
- issueCommon(err) { case _ if ambiguousErrors =>
- if (!pre.isErroneous && !sym1.isErroneous && !sym2.isErroneous)
```
I'd like to state for the record that the if-erroneous in the
case of the partial function looked super-dodgy: it meant that,
when `ambiguousErrors`, `issueCommon` would not get to the `else` branches
that buffer or throw, and if the erroneous condition was met,
nothing would be issued/buffered/thrown.
This refactoring checks this condition up front.
|
| |\ \ \ \ \ \ \
| |_|_|_|/ / /
|/| | | | | | |
SI-8787 Regextraction is null-proof
|
| | | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
Regex is robust when unapplying null. A null never matches.
|
| | | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
Avoid the widening bug for q. This resolution also suffers
from the inference of Any, which can trigger a warning and
an anxiety attack. But that's still better than doing the
wrong thing.
Right?
|
| | |_|/ / / /
|/| | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | | |
The f-interpolator gets a type param that better be Any to avoid
unfortunate widenings.
Hey, it worked!
Unfortunately, when `Any` is inferred, `-Xlint:infer-any` takes notice.
This is probably a greater problem for the f-interpolator than for
quasiquotes, which are a more specialized tool.
|
| |\ \ \ \ \ \
| | | | | | |
| | | | | | | |
test for InnerClass and EnclosingMethod attributes
|
| | |/ / / / /
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | | |
Some parts of the test assert (current) buggy behavior. This is marked
in the test file with TODO. It will be fixed in later work on the
backend.
|
| |\ \ \ \ \ \
| | | | | | |
| | | | | | | |
SI-4563 friendlier behavior for Ctrl+D in the REPL
|
| | | |_|/ / /
| |/| | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | | |
Closing the REPL with Ctrl+D does not issue a newline, so the user's
prompt displays on the same line as the `scala>` prompt. This is bad.
We now force a newline before closing the interpreter, and display
`:quit` while we're at it so that people know how to exit the REPL
(since `exit` doesn't exist anymore).
The tricky part was to only add a newline when the console is
interrupted, and *not* when it is closed by a command (like `:quit`),
since commands are processed after their text (including newline) has
been sent to the console.
|
| |\ \ \ \ \ \
| |_|/ / / /
|/| | | | | |
SI-6476 Improve error on escapement
|
| | | | | | |
| | | | | |
| | | | | |
| | | | | | |
And adjust the test.
|
| | | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | | |
Behavior of escape processing under string interpolation can
be confusing. This commit improves the exception message so
you know at least what sort of escapes are handled.
This came up on SO in the form `s"\d".r`, where it may not
be obvious what is throwing and how to work around it.
```
scala> s"\d".r
scala.StringContext$InvalidEscapeException: invalid escape '\d' not one of [\b, \t, \n, \f, \r, \\, \", \'] at index 0 in "\d". Use \\ for literal \.
scala> s"a\"
scala.StringContext$InvalidEscapeException: invalid escape at terminal index 1 in "a\". Use \\ for literal \.
```
Referencing SI-6476 because that has become the magnet ticket
for "escape processing under string interpolation, huh?"
This doesn't address `$"` and doesn't handle the more interesting
parse error `s"a\"b"`.
|
| |\ \ \ \ \ \
| | | | | | |
| | | | | | | |
Orphan check file
|
| | | |/ / / /
| |/| | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | | |
Orphaned by the override check reversion that deleted the test source.
2524fdde3edc7b668fdb4bf68e990141d3ec18d6
|
| |/ / / / /
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
This mode of macro expansion is used by the presentation compiler to
leave the original macro applications ("expandees") in the type
checked trees, annotated with the types of the expansions.
However, under some circumstances involving implicits, we would
re-expand the macro. If the macro wasn't stable, this could lead
to a type mismatch.
The originally reported problem was with the shapeless
`mkSingletonOps` macro. Its expansion had the type of a freshly-named
class local to the expansion. Upon the re-expansion, a new class
was generated, which lead to errors like:
client/Client.scala:4: error: type mismatch;
found : fresh$macro$2
required: fresh$macro$1
This commit suppressed re-expansion of the expandee by use of
the existing, tree attachment driven mechanism.
|
| |\ \ \ \ \
| | | | | |
| | | | | | |
Better error message than 'bad symbolic reference'.
|
| | | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | | |
Let's not scare people, and try to give them some advice.
PS: we should really come up with a better mechanism for testing errors/warnings
|
| |/ / / / /
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
The code that generated the Java varargs forwarder was basing
things on the `ValDef-s` of the parameters of the source method.
But, their types refer to a type parameter skolems of the enclosing
method, which led to a type mismatch when typechecking the forwarder.
Instead, I've reworked the code to simply use the `DefDef`-s symbol's
info, which *doesn't* refer to skolems. This actually simplifies the
surrounding code somewhat; rather than repeated symbols in a map
we can just time travel the pre-uncurry method signatures to figure
out which params are releated.
|
| |\ \ \ \ \
| | | | | |
| | | | | | |
SI-8736 Restore -language to former glory
|
| | |/ / / /
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
MultiChoice allows -language to work like -Xlint.
The bug as described was that the setting value was set instead of updated
(++=) with additional flags.
The unreported bug was that `_` no longer set all settings.
The corrected behavior is that "contains" means "it was enabled, or
_ was set and it was not disabled explicitly."
That is the behavior of `-Xlint` but with a different mechanism,
since each lint option is a Setting.
A semantic difference is that -Xlint enables all the lint options,
but -language does not enable all the language features. `scalac -X` does
not explain this additional behavior of the `-Xlint` flag.
Also worth noting that `scalac -feature -language unused.scala` failed
in 2.11.1 but succeeds silently now.
|
| |/ / / /
| | | |
| | | |
| | | |
| | | |
| | | | |
Missed the case of comparing a non-empty range to an empty one. Fixed by checking nonEmpty/isEmpty on other collection.
Added a test to verify the behavior.
|
| |/ / /
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Also clarify usage of -Xlint flag.
Align more with javac -Xlint:all,-flag,flag where once a flag is
explicitly enabled it cannot be disabled, but where the wildcard
is a backstop only. (There is no all option yet here.)
-Xlint and -Xlint:_ just set a flag which is consulted by any
unset lint warning.
Xlint warnings consult the state of Xlint when they are unset.
Individual -Ywarn-ings do not.
Other warnings are explicitly set to false. They can only be
enabled programmatically.
Some tests are corrected. Also, option order is no longer
significant, see the unit test.
|
| |\ \ \
| | | |
| | | | |
SI-8525 -Xlint:nowarn-missing-interpolator
|
| | | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Turn anonymous references to `settings.lint` into named settings.
After that, trust to Adriaan to make them filterable.
There are a few remaining top-level -Y lint warnings that are
deprecated.
|
| | | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Turn off lint warnings with negating prefix, and add a lint-only
warning for the infamously nagging "Did you forget the interpolator?"
That message is made more dignified.
Without `-Xlint:false`, there is no mechanism to turn off
anonymous linters once `-Xlint` is selected.
|
| | | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Make -Xlint a "multichoice" option for purposes of option parsing.
This allows turning on "lint with these warnings" instead of only
"turn off these warnings but enable other lint warnings".
```
$ scalac -Xlint:warn-adapted-args linty.scala # lint plus a warning
$ scalac -Xlint warn-adapted-args linty.scala # same
$ scalac -Xlint linty.scala # same as now
$ scalac -Xlint -- linty.scala # ok, not necessary
$ scalac -Xlint _ -- linty.scala # another funky underscore
```
This would also enable Xlint options that are not standalone options,
although that is not implemented in this commit. For example,
`-Xlint:no-missing-interpolator` could be used to disable that
warning. (There is no `-Xoption:flavor=off` syntax.) (`no-` switches
would not be enabled by `_`.)
|
| |\ \ \ \
| | | | |
| | | | | |
SI-8557 make scaladoc normalize paths of external jars.
|
| | | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
Scaladoc compares (string representations of) the paths from
-doc-external-doc and the paths form `sym.underlyingSource`.
We now normalize on both ends before comparing them.
|
| |\ \ \ \ \
| | | | | |
| | | | | | |
SI-5919 TypeTags and Exprs should be serializable
|
| | | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | | |
- Make TypeCreator and TreeCreator extend Serializable.
- When replacing a SerializedTypeTag with a TypeTag or WeakTypeTag,
do not use scala.reflect.runtime.universe.rootMirror, since
it is unlikely to find user classes; instead, create a runtime
mirror using the context ClassLoader of the current thread.
Use the same logic for SerializedExpr.
- Remove writeObject/readObject methods from SerializedTypeTag
and SerializedExpr since they are unused.
- Add @throws annotation on writeReplace and readResolve methods.
- Handle SecurityException if the current thread cannot access the
context ClassLoader.
- To make type tags of primitive value classes serializable, make
PredefTypeCreator a top-level class. Otherwise, it would
retain a reference to the enclosing Universe,
rendering the TypeCreator non-serializable.
Binary compatibility:
- Keep nested PredefTypeCreator class to avoid backward binary
incompatible change.
- Keep `var` modifiers on the class parameters of
SerializedTypeTag for backward binary compatibility.
- Adds filter rules to forward binary compatibility whitelist:
- `TypeCreator`, `PredefTypeCreator`, and `TreeCreator` must now
extend from `Serializable`.
- Must have new class `scala.reflect.api.PredefTypeCreator`
to avoid problematic outer reference.
|
| |\ \ \ \ \ \
| | | | | | |
| | | | | | | |
SI-8703 add support for blocks with just a single expression to quasiquotes
|
| | | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
Previously it was impossible to match a block that was constructed as
Block(Nil, term)
Due to the fact that quasiquotes always flatten those into just term. This is
a correct behaviour for construction (for sake of consistency with parser) but
doing it in deconstruction mode make it impossible to match such blocks which
could have been constructed manually somewhere.
To fix this we just disable block flattening in deconstruction mode.
Interestingly enough this doesn't break existing code due to the fact that
quasiquote's block matcher also matches expressions as single-element blocks.
This allows to match single-element blocks with patterns like q"{ $foo }".
|
| |\ \ \ \ \ \ \
| | | | | | | |
| | | | | | | | |
relaxes attachment-matching rules
|
| | | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
It came as a surprise recently, but attachments.contains/get/update/remove
require the class of the payload to match the provided tag exactly, not
taking subclassing into account. This commit fixes the oversight.
|
| |\ \ \ \ \ \ \ \
| |_|_|/ / / / /
|/| | | | | | | |
prevents c.internal.subpatterns from destroying rangeposes
|
| | |/ / / / / /
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
This commit continues the work started in fcb3932b32.
As we've figured out the hard way, exposing internally maintained trees
(e.g. macro application) to the user is dangerous, because they can mutate
the trees in place using one of the public APIs, potentially corrupting our
internal state.
Therefore, at some point we started duplicating everything that comes from the user
and goes back to the user. This was generally a good idea due to the reason
described above, but there was a problem that we didn't foresee - the problem
of corrupted positions.
It turns out that Tree.duplicate focuses positions in the tree being processed,
turning range positions into offset ones, and that makes it impossible for macro
users to make use of precise position information.
I also went through the calls to Tree.duplicate to see what can be done
to them. In cases when corruptions could happen, I tried to replace duplicate
with duplicateAndKeepPositions.
Some notes:
1) Tree rehashing performed in TreeGen uses duplicates here and there
(e.g. in mkTemplate or in mkFor), which means that if one deconstructs
a macro argument and then constructs it back, some of the positions in
synthetic trees might become inaccurate. That's a general problem with
synthetic trees though, so I don't think it should be addressed here.
2) TypeTree.copyAttrs does duplication of originals, which means that
even duplicateAndKeepPositions will adversely affect positions of certain
publicly accessible parts of type trees. I'm really scared to change this
though, because who knows who can use this invariant.
3) Some methods that can be reached from the public API (Tree.substituteXXX,
c.reifyXXX, c.untypecheck, ...) do duplicate internally, but that shouldn't be
a big problem for us, because nothing is irreversibly corrupted here.
It's the user's choice to call those methods (unlike with TypeTree.copyAttrs)
and, if necessary, they can fixup the positions themselves afterwards.
4) Macro engine internals (macro impl binding creation, exploratory typechecking
in typedMacroBody) use duplicate, but these aren't supposed to be seen by the user,
so this shouldn't be a problem.
5) Certain parser functions, member syntheses and typer desugarings also duplicate,
but in those cases we aren't talking about taking user trees and screwing them up,
but rather about emitting potentially imprecise positions in the first place.
Hence this commit isn't the right place to address these potential issues.
|
| |\ \ \ \ \ \ \
| | | | | | | |
| | | | | | | | |
Fix -Yno-adapted-args, it would just warn, not err
|
| | | | | | | | | |
|
