Remove bug-avoiding StoreConfig * casts for settings#6258
Remove bug-avoiding StoreConfig * casts for settings#6258Ericson2314 merged 2 commits intoNixOS:masterfrom
StoreConfig * casts for settings#6258Conversation
|
Not opposed in theory but it's a huge diff for little gain... I've never really had a problem with this being a pointer so I'm not sure it really makes |
|
@edolstra Sure. So I was flailing around with the GCC bug and trying various things, and made this. The pointer vs ref thing is not very import, agreed. The function, however, is useful if due to that bug need to change what base class we are casting "from". At this point, I am considering instead backporting patch to earlier GCCs (it is very small!), documenting the restriction in the read-me, and just deleting the workarounds. It is a frustrating bug. How does that sound to you? |
|
Hm, requiring GCC 12 or a patched GCC might be pretty inconvenient to downstream packages, so we probably shouldn't do that (yet). |
|
Well, GCC 12, 5 line patched GCC, or any version of Clang. I do feel like requiring bug-free implementations is "morally" different from requiring the latest features, but yes they are harder to distinguish as a practical matter. |
fricklerhandwerk
added
the
contributor-experience
Ericson2314
force-pushed
the
gcc-bug-ergonomics
branch
from
66ae214 to
8cf1148
Compare
Ericson2314
force-pushed
the
gcc-bug-ergonomics
branch
from
8cf1148 to
54d9c64
Compare
github-actions
bot
added
the
store
Ericson2314
changed the title
StoreConfig * casts for settings
|
This PR was completely redone: rather than working around in a different way, we simply don't need to work around at all any more! The only churn that is left is getting rid of hacks :). |
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80431 (test is adapted from issue, test does not test for GCC-specific behavior but rather absence of bug, so test is good with other compilers too.)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80431 has been fixed, and per the previous commit we now check that is the case at build time.
Ericson2314
force-pushed
the
gcc-bug-ergonomics
branch
from
54d9c64 to
b2cae33
Compare
Remove bug-avoiding `StoreConfig *` casts for settings (cherry picked from commit e3febfc) Change-Id: Ifeae276582fdbc781a38581df9de3da67a7e7bf9
4 participants
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80431 has been fixed, the default GCC in Nixpkgs is now new enough, and the fix is trivial to backport for older GCCs too (I did it in NixOS/nixpkgs#210816).