Rakete1111 marked an inline comment as done.
Rakete1111 added inline comments.


================
Comment at: clang/lib/Sema/SemaTemplate.cpp:3377-3379
       // FIXME: This is not quite correct recovery as we don't transform SS
       // into the corresponding dependent form (and we don't diagnose missing
       // 'template' keywords within SS as a result).
----------------
rsmith wrote:
> Rakete1111 wrote:
> > Rakete1111 wrote:
> > > rsmith wrote:
> > > > This FIXME is concerning. Is this a problem with this patch? (Is the 
> > > > FIXME wrong now?)
> > > Yes you're right, I believe it not longer applies due to the change in 
> > > ParseDecl.cpp in ParseDeclarationSpecifiers.
> > Although I'm not that sure I fully understand what that comments alludes to.
> The example would be something like this:
> 
> ```
> template<typename T> struct X {
>   template<typename U> struct Y { Y(); using V = int; };
>   template<typename U> typename Y<U>::V f();
> };
> template<typename T> template<typename U>
> X<T>::Y<U>::V X<T>::f() {} 
> ```
> 
> Clang trunk today points out that the `typename` keyword is missing on the 
> final line, but fails to point out that the `template` keyword is also 
> missing. The reason is that in the case where that construct is valid:
> 
> ```
> template<typename T> template<typename U>
> X<T>::Y<U>::Y() {}
> ```
> 
> ... we are "entering the context" of the //nested-name-specifier//, which 
> means we don't need a `template` keyword.
> 
> If the FIXME is fixed, then we should diagnose the missing `template` in the 
> above program.
> 
> Also, because we don't rebuild the //nested-name-specifier// as a dependent 
> nested name specifier in this case, we fail to match it against the 
> declaration in the class, so in my example above, we also produce an 
> "out-of-line definition does not match" error.
> 
> 
> A closely-related issue can be seen in an example such as:
> 
> ```
> template<typename T> struct X {
>   template<typename U> struct Y { Y(); typedef void V; }; 
>   template<typename U> typename Y<U>::V::W f();
> };
> template<typename T> template<typename U>
> X<T>::template Y<U>::V::W X<T>::f() { return 0; } 
> ```
> 
> Here, we produce a completely bogus error:
> 
> ```
> <stdin>:6:13: error: 'X::Y::V' (aka 'void') is not a class, namespace, or 
> enumeration
> X<T>::template Y<U>::V::W X<T>::f() { return 0; } 
>             ^
> ```
> 
> ... because we parse this in "entering context" mode and so resolve 
> `X<T>::Y<U>::V` to the type in the primary template (that is, `void`). That's 
> wrong: we should defer resolving this name to a type until we know what `T` 
> and `U` are, or until we know that we're *actually* entering the context. 
> Specifically, the above program can be extended as follows:
> 
> ```
> template<> template<> struct X<int>::Y<int> {
>   struct V { using W = int; };
> };
> void call(X<int> x) { x.f<int>(); } // OK, f returns int
> ```
> 
> The above program should be accepted by this patch, if the FIXME is indeed 
> now fixed. Please add it as a testcase :)
Oh ok, got it thanks. So no, the program is still not accepted in clang, and a 
pretty dumb side effect of it is that 

```
template<typename T> struct X {
  template<typename U> struct Y { Y(); using V = int; };
  template<typename U> typename Y<U>::V f();
};
template<typename T> template<typename U>
X<T>::Y<U>::V X<T>::f() {}
```

is accepted without the diagnostic for the missing `template`. :(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D53847



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

Reply via email to