T1265 - Remove deprecation warnings when running with Guile V2

T1265 - Remove deprecation warnings when running with Guile V2 
Also remove Guile 1.6 compatibility code from guile_compatibility.hh

[Neil Puttock, 2010-09-25 22:07:53]

Hi Ian,

Can you upload a revised patchset without all the tab->space changes please?

I'm not sure about gutting guile-compatibility.hh: as part of LilyPond at one
remove from the core code, I don't see any harm in keeping it.  Besides, there
are a few macros in there which are required if compiling LilyPond against Guile
v1.8.

Cheers,
Neil

[Ian Hulin (gmail), 2010-09-26 19:04:13]

On 2010/09/25 22:07:53, Neil Puttock wrote:
> Hi Ian,
> 
> Can you upload a revised patchset without all the tab->space changes please?
> 
Working ...

> I'm not sure about gutting guile-compatibility.hh: as part of LilyPond at one
> remove from the core code, I don't see any harm in keeping it.  Besides, there
> are a few macros in there which are required if compiling LilyPond against
Guile
> v1.8.
> 
Nice catch, Neil.  I missed the #else line and nuked both branches of the
conditional compile block.

However the condition that was in there wouldn't have been safe as it assumed
SCM_MAJOR_VERSION was going to be 1 and was testing just the SCM_MINOR_VERSION <
7.  This would have caused the pre-1.8 definitions to have been activated in
Guile V2.0 - 2.6 !!

Will sort this out.

Cheers,

Ian

[Patrick McCarty, 2010-09-30 20:53:31]

Hi Ian,

Looks good mostly, though I haven't compiled anything yet.  I just have a few
comments for you.

Thanks,
Patrick

http://codereview.appspot.com/2204044/diff/7001/flower/include/guile-compatib...
File flower/include/guile-compatibility.hh (right):

http://codereview.appspot.com/2204044/diff/7001/flower/include/guile-compatib...
flower/include/guile-compatibility.hh:29: #define scm_from_unsigned(x)
scm_from_unsigned_integer (x)
Why keep these if you don't want to use scm_{to,from}_unsigned() anywhere?

Also, the scm_{to,from}_unsigned_integer() functions were introduced at the very
beginning of the 1.7.* series, so I don't think we need any compatibility macros
here.

http://codereview.appspot.com/2204044/diff/7001/flower/include/guile-compatib...
flower/include/guile-compatibility.hh:30: #else  // SCM_MINOR_VERSION > 6 &&
SCM_MINOR_VERSION < 9
For clarity, I would write

#else // SCM_MINOR_VERSION >= 9

http://codereview.appspot.com/2204044/diff/7001/flower/include/guile-compatib...
flower/include/guile-compatibility.hh:35: #else  // SCM_MAJOR_VERSION == 1
And here:

#else // SCM_MAJOR_VERSION != 1

This helps me see that this is an else block

http://codereview.appspot.com/2204044/diff/7001/lily/grob-array-scheme.cc
File lily/grob-array-scheme.cc (right):

http://codereview.appspot.com/2204044/diff/7001/lily/grob-array-scheme.cc#new...
lily/grob-array-scheme.cc:45: vsize i = scm_to_uint32 (index);
Regarding my comments in the previous file, please change to

vsize i = scm_to_unsigned_integer (index);

instead.

[Patrick McCarty, 2010-09-30 21:30:57]

Ian,

One more thing: There are two more instances of scm_int2num in
lily/dispatcher.cc that you can replace with scm_from_int.

Thanks,
Patrick

[Ian Hulin (gmail), 2010-10-03 19:56:12]

New patch-set uploaded.

I've added changes to dispatcher.cc as suggested by Patrick.
scm_to_uint32 --> scm_to_int in guile-compatibility.hh and grob-array-scheme.cc,
#else // comments changed as requested.

Cheers,
Ian

http://codereview.appspot.com/2204044/diff/7001/flower/include/guile-compatib...
File flower/include/guile-compatibility.hh (right):

