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

feat: add witnesses for function_const_removed #977

Merged
merged 2 commits into from
Oct 18, 2024

Conversation

CommanderStorm
Copy link
Contributor

@CommanderStorm CommanderStorm commented Oct 18, 2024

Related to #937

A better whitness would be the following, but I have no clue if this is possible and if yes, how.
I have tried a few things, but I am missing something. Maybe this would require a query.

add() // if len(arguments) == 0
add(...) // if len(arguments) >= 1

=> is the compromise okay of letting this be add(...) to save on adding a whitness-query just for this?

Copy link
Owner

@obi1kenobi obi1kenobi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice, thank you!

For the witness hint, it's totally fine to omit the function arguments and just use .... We're just trying to give the user a broad-strokes idea of what might go wrong. Whether function args are present or not isn't that important in this situation so ... is fine.

Down the line, we'll have stricter rules for actual compilable witnesses as opposed to hints. But that's something for us to tackle down the line.

src/lints/function_const_removed.ron Outdated Show resolved Hide resolved
test_outputs/witnesses/function_const_removed.snap Outdated Show resolved Hide resolved
@obi1kenobi obi1kenobi enabled auto-merge (squash) October 18, 2024 00:39
@obi1kenobi obi1kenobi merged commit 3a51c65 into obi1kenobi:main Oct 18, 2024
34 checks passed
@CommanderStorm CommanderStorm deleted the more-whitness-const branch October 18, 2024 01:13
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.

2 participants