-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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.
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
Extend E0308 description with dynamic dispatch example #70071
Conversation
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Hmm...it appears the error text is included in some ui tests. Will update tomorrow. |
☔ The latest upstream changes (presumably #70118) made this pull request unmergeable. Please resolve the merge conflicts. |
I'm unsure how to fix the numeric-suffix test, there appears to be an error in compiletest. |
@GuilliaumeGomez it appears that this uncovers some breakage in compiletest? Haven't debugged yet, because I didn't compile with debug info yet. |
☔ The latest upstream changes (presumably #70343) made this pull request unmergeable. Please resolve the merge conflicts. |
This pretty nice except for the huge weird bug. Here's what I suggest: change which error code explanation is tested in |
Seems...fine to test some other random error. What's the bug exactly? Something in compiletest? |
I must assume so. Probably some JSON message cut in transit. |
I don't see any cutting. Furthermore, Cargo and other tools rely on parsing the same JSON. |
If so, then the error is only uncovered by this change, not caused by it. But I'll try bisecting the problem. |
Hmm if you can reproduce locally, I suppose you can try removing parts of your message until it goes away? I suspect it's some sequence of characters that doesn't escape properly? |
Yeah, that's the plan. |
I have finally found the time to bisect the problem. The thing works up to the last example reduced to:
If I only add another |
cc @rust-lang/wg-diagnostics |
@llogiq Ping from triage. Needs a rebase here. I'd actually suggest trying and see if there's some way to finish the error code description without triggering this issue locally, and file a separate issue about this at the same time. |
The only thing that comes to mind is that multiline spans don't get fully rendered if they are longer than 7 lines... But I don't see how that would affect this. |
Closing this due to inactivity. |
r? @GuillaumeGomez