Skip to content

[clang] Add test for CWG3129#206335

Open
eisenwave wants to merge 5 commits into
llvm:mainfrom
eisenwave:cwg3129
Open

[clang] Add test for CWG3129#206335
eisenwave wants to merge 5 commits into
llvm:mainfrom
eisenwave:cwg3129

Conversation

@eisenwave

@eisenwave eisenwave commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Resolves https://wg21.link/CWG3129

Clang has supported floating-point literals with overly tiny or huge exponents since 3.0, with only a warning emitted.

@eisenwave eisenwave requested a review from Endilll as a code owner June 28, 2026 13:59
@llvmorg-github-actions llvmorg-github-actions Bot added the clang Clang issues not falling into any other category label Jun 28, 2026
@llvmorg-github-actions

Copy link
Copy Markdown

@llvm/pr-subscribers-clang

Author: Jan Schultke (eisenwave)

Changes

Clang has always supported floating-point literals with overly tiny or huge exponents, with only a warning emitted.


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

1 Files Affected:

  • (modified) clang/www/cxx_dr_status.html (+1-1)
diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index ef30b5eefea13..4eaf0c44ab082 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -21720,7 +21720,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
     <td>[<a href="https://wg21.link/lex.fcon">lex.fcon</a>]</td>
     <td>DR</td>
     <td>Clarify which <I>floating-point-literal</I>s are valid</td>
-    <td class="unknown" align="center">Unknown</td>
+    <td class="full" align="center">Clang 2.7</td>
   </tr>
   <tr id="3130">
     <td><a href="https://cplusplus.github.io/CWG/issues/3130.html">3130</a></td>

@Endilll Endilll left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry, a bit more work is needed here.

  1. You need to add tests to clang/test/CXX/drs/cwg31xx.cpp.
  2. You need to take your tests to Compiler Explorer to see what was the first Clang release that exhibited the expected behavior (call it R).
  3. You need to leave a special comment in the test specifying R.
  4. You need to run clang/www/make_cxx_dr_status to update the cxx_dr_status.html. If it produces more changes, include only the changes relevant for CWG3129. If the script terminates with an error about an unrelated Core issue, let me know, and I'll fix that.

@Endilll

Endilll commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

The PR description would also benefit from a link to CWG3129.

@eisenwave

eisenwave commented Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

You need to take your tests to Compiler Explorer to see what was the first Clang release that exhibited the expected behavior (call it R).

I've tried that, and I couldn't find a version old enough where it wouldn't do what the issue wants. My impression is that 2.7 is being used to mean "since forever" in these issue resolutions, but that's so old, it's not even on Compiler Explorer.

Hmm nevermind, 2.7 doesn't accept it, so 3.0 is actually the fix verison https://godbolt.org/z/zW3c8r6eP

@eisenwave eisenwave requested a review from Endilll June 28, 2026 15:05
Comment thread clang/test/CXX/drs/cwg3129.cpp Outdated
Comment thread clang/test/CXX/drs/cwg3129.cpp Outdated
@eisenwave

Copy link
Copy Markdown
Contributor Author

The one problem with putting it into cwg31xx.cpp is that I had to get rid of the // cxx98-14-no-diagnostics comment; otherwise this test just wouldn't pass.

Maybe that means it should have been in a separate file after all, or is there some clever way to keep the comment?

@eisenwave eisenwave requested a review from Endilll June 28, 2026 16:04

@Endilll Endilll left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The one problem with putting it into cwg31xx.cpp is that I had to get rid of the // cxx98-14-no-diagnostics comment; otherwise this test just wouldn't pass.

This is totally fine. Special needs I mentioned before are scoped to RUN lines.

You should change the title of the PR to something along the lines of "Add test for CWG3129".

Let me know when it's ready to be merged.

@eisenwave eisenwave changed the title [clang] Document CWG3129 as resolved [clang] Add test for CWG3129 Jun 28, 2026
@eisenwave

eisenwave commented Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

@Endilll should be good to go.

OH wait, we got some CI failures.

@eisenwave

Copy link
Copy Markdown
Contributor Author

Very weird. The CI failures is because it's printing

1.18973149535723176508575932662800702E+4932

as the maximum for long double in CI but the test expectations is

1.18973149535723176502E+4932

which did work for me locally. OH well ...

@eisenwave

Copy link
Copy Markdown
Contributor Author

@Endilll should be good to merge now.

Comment thread clang/test/CXX/drs/cwg31xx.cpp Outdated
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants