Skip to content

Remove static local variables from Mesh::hasBndry*Y#3209

Merged
ZedThree merged 1 commit into
nextfrom
remove-some-static-locals
Nov 24, 2025
Merged

Remove static local variables from Mesh::hasBndry*Y#3209
ZedThree merged 1 commit into
nextfrom
remove-some-static-locals

Conversation

@ZedThree
Copy link
Copy Markdown
Member

Fixes #3160

Didn't strictly need to make these virtual, but given they need to be calculated
after load, this was probably the easiest way of achieving this

Fixes #3160

Didn't strictly need to make these virtual, but given they need to be calculated
after `load`, this was probably the easiest way of achieving this
Copy link
Copy Markdown
Contributor

@dschwoerer dschwoerer left a comment

Choose a reason for hiding this comment

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

Should we add some checks to ensure that the result is valid, before we return it?
I would be fine with it being CHECK4() - but that might catch a future use before it is set.

Without being virtual, this function should get inlined by the compiler. Do you see a use where it is not sufficient to set has_boundary_upper_y?

edit: we would need to move that from BoutMesh to Mesh - which seems fine?

@ZedThree
Copy link
Copy Markdown
Member Author

Well, it could only get inlined with LTO as the definition was in the .cxx file, so there's not much difference there.

I'm not sure what check would be required? If we've called Mesh::load, then it's valid -- otherwises, the whole mesh is invalid. Ideally, we'd move Mesh::load into the constructor -- that might actually be possible now.

Copy link
Copy Markdown
Contributor

@dschwoerer dschwoerer left a comment

Choose a reason for hiding this comment

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

Now that the heavy lifting is in the load part, it could make sense to put it in the header.

It certainly does not make things worse, so I am happy to merge it as it, I just thought if we allow for more inlineing, that would be a good thing.

@ZedThree
Copy link
Copy Markdown
Member Author

Yes, I agree! The mesh situation is a bit of mess, we could remove the subclass, and just have Mesh, although there are people looking at doing things with the mesh that might involve a second subclass

@ZedThree ZedThree merged commit 182975e into next Nov 24, 2025
27 checks passed
@ZedThree ZedThree deleted the remove-some-static-locals branch November 24, 2025 09:33
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