rsmith added a comment.
I like @Quuxplusone's suggestions for more heuristics here, but I think they
should probably live under a different warning flag (collectively grouped under
`-Wctad` as @lebedev.ri suggested). Concretely, I think we could warn if,
during template instantiation from a type-dependent initializer, we select the
copy deduction candidate and there was another viable candidate (the `optional
o(x)` case, the `tuple t{args...}`case, etc), maybe under a new `-Wctad-unwrap`
or similar.
I don't see a good general way to catch and warn on bugs like `vector<string>
v("foo", "bar")` (this problem isn't really specific to CTAD), but we could
certainly add some special-case heuristics to detect cases where a pair of
string literals is passed to a constructor of a container-like type (for
example, we could check whether the constructor looks like it can be called
with arbitrary iterators as arguments, the class also has an
`initializer_list<T>` constructor where a string literal can be implicitly
converted to `T`, and the class has `begin` and `end` member functions).
================
Comment at: include/clang/Basic/DiagnosticGroups.td:1054
+
+def ImplicitCTADUsage : DiagGroup<"implicit-ctad">;
----------------
lebedev.ri wrote:
> Should this be in some group?
> Alternatively, would it make sense to add it to new `-Wctad` group?
Let's not add a `-Wctad` until we have more than one thing to put in it. I
expect that to happen relatively soon :)
================
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:
> 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).
================
Comment at: lib/Sema/SemaInit.cpp:9268
+
+ // Record if at least one user-defined deduction guide was considered
+ bool HasUserDefinedDeductionGuideCandidate = false;
----------------
Nit: LLVM / Clang style wants a period at the end of a comment.
================
Comment at: lib/Sema/SemaInit.cpp:9287
+ HasUserDeclaredDeductionGuideCandidate |= !GD->isImplicit();
+
----------------
Quuxplusone wrote:
> Nitpick: I don't know if this is LLVM style, but I wish this were written as
>
> if (!GD->isImplicit())
> HasUserDeclaredDeductionGuideCandidate = true;
>
> for clarity. Also, is it "user-defined" (per the error message) or
> "user-declared" (per the name of this variable)?
Formally, it's actually just "any deduction guides". Constructors aren't
transformed into deduction guides; rather, deduction guides and constructors
both form candidates for CTAD. So `HasAnyDeductionGuides` would work. (I also
think we should omit the "candidates", because we don't care whether any
deduction guides were candidates for this particular overload resolution, only
whether there are any at all -- if we're excluding explicit deduction guides
and the only deduction guides are explicit, we still want to (and do!) suppress
the warning.
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
