-
Notifications
You must be signed in to change notification settings - Fork 169
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
Replace -type-in-type
with Export Unset Universe Checking
in UniMath.Foundations.Init
#1697
base: master
Are you sure you want to change the base?
Replace -type-in-type
with Export Unset Universe Checking
in UniMath.Foundations.Init
#1697
Conversation
@peterlefanulumsdaine thanks a lot for preparing this. There is a warning about deprecated "Export Set" - should we address this before merging? |
I think we should be very careful about merging this. This will effectively enable In particular since we have no way of informing projects that rely on UniMath that we are making this change in their projects. We do not know every user of UniMath, and any problems related to this will only cause headaches for them, not us unfortunately. (Imagine spending hours trying to figure out why your project suddenly has a flat universe hierarchy, despite you being sure that you don't have I like the idea of taking steps towards removing |
This warming is easy to address, btw, just use |
Why not add |
@m-lindgren I feel fairly mixed about this. I agree with your reservations to some extent, but two counter-points:
These are why in the long-term, I don’t think the change is negative. In the short-term I fully agree it’s not great to affect downstream users without notification — but I’m not sure how we can notify them, other than trying this out. So I’d lean to merging this (if others agree) but thinking of it as provisional for a while (at least a month, say), and being willing to roll it back if downstream users report any problems. As long as we don’t merge #1671 (which is blocked anyway if we don’t merge this), I don’t think anything else is likely to interact with this and make it difficult to roll back. |
I really like this solution. I think it addresses Michael’s comments and it properly indicates where type-in-type is used, which is useful for removing the dependency on type-in-type. |
I didn’t have time to carry this through (I tried something similar just far enough yesterday to see it would definitely take more time than I had), but I agree this would be ideal, as long as the number of files that need it isn’t too large. |
What about something like (assuming no spaces in names): debug=1 # change to debug="" to disable waiting on user input
while ! make -k >log 2>&1; do
files="$(grep -B1 "^Error" log | grep -o '^File "[^"]*' | sed 's/^File "//g')"
echo "Errors in $files"
if [ ! -z "$debug" ]; then echo "Press enter to continue, ctrl+c to break..."; read dummyvar; fi
for f in $files; do
echo "editing $f"
if grep -q '#.local. Unset Universe Checking' "$f"; then
echo "File $f requires manual intervention"
exit 1
fi
cp -f "$f" "$f.tmp"
{ echo "#[local] Unset Universe Checking."; cat "$f.tmp"; } > "$f"
done
done or as a one-liner: debug=1; while ! make -k >log 2>&1; do files="$(grep -B1 "^Error" log | grep -o '^File "[^"]*' | sed 's/^File "//g')"; echo "Errors in $files"; if [ ! -z "$debug" ]; then echo "Press enter to continue, ctrl+c to break..."; read dummyvar; fi; for f in $files; do echo "editing $f"; if grep -q '#.local. Unset Universe Checking' "$f"; then; echo "File $f requires manual intervention"; exit 1; fi; cp -f "$f" "$f.tmp"; ( ( echo "#[local] Unset Universe Checking."; cat "$f.tmp") > "$f"); done; done Once you set |
@JasonGross Thanks very much for the script — it came through just as I was starting to scratch my head over putting together such a script myself. Works like a charm, no syntax errors. I’m trying it out in a branch |
This addresses #1696 : it replaces the use of the
-type-in-type
flag (in the Makefile, emacs local variables file, etc) with the commandExport Unset Universe Checking
inUniMath.Foundations.Init
.As discussed in detail in #1696 , these should be entirely equivalent in their current effect within UniMath; the two effective differences are:
Set Universe Checking
after importing UniMath.Compared to f917518 in #1671 , I’ve used
Export
rather thanGlobal
— according to the guidance in the Coq Reference Manual here and here, that seems to me like the correct one.