Gerrit Bot has uploaded this change for review.
runtime/cgo: Fix -Werror=parenthesis compile error
Current runtime may not compile if compiler enforces the `-Werror` flag. It fails with error:
```
In file included from gcc_context.c:8:
libcgo.h:102:15: error: unnecessary parentheses in declaration of '_cgo_get_context_function' [-Werror=parentheses]
102 | extern void (*(_cgo_get_context_function(void)))(struct context_arg*);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libcgo.h:102:15: note: remove parentheses
102 | extern void (*(_cgo_get_context_function(void)))(struct context_arg*);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| - -
cc1plus: all warnings being treated as errors
```
This commit fixes this.
Change-Id: I0df118923daa5938f4a1b5a8592c7db5b3ef24e8
GitHub-Last-Rev: a7f44244d6f8bd7ed61c6b649981dd3a93925572
GitHub-Pull-Request: golang/go#60766
---
M src/runtime/cgo/libcgo.h
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/runtime/cgo/libcgo.h b/src/runtime/cgo/libcgo.h
index 04755f0..5f52e68 100644
--- a/src/runtime/cgo/libcgo.h
+++ b/src/runtime/cgo/libcgo.h
@@ -99,7 +99,7 @@
struct context_arg {
uintptr_t Context;
};
-extern void (*(_cgo_get_context_function(void)))(struct context_arg*);
+extern void (*_cgo_get_context_function(void))(struct context_arg*);
/*
* The argument for the cgo traceback callback. See runtime.SetCgoTraceback.
To view, visit change 502836. To unsubscribe, or for help writing mail filters, visit settings.
Congratulations on opening your first change. Thank you for your contribution!
Next steps:
A maintainer will review your change and provide feedback. See
https://go.dev/doc/contribute#review for more info and tips to get your
patch through code review.
Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.
During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11 or adds a tag like "wait-release", it means that this CL will be
reviewed as part of the next development cycle. See https://go.dev/s/release
for more details.
Attention is currently required from: Ian Lance Taylor.
Patch set 1:Run-TryBot +1
3 comments:
Commit Message:
Minor: we don't use markdown in CL description. Just pasting as plain text is fine. Thanks.
Patchset:
Thanks for the CL.
File src/runtime/cgo/libcgo.h:
Patch Set #1, Line 102: *_cgo_get_context_function(void)
While it is correct without the parentheses, I feel it is clearer to have them. Maybe we could suppress the warning somehow.
To view, visit change 502836. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Cherry Mui.
1 comment:
File src/runtime/cgo/libcgo.h:
Patch Set #1, Line 102: *_cgo_get_context_function(void)
While it is correct without the parentheses, I feel it is clearer to have them. […]
I agree. What exact compiler are you running to get this error? I can't reproduce it with a couple of different versions of GCC.
To view, visit change 502836. To unsubscribe, or for help writing mail filters, visit settings.
File src/runtime/cgo/libcgo.h:
Patch Set #1, Line 102: *_cgo_get_context_function(void)
I agree. […]
I'm still unable to reproduce it, but I note that the error message you are showing implies that gcc_context.c is being compiled by a C++ compiler, which does not make sense. It should be compiled by a C compiler.
To view, visit change 502836. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Cherry Mui, Ian Lance Taylor.
Gerrit Bot uploaded patch set #2 to this change.
The following approvals got outdated and were removed: Run-TryBot+1 by Cherry Mui, TryBot-Result+1 by Gopher Robot
runtime/cgo: Suppress -Werror=parenthesis to avoid compile error
Current runtime may not compile under rare circumstances if compiler
enforces the `-Werror` flag for ambiguous parentheses
(e.g., GCC 11 in C++ mode, but it may enable it in future releases).
Build may fail with this error:
```
In file included from gcc_context.c:8:
libcgo.h:102:15: error: unnecessary parentheses in declaration of '_cgo_get_context_function' [-Werror=parentheses]
102 | extern void (*(_cgo_get_context_function(void)))(struct context_arg*);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libcgo.h:102:15: note: remove parentheses
102 | extern void (*(_cgo_get_context_function(void)))(struct context_arg*);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| - -
cc1plus: all warnings being treated as errors
This commit suppresses this warning, to avoid the possible build errors
on future static analyzer improvements, as these parentheses improves
the code readability.
```
Change-Id: I0df118923daa5938f4a1b5a8592c7db5b3ef24e8
GitHub-Last-Rev: a1212b535a33bf0a03d9bd9416f0b151f14e3d34
GitHub-Pull-Request: golang/go#60766
---
M src/runtime/cgo/libcgo.h
1 file changed, 3 insertions(+), 0 deletions(-)
To view, visit change 502836. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Cherry Mui, Ian Lance Taylor.
Minor: we don't use markdown in CL description. Just pasting as plain text is fine. Thanks.
Yes, fixed.
To view, visit change 502836. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Cherry Mui, Ian Lance Taylor.
2 comments:
Patchset:
Thanks for your feedback, now it suppresses that warning.
File src/runtime/cgo/libcgo.h:
Patch Set #1, Line 102: *_cgo_get_context_function(void)
While it is correct without the parentheses, I feel it is clearer to have them. […]
Ian, thanks, yes, it's accidentally leaked into CC env (in our internal CI, while I worked with C++ part of our internal Go project);
Cherry, yes, absolutely agreed, suppressing this warning on this declaration is better in this case (and may guard us against this warning if compiler vendors (in our case GCC) would improve static detector for C).
To view, visit change 502836. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Alex _F_, Cherry Mui.
1 comment:
File src/runtime/cgo/libcgo.h:
Patch Set #1, Line 102: *_cgo_get_context_function(void)
Ian, thanks, yes, it's accidentally leaked into CC env (in our internal CI, while I worked with C++ […]
If this only happens with C++, which I think is true, then I don't think it's worth fixing at all. The fix is to not compile this code with a C++ compiler.
To view, visit change 502836. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Cherry Mui, Ian Lance Taylor.
Patch Set #1, Line 102: *_cgo_get_context_function(void)
If this only happens with C++, which I think is true, then I don't think it's worth fixing at all. […]
Yes, agreed, so I close the MR.
To view, visit change 502836. To unsubscribe, or for help writing mail filters, visit settings.