Add new flag -Wreturn-mismatch #82872

11happy · 2024-02-24T08:33:47Z

Overview:
This pull request fixes #72116 where a new flag is introduced for compatibility with GCC 14, the functionality of -Wreturn-type is modified to split some of its behaviors into -Wreturn-mismatch

Testing:

Tested the updated code.
Verified that other functionalities remain unaffected.
Sample working:

int foo(int x){
  x = 1;
  return;
}

Dependencies:

No dependencies on other pull requests.

Signed-off-by: 11happy <[email protected]>

llvmbot · 2024-02-24T08:34:18Z

@llvm/pr-subscribers-clang

Author: Bhuminjay Soni (11happy)

Changes

Overview:
This pull request fixes #72116 where a new flag is introduced for compatibility with GCC 14, the functionality of -Wreturn-type is modified to split some of its behaviors into -Wreturn-mismatch

Testing:

Tested the updated code.
Verified that other functionalities remain unaffected.
Sample working:

int foo(int x){
  x = 1;
  return;
}

Dependencies:

No dependencies on other pull requests.

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

7 Files Affected:

(modified) clang/include/clang/Basic/DiagnosticGroups.td (+1)
(modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+3-3)
(modified) clang/test/Analysis/null-deref-ps.c (+2-2)
(modified) clang/test/CodeGen/statements.c (+1-1)
(modified) clang/test/CodeGen/volatile-1.c (+1-1)
(modified) clang/test/Sema/return-silent.c (+1-1)
(modified) clang/test/Sema/return.c (+1-1)

diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 6765721ae7002c..7a786a24083583 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -607,6 +607,7 @@ def RedundantMove : DiagGroup<"redundant-move">;
 def Register : DiagGroup<"register", [DeprecatedRegister]>;
 def ReturnTypeCLinkage : DiagGroup<"return-type-c-linkage">;
 def ReturnType : DiagGroup<"return-type", [ReturnTypeCLinkage]>;
+def ReturnMismatch : DiagGroup<"return-mismatch">;
 def BindToTemporaryCopy : DiagGroup<"bind-to-temporary-copy",
                                     [CXX98CompatBindToTemporaryCopy]>;
 def SelfAssignmentField : DiagGroup<"self-assign-field">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 24d32cb87c89e2..7057b4b2123671 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10212,14 +10212,14 @@ def warn_second_parameter_to_va_arg_never_compatible : Warning<
 
 def warn_return_missing_expr : Warning<
   "non-void %select{function|method}1 %0 should return a value">, DefaultError,
-  InGroup<ReturnType>;
+  InGroup<ReturnMismatch>;
 def ext_return_missing_expr : ExtWarn<
   "non-void %select{function|method}1 %0 should return a value">, DefaultError,
-  InGroup<ReturnType>;
+  InGroup<ReturnMismatch>;
 def ext_return_has_expr : ExtWarn<
   "%select{void function|void method|constructor|destructor}1 %0 "
   "should not return a value">,
-  DefaultError, InGroup<ReturnType>;
+  DefaultError, InGroup<ReturnMismatch>;
 def ext_return_has_void_expr : Extension<
   "void %select{function|method|block}1 %0 should not return void expression">;
 def err_return_init_list : Error<
diff --git a/clang/test/Analysis/null-deref-ps.c b/clang/test/Analysis/null-deref-ps.c
index d80de15c05a3fe..2682ad02ee37f9 100644
--- a/clang/test/Analysis/null-deref-ps.c
+++ b/clang/test/Analysis/null-deref-ps.c
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -Wno-int-conversion -Wno-strict-prototypes -Wno-tautological-constant-compare -Wtautological-unsigned-zero-compare -analyzer-checker=core,deadcode,alpha.core -std=gnu99 -analyzer-purge=none -verify %s -Wno-error=return-type
-// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -Wno-int-conversion -Wno-strict-prototypes -Wno-tautological-constant-compare -Wtautological-unsigned-zero-compare -analyzer-checker=core,deadcode,alpha.core -std=gnu99 -verify %s -Wno-error=return-type
+// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -Wno-int-conversion -Wno-strict-prototypes -Wno-tautological-constant-compare -Wtautological-unsigned-zero-compare -analyzer-checker=core,deadcode,alpha.core -std=gnu99 -analyzer-purge=none -verify %s -Wno-error=return-type -Wno-error=return-mismatch
+// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -Wno-int-conversion -Wno-strict-prototypes -Wno-tautological-constant-compare -Wtautological-unsigned-zero-compare -analyzer-checker=core,deadcode,alpha.core -std=gnu99 -verify %s -Wno-error=return-type -Wno-error=return-mismatch
 
 typedef unsigned uintptr_t;
 