http://codereview.appspot.com/2204044/diff/7001/flower/include/guile-compatib...
flower/include/guile-compatibility.hh:29: #define scm_from_unsigned(x)
scm_from_unsigned_integer (x)
On 2010/09/30 20:53:31, Patrick McCarty wrote:
> Why keep these if you don't want to use scm_{to,from}_unsigned() anywhere?
> 
> Also, the scm_{to,from}_unsigned_integer() functions were introduced at the
very
> beginning of the 1.7.* series, so I don't think we need any compatibility
macros
> here. 

1. I was trying to disturb as little as possible for 1.8.7
2. scm_from_unsigned(x) does map through to scm_from_unsigned_integer but
scm_to_unsigned(x) needs extra parameters if you use scm_to_unsigned_integer,
something like scm_to_unsigned_integer(x, 0, UINTMAX_MAX).

Unfortunately UINT*_MAX constants are not declared in the Guile V1.8.7 header
files so we can't use them on both platforms.

Also there's a small chance that some users may have one of these in  scm code
within one of their .ly files.

Ian

http://codereview.appspot.com/2204044/diff/7001/flower/include/guile-compatib...
flower/include/guile-compatibility.hh:30: #else  // SCM_MINOR_VERSION > 6 &&
SCM_MINOR_VERSION < 9
On 2010/09/30 20:53:31, Patrick McCarty wrote:
> For clarity, I would write
> 
> #else // SCM_MINOR_VERSION >= 9 

Done.

http://codereview.appspot.com/2204044/diff/7001/flower/include/guile-compatib...
flower/include/guile-compatibility.hh:35: #else  // SCM_MAJOR_VERSION == 1
On 2010/09/30 20:53:31, Patrick McCarty wrote:
> And here:
> 
> #else // SCM_MAJOR_VERSION != 1
> 
> This helps me see that this is an else block 

Done.

http://codereview.appspot.com/2204044/diff/7001/lily/grob-array-scheme.cc
File lily/grob-array-scheme.cc (right):

http://codereview.appspot.com/2204044/diff/7001/lily/grob-array-scheme.cc#new...
lily/grob-array-scheme.cc:45: vsize i = scm_to_uint32 (index);
On 2010/09/30 20:53:31, Patrick McCarty wrote:
> Regarding my comments in the previous file, please change to
> 
> vsize i = scm_to_unsigned_integer (index);
> 
> instead. 
This wouldn't compile.
From Guile API reference.
C Function: scm_t_uintmax scm_to_unsigned_integer (SCM x, scm_t_uintmax min,
scm_t_uintmax max)
It would have to be something like

vsize i = scm_to_unsigned_integer (index, 0, UINT32_MAX);

to avoid compilation diagnostics. Is that really clearer than using
scm_to_unit32, which is also supported in the Guile API?

Update:
vsize i = scm_to_unsigned_integer (index, 0, UINT32_MAX);

Doesn't compile with guile V1.8.7, saying UINT32_MAX isn't declared.

However 

vsize i = scm_to_uint (index);

 does work and integration tests run OK, so I'll use that.

Cheers,

Ian

[Patrick McCarty, 2010-10-08 02:35:44]

Hi Ian,

Just a few more comments for you.

I am okay with using scm_to_uint(); that looks like a safe route for us to take.

Thanks,
Patrick

http://codereview.appspot.com/2204044/diff/14001/flower/include/guile-compati...
File flower/include/guile-compatibility.hh (right):

http://codereview.appspot.com/2204044/diff/14001/flower/include/guile-compati...
flower/include/guile-compatibility.hh:28: #define scm_to_unsigned(x) scm_to_uint
(x)
This macro is not needed, either, AFAICS.

If you say

  #define FOO 1

all instances of FOO are replaced with 1.

With this patch, you do away with the one instance of "scm_to_unsigned(x)",
replacing it with "scm_to_uint(x)".  Since "scm_to_unsigned(x)" no longer
appears in the code base (after applying this patch), this macro does nothing.

