some linting

[fperrad]

please double check mp_div.c (related to the merge of #370 )

[minad]

@fperrad could you comment please on the question if mp_bool could be treated as bool in the linter? See #405. Teat the mp_bool warnings as false positives?

[minad]

@czurnieden please check mp_div

[czurnieden]

@czurnieden please check mp_div

What have I broken now? ;-)

Ah, I see, thanks!

[fperrad]

@fperrad could you comment please on the question if mp_bool could be treated as bool in the linter? See #405. Teat the mp_bool warnings as false positives?

@minad my linter knows bool (C99), so I think that the following trick will do the job

#if LINTER
#include <stdbool.h>
typedef bool mp_bool;
#define MP_NO   false
#define MP_YES  true
#else
typedef enum {
   MP_NO = 0,
   MP_YES = 1
} mp_bool;
#endif

[minad]

@fperrad Great, thanks! I will add this to my PR #405.

[fperrad]

@minad could we easily detect that the compiler supports C99 or has a stdbool.h ?

[minad]

On modern compilers we have things like __has_include but I don't think there is a way besides whitelisting compilers. Maybe we should just make the switch to stdbool.h? Platforms which don't have it can easily provide this header themselves. If no stdbool.h is desired it is also no problem to do the replacement using sed automatically, like in the no-stdint-h branch.

#ifndef _stdbool_h
#define _stdbool_h
typedef enum { false, true } bool;
#endif

[fperrad]

@minad, the problem is usually with MSVC (not modern)

#ifndef _MSC_VER
#include <stdbool.h>
#endif

#ifndef __bool_true_false_are_defined
typedef enum { false, true } bool;
#endif

note: __bool_true_false_are_defined is described in the standard C99.

[minad]

@fperrad Yes. But I am hesitant to add such compatibility code to the header since it might interfer with other headers. If we add compatibility code with a guard it should be unproblematic. Alternatively I simply provide stdint.h and stdbool.h myself as compatibility headers on legacy platforms.

#ifndef __bool_true_false_are_defined
typedef enum { false, true } bool;
#define __bool_true_false_are_defined
#endif

However @nijtmans instead decided to patch tommath in tcl, introducing MP_NO_STDINT and redefining mp_digit as TclWideUInt on 64 platforms. See https://github.com/tcltk/tcl/blob/master/libtommath/tommath.h

I asked @nijtmans in the other PR for his opinion. Unfortunately the last time I proposed applying such a change the discussion got quite heated. Hopefully we will find a good solution now.

[nijtmans]

However @nijtmans instead decided to patch tommath in tcl, introducing MP_NO_STDINT and redefining mp_digit as TclWideUInt on 64 platforms. See https://github.com/tcltk/tcl/blob/master/libtommath/tommath.h

For Tcl 8.6 and 8.7, the minimum requirement is VC++ 6.0 on Windows XP. The Tcl 8.x line will stay on libtommath 1.2.0, since moving to 2.0 will be binary incompatle. That needs to wait for Tcl 9.0, which is what the Tcl's "master" branch aims at.

I asked @nijtmans in the other PR for his opinion. Unfortunately the last time I proposed applying such a change the discussion got quite heated. Hopefully we will find a good solution now.

Well, Tcl's master branch is aimed at Tcl 9.0, which needs to build on Visual Studio 2008 and higher (I believe minimum Vista, but it might as well become Windows 7). So, anything that builds on Visual Studio 2008 is acceptable to me. For the libtommath 1.2 line, I'm not spending any effort any more.

[minad]

@nijtmans Unfortunately I don't know enough about MSVC. The last version I used was VC6. Is stdbool acceptable for you? I mean after 20 years we can reasonably require that header to be available. Given the advantage regarding static analysis even more. And it will be at least half a year until 2.0 is out I think.

[minad]

It seems they added stdbool in VC2015. I don't understand Microsoft. Why are they so slow. Maybe this was some kind of refusal to implement part of the standard with which they don't agree. @nijtmans is it acceptable if you provide a compatibility header in tcl or if you replace the stdbool include by the previously given guarded definition?

[nijtmans]

It seems they added stdbool in VC2015. I don't understand Microsoft. Why are they so slow. Maybe this was some kind of refusal to implement part of the standard with which they don't agree. @nijtmans is it acceptable if you provide a compatibility header in tcl or if you replace the stdbool include by the previously given guarded definition?

How about MP_NO_STDBOOL ???

[minad]

Yes we could do that. But this imposes a new legacy cost on this project. It is not unreasonable to drop legacy support in ltm given the fact that stdbool is trivially to provide and it's been 20 years now.
Furthermore I think that ltm can also make that decision independently of the downstream projects. But since you @nijtmans are also involved here and in tcl you are obviously part of the decision process. And for sure - the goal is not to mess with the downstream users for no reason. I just want to argue that there are two sides.

Even for tcl - why are you aiming for vs2008 instead of 2015 for the new version? Why support software which is already out if the support window?

I think in the end we should just have a vote:

1. use stdbool unconditionally (simplest, no legacy solution)
2. provide our own mp_bool if MP_NO_STDBOOL is defined or define mp_bool as bool otherwise
3. Keep things as is with our own mp_bool (however there are problems with static analysis)

