clisp-devel

Getting rid of varbrace

[Daniel J. <dan...@gm...>]

The following two patches remove varbrace from the build and fix some
compilation errors due to that removal. My configuration built with those
two patches works fine (according to make check).

I didn't do an exhaustive test of different configurations, so this
definitively needs more testing. Also the patches:

- Contain no comment what the #define var does and why its there.
- Don't mention any change in the change log.
- Remove an (hopefully!) unused variable. This should probably be a
separate commit.
- Manually add a pairs of braces at two places; I couldn't figure out what
exactly the code there does, so I went the safe route.
- Don't remove the varbrace util yet; it's just not built / used.

Most of the compilation errors complained about having only a declaration
following an (goto) label. I solved this by adding a semicolon behind each
such label ("label:" becomse "label:;").

Is this a viable approach at getting rid of varbrace soon? If so then I can
"polish" those patches further.

Re: Getting rid of varbrace

[Sam S. <sd...@gn...>]

Daniel,

I will leave installing the patch to Bruno because this is a portability
issue.
However, I have some comments inline below.

> * Daniel Jour <qna...@tz...> [2017-03-21 01:01:13 +0000]:
>
> - Contain no comment what the #define var does and why its there.

Not good.
Given that we have almost 12k "var" instances:

--8<---------------cut here---------------start------------->8---
$ grep '^ *var ' *.d | wc -l
   11760
--8<---------------cut here---------------end--------------->8---

we need either a comment or a thorough purge.
I favor the latter because I want the CLISP sources to _look_ like
regular C.

Note that the purge can be done with a perl one-liner.

> - Don't mention any change in the change log.

Bruno will be irate :-)

> - Remove an (hopefully!) unused variable. This should probably be a
> separate commit.

Absolutely. Please either make a separate patch or tell me where the
unused var is.

> - Manually add a pairs of braces at two places; I couldn't figure out what
> exactly the code there does, so I went the safe route.

Why did you need the braces at all?

> Most of the compilation errors complained about having only a declaration
> following an (goto) label. I solved this by adding a semicolon behind each
> such label ("label:" becomse "label:;").

No, please add braces instead.

> Is this a viable approach at getting rid of varbrace soon? If so then
> I can "polish" those patches further.

I will let Bruno decide on this.

-- 
Sam Steingold (http://sds.podval.org/) on darwin Ns 10.3.1504
http://steingoldpsychology.com http://www.childpsy.net http://www.memritv.org
http://memri.org http://honestreporting.com http://think-israel.org
Don't take life too seriously, you'll never get out of it alive!

Re: Getting rid of varbrace

[Sam S. <sd...@gn...>]

Hi Bruno,

> * Sam Steingold <fq...@ta...> [2017-03-21 09:19:08 -0400]:
>
> Given that we have almost 12k "var" instances:
>
> $ grep '^ *var ' *.d | wc -l
>    11760
>
> we need either a comment or a thorough purge.
> I favor the latter because I want the CLISP sources to _look_ like
> regular C.
>
> Note that the purge can be done with a perl one-liner.

Here is the sed(1) one-liner (tested on zthread.d and unixaux.d):

--8<---------------cut here---------------start------------->8---
sed -i "" -e 's/^\( *\)var /\1/' *.d
--8<---------------cut here---------------end--------------->8---

Are we using any non-C99 compilers?

Thanks.

-- 
Sam Steingold (http://sds.podval.org/) on darwin Ns 10.3.1504
http://steingoldpsychology.com http://www.childpsy.net http://jij.org
http://americancensorship.org http://camera.org https://ffii.org
DRM "access management" == prison "freedom management".

Re: Getting rid of varbrace

[Reini U. <rei...@gm...>]

> On Mar 21, 2017, at 5:47 PM, Sam Steingold <sd...@gn...> wrote:
> 
> Here is the sed(1) one-liner (tested on zthread.d and unixaux.d):
> 
> --8<---------------cut here---------------start------------->8---
> sed -i "" -e 's/^\( *\)var /\1/' *.d
> --8<---------------cut here---------------end--------------->8---
> 
> Are we using any non-C99 compilers?

Yes, we are. MSVC still, though we could get away only using mingw and dump msvc.

Re: Getting rid of varbrace

[Sam S. <sd...@gn...>]

> * Reini Urban <erv...@tz...> [2017-03-22 10:34:06 +0100]:
>
>> On Mar 21, 2017, at 5:47 PM, Sam Steingold <sd...@gn...> wrote:
>> 
>> Here is the sed(1) one-liner (tested on zthread.d and unixaux.d):
>> 
>> --8<---------------cut here---------------start------------->8---
>> sed -i "" -e 's/^\( *\)var /\1/' *.d
>> --8<---------------cut here---------------end--------------->8---
>> 
>> Are we using any non-C99 compilers?
>
> Yes, we are. MSVC still, though we could get away only using mingw and dump msvc.

If we are to believe [MS]
(https://msdn.microsoft.com/en-us/library/hh409293(v=vs.120).aspx):
Visual Studio 2013 Supports these ISO C99 language features
* Mixing declarations with code.


-- 
Sam Steingold (http://sds.podval.org/) on darwin Ns 10.3.1504
http://steingoldpsychology.com http://www.childpsy.net https://ffii.org
http://jij.org http://www.dhimmitude.org http://think-israel.org
If your VCR is still blinking 12:00, you don't want Linux.

Re: Getting rid of varbrace

[Daniel J. <dan...@gm...>]

Hi Sam,

> we need either a comment or a thorough purge.
> I favor the latter because I want the CLISP sources to _look_ like
> regular C.

Me too. Though I got the impression from our discussions last year
that a full purge - due to the massive commit this would cause - is
not wanted. That's why I went the `#define var` way.


> Note that the purge can be done with a perl one-liner.

Yes, but bear in mind that the purge is only the first step.  Removing
var causes problems.  In the second patch of the original mail I only
corrected those which caused compile errors for my configuration.
There are possibly many more of them when looking through all the
code.  Examples:

A declaration cannot immediately follow an label. There are LOTS of
those errors. Example:

label:
  var type name;

I solved this like so:

label:;
  var type name;


> No, please add braces instead.

This might work in some cases, but most of the affected code is highly
"unstructured". If I could easily add braces without rewritting it,
then there wouldn't be labels and gotos but if, while and friends.

I tried to find more instances of those with:

find src -type f -name '*.d' -exec \
  pcregrep -MHno \
    '[[:alnum:]]+:([^:;{]|\n)*?var([^;]|\n)*+;' '{}' ';' \
    2>/dev/null \
  | sed 's_^src/_\x0src/_g' \
  | grep -z -v -F ': */' \
  | less

But this shows many false positives...


Another issue is conflicting definitions of the same variable. Example:

var type1 foo;
// some code
var type2 foo;

This is solved by renaming one of them, if possible. In two places, I
manually added braces (like varbrace would've done), specifically in
src/intlog.d, function I_logcount_I, the variables with the name (as
written in the source code) "x16". The issue is that "x16" might be
"#define" to something else, and I didn't exactly understood why and
when.


> Please either make a separate patch or tell me where the
unused var is.

I attached a patch with the small change (including an entry in the
changelog, just to be sure ;) )


> I will let Bruno decide on this.

Bruno, what do you think?