Stefan Westerfeld | 1 Aug 17:04
Picon

MERGE: bse2cxx-part4

   Hi!

I've ported a few bse more sources to C++. 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.

git fetch http://space.twc.de/public/git/stwbeast.git bse2cxx-part4:bse2cxx-part4

  Cu... Stefan
--

-- 
Stefan Westerfeld, Hamburg/Germany, http://space.twc.de/~stefan
Tim Janik | 1 Aug 17:46
Picon
Gravatar

Re: MERGE: bse2cxx-part3

On 27.07.2011 20:03, Stefan Westerfeld wrote:
>     Hi!
>
> On Tue, Jul 26, 2011 at 04:17:37AM +0200, Tim Janik wrote:
>> On Fri, 22 Jul 2011, Stefan Westerfeld wrote:
>>
>>>   Hi!
>>>
>>> I've ported a few bse more sources to C++. Sorry about minor mistakes in the
>>> commit order. Anyway, you can fetch the branch into your bse repo by using:
>>>
>>> git fetch http://space.twc.de/public/git/stwbeast.git bse2cxx-part3:bse2cxx-part3
>>
>> Thanks, great. I see you put a lot of work in there,
>> here're my bits:
>> - please use "uint" in the future, see my other mail.
>> - remove cxxdummy.cc from compilation rules if another C++ source is present.
>> - "void * boxed" should be "void *boxed" (unfixed in master)
>> - pointer casts should be written as: (void*) ptr
>>    (no extra space between "void" and "*")
>
> Ok. Here is my cxxport.pl script, let me know if you need changes to that.
>
> #!/usr/bin/perl -w -pi.bak
> s,\bclass\b,klass,g;
> s,\bgint\b,int,g;
> s,\bguint\b,uint,g;
> s,\bgdouble\b,double,g;
> s,\bgfloat\b,float,g;
> s,\bgchar\b,char,g;
(Continue reading)

Tim Janik | 1 Aug 18:17
Picon
Gravatar

Re: MERGE: bse2cxx-part4

On 01.08.2011 17:04, Stefan Westerfeld wrote:
>     Hi!
>
> 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
- "(gpointer)" -> "(void*)", no extra space, i.e. not (void *)

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);

> 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?

>    Cu... Stefan
(Continue reading)

Stefan Westerfeld | 2 Aug 13:25
Picon

Re: MERGE: bse2cxx-part4

   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
(Continue reading)

Stefan Westerfeld | 5 Aug 19:47
Picon

MERGE: bse2cxx-part5

   Hi!

I've ported a few bse more sources to C++, no problems occurred.

git fetch http://space.twc.de/public/git/stwbeast.git bse2cxx-part5:bse2cxx-part5

  Cu... Stefan
--

-- 
Stefan Westerfeld, Hamburg/Germany, http://space.twc.de/~stefan
Tim Janik | 5 Aug 20:35
Picon
Gravatar

Re: MERGE: bse2cxx-part5

On 05.08.2011 19:47, Stefan Westerfeld wrote:
> git fetchhttp://space.twc.de/public/git/stwbeast.git  bse2cxx-part5:bse2cxx-part5

Thanks again.

-      self->controls[1] = g_value_get_enum (value);
+      self->controls[1] = BseMidiSignalType (g_value_get_enum (value));

This should rather be:
+      self->controls[1] = (BseMidiSignalType) g_value_get_enum (value);

We see a lot of those enum conversion though, so I'm wondering if it 
isn't better for the code base to allow automatic int->enum conversion 
operators... Any experience with that?

--

-- 
Yours sincerely,
Tim Janik

---
http://lanedo.com/~timj/ - Founder and CEO of Lanedo GmbH.
Free software author and contributor on various projects.
Tim Janik | 5 Aug 20:57
Picon
Gravatar

Re: MERGE: bse2cxx-part4

On 02.08.2011 14:42, Stefan Westerfeld wrote:
> In the bseloader-bsewave.cc code, the next step is to convert that enum value
> into an uint type variable. Finally, we switch on the uint value. The thing
> that happens is that although the value get_b() returned was 4, the case 3:
> code is executed.
>
> Now one might say that b never had the value 4 in the first place (as you
> say due to narrowing the assignment to the possible enum choices), but a
> printf on b's value confirms that its in fact 4.

Are you saing that GTokenType var; can hold a value *other* than the 
possible enum values, but that narrowing occours:
- not when reading it out by printf (shows "other")
- not when assigning var ot an int (assigns "other")
- only when assigning var to a uint (assigns a close enum value)
?

That sounds utterly broken to me. If at all, I'd expect the narrowing to 
occour on the *assignment*of*var*.

> Have you tried make report on your machine yet?

Nope, I thought our normal tests run by make check or installcheck are 
good enough to trigger this?
I'm running make report only close to releases because the suite takes 
so long with it, any progress on the bsescm speedups btw? ;)

>> What do you mean, why is compilation with gcc-4.5 not allowed?
>
> Well, you can try, but g++-4.5 will stop compiling with these error messages.
(Continue reading)


Gmane