[GiNaC-devel] [SCM] GiNaC -- a C++ library for symbolic computations branch, master, updated. release_1-4-0-708-ga569b474

[Alexey Sheplyakov]

Dear Richard,

On 2/22/21 12:12 AM, Richard B. Kreckel wrote:

> Okay, it is indeed weird.
> 
> CLang++ complains about your explicit instantiation in lst.cpp: 
> template bool container<std::list>::info(unsigned) const; saying that
> the previous template specialization is in lst.hpp: template<> bool
> lst::info(unsigned inf) const; which is merely a declaration.
> 
> So, CLang++ seems to miss the entire the point of the explicit 
> instantiation.

Let's put it this way: either clang++ or /me misses the entire
point of the explicit instantiation.

>> // ignore bogus "explicit instantiation that occurs after an
>> explicit specialization" warning #if defined(__clang__) && 
`>> __has_warning("-Winstantiation-after-specialization") #pragma clang
>> diagnostic ignored "-Winstantiation-after-specialization" #endif
>> 
>>> Wouldn't it be sufficent to include "lst.h" in
>>> "integration_kernel.cpp"?
> 
> For the record: #including "lst.h" in "integration_kernel.cpp" as
> your patch does is indeed enough for building with MinGW64.

That's a good news. However it's still implementation defined.

>> I think it's implementation defined. The definition of lst::info()
>> is available in lst.cpp only, and nothing in that translation unit
>> uses lst::info(). Therefore the compiler is not obliged to
>> instantiate lst::info() (although it's OK to do so). gcc and clang
>> seem to instantiate it, but msvc needs the explicit instantiation
>> (or something which triggers the implicit instantiation, like 
>> `dummy_func` in exprseq.cpp)
> 
> I would like to keep the lst.* and exprseq.* similar to make it
> easier to read the code. (I had quite a hard time reading it.)

/me too. In fact I've got a patch which adds an explicit instantiation
of exprseq::info() and removes dummy_func(). I haven't had a chance to
test it with msvc yet.

> So I consider replacing the explicit instantiation in lst.* with the 
> dummy_func() trick in exprseq.*, or remove it altogether. (Do recent 
> versions of MSVC still need it?)

I don't quite like this idea. Let's keep the explicit instantiations,
and suppress clang++ warnings for now.

Making lst::info inline does not sound like a good idea either.
For one any change in inline function typically requires SONAME
bump [1], so there should be a pretty good reason to make
a function/method inline. Usually it's the performance, not
a (bogus) compiler warning.

[1] SONAME bump is NOT required if it's safe for programs linked with
     the prior version of the library to call the old implementation.
     However it's tricky and might be dangerous.


Best regards,
     Alexey