This is an archive of the discontinued LLVM Phabricator instance.

[clang] Add test for CWG2396
ClosedPublic

Authored by Endill on Jan 22 2023, 11:53 AM.

Details

Reviewers
shafik
Group Reviewers
Restricted Project
Commits
rG51a07fc24cb9: [clang] Add test for CWG2396
Summary

Also mark CWG1291 as "na".

P1787: CWG1291 and CWG2396 are resolved by explicitly specifying how to look up names in a conversion-type-id.
Wording: see changes to [basic.lookup.unqual]/5 and [basic.lookup.qual]/2.

"Calling a conversion function" example in P1787 is also relevant.

Diff Detail

Event Timeline

Endill created this revision.Jan 22 2023, 11:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 22 2023, 11:53 AM
Endill requested review of this revision.Jan 22 2023, 11:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 22 2023, 11:53 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
shafik added a subscriber: shafik.EditedJan 23 2023, 8:03 AM

You mention CWG2385 as na but you don't explain how it was resolved, was it superceded by p1787.

Also nitpick you mentioned P1787 in the description but it links to a phab review, you should probably just use a link like I did above to refer to papers. Same with the defect reports also like I did above. Folks may not know how to find defect reports and or papers.

You mention CWG2385 as na but you don't explain how it was resolved, was it superceded by p1787.

CWG2385 points out to inconsistency in wording introduced by resolution of CWG1111, so I consider it a pure wording change. This issue, as well as aforementioned CWG1111, were resolved independently of P1787, hence I didn't provide any explanation for them.

Also nitpick you mentioned P1787 in the description but it links to a phab review, you should probably just use a link like I did above to refer to papers. Same with the defect reports also like I did above. Folks may not know how to find defect reports and or papers.

I felt that it's going to worsen the readability of raw commit messages, but I can indeed use markdown links if it helps.

Endill updated this revision to Diff 492433.Jan 26 2023, 6:47 AM
Endill edited the summary of this revision. (Show Details)

Move CWG2385 out to D142315, because it's been resolved prior to P1787, and it's easier to explain it there.

shafik added inline comments.Jan 26 2023, 1:25 PM
clang/test/CXX/drs/dr23xx.cpp
204

While gcc accepts the first three it does not like the last two: https://godbolt.org/z/js8Pz14Eo

I believe they should also be covered but not confident.

Endill added inline comments.Jan 27 2023, 2:52 AM
clang/test/CXX/drs/dr23xx.cpp
204

I agree they should. I can't find any special considerations in the standard regarding unqualified name lookup of template arguments.

Are there any action items for me here?

shafik added inline comments.Jan 30 2023, 8:46 AM
clang/test/CXX/drs/dr23xx.cpp
204

Yeah, can you file a gcc bug report for the last two? If they agree it is a gcc but then we are all good, if not then we need to see what they say.

Endill marked an inline comment as not done.Feb 12 2023, 4:44 AM
Endill added inline comments.
clang/test/CXX/drs/dr23xx.cpp
204

Upon closer inspection, I think I'm wrong to put those two tests that GCC does not accept:

  1. basic.lookup.unqual#5: An unqualified name that is a component name of a type-specifier or ptr-operator of a conversion-type-id is looked up in the same fashion as the conversion-function-id in which it appears
  1. expr.prim.id.unqual#2: A component name of an unqualified-id U is: — U if it is a name or; — the component name of the template-id or type-name of U, if any.
  1. temp.names#2: The component name of a simple-template-id, template-id, or template-name is the first name in it

As I understand it now, identity is a component name of ptr-declarator and falls under "is looked up in the same fashion as the conversion-function-id in which it appears", but its argument B does not.

Endill updated this revision to Diff 496849.Feb 12 2023, 11:51 PM

Remove two examples with identity<B> for reasons explained in D142316#4121079.

shafik accepted this revision.Feb 13 2023, 10:32 AM

LGTM

This revision is now accepted and ready to land.Feb 13 2023, 10:32 AM
This revision was landed with ongoing or failed builds.Feb 13 2023, 10:53 AM
This revision was automatically updated to reflect the committed changes.