Revert inliner to work on typed trees #5138

odersky · 2018-09-21T15:20:41Z

This PR removes functionality to inline typed trees, and also provides better help to tracking arguments for staging.

allanrenucci · 2018-09-21T16:00:20Z

test performance please

dottybot · 2018-09-21T16:01:45Z

performance test scheduled: 1 job(s) in queue, 1 running.

dottybot · 2018-09-21T18:43:12Z

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/5138/ to see the changes.

Benchmarks is based on merging with master (20ad4a0)

odersky · 2018-09-23T09:34:52Z

test performance please.

dottybot · 2018-09-23T09:36:24Z

performance test scheduled: 1 job(s) in queue, 0 running.

dottybot · 2018-09-23T10:46:23Z

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/5138/ to see the changes.

Benchmarks is based on merging with master (a0b429b)

Rename `tree`, `trees` and associated methods in `ClassSymbol` to `rootTree`, `rootTrees`. We want to add another tree field to symbols that points directly to the symbol's definition tree. So we should be more specific about `rootTree`.

Previous scheme did not work if fixed completer was subsequently overridden by another completer. The new scheme goes directly to the places where parents are computed in various completers.

Normalizing via simplification is not enough since sometimes we need to "reach into" a pre-simplified type via the `isMatchedBy` method of a SelectionProto.

Match-pattern bound variables should not assume to match all other types, even if TypeVarsMissContext is true.

While we transition from rewrite matches to staging, we should not be dependent on rewrite methods for tuples. So for now, all tuple operations are dynamic.

- Fix isUptoDate conditon - Include TypeVars in footprint

Since we don't support rewrite matches and implicit matches in their current form, some tests need to be reclassified. They are moved to invalid. Maybe we can bring them back later as staging tests.

XmlQuote broke since proxies work now differently.

Analyzing tuples2 has shown that there's a quadratic blowup due to the occursIn check in ConstraintHandling#approximate. This change avoids traversing types for occursIn as long as they are ground.

Track proxies of inline arguments with a flag. Note: Some of the optimizations of the inliner don't update the defTree of the argument symbol even if they change the definition. But most of these optimizations will go away anyway. We should review once the inliner stabilizes.

- Remove unnecessary inline implicit conversion to `SCOps` - Use Symbol::fullName instead of tree extractors

The optimization was to disregard a prefix of a named type if its symbol is static and none of the mapped symbols in `from` is static. This is incorrect, since the prefix might be a non-static reference to a static object. In that case the prefix still has to be considered. i2895a.scala contains code that failed when running with the cbn implementation of `assert` in the next commit.

This generates more efficient code by inlining the argument expression into the if-then-else.

Align position of binding and position of its defined symbol. This avoids some test-pickler errors observed with the new inliner.

odersky · 2018-09-25T11:24:50Z

test performance please.

dottybot · 2018-09-25T11:25:35Z

performance test scheduled: 1 job(s) in queue, 0 running.

dottybot · 2018-09-25T12:34:44Z

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/5138/ to see the changes.

Benchmarks is based on merging with master (a4b60c4)

Needs a revised Inlined pickling format

The change accidentally part of a previous commit

nicolasstucki · 2018-09-26T12:11:21Z

So far it looks good. Are there more things that will go into this PR? I would suggest merging as is as would unblock a couple of other PRs (the retaining tree part).

allanrenucci · 2018-09-26T12:21:22Z

