Re: [XEN PATCH] docs/misra: document the C dialect and translation toolchain assumptions.

[Roberto Bagnara <roberto.bagnara@xxxxxxxxxxx>]

On 16/06/23 22:43, Stefano Stabellini wrote:
> On Fri, 16 Jun 2023, Roberto Bagnara wrote:
> > > > +   * - Implicit conversion from a pointer to an incompatible pointer
> > > > +     - ARM64, X86_64
> > > > +     - Non-documented GCC extension.
> > > Is this related to -Wincompatible-pointer-types?
> > In my opinion, this does not specify what the result of the
> > conversion is.
> Fair enough. However, if -Wincompatible-pointer-types and "Implicit
> conversion from a pointer to an incompatible pointer" are related, it
> would add -Wincompatible-pointer-types as extra info about it. See also
> below.
>
>
> > > > +   * - Pointer to a function is converted to a pointer to an object or a
> > > > pointer to an object is converted to a pointer to a function
> > > > +     - X86_64
> > > > +     - Non-documented GCC extension.
> > > Is this J.5.7 of n1570?
> > > https://www.iso-9899.info/n1570.html
> > This says that function pointer casts are a common extension.
> > What we need here is documentation for GCC that assures us
> > that the extension is implemented and what its semantics is.
> >
> > > Or maybe we should link https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83584
> > My opinion is that this might not be accepted by an assessor:
> > if I was an assessor, I would not accept it.
> I understand your point and I think it is valid. My observation was
> that it is better to provide as much information for these undocumented
> extensions as we can. Not necessarily to help with an assessors, but for
> a new engineer working on this project, reading this document and
> understanding what can be done.
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83584 might not be an
> official documentation of the extension but it is better than no
> documentation at all. Even better might be a code example.
>
> I am not saying we should document ourselves what GCC failed to
> document. I am only saying we should add enough description to
> understand what we are talking about.
>
> For instance, I read "Pointer to a function is converted to a pointer to
> an object or a pointer to an object is converted to a pointer to a
> function" and I have an idea about what this is but I am not really
> sure. I googled the sentence and found information on Stackoverflow. I
> think it is better to link
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83584 or a couple of
> sentences from it, although it might not be official.
>
>
> > > > +   * - Ill-formed source detected by the parser
> > > As we are documenting compiler extensions that we are using, I am a bit
> > > confused by the name of this category of compiler extensions, and the
> > > reason why they are bundled together. After all, they are all separate
> > > compiler extensions? Should each of them have their own row?
> > Agreed.
> >
> > > > +     - ARM64, X86_64
> > > > +     - token pasting of ',' and __VA_ARGS__ is a GNU extension:
> > > > +          see Section "6.21 Macros with a Variable Number of Arguments"
> > > > of GCC_MANUAL.
> > > > +       must specify at least one argument for '...' parameter of variadic
> > > > macro:
> > > > +          see Section "6.21 Macros with a Variable Number of Arguments"
> > > > of GCC_MANUAL.
> > > > +       void function should not return void expression:
> > > I understand that GCC does a poor job at documenting several of these
> > > extensions. In fact a few of them are not even documented at all.
> > > However, if they are extensions, they should be described for what they
> > > do, not for the rule they violate. What do you think?
> > The point is that we don't know what they do.  We might make observations,
> > and our observations might substantiate what we believe they do.
> > But this would not allow us to generalize them.
> >
> > > For example, in this case maybe we should say "void function can return
> > > a void expression" ?
> > We can certainly say that, but this might not convince an assessor.
> > One possibility would be to submit patches to the GCC manual and see
> > whether they are accepted.
> I think we have two different target audiences for this document. One
> target is an assessors, and I understand that extra unofficial
> information might not help there.
>
> However another target is the community. This document should help the
> Xen community write better code, not just the assessors raise red flags.
> Right? It should help us have better compiler compatibility, and making
> sure that we are clear about the C dialect we use. Actually, I think
> this document could be of great help. Do you agree?
>
>  From that point of view "void function should not return void
> expression" is not understandable. At least I don't understand it.
>
> A different approach would be to say:
>
> - this is a MISRA C violation or compiler warning/error
> - it is not C99 compliant
> - it is not documented behavior by GCC
>
> Not try to describe what the extension is at all, and instead focus on
> what the MISRA C violation or compiler warning is.
>
> I think it is OK to go down that route, but in that case we need to
> reorganize the document so that:
> - all documented extensions are referred to as extensions
> - all undocumented extensions are referred to by the warning they
>    trigger
>
> I think that we would be OK but honestly I prefer the current approach
> and we just need to add a few extra words to better explain what the
> undocumented extensions are. Not to replace the GCC manual but simply
> because otherwise we are not understanding each other (at least I am not
> understanding.)
>
>
> > > > +          see the documentation for -Wreturn-type in Section "3.8 Options
> > > > to Request or Suppress Warnings" of GCC_MANUAL.
> > > > +       use of GNU statement expression extension from macro expansion:
> > > > +          see Section "6.1 Statements and Declarations in Expressions" of
> > > > GCC_MANUAL.
> > > > +       invalid application of sizeof to a void type:
> > > > +          see Section "6.24 Arithmetic on void- and Function-Pointers" of
> > > > GCC_MANUAL.
> > > > +       redeclaration of already-defined enum is a GNU extension:
> > > > +          see Section "6.49 Incomplete enum Types" of GCC_MANUAL.
> > > > +       static function is used in an inline function with external
> > > > linkage:
> > > > +          non-documented GCC extension.
> > > I am not sure if I follow about this one. Did you mean "static is used
> > > in an inline function with external linkage" ?
> > An inline function with external linkage can be inlined everywhere.
> > If that calls a static functions, which is not available everywhere,
> > the behavior is not defined.
> Got it. Can we add this sentence you wrote to the doc?
Hi Stefano.

I think all the feedback received has been taken into account.
I will send a revised patch soon.
Kind regards,

   Roberto