[GiNaC-list] msvc support patch (Was: use of STL iterators in GiNaC)
michael.goffioul at gmail.com
Fri Apr 17 12:12:31 CEST 2009
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++.
> (How) Did you manage to compile CLN with msvc?
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.
More information about the GiNaC-list