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 remaining clang instantiation warnings. #517

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jwpeterson
Copy link
Collaborator

This commit is along the same lines as #513, however these were just warnings while the others were actual compiler errors under clang. Anyone compiling GRINS with clang will probably appreciate having this patch (otherwise there are a lot of warnings) but I also think this code is pretty ugly/not maintainable so in the long run some kind of redesign might be the best solution...

These warnings were specific to the static data members of the class
and looked like the following:

../../src/boundary_conditions/include/grins/dirichlet_bc_factory_function_old_style_base.h:88:16: warning:
instantiation of variable 'GRINS::DirichletBCFactoryFunctionOldStyleBase<libMesh::FunctionBase >::_var_names_old_style' required here, but no definition is
available [-Wundefined-var-template]
if( !this->_var_names_old_style )
^
../../src/boundary_conditions/include/grins/prescribed_vector_value_dirichlet_old_style_bc_factory.h:40:5: note:
in instantiation of member function 'GRINS::DirichletBCFactoryFunctionOldStyleBase<libMesh::FunctionBase >::check_state' requested here
PrescribedVectorValueDirichletOldStyleBCFactory( const std::string& bc_type_name )
^
../../src/boundary_conditions/include/grins/dirichlet_bc_factory_function_old_style_base.h:72:44:
note: forward declaration of template entity is here
static const std::vectorstd::string* _var_names_old_style;
^
../../src/boundary_conditions/include/grins/dirichlet_bc_factory_function_old_style_base.h:88:16:
note: add an explicit instantiation declaration to suppress this warning if 'GRINS::DirichletBCFactoryFunctionOldStyleBase<libMesh::FunctionBase >::_var_names_old_style'
is explicitly instantiated in another translation unit
if( !this->_var_names_old_style )
^

These warnings were specific to the static data members of the class
and looked like the following:

../../src/boundary_conditions/include/grins/dirichlet_bc_factory_function_old_style_base.h:88:16: warning:
instantiation of variable 'GRINS::DirichletBCFactoryFunctionOldStyleBase<libMesh::FunctionBase<double> >::_var_names_old_style' required here, but no definition is
available [-Wundefined-var-template]
    if( !this->_var_names_old_style )
               ^
../../src/boundary_conditions/include/grins/prescribed_vector_value_dirichlet_old_style_bc_factory.h:40:5: note:
in instantiation of member function 'GRINS::DirichletBCFactoryFunctionOldStyleBase<libMesh::FunctionBase<double> >::check_state' requested here
    PrescribedVectorValueDirichletOldStyleBCFactory( const std::string& bc_type_name )
    ^
../../src/boundary_conditions/include/grins/dirichlet_bc_factory_function_old_style_base.h:72:44:
note: forward declaration of template entity is here
    static const std::vector<std::string>* _var_names_old_style;
                                           ^
../../src/boundary_conditions/include/grins/dirichlet_bc_factory_function_old_style_base.h:88:16:
note: add an explicit instantiation declaration to suppress this warning if 'GRINS::DirichletBCFactoryFunctionOldStyleBase<libMesh::FunctionBase<double> >::_var_names_old_style'
      is explicitly instantiated in another translation unit
    if( !this->_var_names_old_style )
               ^
@jwpeterson
Copy link
Collaborator Author

Hmm... the fact that this fixes warnings in clang while breaking the build in GCC has me very confused.

@pbauman
Copy link
Member

pbauman commented Dec 10, 2017

All jobs on d775d1f : canceled by @jwpeterson

This breaks the build, no reason to keep testing.

@pbauman
Copy link
Member

pbauman commented Dec 10, 2017

but I also think this code is pretty ugly/not maintainable so in the long run some kind of redesign might be the best solution...

Suggestions always welcome.

@pbauman
Copy link
Member

pbauman commented Dec 10, 2017

I should have some time this afternoon/evening. Will see if I can reproduce clang warnings on my laptop.

@jwpeterson
Copy link
Collaborator Author

but I also think this code is pretty ugly/not maintainable so in the long run some kind of redesign might be the best solution...

Suggestions always welcome.

Sorry, my comment may have made it sound like I was referring to the original code as ugly, but I only meant the workaround.

@pbauman
Copy link
Member

pbauman commented Dec 11, 2017

I did indeed think you were referring to the original code. :) No offense taken. Quite the contrary, I always welcome feedback on design improvements. Sorry I didn't have time to check this yesterday.

@pbauman
Copy link
Member

pbauman commented Apr 1, 2019

So I was tinkering with clang on my laptop this weekend and I could reproduce all these warnings. It looks to me like these are really unnecessary because you'll end up with a linker error later if you don't eventually instantiate the objects (which is what this warning is about). I would suggest that we just instead disable that particular warning when using clang. FWIW, when I was googling about this error, I came across this deal.ii thread where they came to the same conclusion.

@pbauman pbauman mentioned this pull request Apr 10, 2022
@hartwork
Copy link

@jwpeterson thanks for this pull request: it helped me with making commit Libvisual/libvisual@8585057 of pull request Libvisual/libvisual#121 🙏

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