Skip to content

[clang][docs] Reflect the implementation status for P2280R4 #127166

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

frederick-vs-ja
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja commented Feb 14, 2025

P2280R4 is officially a defect report per N4916. Although Clang 20 only implemented it for C++23 and later. I think we should mention that it is not backported yet.

P2280R4 is officially a defect report. Although Clang 20 only implementated it
for C++23 and later. I think we should mention that it is a DR but not backported yet.
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Feb 14, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 14, 2025

@llvm/pr-subscribers-clang

Author: A. Jiang (frederick-vs-ja)

Changes

P2280R4 is officially a defect report. Although Clang 20 only implementated it for C++23 and later, and #95474 accidently removed the DR status in cxx_status.html. I think we should mention that it is a DR but not backported yet.


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

1 Files Affected:

  • (modified) clang/www/cxx_status.html (+8-2)
diff --git a/clang/www/cxx_status.html b/clang/www/cxx_status.html
index 0fc3b1d314698..70490b2e4331f 100755
--- a/clang/www/cxx_status.html
+++ b/clang/www/cxx_status.html
@@ -439,8 +439,8 @@ <h2 id="cxx23">C++23 implementation status</h2>
     </tr>
     <tr>
       <td>Using unknown pointers and references in constant expressions</td>
-      <td><a href="https://wg21.link/P2280R4">P2280R4</a></td>
-      <td class="unreleased" align="center">Clang 20</td>
+      <td><a href="https://wg21.link/P2280R4">P2280R4</a> (<a href="#dr">DR</a>)</td>
+      <td class="unreleased" align="center">Clang 20 <a href="#p2280">(12)</a></td>
     </tr>
     <tr>
       <td>static <code>operator()</code></td>
@@ -510,6 +510,12 @@ <h2 id="cxx23">C++23 implementation status</h2>
       <td class="full" align="center">Yes</td>
     </tr>
 </table>
+
+<p>
+<span id="p2280">(12): In Clang 20, this change is not yet retroactively
+applied to pre-C++23 modes.
+</span>
+</p>
 </details>
 
 

@zyn0217 zyn0217 requested a review from Endilll February 14, 2025 07:41
@frederick-vs-ja frederick-vs-ja changed the title [clang][docs] Fix DR staus for P2280R4 [clang][docs] Fix DR status for P2280R4 Feb 14, 2025
Copy link
Contributor

@Endilll Endilll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to see a test that checks that it's available in C++23 but not C++20 (with a fixme on this). You can also point me out to an existing one.

Co-authored-by: Vlad Serebrennikov <[email protected]>
@frederick-vs-ja
Copy link
Contributor Author

@Endilll These checks (modified by #95474) and the newly added clang/test/SemaCXX/constant-expression-p2280r4.cpp are relevant.

template<typename T> void f2(const T &t) { // cxx11_20-note 2{{declared here}}
constexpr int k = t.size(); // cxx11_20-error 2{{constexpr variable 'k' must be initialized by a constant expression}} cxx11_20-note 2{{function parameter 't' with unknown value cannot be used in a constant expression}}
}

void g(array<3> a) {
f1(a);
f2(a); // cxx11_20-note {{in instantiation of function template}}
f3(a);
}

Ideally, the C++23 behavior should also be present in C++11~20 modes. Did you want to add FIXME to these checks?

@frederick-vs-ja frederick-vs-ja changed the title [clang][docs] Fix DR status for P2280R4 [clang][docs] Reflect the implementation status for P2280R4 Mar 4, 2025
@Endilll
Copy link
Contributor

Endilll commented Mar 4, 2025

Ideally, the C++23 behavior should also be present in C++11~20 modes. Did you want to add FIXME to these checks?

Yes, this would be an improvement.

@frederick-vs-ja

This comment was marked as resolved.

@frederick-vs-ja
Copy link
Contributor Author

frederick-vs-ja commented Mar 14, 2025

Oh, there seem to be a few remaining bugs. Now I've added some FIXME comments to the most related test files. @Endilll

@@ -1,6 +1,8 @@
// RUN: %clang_cc1 -std=c++23 -verify=expected,nointerpreter %s
// RUN: %clang_cc1 -std=c++23 -verify %s -fexperimental-new-constant-interpreter

// FIXME: P2280R4 should be backported. Run this in C++11 and later modes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On top of this FIXME, add RUN lines for C++11 mode, then add expected directives for whatever comes out of it. So that if someone accidentally makes it work, it doesn't go unnoticed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should instead do that in #129646 (and merge that first)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants