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

Fixing a range of compiler warnings for shadow/unused variables #77

Closed

Conversation

hughcars
Copy link
Collaborator

@hughcars hughcars commented Jul 25, 2023

Description of changes:

  • Turned on some extra warnings (Wshadow) whilst hunting a bug on another branch, there were a few extra errors here that produced some noise. This PR resolves them, I don't believe there were any correctness issues, due to type resolution picking the correct variable in many cases.
  • The change to SurfacePostOperator is the most significant. The substructs were in error marking the GetCoefficient arguments as shadowing mat_op and local_to_shared. Given these subtypes were already being leaked through the accessor methods for their containers, I just moved the types in to the palace namespace.
  • In a few other spots, the ambiguity was resolved by switching to no argument or static methods, and in others by changing a local variable name.

- Most of the OOP/functional clashes were resolved by static where possible, or a free function
- SurfacePostOperator involved breaking out private struct definitions into the palace namespace
these struct definitions had already bled into the global namespace through the use of accessor functions.
- Source of the issue being methods of internal structs believing mat_op and local_to_shared were mirroring
the containing classes variables, despite this being impossible.
@hughcars hughcars force-pushed the hughcars/fix-shadow-and-unused-variable-warnings branch from b8aacb7 to d70060b Compare July 25, 2023 20:59
@hughcars hughcars closed this Sep 7, 2023
@hughcars
Copy link
Collaborator Author

hughcars commented Sep 7, 2023

Closed as the code base has moved so much underneath that all these have been addressed.

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.

1 participant