compiler/src/dotty/tools/dotc/tastyreflect/TreeOpsImpl.scala

  def TermDeco(term: Term): TermAPI = new TermAPI {
    def pos(implicit ctx: Context): Position = term.pos
    def tpe(implicit ctx: Context): Types.Type = term.tpe
+    def underlyingArgument(implicit ctx: Context): Term = {


Maybe make this part of the scala.quoted.Expr API as suggested by @nicolasstucki initially

No, this is the correct way to do it. If you do not inspect the tree, you should not know about the implementation details. I mean when using purely generative macros.

Sorry, I meant as an extension method like toTasty:
https://github.com/lampepfl/dotty/blob/06738295d54f1e945becf89dc809b3ff923dc57a/compiler/src/dotty/tools/dotc/tastyreflect/QuotedOpsImpl.scala#L11

So not visible when doing only purely generative macros

Oh yes. This way is more flexible as it does not only work for the parameters but for any term you might have inside the tree.

compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala

scala deleted a comment from odersky Sep 21, 2018

odersky force-pushed the revert-inliner branch from 7a6a5c7 to 4cc3e32 Compare September 22, 2018 16:52

odersky force-pushed the revert-inliner branch from 1832dc2 to 167c945 Compare September 24, 2018 10:12

allanrenucci added this to the 0.10 Tech Preview milestone Sep 24, 2018

odersky force-pushed the revert-inliner branch from f0e49b8 to a5d1802 Compare September 24, 2018 15:31

biboudis mentioned this pull request Sep 24, 2018

Unexpected tree could not be interpreted #5097

Closed

odersky added 15 commits September 24, 2018 18:17

Rename tree -> rootTree

ea9d931

Rename `tree`, `trees` and associated methods in `ClassSymbol` to `rootTree`, `rootTrees`. We want to add another tree field to symbols that points directly to the symbol's definition tree. So we should be more specific about `rootTree`.

Fix tests

c18d8e8

Add defTree field to trees

5db3c5d

More robust scheme to add tuple members

ea29eaf

Previous scheme did not work if fixed completer was subsequently overridden by another completer. The new scheme goes directly to the places where parents are computed in various completers.

Normalize match types when computing members

10e4494

Normalizing via simplification is not enough since sometimes we need to "reach into" a pre-simplified type via the `isMatchedBy` method of a SelectionProto.

Refine TypeVarsMissContext for MatchTypes

2493ae2

Match-pattern bound variables should not assume to match all other types, even if TypeVarsMissContext is true.

Make Tuple ops dynamic only

3aed2e4

While we transition from rewrite matches to staging, we should not be dependent on rewrite methods for tuples. So for now, all tuple operations are dynamic.

Bring back typed inliner

ceef996

Fix matchtype redux caching

f68e3be

- Fix isUptoDate conditon - Include TypeVars in footprint

Reclassify tests

5cd998f

Since we don't support rewrite matches and implicit matches in their current form, some tests need to be reclassified. They are moved to invalid. Maybe we can bring them back later as staging tests.

Reclassify more tests

0cdb63d

XmlQuote broke since proxies work now differently.

Optimize occursIn

b19ea47

Analyzing tuples2 has shown that there's a quadratic blowup due to the occursIn check in ConstraintHandling#approximate. This change avoids traversing types for occursIn as long as they are ground.

Fix isGround logic

d1ce179

Add underlyingArgument to Tasty reflection.

2dbec65

allanrenucci force-pushed the revert-inliner branch from a5d1802 to c553b8d Compare September 24, 2018 16:45

Simplify XML interpolator implementation

b2cc163

- Remove unnecessary inline implicit conversion to `SCOps` - Use Symbol::fullName instead of tree extractors

allanrenucci force-pushed the revert-inliner branch from c553b8d to b2cc163 Compare September 24, 2018 16:56

odersky added 4 commits September 25, 2018 13:05

Make the condition of assert a call-by-name argument.

a0a184e

This generates more efficient code by inlining the argument expression into the if-then-else.

Set positions of inline bindings

6eb54f8

Align position of binding and position of its defined symbol. This avoids some test-pickler errors observed with the new inliner.

Drop TransparentTyper

f6c440f

odersky force-pushed the revert-inliner branch from 0742f28 to f6c440f Compare September 25, 2018 15:18

odersky added 2 commits September 25, 2018 17:47

Drop more bits from inliner

e2d8300

Drop InlineIf and InlineMatch

8ae9d6a

odersky force-pushed the revert-inliner branch from f321394 to 8ae9d6a Compare September 25, 2018 21:17

odersky added 4 commits September 26, 2018 08:23

Drop UntypedSplice

1ecfdae

Drop implicit matches

12862de

Drop untyped Tasty

432d471

Drop EMPTYTREE tag in Tasty

4118884

Needs a revised Inlined pickling format

odersky added the stat:needs review label Sep 26, 2018

Store post-typer transformed trees in defTree

241122e

odersky mentioned this pull request Sep 26, 2018

Add defTree field to symbols #5131

Closed

Revert Config option

8fd21f7

The change accidentally part of a previous commit

nicolasstucki self-requested a review September 26, 2018 11:59

nicolasstucki approved these changes Sep 26, 2018

View reviewed changes

nicolasstucki assigned odersky Sep 26, 2018

allanrenucci reviewed Sep 26, 2018

View reviewed changes

Address rveiew comment

0b8ffaf

nicolasstucki approved these changes Sep 26, 2018

View reviewed changes

odersky merged commit 90671f0 into scala:master Sep 26, 2018

allanrenucci deleted the revert-inliner branch September 26, 2018 14:27

Revert inliner to work on typed trees #5138

Revert inliner to work on typed trees #5138

Uh oh!

Conversation

odersky commented Sep 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

allanrenucci commented Sep 21, 2018

Uh oh!

dottybot commented Sep 21, 2018

Uh oh!

dottybot commented Sep 21, 2018

Uh oh!

odersky commented Sep 23, 2018

Uh oh!

dottybot commented Sep 23, 2018

Uh oh!

dottybot commented Sep 23, 2018

Uh oh!

odersky commented Sep 25, 2018

Uh oh!

dottybot commented Sep 25, 2018

Uh oh!

dottybot commented Sep 25, 2018

Uh oh!

nicolasstucki commented Sep 26, 2018

Uh oh!

allanrenucci Sep 26, 2018

Choose a reason for hiding this comment

Uh oh!

nicolasstucki Sep 26, 2018

Choose a reason for hiding this comment

Uh oh!

allanrenucci Sep 26, 2018

Choose a reason for hiding this comment

Uh oh!

nicolasstucki Sep 26, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

odersky commented Sep 21, 2018 •

edited

Loading