This is an archive of the discontinued LLVM Phabricator instance.

[clang] Add test for CWG1111
ClosedPublic

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

Details

Reviewers
erichkeane
Group Reviewers
Restricted Project
Commits
rG5fc73b7502fb: [clang] Add test for CWG1111
Summary

Also mark CWG2385 as na, because it eliminates wording inconsistency introduced by CWG1111 resolution.

Diff Detail

Event Timeline

Endill created this revision.Jan 22 2023, 11:17 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 22 2023, 11:17 AM
Endill requested review of this revision.Jan 22 2023, 11:17 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 22 2023, 11:17 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
erichkeane added inline comments.
clang/test/CXX/drs/dr11xx.cpp
31

Can you also add the last example (with struct A) from the issue, and find a way to confirm that it is calling the right version?

clang/www/cxx_dr_status.html
6476

Were you able to track down which patch fixed this in clang 6?

I also don't think we can mark this as 'full' support, if its still broken in C++98. Unless we can find a good reason this wasn't/wont be done for C++98 as well, I don't know if we can mark this as 'done'.

aaron.ballman added inline comments.
clang/www/cxx_dr_status.html
6476

We can mark it as partial and put the reason in the details. I'm not certain if the python script has a way to do that easily or not.

shafik added a subscriber: shafik.Jan 23 2023, 7:48 AM
shafik added inline comments.
clang/www/cxx_dr_status.html
6476

I think this might be related as well: https://github.com/llvm/llvm-project/issues/59910

Endill added inline comments.Jan 23 2023, 8:04 AM
clang/www/cxx_dr_status.html
6476

Were you able to track down which patch fixed this in clang 6?

I haven't even tried, since I don't intend to fix this for now.

We can mark it as partial and put the reason in the details.

Is false-positive warning enough to degrade full to partial despite the correct behavior? Just confirming here.

I didn't internalize that it was 'just a warning' (it was a -error in the test above), we should have a task for someone to remove that warning as it is no longer ambiguous. But I don't think it makes it 'partial' here anymore.

clang/test/CXX/drs/dr11xx.cpp
1

I see what happened now: Add -Wno-error=ambiguous-member-template to this line.

I didn't internalize that it was 'just a warning' (it was a -error in the test above), we should have a task for someone to remove that warning as it is no longer ambiguous. But I don't think it makes it 'partial' here anymore.

It is an error, because -pedantic-errors is enabled across DR test suite. I wonder why, given that even notes can't escape -verify mode unhandled.

Endill added inline comments.Jan 23 2023, 8:39 AM
clang/test/CXX/drs/dr11xx.cpp
31

Nice catch, thank you. Long exposure to P1787 makes one pay less attention to resolutions in CWG issues, it seems :)

Endill updated this revision to Diff 492395.Jan 26 2023, 4:54 AM

Change status from "6" to "yes" since "ambiguous-member-template" warning is a false-positive now.

  • Disable "ambiguous-member-template" diagnostic in 98 and 03 modes
  • Add an example (struct A)
  • Apply clang-format
Endill added inline comments.Jan 26 2023, 4:57 AM
clang/test/CXX/drs/dr11xx.cpp
1

Thank you for suggestion, but I'd like to avoid making changes that could affect other tests (now and in the future), so I went pragma way.

31

Done!

erichkeane accepted this revision.Jan 26 2023, 5:56 AM
This revision is now accepted and ready to land.Jan 26 2023, 5:56 AM
Endill updated this revision to Diff 492430.Jan 26 2023, 6:37 AM
Endill edited the summary of this revision. (Show Details)

Move CWG2385 ("na") from D142316 into this patch. It makes more sense to put it here, because it fixes inconsistent wording introduced by CWG1111 resolution.

This revision was landed with ongoing or failed builds.Feb 10 2023, 11:53 PM
This revision was automatically updated to reflect the committed changes.