[PATCH] D56731: Add -Wimplicit-ctad warning to diagnose CTAD on types with no user defined deduction guides.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2129
+def warn_class_template_argument_deduction_no_user_defined_guides : Warning<
+  "class template argument deduction for %0 that has no user defined deduction guides" >,
+  InGroup<ImplicitCTADUsage>, DefaultIgnore;
----------------
`s/user defined/user-defined/`, and perhaps `s/that has/with/` for brevity.


================
Comment at: lib/Sema/SemaInit.cpp:9287
 
+      HasUserDeclaredDeductionGuideCandidate |= !GD->isImplicit();
+
----------------
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)?


================
Comment at: test/SemaCXX/cxx1z-class-template-argument-deduction.cpp:426
+// expected-warning at +1 {{class template argument deduction for 'NoExplicit' that has no user defined deduction guides}}
+NoExplicit ne(42);
+
----------------
I suggest that additional (more realistic) motivating examples can be found in STL's CppCon 2018 talk. E.g.

```
template<class T, class U>
struct Pair {
    T first; U second;
    explicit Pair(const T& t, const U& u) : first(t), second(u) {}
};
Pair p(42, "hello world");  // constructs a Pair<int, char[12]>

template<class T, class U>
struct Pair2 {
    T first; U second;
    explicit Pair2(T t, U u) : first(t), second(u) {}
};
Pair2 p(42, "hello world");  // constructs a Pair2<int, const char*>
```

I would expect (and, with your patch, receive) diagnostics for both of these.

But for something like `Vector("a", "b")` your patch gives no diagnostic: https://godbolt.org/z/_9zhav
And for something like [`std::optional o(x);` in generic context](https://quuxplusone.github.io/blog/2018/12/11/dont-inherit-from-std-types/) your patch gives no diagnostic.

I do have a patch out for a general-purpose `-Wctad` that would give warnings on those latter cases as well: D54565
I think `-Wimplicit-ctad` is a good start, but it doesn't solve all the cases I'd like to see solved.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56731/new/

https://reviews.llvm.org/D56731