search

xiph / rnnoise

Recurrent neural network for audio noise reduction
BSD 3-Clause "New" or "Revised" License
3.89k stars 882 forks source link

Replace bad manual traps (error in modern compiler vers) #205

Closed wallabra closed 2 months ago

wallabra commented 2 years ago

(Elevated from https://github.com/noisetorch/rnnoise/pull/1)

This style of inducing the compiler to produce a x86 trap isn't used anymore, because it's opaque in its meaning, and has been replaced with internals like __builtin_trap or __builtin_unreachable. Here I use the latter because I think it conveys the semantics better, to both the compiler and the reader.

The main reason of this pull request, of course, is that, at least with Clang (but possibly also GCC), the lines of code in question were producing fatal errors with the default build configuration, which included -Werror. A workaround is to add -Wno-error=null-dereference, but why sweep the issues under the rug and let them stack for our future selves when we can solve them right away? Those edits were pretty simple to make :)

-----BEGIN PGP SIGNATURE-----

iQGzBAABCAAdFiEErLuenQucPUxdd2DaVrpxUP5g8OIFAmKhWlgACgkQVrpxUP5g
8OKcEwv+L8cdeA5en23/19X5ZofkZcdF3wqUmMsm2yflneyqV7Mf8WWEuCKPIkII
bRgrlrp5+4OzAe8eN8+PYZJ79hOYL9IfvUR3Yia1piJJqZ2LQlFs+6xvKZb71iNc
FtNJPo58PulseRO2vnLnOctZiHyWRMRtIRTd58vCFEVNh27xVtnf0Us+pjaxEJNy
C7fMwDHDGc8N4TfK8r58erEAd9yfEy2FQoztsa/8TGvFkDFpGN17sJQnmDi04f4w
Wwya1mcqKLR0QtCJybbfwl4s4zKBc7HFnIRe2ISHX3IukLib8qlOydb9Ji2l7N3J
6+0Ep3LQWyEjsmUo0szKVIb2do6kz2eeX6AGnSgtDBeC9EYv+4O+LbPbN7r8tn+/
UfAF6KJQj+8NZhjpMptAbzZgUDBP8Q8bMayDjAOZAeXMhrDI8+6vWOOhc1fxTg9X
LT0dYCAqskSDSgP9FU/unvrrqiP0UZYfwEtFql5l074Wv+TVvMEIYtK3hYWkfEV8
6ZbIwIat
=fbsr
-----END PGP SIGNATURE-----

Detached signature of pull request message (Markdown source code minus signature block itself).
j-schultz commented 2 years ago

__builtin_unreachable is not part of the C standard and not portable (as indicated by the double-underscore). MSVC offers __assume(0) as an alternative, but there also needs to be an alternative implementation for any other compilers supporting neither of those.

wallabra commented 2 years ago

I don't know of a portable and idiomatic alternative, but there probably is one.

In the meanwhile, it might be best to use the preprocessor and using defines within ifdef clauses.

jmvalin commented 2 months ago

Issue fixed in main