[PATCH] D60570: [Sema] Add more tests for the behavior of argument-dependent name lookup

Sun Apr 14 17:25:13 PDT 2019

Quuxplusone added inline comments.

================
Comment at: test/CXX/basic/basic.lookup/basic.lookup.argdep/p2-associated-namespaces-classes.cpp:304
+    static_assert(f(g3) == 4, "");        // FIXME: Also well-formed from the union rule.
+                                          // expected-error at -1 {{use of undeclared}}
+  }
----------------
riccibruno wrote:
> Quuxplusone wrote:
> > riccibruno wrote:
> > > Quuxplusone wrote:
> > > > I see how `g3` matches the example in CWG997
> > > > http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#997
> > > > However, I don't see how CWG997's resolution actually affected this example in the slightest. The wording inserted for CWG997 was, "Additionally, if the aforementioned set of overloaded functions is named with a template-id, its associated classes and namespaces are those of its type template-arguments and its template template-arguments." That makes e.g.
> > > > 
> > > >     f(g3<N::S>)
> > > > 
> > > > consider `N::f`, because `N::S` is a "type template-argument" of the template-id `g3<N::S>` which names the set of overloaded functions.  But it doesn't do anything at all to `f(g3)` because `g3` is not a template-id and doesn't have any template-arguments.
> > > > 
> > > > This piece of ADL is implemented only by GCC (not EDG, Clang, or MSVC), and personally I would very much like to keep it that way. We know there's no real-world code that expects or relies on CWG997 — because such code would never work in practice except on GCC. Let's keep it that way!  As soon as you implement a crazy arcane rule, such that code _could_ portably rely on it, code _will start_ relying on it... and then we'll never be able to simplify the language.
> > > > Case in point: the piece of ADL described in this blog post --
> > > > https://quuxplusone.github.io/blog/2019/04/09/adl-insanity-round-2/
> > > > As soon as the above-described arcane ADL rule was implemented in GCC and Clang, Boost.Hana started relying on it; and now the rule is "locked in" to the paper standard because there's real-world code relying on it.
> > > > Personally I'd like to _keep_ real-world code from relying on CWG997, until someone figures out what CWG was thinking when they added it.
> > > I think that the relevant part of CWG 997 is the removal of the restriction on non-dependent parameter types. Sure, `g3` is not a `template-id`, but it refers to an overload set which contains the second `g3`, and one of the parameter of this second `g3` is `N::Q<T>`.
> > > 
> > > I don't think this is a surprising rule. It matches the general intuition that for function types ADL is done based on the function parameter types and return type. Not having this rule introduces a difference between function templates and functions in overload sets. Consider https://godbolt.org/z/UXHqm2 :
> > > ```
> > > namespace N {
> > >     struct S1 {};
> > >     template <typename> struct S2 {};
> > > 
> > >     void f(void (*g)());
> > > }
> > > 
> > > void g1();          // #1
> > > void g1(N::S1);     // #2
> > > 
> > > void g2();                                  // #3
> > > template <typename T> void g2(N::S2<T>);    // #4
> > > 
> > > void test() {
> > >     f(g1); // ok, g1 is #1
> > >     f(g2); // should be ok, g2 is #3
> > > }
> > > ```
> > > I think that the relevant part of CWG 997 is the removal of the restriction on non-dependent parameter types.
> > 
> > Ah, I had missed the removal of the word `(non-dependent)` in my reading of CWG997. So just that one-word removal is what fixed their example, and is what you're testing with `g3`.
> > 
> > I still object to `g2` — I would like that `FIXME` to say `PLEASEDONTFIXME` or something. :)
> I don't understand your objection to the template-id rule of CWG 997 (which is what `f(g2<N::S>)` tests). It seems to me that the following should be well-formed (https://godbolt.org/z/J5uhQ-) :
> 
> 
> ```
> namespace N {
>     template <typename T> void f(T &&);
>     struct S {};
> }
> 
> template <typename T> struct C {};
> template <typename T> void g();
> 
> void test() {
>     f(C<N::S>{}); // ok
>     f(g<N::S>);   // should be ok (and is ok according to the spec)
> }
> ```
I admit that your comparison makes the rule look reasonable. :)  But here are some comparisons designed to make the rule look UNreasonable.
https://godbolt.org/z/n5kvUE

```
namespace N {
    void foo(void(*)());

    class T {};
    using A = int;
    template<class> class TT {};
    template<class> using AT = int;
    void F();
    void (*V)();
}

template<class> void ft_t();
template<class> void (*vt_t)();
template<auto&> void ft_nt();
template<auto&> void (*vt_nt)();
template<template<class> class> void ft_tt();
template<template<class> class> void (*vt_tt)();

void test() {
    foo(ft_t<N::T>);  // OK: ADL considers N::foo (nobody but GCC implements this)
    foo(vt_t<N::T>);  // Error: ADL does not consider N::foo
    foo(ft_t<N::A>);  // Error: ADL does not consider N::foo
    foo(vt_t<N::A>);  // Error: ADL does not consider N::foo

    foo(ft_nt<N::V>);  // Error: ADL does not consider N::foo
    foo(vt_nt<N::V>);  // Error: ADL does not consider N::foo
    foo(vt_nt<N::F>);  // Error: ADL does not consider N::foo

    foo(ft_tt<N::TT>);  // OK: ADL considers N::foo (nobody but GCC implements this)
    foo(vt_tt<N::TT>);  // Error: ADL does not consider N::foo
    foo(ft_tt<N::AT>);  // OK: ADL considers N::foo (nobody but GCC implements this)

    foo( (ft_t<N::T>) );  // OK??: ADL considers N::foo (nobody but GCC implements this)
    foo( &ft_t<N::T>  );  // OK??: ADL considers N::foo (nobody but GCC implements this)
    foo( (ft_tt<N::TT>) );  // OK??: ADL considers N::foo (nobody but GCC implements this)
    foo( &ft_tt<N::TT>  );  // OK??: ADL considers N::foo (nobody but GCC implements this)
    foo( (ft_tt<N::AT>) );  // OK??: ADL considers N::foo (nobody but GCC implements this)
    foo( &ft_tt<N::AT>  );  // OK??: ADL considers N::foo (nobody but GCC implements this)
}
```

In all of these cases there is no ADL, on any of the Big 4 compilers, //except// for GCC in //some// of the cases. So it would be really nice IMO to just say "okay, let's just strike those corner cases from C++2b, eliminate these cases of ADL from GCC, the world gets a little bit simpler, no code breaks, everyone is happy." If Clang moves closer to GCC (and further from EDG/MSVC), then the happy path actually becomes harder to sell.

(I'm pretty sure the cases I marked `OK??` are indeed OK according to the paper standard; I think that's why [basic.lookup.argdep] says "...if the argument is the name **or address** of a set of overloaded functions...")

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60570/new/

https://reviews.llvm.org/D60570