ayzhao 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). ---------------- Rakete1111 wrote: > 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`. :( Currently looking into this. It is interesting to note that for the following example in one of the parent comments: ``` 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; } ``` GCC doesn't like the fact that the `template` keyword is on the final line if compiled with `-pedantic` [0]: ``` <source>:6:16: warning: keyword 'template' not allowed in declarator-id [-Wpedantic] 6 | X<T>::template Y<U>::V::W X<T>::f() { return 0; } | ^~~~ Compiler returned: 0 ``` [0]: https://godbolt.org/z/dYddW3ErY 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