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