vsapsai added inline comments.
================
Comment at: FixIt/fixit-typedef-instead-of-typename-typo.cpp:14
+// CHECK: expected-error@-6 {{unknown type name 'a'}}
+// CHECK: expected-error@-6 {{expected member name or ';' after declaration
specifiers}}
----------------
You don't need `// CHECK:` prefix for `expected-error`. And without `FileCheck`
CHECK lines aren't verified. In fact, the test should fail as it expects the
fix-it to be for the line 4, not line 3. I'm not sure you can test diagnostic
and parseable fixits with a single RUN line, luckily we can have multiple RUN
lines.
I don't think those relative error lines are good from readability standpoint.
But please keep in mind that all my comments about readability are subjective
and should be taken with the grain of salt. The problem is that one needs to
count lines to understand which line diagnostic is referring to. From my
experience it is easy to understand something like
```
blah // expected-error
// expected-note@-1
```
because you don't need to calculate lines, it is clear -1 refers to the
previous line. But in your case I have to calculate 6 or 7 lines back.
Here is for comparison "expected-" annotations closer to corresponding lines
```lang=c++
template <typename A, typedef B> // expected-error{{expected template
parameter}}
// expected-note@-1{{did you mean to use
'typename'?}}
// CHECK: fix-it:{{.*}}:{4:23-4:30}:"typename"
struct Foo {
// Should not produce error about type since parsing speculatively with fixit
applied.
B member;
// Let's just check that other errors get reported.
a // expected-error{{unknown type name 'a'}}
}; // expected-error{{expected member name or ';' after declaration specifiers}}
```
I'm not entirely happy with the result as it looks too dense and noisy but it
conveys my thought.
https://reviews.llvm.org/D42170
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
