aaron.ballman 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}}
+}
----------------
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).
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
