[PATCH] D24977: [CUDA] Declare our __device__ math functions in the same inline namespace as our standard library.

Justin Lebar via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 27 13:19:05 PDT 2016


jlebar added a comment.

> That is way too much knowledge about details of standard library implementation.


Honestly I think this looks a lot scarier than it is.  Or, to be specific, I think we are already relying on implementation details much more implicit and fragile than what is explicit here.  See the git log of all of the changes I've had to make to this file before now to make us compatible with all of the standard libraries we want to support.

> If it changes, I suspect users will end up with a rather uninformative error.


You mean, if the standard libraries change the macro they're using here?  If so, we'll fall back to plain "namespace std", which is what we had before, so it should work fine.  In fact the only way I think this can affect things one way or another is if the standard library does

  namespace std {
  inline namespace foo {
  void some_fn(std::complex<float>);
  
  void test() {
    some_fn(std::complex<float>());
  }
  } // inline namespace foo
  }  // namespace std

ADL on some_fn will prefer the some_fn inside std::foo, so if we declare an overload of some_fn inside plain namespace std, it won't match.

> We could whitelist libc++/libstdc++ version we've tested with and produce #warning "Unsupported standard library version" if we see something else.


In practice, we are testing with versions of libstdc++ that are so much newer than what anyone has on their systems, I am not exactly worried about this.

But I think more generally these questions are probably better handled in a separate patch?  Like I say, we are already rather tightly-coupled to the standard libraries -- I don't think this patch changes that reality too much.


https://reviews.llvm.org/D24977





More information about the cfe-commits mailing list