[PATCH] D147920: [clang] Add test for CWG399

[Vlad Serebrennikov via Phabricator via cfe-commits]

Endill added a comment.

In D147920#4258680 <https://reviews.llvm.org/D147920#4258680>, @shafik wrote:

> In D147920#4257369 <https://reviews.llvm.org/D147920#4257369>, @Endill wrote:
>
>> I think I haven't stressed it enough, but this whole test is copied from dr244, which is written by Richard.
>
> Understood, I appreciate the patience in explaining what I am missing. Sometimes that means things could be explained better.

No worries. Those DRs are involved enough that it could be challenging to understand the context quickly.



================
Comment at: clang/test/CXX/drs/dr3xx.cpp:1492
+    // This is technically ill-formed; G is looked up in 'N::' and is not found.
+    // Rejecting this seems correct, but most compilers accept, so we do also.
+    f.N::F::~G(); // expected-error {{qualified destructor name only found in lexical scope; omit the qualifier to find this type name by unqualified lookup}}
----------------
shafik wrote:
> Endill wrote:
> > shafik wrote:
> > > You say we accept the next line but it has an `expected-error` on it?
> > It's an error because of `-pedantic-errors`. It's a warning by default.
> That makes a lot more sense, I was wondering what was I missing.
> 
> Can we note that in the comment b/c it is pretty confusing otherwise. 
> 
> I wonder if there is a good reason to not make this ill-formed by default? Worth a bug report.
> Can we note that in the comment b/c it is pretty confusing otherwise.

I get where you come from, but all DR testing is done under `-pedantic-errors`. Do you have ideas for a more systematic approach? One of the options is to use `-Wpedantic` instead, but I expect that to require a decent amount of mechanical replacements for existing tests, which LLVM community doesn't appreciate, as far as I know. I'll edit the comment if we won't come up with something better.

> I wonder if there is a good reason to not make this ill-formed by default? Worth a bug report.

Does the fact that we accepted such code since (at least) 3.5 through 10 make for a good reason? https://godbolt.org/z/16GYWh3Po


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147920