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

[Luca Fancellu <luca.fancellu@xxxxxxx>]

> On 8 Apr 2021, at 12:40, Julien Grall <julien@xxxxxxx> wrote:
> 
> Hi Luca,
> 
> On 08/04/2021 12:02, Luca Fancellu wrote:
>>> On 7 Apr 2021, at 22:26, Stefano Stabellini <sstabellini@xxxxxxxxxx> wrote:
>>> 
>>> On Wed, 7 Apr 2021, Jan Beulich wrote:
>>>> On 07.04.2021 10:42, Luca Fancellu wrote:
>>>>> Just to be sure that we are in the same page, are you suggesting to 
>>>>> modify the name
>>>>> In this way?
>>>>> 
>>>>> struct gnttab_cache_flush {
>>>>> -    union {
>>>>> +    union xen_gnttab_cache_flush_a {
>>>>>        uint64_t dev_bus_addr;
>>>>>        grant_ref_t ref;
>>>>>    } a;
>>>>> 
>>>>> Following this kind of pattern: xen_<upper struct name>_<member name> ?
>>>> 
>>>> While in general I would be fine with this scheme, for field names like
>>>> "a" or "u" it doesn't fit well imo.
>>> 
>>> "a" is a bad name anyway, even for the member. We can take the
>>> opportunity to find a better name. Almost anything would be better than
>>> "a". Maybe "refaddr"?
>>> 
>>> 
>>>> I'm also unconvinced this would be
>>>> scalable to the case where there's further struct/union nesting.
>>> 
>>> How many of these instances of multilevel nesting do we have? Luca might
>>> know. Probably not many? They could be special-cased.
>> There are not many multilevel nesting instances of anonymous struct/union 
>> and the maximum level of nesting I found in the public headers is 2:
>> union {
>>      union/struct {
>>              …
>>      } <name>
>> } <name>
>> I also see that in the majority of cases the unions have not meaningful 
>> names like “a” or “u” as member name, instead struct names are fine,
>> It could be fine to keep the meaningful name the same for the struct type 
>> name and use the pattern for the non-meaningful ones as long
>> as the names doesn’t create compilation errors?
>> Example:
>> struct upper_level {
>>      union {
>>              struct {
>>              
>>              } meaningful_name1;
>>              struct {
>>              
>>              } meaningful_name2;
>>      } u;
>> };
>> becomes:
>> struct upper_level {
>>      union upper_level_u {
>>              struct meaningful_name1 {
>>              
>>              } meaningful_name1;
>>              struct meaningful_name2 {
>>              
>>              } meaningful_name2;
>>      } u;
>> };
> 
> If I understand correctly your proposal, the name of the structure would be 
> the name of the field. The name of the fields are usually pretty generic so 
> you will likely end up to redefine the structure name.
> 
> Unless we want to provide random name, the only safe naming would be to 
> define the structure as upper_level_u_meaningful_name{1, 2}. But, this is 
> going to be pretty awful to read.
> 
> But I am still a bit puzzled by the fact doxygen is not capable to deal with 
> anynomous/unamed union. How do other projects deal with them?
> 
>> Doing this will help a lot the documentation side because the html page will 
>> show the "struct upper_level" with inside the “union upper_level_u" and 
>> inside again
>> the two struct “meaningful_name1" and “meaningful_name2", and from the point 
>> of view of the developer it can tell her/him exactly the name of the member 
>> to
>> access when writing code (apart from the upper_level_u that can be accessed 
>> by u, but we can’t clearly change it).
> 
> I don't quite understand the last point. Wouldn't the developper see the 
> field name? So how is it going to be different from seeing the structure name?

The developer, that is using the documentation generated with sphinx+doxygen, 
will see the structure name and not the field name because this is the way
sphinx+doxygen is rendering the code structures. You can see it in the 
generated documentation using this serie.

> 
>> If this sounds difficult to understand when reading, please generate the 
>> documentation and have a look on the page in one side and the source code in 
>> another.
> 
> Just to be clear, do you mean understanding what you wrote or a developper 
> trying to understand the code?

I meant understanding what I wrote, because I know it’s difficult to describe 
the concept without visualising the html page, it would have been much easier 
to attach
an image to clarify.

> 
> Cheers,
> 
> -- 
> Julien Grall