Quuxplusone added a comment.
Comments on tests.
================
Comment at: clang/lib/Parse/ParseDeclCXX.cpp:1032
+ // the typename-specifier in a function-style cast expression may
+ // be 'auto' since C++2b
Diag(Tok.getLocation(),
----------------
rsmith wrote:
> Nice catch :)
I see zero hits for `git grep 'decltype[(]auto[^)] clang/`. So it seems this
corner of the grammar has been missing any tests for a long time, but I hope
this PR will add some.
```
decltype(auto*) i = 42; // should be a syntax error
decltype(auto(42)) i = 42; // should be OK
decltype(auto()) i = 42; // should be a syntax error
```
Right now I don't see any tests for this stuff in this PR either. So it needs
some.
================
Comment at:
clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.auto.deduct/p2.cpp:6-28
+ static_assert(__is_same(decltype(auto(v)), int *));
+ static_assert(__is_same(decltype(auto("lit")), char const *));
+
+ int fn(char *);
+ static_assert(__is_same(decltype(auto(fn)), int (*)(char *)));
+
+ constexpr long i = 1;
----------------
It is definitely a good idea to repeat all of these tests with braces too:
`auto{v}`, `auto{"lit"}`, etc.
================
Comment at:
clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.auto.deduct/p2.cpp:51
+ f(A(*this)); // ok
+ f(auto(*this)); // ok in P0849
+ }
----------------
Should you check that `__is_same(decltype(auto(*this)), A)`?
Should you check that `__is_same(decltype(auto(this)), A*)`?
Should you test inside a const member function too?
Should you check that the friend function actually can call
`auto(some_a_object)`, the same way it can call `A(some_a_object)` — and more
to the point, that a non-friend //can't//?
================
Comment at:
clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.auto.deduct/p2.cpp:56
+ A(const A &);
+};
----------------
I'm not sure which test file it should go in, but you definitely want a test
somewhere proving that `auto` doesn't copy prvalues. E.g.
```
struct Uncopyable {
explicit Uncopyable(int);
Uncopyable(Uncopyable&&) = delete;
};
Uncopyable u = auto(Uncopyable(auto(Uncopyable(42))));
```
================
Comment at: clang/test/CXX/expr/expr.post/expr.type.conv/p1-2b.cpp:17-18
+
+ foo(auto(a));
+ foo(auto {a});
+ foo(auto(a));
----------------
What is the significance of the whitespace here? Is this a lexer test?
Normally it'd be just `auto(a)`, `auto{a}` — just like `T(a)`, `T{a}`. I think
we should follow convention (again, unless this is a lexer test — in which case
we should probably test //both// syntaxes with and without whitespace).
================
Comment at: clang/test/CXX/expr/expr.post/expr.type.conv/p1-2b.cpp:25
+ foo(auto {a, 3.14}); // expected-error {{initializer for functional-style
cast to 'auto' contains multiple expressions}}
+ foo(auto({a, 3.14})); // expected-error {{initializer for functional-style
cast to 'auto' contains multiple expressions}}
+}
----------------
I'd also check `auto{1,2}` (ill-formed, does //not// deduce
`initializer_list<int>`) and `auto{{1,2}}` (also ill-formed).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113393/new/
https://reviews.llvm.org/D113393
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
