rsmith added inline comments.
================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:746
+ diag::note_template_class_instantiation_here)
+ << CTD << Active->InstantiationRange;
} else {
----------------
This diagnostic won't include the template arguments that we're using to
instantiate, which seems like important information.
Do you know where we're creating the `InstantiatingTemplate` instance
corresponding to this diagnostic? Usually we pass in the declaration whose
definition we're instantiating for a `TemplateInstantiation` record; I wonder
if we could do the same for this case.
================
Comment at: clang/test/SemaCXX/cxx1z-copy-omission.cpp:175
+
+// Make sure we don't consider conversion functions for guaranteed copy elision
+namespace GH39319 {
----------------
We should also test that the right function is actually being selected and
called. For example:
```
struct A {
A();
A(const A&) = delete;
};
struct B {
operator A();
} C;
A::A() : A(C) {}
```
... should be rejected because it calls the deleted copy constructor. (Or you
could test this via constant evaluation or look at the generated code, but this
is probably the simplest way.)
================
Comment at: clang/test/SemaCXX/cxx1z-copy-omission.cpp:193
+template<typename A3>
+class B3 : A3 // expected-error {{expected '{' after base class list}}
+ template<bool = C3<B3>()> // expected-warning 2{{use of function template
name with no prior declaration in function call with explicit}}
----------------
Do you need to omit the `{` here as part of this test? This will cause problems
for people editing the file due to mismatched braces, and makes this test
fragile if our error recovery changes. It's best for the test case to be free
of errors other than the one(s) being exercised. (Same for missing `;` errors
below.)
If these errors are necessary to trigger the bug you found, I wonder if there's
a problem with our error recovery. (Maybe we create a "bad"
`InstantiatingTemplate` record during error recovery, for example.)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148474/new/
https://reviews.llvm.org/D148474
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
