[clang] Cover CWG issues about export template

[Endilll]

This PR covers the following Core issues:
CWG204 "Exported class templates"
CWG323 "Where must export appear?"
CWG335 "Allowing export on template members of nontemplate classes"
CWG820 "Deprecation of export"

I believe the list above is entirety of Core issues that are dedicated solely to export template.

I believe we have two main points of view here, which command what this PR should do:

1. (easy) Removal of export template was done as a defect report in CWG820, and the rest are effectively superseded by it, because we apply defect reports retroactively.
2. (harder) Those Core issues are testable individually, so we should test them for the behavior Core wanted at the time.

This PR implements the first option, making our C++ DR status page greener.
I think I can be persuaded to go with the second option, if reviewers have strong preference for it.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Vlad Serebrennikov (Endilll)

Changes

This PR covers the following Core issues:
CWG204 "Exported class templates"
CWG323 "Where must export appear?"
CWG335 "Allowing export on template members of nontemplate classes"
CWG820 "Deprecation of export"

I believe the list above is entirety of Core issues that are dedicated solely to export template.

I believe we have two main points of view here, which command what this PR should do:

1. (easy) Removal of export template was done as a defect report in CWG820, and the rest are effectively superseded by it, because we apply defect reports retroactively.
2. (harder) Those Core issues are testable individually, so we should test them for the behavior Core wanted at the time.

This PR implements the first option, making our C++ DR status page greener.
I think I can be persuaded to go with the second option, if reviewers have strong preference for it.

---

Full diff: https://github.com/llvm/llvm-project/pull/94876.diff

4 Files Affected:

• (modified) clang/test/CXX/drs/cwg2xx.cpp (+1-1)
• (modified) clang/test/CXX/drs/cwg3xx.cpp (+2-2)
• (modified) clang/test/CXX/drs/cwg8xx.cpp (+15-10)
• (modified) clang/www/cxx_dr_status.html (+4-4)

diff --git a/clang/test/CXX/drs/cwg2xx.cpp b/clang/test/CXX/drs/cwg2xx.cpp
index 99916dea9a912..926cb19596026 100644
--- a/clang/test/CXX/drs/cwg2xx.cpp
+++ b/clang/test/CXX/drs/cwg2xx.cpp
@@ -41,7 +41,7 @@ namespace cwg202 { // cwg202: 3.1
   template struct X<f>;
 }
 
