search

schellingb / TinySoundFont

SoundFont2 synthesizer library in a single C/C++ file
MIT License
623 stars 72 forks source link

"conversion may change the value [-Werror=conversion]" compilation errors on MCST lcc compiler #52

Open r-a-sattarov opened 3 years ago

r-a-sattarov commented 3 years ago

When compiling TinySoundFont library as part of OpenGothic project (https://github.com/Try/OpenGothic), I get the following compilation errors on the MCST lcc compiler:

lcc: "/root/dev/r-a-sattarov/OpenGothic/lib/TinySoundFont/tsf.h", line 829: error #2463: conversion may change the value [-Werror=conversion] samplesLeft = *fontSampleCount = chunkSmpl->size / sizeof(short);

lcc: "/root/dev/r-a-sattarov/OpenGothic/lib/TinySoundFont/tsf.h", line 835: error #2463: conversion may change the value [-Werror=conversion] stream->read(stream->data, sampleBuffer, samplesToRead * sizeof(short));

lcc: "/root/dev/r-a-sattarov/OpenGothic/lib/TinySoundFont/tsf.h", line 1483: error #2463: conversion may change the value [-Werror=conversion] int channelSamples = (f->outputmode == TSF_MONO ? 1 : 2) samples, floatBufferSize = channelSamples sizeof(float);

log: TinySoundFont-OpenGothic_lcc_compile_err.log

original issue https://github.com/Try/OpenGothic/issues/91

ell1e commented 2 years ago

Isn't a signed to unsigned conversion on assignment relatively standard to do implicitly like that? Of course if one involved value is negative that's bad, but I don't really see that potential issue here. So I'm personally wondering if the best way to proceed would be to just suggest to not make use of that warning. (Note: just a random contributor, so my opinion isn't too relevant. But maybe someone finds it interesting)

schellingb commented 2 years ago

I'm usually on the mindset that libraries should be as friendly as possible in regards to compiler warnings. That usually means no warnings on GCC/clang with -Wall -Wpedantic, as well as no warnings on MSVC with /W4.

Now, non-default warnings like this -Wconversion (or MSVC's /Wall) can be a bit annoying for sure... For tsf.h we get 45 conversion warnings which probably all are irrelevant and just need some cast to be silenced. Sometimes this can lead to better code (i.e. finding multiple variables which shouldn't be different types) but also some cases need more thought than one might expect (i.e. figuring out if it's theoretically possible for something to ever be negative). I would happily merge a patch for sure :-)