Skip to content

Macro tweaks#1868

Merged
roystgnr merged 10 commits into
libMesh:masterfrom
roystgnr:macro_tweaks
Oct 2, 2018
Merged

Macro tweaks#1868
roystgnr merged 10 commits into
libMesh:masterfrom
roystgnr:macro_tweaks

Conversation

@roystgnr
Copy link
Copy Markdown
Member

In theory this PR is actually only dozens of lines of additions, hundreds of deletions, and thousands of moves (a bunch of code that had with-mpi and without-mpi versions now has a common implementation outside the ifdef LIBMESH_HAVE_MPI block), but my git version doesn't support --color-moved and I can't figure out how to make Github use it either.

In practice I did have a few rebase conflicts; hopefully Civet will verify that I didn't screw up any of the resolutions.

We can put an ifdef here to get rid of a lot of ifdefs elsewhere.
We shouldn't have libmesh_ignored this, it was an actual mistake!
We now die at configure time if static_assert fails, and we use it
unguarded elsewhere
@jwpeterson
Copy link
Copy Markdown
Member

There are reams of template error messages in the CIVET output, here is the first one for Communicator::min() FWIW.

In file included from ./include/libmesh/parallel.h:47:0,
                 from ./include/libmesh/libmesh.h:27,
                 from ../src/base/libmesh_common.C:20:
./include/libmesh/parallel_implementation.h:2103:13: error: prototype for ‘void libMesh::Parallel::Communicator::min(bool&) const’ does not match any in class ‘libMesh::Parallel::Communicator’
 inline void Communicator::min(bool & r) const
             ^~~~~~~~~~~~
In file included from ./include/libmesh/parallel.h:24:0,
                 from ./include/libmesh/libmesh.h:27,
                 from ../src/base/libmesh_common.C:20:
./include/libmesh/communicator.h:237:8: error: candidate is: template<class T> void libMesh::Parallel::Communicator::min(T&) const
   void min(T & r) const;
        ^~~

@roystgnr
Copy link
Copy Markdown
Member Author

Thanks; I can reproduce the problems and I've got them mostly fixed. This PR got interrupted by my trip while I was 80% done with it and when I got back I'd forgotten that the other 20% included --disable-mpi testing...

@roystgnr roystgnr force-pushed the macro_tweaks branch 2 times, most recently from afa06bd to d64c4d7 Compare September 29, 2018 18:45
This lets us delete most of the shims in our without-MPI code section.
If we want to use more of the same code with and without MPI, we need
to be able to suppress unused argument warnings in the without case.
@roystgnr
Copy link
Copy Markdown
Member Author

roystgnr commented Oct 1, 2018

Planning to merge this soon; it's only a minor improvement but moving all that code is going to cause no end of conflicts when anyone tries to play with moved sections of parallel_implementation.h while it's outstanding. Fortunately the new method in #1826 is surrounded by unmoved code so should be safe.

@jwpeterson
Copy link
Copy Markdown
Member

The no-MPI test should already be covered by the --enable-complex case, but since there were so many changes I thought it would be good to test the no MPI and !LIBMESH_USE_COMPLEX_NUMBERS configuration...

// to shut up unused variable compiler warnings on a case by case
// basis.
template<class T> inline void libmesh_ignore( const T & ) { }
template<class ...Args> inline void libmesh_ignore( const Args&... ) { }
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is handy... there are several places in the examples where we ignore multiple variables in the same scope.

Copy link
Copy Markdown
Member

@jwpeterson jwpeterson left a comment

Choose a reason for hiding this comment

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

Looks like a big improvement to having ifdefs all over the place.

@roystgnr roystgnr merged commit d119074 into libMesh:master Oct 2, 2018
@roystgnr roystgnr deleted the macro_tweaks branch October 2, 2018 14:10
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