gromer marked an inline comment as done.
gromer added inline comments.
================
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;
----------------
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.
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