diff --git a/clang/test/CodeGen/statements.c b/clang/test/CodeGen/statements.c
index 07ae075d6d807e..76f4f254dfd1dc 100644
--- a/clang/test/CodeGen/statements.c
+++ b/clang/test/CodeGen/statements.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -Wno-error=return-type -Wno-error=int-conversion %s -emit-llvm-only
+// RUN: %clang_cc1 -Wno-error=return-type -Wno-error=return-mismatch -Wno-error=int-conversion %s -emit-llvm-only
 // REQUIRES: LP64
 
 // Mismatched type between return and function result.
diff --git a/clang/test/CodeGen/volatile-1.c b/clang/test/CodeGen/volatile-1.c
index cd919c78989fe2..80c40d21cf593e 100644
--- a/clang/test/CodeGen/volatile-1.c
+++ b/clang/test/CodeGen/volatile-1.c
@@ -1,5 +1,5 @@
 // XFAIL: target=aarch64-pc-windows-msvc
-// RUN: %clang_cc1 -Wno-return-type -Wno-unused-value -emit-llvm %s -w -o - | FileCheck %s
+// RUN: %clang_cc1 -Wno-return-type -Wno-return-mismatch -Wno-unused-value -emit-llvm %s -w -o - | FileCheck %s
 
 // CHECK: @i = {{(dso_local )?}}global [[INT:i[0-9]+]] 0
 volatile int i, j, k;
