Opened 3 years ago
Closed 3 years ago
#65016 closed defect (fixed)
py-pytorch @1.8.1: error: variable 'sigma_gn' set but not used [-Werror,-Wunused-but-set-variable]
Reported by: | mascguy (Christopher Nielsen) | Owned by: | mascguy (Christopher Nielsen) |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | ports | Version: | 2.7.2 |
Keywords: | sierra highsierra haspatch | Cc: | cjones051073 (Chris Jones), essandess (Steve Smith) |
Port: | py-pytorch |
Description (last modified by mascguy (Christopher Nielsen))
This port was rev-bumped, due to a breaking ABI change related to opencv4
4.5.5. However, builds are now failing on 10.12 and 10.13 - even though they succeeded previously - and despite no updates to py-pytorch
.
The compilation errors appear to be (potentially) utterly trivial:
/opt/local/var/macports/build/_opt_bblocal_var_buildworker_ports_build_ports_python_py-pytorch/py39-pytorch/work/pytorch-1.8.1/third_party/benchmark/src/complexity.cc:82:10: error: variable 'sigma_gn' set but not used [-Werror,-Wunused-but-set-variable] double sigma_gn = 0.0; ^ 1 error generated.
It's a bit unclear as to why a set-but-unused variable is now a compilation error, when that was not the case previously. And this doesn't appear related to the Clang version either: Whether this port is compiled with older Xcode Clangs (8.0/9.0) - or MacPorts Clang 13 - this case is now flagged as an error [on 10.12/10.13].
In other words, yes: I've tried blacklisting Xcode Clangs < 1001, to ensure Clang 13 is used for compilation on 10.13. Yet, we're still seeing this.
However, the compilation defaults on 10.14 and higher are such, that this case is NOT flagged as an error. Does anyone happen to know why the defaults are different for 10.14 and above, vs. 10.13 and below... regardless of which Clang version is used? (I could probably figure this out, with enough time spent digging through the changes to both our portgroups - and base 2.7.2 - but hoping someone else might know off the top of their head.)
As for the fix, it may be a simple matter of ensuring that set-but-unused variables are not flagged as errors. So potentially no big deal there.
But I'm more curious as to where this behavior has changed, between our portgroups and base 2.7.2. Thoughts?
Change History (5)
comment:1 Changed 3 years ago by mascguy (Christopher Nielsen)
Description: | modified (diff) |
---|
comment:2 Changed 3 years ago by mascguy (Christopher Nielsen)
comment:3 follow-up: 4 Changed 3 years ago by ryandesign (Ryan Carsten Schmidt)
Keywords: | sierra highsierra haspatch added |
---|---|
Summary: | py-pytorch: builds now failing for 10.12 and 10.13 → py-pytorch @1.8.1: error: variable 'sigma_gn' set but not used [-Werror,-Wunused-but-set-variable] |
It's using MacPorts clang 13 now. The previous build of this port on 10.13 used MacPorts clang 12 because it occurred on February 16, which predated the February 24 change that made MacPorts choose MacPorts clang 13 in preference to MacPorts clang 12.
MacPorts clang 13 introduces this new unused-but-set-variable
warning. It's not enabled by default, however it is enabled if you request all warnings with -Wall
which this project does do and which is fairly common.
Normally it would just be a warning but it's clearly being treated as an error here and in another port in #65013. I'm not sure why. In #65013 it's because that project is requesting that all warnings be treated as errors with -Werror
(which we don't recommend) but pytorch doesn't appear to be doing that. If I try to compile a sample program that has this problem I only get a warning. Maybe some additional flags are being used by pytorch that we're not seeing. It's difficult to tell because this port uses silent build rules. (Silent rules should be disabled so that we can see the exact commands used to compile each object.)
A workaround would be to add -Wno-error=unused-but-set-variable
with clang 13 or later. The better solution would be to fix the code for example by removing the unused variable. It's an upstream bug, of course, so consultation with the developers is recommended. In this case, such consultation has already occurred. Here is their bug report:
https://github.com/google/benchmark/issues/1172
And their fix which we should apply to the port:
https://github.com/google/benchmark/commit/e991355c02b93fe17713efe04cbc2e278e00fdbd
comment:4 Changed 3 years ago by mascguy (Christopher Nielsen)
Replying to ryandesign:
A workaround would be to add
-Wno-error=unused-but-set-variable
with clang 13 or later. The better solution would be to fix the code for example by removing the unused variable. It's an upstream bug, of course, so consultation with the developers is recommended. In this case, such consultation has already occurred. Here is their bug report:https://github.com/google/benchmark/issues/1172
And their fix which we should apply to the port:
https://github.com/google/benchmark/commit/e991355c02b93fe17713efe04cbc2e278e00fdbd
Thanks for the background detail, as well as a link to the patch. (Googling this was on my to-do list, but the sudden behavior change made me second-guess my sanity!)
comment:5 Changed 3 years ago by Christopher Nielsen <mascguy@…>
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
I suppose this also could be related to updated defaults among the various build tools, such as Ninja and CMake. Hmmm...