Skip to content

TreeCopier doesn't preserve attachments #4291

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
Duhemm opened this issue Apr 11, 2018 · 3 comments
Closed

TreeCopier doesn't preserve attachments #4291

Duhemm opened this issue Apr 11, 2018 · 3 comments
Assignees
@smarter @odersky
Labels
itype:question

Comments

@Duhemm
Copy link
Contributor

Duhemm commented Apr 11, 2018

While working on embedding the doc in Tasty, I noticed that the TreeCopier subclasses don't copy over the attachments of the trees.

Is this on purpose?

Doc comments are stored as attachments on MemberDef trees, and cannot be recovered in the REPL for instance, because they're discarded when the trees are modified.
@Duhemm
Copy link
Contributor Author

Duhemm commented Apr 11, 2018

It looks like they're not copied over on purpose: it appears that copying them breaks a lot of things.

Should the docs be stored somewhere else that will not be lost down the line? Should we only copy the doc comment in TreeCopier?

@smarter
Copy link
Member

smarter commented Apr 11, 2018 •
edited
Loading

It looks like they're not copied over on purpose: it appears that copying them breaks a lot of things.

What kind of failures are you getting?

Should the docs be stored somewhere else that will not be lost down the line? Should we only copy the doc comment in TreeCopier?

Maybe we need two kinds of attachments: sticky and non-sticky.

@Duhemm
Copy link
Contributor Author

Duhemm commented Apr 11, 2018

@smarter Here's sbt's log for dotty-compiler/testOnly dotty.tools.dotc.CompilationTests -- --tests=compilePos:
https://gist.github.com/Duhemm/5eff50bcf9981aa941773d252e22e8eb

Maybe we need two kinds of attachments: sticky and non-sticky.

Sounds good! I'll look into that.

Duhemm added a commit to dotty-staging/dotty that referenced this issue Apr 11, 2018
@Duhemm
Fix scala#4291: Copy sticky attachments in TreeCopier
4062073 
Sticky attachments are attachments whose key is a subclass of
`StickyKey`. Those attachments should be copied when a tree is copied
using a `TreeCopier`.

Fixes scala#4291
@Duhemm Duhemm mentioned this issue Apr 11, 2018
Merged
odersky added a commit that referenced this issue Apr 27, 2018
@odersky
Merge pull request #4292 from dotty-staging/fix/4291
e515bd0 
Fix #4291: Copy sticky attachments in `TreeCopier`
Duhemm added a commit to dotty-staging/dotty that referenced this issue May 23, 2018
@Duhemm
Revert "Fix scala#4291: Copy sticky attachments in TreeCopier"
5e03451 
This reverts commit 4062073.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@smarter smarter

@odersky odersky

Labels
itype:question
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@smarter @odersky @Duhemm