Skip to content

Conversation

@avar
Copy link

@avar avar commented May 5, 2016

The src/test/scheduler_tests.cpp test has been disabled since
v0.9.0rc2-4332-g8f0d79e, since then it's been warning about the
MicroSleep() function being unused, e.g. on GCC 4.9.2-10:

test/scheduler_tests.cpp:32:13: warning: ‘void
scheduler_tests::MicroSleep(uint64_t)’ defined but not used
[-Wunused-function]

The bitcoin developers don't want this warning fixed, and are instead
using it as a reminder to fix the test. Since this is a rather
unorthodox use of compiler warnings add a comment about this so people
who build bitcoin and notice compiler warnings don't try to submit
patches for this one.

See #8003 and
#7169 for past attempts to fix
this warning which have been rejected.

The src/test/scheduler_tests.cpp test has been disabled since
v0.9.0rc2-4332-g8f0d79e, since then it's been warning about the
MicroSleep() function being unused, e.g. on GCC 4.9.2-10:

    test/scheduler_tests.cpp:32:13: warning: ‘void
    scheduler_tests::MicroSleep(uint64_t)’ defined but not used
    [-Wunused-function]

The bitcoin developers don't want this warning fixed, and are instead
using it as a reminder to fix the test. Since this is a rather
unorthodox use of compiler warnings add a comment about this so people
who build bitcoin and notice compiler warnings don't try to submit
patches for this one.

See bitcoin#8003 and
bitcoin#7169 for past attempts to fix
this warning which have been rejected.
@laanwj
Copy link
Member

laanwj commented May 5, 2016

I'd rather have the test fixed, or replaced by a test without the race condition.

But given no one is doing that at the moment, this warning is fine with me.

@avar
Copy link
Author

avar commented May 5, 2016

@laanwj Yes it's clear from the various refused pull requests that you want the test fixed and want to keep this warning. This pull request is not orthogonal to that, but is rather for noting that this "compiler warning as a reminder" shouldn't be fixed.

@laanwj
Copy link
Member

laanwj commented May 5, 2016

I agree.
On a higher level the philosophy of 'fixing compiler warnings' is wrong. It's fighting symptoms, not the underlying cause. It's like 'fixing' a building by papering over cracks.
In a way it is compiler's own fault for crying wolf too many times, warning about silly things, so it's understandable, but I've often seen cases where people bash in compiler warnings without even considering the problem they're warning about - which were pointing out a legit bug.

@laanwj
Copy link
Member

laanwj commented May 10, 2016

No longer necessary after #8016, which fixes the underlying issue.

@laanwj laanwj closed this May 10, 2016
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants