-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Remove using namespace libMesh #28842
base: next
Are you sure you want to change the base?
Conversation
1eb640f
to
8018a7f
Compare
a4a4951
to
2a2f71f
Compare
18448c7
to
5c63179
Compare
b2c1d40
to
cc51284
Compare
framework/include/base/Moose.h
Outdated
#include "libmesh/perf_log.h" | ||
#include "libmesh/libmesh_common.h" | ||
#include "XTermConstants.h" | ||
#include "libmesh/utility.h" // to declare namespace |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self-note to remove this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You want it just for the namespace declaration? You can just put namespace libMesh {}
in MOOSE code to get that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup! I ended up figuring this out later on
Job Documentation, step Docs: sync website on 340196c wanted to post the following: View the site here This comment will be updated on new commits. |
Job Coverage, step Generate coverage on 340196c wanted to post the following: Framework coverage
Modules coverageHeat transfer
Level set
Navier stokes
Phase field
Porous flow
Ray tracing
Reactor
Richards
Solid mechanics
Stochastic tools
Thermal hydraulics
Xfem
Full coverage reportsReports
Warnings
This comment will be updated on new commits. |
All (needed with the huge concessions on the namespace) patches are merged this should be good to go. This will collect merge conflicts and unexpected failures (from new uses of objects that are not in the namespace) like no other PR so it'd be great to process it fast |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the very least needs a newsletter entry. this is pretty significant
Added more testing for nonstandard configurations. I suspect we might break a few apps with nonstandard configs (that aren't tested in on-moose app tests), but we can deal with that on app devels as they pop up
- declare sub-namespaces of libMesh as their own namespaces - add libMesh:: in front of many objects - add using libMesh::many_common_ones refs idaholab#28499
- heavily rely on using namespace libMesh - for some files, add libMesh:: everywhere refs idaholab#28499
cc51284
to
340196c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like a great start
Thanks thanks |
340196c
to
8966c74
Compare
closes #28499
patches:
I used the techniques below to make this a little painful. Some of these are not "best practices" so I can avoid these shortcuts, but at the cost of a lot more work most likely.
For source files:
libMesh::
in a lot of source files initially. This was taking forever so I...using namespace libMesh
at the top of source files. This is probably the cleaner way to do this. Unfortunately this quick and easy solution should not be done in headers. And because of unity build I probably have to do this again a few times to get every single source file that actually needs itFor headers:
libMesh::
in a lot of headers as well. This was also taking forever, it would be needed in thousands of placesusing libMesh::Real/ .../
in common headers. Namely MooseTypes.h, MooseMesh.h, AuxiliarySystem.h .... This is not a recommended practice but I don't really see us havinglibMesh::Real
(orusing libMesh::Real;
) in every header?? But maybe we should. Since too many files were needed, I ended up centralizing in libMeshReduceNamespace.hnamespace Threads = libMesh::Threads
which effectively nullifies the effect of removingusing namespace libMesh
. So we keep the same degree of contamination with regards to these nested namespaces of libMesh (e.g. you can't use a Threads namespace with overlapping symbols from another library)using libMesh::
at the end of the libMeshReducedNamespace.h. The alternative is to patch every single app, which is not very reasonable at this point. Unfortunately most apps do not have aXYZnameoftheapp.h
header they include everywhere.In any case, even if we don't like any of these practices, we can fix these gradually, unlike the removal of
using namespace libMesh
which can only be brutalI think I can improve what I did in
libMeshReducedNamespace.h
by using forward declarations. I ll try thatEDIT: done
@grmnptr