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 dangling reference warning in buildingplan plugin #4540

Merged
merged 2 commits into from
May 16, 2024

Conversation

cvuchener
Copy link
Contributor

fix #4534

@cvuchener
Copy link
Contributor Author

A version check is needed in the macro definition. :(

@cvuchener
Copy link
Contributor Author

updated

I don't like this compiler/version check, it's too complex. Can it be better? Maybe you want to wait for your tool chain to update to gcc 14 before fixing this issue.

@ab9rf
Copy link
Member

ab9rf commented May 1, 2024

"All attributes unknown to an implementation are ignored without causing an error" so if an undefined attribute is causing a compilation error this itself is a compiler bug.

@ab9rf
Copy link
Member

ab9rf commented May 1, 2024

We are not likely to move to gcc 14 any time soon due to basically nonexistent support for gcc 14's runtime in widely available distributions. Building DFHack with a version of gcc other than the one DF is built with (currently gcc 11) is unsupported.

@cvuchener
Copy link
Contributor Author

"All attributes unknown to an implementation are ignored without causing an error" so if an undefined attribute is causing a compilation error this itself is a compiler bug.

It causes a warning and with -Werror it causes an error. See on compiler explorer

We are not likely to move to gcc 14 any time soon due to basically nonexistent support for gcc 14's runtime in widely available distributions. Building DFHack with a version of gcc other than the one DF is built with (currently gcc 11) is unsupported.

Yes, forget about this. I'll keep my workaround in my cmake configuration.

@cvuchener cvuchener closed this May 1, 2024
@myk002
Copy link
Member

myk002 commented May 1, 2024

Yes, forget about this. I'll keep my workaround in my cmake configuration.

I think this can still be a useful addition. we don't distribute DFHack built with newer compilers, but we do build with newer compilers. We build with gcc-12 in CI, for example, to catch warnings that gcc-10 misses.

@myk002 myk002 reopened this May 1, 2024
@ab9rf
Copy link
Member

ab9rf commented May 2, 2024

I would use the "pragma" approach to disable the warnings instead of using an attribute, since (I sincerely hope) that no compiler author is doofus enough to issue even so much as a warning for an unrecognized pragma. 🤦‍♀️

@@ -15,6 +15,12 @@
#include "df/organic_mat_category.h"
#include "df/world.h"

#if (defined(__GNUC__) && !defined(__clang__) && __GNUC__ >= 14)
Copy link
Member

Choose a reason for hiding this comment

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

could also be #if __has_cpp_attribute(no_dangling), I think

Copy link
Member

Choose a reason for hiding this comment

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

@myk002 myk002 merged commit 0d4dbf9 into DFHack:develop May 16, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Failed to build buildingplan with GCC 14 (compiler bug?)
3 participants