add appveyor build matrix with debug build

[sjaeckel]

No description provided.

[minad]

Can you additionally enable optimization to the debug build?

[sjaeckel]

which https://docs.microsoft.com/en-us/cpp/build/reference/o-options-optimize-code?view=vs-2019 ?

you can also add it yourself, I'm afk for a moment

[minad]

Hmm, it seems /Od is the mode without any optimization. /DEBUG is for debugging infos.

I think you basically never want /Od except for fast compilation speed etc. What do you propose @sjaeckel?

[sjaeckel]

AFAIU /Od is the default, as if no option is given, so we should probably make our code deal with that!?

[minad]

Yes, then we could to use the fix I described in #438. This would make the standard build compatible with the standard compiler settings. (Even if those settings are not good - I wonder what people are creating these unoptimized debug builds for.)

But still, the MP_HAS infrastructure would not work with /Od if functions are disabled in tommath_class.h. This means the fix would be incomplete. But if tommath_class is tweaked we are already doing a non standard build. In that case it is ok to require tweaking the options (disallowing /Od setting).

If we want to reconsider things regarding how MP_HAS works:
I would like to know if if (0) { doesnotexist(); } works with /Od. If that's the case the MP_HAS macro is too complicated and we might think about changing that.
But I don't think there are many options. Right now we rely on a tiny bit of constant folding. An alternative would be to not rely on constant folding but on variadic c99 macros instead, that could be enabled on newer compilers at least.

@nijtmans your opinion on this? You seem to use Windows/msvc mostly?

[sjaeckel]

Yes, then we could to use the fix I described in #438. This would make the standard build compatible with the standard compiler settings.

But still, the MP_HAS infrastructure would not work with /Od if functions are disabled in tommath_class.h. This means the fix would be incomplete. But if tommath_class is tweaked we are already doing a non standard build. In that case we could require tweaking the options (disallowing /Od setting).

it sounds like a good trade-off to have the standard build compile with standard compiler settings and if someone wants to tweak the library the compiler settings have to be tweaked as well

(Even if those settings are not good - I wonder what people are creating these unoptimized debug builds for.)

this was just a test to try something out

option 2 would be to leave it as is and only add a comment what has to be done to fix it in the documentation and maybe a pointer in s_mp_rand_platform.c!?
This is now a single spot where the build will break whereas when we patch the sources as proposed in #438 and then someone starts to tweak the library it will break on multiple spots...

If we want to reconsider things:

I like the current approach! I don't want to reconsider, do you?

I would like to know if if (0) { doesnotexist(); } works with /Od

doesn't look like it works:
https://ci.appveyor.com/project/libtom/libtommath/builds/28686042/job/3jvoclwohe5tjftj#L350

[minad]

Ok, I think we should just keep things as is and add a comment to s_mp_rand_platform, which says that /Od doesn't work due to insufficient dead code analysis. And if for some reason the user wants to use /Od, they can apply the patch themselves.

[minad]

closed in favor of #456