This patch adds a missing test, covering the different kinds of floating-point
literals, like hexadecimal floats, and testing extreme cases & normal cases.
Details
Details
Diff Detail
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
| clang/test/Sema/warn-literal-range.c | ||
|---|---|---|
| 26 | I think we should probably have test coverage for long double as well, but I also wonder whether it makes sense to add coverage for the small floating-point types (like _Float16) as well, or whether we already have coverage for those elsewhere. | |
Comment Actions
I think we should probably have test coverage for long double as well, but I also wonder whether it makes sense to add coverage for the small floating-point types (like _Float16) as well, or whether we already have coverage for those elsewhere.
Test long double is fine for me. But I'm not really sure if we need to test for these half-precision floats, as they are part of clang language extension and not supported in all targets. IMHO, maybe we can just simply test these normal or standard floats first?
Comment Actions
I'm fine with that approach. Thanks for adding the long double support, but it looks like the precommit CI spotted an issue:
FAIL: Clang :: Sema/warn-literal-range.c (12332 of 29830) ******************** TEST 'Clang :: Sema/warn-literal-range.c' FAILED ******************** Script: -- : 'RUN: at line 1'; c:\ws\w32-1\llvm-project\premerge-checks\build\bin\clang.exe -cc1 -internal-isystem c:\ws\w32-1\llvm-project\premerge-checks\build\lib\clang\15.0.0\include -nostdsysteminc -std=c99 -verify C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c -- Exit Code: 1 Command Output (stdout): -- $ ":" "RUN: at line 1" $ "c:\ws\w32-1\llvm-project\premerge-checks\build\bin\clang.exe" "-cc1" "-internal-isystem" "c:\ws\w32-1\llvm-project\premerge-checks\build\lib\clang\15.0.0\include" "-nostdsysteminc" "-std=c99" "-verify" "C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c" # command stderr: error: 'warning' diagnostics expected but not seen: File C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c Line 29: magnitude of floating-point constant too small for type 'long double'; minimum is 3.64519953188247460253E-4951 File C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c Line 31: magnitude of floating-point constant too large for type 'long double'; maximum is 1.18973149535723176502E+4932 File C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c Line 35: magnitude of floating-point constant too small for type 'long double'; minimum is 3.64519953188247460253E-4951 File C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c Line 37: magnitude of floating-point constant too large for type 'long double'; maximum is 1.18973149535723176502E+4932 error: 'warning' diagnostics seen but not expected: File C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c Line 29: magnitude of floating-point constant too small for type 'long double'; minimum is 4.9406564584124654E-324 File C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c Line 31: magnitude of floating-point constant too large for type 'long double'; maximum is 1.7976931348623157E+308 File C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c Line 35: magnitude of floating-point constant too small for type 'long double'; minimum is 4.9406564584124654E-324 File C:\ws\w32-1\llvm-project\premerge-checks\clang\test\Sema\warn-literal-range.c Line 37: magnitude of floating-point constant too large for type 'long double'; maximum is 1.7976931348623157E+308 8 errors generated. error: command failed with exit status: 1 -- ********************
You might need to add a target triple to the test to specify exactly which target you are testing for. Bonus points if you pick a second target with different range of values. You can do that with something like:
// RUN: %clang_cc1 -triple=whatever -std=c99 -verify=whatever %s
// RUN: %clang_cc1 -triple=whatever-else -std=c99 -verify=whatever-else %s
long double ld5 = 0x0.42p+42000L; // whatever-warning {{magnitude of floating-point constant too large for type 'long double'; maximum is 1.18973149535723176502E+4932}} \
whatever-else-warning {{magnitude of floating-point constant too large for type 'long double'; maximum is 12}} \(the identifier after -verify= is used in place of the expected in diagnostic verification so you can vary which warnings are checked against which RUN lines.)
Comment Actions
Thanks for spending time looking at the CI issue, I thought it was something wrong with itself!
You might need to add a target triple to the test to specify exactly which target you are testing for. Bonus points if you pick a second target with different range of values. You can do that with something like:
// RUN: %clang_cc1 -triple=whatever -std=c99 -verify=whatever %s // RUN: %clang_cc1 -triple=whatever-else -std=c99 -verify=whatever-else %s long double ld5 = 0x0.42p+42000L; // whatever-warning {{magnitude of floating-point constant too large for type 'long double'; maximum is 1.18973149535723176502E+4932}} \ whatever-else-warning {{magnitude of floating-point constant too large for type 'long double'; maximum is 12}} \(the identifier after -verify= is used in place of the expected in diagnostic verification so you can vary which warnings are checked against which RUN lines.)
Thanks for telling me these about the regression tests! But I have no LLVM development set up in Windows so I guess one triple test should be fine. Maybe I can use the info given by the CI? Yeah that's true but then I can't verify the results locally and may break things and I don't like it...
Comment Actions
LGTM, thank you for the additional test coverage! Do you need me to commit on your behalf? If so, what name and email address would you like me to use for patch attribution?
I think we should probably have test coverage for long double as well, but I also wonder whether it makes sense to add coverage for the small floating-point types (like _Float16) as well, or whether we already have coverage for those elsewhere.