[PATCH] D79800: [Sema] Implement DR2233

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Unfortunately, I think this approach basically can't work, because we need to consider inheriting default arguments from a previous (or subsequent!) declaration before dropping default arguments preceding a pack. I think we will instead need to detect this situation when issuing the diagnostic for a parameter with a default argument followed by a parameter without one, and will need to make sure that all the parts of Clang that look at default arguments can cope with them being discontiguous.



================
Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:1978
+
+    // DR777, DR2233.
+    // Parameter packs are allowed after and inbetween parameters with default
----------------
This code is only reached when substituting into non-member functions, so this change would not address the corresponding problem for member function templates or for member functions of class templates. It would be better to handle this in `InitFunctionInstantiation`, which is called for all function instantiations. (But as noted below, doing this during instantiation doesn't seem to be correct.)

In any case, we should have some testcases for member functions.


================
Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:2090
     // as described in ActOnFriendFunctionDecl.
     SemaRef.LookupQualifiedName(Previous, DC->getRedeclContext());
 
----------------
We need to do this lookup and the merging below in `CheckFunctionDeclaration` before we drop any default arguments. Consider this pathological-but-valid testcase (that Clang currently accepts and the other compilers incorrectly reject):

```
template<typename ...T> void f() {
    void g(int, int = 0);
    void g(int = 0, T...);
    g();
}
void h() { f<int>(); }
```

Here, we only find out that the second `g` redeclares the first one during instantiation, and only after that do we inherit the `=0` for the second parameter of the first `g` onto the second parameter of the second `g`.

And that's not even the worst testcase of this kind. Consider:

```
template<typename ...T> void f() {
    void g(int = 0, T...);
    void g(int, int = 0);
    g();
}
void h() { f<int>(); }
```

This is valid under DR2233. The first declaration of `g` provides a default argument for the first parameter, the second provides a default argument for the second parameter, and the call to `g()` is valid because both parameters have default arguments. So it is not correct to discard the default arguments when instantiating the first declaration of `g`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79800