[PATCH] D37538: [libc++] Remove problematic ADL in container implementations.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.


================
Comment at: include/memory:1349
         is_same<
-            decltype(__has_construct_test(declval<_Alloc>(),
-                                          declval<_Pointer>(),
-                                          declval<_Args>()...)),
+            decltype(_VSTD::__has_construct_test(declval<_Alloc>(),
+                                                 declval<_Pointer>(),
----------------
EricWF wrote:
> Wouldn't the `declval` calls also need qualification?
(Sorry I'm jumping in before I know the full backstory here.) I would expect that the "declval" call doesn't need qualification because its argument-list is empty. ADL doesn't apply to template arguments AFAIK.
(Test case proving it to myself: https://wandbox.org/permlink/i0YarAfjOYKOhLFP )

Now, IMHO, `__has_construct_test` also doesn't need guarding against ADL, because it contains a double underscore and is thus firmly in the library's namespace. If the user has declared an ADL-findable entity `my::__has_construct_test`, they're already in undefined behavior land and you don't need to uglify libc++'s code just to coddle such users.

And, IMO, to the extent that ADL *does* work on the old code here, that's a feature, not a bug. As the implementor of some weird fancy pointer or iterator type, I might *like* having the power to hook into libc++'s customization points here. Of course I wouldn't try to hook into `__has_construct_test`, but `__to_raw_pointer` feels more likely. (At my current low level of understanding, that is.)


https://reviews.llvm.org/D37538