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