Skip to content
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

[Probably dangerous?] Fixes printing argument tuple name in a tooltip #10461

Closed
wants to merge 5 commits into from

Conversation

En3Tho
Copy link
Contributor

@En3Tho En3Tho commented Nov 14, 2020

This tries to fix printing tuple name (#10441) like in this case:
before:
image
after:
image

or this:
before:
image
after:
image

or this:
before:
image
after:
image

Needs testing as it's goes way lower than Tooltip's creation

@En3Tho
Copy link
Contributor Author

En3Tho commented Nov 14, 2020

Also, I've found another bug in tooltips now with named tuple args:
with simple tuple they show:
image
with struct tuple they don't:
image

I think all these bugs are coming from NicePrint module using quite skewed compiler's typed tree, instead of something like "what human sees". In compiler's point of view ref tuple can be either a ref tuple or a group of parameters and struct tuple is always a single parameter, but I don't think information tooltips really need to respect this point of view. On the countrary, they shouldn't care about compiler low level details, they should only care about what human sees

@En3Tho
Copy link
Contributor Author

En3Tho commented Nov 14, 2020

Yep, already broke something that did not show on my machine because I was only using FCS, that's unfortunate. But well, kinda expected this kind of thing to happen.
Can anyone help me with this?

@cartermp
Copy link
Contributor

I think it has something to do with signature file generation based on the errors

@En3Tho
Copy link
Contributor Author

En3Tho commented Nov 15, 2020

@cartermp seems like all of those failed tests are Azure related if I'm not wrong.

@cartermp cartermp closed this Nov 16, 2020
@cartermp cartermp reopened this Nov 16, 2020
@cartermp
Copy link
Contributor

Yep, it tried to download stuff and computer says no

@cartermp cartermp closed this Jan 21, 2021
@cartermp cartermp reopened this Jan 21, 2021
@cartermp
Copy link
Contributor

Restarting CI to get fresh failures since the build was discarded by CI

@cartermp
Copy link
Contributor

@En3Tho this looks like it requires an updated baseline to proceed.

@En3Tho
Copy link
Contributor Author

En3Tho commented Jan 22, 2021

@cartermp Actually I think this one should be closed as it is dangerous. This arity thing, which I fixed here has special meaning for the compiler on the next stage. So it's kinda ok to have this line for FCS (I'm actually using it in my private build), but surely not for FSC. I think there should be another way to save variable name information. Also, this doesn't cover value tuples anyway

@cartermp
Copy link
Contributor

Sounds good.

@cartermp cartermp closed this Jan 22, 2021
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.

2 participants