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

Faithfully print with-kinds by reconstructing modalities for types #3410

Open
wants to merge 8 commits into
base: aspsmith/with-kind-modalities
Choose a base branch
from

Conversation

glittershark
Copy link
Member

@glittershark glittershark commented Dec 27, 2024

Reconstruct a more faithful representation of with-kinds when constructing outcome jkinds from Jkind.ts, using constant modalities on axes to omit types from axes which don't have those types in their baggage.

This approach is pretty dumb (we just walk the baggage of each axis building up a map tracking which types are mentioned on which axes), but from reading the existing test cases it seems to do a pretty good job of constructing the jkind that the user would have written down themselves - plus it's vastly better than the previous approach of just printing a kind that wouldn't even parse, let alone be correct.

Plausibly this approach would need to be rethought completely once we introduce non-constant/identity modalities, but those are far enough off that I claim this should serve us fine for the time being.

Review the whole diff, rather than individual commits.

@glittershark glittershark force-pushed the aspsmith/normalize-mode-axis-types branch from 4c2935f to f2abf37 Compare December 27, 2024 05:10
@glittershark glittershark force-pushed the aspsmith/normalize-mode-axis-types branch from f2abf37 to 3b9194f Compare December 27, 2024 15:28
@glittershark glittershark force-pushed the aspsmith/normalize-mode-axis-types branch from 3b9194f to cb9f1f9 Compare December 27, 2024 16:54
@glittershark glittershark force-pushed the aspsmith/normalize-mode-axis-types branch from cb9f1f9 to 7f571bb Compare December 27, 2024 16:58
@glittershark glittershark force-pushed the aspsmith/normalize-mode-axis-types branch from 7f571bb to e8ad948 Compare December 28, 2024 00:10
@glittershark glittershark force-pushed the aspsmith/untype-jkind branch 4 times, most recently from 1d38927 to c1b370e Compare December 28, 2024 03:09
@glittershark glittershark changed the title printing for with-kinds Faithfully print with-kinds by reconstructing modalities for types Dec 28, 2024
@glittershark glittershark marked this pull request as ready for review December 28, 2024 03:09
@glittershark
Copy link
Member Author

not quite sure what's going on with the test failure here; hopefully this is reviewable regardless?

@glittershark glittershark force-pushed the aspsmith/normalize-mode-axis-types branch from e8ad948 to ffeb5de Compare December 28, 2024 17:24
Per zqian's CR to this effect. note that this does not do any of the
simplification that this unlocks yet, that'll happen later
Now that the types for jkind axes and mode axes have been unified, we
can express the condition for "modality is const for axis" directly,
without resorting to a huge ugly pattern match on the axis and the
modality's atoms.
@glittershark glittershark force-pushed the aspsmith/normalize-mode-axis-types branch from ffeb5de to 6ab8dcd Compare December 28, 2024 20:54
@glittershark glittershark mentioned this pull request Dec 30, 2024
42 tasks
we need to sort types in a [with] because the order comes from iterating a
TypeHash, which is nondeterministic (specifically, the order is different if we
disable stack allocation) and brittle. Unfortunately, we lack a good comparison
for types, so as a hacky workaround here we format to a string (actually, a
string list for easier line breaking when pretty printing) and sort
lexicographically before finally printing.
Base automatically changed from aspsmith/normalize-mode-axis-types to aspsmith/with-kind-modalities January 6, 2025 11:02
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.

1 participant