Endill added inline comments.

================
Comment at: clang/test/CXX/drs/dr6xx.cpp:18
+  sp->f(2);
+  sp->f(2.2); // expected-error {{is a private member}}
+}
----------------
aaron.ballman wrote:
> Endill wrote:
> > Endill wrote:
> > > shafik wrote:
> > > > Maybe add a comment above this saying something like:
> > > > 
> > > > ```
> > > > // access control is applied after overload resolution
> > > > // [class.access.general]p4 "For an overload set, access control is 
> > > > applied only to the function selected by overload resolution."
> > > > ```
> > > I tend to like the idea, but I wonder about general rule for adding such 
> > > explanations. Currently DR tests contain very little of those. 
> > > 
> > > If we're going to add explanations, we should also decide whether we're 
> > > going to cite the standard, or paraphrase (and/or) explain intent. My 
> > > concern is that both references to standard and citations could grow old 
> > > relatively quickly, and we don't have any tools to help, at least yet.
> > @aaron.ballman What do you think?
> We don't typically add a significant amount of comments to test files unless 
> what is being tested needs some explanation. So, IMO, if the DR test case is 
> "tricky" in some way and we're trying to demonstrate that we're testing a 
> specific sentence or two from the standard, then I think a comment with 
> standards wording is reasonable (please don't just cite `[foo.bar]p12` though 
> -- add the standards wording!). However, if the test is pretty 
> straightforward, or the DR explains in more detail what's going on, then I 
> don't think we need to add the comment (the references get stale rather 
> quickly, even with stable names as in C++).
> 
> All that said, if you're on the fence about whether to add a comment or not, 
> go ahead and add it (IMO).
I think that this one is the case when DR explains it better, because that's 
what it is about. Adding a comment seems more appropriate for something that's 
not given enough attention or accent in the whole context of a DR test, as 
opposed to test code itself.

@shafik Is it fine by you if I don't add any comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139173

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to