vsapsai added inline comments.
================
Comment at: Parse/ParseTemplate.cpp:492
+ // Is there just a typo in the input code? ('typedef' instead of 'typename')
+ if (Tok.is(tok::kw_typedef)) {
+ Diag(Tok.getLocation(), diag::err_expected_template_parameter);
----------------
jkorous-apple wrote:
> vsapsai wrote:
> > How does it work when you have `typedef` for the first template parameter?
> It works all right but I am puzzled by your question now :-)
>
>
> ```
> /Users/jankorous/src/oss/llvm/llvm/tools/clang/test/Parser/typedef-instead-of-typename-typo.hpp:3:11:
> error: expected template parameter
> template <typedef A, typename B> struct Foo {
> ^
> /Users/jankorous/src/oss/llvm/llvm/tools/clang/test/Parser/typedef-instead-of-typename-typo.hpp:3:11:
> note: Did you mean to use 'typename'?
> template <typedef A, typename B> struct Foo {
> ^~~~~~~
> typename
> fix-it:"/Users/jankorous/src/oss/llvm/llvm/tools/clang/test/Parser/typedef-instead-of-typename-typo.hpp":{3:11-3:18}:"typename"
> /Users/jankorous/src/oss/llvm/llvm/tools/clang/test/Parser/typedef-instead-of-typename-typo.hpp:4:3:
> error: unknown type name 'a'
> a
> ^
>
> ```
>
> I am probably not following you here. Do you have any specific reason on your
> mind? Anything I should think about or check in the source code?
I was suspicious that
```lang=c++
if (isStartOfTemplateTypeParameter())
return ParseTypeParameter(Depth, Position);
```
earlier in the function won't let your code run for the first template
parameter. But `isStartOfTemplateTypeParameter` has
```lang=c++
if (Tok.isNot(tok::kw_typename))
return false;
```
so everything works fine.
================
Comment at: Parser/typedef-instead-of-typename-typo.hpp:5
+ a
+}; // CHECK: expected-error{{expected template parameter}} \
+// CHECK: expected-note{{Did you mean to use 'typename'?}} \
----------------
jkorous-apple wrote:
> vsapsai wrote:
> > It is a little bit confusing to what lines the messages would be attributed
> > to. Need to check locally because not sure I interpret all those
> > backslashes the same way lit does.
> >
> > Also idea for the test. To check that the fix-it was applied properly you
> > can add a member like `B b;` and it shouldn't trigger any errors.
> I am totally open to suggestions how to write better tests using FileCheck.
> As far as I understand it I have to keep the expected-* macro on the same
> line as the code. Or is there any better way?
>
> Do I understand it right that you suggest to create another test in which to
> try applying fixit and check that compilation was successful? Do you mean to
> create compile_commands.json temporarily and use clang-check -fixit?
>
>
To answer your question. You can keep the expected-* macro on the same line and
attach subsequent expectations with backslash. Another option is doing
something like `expected-note@-1 {{...}}`. That `-1` is a relative line number.
Back to the test case. I believe you need to separate expected-* checks from
fixit output check. The former uses `-verify` and the latter `| FileCheck %s`.
I think test/FixIt/fixit-vexing-parse.cpp is a good example. And remember that
you can split long lines, `template <typename A, typedef B> struct Foo` doesn't
have to be on the same line.
For suggested test I didn't mean anything fancy. Just something like adding a
line `B b;` Because currently for
```lang=c++
template <typename A, typedef B>
struct Foo {
B b;
};
```
the output is
```
recovery.cc:1:31: error: unknown type name 'B'
template <typename A, typedef B>
^
recovery.cc:3:5: error: unknown type name 'B'
B b;
^
2 errors generated.
```
And with your change we won't have the second error.
https://reviews.llvm.org/D42170
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
