[clang] move -Wcast-function-type under -Wextra #77178

Abhinkop · 2024-01-06T04:42:20Z

The -Wcast-fuction-type-strict has been moved under dignstic group -Wextra.
Edited the test cases for -Wcast-fuction-type-strict and -Wcast-fuction-type in Sema an SemaCXX.

Added a new test case which include a functionality that was already in the -Wextra group, i.e -Wignored-qualifiers with -Wcast-fuction-type-strict.

Fixes: #76872

github-actions · 2024-01-06T04:42:37Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2024-01-06T04:43:05Z

@llvm/pr-subscribers-clang

Author: Abhin P Jose (Abhinkop)

Changes

This patch is for github issue #76872.

The -Wcast-fuction-type-strict has been moved under dignstic group -Wextra.
Edited the test cases for -Wcast-fuction-type-strict and -Wcast-fuction-type in Sema an SemaCXX.

Added a new test case which include a functionality that was already in the -Wextra group, i.e -Wignored-qualifiers with -Wcast-fuction-type-strict.

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

6 Files Affected:

(modified) clang/include/clang/Basic/DiagnosticGroups.td (+1)
(modified) clang/test/Sema/warn-cast-function-type-strict.c (+1-1)
(modified) clang/test/Sema/warn-cast-function-type.c (+1)
(added) clang/test/Sema/warn-extra-cast-function-type-strict.c (+42)
(modified) clang/test/SemaCXX/warn-cast-function-type-strict.cpp (+1)
(modified) clang/test/SemaCXX/warn-cast-function-type.cpp (+1)

diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 6765721ae7002c..d3e57de8ac7a58 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1026,6 +1026,7 @@ def Extra : DiagGroup<"extra", [
     EmptyInitStatement,
     StringConcatation,
     FUseLdPath,
+    CastFunctionType,
   ]>;
 
 def Most : DiagGroup<"most", [
diff --git a/clang/test/Sema/warn-cast-function-type-strict.c b/clang/test/Sema/warn-cast-function-type-strict.c
index 5233680796e972..68b49bb0d05d06 100644
--- a/clang/test/Sema/warn-cast-function-type-strict.c
+++ b/clang/test/Sema/warn-cast-function-type-strict.c
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 %s -fsyntax-only -Wcast-function-type -verify
 // RUN: %clang_cc1 %s -fsyntax-only -Wcast-function-type-strict -verify
-
+// RUN: %clang_cc1 %s -fsyntax-only -Wextra -Wno-ignored-qualifiers -verify
 
 int t(int array[static 12]);
 int u(int i);
diff --git a/clang/test/Sema/warn-cast-function-type.c b/clang/test/Sema/warn-cast-function-type.c
index d7ddcdb73725c0..09d169026b1c86 100644
--- a/clang/test/Sema/warn-cast-function-type.c
+++ b/clang/test/Sema/warn-cast-function-type.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 %s -fsyntax-only -Wcast-function-type -Wno-cast-function-type-strict -verify
+// RUN: %clang_cc1 %s -fsyntax-only -Wextra -Wno-cast-function-type-strict -verify
 
 int x(long);
 
diff --git a/clang/test/Sema/warn-extra-cast-function-type-strict.c b/clang/test/Sema/warn-extra-cast-function-type-strict.c
new file mode 100644
index 00000000000000..ef8853616ba832
--- /dev/null
+++ b/clang/test/Sema/warn-extra-cast-function-type-strict.c
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 %s -fsyntax-only -Wextra -verify
+
+
+int t(int array[static 12]);
+int u(int i);
+const int v(int i); /* expected-warning {{'const' type qualifier on return type has no effec}} */
+int x(long);
+
+typedef int (f1)(long);
+typedef int (f2)(void*);
+typedef int (f3)();
+typedef void (f4)();
+typedef void (f5)(void);
+typedef int (f6)(long, int);
+typedef int (f7)(long,...);
+typedef int (f8)(int *);
+typedef int (f9)(const int);
+typedef int (f10)(int);
+
+f1 *a;
+f2 *b;
+f3 *c;
+f4 *d;
+f5 *e;
+f6 *f;
+f7 *g;
+f8 *h;
+f9 *i;
+f10 *j;
+
+void foo(void) {
+  a = (f1 *)x;
+  b = (f2 *)x; /* expected-warning {{cast from 'int (*)(long)' to 'f2 *' (aka 'int (*)(void *)') converts to incompatible function type}} */
+  c = (f3 *)x; /* expected-warning {{cast from 'int (*)(long)' to 'f3 *' (aka 'int (*)()') converts to incompatible function type}} */
+  d = (f4 *)x; /* expected-warning {{cast from 'int (*)(long)' to 'f4 *' (aka 'void (*)()') converts to incompatible function type}} */
+  e = (f5 *)x; /* expected-warning {{cast from 'int (*)(long)' to 'f5 *' (aka 'void (*)(void)') converts to incompatible function type}} */
+  f = (f6 *)x; /* expected-warning {{cast from 'int (*)(long)' to 'f6 *' (aka 'int (*)(long, int)') converts to incompatible function type}} */
+  g = (f7 *)x; /* expected-warning {{cast from 'int (*)(long)' to 'f7 *' (aka 'int (*)(long, ...)') converts to incompatible function type}} */
+  h = (f8 *)t;
+  i = (f9 *)u;
+  j = (f10 *)v; /* expected-warning {{cast from 'const int (*)(int)' to 'f10 *' (aka 'int (*)(int)') converts to incompatible function type}} */
+}
diff --git a/clang/test/SemaCXX/warn-cast-function-type-strict.cpp b/clang/test/SemaCXX/warn-cast-function-type-strict.cpp
index f7ee55f84ac280..b3164afde5a0ca 100644
--- a/clang/test/SemaCXX/warn-cast-function-type-strict.cpp
+++ b/clang/test/SemaCXX/warn-cast-function-type-strict.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 %s -fblocks -fsyntax-only -Wcast-function-type -verify
 // RUN: %clang_cc1 %s -fblocks -fsyntax-only -Wcast-function-type-strict -verify
+// RUN: %clang_cc1 %s -fblocks -fsyntax-only -Wextra -verify
 
 int x(long);
 
diff --git a/clang/test/SemaCXX/warn-cast-function-type.cpp b/clang/test/SemaCXX/warn-cast-function-type.cpp
index c613aaea1e33f2..db2ee030fcbfc9 100644
--- a/clang/test/SemaCXX/warn-cast-function-type.cpp
+++ b/clang/test/SemaCXX/warn-cast-function-type.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 %s -fblocks -fsyntax-only -Wcast-function-type -Wno-cast-function-type-strict -verify
+// RUN: %clang_cc1 %s -fblocks -fsyntax-only -Wextra -Wno-cast-function-type-strict -verify
 
 int x(long);

Abhinkop · 2024-01-06T04:47:08Z

@nickdesaulniers

shafik

This should have a release note and I think this is a potentially breaking change since folks using Wextra may get this diagnostic now.

Endilll

I think this PR moves things in the right direction, but I'm not well-versed in C enough to approve.

This should have a release note and I think this is a potentially breaking change since folks using Wextra may get this diagnostic now.

I agree that release note should be added, but I'm not sure we break people with this. We ship new versions with new on-by-default diagnostics. How is this different? Not to say that this improves GCC compatibility (more on this below).

Out of 7 warning we expect in the test, GCC 13.1 issues a warning only for f2, f4, and f6 cases: https://godbolt.org/z/18Eqnsqba. Same goes for GCC trunk. I'm not qualified enough in C to be sure which compiler gets this right. @AaronBallman would be very welcome here.

Endilll · 2024-01-06T17:33:25Z

clang/test/Sema/warn-extra-cast-function-type-strict.c

+
+int t(int array[static 12]);
+int u(int i);
+const int v(int i); /* expected-warning {{'const' type qualifier on return type has no effec}} */


Suggested change

const int v(int i); /* expected-warning {{'const' type qualifier on return type has no effec}} */

const int v(int i); /* expected-warning {{'const' type qualifier on return type has no effect}} */

Didn't have to do it as the file is now not added.

Endilll · 2024-01-06T17:35:26Z

clang/test/Sema/warn-extra-cast-function-type-strict.c

@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 %s -fsyntax-only -Wextra -verify


As written, tests checks only default language mode for C (which is gnu99 AFAIK). Maybe we want to check other modes as well?

add runs for -std=cNN for all remaining 5 of them? and also add extra test cases where conflicts arise?

Yes, more RUN lines with different -std= options, and potentially more expected directives tailored for particular language mode. We have an example of this in C++ defect report tests: https://github.com/llvm/llvm-project/blob/main/clang/test/CXX/drs/dr6xx.cpp

Note that I might be biased here, as someone who've spent a significant time with C++ defect report test suite.

Endilll · 2024-01-06T17:51:20Z

clang/test/Sema/warn-extra-cast-function-type-strict.c

+  g = (f7 *)x; /* expected-warning {{cast from 'int (*)(long)' to 'f7 *' (aka 'int (*)(long, ...)') converts to incompatible function type}} */
+  h = (f8 *)t;
+  i = (f9 *)u;
+  j = (f10 *)v; /* expected-warning {{cast from 'const int (*)(int)' to 'f10 *' (aka 'int (*)(int)') converts to incompatible function type}} */


On line 6 we issue a diagnostic that const qualifier doesn't have effect on return type, yet we issue a diagnostic here telling that this makes function pointer type incompatible. It feels like we fail to make up our minds on the matter, or that we are overly pedantic without pedantic warnings enabled.

Yeah, even I found this strange. Is there a place we can verify which is the correct behavior?

I hope @AaronBallman can clarify.

I think the diagnostic here is incorrect.

C23 6.7.6.3p4:
If, in the declaration "T D1", D1 has the form
D ( parameter-type-listopt ) attribute-specifier-sequenceopt
and the type specified for ident in the declaration "T D" is "derived-declarator-type-list T", then the
type specified for ident is "derived-declarator-type-list function returning the unqualified, non-atomic
version of T". The optional attribute specifier sequence appertains to the function type.

So the type of the function does not include the qualifiers on the return type, which means that const int f(void) and int f(void) should result in the same type.

We get this wrong: https://godbolt.org/z/xvh449xrd

I think this is a more general problem with how we model the function type: https://godbolt.org/z/G3vxjW9dh

So, create an issue? Comment out this test case and put a comment relating to the new issue?

No need to create an issue, we've already got one: #39494

Probably worth it to add a FIXME comment above the test case and point to that issue, though.

So finally the things. I need to do:
Add FIXME comment and leave the test case as is. Remove the clang/test/Sema/warn-extra-cast-function-type-strict.c file . Update release notes. Then update the pull request. Agree? @nickdesaulniers @AaronBallman @Endilll @shafik

That sounds correct to me

nickdesaulniers · 2024-01-08T16:11:41Z

clang/test/Sema/warn-extra-cast-function-type-strict.c

@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 %s -fsyntax-only -Wextra -verify


If this diagnostic is not already well tested, adding all of these additional unit tests makes sense.

But it looks like there's quite a number of existing tests in other files. I don't think we should add a new file just to check this diagnostics behavior via -Wextra via a whole new file.

I was thinking of adding extra runs for both -Wcast-function-type and -Wextra or should the one for -Wcast-function-type be done in a different issue?

I would just remove the newly created clang/test/Sema/warn-extra-cast-function-type-strict.c. If you have cases that expose bugs in the implementation, that is distinctly a separate issue from adding this warning under -Wextra.

Ok. sounds good I will do that.

Done. File removed, and a FIXME comment added for the existing test.

nickdesaulniers · 2024-01-08T16:13:32Z

Github trick not specific to llvm-project. If you put

Fixes: #76872

In the commit message, then merging this PR will automatically close #76872.

Abhinkop · 2024-03-20T09:58:08Z

@nickdesaulniers @AaronBallman Can you'll take a look at this pull request?

AaronBallman

LGTM! I'll fix up the nit and land on your behalf; thank you for the improvement!

clang/docs/ReleaseNotes.rst

amy-kwan · 2024-03-21T03:28:45Z

I believe this patch is causing some issues on two PPC bots. Would you be able to help take a look?
https://lab.llvm.org/buildbot/#/builders/57/builds/33601/steps/5/logs/stdio
https://lab.llvm.org/buildbot/#/builders/36/builds/43759/steps/12/logs/stdio

Abhinkop · 2024-03-21T06:38:30Z

I believe this patch is causing some issues on two PPC bots. Would you be able to help take a look? https://lab.llvm.org/buildbot/#/builders/57/builds/33601/steps/5/logs/stdio https://lab.llvm.org/buildbot/#/builders/36/builds/43759/steps/12/logs/stdio

I guess adding this flag (-Wno-cast-function-type-strict) will make the error go away. But it is caused by the conversion of the second argument "void*" to "siginfo_t " when casting from "SignalHandlerType" i.e. "void (__sanitizer::SignalHandlerType)(int, void *, void *);" to "void (*sa_sigaction) (int, siginfo_t *, void *);". If you are sure that this conversion is valid, I guess we can add this flag. Might be similar for others.

lands and reverts due to hip catch tests needing new flag 1de7e6c [clang] move -Wcast-function-type under -Wextra (llvm#77178) Change-Id: Ie394e0f392be21823021ce464ceea9ead809922a

AaronBallman · 2024-03-21T12:18:01Z

I'm confused as to how this code ever compiled in the first place... In each case, this is C++ code that's failing:

/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp:219:25: error: cast from 'SignalHandlerType' (aka 'void (*)(int, void *, void *)') to 'sa_sigaction_t' (aka 'void (*)(int, siginfo_t *, void *)') converts to incompatible function type [-Werror,-Wcast-function-type-strict]

/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp:437:4: error: cast from 'void (*)()' to 'void (*)(void *)' converts to incompatible function type [-Werror,-Wcast-function-type-strict]

/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp:460:54: error: cast from 'void (*)(void *)' to 'void (*)()' converts to incompatible function type [-Werror,-Wcast-function-type-strict]

All of which is being compiled in -std=c++17 and none of which are valid in C++ and the behavior is the same between Clang 18, trunk, and GCC: https://godbolt.org/z/WY9zvsa8z

@amy-kwan we may need some help from you with investigating this; but in the meantime, this commit can be reverted to get the bots back to green if that's blocking you.

Abhinkop · 2024-03-21T12:46:03Z

I'm confused as to how this code ever compiled in the first place... In each case, this is C++ code that's failing:
/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp:219:25: error: cast from 'SignalHandlerType' (aka 'void (*)(int, void *, void *)') to 'sa_sigaction_t' (aka 'void (*)(int, siginfo_t *, void *)') converts to incompatible function type [-Werror,-Wcast-function-type-strict]

/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp:437:4: error: cast from 'void (*)()' to 'void (*)(void *)' converts to incompatible function type [-Werror,-Wcast-function-type-strict]

/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp:460:54: error: cast from 'void (*)(void *)' to 'void (*)()' converts to incompatible function type [-Werror,-Wcast-function-type-strict]
All of which is being compiled in -std=c++17 and none of which are valid in C++ and the behavior is the same between Clang 18, trunk, and GCC: https://godbolt.org/z/WY9zvsa8z

@amy-kwan we may need some help from you with investigating this; but in the meantime, this commit can be reverted to get the bots back to green if that's blocking you.

@AaronBallman There was a forced/explicit typecasting done, and hence the compiler didn't complain. for example, https://godbolt.org/z/G94r7qvxh . When the -Werror flag coupled with -Wextra flag caught this conversion, for example, https://godbolt.org/z/6bMrdfe4n. I hope this might help to provide some context.

amy-kwan · 2024-03-21T13:31:37Z

I'm confused as to how this code ever compiled in the first place... In each case, this is C++ code that's failing:
/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp:219:25: error: cast from 'SignalHandlerType' (aka 'void (*)(int, void *, void *)') to 'sa_sigaction_t' (aka 'void (*)(int, siginfo_t *, void *)') converts to incompatible function type [-Werror,-Wcast-function-type-strict]

/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp:437:4: error: cast from 'void (*)()' to 'void (*)(void *)' converts to incompatible function type [-Werror,-Wcast-function-type-strict]

/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp:460:54: error: cast from 'void (*)(void *)' to 'void (*)()' converts to incompatible function type [-Werror,-Wcast-function-type-strict]
All of which is being compiled in -std=c++17 and none of which are valid in C++ and the behavior is the same between Clang 18, trunk, and GCC: https://godbolt.org/z/WY9zvsa8z

@amy-kwan we may need some help from you with investigating this; but in the meantime, this commit can be reverted to get the bots back to green if that's blocking you.

@AaronBallman Sounds good. I am ok with this commit being reverted and we can investigate on the side.

AaronBallman · 2024-03-21T13:39:45Z

I'm confused as to how this code ever compiled in the first place... In each case, this is C++ code that's failing:
/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp:219:25: error: cast from 'SignalHandlerType' (aka 'void (*)(int, void *, void *)') to 'sa_sigaction_t' (aka 'void (*)(int, siginfo_t *, void *)') converts to incompatible function type [-Werror,-Wcast-function-type-strict]

/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp:437:4: error: cast from 'void (*)()' to 'void (*)(void *)' converts to incompatible function type [-Werror,-Wcast-function-type-strict]

/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp:460:54: error: cast from 'void (*)(void *)' to 'void (*)()' converts to incompatible function type [-Werror,-Wcast-function-type-strict]
All of which is being compiled in -std=c++17 and none of which are valid in C++ and the behavior is the same between Clang 18, trunk, and GCC: https://godbolt.org/z/WY9zvsa8z
@amy-kwan we may need some help from you with investigating this; but in the meantime, this commit can be reverted to get the bots back to green if that's blocking you.
@AaronBallman There was a forced/explicit typecasting done, and hence the compiler didn't complain. for example, https://godbolt.org/z/G94r7qvxh . When the -Werror flag coupled with -Wextra flag caught this conversion, for example, https://godbolt.org/z/6bMrdfe4n. I hope this might help to provide some context.

Ah! That does provide the context I needed, thank you! And I think I see the issue:

llvm-project/clang/include/clang/Basic/DiagnosticGroups.td

Line 576 in 7340263

    
           def CastFunctionType : DiagGroup<"cast-function-type", [CastFunctionTypeStrict]>;

-Wcast-function-type also includes -Wcast-function-type-strict; we don't want the strict warning in -Wextra, just the base warning. I'm looking to see if there's a reasonable way to accomplish this.

Abhinkop · 2024-03-21T13:49:12Z

I'm confused as to how this code ever compiled in the first place... In each case, this is C++ code that's failing:
/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp:219:25: error: cast from 'SignalHandlerType' (aka 'void (*)(int, void *, void *)') to 'sa_sigaction_t' (aka 'void (*)(int, siginfo_t *, void *)') converts to incompatible function type [-Werror,-Wcast-function-type-strict]

/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp:437:4: error: cast from 'void (*)()' to 'void (*)(void *)' converts to incompatible function type [-Werror,-Wcast-function-type-strict]

/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp:460:54: error: cast from 'void (*)(void *)' to 'void (*)()' converts to incompatible function type [-Werror,-Wcast-function-type-strict]
All of which is being compiled in -std=c++17 and none of which are valid in C++ and the behavior is the same between Clang 18, trunk, and GCC: https://godbolt.org/z/WY9zvsa8z
@amy-kwan we may need some help from you with investigating this; but in the meantime, this commit can be reverted to get the bots back to green if that's blocking you.
@AaronBallman There was a forced/explicit typecasting done, and hence the compiler didn't complain. for example, https://godbolt.org/z/G94r7qvxh . When the -Werror flag coupled with -Wextra flag caught this conversion, for example, https://godbolt.org/z/6bMrdfe4n. I hope this might help to provide some context.
Ah! That does provide the context I needed, thank you! And I think I see the issue:

llvm-project/clang/include/clang/Basic/DiagnosticGroups.td

Line 576 in 7340263

def CastFunctionType : DiagGroup<"cast-function-type", [CastFunctionTypeStrict]>;

-Wcast-function-type also includes -Wcast-function-type-strict; we don't want the strict warning in -Wextra, just the base warning. I'm looking to see if there's a reasonable way to accomplish this.

Change def CastFunctionType : DiagGroup<"cast-function-type", [CastFunctionTypeStrict]>; to def CastFunctionType : DiagGroup<"cast-function-type">; and introduce a new one def CastFunctionTypeDefault : DiagGroup<[CastFunctionType], [CastFunctionTypeStrict]>; and some how pass this on where CastFunctionType was used earlier?

AaronBallman · 2024-03-21T14:07:14Z

Yeah, I was hoping to avoid having to introduce another diagnostic group, but I think that's going to be the best way forward. I've got a patch in progress to add -Wcast-function-type-mismatch so we end up with:

-Wcast-function-type
|- -Wcast-function-type-mismatch
|- -Wcast-function-type-strict

and then add -Wcast-function-type-mismatch to -Wextra, but still thinking through the changes and whether there's a different approach.

AaronBallman · 2024-03-21T15:13:15Z

I've put up a PR for review at #86131

thurstond · 2024-03-21T16:58:58Z

I believe this patch is causing some issues on two PPC bots. Would you be able to help take a look? https://lab.llvm.org/buildbot/#/builders/57/builds/33601/steps/5/logs/stdio https://lab.llvm.org/buildbot/#/builders/36/builds/43759/steps/12/logs/stdio

I guess adding this flag (-Wno-cast-function-type-strict) will make the error go away. But it is caused by the conversion of the second argument "void*" to "siginfo_t _" when casting from "SignalHandlerType" i.e. "void (___sanitizer::SignalHandlerType)(int, void *, void *);" to "void (*sa_sigaction) (int, siginfo_t *, void *);". If you are sure that this conversion is valid, I guess we can add this flag. Might be similar for others.

The reason for the weird cast: SignalHandlerType has 'void*' instead of 'siginfo_t*' because SignalHandlerType is typedef'ed in sanitizer_common/sanitizer_common.h, which does not have access to the header (signal.h) that defines siginfo_t; we therefore cannot fix SignalHandlerType.

Fixed a small issue of matching pthread signature, which was causing the build to break for the compiler-rt project after adding -Wcast-function-type-mismatch to -Wextra dignostic group (#77178 & #86131).

The -Wcast-fuction-type-strict has been moved under dignstic group -Wextra. Edited the test cases for -Wcast-fuction-type-strict and -Wcast-fuction-type in Sema an SemaCXX. Added a new test case which include a functionality that was already in the -Wextra group, i.e -Wignored-qualifiers with -Wcast-fuction-type-strict. Fixes: llvm#76872

Fixed a small issue of matching pthread signature, which was causing the build to break for the compiler-rt project after adding -Wcast-function-type-mismatch to -Wextra dignostic group (llvm#77178 & llvm#86131).

The -Wcast-fuction-type-strict has been moved under dignstic group -Wextra. Edited the test cases for -Wcast-fuction-type-strict and -Wcast-fuction-type in Sema an SemaCXX. Added a new test case which include a functionality that was already in the -Wextra group, i.e -Wignored-qualifiers with -Wcast-fuction-type-strict. Fixes: llvm#76872 Change-Id: I800fdce58c0429df56c07bfa17a7e3ba98684e8d

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

Abhinkop mentioned this pull request Jan 6, 2024

[clang] move -Wcast-function-type under -Wextra #76872

Closed

[clang] move -Wcast-function-type under -Wextra

d147292

Abhinkop force-pushed the 76872 branch from adeed20 to d147292 Compare January 6, 2024 05:13

shafik reviewed Jan 6, 2024

View reviewed changes

shafik requested review from nickdesaulniers, AaronBallman and Endilll January 6, 2024 05:45

Endilll reviewed Jan 6, 2024

View reviewed changes

nickdesaulniers reviewed Jan 8, 2024

View reviewed changes

Abhinkop added 3 commits February 18, 2024 13:44

Added fixme comment. Removed the extra testcase. Added release note

386ea16

Fixed a typo

d14a0dc

Removed added notes by merge conflict

daa462a

Abhinkop force-pushed the 76872 branch from a6e9581 to daa462a Compare February 18, 2024 13:20

Abhin Parekadan Jose and others added 5 commits February 18, 2024 14:27

[clang] move -Wcast-function-type under -Wextra

eb8ad4f

Added fixme comment. Removed the extra testcase. Added release note

b2c2569

Fixed a typo

3774838

Removed added notes by merge conflict

f07fb78

Merge branch '76872' of github.com:Abhinkop/llvm-project into 76872

1cccdb8

AaronBallman approved these changes Mar 20, 2024

View reviewed changes

clang/docs/ReleaseNotes.rst Outdated Show resolved Hide resolved

Reword release note slightly; NFC

ec4c425

AaronBallman merged commit 1de7e6c into llvm:main Mar 20, 2024
4 of 5 checks passed

Abhinkop mentioned this pull request Mar 22, 2024

Fixed build breaking due to #77178 and #86131 #86290

Merged

	const int v(int i); /* expected-warning {{'const' type qualifier on return type has no effec}} */
	const int v(int i); /* expected-warning {{'const' type qualifier on return type has no effect}} */

		@@ -0,0 +1,42 @@
		// RUN: %clang_cc1 %s -fsyntax-only -Wextra -verify

[clang] move -Wcast-function-type under -Wextra #77178

[clang] move -Wcast-function-type under -Wextra #77178

Conversation

Abhinkop commented Jan 6, 2024 • edited by AaronBallman Loading

github-actions bot commented Jan 6, 2024

llvmbot commented Jan 6, 2024

Abhinkop commented Jan 6, 2024

shafik left a comment

Choose a reason for hiding this comment

Endilll left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Abhinkop Jan 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nickdesaulniers commented Jan 8, 2024

Abhinkop commented Mar 20, 2024

AaronBallman left a comment

Choose a reason for hiding this comment

amy-kwan commented Mar 21, 2024

Abhinkop commented Mar 21, 2024 • edited Loading

AaronBallman commented Mar 21, 2024

Abhinkop commented Mar 21, 2024

amy-kwan commented Mar 21, 2024

AaronBallman commented Mar 21, 2024

Abhinkop commented Mar 21, 2024

AaronBallman commented Mar 21, 2024

AaronBallman commented Mar 21, 2024

thurstond commented Mar 21, 2024 • edited Loading

Abhinkop commented Jan 6, 2024 •

edited by AaronBallman

Loading

Endilll left a comment •

edited

Loading

Abhinkop Jan 8, 2024 •

edited

Loading

Abhinkop commented Mar 21, 2024 •

edited

Loading

thurstond commented Mar 21, 2024 •

edited

Loading