Re: [Pan-users] Pan bug 712539 and 757672

[Rhialto]

On Sat 05 Mar 2016 at 21:53:13 +0100, Detlef Graef wrote:
> https://bugzilla.gnome.org/show_bug.cgi?id=712539
> https://bugzilla.gnome.org/show_bug.cgi?id=757672

> error: invalid application of 'sizeof' to an incomplete type
> 'value_type' (aka 'pan::FilterInfo')
> 
> As describe in bug 712539 it seems the problem is at line 77 in the
> following file:
> 
> https://git.gnome.org/browse/pan2/tree/pan/usenet-utils/filter-info.h
> 
> Line 77: typedef std::deque<FilterInfo> aggregates_t;
> 
> The class FilterInfo is used within its own definition. Is there a
> simple way to fix this? Any way of some kind of forward declaration?

I've seen that problem before, also when clang was to compile Pan,
think. Or it may have been something else maybe. But I'd swear it was
with Pan.

The issue is that  a std::deque (or std::list or whatever) can only
contain completed types according to the standard. But the g++ library
is made in such a way that it also works for incomplete types. Only when
you use clang, where the library does enforce the limitation, you see it
fail.

In this case, like in the case I remember, the code was trying to define
a logical impossibility. Here we have a FilterInfo object, that is
supposed to contain multiple FilterInfo objects. If you think about
that, it is not possible. What likely really was going on was that a g++
deque would use pointers or references or some such indirections to make
the implementation work.

That also indicates the solution: introduce an indirection.
1) Make it a deque of pointers to FilterInfo.

      /** Convenience typedef. */
      typedef std::deque<FilterInfo *> aggregates_t;

      /** When `_type' is AGGREGATE_OR or AGGREGATE_AND,
          these are the filters being or'ed or and'ed together. */
      aggregates_t _aggregates;

OR 2) make a pointer to the deque, if the typedef can be fixed somehow:

      /** Convenience typedef. */
      typedef std::deque<FilterInfo> aggregates_t;

      /** When `_type' is AGGREGATE_OR or AGGREGATE_AND,
          these are the filters being or'ed or and'ed together. */
      aggregates_t *_aggregates;

That can probably be done easiest by defining aggregates_t as a separate
class after the definition of FilterInfo.

Unfortunately that means that all uses of the typedef/class must change
too.  At least, all objects that are  to be put in the deque should
probably be allocated explicitly now.

The easiest way to find all uses is to simply change it and count
compiler errors; see which alternative has the fewest and go with that
:-)
I'd probably prefer option 1) since altering the deque then doesn't
involve copying whole FilterInfo objects around, just pointers.

> The fix should work with gcc and clang. :-)

It will :-)

Ah, I found a reference:
http://cvsweb.netbsd.org/bsdweb.cgi/pkgsrc/news/pan/patches/Attic/patch-pan_usenet-utils_filter-info.h?rev=1.3&content-type=text/x-cvsweb-markup
Look for more patches with revisions labeled "Add patches from joerg to
fix build with clang/libc++." near there. Apparently the patches have
been removed later because "Sadly, they make pan very unstable. ".
So somebody should re-do them correctly.

> Detlef
-Olaf.
-- 
___ Olaf 'Rhialto' Seibert  -- The Doctor: No, 'eureka' is Greek for
\X/ rhialto/at/xs4all.nl    -- 'this bath is too hot.'

signature.asc
Description: PGP signature