jkorous-apple 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);
----------------
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?
================
Comment at: Parser/typedef-instead-of-typename-typo.hpp:3
+
+template <typename A, typedef B> struct Foo {
+ a
----------------
vsapsai wrote:
> Maybe put this test in clang/test/FixIt ?
>
> Also please check what file extensions are used for testing templates. .hpp
> reflects real-life usage but most tests are .cpp. Or maybe I wasn't paying
> attention.
Oh, I completely missed that directory! Thanks.
It seems like there are some header files with ".h" extension but these are
usually somehow special (empty file or something) so I figure you are right and
the extension should be ".cpp".
================
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'?}} \
----------------
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?
================
Comment at: clang/Basic/DiagnosticParseKinds.td:1167
+def note_meant_to_use_typename : Note<
+ "Did you mean to use 'typename'?">;
}
----------------
vsapsai wrote:
> Looks like other diagnostic messages "did you mean to use …" have lowercase
> "d" in "did". Though I haven't checked how it looks in various situations.
You are right. Thanks.
https://reviews.llvm.org/D42170
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
