[clang] Choose non-templated ctor as deduction guide unambiguously #66487
Conversation
llvmbot
added
clang
If there are two guides, one of them generated from a non-templated constructor and the other from a templated constructor, then the standard gives priority to the first. Clang detected ambiguity before, now the correct guide is chosen. As an unrelated minor change, fix the issue llvm#64020, which could've led to incorrect behavior if further development inserted code after a call to isAddressSpaceSubsetOf() which specified the two parameters in the wrong order.
|
@zygoloid @yuanfang-chen @faisalv @OleStrohm |
There was a problem hiding this comment.
This seems right to me, however we need to do 2 things:
1- This needs a release note.
2- We need to track down when this changed in the standard, and why. If it was a DR, we need to do make sure we update that we've implemented that DR. If it was a normal paper, we need to figure out which versions of the standard it applies to and do THAT correctly. Either way, we'd have to update the paper tracking doc.
|
I added a comment in the issue her: #67959 (comment) Please make sure to add references to the standard in appropriate places in this PR. |
|
The paper where this came from is: https://wg21.link/P0620R0. most of this paper was already implemented. Where should I reference the standard in the PR? In the Description? |
|
Deduction guides were added in C++17, and This paper is also in C++17, so I don't think we need to check for versions in the code (I don't create any deduction guides I just use them so before C++17 my code is never used). |
cxx_status.html is the paper tracking doc, if we claim it was already implemented than there is nothing to do there. For release notes, ForC++Support seems like the right area for me. I've confirmed that P06020R0 is in N4659 (Final draft for C++17), so you're right, there is no version checking we have to do. Thanks for doing the leg work here! |
clang/test/CXX/over/over.match/over.match.funcs/over.match.class.deduct/p2.cpp
Outdated
Show resolved
Hide resolved
|
@erichkeane @shafik I don't have write access. Could you merge if there is nothing to be done? |
clang/test/CXX/over/over.match/over.match.funcs/over.match.class.deduct/p2.cpp
Outdated
Show resolved
Hide resolved
whisperity
changed the title
|
I had permissions, so I squash/merged. |
|
It looks like the authorship metadata was set up incorrectly in the commits, note the typo: |
|
Ah, I see, apologies. I thought you meant the resign/etc complaint meant you couldn't commit through the UI. |
If there are two guides, one of them generated from a non-templated constructor
and the other from a templated constructor, then the standard gives priority to
the first. Clang detected ambiguity before, now the correct guide is chosen.
The correct behavior is described in this paper: http://wg21.link/P0620R0
Example for the bug: http://godbolt.org/z/ee3e9qG78
As an unrelated minor change, fix the issue #64020,
which could've led to incorrect behavior if further development inserted code after a call to
isAddressSpaceSubsetOf(), which specified the two parameters in the wrong order.