Endill added a comment.
@shafik ping
================
Comment at: clang/test/CXX/drs/dr3xx.cpp:1439
+
+namespace dr399 { // dr399: 11
+ // NB: reuse dr244 test
----------------
aaron.ballman wrote:
> 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?
> @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
