rsmith accepted this revision.
rsmith added inline comments.
This revision is now accepted and ready to land.
================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2129
+def warn_class_template_argument_deduction_no_user_defined_guides : Warning<
+ "using class template argument deduction for %0 that has no user-defined
deduction guides" >,
+ InGroup<ImplicitCTADUsage>, DefaultIgnore;
----------------
gromer wrote:
> EricWF wrote:
> > rsmith wrote:
> > > gromer wrote:
> > > > I'd prefer a phrasing more along the lines of "using class template
> > > > argument deduction for %0 that might not intentionally support it".
> > > > That gives us more room to do things like add an attribute later if
> > > > necessary, and it helps the user understand why this warning indicates
> > > > a potential problem.
> > > I like that approach; something like "using class template argument
> > > deduction for %0 that might not intend to support it" -- or perhaps more
> > > directly "%0 might not intend to support class template argument
> > > deduction" -- along with a note describing how to syntactically suppress
> > > the warning (w"add a deduction guide to suppress this warning" or "use
> > > the [[clang::ctad]] attribute to suppress this warning" or whatever we
> > > decide is best).
> > This sounds like a reasonable change to me. Done.
> >
> > I'm not sure an attribute is needed at this point; AFAIK there is no case
> > where a user defined guide can't be added. Even if it's just a dummy guide
> > to suppress the warning. For example:
> >
> > ```
> >
> > struct allow_ctad_t {
> > allow_ctad_t() = delete;
> > };
> >
> > template <class T>
> > struct TestSuppression {
> > TestSuppression(int, T) {}
> > };
> > TestSuppression(allow_ctad_t) -> TestSuppression<void>;
> > ```
> >
> > Also, before we add an attribute, we want to be sure that it's not harmful
> > to allowing users to suppress the warning without actually addressing the
> > root problem (tha implicit CTAD results are often surprising). I would like
> > to see real world examples of types that don't need user-defined guides to
> > work.
> >
> > I'm not sure an attribute is needed at this point
>
> I agree; I just want to keep the option open in case we decide we need one
> later.
`TestSuppression(allow_ctad_t) -> TestSuppression<void>;` doesn't always work:
```
struct X { X(); };
template<typename T = X> struct TestSuppression {
TestSuppression(T);
};
TestSuppression(allow_ctad_t) -> TestSuppression<void>;
TestSuppression x({}); // ok without deduction guide, ill-formed with
```
I think this should work reliably (note that the incomplete parameter type
makes the deduction guide always non-viable):
```TestSuppression(struct AllowCTAD) -> TestSuppression<void>;```
... and maybe we should suggest that in our note, perhaps with a fix-it hint
pointing immediately after the class template definition?
================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2132
+def note_suppress_ctad_maybe_unsupported : Note<
+ "add a user-defined deduction guide to suppress this warning">;
+
----------------
Drop "user-defined" here? (There's no other kind of deduction guide.)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56731/new/
https://reviews.llvm.org/D56731
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
