xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
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,
> -- 
> Julien Grall

  reply	other threads:[~2021-04-07 16:06 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-06 10:36 [PATCH 0/3] Use Doxygen and sphinx for html documentation Luca Fancellu
2021-04-06 10:36 ` [PATCH 1/3] docs: add doxygen support " Luca Fancellu
2021-04-06 10:36 ` [PATCH 2/3] docs: hypercalls sphinx skeleton for generated html Luca Fancellu
2021-04-06 10:36 ` [PATCH 3/3] docs/doxygen: doxygen documentation for grant_table.h Luca Fancellu
2021-04-06 11:19   ` Jan Beulich
2021-04-06 21:46     ` Stefano Stabellini
2021-04-07  8:10       ` Jan Beulich
2021-04-07  8:42         ` Luca Fancellu
2021-04-07  8:58           ` Jan Beulich
2021-04-07 21:26             ` Stefano Stabellini
2021-04-08  5:59               ` Jan Beulich
2021-04-08 11:02               ` Luca Fancellu
2021-04-08 11:28                 ` Jan Beulich
2021-04-08 11:40                 ` Julien Grall
2021-04-08 11:50                   ` Julien Grall
2021-04-08 13:13                     ` Luca Fancellu
2021-04-08 11:58                   ` Luca Fancellu
2021-04-07 13:13   ` Julien Grall
2021-04-07 13:19     ` Luca Fancellu
2021-04-07 13:56       ` Julien Grall
2021-04-07 14:51         ` Luca Fancellu
2021-04-07 15:19       ` Ian Jackson
2021-04-07 15:29         ` Bertrand Marquis
2021-04-07 15:54           ` Jan Beulich
2021-04-07 16:07             ` Bertrand Marquis
2021-04-07 15:55           ` Julien Grall
2021-04-07 16:06             ` Bertrand Marquis [this message]
2021-04-07 16:12               ` Ian Jackson
2021-04-06 11:53 ` [PATCH 0/3] Use Doxygen and sphinx for html documentation Andrew Cooper
2021-04-06 21:24   ` Stefano Stabellini
2021-04-07 11:15     ` Andrew Cooper
2021-04-07 13:05       ` Bertrand Marquis
2021-04-07 13:07 ` Julien Grall
2021-04-07 13:16   ` Luca Fancellu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9B8BD2C5-297C-4BE3-8476-5D10699712E2@arm.com \
    --to=bertrand.marquis@arm.com \
    --cc=Luca.Fancellu@arm.com \
    --cc=Wei.Chen@arm.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=george.dunlap@citrix.com \
    --cc=iwj@xenproject.org \
    --cc=jbeulich@suse.com \
    --cc=julien@xen.org \
    --cc=sstabellini@kernel.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \


* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).