[PATCH] D139095: [clang] Mark CWG405 as a duplicate of CWG218

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D139095#3966626 <https://reviews.llvm.org/D139095#3966626>, @Endill wrote:

>>> We can do it the following way then: // dr405: yes \n // NB: also dup 218.
>>
>> That would be fine by me!
>
> Which way should we handle this? I'd prefer to do it without test duplication, but making it clear for readers is a serious concern indeed. (Read on, I elaborate on this below.)

Sorry, I was unclear! I meant that I would be fine with:

  ... test code ... // dr405: yes
                    // NB: also dup 218
  ...
  ...
  ...
  
  ... test code ... // dr218: yes
                    // NB: also dup 405

So the test code is duplicated AND we've left a comment linking the two tests together.

>>> What test for 405 is going to be if not a copy-and-paste of a part of 218 test?
>>
>> In terms of test *coverage*, no benefit. In terms of *implementation status*, it makes it clear we considered the DR explicitly instead of leaving future folks to wonder.
>
> Should we really try to meet an expectation that Clang developers haven't considered something as obvious as explicitly handling a DR not marked as duplicated or superseded officially? It doesn't seem a reasonable expectation to me, and at least my personal attitude have always been the opposite.
>
> To be fully honest here, I don't even remember myself how I came across CWG218, because I did this test back in spring. So marking it as a duplicate (one way or another) on the contrary seems very considerate handling.

There's two approaches: assume it's obvious that since it's a duplicate we thought about it, or put in test coverage + comments to prove we thought about it. I'd prefer putting in the test coverage in this case -- I've been bitten too many times by "was <this> considered?" when doing code archeology in the project (granted, most of the severe cases come from when we were doing far less patch review before committing things).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139095