Skip to content

SD-186 Fix positions in trait method bytecode#5296

Merged
lrytz merged 1 commit intoscala:2.12.xfrom
retronym:ticket/SD-186
Jul 22, 2016
Merged

SD-186 Fix positions in trait method bytecode#5296
lrytz merged 1 commit intoscala:2.12.xfrom
retronym:ticket/SD-186

Conversation

@retronym
Copy link
Member

@retronym retronym commented Jul 21, 2016

Concrete, non private methods in traits are translated into a static
method with an explicit $this parameter. After this translation,
the references to $this (subistuted for this in user written code)
where being positioned at the position of the method, which makes
debugging unpleasant.

This commit leaves the Ident($this) trees unpositioned. This is
analagous to what we do in the body of extension methods, which
is the other user of ThisSubstitutor.

It would be more correct to copy the position of each This
tree over to the substituted tree. That would let us set a breakpoint
on a line that only contained this. But in 99% of cases users
won't be able to spot the difference, so I've opted for the tried
and tested approach here.

Review by @lrytz

@retronym
Copy link
Member Author

#5296

Concrete, non private methods in traits are translated into a static
method with an explicit `$this` parameter. After this translation,
the references to `$this` (subistuted for `this` in user written code)
where being positioned at the position of the method, which makes
debugging unpleasant.

This commit leaves the `Ident($this)` trees unpositioned. This is
analagous to what we do in the body of extension methods, which
is the other user of `ThisSubstitutor`.

It would be more correct to copy the position of each `This`
tree over to the substituted tree. That would let us set a breakpoint
on a line that _only_ contained `this`. But in 99% of cases users
won't be able to spot the difference, so I've opted for the tried
and tested approach here.
@lrytz
Copy link
Member

lrytz commented Jul 22, 2016

LGTM

@lrytz lrytz merged commit ab83b26 into scala:2.12.x Jul 22, 2016
@adriaanm adriaanm added the 2.12 label Oct 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants