Skip to content

Conversation

@whisperity
Copy link
Member

Although "implicit int conversions" is supposed to be a superset containing the more specific "64-to-32" case, previously they were a disjoint set, only enabled in common in the much larger -Wconversion.

Closes #69444.

diagtool tree diff:

   -Wconversion
     # ...
     -Wfloat-conversion
       -Wfloat-overflow-conversion
       -Wfloat-zero-conversion
-    -Wshorten-64-to-32
     -Wint-conversion
     -Wimplicit-int-conversion
+      -Wshorten-64-to-32
       -Wobjc-signed-char-bool-implicit-int-conversion
     -Wimplicit-float-conversion
       -Wimplicit-int-float-conversion
         -Wimplicit-const-int-float-conversion
       -Wobjc-signed-char-bool-implicit-float-conversion
     # ...
@whisperity whisperity added clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer labels Feb 6, 2024
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Feb 6, 2024
@llvmbot
Copy link
Member

llvmbot commented Feb 6, 2024

@llvm/pr-subscribers-clang

Author: None (whisperity)

Changes

Although "implicit int conversions" is supposed to be a superset containing the more specific "64-to-32" case, previously they were a disjoint set, only enabled in common in the much larger -Wconversion.

Closes #69444.

diagtool tree diff:

   -Wconversion
     # ...
     -Wfloat-conversion
       -Wfloat-overflow-conversion
       -Wfloat-zero-conversion
-    -Wshorten-64-to-32
     -Wint-conversion
     -Wimplicit-int-conversion
+      -Wshorten-64-to-32
       -Wobjc-signed-char-bool-implicit-int-conversion
     -Wimplicit-float-conversion
       -Wimplicit-int-float-conversion
         -Wimplicit-const-int-float-conversion
       -Wobjc-signed-char-bool-implicit-float-conversion
     # ...

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

3 Files Affected:

  • (modified) clang/include/clang/Basic/DiagnosticGroups.td (+3-3)
  • (modified) clang/test/Sema/conversion-64-32.c (+5-1)
  • (added) clang/test/Sema/conversion-implicit-int-includes-64-to-32.c (+21)
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 6765721ae7002c..975eca0ad9b642 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -108,8 +108,10 @@ def EnumConversion : DiagGroup<"enum-conversion",
                                 EnumCompareConditional]>;
 def ObjCSignedCharBoolImplicitIntConversion :
   DiagGroup<"objc-signed-char-bool-implicit-int-conversion">;
+def Shorten64To32 : DiagGroup<"shorten-64-to-32">;
 def ImplicitIntConversion : DiagGroup<"implicit-int-conversion",
-                                     [ObjCSignedCharBoolImplicitIntConversion]>;
+                                     [Shorten64To32,
+                                      ObjCSignedCharBoolImplicitIntConversion]>;
 def ImplicitConstIntFloatConversion : DiagGroup<"implicit-const-int-float-conversion">;
 def ImplicitIntFloatConversion : DiagGroup<"implicit-int-float-conversion",
  [ImplicitConstIntFloatConversion]>;
@@ -631,7 +633,6 @@ def Shadow : DiagGroup<"shadow", [ShadowFieldInConstructorModified,
 def ShadowAll : DiagGroup<"shadow-all", [Shadow, ShadowFieldInConstructor,
                                          ShadowUncapturedLocal, ShadowField]>;
 
-def Shorten64To32 : DiagGroup<"shorten-64-to-32">;
 def : DiagGroup<"sign-promo">;
 def SignCompare : DiagGroup<"sign-compare">;
 def SwitchDefault  : DiagGroup<"switch-default">;
@@ -942,7 +943,6 @@ def Conversion : DiagGroup<"conversion",
                             EnumConversion,
                             BitFieldEnumConversion,
                             FloatConversion,
-                            Shorten64To32,
                             IntConversion,
                             ImplicitIntConversion,
                             ImplicitFloatConversion,
diff --git a/clang/test/Sema/conversion-64-32.c b/clang/test/Sema/conversion-64-32.c
index dc417edcbc2168..c172dd109f3be2 100644
--- a/clang/test/Sema/conversion-64-32.c
+++ b/clang/test/Sema/conversion-64-32.c
@@ -9,9 +9,13 @@ typedef long long long2 __attribute__((__vector_size__(16)));
 
 int4 test1(long2 a) {
   int4  v127 = a;  // no warning.
-  return v127; 
+  return v127;
 }
 
 int test2(long v) {
   return v / 2; // expected-warning {{implicit conversion loses integer precision: 'long' to 'int'}}
 }
+
+char test3(short s) {
+  return s * 2; // no warning.
+}
diff --git a/clang/test/Sema/conversion-implicit-int-includes-64-to-32.c b/clang/test/Sema/conversion-implicit-int-includes-64-to-32.c
new file mode 100644
index 00000000000000..e22ccbe821f65c
--- /dev/null
+++ b/clang/test/Sema/conversion-implicit-int-includes-64-to-32.c
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wimplicit-int-conversion -triple x86_64-apple-darwin %s
+
+int test0(long v) {
+  return v; // expected-warning {{implicit conversion loses integer precision}}
+}
+
+typedef int  int4  __attribute__ ((vector_size(16)));
+typedef long long long2 __attribute__((__vector_size__(16)));
+
+int4 test1(long2 a) {
+  int4  v127 = a;  // no warning.
+  return v127;
+}
+
+int test2(long v) {
+  return v / 2; // expected-warning {{implicit conversion loses integer precision: 'long' to 'int'}}
+}
+
+char test3(short s) {
+  return s * 2; // expected-warning {{implicit conversion loses integer precision: 'int' to 'char'}}
+}
@whisperity whisperity force-pushed the feat/shorten-64-to-32-as-implicit-int-conversion branch from ab626cf to ac12272 Compare February 6, 2024 10:02
Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

LGTM aside from a minor change to the release notes, thank you for this improvement!

@AaronBallman
Copy link
Collaborator

(Note, precommit CI failures are unrelated.)

@rjmccall
Copy link
Contributor

rjmccall commented Feb 7, 2024

This looks right to me, thanks.

Co-authored-by: Aaron Ballman <aaron@aaronballman.com>
@whisperity whisperity merged commit 4b72c5e into llvm:main Feb 8, 2024
@whisperity whisperity deleted the feat/shorten-64-to-32-as-implicit-int-conversion branch February 8, 2024 12:38
sayboras added a commit to cilium/cilium that referenced this pull request Nov 26, 2024
As per the below PR, clang 19.1.x is having -Wshorten-64-to32 under the
-Wimplicit-int-conversion, which is already forbidden in Makefile.bpf.
This commit is to avoid the conversion error as per below sample

```
./lib/nodeport.h:1825:18: error: implicit conversion loses integer precision: '__u64' (aka 'unsigned long long') to '__s32' (aka 'int') [-Werror,-Wshorten-64-to-32]
 1825 |         __s32 len_old = ctx_full_len(ctx);
```

Relates: llvm/llvm-project#80814
Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit to cilium/cilium that referenced this pull request Nov 26, 2024
As per the below PR, clang 19.1.x is having -Wshorten-64-to32 under the
-Wimplicit-int-conversion, which is already forbidden in Makefile.bpf.
This commit is to avoid the conversion error as per below sample

```
./lib/nodeport.h:1825:18: error: implicit conversion loses integer precision: '__u64' (aka 'unsigned long long') to '__s32' (aka 'int') [-Werror,-Wshorten-64-to-32]
 1825 |         __s32 len_old = ctx_full_len(ctx);
```

Relates: llvm/llvm-project#80814
Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit to cilium/cilium that referenced this pull request Nov 26, 2024
As per the below PR, clang 19.1.x is having -Wshorten-64-to32 under the
-Wimplicit-int-conversion, which is already forbidden in Makefile.bpf.
This commit is to avoid the conversion error as per below sample

```
./lib/nodeport.h:1825:18: error: implicit conversion loses integer precision: '__u64' (aka 'unsigned long long') to '__s32' (aka 'int') [-Werror,-Wshorten-64-to-32]
 1825 |         __s32 len_old = ctx_full_len(ctx);
```

Relates: llvm/llvm-project#80814
Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit to cilium/cilium that referenced this pull request Nov 28, 2024
As per the below PR, clang 19.1.x is having -Wshorten-64-to32 under the
-Wimplicit-int-conversion, which is already forbidden in Makefile.bpf.
This commit is to avoid the conversion error as per below sample

```
./lib/nodeport.h:1825:18: error: implicit conversion loses integer precision: '__u64' (aka 'unsigned long long') to '__s32' (aka 'int') [-Werror,-Wshorten-64-to-32]
 1825 |         __s32 len_old = ctx_full_len(ctx);
```

Relates: llvm/llvm-project#80814
Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit to cilium/cilium that referenced this pull request Nov 30, 2024
As per the below PR, clang 19.1.x is having -Wshorten-64-to32 under the
-Wimplicit-int-conversion, which is already forbidden in Makefile.bpf.
This commit is to avoid the conversion error as per below sample

```
./lib/nodeport.h:1825:18: error: implicit conversion loses integer precision: '__u64' (aka 'unsigned long long') to '__s32' (aka 'int') [-Werror,-Wshorten-64-to-32]
 1825 |         __s32 len_old = ctx_full_len(ctx);
```

Relates: llvm/llvm-project#80814
Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit to cilium/cilium that referenced this pull request Dec 4, 2024
As per the below PR, clang 19.1.x is having -Wshorten-64-to32 under the
-Wimplicit-int-conversion, which is already forbidden in Makefile.bpf.
This commit is to avoid the conversion error as per below sample

```
./lib/nodeport.h:1825:18: error: implicit conversion loses integer precision: '__u64' (aka 'unsigned long long') to '__s32' (aka 'int') [-Werror,-Wshorten-64-to-32]
 1825 |         __s32 len_old = ctx_full_len(ctx);
```

Relates: llvm/llvm-project#80814
Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit to cilium/cilium that referenced this pull request Dec 4, 2024
As per the below PR, clang 19.1.x is having -Wshorten-64-to32 under the
-Wimplicit-int-conversion, which is already forbidden in Makefile.bpf.
This commit is to avoid the conversion error as per below sample

```
./lib/nodeport.h:1825:18: error: implicit conversion loses integer precision: '__u64' (aka 'unsigned long long') to '__s32' (aka 'int') [-Werror,-Wshorten-64-to-32]
 1825 |         __s32 len_old = ctx_full_len(ctx);
```

Relates: llvm/llvm-project#80814
Signed-off-by: Tam Mach <tam.mach@cilium.io>
github-merge-queue bot pushed a commit to cilium/cilium that referenced this pull request Dec 4, 2024
As per the below PR, clang 19.1.x is having -Wshorten-64-to32 under the
-Wimplicit-int-conversion, which is already forbidden in Makefile.bpf.
This commit is to avoid the conversion error as per below sample

```
./lib/nodeport.h:1825:18: error: implicit conversion loses integer precision: '__u64' (aka 'unsigned long long') to '__s32' (aka 'int') [-Werror,-Wshorten-64-to-32]
 1825 |         __s32 len_old = ctx_full_len(ctx);
```

Relates: llvm/llvm-project#80814
Signed-off-by: Tam Mach <tam.mach@cilium.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

4 participants