-
Notifications
You must be signed in to change notification settings - Fork 716
Introduce PolyFlags to gather flags for polymorphic defs #21419
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
Conversation
7814cae to
83d9877
Compare
yannl35133
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
General comments:
- Is the future umbrella term not going to be Universe Polymorphism? I believe the structure should be called UnivPolyFlags rather than SortPolyFlags
- There are a few
SortPolyFlags.of_polyand a fewSortPolyFlags.make ~level_polymorphic:poly ~sort_polymorphic:false ~cumulative:false, why such a difference ?
I would also like to explain it like this: A universe is a sort and an optional universe level
They should all be |
Just say "level polymorphism". |
|
Thanks for the feedback. So I will correct the little mistakes here and there and:
Please +1 this message or state your disagreement :) |
Why not as an abstract type in UState? I'm still not a fan of having to write |
Either you remove the alias or you consider it as more than an alias and you properly document the fact. |
|
What are the flags when doing |
You prefer to write |
|
@SkySkimmer just a |
|
@SkySkimmer Also, any idea about the artifact errors on the CI? |
and the stuff in top_printers.ml / top_printers.dbg to get it automatically in ocamldebug |
no |
Adapt the whole code base to using this instead of the multiple poly/sort_poly/cumulative flags. poly_flags -> poly PolyFlags.of_poly -> of_level_poly sort_polymorphic -> implicit_sort_polymorphic Implement suggested fixes from PR level_polymorphic -> univ_polymorphic implicit_sort_polymorphic -> collapse_sorts_to_type univ_polymorphic -> univ_poly cleaner parsing of attributes (cumulative and collapse_sort_variables are just not settable without universe polymorphism). Fixed comments Apply suggestions from code review Spacing/indentation Co-authored-by: yannl35133 <40719961+yannl35133@users.noreply.github.com> Fix wrong poly kind used for inductive and record decls Remove options that are not supported yet Add Changelog entry PolyFlags.assumption_or_definition -> PolyFlags.construction_kind Support the polymorphism attributes in Hint Rewrite Change plugin_tutorial to use the forward-compatible poly_def attribute Add overlays Fix rewrite rule integration of polyflags as per Y. Leray's comments Add debug printer
f8a513b to
e555445
Compare
|
I added the debug printer stuff now |
|
I don't think this needs another full ci |
|
@coqbot run full ci |
|
@coqbot merge now |
|
@ppedrot: Please take care of the following overlays:
|
Adapt the whole code base to use this instead of the multiple poly/"sort_poly"/cumulative flags. Actually the sort_poly flag is renamed collapse_sort_variables as it is the only thing it will govern. We use
PolyFlags.univ_polyto check for universe polymorphism (= sort and level polymorphism).Consistently name the argument
~poly:PolyFlags.t. Also adds an attributepoly : PolyFlags.construction_kind -> PolyFlags.t attributethat decides depending on the construction (assumption, definition or inductive) which flags are supported and how they should be set. Subsequent PRs will add new options for cumulativity of assumptions and definitions (in this PR they are still unsupported as before), and will usecollapse_sort_variablesto govern implicit sort polymorphism (do metavariable sorts default toType(the current default) or not).These flags are not used in all places they are passed yet, but will be in the upcoming PRs around sort-elim/univ-poly/variances and cumulativity. The goal is just to do a functionality-preserving API change here, that should reduce conflicts down the road for these feature PRs, with this one requiring simple overlays.
In particular this is a prerequisite for @TDiazT 's subsequent PRs on adding sort elaboration.