[PATCH] D16579: Warn if friend function depends on template parameters.

[Serge Pavlov via cfe-commits]

2016-03-18 0:35 GMT+06:00 Arthur O'Dwyer <arthur.j.odwyer at gmail.com>:

> I'm not qualified to comment on the implementation, but I'm a bit
> skeptical that this warning is appropriate in the first place. I've often
> declared friend non-template functions, e.g. swap(). I've never intended to
> declare a friend *template specialization*. Is declaring friend template
> specializations something that people do often? Is it something that Clang
> should be encouraging newbies to do *more* often?
>

It is not about intention but misunderstanding. Users often think that if a
function is declared inside a template class and depends on template
parameters, it is template. That expect the following code to work:

    template<typename T> void foo(T *x) {…}
    template<typename T> class Bar {
      void foo(T *x);
    }

There are lots of questions in forums (search by keywords "friend template
function link error"), in which people feel surprised why such code results
in undefined reference. The proposed compiler message is just a hint how to
make the code working as expected by a user.


>
> GCC suppresses the diagnostic when the friend function is declared inline,
> which is good, because it shuts up the diagnostic in this very common case:
>
> template<class T>
> struct vector {
>     void swap(vector& b) { /* swap members */ }
>     friend void swap(vector& a, vector& b) { a.swap(b); }  // friend
> non-template function
> };
>
> IMHO you should add an explicit test confirming that the warning is
> suppressed in this case.
>

There is such check already, file `test/SemaCXX/friend.cpp`:

    template <typename T>
    struct Arg {
      friend bool operator==(const Arg& lhs, T rhs) {
       return false;
      }
    …
    };
    …
    bool foo() {
      Arg<int> arg;
      return (arg == 42) || (arg != 42);
    }

Operator `==` is defined inline, no warning expected.


>
The example in PR23342 doesn't seem like a good example, because GCC gives
> not only the proposed warning for that code, but also a linker error; i.e.,
> there's no danger of someone accidentally getting wrong runtime behavior
> there. Is there a compelling
>

Clang behaves exactly the same, compilation eventually results in a linker
error.


> example of how someone could write code that has wrong runtime behavior,
> *and* which code would be *fixed* by the suggested addition of template<>?
> I think it's much more likely that the appropriate fix would be to move the
> function definition inline.
>

There seems to be no danger to get wrong behavior here, only undefined
symbols at link stage. If code builds, it works right.


> https://llvm.org/bugs/show_bug.cgi?id=23342
>
> Why did you need to suppress the warning in
> "test/CXX/temp/temp.decls/temp.friend/p1.cpp"? Unless I'm mistaken, GCC
> doesn't give the diagnostic on that file (and nor should it).
>
> my $.02,
>

Good catch, thank you for your $0.02 :) There is no point in warnings for
class members. Patch fixed accordingly.

Thanks,
--Serge
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160318/cda7d3da/attachment.html>