This is an archive of the discontinued LLVM Phabricator instance.

[Clang][Sema] Add -Wctad-selects-copy to diagnose copy deduction candidate
Needs ReviewPublic

Authored by rZhBoYao on Jul 17 2023, 8:41 AM.

Details

Reviewers
EricWF
rsmith
aaron.ballman
Group Reviewers
Restricted Project
Summary
  1. '-Wctad-selects-copy' warns when copy deduction candidate is selected in CTAD, and the initialization is a direct-initialization. The warning is suppressed when the class template is in a system header and has at least one explicit deduction guide.
  2. Add a group '-Wctad-maybe-unintended' controlling '-Wctad-selects-copy' and '-Wctad-maybe-unsupported'.

Fixes #63288

Diff Detail

Event Timeline

rZhBoYao created this revision.Jul 17 2023, 8:41 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 17 2023, 8:41 AM
rZhBoYao requested review of this revision.Jul 17 2023, 8:41 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 17 2023, 8:41 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
rZhBoYao added a reviewer: Restricted Project.Jul 17 2023, 8:41 AM
rZhBoYao updated this revision to Diff 541151.Jul 17 2023, 11:17 AM
rZhBoYao retitled this revision from [Clang][Sema] Add -Wctad-copy-not-wrap to diagnose copy deduction candidate to [Clang][Sema] Add -Wctad-selects-copy to diagnose copy deduction candidate.
rZhBoYao edited the summary of this revision. (Show Details)
shafik added a subscriber: shafik.

Adding Aaron.

I have a couple questions but overall I think this is a great warning to add.
Sorry it took us a while to get to your PR

clang/include/clang/Basic/DiagnosticGroups.td
1384

Maybe we should just call that Wctad?

clang/include/clang/Basic/DiagnosticSemaKinds.td
2491–2494

This message is a bit difficult to understand.
Maybe The type of this expression is Y<Z> because it was deduced by CTAD from the copy constructor of Y?
It's not an easy warning to phrase!

clang/lib/Sema/SemaInit.cpp
22

I don't think that adding this header is necessary

10932

So the idea here is that the standard library would have protected against that by providing deduction guides?
Which is funny because the original issue was reported because they did not.
Do you have an example of when we should not emit the diagnostic for a library type?

@ldionne @philnik Opinions on this warning?

clang/test/SemaTemplate/ctad.cpp
51
philnik added inline comments.Sep 14 2023, 10:02 AM
clang/lib/Sema/SemaInit.cpp
10932

I think this definitely has to be suppressed in system headers somehow. e.g. as_rvalue_view uses the copy ctor CTAD to avoid double-wrapping iterators. (see https://github.com/llvm/llvm-project/blob/main/libcxx/include/__ranges/as_rvalue_view.h#L75). OTOH I'm not sure whether this should be suppressed if the type is from a system header, since reverse_iterator{some-other-reverse-iterator} might not be intended either, regardless of deduction guides.