-
Notifications
You must be signed in to change notification settings - Fork 63
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
Deprecate llvm_struct #2183
base: master
Are you sure you want to change the base?
Deprecate llvm_struct #2183
Conversation
So, it seems a bunch of the s2n proofs use llvm_struct. Should we fix them, or put this off? (and if we put it off, what eventually changes? it seems unlikely we'll drop the s2n proofs entirely from the testing) |
While it would be nice to fix the s2n proofs to use |
Because when you mark something deprecated, you then have to use |
Oh wow, I was blissfully ignorant of this. That certainly feels like a strange design—I would argue it makes much more sense for seeing deprecated functions to be opt-out rather than opt-in.
I agree. Shall we open a separate issue about this? It feels as though we'd want |
It's more complicated than that because I don't think we want the existing not-visible deprecated functions to come back to life. Anyway I opened #2184 for this. |
acb5224
to
c9166e6
Compare
I force-pushed it to split the changes to prepare for deprecation (clearing out the uses of |
c9166e6
to
0faef8f
Compare
0faef8f
to
7d9d9af
Compare
I have rebased this on head again after merging #2193, which contained the cleanup part of what was previously in this changeset. (Now this branch has only the actual deprecation setting, which is going to change again after #2184 makes it possible to have warn-only deprecation, and the changelog entry.) |
Closes #2159.