Re: [PATCH 3/3] docs/doxygen: doxygen documentation for grant_table.h

From: Bertrand Marquis <Bertrand.Marquis@arm.com>
To: Julien Grall <julien@xen.org>
Cc: Ian Jackson <iwj@xenproject.org>,
	Luca Fancellu <Luca.Fancellu@arm.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	Wei Chen <Wei.Chen@arm.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Jan Beulich <jbeulich@suse.com>,
	Stefano Stabellini <sstabellini@kernel.org>, Wei Liu <wl@xen.org>
Subject: Re: [PATCH 3/3] docs/doxygen: doxygen documentation for grant_table.h
Date: Wed, 7 Apr 2021 16:06:05 +0000	[thread overview]
Message-ID: <9B8BD2C5-297C-4BE3-8476-5D10699712E2@arm.com> (raw)
In-Reply-To: <8b0fafe2-989e-5a5a-05ec-f55e096e4b54@xen.org>

Hi Julien,

> On 7 Apr 2021, at 16:55, Julien Grall <julien@xen.org> wrote:
> 
> 
> 
> On 07/04/2021 16:29, Bertrand Marquis wrote:
>> Hi Ian,
>>> On 7 Apr 2021, at 16:19, Ian Jackson <iwj@xenproject.org> wrote:
>>> 
>>> Luca Fancellu writes ("Re: [PATCH 3/3] docs/doxygen: doxygen documentation for grant_table.h"):
>>>> The problem is that Doxygen can’t generate proper documentation for anonymous union/structure, it ends up with warning and/or producing wrong documentation like
>>>> changing names or giving field description to the wrong field.
>>> 
>>> This does not seem like it would be an impossibly hard feature to add
>>> to doxygen.
>> Modifying doxygen is not really in our planned efforts and if someone does it that would put an hard constraint on the version of doxygen possible to use.
> 
> Are you saying that anyone who want to use doxygen has to waive off the use of anonymous union/struct? Is it the only thing doxygen can't deal with?

That is the main one we came into while doing this but there might be other going forward, hard to be sure at this stage.

> 
>> But is adding names to anonymous elements really an issue here ?
>> If we agree on names or on a convention for name the result will not impact the code or backward compatibility.
> 
> I think the naming is only the tip of the problem. One advantage of anymous union/struct is you make clear they are not meant to be used outside of the context. So they should mostly be seen as an easy way to access some part of the "parent" structure directly. Therefore, IMHO, they don't deserve to be documented separately.

Somehow in the documentation when you have a union you will need to document that it is a union and the possible entries.
Having a name to refer to it sounds to me a lot easier than making it anonymous.

One way or an other most standards like MISRA are forbidding anonymous entries as they cannot be referred to.

> 
> In fact, this is the first thing I noticed when building the documentation because 'union a' was in global index.

Definitely I agree the “a” is not a good solution and we need to find meaningful names.
But this is in fact true for the sub-element in the structure (from which the name was taken), using “a” as an identifier is not really explanatory of what that is.
“u” for union can be see as a standard though.

This is why i think we should put names which a meaning but this is not always easy to find.

Cheers
Bertrand

> 
> Cheers,
> 
> -- 
> Julien Grall