[GiNaC-list] msvc support patch (Was: use of STL iterators in GiNaC)

[Michael Goffioul]

On Fri, Apr 17, 2009 at 9:17 AM, Alexei Sheplyakov <varg at metalica.kh.ua> wrote:
> Thanks for a patch. Supporting more platforms/compilers  is certainly
> a good thing as long as it doesn't break the code and doesn't introduce
> subtle changes. Unfortunately your patch *does* introduce subtle changes
> (and breaks compilation with any non-m$ compiler).
>
>> Note that it is not intended to be applied as-is to ginac source tree.
>
> Anyway, it would be nice if you avoid introducing the breakage and semantic
> changes, or at least clearly mark them.

You know, it took me some time to track down incorrect usage of
STL iterators in ginac sources. I tried to fix the ones I could find
(because test suite crashed), but I'm sure there are more of them.
Yes, it's possible that I introduced other problems, but I have no
knowledge of ginac and what it's doing internally, so those fixes
are just blind guesses.

The patch was more intended to identify problems and propose
a naive solution (which at least you know works with MSVC). If
you don't want to use it, then don't.

> diff -ur ginac-1.5.1/ginac/ncmul.cpp ginac-1.5.1-new/ginac/ncmul.cpp
> --- ginac-1.5.1/ginac/ncmul.cpp 2009-02-17 13:39:22 +0000
> +++ ginac-1.5.1-new/ginac/ncmul.cpp     2009-04-15 23:14:00 +0100
> @@ -340,7 +340,7 @@
>
>        // determine return types
>        unsignedvector rettypes;
> -       rettypes.reserve(assocseq.size());
> +       rettypes.resize(assocseq.size());
>
> I'm afraid this change breaks canonicalization of non-commutative products.

I'm afraid that setting vector's capacity doesn't allow you to
reference elements outside of the actual vector's size.

>> Some comments about the patch:
>> - the __alignof__ vs. __alignof could be turned into a configure test
>
> The patch as is breaks compilation with any non-m$ compiler. That's
> certainly not welcome.

I know it breaks non-VC compiler. That's why I mention the
use of a configure test. But I leave that up to you.

>> - VC++ does not support __func__ macro, so I changed it into __FILE__
>
> Stripping debugging info is not welcome either. Please #ifdef _MSVC (or
> whatever it is) these changes.
>
>> - in exprseq.cpp, I had to add a dummy function to make VC++ instantiate
>> expreseq::info(); don't know why...
>
> Please #ifdef _MSVC that function. Also it would be nice to have a comment
> in the source explaining why it's necessary (otherwise it's so tempting to
> delete that pointless code). Other changes, like
>
> diff -ur ginac-1.5.1/ginac/parser/parser.cpp ginac-1.5.1-new/ginac/parser/parser.cpp
> --- ginac-1.5.1/ginac/parser/parser.cpp 2009-02-17 13:39:22 +0000
> +++ ginac-1.5.1-new/ginac/parser/parser.cpp     2009-04-15 20:59:47 +0100
> @@ -82,7 +82,7 @@
>  }
>
>  extern numeric* _num_1_p;
> -extern ex _ex0;
> +extern const ex _ex0;
>
> need such an #ifdef and a comment too.

Why? The exported symbol uses the "const" modifier. Declaring it non-const
like this implicitely assumes that the C++ mangled symbols (const and
non-const) will be the same. That's not the case in VC++.

> P.S.
> (How) Did you manage to compile CLN with msvc?

Magic.

VC++ per-se is quite (not fully) standard compliant. Most of the
problems are in the autotool based build framework. With a bit
of coding, you can make VC++ look like GCC.

Michael.