[clang] fix bug that undefined internal is a warning only for -pedantic-errors (PR #98016)
https://github.com/ccrownhill created https://github.com/llvm/llvm-project/pull/98016 This fixes issue #39558 which mentions the problem when compiling the following code with `-pedantic-errors` flag (it also mentions `-Wall -Wextra` but those shouldn't change the output, which is also the case in GCC): ```c static void f(); int main() { f; } ``` Clang only outputs an `undefined-internal` warning on the first line, but according to 6.9/3 ``` "... Moreover, if an identifier declared with internal linkage is used in an expression (other than as a part of the operand of a sizeof or _Alignof operator whose result is an integer constant), there shall be exactly one external definition for the identifier in the translation unit." ``` this should be illegal and hence an error. I fixed this by changing the warning type from `Warning` to `ExtWarn` and by adding a suitable test. >From 5dd2bb12dee26ba93d927ad1cd99fa610f9ace97 Mon Sep 17 00:00:00 2001 From: ccrownhill Date: Mon, 8 Jul 2024 11:42:37 +0100 Subject: [PATCH] fix bug that undefined internal is a warning only for -pedantic-errors --- clang/include/clang/Basic/DiagnosticSemaKinds.td | 2 +- clang/test/Sema/undefined_internal.c | 11 +++ 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 clang/test/Sema/undefined_internal.c diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 44fd51ec9abc9..ae3dbedd01693 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -6013,7 +6013,7 @@ def note_deleted_assign_field : Note< "because field %2 is of %select{reference|const-qualified}4 type %3">; // These should be errors. -def warn_undefined_internal : Warning< +def warn_undefined_internal : ExtWarn< "%select{function|variable}0 %q1 has internal linkage but is not defined">, InGroup>; def err_undefined_internal_type : Error< diff --git a/clang/test/Sema/undefined_internal.c b/clang/test/Sema/undefined_internal.c new file mode 100644 index 0..1b6c3a4b76e05 --- /dev/null +++ b/clang/test/Sema/undefined_internal.c @@ -0,0 +1,11 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s -pedantic-errors + +static void f(void); // expected-error {{function 'f' has internal linkage but is not defined}} + +int main(void) +{ +f; +// expected-note@-1 {{used here}} +// expected-warning@-2 {{expression result unused}} +} + ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] fix bug that undefined internal is a warning only for -pedantic-errors (PR #98016)
ccrownhill wrote: Ok I just added a commit with the addition to `clang/docs/ReleaseNotes.rst`. https://github.com/llvm/llvm-project/pull/98016 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] fix bug that undefined internal is a warning only for -pedantic-errors (PR #98016)
https://github.com/ccrownhill updated https://github.com/llvm/llvm-project/pull/98016 >From 5dd2bb12dee26ba93d927ad1cd99fa610f9ace97 Mon Sep 17 00:00:00 2001 From: ccrownhill Date: Mon, 8 Jul 2024 11:42:37 +0100 Subject: [PATCH 1/2] fix bug that undefined internal is a warning only for -pedantic-errors --- clang/include/clang/Basic/DiagnosticSemaKinds.td | 2 +- clang/test/Sema/undefined_internal.c | 11 +++ 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 clang/test/Sema/undefined_internal.c diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 44fd51ec9abc9..ae3dbedd01693 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -6013,7 +6013,7 @@ def note_deleted_assign_field : Note< "because field %2 is of %select{reference|const-qualified}4 type %3">; // These should be errors. -def warn_undefined_internal : Warning< +def warn_undefined_internal : ExtWarn< "%select{function|variable}0 %q1 has internal linkage but is not defined">, InGroup>; def err_undefined_internal_type : Error< diff --git a/clang/test/Sema/undefined_internal.c b/clang/test/Sema/undefined_internal.c new file mode 100644 index 0..1b6c3a4b76e05 --- /dev/null +++ b/clang/test/Sema/undefined_internal.c @@ -0,0 +1,11 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s -pedantic-errors + +static void f(void); // expected-error {{function 'f' has internal linkage but is not defined}} + +int main(void) +{ +f; +// expected-note@-1 {{used here}} +// expected-warning@-2 {{expression result unused}} +} + >From 9e679ac9c2c669b595d361516b7e499e0b0457ba Mon Sep 17 00:00:00 2001 From: ccrownhill Date: Wed, 10 Jul 2024 12:06:28 +0100 Subject: [PATCH 2/2] add to change of undefined_internal to error for -pedantic-errors to release notes --- clang/docs/ReleaseNotes.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 838cb69f647ee..a4146aeee25de 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -554,6 +554,9 @@ Attribute Changes in Clang Improvements to Clang's diagnostics --- +- Clang now emits an error instead of a warning for `undefined_internal` + when compiling with `-pedantic-errors` to conform to the C standard + - Clang now applies syntax highlighting to the code snippets it prints. ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] fix bug that undefined internal is a warning only for -pedantic-errors (PR #98016)
ccrownhill wrote: That sounds good. I will have a look at that. https://github.com/llvm/llvm-project/pull/98016 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] fix bug that undefined internal is a warning only for -pedantic-errors (PR #98016)
https://github.com/ccrownhill updated https://github.com/llvm/llvm-project/pull/98016 >From 5dd2bb12dee26ba93d927ad1cd99fa610f9ace97 Mon Sep 17 00:00:00 2001 From: ccrownhill Date: Mon, 8 Jul 2024 11:42:37 +0100 Subject: [PATCH 1/3] fix bug that undefined internal is a warning only for -pedantic-errors --- clang/include/clang/Basic/DiagnosticSemaKinds.td | 2 +- clang/test/Sema/undefined_internal.c | 11 +++ 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 clang/test/Sema/undefined_internal.c diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 44fd51ec9abc9..ae3dbedd01693 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -6013,7 +6013,7 @@ def note_deleted_assign_field : Note< "because field %2 is of %select{reference|const-qualified}4 type %3">; // These should be errors. -def warn_undefined_internal : Warning< +def warn_undefined_internal : ExtWarn< "%select{function|variable}0 %q1 has internal linkage but is not defined">, InGroup>; def err_undefined_internal_type : Error< diff --git a/clang/test/Sema/undefined_internal.c b/clang/test/Sema/undefined_internal.c new file mode 100644 index 0..1b6c3a4b76e05 --- /dev/null +++ b/clang/test/Sema/undefined_internal.c @@ -0,0 +1,11 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s -pedantic-errors + +static void f(void); // expected-error {{function 'f' has internal linkage but is not defined}} + +int main(void) +{ +f; +// expected-note@-1 {{used here}} +// expected-warning@-2 {{expression result unused}} +} + >From 9e679ac9c2c669b595d361516b7e499e0b0457ba Mon Sep 17 00:00:00 2001 From: ccrownhill Date: Wed, 10 Jul 2024 12:06:28 +0100 Subject: [PATCH 2/3] add to change of undefined_internal to error for -pedantic-errors to release notes --- clang/docs/ReleaseNotes.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 838cb69f647ee..a4146aeee25de 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -554,6 +554,9 @@ Attribute Changes in Clang Improvements to Clang's diagnostics --- +- Clang now emits an error instead of a warning for `undefined_internal` + when compiling with `-pedantic-errors` to conform to the C standard + - Clang now applies syntax highlighting to the code snippets it prints. >From b553d87a675cac5d60df2b1fdc586d6b7af6b687 Mon Sep 17 00:00:00 2001 From: ccrownhill Date: Wed, 10 Jul 2024 19:04:17 +0200 Subject: [PATCH 3/3] add more tests for undefined internals --- ...undefined_internal.c => undefined-internal-basic.c} | 0 clang/test/Sema/undefined-internal-generic-selection.c | 10 ++ clang/test/Sema/undefined-internal-sizeof.c| 10 ++ clang/test/Sema/undefined-internal-typeof.c| 10 ++ 4 files changed, 30 insertions(+) rename clang/test/Sema/{undefined_internal.c => undefined-internal-basic.c} (100%) create mode 100644 clang/test/Sema/undefined-internal-generic-selection.c create mode 100644 clang/test/Sema/undefined-internal-sizeof.c create mode 100644 clang/test/Sema/undefined-internal-typeof.c diff --git a/clang/test/Sema/undefined_internal.c b/clang/test/Sema/undefined-internal-basic.c similarity index 100% rename from clang/test/Sema/undefined_internal.c rename to clang/test/Sema/undefined-internal-basic.c diff --git a/clang/test/Sema/undefined-internal-generic-selection.c b/clang/test/Sema/undefined-internal-generic-selection.c new file mode 100644 index 0..0a2b4b61e68e7 --- /dev/null +++ b/clang/test/Sema/undefined-internal-generic-selection.c @@ -0,0 +1,10 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s -pedantic-errors + +// expected-no-diagnostics + +static int f(void); + +int main(void) +{ +int x = _Generic(f(), int: 10, short: 20); +} diff --git a/clang/test/Sema/undefined-internal-sizeof.c b/clang/test/Sema/undefined-internal-sizeof.c new file mode 100644 index 0..f609473b1341c --- /dev/null +++ b/clang/test/Sema/undefined-internal-sizeof.c @@ -0,0 +1,10 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s -pedantic-errors + +// expected-no-diagnostics + +static int f(void); + +int main(void) +{ +int x = sizeof f(); +} diff --git a/clang/test/Sema/undefined-internal-typeof.c b/clang/test/Sema/undefined-internal-typeof.c new file mode 100644 index 0..51c682ef7e942 --- /dev/null +++ b/clang/test/Sema/undefined-internal-typeof.c @@ -0,0 +1,10 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c23 -pedantic-errors + +// expected-no-diagnostics + +static int f(void); + +int main(void) +{ +typeof(f()) x; +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] fix bug that undefined internal is a warning only for -pedantic-errors (PR #98016)
ccrownhill wrote: I added three more tests covering these points: — part of the operand of a sizeof operator whose result is an integer constant; — part of the controlling expression of a generic selection; — or, part of the operand of any typeof operator whose result is not a variably modified type. I did not see how to add tests for the following since they require just a type which I thought to be possible to achieve with an undefined internal since for a type the first declaration is the definition, e.g. with typedef. — part of the operand of an alignof operator whose result is an integer constant; — part of the expression in a generic association that is not the result expression of its generic selection; (*this one is also a type since the association map contains only pairs of type names and result expressions*) Is there a way to test undefined internals for these cases? https://github.com/llvm/llvm-project/pull/98016 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] fix bug that undefined internal is a warning only for -pedantic-errors (PR #98016)
ccrownhill wrote: Ah I see. Thank you for the suggestions! I also tried the same for `_Alignof` but was put off by the errors generated by the `-pedantic-errors` flag. But I guess I can just check that I don't get the `undefined-internal` error. For the other two I had something similar but that still misses this point: — part of the expression in a generic association that is not the result expression of its generic selection But that might also be because I don't understand this point well enough. I don't see which expression is there that is not the controlling expression and not a result expression. Also, I was thinking that it would be best to group all tests into one file since they all test just one feature. What do you think of that? https://github.com/llvm/llvm-project/pull/98016 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] fix bug that undefined internal is a warning only for -pedantic-errors (PR #98016)
https://github.com/ccrownhill updated https://github.com/llvm/llvm-project/pull/98016 >From 5dd2bb12dee26ba93d927ad1cd99fa610f9ace97 Mon Sep 17 00:00:00 2001 From: ccrownhill Date: Mon, 8 Jul 2024 11:42:37 +0100 Subject: [PATCH 1/4] fix bug that undefined internal is a warning only for -pedantic-errors --- clang/include/clang/Basic/DiagnosticSemaKinds.td | 2 +- clang/test/Sema/undefined_internal.c | 11 +++ 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 clang/test/Sema/undefined_internal.c diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 44fd51ec9abc9..ae3dbedd01693 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -6013,7 +6013,7 @@ def note_deleted_assign_field : Note< "because field %2 is of %select{reference|const-qualified}4 type %3">; // These should be errors. -def warn_undefined_internal : Warning< +def warn_undefined_internal : ExtWarn< "%select{function|variable}0 %q1 has internal linkage but is not defined">, InGroup>; def err_undefined_internal_type : Error< diff --git a/clang/test/Sema/undefined_internal.c b/clang/test/Sema/undefined_internal.c new file mode 100644 index 0..1b6c3a4b76e05 --- /dev/null +++ b/clang/test/Sema/undefined_internal.c @@ -0,0 +1,11 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s -pedantic-errors + +static void f(void); // expected-error {{function 'f' has internal linkage but is not defined}} + +int main(void) +{ +f; +// expected-note@-1 {{used here}} +// expected-warning@-2 {{expression result unused}} +} + >From 9e679ac9c2c669b595d361516b7e499e0b0457ba Mon Sep 17 00:00:00 2001 From: ccrownhill Date: Wed, 10 Jul 2024 12:06:28 +0100 Subject: [PATCH 2/4] add to change of undefined_internal to error for -pedantic-errors to release notes --- clang/docs/ReleaseNotes.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 838cb69f647ee..a4146aeee25de 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -554,6 +554,9 @@ Attribute Changes in Clang Improvements to Clang's diagnostics --- +- Clang now emits an error instead of a warning for `undefined_internal` + when compiling with `-pedantic-errors` to conform to the C standard + - Clang now applies syntax highlighting to the code snippets it prints. >From b553d87a675cac5d60df2b1fdc586d6b7af6b687 Mon Sep 17 00:00:00 2001 From: ccrownhill Date: Wed, 10 Jul 2024 19:04:17 +0200 Subject: [PATCH 3/4] add more tests for undefined internals --- ...undefined_internal.c => undefined-internal-basic.c} | 0 clang/test/Sema/undefined-internal-generic-selection.c | 10 ++ clang/test/Sema/undefined-internal-sizeof.c| 10 ++ clang/test/Sema/undefined-internal-typeof.c| 10 ++ 4 files changed, 30 insertions(+) rename clang/test/Sema/{undefined_internal.c => undefined-internal-basic.c} (100%) create mode 100644 clang/test/Sema/undefined-internal-generic-selection.c create mode 100644 clang/test/Sema/undefined-internal-sizeof.c create mode 100644 clang/test/Sema/undefined-internal-typeof.c diff --git a/clang/test/Sema/undefined_internal.c b/clang/test/Sema/undefined-internal-basic.c similarity index 100% rename from clang/test/Sema/undefined_internal.c rename to clang/test/Sema/undefined-internal-basic.c diff --git a/clang/test/Sema/undefined-internal-generic-selection.c b/clang/test/Sema/undefined-internal-generic-selection.c new file mode 100644 index 0..0a2b4b61e68e7 --- /dev/null +++ b/clang/test/Sema/undefined-internal-generic-selection.c @@ -0,0 +1,10 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s -pedantic-errors + +// expected-no-diagnostics + +static int f(void); + +int main(void) +{ +int x = _Generic(f(), int: 10, short: 20); +} diff --git a/clang/test/Sema/undefined-internal-sizeof.c b/clang/test/Sema/undefined-internal-sizeof.c new file mode 100644 index 0..f609473b1341c --- /dev/null +++ b/clang/test/Sema/undefined-internal-sizeof.c @@ -0,0 +1,10 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s -pedantic-errors + +// expected-no-diagnostics + +static int f(void); + +int main(void) +{ +int x = sizeof f(); +} diff --git a/clang/test/Sema/undefined-internal-typeof.c b/clang/test/Sema/undefined-internal-typeof.c new file mode 100644 index 0..51c682ef7e942 --- /dev/null +++ b/clang/test/Sema/undefined-internal-typeof.c @@ -0,0 +1,10 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c23 -pedantic-errors + +// expected-no-diagnostics + +static int f(void); + +int main(void) +{ +typeof(f()) x; +} >From 72a5f4084167b9c1930adbd3756450e5e908b674 Mon Sep 17 00:00:00 2001 From: ccrownhill Date: Thu, 11 Jul 2024 10:17:58 +0200 Subject: [PATCH 4/4] cover alignof as
[clang] fix bug that undefined internal is a warning only for -pedantic-errors (PR #98016)
ccrownhill wrote: Since one error message will occlude others I had to split the files as I did in the latest commit. I have added one for the `_Alignof` case checking for the errors I was getting in godbolt while also checking that no `undefined-internal` error occurred. Lastly, I added the one illegal use case of `_Generic` from @AaronBallman 's suggestions. I think this should cover everything. What do you think? https://github.com/llvm/llvm-project/pull/98016 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] fix bug that undefined internal is a warning only for -pedantic-errors (PR #98016)
ccrownhill wrote: Oh that makes sense so ``` static void f(); void *k = _Generic(&f, void (*)(void) : 0, default : f); ``` should be ok since it is not in the result expression? https://github.com/llvm/llvm-project/pull/98016 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] fix bug that undefined internal is a warning only for -pedantic-errors (PR #98016)
ccrownhill wrote: Sorry, my mistake. This doesn't work for `default` but the problem is that in this example ``` static void *f(void); void *k = _Generic(&f, void *(*)(void) : 0, int (*)(void) : f()); ``` the `undefined-internal` error is still raised even though it is not in the result expression anymore. https://github.com/llvm/llvm-project/pull/98016 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] fix bug that undefined internal is a warning only for -pedantic-errors (PR #98016)
ccrownhill wrote: Ok, that's done with the FIXME comment for now. Maybe it is only because this is my first time contributing and I am not too familiar with the codebase but I couldn't see a trivial fix for the issue. For now I think it's best to land these changes first. I would just add an issue about it. https://github.com/llvm/llvm-project/pull/98016 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] fix bug that undefined internal is a warning only for -pedantic-errors (PR #98016)
ccrownhill wrote: Thank you very much for fixing that:) https://github.com/llvm/llvm-project/pull/98016 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits