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

More progress towards Alpha cleanup #849

Merged
merged 1 commit into from
Nov 14, 2023
Merged

More progress towards Alpha cleanup #849

merged 1 commit into from
Nov 14, 2023

Conversation

arthurpaulino
Copy link
Member

  • Get rid of unnecessary parsing tests, as src/parser/syntax.rs already contains the cases we want to cover
  • Finish up the migration of gadgets specific to Lurk data construction and deconstruction

Closes #494

Copy link
Collaborator

@porcuquine porcuquine left a comment

Choose a reason for hiding this comment

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

See comments.

@arthurpaulino arthurpaulino force-pushed the more-migrations branch 2 times, most recently from 9253717 to 6f96d94 Compare November 4, 2023 17:59
@arthurpaulino arthurpaulino force-pushed the more-migrations branch 2 times, most recently from f937580 to 1906714 Compare November 4, 2023 18:21
@huitseeker
Copy link
Member

You may want to make a mention of #494

@arthurpaulino
Copy link
Member Author

It's already set to close #494

@arthurpaulino arthurpaulino force-pushed the more-migrations branch 4 times, most recently from 2a91c45 to 7277446 Compare November 11, 2023 13:10
huitseeker
huitseeker previously approved these changes Nov 13, 2023
Copy link
Member

@huitseeker huitseeker left a comment

Choose a reason for hiding this comment

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

Nothing blocking, but I added ways to make this look a bit nicer inline.

src/coprocessor/gadgets.rs Outdated Show resolved Hide resolved
src/coprocessor/gadgets.rs Outdated Show resolved Hide resolved
src/coprocessor/gadgets.rs Outdated Show resolved Hide resolved
src/coprocessor/gadgets.rs Outdated Show resolved Hide resolved
src/coprocessor/gadgets.rs Outdated Show resolved Hide resolved
@arthurpaulino
Copy link
Member Author

arthurpaulino commented Nov 13, 2023

@huitseeker there's no API for AllocatedPtr::alloc_infallible

@huitseeker
Copy link
Member

@arthurpaulino my bad you're right. It's in #881 if you want it.

src/coprocessor/gadgets.rs Show resolved Hide resolved
src/coprocessor/gadgets.rs Outdated Show resolved Hide resolved
src/coprocessor/gadgets.rs Outdated Show resolved Hide resolved
huitseeker
huitseeker previously approved these changes Nov 14, 2023
Copy link
Member

@huitseeker huitseeker left a comment

Choose a reason for hiding this comment

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

Thanks a bunch! ✨

* Get rid of unecessary parsing tests

* Finish up the migration of gadgets specific to Lurk data construction and deconstruction
self.0
.get(&FWrap(f))
.ok_or_else(|| anyhow!("Global allocation not found for {}", f.hex_digits()))
.ok_or_else(|| SynthesisError::AssignmentMissing)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you!

Copy link
Collaborator

@porcuquine porcuquine left a comment

Choose a reason for hiding this comment

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

We may want to consider creating a real error to identify the case in which failure is due to 'lack of knowledge' — since this is likely to be an entirely real use case in eventual Lurk applications. Since 'knowledge is power', trying to evaluate (or prove) expressions to which (it turns out) one doesn't have the ability to dereference a pointer is conceptually equivalent to a kind of 'access denied'. It's true that this should lead to failure (i.e. proofs or evaluations cannot be created), but there are likely cases where this should be handled specially.

For example, it may be that when an agent tries to access privileged information, the user is prompted to explicitly grant access before continuing. (This is just a suggestive example, and one can imagine many more cases in which this kind of non-deterministic ignorance is either ephemeral or should be handled in a specific, non-fatal way.)

@arthurpaulino arthurpaulino added this pull request to the merge queue Nov 14, 2023
Merged via the queue into main with commit b65ded0 Nov 14, 2023
12 checks passed
@arthurpaulino arthurpaulino deleted the more-migrations branch November 14, 2023 19:09
huitseeker added a commit to huitseeker/lurk-rs that referenced this pull request Nov 16, 2023
huitseeker added a commit to huitseeker/lurk-rs that referenced this pull request Nov 16, 2023
huitseeker added a commit to huitseeker/lurk-rs that referenced this pull request Nov 16, 2023
huitseeker added a commit to huitseeker/lurk-rs that referenced this pull request Nov 16, 2023
github-merge-queue bot pushed a commit that referenced this pull request Nov 17, 2023
* fix: make z_data::serde private

* refactor: restrict visibility of CLI backend/config

* make circuit::data pub(crate)

* refactor: make gadgets from #849 explicitly unused

* make interpreter pub(crate)

* refactor: make top-level  expr, cont, hash, package private

* refactor: make ptr pub(crate)

* refactor: make symbol crate-private

* refactor: make syntax crate-private

* refactor: make tag crate-private

* refactor: make uint crate-private

* refactor: make writer crate-private

* refactor: limit visibility in public_params

* refactor: Refactor variant naming in public parameters module

- Renamed `Error::IOError` to `Error::IO` and `Error::CacheError` to `Error::Cache` across multiple files for consistency.

* fix: repair doctest
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.

Adapt parser tests
3 participants