Re: MERGE: bse2cxx-part4

[Stefan Westerfeld <stefan space twc de>]

   Hi!

On Mon, Aug 01, 2011 at 06:17:36PM +0200, Tim Janik wrote:
> On 01.08.2011 17:04, Stefan Westerfeld wrote:
> >I've ported a few bse more sources to C++.
> 
> Great, the most parts look really good, thanks for doing the work.
> Things that can still use polishing:
> - gboolean -> bool

I can try, but sometimes gboolean and bool will behave a little different, so
thats not just search & replace, but every case needs to be checked after
conversion.

> - "(gpointer)" -> "(void*)", no extra space, i.e. not (void *)
I try to do that.

> This is not ok:
> -    i = (guint) g_param_spec_get_qdata (pspec, ...);
> +    i = (unsigned long) g_param_spec_get_qdata (pspec, ...);
> 
> Never introduce a long, it's an evil type that should never be used
> (see my other emails on the list). Not even for 64bit pointer
> conversions (breaks on windows). For pointer -> int conversions, you
> need to:
> uint i = ptrdiff_t (some_pointer);

Well targetting windows, at this point, mostly means targetting win32; then
long should work (as pointers are still 32-bit). I've never built code
targetting 64-bit windows, and last time I checked, the gcc/mingw toolchain did
not support it (we need gcc on windows anyway). I don't know if a 64-bit gcc on
windows would have sizeof (long) == 8. In any case ptrdiff_t is fine with me,
so I'll try to use this in the future.

> >The very last commit regarding enum
> >conversion is a little odd, but required for g++-4.4 (see commit log).
> >Everything else should be straight forward, and make report passes within the
> >branch.
> 
> I can't reproduce any problem here, running:
> gcc (Ubuntu/Linaro 4.4.4-14ubuntu5) 4.4.5
> 
> Could you elaborate on the exact error you're seeing?

$ make report
...
TEST: adsr-wave-1-test
make[4]: Entering directory `/home/stefan/src/beast/tests/audio'
../../shell/bsescm-0.7.5 --bse-mixing-freq=48000 -p null=nosleep -m null --bse-rcfile "/dev/null" --bse-no-load --bse-override-plugin-globs '../../plugins/.libs/*.so:../../plugins/freeverb/.libs/*.so' --bse-override-sample-path '../../tests/audio:../../library/samples' --bse-disable-randomization -s ./bse2wav.scm ./adsr-wave-1-test.bse adsr-wave-1-test.wav
/home/stefan/src/beast/tests/audio/../../tests/audio/pseudo-saw.bsewave:6: error: invalid keyword, expected valid keyword - discarding wave
./adsr-wave-1-test.bse:11: failed to load wave "pseudo-saw" from "pseudo-saw.bsewave": Datei ist leer
Recording to WAV file: adsr-wave-1-test.wav
Playing ./adsr-wave-1-test.bse: done.
../../tools/bsefextract adsr-wave-1-test.wav --cut-zeros --channel 0 --avg-spectrum --spectrum --avg-energy --end-time  > adsr-wave-1-test.tmp
../../tools/bsefextract adsr-wave-1-test.wav --cut-zeros --channel 1 --avg-spectrum --spectrum --avg-energy --end-time >> adsr-wave-1-test.tmp
../../tools/bsefcompare ./adsr-wave-1-test.ref adsr-wave-1-test.tmp --threshold 99.99
adsr-wave-1-test.tmp:11: error: unexpected identifier `inf', expected number (float)
make[4]: *** [adsr-wave-1-test] Fehler 1

I've created a minimal testcase (attached). Here are the results for that on
Debian unstable amd64:

stefan@quadcorn64:~/src/sandbox/enumswitch$ for CXX in g++-4.4 g++-4.5 g++-4.6; do $CXX --version && $CXX -O2 -Wall -o es es.cc && es; done
g++-4.4 (Debian 4.4.6-7) 4.4.6
Copyright (C) 2010 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

switch (b) => 3
printf (...b) => 4
g++-4.5 (Debian 4.5.3-4) 4.5.3
Copyright (C) 2010 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

switch (b) => 3
printf (...b) => 4
g++-4.6 (Debian 4.6.1-5) 4.6.1
Copyright (C) 2011 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

switch (b) => 4
printf (...b) => 4

As you can see, older gcc versions take the case 3: branch in the code, which is
the same thing that is happening with bseloader-bsewave.cc:

        token = g_scanner_get_next_token (scanner);
        switch (token)
          {
          case BSEWAVE_TOKEN_LITTLE_ENDIAN:
          case BSEWAVE_TOKEN_LITTLE:        dsc->dfl_byte_order = G_LITTLE_ENDIAN; break;
          case BSEWAVE_TOKEN_BIG_ENDIAN:
          case BSEWAVE_TOKEN_BIG:           dsc->dfl_byte_order = G_BIG_ENDIAN;    break;
          default:                          return BSEWAVE_TOKEN_LITTLE_ENDIAN;
          }

around line 460, causing make report to fail. Maybe we should allow compilation
with g++ version != 4.4 - its just three lines in birnet (see startup assertion
suggestion) that need fixing. Like checking for g++ version and removing the
assertions that break compilation in that case.

   Cu... Stefan
-- 
Stefan Westerfeld, Hamburg/Germany, http://space.twc.de/~stefan

#include <stdio.h>

enum Base {
  FOO,
  BAR,
  BAZZ
};

// get_b should return 4; prevent optimizer from substituting this as constant expression
volatile int i = 4;

Base
get_b()
{
  return Base (i);
}

main()
{
  unsigned int b = get_b();
  switch (b)
    {
      case 0: printf ("switch (b) => 0\n"); break;
      case 1: printf ("switch (b) => 1\n"); break;
      case 2: printf ("switch (b) => 2\n"); break;
      case 3: printf ("switch (b) => 3\n"); break;
      case 4: printf ("switch (b) => 4\n"); break;
      default: printf ("switch (b) => default\n"); break;
    }
  printf ("printf (...b) => %d\n", b);
}