Scsynth & Plugin: Remove c cast from CALCSCOPE and fix GVerbs roomsize. #6262
Conversation
`decltype( a[0] )` returns a referenced type. This sc_typeof_cast used this to perform a c cast, when converting from a double to a float, producing the wrong value. This PR creates a new conversion function using static_asserts and static_cast that does the correct behaviour and will break at compile time if the user attempts to pass something unexpected. An example of a bug caused by this error can be found in the following PR.
…nning. The previous commit no longer causes GVerb to blow up when the room size is dynamically changed, but reveals another issue with bursts of noise. This commit converts all power functions to use doubles and, following clangds hints, places more static_casts. The issue with GVerb has existed since 2007. After this commit, GVerb still does unusual pitch shifting and introduces noise when changing the roomsize while running, but it does not blow up as it did before.
JordanHendersonMusic
force-pushed
the
topic/fix-easy-ccast-mistake-in-calcscope
branch
from
0857dc3
to
83cf08b
Compare
JordanHendersonMusic
added
bug
JordanHendersonMusic
changed the title
Is there some easy way to test this? As far as I can tell, the only other places where an array is passed (grepping across this repo and sc3-plugins) is here in GVerb and in SkUGens:FM7, who narrowly avoids this bug by placing the array in another slot. Perhaps limiting this to just arithmetic types is too restrictive? Copying off cpp-ref
As far as I can tell, this means the only way something would break was if someone passed in a struct for which those operators where defined, as the macro requires '-' and '*' be defined. I could remove the type trait checks, and instead just let static_cast deal with it. Edit: Actualy, I think just doing the static_cast is enough here. |
Previously, the cast was restricted to arithmetic types, this lifts that restriction, but will always do a copy.
JordanHendersonMusic
removed
the
bug
Purpose and Motivation
This decltype...
...returns a referenced type, e.g.
float&, notfloat.The macro
sc_typeof_castandCALCSCOPEused this to perform a c cast.When converting from a double to a float, this is fine.
When converting from an double to a float&, this is very bad, and was the main issue with GVerb's roomsize.
This PR creates a new conversion function using static_cast that does the correct behaviour and will break at compile time if the user attempts to pass something unexpected.
Following the alias violations (#6245) I retested GVerb, and this stopped it from blowing up. The second commit here just adds just more static_casts and uses
std::powconverting everything to doubles, which further improved it.Fixes #2302.
Partially fixes #6245, and an outstanding issues from 2007 in GVerb's roomsize — @joshpar.
Is it possible to remove
sc_typeof_cast? It is in a header that other plugins use. Is there any way to stop people from using it if it can't be removed?Types of changes
To-do list