diff --git a/clang/test/Sema/return-silent.c b/clang/test/Sema/return-silent.c
index 720128d7ea60b2..e510c9c678c2bd 100644
--- a/clang/test/Sema/return-silent.c
+++ b/clang/test/Sema/return-silent.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -Wno-return-type -fsyntax-only -verify
+// RUN: %clang_cc1 %s -Wno-return-type -Wno-return-mismatch -fsyntax-only -verify
 // expected-no-diagnostics
 
 int t14(void) {
diff --git a/clang/test/Sema/return.c b/clang/test/Sema/return.c
index 14d5300e819492..b4f3f132dce7a1 100644
--- a/clang/test/Sema/return.c
+++ b/clang/test/Sema/return.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -fsyntax-only -Wignored-qualifiers -Wno-error=return-type -Wno-error=implicit-int -verify -fblocks -Wno-unreachable-code -Wno-unused-value -Wno-strict-prototypes
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -fsyntax-only -Wignored-qualifiers -Wno-error=return-type -Wno-error=return-mismatch -Wno-error=implicit-int -verify -fblocks -Wno-unreachable-code -Wno-unused-value -Wno-strict-prototypes
 
 // clang emits the following warning by default.
 // With GCC, -pedantic, -Wreturn-type or -Wall are required to produce the

11happy · 2024-03-02T10:19:41Z

Its a Humble Ping!

AaronBallman

Thank you for the changes! Please be sure to add a release note to clang/docs/ReleaseNotes.rst so users know about the new diagnostic flag, and the changes should come with more test coverage. I'd like to see to see a test with -Wreturn-type -Wno-return-mismatch and -Wno-return-type -Wreturn-mismatch to demonstrate that we properly enable/disable the correct diagnostics. Be sure to test the test cases against GCC trunk as well to see whether we match GCC's behavior or not.

clang/test/Sema/return.c

clang/test/Analysis/null-deref-ps.c

clang/test/CodeGen/volatile-1.c

Signed-off-by: 11happy <[email protected]>

AaronBallman · 2024-03-05T14:10:34Z

clang/test/Sema/return-mismatch.c

+
+int test6() {
+  return 0; // no-warning
+}


Please add a newline to the end of the file.

Some other test cases to add:

int test7(void) { foo(); // no warning } int test8(void) { bar(); // warning: non-void function does not return a value }

AaronBallman · 2024-03-05T14:12:43Z

clang/test/Sema/return-type.c

@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -Wreturn-type -Wno-return-mismatch -fsyntax-only -verify %s


Because this is the same test code in both files, let's remove this file, add this RUN line to the other file, and rename the other file return-type-mismatch.h.

AaronBallman · 2024-03-05T14:13:36Z

clang/test/Sema/return-mismatch.c

@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -Wno-return-type -Wreturn-mismatch -fsyntax-only -verify %s


When you add the other RUN line, it should use -verify=return-type.

Suggested change

// RUN: %clang_cc1 -Wno-return-type -Wreturn-mismatch -fsyntax-only -verify %s

// RUN: %clang_cc1 -Wno-return-type -Wreturn-mismatch -fsyntax-only -verify=return-mismatch %s

AaronBallman · 2024-03-05T14:13:48Z

clang/test/Sema/return-mismatch.c

+int bar(void);
+
+void test1() {
+  return 1; // expected-warning{{void function 'test1' should not return a value}}


Suggested change

return 1; // expected-warning{{void function 'test1' should not return a value}}

return 1; // return-mismatch-warning{{void function 'test1' should not return a value}}

AaronBallman · 2024-03-05T14:13:58Z

clang/test/Sema/return-mismatch.c

+}
+
+int test2() { 
+    return; // expected-warning{{non-void function 'test2' should return a value}}


Suggested change

return; // expected-warning{{non-void function 'test2' should return a value}}

return; // return-mismatch-warning{{non-void function 'test2' should return a value}}

AaronBallman · 2024-03-05T14:14:50Z

clang/test/Sema/return-mismatch.c

+} 
+
+int test3() { 
+    // no-warning


When you add the other RUN line, you'll use // return-type-warning@+1 {{non-void function does not return a value}} (similar for the other return-type.c warnings that will show up in this file).

AaronBallman · 2024-03-05T14:19:59Z

clang/test/Sema/return-mismatch.c

+int foo(void) __attribute__((noreturn));
+int bar(void);
+
+void test1() {


Suggested change

void test1() {

void test1(void) {

Same change should be made to all the functions in this file.

AaronBallman · 2024-03-05T14:23:02Z

clang/docs/ReleaseNotes.rst

@@ -94,6 +94,7 @@ Deprecated Compiler Flags

 Modified Compiler Flags
 -----------------------
+- The `-Wreturn-type` flag has been modified to split some of its functionality into `-Wreturn-mismatch` flag.


Suggested change

- The `-Wreturn-type` flag has been modified to split some of its functionality into `-Wreturn-mismatch` flag.

- Added a new diagnostic flag ``-Wreturn-mismatch`` which is grouped under

``-Wreturn-type``, and moved some of the diagnostics previously controlled by

``-Wreturn-type`` under this new flag. Fixes #GH72116.

Signed-off-by: 11happy <[email protected]>

11happy · 2024-03-11T10:43:39Z

Are there any more changes for this PR required from my end?
Thank you

AaronBallman

LGTM!

add new flag -Wreturn-mismatch

557ee75

Signed-off-by: 11happy <[email protected]>

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Feb 24, 2024

AaronBallman reviewed Mar 4, 2024

View reviewed changes

clang/test/Sema/return.c Outdated Show resolved Hide resolved

clang/test/Analysis/null-deref-ps.c Outdated Show resolved Hide resolved

clang/test/CodeGen/volatile-1.c Outdated Show resolved Hide resolved

add test and release notes

fd543c8

Signed-off-by: 11happy <[email protected]>

AaronBallman reviewed Mar 5, 2024

View reviewed changes

reword release notes and add more tests

2059e2a

Signed-off-by: 11happy <[email protected]>

11happy requested a review from AaronBallman March 8, 2024 13:53

AaronBallman approved these changes Mar 11, 2024

View reviewed changes

AaronBallman merged commit 8467457 into llvm:main Mar 11, 2024

		@@ -0,0 +1,29 @@
		// RUN: %clang_cc1 -Wreturn-type -Wno-return-mismatch -fsyntax-only -verify %s

		@@ -0,0 +1,29 @@
		// RUN: %clang_cc1 -Wno-return-type -Wreturn-mismatch -fsyntax-only -verify %s

	return 1; // expected-warning{{void function 'test1' should not return a value}}
	return 1; // return-mismatch-warning{{void function 'test1' should not return a value}}

	return; // expected-warning{{non-void function 'test2' should return a value}}
	return; // return-mismatch-warning{{non-void function 'test2' should return a value}}

-- The `-Wreturn-type` flag has been modified to split some of its functionality into `-Wreturn-mismatch` flag.
+- Added a new diagnostic flag ``-Wreturn-mismatch`` which is grouped under
+``-Wreturn-type``, and moved some of the diagnostics previously controlled by
+``-Wreturn-type`` under this new flag. Fixes #GH72116.

Add new flag -Wreturn-mismatch #82872

Add new flag -Wreturn-mismatch #82872

Uh oh!

Conversation

11happy commented Feb 24, 2024

Uh oh!

llvmbot commented Feb 24, 2024

Uh oh!

11happy commented Mar 2, 2024

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

11happy commented Mar 11, 2024

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!