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

Exported structs and globals are marked as unused #6250

Closed
Thunkar opened this issue Oct 8, 2024 · 6 comments · Fixed by #6258
Closed

Exported structs and globals are marked as unused #6250

Thunkar opened this issue Oct 8, 2024 · 6 comments · Fixed by #6258
Assignees
Labels
bug Something isn't working

Comments

@Thunkar
Copy link
Contributor

Thunkar commented Oct 8, 2024

Aim

#[abi(notes)]
global note_export: (Field, Field) = (123, 124);

#[abi(functions)] 
struct MyFnAbi {
    parameters: MyFnParameters,
    return_value: Field,
}

This code generates warnings for unused structs and globals

Expected Behavior

Being able to generate the exports without warnings being emitted

Bug

Warnings are generated

To Reproduce

Use the example above

Workaround

None

Workaround Description

No response

Additional Context

No response

Project Impact

None

Blocker Context

No response

Nargo Version

No response

NoirJS Version

No response

Proving Backend Tooling & Version

No response

Would you like to submit a PR for this Issue?

None

Support Needs

No response

@Thunkar Thunkar added the bug Something isn't working label Oct 8, 2024
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Oct 8, 2024
@Thunkar Thunkar changed the title exported structs and globals are marked as unused Exported structs and globals are marked as unused Oct 8, 2024
@TomAFrench
Copy link
Member

@asterite Are we marking structs as unused before macros run?

@TomAFrench
Copy link
Member

Ah, nvm. This is a different kind of exported.

@asterite
Copy link
Collaborator

asterite commented Oct 8, 2024

So is the rule that if there's #[abi(...)]` on top of a struct then it shouldn't warn if it's unused?

@TomAFrench
Copy link
Member

Yeah, this annotation marks that we should create a template for that struct type in the ABI. Similarly for globals, it puts the value of those in the ABI.

@asterite
Copy link
Collaborator

asterite commented Oct 9, 2024

In the example above, I guess MyFnParameters would also be considered used because it's inside a struct that's marked with #[abi]?

@TomAFrench
Copy link
Member

Yeah, we should mark anything reachable from these structs as used.

@asterite asterite self-assigned this Oct 9, 2024
github-merge-queue bot pushed a commit that referenced this issue Oct 9, 2024
# Description

## Problem

Part of #6250

## Summary

Also renames a method to have a clearer name.

## Additional Context

## Documentation

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
github-merge-queue bot pushed a commit that referenced this issue Oct 9, 2024
# Description

## Problem

Resolves #6250

## Summary

## Additional Context


## Documentation

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants