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

Fix for #976 to Ignore Unused Stock Variables #979

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

Conversation

c0rp3n
Copy link
Contributor

@c0rp3n c0rp3n commented Jun 9, 2024

Solution

This change looks to ignore stock variables that are not from the main source file.

Testing

Added two new compile tests for stock variables and arrays.

This change treats stock variables the same as public during when checking for unused variables.
@dvander
Copy link
Member

dvander commented Jun 9, 2024

This error is intentional. The warning should only elide on includes.

@c0rp3n
Copy link
Contributor Author

c0rp3n commented Jun 9, 2024

Yup, okay.

I will go to take a look tomorrow for whether the messages were present from includes.

Then, take another look at a code fix.

@A1mDev
Copy link

A1mDev commented Jun 10, 2024

Yes, there are messages from inclusions. #976

This changes to also check swhether the VarDecl is from the main source file or not.
Checking this seems easy enough with the new source file tracking.

If a variable is from the main source file even if it is marked with stock we shall now report a warning.

We could also ignore any unused variable that is not from the main file.
But I shall stick with only stock for now.

Testing

Extended out the test to cover both the pass and fail cases.
@c0rp3n
Copy link
Contributor Author

c0rp3n commented Jun 11, 2024

Updated to elide the warning on includes.
I could also look to have the same behaviour for #972 but they would likely want to share code.

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.

3 participants