Skip to content

Conversation

@Aatch
Copy link
Contributor

@Aatch Aatch commented Jul 8, 2013

Revert the removal of the @ from Ty as I think it caused the memory regression seen here in #7615. The graph posted on that PR is from before this change.

Since Ty is a recursive type, I think that the change from @ to ~ meant that a copy that would previously stop at the @ continued. It seems to me that during syntax folding, the data was being kept around somehow, causing the bloat.

This reverts commit 47eca21.

@thestinger
Copy link
Contributor

@bors: retry

@graydon
Copy link
Contributor

graydon commented Jul 18, 2013

From @cmr on the mailing list: "@Aatch said it is broken on Windows with no obvious fix besides de-sharing the rest of the AST"

How so? Why would this be platform specific?

@Aatch
Copy link
Contributor Author

Aatch commented Jul 19, 2013

@graydon, I think this is some mis-communication.

The error that keeps arising is platform specific, only appearing on the windows bot. I don't get any similar error here and have no idea what might cause it.

The fix of completely removing @ from the AST is what I described as the "proper" way to fix it. It's very hard however, while not much code relies on the sharing of @ some does, and it's enough to make completely removing @ very difficult.

@graydon
Copy link
Contributor

graydon commented Jul 19, 2013

Sure, I'm just curious why there's anything platform specific in here at all.

@graydon
Copy link
Contributor

graydon commented Jul 27, 2013

continued in #8072

@graydon graydon closed this Jul 27, 2013
flip1995 pushed a commit to flip1995/rust that referenced this pull request Sep 8, 2021
Rustup r? `@ghost` changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

5 participants