About ProgressReport and msn-pecan

[Felipe Contreras]

On Sun, Jun 15, 2008 at 8:08 PM, Tim Ringenbach
<tim.ringenbach at gmail.com> wrote:
> On Sun, Jun 15, 2008 at 6:24 AM, Felipe Contreras
> <felipe.contreras at gmail.com> wrote:
>> Hi Tim,
>>
>> On Sun, Jun 15, 2008 at 1:58 AM, Tim Ringenbach
>> <tim.ringenbach at gmail.com> wrote:
>>> On Fri, Jun 13, 2008 at 8:18 PM, Felipe Contreras
>>> <felipe.contreras at gmail.com> wrote:
>>>> account.h:
>>>> typedef struct PurpleAccount PurpleAccount;
>>>>
>>>> account_private.h:
>>>> struct PurpleAccount
>>>> {
>>>>        char *username;
>>>>        char *alias;
>>>>        char *password;
>>>>        char *user_info;
>>>>        ...
>>>>        PurplePresence *presence;
>>>>        ...
>>>> }
>>>>
>>>> That means that if you include account.h you can't do
>>>> account->username, because "account" is an incomplete type, just like
>>>> a void *. Note also that PurplePresence doesn't need to be defined in
>>>> account.h, hence you remove a header dependency for all the files that
>>>> include account.h, and you remove all the headers that presence.h
>>>> include, and so on.
>>>
>>> Couldn't you do the same thing by just not including the extra files,
>>> and adding struct _PurpleBlah; for each missing type, and changing the
>>> prototypes to use the struct _PurpleBlah instead of PurpleBlah? (IIRC,
>>> if you try to do the typedef more than once you get an error, which is
>>> why I said use the struct version of the name)
>>
>> I don't understand what you are saying.
>
> I'm saying you can avoid including the presence.h header in the
> accounts.h header by just saying struct _PurplePresence;. And that
> IIRC you can say struct _PurplePresence; more than once but if you try
> to say typedef struct _PurplePresence PurplePresence; more than once
> you get a compiler error.

Oh, yes, you can do that, but things become messy.

>>> What's the end goal here? Just to speed up compiling and make the
>>> graph shorter, if technically misleading? (By misleading I mean the
>>> account module really does use the presence module even though it
>>> wouldn't show on the graph anymore because the header isn't including
>>> it)
>>
>> No. The public and the private fields are two completely different things.
>
> Ok, so are you talking about moving toward struct hiding, which is
> ultimately an ABI breaking change, though as you said in a later email
> it's not impossible to move toward it without breaking the ABI.

Right.

>> You might find surprising that GHashNode is not available in
>> any header file, ever. That's a clean modularization.
>
> I actually don't find that surprising because I understand struct
> hiding. I just wanted to know what your goals were. If your goal was
> struct hiding then I wanted to make sure you knew the final cutover to
> that had to wait until we could break the API.
>
> However, you're still going to be including just as many files, are
> you not? They'll be slightly shorter files (no structs in them). But
> you'll still have the prototypes of all the public functions (which
> they'll likely be a lot since we're doing struct hiding). And even
> though you don't call those functions from another header file, that
> header file still has to include the headers for all the types it
> uses.

When things are badly modularized, yes. But even in the current state
of Pidgin, I would guess there will be a few savings here and there.

In any case, splitting the headers makes these issues more visible.

>> Why burden the user, the compiler, and the documentation generation
>> tools with things that should be transparent?
>>
>> What is _misleading_ is that you need to know all the internal details
>> of Presence; you don't, you just need the public API. Actually, I just
>> checked and Presence is properly hiding the internal details, but not
>> Account, nor Proxy which is used by Account.
>
> Point taken. But currently the struct definition is part of the public
> api, and even if it ought not be, we can't simply remove it without
> breaking the API compatibility promises we made. But I think that
> point has probably been explained enough.

I hope you are talking about my explanation about foo_public.h and
foo_private.h both being included in foo.h as a temporal (API stable)
solution.

Best regards.

-- 
Felipe Contreras