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

Disable strict-aliasing temporarily with GCC for container_init in init_kernel.hpp #1326

Open
Alexhuszagh opened this issue Jul 21, 2017 · 3 comments

Comments

@Alexhuszagh
Copy link

Alexhuszagh commented Jul 21, 2017

GCC is fairly well known to be very aggressive with strict aliasing, for good reason. However, on Fedora MinGW (version 7.1.0 20170502), both 32-bit and 64-bit, the reinterpret_cast<kernel *> calls trigger warning: dereferencing type-punned pointer will break strict-aliasing rules. I believe this is a compiler bug, and not actually a bug with DyND, but the following does "patch" the issue.

I'm not sure how desirable this "patch" is, however, implementing it allows Fedora MinGW to fully compile DyND (along with the previous patches mentioned).

diff --git a/include/dynd/kernels/init_kernel.hpp b/include/dynd/kernels/init_kernel.hpp
index 33fd5af47..83988331c 100644
--- a/include/dynd/kernels/init_kernel.hpp
+++ b/include/dynd/kernels/init_kernel.hpp
@@ -263,6 +263,8 @@ namespace nd {
     void init(const ndt::type &tp, const char *metadata) {
       typedef detail::init_kernel<ResType, ContainerType> kernel;
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wstrict-aliasing"
       new (&child) kernel(tp, metadata);
       destruct_wrapper = [](container_init *self) { reinterpret_cast<kernel *>(&self->child)->~kernel(); };
       single_wrapper = [](container_init *self, char *data, const ContainerType &values) {
@@ -271,6 +273,7 @@ namespace nd {
       contiguous_wrapper = [](container_init *self, char *data, const ContainerType *values, size_t size) {
         reinterpret_cast<kernel *>(&self->child)->contiguous(data, values, size);
       };
+#pragma GCC diagnostic pop
     }
 
     container_init(const ndt::type &tp, const char *metadata) {
@insertinterestingnamehere
Copy link
Member

Yep. That's the right way to locally silence a warning like this. This came up in #1315 as well. The solution we agreed on there was to just cut out the warning-is-an-error behavior until we can pin down definitively whether or not this is a false positive. The fact that it's complaining when we use reinterpret_cast is odd since we're already saying "trust us that the types work fine on this one." Silencing it in the mean time is fine with me.

@Alexhuszagh
Copy link
Author

@insertinterestingnamehere Just an FYI, I'm currently writing my thesis so I will try to get a PR done this week, but if there's been any major delays, I apologize. Feel free to commit changes without me.

@insertinterestingnamehere
Copy link
Member

Thanks for the heads up. There's no rush. Writing a thesis will keep you very busy. Good luck. I'm working on solving some other issues here at the moment but can come back to these if you don't get to them.

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

No branches or pull requests

2 participants