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

Workaround gcc 13 -Wdangling-reference warning #28837

Draft
wants to merge 1 commit into
base: next
Choose a base branch
from

Conversation

roystgnr
Copy link
Contributor

Closes #28835

Reason

Building MOOSE with gcc 13.2.0 gives me dangling reference warnings.

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.

Design

Removing the nice beautiful modern-style if constexpr lambda function in a constructor initializer list and replacing it with an ugly 20th-century-style template metafunction silences the warning for me.

Impact

This PR fixes a nasty false-positive compiler warning if I'm right, or fixes a serious bug if I'm wrong. Either way it hurts the code aesthetics so I'd like to hope there's a better solution.

git blame seems to point to @dschwen for the code I'm removing here, so I'm pinging him in case he sees another solution or any non-aesthetic issues with my solution.

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
@moosebuild
Copy link
Contributor

Job Documentation, step Docs: sync website on 354f5b6 wanted to post the following:

View the site here

This comment will be updated on new commits.

@moosebuild
Copy link
Contributor

Job Coverage, step Generate coverage on 354f5b6 wanted to post the following:

Framework coverage

45387e #28837 354f5b
Total Total +/- New
Rate 85.05% 85.05% +0.00% 100.00%
Hits 106062 106063 +1 5
Misses 18646 18646 - 0

Diff coverage report

Full coverage report

Modules coverage

Coverage did not change

Full coverage reports

Reports

This comment will be updated on new commits.

template <typename T, bool is_ad, bool is_functor, typename RT>
MaterialAuxBaseTempl<T, is_ad, is_functor, RT>::MaterialAuxBaseTempl(
const InputParameters & parameters)
: AuxKernelTempl<RT>(parameters),
_prop([this]() -> const auto & {
Copy link
Member

Choose a reason for hiding this comment

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

🤮

@dschwen
Copy link
Member

dschwen commented Oct 18, 2024

My vote would be to locally silence this bogus warning. The new code adds too much maintenance burden.

@roystgnr
Copy link
Contributor Author

My vote would be to locally silence this bogus warning. The new code adds too much maintenance burden.

That's fair. I'll put up a draft PR doing that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

-Wdangling-reference warning with newer GCC
4 participants