museStringToDouble doesn't return anything

[yurivict]

Describe the bug This function double museStringToDouble(const QString &s, bool *ok) doesn't return anything when HAVE_ISTRINGSTREAM_HEXFLOAT is defined.

clang generated the warning, and added the ud2 assembler instruction that crashes the code in this place.

FreeBSD 13.2

[terminator356]

Hi. So our cmake script found that your version of istringstream supports hexfloats, but it is not actually working?

(Be sure to use the very latest github master, fixes were completed several days ago.)

Question: What locale are you in? (ie. what character is used for decimal points in your area?)

Hm. Here is our cmake test for istringstream hexfloat support:

function(check_istringstream_hexfloat varname)
  check_cxx_source_runs("
  #include <sstream>
  int main() {
  double value = 0;
  std::istringstream(\"0x1.8p+0\") >> value;
  return value != 1.5;
  }
  " ${varname})
endfunction()

As can be seen, it pretty much does exactly what museStringToDouble does. Well, except that in museStringToDouble, we force (imbue) the 'C' locale:

#ifdef HAVE_ISTRINGSTREAM_HEXFLOAT

  // If C++ istringstream supports hexfloat, use it.
  std::istringstream ss(s.toStdString());
  ss.imbue(std::locale("C"));
  double value = 0.0;
  ss >> value;
  if(ok)
    *ok = true;

#else // C++ istringstream does not support hexfloat

I wonder if the 'imbue' is causing a problem, or, I wonder if I should have added that 'imbue' line to our cmake test.

Be sure that you are not trying to open song files that were made before or during all of these recent 'hexfloat' fixes.

Try creating a new song and adding some audio controller random graph points. Verify that the song file does indeed contain 'hexfloats' - and that their decimal points are in fact a period '.' not a comma ','

[yurivict]

There's no return statement in that branch.

[terminator356]

Oh crud. I didn't return anything! Duh!

Let me fix that. Give me a bit of time, I'm working on something else. I'll get on it as soon as possible.

Thanks for spotting that.

[terminator356]

OK try it now. I also added the C locale 'imbue' statement to our cmake hexfloat check.

I am not sure why I left the 'imbue' out of the check. Possibly a sleepy-eyed copy and paste typo thing. But, my test string contains a period for a decimal point, so I think I should ensure C locale there. After all, this test is for hexfloat support, not locale support.

We will know if it works as soon as you compile and run it.

It is interesting that your clang supports istringstream hexfloat. My tests (and a lot of reading) seemed to show that only experimental clang versions support it at the moment. And support is not here yet in GCC.

[yurivict]

I already patched this problem out in the FreeBSD port.

That is clang-15.0.7 on FreeBSD that supports istringstream hexfloat.

[terminator356]

OK Thanks. Let me know if any more trouble. Close if appropriate.

[yurivict]

Ok, will do.

Thanks!