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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
