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.

Sign up for GitHub

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

Jump to bottom

Failed to build buildingplan with GCC 14 (compiler bug?) #4534

Closed
cvuchener opened this issue Apr 29, 2024 · 7 comments · Fixed by #4540
Closed

Failed to build buildingplan with GCC 14 (compiler bug?) #4534

cvuchener opened this issue Apr 29, 2024 · 7 comments · Fixed by #4540

Comments

@cvuchener
Copy link
Contributor

Compiling 3402d14 on linux (Fedora 40) with gcc (gcc (GCC) 14.0.1 20240411 (Red Hat 14.0.1-0)), I get:

dfhack/plugins/buildingplan/buildingplan.cpp: In function 'int scanAvailableItems(DFHack::color_ostream&, df::enums::building_type::building_type, int16_t, int32_t, int, bool, bool, HeatSafety*, std::vector<int>*, std::map<DFHack::MaterialInfo, int>*)':
dfhack/plugins/buildingplan/buildingplan.cpp:634:11: error: possibly dangling reference to a temporary [-Werror=dangling-reference]
  634 |     auto &job_items = get_job_items(out, key);
      |           ^~~~~~~~~
dfhack/plugins/buildingplan/buildingplan.cpp:634:36: note: the temporary was destroyed at the end of the full expression 'get_job_items((* & out), key)'
  634 |     auto &job_items = get_job_items(out, key);
      |                       ~~~~~~~~~~~~~^~~~~~~~~~
dfhack/plugins/buildingplan/buildingplan.cpp: In function 'int countAvailableItems(DFHack::color_ostream&, df::enums::building_type::building_type, int16_t, int32_t, int)':
dfhack/plugins/buildingplan/buildingplan.cpp:737:11: error: possibly dangling reference to a temporary [-Werror=dangling-reference]
  737 |     auto &job_items = get_job_items(out, key);
      |           ^~~~~~~~~
dfhack/plugins/buildingplan/buildingplan.cpp:737:36: note: the temporary was destroyed at the end of the full expression 'get_job_items((* & out), key)'
  737 |     auto &job_items = get_job_items(out, key);
      |                       ~~~~~~~~~~~~~^~~~~~~~~~

After a quick look at the code, it seems the reference comes from a cache, it may be a false positive. Can the author (@myk002?) confirm all is right?
@cvuchener
Copy link
Contributor Author

Reported to fedora bug tracker.

@myk002
Copy link
Member

myk002 commented Apr 29, 2024

I believe this is ok, but I'll take a closer look

@myk002
Copy link
Member

myk002 commented Apr 30, 2024

the reference returned from get_job_items is valid throughout the lifetime of the loaded library. I'd say this is a false positive

@myk002 myk002 closed this as completed Apr 30, 2024
@cvuchener
Copy link
Contributor Author

To anyone having this issue, just add -Wno-dangling-reference to your compile flags (CMAKE_CXX_FLAGS).

@ab9rf
Copy link
Member

ab9rf commented Apr 30, 2024

the only way for a reference to a data element of a unordered_map to become invalid is if the element is erased or if the map itself is deleted. the only event that causes either of these events is plugin shutdown, so yes, this code is safe

@ab9rf
Copy link
Member

ab9rf commented Apr 30, 2024

To anyone having this issue, just add -Wno-dangling-reference to your compile flags (CMAKE_CXX_FLAGS).

i would suggest bracketing the affected code with a #pragma to disable this warning for just that code block rather than disabling this warning for the entire codebase

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wno-dangling-reference"
....
#pragma GCC diagnostic pop

@cvuchener
Copy link
Contributor Author

I got an answer from Fedora bug tracker:

Unfortunately I don't think I can fix it. We see it's a function returning
a reference and taking a temporary as an argument, so we warn. We cannot
analyze the body of the function f in the front end. But you can suppress
the warning with the [[gnu::no_dangling]] attribute like this:

[[gnu::no_dangling]]
static const std::vector<int> &f(my_key_t key)
{
    auto &v = cache[key];
    return v;
}

@cvuchener cvuchener mentioned this issue May 1, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
50.13-r3
Status: Done
50.13-r2
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix dangling reference warning in buildingplan plugin cvuchener/dfhack
3 participants
@myk002 @ab9rf @cvuchener