What do you think @sjaeckel, @czurnieden, @fperrad, @nijtmans?

[nijtmans]

I think in the end we should just have a vote:

1. use stdbool unconditionally (simplest, no legacy solution)
2. provide our own mp_bool if MP_NO_STDBOOL is defined or define mp_bool as bool otherwise
3. Keep things as is with our own mp_bool (however there are problems with static analysis)

What do you think @sjaeckel, @czurnieden, @fperrad, @nijtmans?

My vote would be for 2)

[minad]

I vote for 1.

[sjaeckel]

why are you aiming for vs2008 instead of 2015 for the new version? Why support software which is already out if the support window?

@nijtmans can you answer that question?

[nijtmans]

why are you aiming for vs2008 instead of 2015 for the new version? Why support software which is already out if the support window?

@nijtmans can you answer that question?

Because that's what users compiling Tcl still use. I think the reason that VC++ 6.0 is still used, is because some users still run Tcl on Windows XP. VC++ was the last compiler from Microsoft supporting that. For Tcl 9.0 that cannot hold any more, the reason is the time_t type. Starting with VS 2008, time_t became 64-bit by default, we don't want to use time64_t everywhere in Tcl. That's why the minimum will become VS2008, I'm sure there are Tcl users which don't want to upgrade their Visual Studio installation. The time_t issue is a good argument stopping supporting for < VS2008.

[minad]

I think there is also a strong argument to drop legacy support. And this is better static analysis and better code quality. For example the better scoping in c99 is a huge win for correctness. But this is not even in the table here. In particular since we already have stdint, stdbool is not a heavy additional requirement.

Furthermore what worries me - if we decide to delay future proving the code now, when could it possibly happen? In 2030? This is not the timescales I am interested in right now.

But it is definitely a clash of different development styles. I rather prefer using better tools at the cost of cutting out legacy stuff. @nijtmans wants to support everything that could possibly be supported.

[minad]

I forgot - in the options given above, there should also be the possibility to define bool ourselves in a guard. But I don't particularly like it since we define sth not in our mp_ namespace.

[sjaeckel]

I think in the end we should just have a vote:

1. use stdbool unconditionally (simplest, no legacy solution)
2. provide our own mp_bool if MP_NO_STDBOOL is defined or define mp_bool as bool otherwise
3. Keep things as is with our own mp_bool (however there are problems with static analysis)

My clear preference is 1. but I don't see a good reason why we shouldn't do 2.
IIUC writing code for and against ltm won't change or am I missing something? The only difference between 1 and 2 will be the API signatures&local variables having either bool or mp_bool!?

[minad]

I think there is an improvement in readability/clarity if not every library introduces their own types for standard things. Furthermore there is the consistency argument, we also use stdint and not mp_uint32 etc. I don't see why we should do it differently this time. Tcl can work around stdint not being present, they can work around stdbool not being present.
However I would advise them also to not patch tommath in the way they are doing, eg using TclWideUInt instead of uint64_t. Instead adding compatibility headers to the tcl codebase would work. I've not seen an argument against that yet. There is just a resistance against introducing a compatibility header. I don't understand the reason however.

To the reason in the greater context - I would like to push for a modernized code base at some point, this means c99. But already at the harmless triviality of stdbool, there is resistance. I don't buy the argument to support users who don't want to update their legacy software. In particular since there are better alternatives available (more modern MSVC, modern gcc, modern clang, ...) This is bad for security, bad for maintainance etc etc. This is not something I support. If tcl wants to do that, sure! But in a project I am contributing to, my vote goes to the modern approach. This is also what I mean with independence - if two projects have differing approaches to things that's ok. I don't see why we should be forced to do things the tcl way. But I am not pushing for things that are unreasonable, with stdbool it won't be harder to do the patching they are doing. They are essentially relying on a fork.

Now after having complained about tcl and the ltm integration, I should also say that I used tcl for quite a while, liked it quite a bit and I am happy that there is still development going on there. So I am not interested in somehow sabotaging their development.

[minad]

@nijtmans @sjaeckel since you are working with a ltm fork in tcl, would it be acceptable if we use stdbool.h in develop etc but provide a command "helper.pl --no-stdbool" which simply perforns the necessary substitutions? We can even test this in travis-ci/appveyor to make sure everything works as expected. This is a bit like the no-stdint-h branch but automated and tested.

[czurnieden]

What do you think

As long as nobody adds a FILE_NOT_FOUND ;-)

But lame jokes aside: I was under the impression that our minimum supported C standard has already been raised to C99 for version 2.0.0? In that case it would be "1".

On the other side: I know the pain of supporting legacy…uhm…stuff so it can be "2", too.
Be it with a flag like MP_NO_STDBOOL or, as @minad suggested, a bit of S&R with a line or two in helper.pl.

[minad]

@czurnieden I think we should go to c99 in 2.0. This is the 2020 version and it will be over 20 years of the "new" standard. But this will be a little bit more work.
My proposal regarding legacy support would be that we ensure that the 1.x series is maintained (adding bugfixes, but no new features or other improvements).

[minad]

@czurnieden I made an issue here #416