aaron.ballman accepted this revision.
aaron.ballman added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clang/test/CXX/drs/dr3xx.cpp:1439
+
+namespace dr399 { // dr399: 11
+ // NB: reuse dr244 test
----------------
aaron.ballman wrote:
> shafik wrote:
> > Endill wrote:
> > > shafik wrote:
> > > > Endill wrote:
> > > > > Despite a couple of FIXME in CWG244 test (out of dozens of examples),
> > > > > it claims full availability since Clang 11. I'd take a more
> > > > > conservative approach, declaring partial support, but I think that
> > > > > declaring different availability for the same test would bring
> > > > > unnecessary confusion. So I followed CWG244 availability.
> > > > >
> > > > > Alternative is to demote CWG244 to partial, but I'm not sure we
> > > > > should go back on our claims for CWG support that has been out for so
> > > > > long.
> > > > I think the bugs are not awful, we should file bug reports if we don't
> > > > already have them. Some of them seem like they should be not too bad to
> > > > fix.
> > > >
> > > > CC @aaron.ballman to get a second opinion
> > > If we are to file bug reports, I'm not sure what wording makes those
> > > examples ill-formed. Is it [[
> > > http://eel.is/c++draft/basic.lookup#qual.general-4.6 | qual.general-4.6
> > > ]]: `The type-name that is or contains Q shall refer to its (original)
> > > lookup context (ignoring cv-qualification) under the interpretation
> > > established by at least one (successful) lookup performed.`? I interpret
> > > it as requiring names to the left and to the right of `~` to be found in
> > > the same scope (lookup context; `namespace dr244` in our case). Could it
> > > actually mean that they have to refer to the same type?
> > I am not sure maybe @rsmith might be able to help us here.
> I think we want to start being more conservative with claiming support for
> features and DRs, and that means being more honest with "partial" markings
> (with comments as to WHY the support is only partial, what's still left to be
> done, etc). I don't think it's a problem to say "we've discovered enough
> issues with this that we no longer claim to support it" when that's accurate.
>
> I don't think we have a hard and fast rule for when a bug is sufficiently
> worrying to merit partial vs full support; it's going to depend on the
> situation, I think. Failing to diagnose incorrect code is a different kind of
> problem from diagnosing correct code from crashing bugs from etc. and it's
> going to be up to the patch author and reviewers to make a value judgement.
> But that's why I think it's fine for us to update the status when we learn
> more information, too.
>
> That said, when we do have partial support, we definitely need to file issues
> to address the remaining bits at some point.
>
> > I interpret it as requiring names to the left and to the right of ~ to be
> > found in the same scope (lookup context; namespace dr244 in our case).
> > Could it actually mean that they have to refer to the same type?
>
> I've read that wording a few times now and can't make heads or tails of what
> it's trying to say. Perhaps @rsmith or @hubert.reinterpretcast can help
> illuminate us?
@Endill reminded me off-list that the FIXME comments here are existing
comments; some of these test cases are lifted from the dr244 test cases. Given
that and it's been a few weeks and we've not determine what issues to file, I
think we should unblock this review as it makes forward progress on our test
coverage for dr399. Filing issues would be good, but not a prerequisite for
landing this. WDYT @shafik?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D147920/new/
https://reviews.llvm.org/D147920
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
