[PATCH] D63894: [CXX] Exercise all paths through these tests

[Paul Robinson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL36: [CXX] Exercise all paths through these tests. 
(authored by probinson, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63894?vs=206921=208802#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63894

Files:
  cfe/trunk/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
  cfe/trunk/test/SemaCXX/class.cpp
  cfe/trunk/test/SemaCXX/linkage2.cpp


Index: cfe/trunk/test/SemaCXX/linkage2.cpp
===
--- cfe/trunk/test/SemaCXX/linkage2.cpp
+++ cfe/trunk/test/SemaCXX/linkage2.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=gnu++11 %s
-// RUN: %clang_cc1 -fsyntax-only -verify -Wno-c++11-extensions 
-Wno-local-type-template-args %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-c++11-extensions 
-Wno-local-type-template-args %s -std=gnu++98
 // RUN: %clang_cc1 -fsyntax-only -verify -Wno-c++11-extensions 
-Wno-local-type-template-args -fmodules %s
 
 namespace test1 {
Index: cfe/trunk/test/SemaCXX/class.cpp
===
--- cfe/trunk/test/SemaCXX/class.cpp
+++ cfe/trunk/test/SemaCXX/class.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wc++11-compat %s 
+// RUN: %clang_cc1 -fsyntax-only -verify -Wc++11-compat %s -std=c++98
 class C {
 public:
   auto int errx; // expected-error {{storage class specified for a member 
declaration}}
Index: cfe/trunk/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
===
--- cfe/trunk/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
+++ cfe/trunk/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
 
 // C++03 [namespace.udecl]p4:


Index: cfe/trunk/test/SemaCXX/linkage2.cpp
===
--- cfe/trunk/test/SemaCXX/linkage2.cpp
+++ cfe/trunk/test/SemaCXX/linkage2.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=gnu++11 %s
-// RUN: %clang_cc1 -fsyntax-only -verify -Wno-c++11-extensions -Wno-local-type-template-args %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-c++11-extensions -Wno-local-type-template-args %s -std=gnu++98
 // RUN: %clang_cc1 -fsyntax-only -verify -Wno-c++11-extensions -Wno-local-type-template-args -fmodules %s
 
 namespace test1 {
Index: cfe/trunk/test/SemaCXX/class.cpp
===
--- cfe/trunk/test/SemaCXX/class.cpp
+++ cfe/trunk/test/SemaCXX/class.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wc++11-compat %s 
+// RUN: %clang_cc1 -fsyntax-only -verify -Wc++11-compat %s -std=c++98
 class C {
 public:
   auto int errx; // expected-error {{storage class specified for a member declaration}}
Index: cfe/trunk/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
===
--- cfe/trunk/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
+++ cfe/trunk/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
 
 // C++03 [namespace.udecl]p4:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63894: [CXX] Exercise all paths through these tests

[Paul Robinson via Phabricator via cfe-commits]

probinson marked an inline comment as done.
probinson added inline comments.



Comment at: clang/test/SemaCXX/linkage2.cpp:3
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-c++11-extensions 
-Wno-local-type-template-args %s -std=gnu++98
 // RUN: %clang_cc1 -fsyntax-only -verify -Wno-c++11-extensions 
-Wno-local-type-template-args -fmodules %s
 

rsmith wrote:
> Should this one be run in C++98 mode too? (The use of `-Wno-c++11-extensions` 
> suggests to me that C++98 was expected.)
I don't think so; `-Wno-c++11-extensions` will not suppress diagnostics about 
GNU extensions.  I think GNU++98 is what's wanted here (and is the old default 
dialect).


Repository:
  rC Clang

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

https://reviews.llvm.org/D63894



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

[PATCH] D63894: [CXX] Exercise all paths through these tests

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith accepted this revision.
rsmith added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/test/SemaCXX/linkage2.cpp:3
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-c++11-extensions 
-Wno-local-type-template-args %s -std=gnu++98
 // RUN: %clang_cc1 -fsyntax-only -verify -Wno-c++11-extensions 
-Wno-local-type-template-args -fmodules %s
 

Should this one be run in C++98 mode too? (The use of `-Wno-c++11-extensions` 
suggests to me that C++98 was expected.)


Repository:
  rC Clang

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

https://reviews.llvm.org/D63894



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

[PATCH] D63894: [CXX] Exercise all paths through these tests

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In D63894#1576288 , @probinson wrote:

> there is no reason to think those paths are tested elsewhere.


Well that may be a tad strong. But I'm reluctant to remove them unless they are 
demonstrably tested elsewhere, and it's not clear to me how to look.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63894



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

[PATCH] D63894: [CXX] Exercise all paths through these tests

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In D63894#1574419 , @dblaikie wrote:

> I would've assumed these conditionals were added by Sony folks for their 
> change in default dialect - that doesn't necessarily mean these tests are 
> needed upstream (the functionality may be tested elsewhere)


For two of the files, the conditionals were added by Richard Smith, not Sony.  
For the Sony case, it is the *older* path that was upstream originally, and 
that is the one that the tests stopped testing; there is no reason to think 
those paths are tested elsewhere.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63894



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

[PATCH] D63894: [CXX] Exercise all paths through these tests

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

I would've assumed these conditionals were added by Sony folks for their change 
in default dialect - that doesn't necessarily mean these tests are needed 
upstream (the functionality may be tested elsewhere)


Repository:
  rC Clang

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

https://reviews.llvm.org/D63894



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

[PATCH] D63894: [CXX] Exercise all paths through these tests

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

Ping. This is pretty straightforward, the only question is whether we want to 
preserve these older-dialect tests or rip them out.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63894



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

[PATCH] D63894: [CXX] Exercise all paths through these tests

[Paul Robinson via Phabricator via cfe-commits]

probinson created this revision.
probinson added a reviewer: rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

These 3 tests have dialect-based conditionals, but weren't running Clang with 
enough different dialects to actually enable those conditional sections.


Repository:
  rC Clang

https://reviews.llvm.org/D63894

Files:
  clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
  clang/test/SemaCXX/class.cpp
  clang/test/SemaCXX/linkage2.cpp


Index: clang/test/SemaCXX/linkage2.cpp
===
--- clang/test/SemaCXX/linkage2.cpp
+++ clang/test/SemaCXX/linkage2.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=gnu++11 %s
-// RUN: %clang_cc1 -fsyntax-only -verify -Wno-c++11-extensions 
-Wno-local-type-template-args %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-c++11-extensions 
-Wno-local-type-template-args %s -std=gnu++98
 // RUN: %clang_cc1 -fsyntax-only -verify -Wno-c++11-extensions 
-Wno-local-type-template-args -fmodules %s
 
 namespace test1 {
Index: clang/test/SemaCXX/class.cpp
===
--- clang/test/SemaCXX/class.cpp
+++ clang/test/SemaCXX/class.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wc++11-compat %s 
+// RUN: %clang_cc1 -fsyntax-only -verify -Wc++11-compat %s -std=c++98
 class C {
 public:
   auto int errx; // expected-error {{storage class specified for a member 
declaration}}
Index: clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
===
--- clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
+++ clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
 
 // C++03 [namespace.udecl]p4:


Index: clang/test/SemaCXX/linkage2.cpp
===
--- clang/test/SemaCXX/linkage2.cpp
+++ clang/test/SemaCXX/linkage2.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=gnu++11 %s
-// RUN: %clang_cc1 -fsyntax-only -verify -Wno-c++11-extensions -Wno-local-type-template-args %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-c++11-extensions -Wno-local-type-template-args %s -std=gnu++98
 // RUN: %clang_cc1 -fsyntax-only -verify -Wno-c++11-extensions -Wno-local-type-template-args -fmodules %s
 
 namespace test1 {
Index: clang/test/SemaCXX/class.cpp
===
--- clang/test/SemaCXX/class.cpp
+++ clang/test/SemaCXX/class.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wc++11-compat %s 
+// RUN: %clang_cc1 -fsyntax-only -verify -Wc++11-compat %s -std=c++98
 class C {
 public:
   auto int errx; // expected-error {{storage class specified for a member declaration}}
Index: clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
===
--- clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
+++ clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
 
 // C++03 [namespace.udecl]p4:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits