[clang] fix bug that undefined internal is a warning only for -pedantic-errors (PR #98016)

2024-07-08 Thread Constantin Kronbichler via cfe-commits

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)

2024-07-10 Thread Constantin Kronbichler via cfe-commits

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)

2024-07-10 Thread Constantin Kronbichler via cfe-commits

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)

2024-07-10 Thread Constantin Kronbichler via cfe-commits

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)

2024-07-10 Thread Constantin Kronbichler via cfe-commits

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)

2024-07-10 Thread Constantin Kronbichler via cfe-commits

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)

2024-07-11 Thread Constantin Kronbichler via cfe-commits

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)

2024-07-11 Thread Constantin Kronbichler via cfe-commits

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)

2024-07-11 Thread Constantin Kronbichler via cfe-commits

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)

2024-07-11 Thread Constantin Kronbichler via cfe-commits

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)

2024-07-11 Thread Constantin Kronbichler via cfe-commits

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)

2024-07-11 Thread Constantin Kronbichler via cfe-commits

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)

2024-07-11 Thread Constantin Kronbichler via cfe-commits

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