-// FIXME (export) cwg204: no
+// cwg204: sup 820
 
 namespace cwg206 { // cwg206: yes
   struct S; // #cwg206-S
diff --git a/clang/test/CXX/drs/cwg3xx.cpp b/clang/test/CXX/drs/cwg3xx.cpp
index 94227dc031c6a..a10ed95941ba4 100644
--- a/clang/test/CXX/drs/cwg3xx.cpp
+++ b/clang/test/CXX/drs/cwg3xx.cpp
@@ -377,7 +377,7 @@ namespace cwg322 { // cwg322: 2.8
   int &s = a;
 }
 
-// cwg323: no
+// cwg323: sup 820
 
 namespace cwg324 { // cwg324: 3.6
   struct S { int n : 1; } s; // #cwg324-n
@@ -587,7 +587,7 @@ namespace cwg334 { // cwg334: yes
   template void f<S>();
 }
 
-// cwg335: no
+// cwg335: sup 820
 
 namespace cwg336 { // cwg336: yes
   namespace Pre {
diff --git a/clang/test/CXX/drs/cwg8xx.cpp b/clang/test/CXX/drs/cwg8xx.cpp
index eba601300584d..c8cbdfcee3f4d 100644
--- a/clang/test/CXX/drs/cwg8xx.cpp
+++ b/clang/test/CXX/drs/cwg8xx.cpp
@@ -1,14 +1,19 @@
-// RUN: %clang_cc1 -std=c++98 -triple x86_64-unknown-unknown %s -verify=expected -fexceptions -fcxx-exceptions -pedantic-errors
-// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown %s -verify=expected,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
-// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown %s -verify=expected,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
-// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-unknown %s -verify=expected,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
-// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-unknown %s -verify=expected,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
-// RUN: %clang_cc1 -std=c++23 -triple x86_64-unknown-unknown %s -verify=expected,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
-// RUN: %clang_cc1 -std=c++2c -triple x86_64-unknown-unknown %s -verify=expected,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++98 -triple x86_64-unknown-unknown %s -verify=expected,cxx98-17 -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown %s -verify=expected,cxx98-17,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown %s -verify=expected,cxx98-17,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-unknown %s -verify=expected,cxx98-17,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-unknown %s -verify=expected,since-cxx20,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++23 -triple x86_64-unknown-unknown %s -verify=expected,since-cxx20,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++2c -triple x86_64-unknown-unknown %s -verify=expected,since-cxx20,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
 
-#if __cplusplus == 199711L
-// expected-no-diagnostics
-#endif
+namespace cwg820 { // cwg820: 2.7
+export template <class T> struct B {};
+// cxx98-17-warning@-1 {{exported templates are unsupported}}
+// since-cxx20-error@-2 {{export declaration can only be used within a module purview}}
+export template<typename T> void f() {}
+// cxx98-17-warning@-1 {{exported templates are unsupported}}
+// since-cxx20-error@-2 {{export declaration can only be used within a module purview}}
+}
 
 namespace cwg873 { // cwg873: 3.0
 #if __cplusplus >= 201103L
diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index 5e2ab06701703..3fdf6bae0a3d5 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -1262,7 +1262,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
     <td><a href="https://cplusplus.github.io/CWG/issues/204.html">204</a></td>
     <td>CD1</td>
     <td>Exported class templates</td>
-    <td class="none" align="center">No</td>
+    <td class="full" align="center">Superseded by <a href="#820">820</a></td>
   </tr>
   <tr class="open" id="205">
     <td><a href="https://cplusplus.github.io/CWG/issues/205.html">205</a></td>
@@ -1978,7 +1978,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
     <td><a href="https://cplusplus.github.io/CWG/issues/323.html">323</a></td>
     <td>CD1</td>
     <td>Where must <TT>export</TT> appear?</td>
-    <td class="none" align="center">No</td>
+    <td class="full" align="center">Superseded by <a href="#820">820</a></td>
   </tr>
   <tr id="324">
     <td><a href="https://cplusplus.github.io/CWG/issues/324.html">324</a></td>
@@ -2050,7 +2050,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
     <td><a href="https://cplusplus.github.io/CWG/issues/335.html">335</a></td>
     <td>CD1</td>
     <td>Allowing <TT>export</TT> on template members of nontemplate classes</td>
-    <td class="none" align="center">No</td>
+    <td class="full" align="center">Superseded by <a href="#820">820</a></td>
   </tr>
   <tr id="336">
     <td><a href="https://cplusplus.github.io/CWG/issues/336.html">336</a></td>
@@ -4914,7 +4914,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
     <td><a href="https://cplusplus.github.io/CWG/issues/820.html">820</a></td>
     <td>CD2</td>
     <td>Deprecation of <TT>export</TT></td>
-    <td class="unknown" align="center">Unknown</td>
+    <td class="full" align="center">Clang 2.7</td>
   </tr>
   <tr id="822">
     <td><a href="https://cplusplus.github.io/CWG/issues/822.html">822</a></td>

[cor3ntin]

I would prefer we say N/A rather than pretend we support something that was never implemented (to the extend i think it might be better to have an error about modules rather than exported templates in older language modes)

[Endilll]

Using N/A for 204, 323, and 335 would be novel, because at the moment it's used for wording changes that do not affect implementations. Those Core issues clearly affected the implementations back when they were resolved.

[cor3ntin]

We never implemented export template - and no one did, therefore we are not affected by changes to it (no one did except for EDG, i don't even know if they shipped it)

It is perfectly reasonable not to have tests whatsoever for that either (although I find the pre-c++20 diagnostic funny, we would be better of saying that modules are a c++20 feature).

[Endilll]

We never implemented export template - and no one did, therefore we are not affected by changes to it (no one did except for EDG, i don't even know if they shipped it)

When I said "wording changes that do not affect implementations", I meant a hypothetical conforming implementation. In other words, N/A is for wording changes that can't be exhibited by writing programs and feeding them into the compiler.

[Endilll]

Me and Corentin discussed this offline. Two points emerged there:

1. He's more concerned with the fact that 204 and other Core issues are highlighted green, inheriting the status of the issue that superseded them, than with the fact that they are marked as superseded. Styles we use in cxx_dr_status.html can be changed in a subsequent PR (to e.g. use N/A style for superseded issues).
2. In previous discussions on this topic, I remember that we together with @AaronBallman agreed that statuses on our page should follow the official statuses. This PR goes against it, because I know from experience that the current CWG chair is not interested in revisiting Core issues that were closed long ago, and mark them as superseded where appropriate. We might need to revisit out previous consensus in the light of this PR and aforementioned experience talking to Core.

[AaronBallman]

The goal of the status page is to convey implementation status to our users, and so from that perspective I think N/A provides the least information to users because it basically says "this entry doesn't apply to us". So in these kinds of cases, sup conveys more information because the DRs would apply to us, except another DR superseded the need for making the changes.

In terms of the color used, we want green to mean "we're behaving in a conforming way" so users can tell at a glance where the edge cases are. This case is a bit weird because "conform" means we don't implement something rather than we do implement it, but I still think green is appropriate because we're following the standard. It might be fine to give a different color to superseded, but I would guess we'd want that to be a lighter form version of whatever the superseding issue is colored. e.g., given that conforming = green and non-conforming = red, if we have a superseded issue we conform to, it would be light green, and if we didn't conform it would be pink. WDYT?

[Endilll]

Thank you for chiming in!

It might be fine to give a different color to superseded, but I would guess we'd want that to be a lighter form version of whatever the superseding issue is colored. e.g., given that conforming = green and non-conforming = red, if we have a superseded issue we conform to, it would be light green, and if we didn't conform it would be pink. WDYT?

I'm thinking of reducing opacity of the color instead of picking exact shades, but yeah, we can definitely do something of this sort.

[shafik]

LGTM after addressing coloring issues.

[cor3ntin]

I think as a follow up we should get rid of the "exported templates are unsupported" warning which makes no sense whatsoever and either use the c++20 error in all language modes or just a generic "unexpected 'export' keyword here" diagnostic