[PATCH] D148433: [clang] Add tests for DRs about complete-class context

Shafik Yaghmour via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 17 11:59:48 PDT 2023


shafik added inline comments.


================
Comment at: clang/test/CXX/drs/dr23xx.cpp:42
 
+namespace dr2335 { // dr2335: no drafting
+// FIXME: all of the examples are well-formed.
----------------
Endill wrote:
> aaron.ballman wrote:
> > Endill wrote:
> > > shafik wrote:
> > > > My comment on 1890 applies here as well.
> > > > 
> > > > CC @rsmith @aaron.ballman how should we handle DRs that are still in process? While we may think we know the direction it is going in, it could change.
> > > > 
> > > > So maybe we should avoid expressing an opinion on whether these are well-formed or not?
> > > I asked Aaron even before uploading the patch. His response was:
> > > > I think it's a judgement call -- if the CWG consensus seems like it makes a lot of sense, then I see no reason not to test them but maybe leave a FIXME comment about the issue technically being open still. If the CWG consensus doesn't make sense, that might be time to get on the reflectors and ask questions
> > > 
> > > Consensus documented in 2335 and drafting notes in P1787 that I quote in the summary made sense to me, so I published this patch. I can abandon it if there are fundamental issues with the them, rendering my judgement wrong.
> > I'd be curious to hear more thoughts on this.
> > 
> > In this particular case, this has been in drafting status since 2017, but the final comments on the open issue are:
> > ```
> > Notes from the June, 2018 meeting:
> > 
> > The consensus of CWG was to treat templates and classes the same by "instantiating" delayed-parse regions when they are needed instead of at the end of the class.
> > 
> > See also issue 1890.
> > ```
> > which seemed sufficiently firm in direction to warrant testing the behavior. I think any open DR being tested is subject to change in Clang and should continue to be documented in cxx_dr_status.html as "not resolved", though (so if CWG does make a change, we can still react to it without having promised the current behavior to users).
> As a recap, in D138901 I updated `make_cxx_dr_status` script, so that it can take into account unresolved issues. Precedents were 2565 and 2628, which resorted to editing `cxx_dr_status.html` manually. Script now make sure that `open` or `drafting` bit in the comment matches status from the official page, throwing an exception otherwise.
I think documenting this is important.

What Aaron says makes sense to me, I just find the current comment `all of the examples are well-formed.` feels like a promise even though in the status page we may say not resolved. 

I feel like the wording should be more cautious, maybe more like `Current consensus is that the examples are well-formed`.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148433



More information about the cfe-commits mailing list