Simplify ly_symbol2scm

Simplify ly_symbol2scm

[Dan Eble, 2018-08-04 12:49:43]

https://codereview.appspot.com/363750043/diff/1/lily/include/lily-guile-macro...
File lily/include/lily-guile-macros.hh (right):

https://codereview.appspot.com/363750043/diff/1/lily/include/lily-guile-macro...
lily/include/lily-guile-macros.hh:95: value;                                    
                         \
This is new to me.  It is a GCC extension, according to
https://stackoverflow.com/a/1635559.  I'd probably add a TODO comment that
suggests converting this to a lambda once we can depend on C++11.

[dak, 2018-08-04 14:06:51]

https://codereview.appspot.com/363750043/diff/1/lily/include/lily-guile-macro...
File lily/include/lily-guile-macros.hh (right):

https://codereview.appspot.com/363750043/diff/1/lily/include/lily-guile-macro...
lily/include/lily-guile-macros.hh:95: value;                                    
                         \
On 2018/08/04 12:49:42, Dan Eble wrote:
> This is new to me.  It is a GCC extension, according to
> https://stackoverflow.com/a/1635559.  I'd probably add a TODO comment that
> suggests converting this to a lambda once we can depend on C++11.

__builtin_constant_p would not work in a lambda expression, so the caching for
constants would not work, making the code equivalent (though much more
complicated) to the #else path below.

So you'd need to create a top-level ?: expression predicated on
__built_in_constant_p and fork out into an immediately called lambda expression
in the ? branch for the only purpose of creating a static variable.

And then check that the optimizer does not do anything stupid.

Note that we already _could_ do the ?: wrapper, turning this into
(__built_in_constant_p (x) ? ({ static SCM cached = ...; cached})
                            : scm_or_str2symbol (x))

You propose using instead
(__builtin_constant_p (x) ? [] (SCM arg){ static SCM cached = ...; return
cached;}()
                          : scm_or_str2symbol (x));

Frankly, it doesn't read better to me and one would have to check pretty
carefully that it does not generate worse code since this memoization is used a
lot in frequently travelled code paths.

There are a lot of other occasions for using lambda that have less questionable
payoff.

The core "if" or equivalently ?: has to be a macro or __builtin_constant_p will
not be able to do its part of the job.  And __builtin_constant_p, a mandatory
part of this optimization, is already a GCC extension so there is absolutely no
gain for going to C++11 here.

I'm actually sympathetic to going the ?: route which I personally find more
readable than introducing an extra "value" variable, and if "x" contains "value"
in some respect, the extra hygiene would pay off.

I'll do that and test the results.  But C++11 here would not help in any manner
I see.

[dak, 2018-08-04 15:15:42]

Use ?: since it streamlines the assignment maze

[Dan Eble, 2018-08-05 01:21:00]

On 2018/08/04 15:15:42, dak wrote:
> Use ?: since it streamlines the assignment maze

LGTM