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

-Wdangling-reference warning with newer GCC #28835

Open
roystgnr opened this issue Oct 11, 2024 · 0 comments · May be fixed by #28837
Open

-Wdangling-reference warning with newer GCC #28835

roystgnr opened this issue Oct 11, 2024 · 0 comments · May be fixed by #28837
Labels
P: minor A defect that does not affect the accuracy of results. T: defect An anomaly, which is anything that deviates from expectations.

Comments

@roystgnr
Copy link
Contributor

Bug Description

When building MOOSE with gcc 13, instantiations of MaterialAuxBaseTempl give me warnings such as:

/home/stogrh/git/moose-fresh/framework/build/header_symlinks/MaterialAuxBase.h: In instantiation of 'MaterialAuxBaseTempl<T, is_ad, is_functor, RT>::MaterialAuxBaseTempl(const InputParameters&) [with T = libMesh::DenseMatrix<double>; bool is_ad = false; bool is_functor = false; RT = double]':
/home/stogrh/git/moose-fresh/framework/src/auxkernels/MaterialRealDenseMatrixAux.C:28:42:   required from here
/home/stogrh/git/moose-fresh/framework/build/header_symlinks/MaterialAuxBase.h:47:24: warning: possibly dangling reference to a temporary [-Wdangling-reference]
   47 |   const PropertyType & _prop;
      |                        ^~~~~
/home/stogrh/git/moose-fresh/framework/build/header_symlinks/MaterialAuxBase.h:102:6: note: the temporary was destroyed at the end of the full expression '<lambda closure object>MaterialAuxBaseTempl<libMesh::DenseMatrix<double>, false, false, double>::MaterialAuxBaseTempl(const InputParameters&)::<lambda()>{((MaterialAuxBaseTempl<libMesh::DenseMatrix<double>, false, false, double>*)this)}.MaterialAuxBaseTempl<libMesh::DenseMatrix<double>, false, false, double>::MaterialAuxBaseTempl(const InputParameters&)::<lambda()>()'
   97 |     _prop([this]() -> const auto & {
      |           ~~~~~~~~~~~~~~~~~~~~~~~~~~
   98 |       if constexpr (is_functor)
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~
   99 |         return this->template getFunctor<Moose::GenericType<T, is_ad>>("functor");
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  100 |       else
      |       ~~~~
  101 |         return this->template getGenericMaterialProperty<T, is_ad>("property");
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  102 |     }()),
      |     ~^~

Steps to Reproduce

Compile MOOSE with g++ (Ubuntu 13.2.0-23ubuntu4) 13.2.0, the default compiler on Ubuntu 24.04.1 LTS

Impact

If there isn't a dangling reference bug here, this is just an annoyance, a false positive warning that would at worst encourage developers to ignore more serious warnings in new code.

I don't believe there is a dangling reference bug here - the code looks good, and recent GCC versions have had false positives emitted for that warning. The false positives I've tested do seem to be fixed for my GCC build, but there are some very similar cases in bug reports.

If there is a dangling reference bug here, of course, that's the worst kind of bug; undefined behavior with "nasal demons" in the best case and silent corruption of data in the worst (which is also the typical) case.

I've got a workaround that I'll put up as a PR for consideration, but with that the impact is that we'd be replacing 5 lines of tight C++17-style code with 20 lines of C++98-style template metafunctions.

[Optional] Diagnostics

No response

@roystgnr roystgnr added P: minor A defect that does not affect the accuracy of results. T: defect An anomaly, which is anything that deviates from expectations. labels Oct 11, 2024
roystgnr added a commit to roystgnr/moose that referenced this issue Oct 11, 2024
I don't see how this could be an actual dangling reference, and gcc has
bug reports specifically related to dangling reference false positives
from lambda functions, but the test case in bug report
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111607
doesn't actually trigger my gcc version.

This template metafunction workaround is much uglier code than the if
constexpr, but it should compile to the same binary and it doesn't
trigger any warnings.

Closes idaholab#28835
@roystgnr roystgnr linked a pull request Oct 11, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P: minor A defect that does not affect the accuracy of results. T: defect An anomaly, which is anything that deviates from expectations.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant