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

Erase functions to fun #3661

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Erase functions to fun #3661

wants to merge 4 commits into from

Conversation

gebner
Copy link
Contributor

@gebner gebner commented Jan 8, 2025

Currently all erasable terms are erased to unit. This changes the non_informative function to return an option term instead of bool. This optional term specifies what the type should be erased to. For example, non_informative `(unit -> prop) returns Some `(fun _ -> ()).

There are no user-facing changes, you still write [@@erasable] val foo ... as before. This PR also makes the must_erase_for_extraction attribute a (deprecated) alias of erasable.

Fixes #3366. (only when --cmi is enabled, but we're planning to make that the default anyhow)

@gebner
Copy link
Contributor Author

gebner commented Jan 9, 2025

Check-world found two interesting regressions:

  1. The Inria folks like to treat warnings as errors, which breaks now because the warning number changed.
  2. The C code produced by Karamel in merkle-tree doesn't compile due to incorrect function names (?!):
MerkleTree.c: In function 'mt_init_hash':
MerkleTree.c:86:10: error: implicit declaration of function 'MerkleTree_Low_Hashfunctions_init_hash'; did you mean 'MerkleTree_Low_Hashfunctions_free_hash'? [-Werror=implicit-function-declaration]
   86 |   return MerkleTree_Low_Hashfunctions_init_hash(hash_size);
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |          MerkleTree_Low_Hashfunctions_free_hash
MerkleTree.c:86:10: error: returning 'int' from a function with return type 'uint8_t *' {aka 'unsigned char *'} makes pointer from integer without a cast [-Werror=int-conversion]
   86 |   return MerkleTree_Low_Hashfunctions_init_hash(hash_size);
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

@TWal
Copy link
Contributor

TWal commented Jan 9, 2025

I guess I am the Inria folk who like to treat warning as errors (sorry!), I can disable some more warnings if needed. I am doing this mainly because I was annoyed by the many warnings that shouldn't be warnings (e.g. #2705), but also because I think it's best practice (e.g. to ensure we don't have the proof splitting warning sprinkled throughout the project)

@gebner
Copy link
Contributor Author

gebner commented Jan 9, 2025

Sorry, that wasn't meant negatively at all! The F* library has lots of warnings too, and I agree it would be nicer to avoid them in the first place.

@TWal
Copy link
Contributor

TWal commented Jan 9, 2025

No worries, didn't take that negatively!

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.

Incorrect erasure of function types
2 participants