-
Notifications
You must be signed in to change notification settings - Fork 451
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
fix: explicitly initialize Std
in lean_initialize
#4668
Conversation
@@ -5,5 +5,6 @@ Authors: Markus Himmel | |||
-/ | |||
prelude | |||
import Std.Data.DHashMap.Basic | |||
import Std.Data.DHashMap.RawLemmas |
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.
@TwoFX Just checking, was orphaning the file intentional?
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.
Yes, the idea was that in the "normal" use case where you just want to use HashMap
and not HashMap.Raw
you can just import Std.Data.HashMap
and import only the files that are needed for HashMap
.
In fact, the same thing should be true for all three of DHashMap
, HashMap
and HashSet
, so I'm surprised that just changing this for DHashMap
made any difference.
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.
Ah, maybe it's because DHashMap.RawLemmas
has macros in it, unlike HashMap.RawLemmas
and HashSet.RawLemmas
?
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.
Yes, exactly. It's a limitation of the bootstrapping setup, if you will. But note that a Lake build would also skip orphan files.
Perhaps the import should be moved to Std.Data instead then? Unlikely people will import that and then complain that it contains too much stuff.
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.
Sounds good to me, I'll open a PR.
Fixes the stage 2 build, which runs with
prefer_native=true