http://codereview.appspot.com/2204044/diff/14001/flower/include/guile-compati...
flower/include/guile-compatibility.hh:30: #else  // SCM_MINOR_VERSION > 6 &&
SCM_MINOR_VERSION >= 9
Just about :)

The comment is now redundant...

This should be

#else // SCM_MINOR_VERSION >= 9

http://codereview.appspot.com/2204044/diff/14001/lily/dispatcher.cc
File lily/dispatcher.cc (right):

http://codereview.appspot.com/2204044/diff/14001/lily/dispatcher.cc#newcode195
lily/dispatcher.cc:195: }
Please fix the indentation here (revert to using a tab).

http://codereview.appspot.com/2204044/diff/14001/lily/include/lily-guile-macr...
File lily/include/lily-guile-macros.hh (right):

http://codereview.appspot.com/2204044/diff/14001/lily/include/lily-guile-macr...
lily/include/lily-guile-macros.hh:146: TYPE ## _ ## FUNC ## _init_functions);
Revert back to tabs here.

[Ian Hulin (gmail), 2010-10-18 19:25:20]

http://codereview.appspot.com/2204044/diff/14001/flower/include/guile-compati...
File flower/include/guile-compatibility.hh (right):

http://codereview.appspot.com/2204044/diff/14001/flower/include/guile-compati...
flower/include/guile-compatibility.hh:28: #define scm_to_unsigned(x) scm_to_uint
(x)
On 2010/10/08 02:35:44, Patrick McCarty wrote:
> This macro is not needed, either, AFAICS.
> 
> If you say
> 
>   #define FOO 1
> 
> all instances of FOO are replaced with 1.
> 
> With this patch, you do away with the one instance of "scm_to_unsigned(x)",
> replacing it with "scm_to_uint(x)".  Since "scm_to_unsigned(x)" no longer
> appears in the code base (after applying this patch), this macro does nothing.

Done.

http://codereview.appspot.com/2204044/diff/14001/flower/include/guile-compati...
flower/include/guile-compatibility.hh:30: #else  // SCM_MINOR_VERSION > 6 &&
SCM_MINOR_VERSION >= 9
On 2010/10/08 02:35:44, Patrick McCarty wrote:
> Just about :)
> 
> The comment is now redundant...
> 
> This should be
> 
> #else // SCM_MINOR_VERSION >= 9

Done.

http://codereview.appspot.com/2204044/diff/14001/lily/dispatcher.cc
File lily/dispatcher.cc (right):

http://codereview.appspot.com/2204044/diff/14001/lily/dispatcher.cc#newcode195
lily/dispatcher.cc:195: }
On 2010/10/08 02:35:44, Patrick McCarty wrote:
> Please fix the indentation here (revert to using a tab).

Done.

http://codereview.appspot.com/2204044/diff/14001/lily/include/lily-guile-macr...
File lily/include/lily-guile-macros.hh (right):

http://codereview.appspot.com/2204044/diff/14001/lily/include/lily-guile-macr...
lily/include/lily-guile-macros.hh:146: TYPE ## _ ## FUNC ## _init_functions);
On 2010/10/08 02:35:44, Patrick McCarty wrote:
> Revert back to tabs here.

Done.

[Patrick McCarty, 2010-10-18 20:31:22]

LGTM.

Ian, please send me the git patch, and I'll apply it for you.

Thanks,
Patrick

[Ian Hulin (gmail), 2010-10-19 18:31:27]

 Hi Patrick,

Patch is attached.

Cheers,

Ian

On 18/10/10 21:31, pnorcks@gmail.com wrote:
> LGTM.
>
> Ian, please send me the git patch, and I'll apply it for you.
>
> Thanks,
> Patrick
>
> http://codereview.appspot.com/2204044/

[Patrick McCarty, 2010-10-19 18:46:39]

On Tue, Oct 19, 2010 at 11:31 AM, Ian Hulin <ianhulin44@gmail.com> wrote:
>  Hi Patrick,
>
> Patch is attached.

Thanks, I've pushed it.

Regards,
Patrick