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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to