[sdcc-devel] Bug #3916 and a grammar wart: (void (code *) ())

[Christopher B. <chr...@gm...>]

Dear all,

In a comment, Philipp pointed out test failures caused by an attempted
fix of bug #3916, "Syntax error if a function type name is the operand
of typeof":
https://sourceforge.net/p/sdcc/bugs/3916/#4858/ca9d

The failure in support/regression/tests/bug-716242.c was the more
challenging one.
To my surprise, SDCC accepts the following statement:

((void (__code *) (void)) p) ();

This appears to be a cast of 'p' to type
pointer-to-__code-qualified-function.  I don't see the point of the
address space qualifier in this cast: aren't all functions in the
__code address space? Even if that is somehow not true, aren't they
__code by default?

I found it hard to understand what the intent really is with this
grammar extension because it is accepted inconsistently by SDCC: only
in casts or typeof operands and not in declarations of function
pointers.

int main(void)
{
    (int (__code *)(void))0; // OK
    typeof(int (__code *)(void)) p = 0; // OK

    (int (_Optional *)(void))0; // OK
    typeof(int (_Optional *)(void)) r = 0; // OK

    (int (const *)(void))0; // OK
    typeof(int (const *)(void)) u = 0; // OK

    int (__code *q)(void) = 0; // syntax error
    int (_Optional *t)(void) = 0; // syntax error

    (int (*)(void) __code)0; // syntax error
    typeof(int (*)(void) __code) v = 0; // syntax error

    (int (*)(void) _Optional)0; // syntax error
    typeof(int (*)(void) _Optional) v = 0; // syntax error

    return 0;
}
https://godbolt.org/z/KWxaocKG9

I managed to trace its introduction to revision r1947 ('fixed bug
#516625'), by Johan Knol on 22 February 2002:

Index: src/SDCC.y
===================================================================
--- src/SDCC.y (revision 1946)
+++ src/SDCC.y (revision 1947)
@@ -1156,6 +1156,12 @@
        // $1 must be a pointer to a function
        sym_link *p=newLink();
        DCL_TYPE(p) = FUNCTION;
+       if (!$1) {
+ // ((void (code *) (void)) 0) ()
+ $1=newLink();
+ DCL_TYPE($1)=CPOINTER;
+ $$ = $1;
+       }
        $1->next=p;
      }
    }

Bug #516625 is now https://sourceforge.net/p/sdcc/bugs/294/ which says:

In a library for the Keil compiler this piece of code
is used for restarting the program:
#define abort() (((void (code *) (void)) 0) ())

So, that might explain why the weird grammar is only supported in
casts: it was introduced for compatibility with a specific header.

Section 5.3 of the draft ISO Technical Report 'Extensions to support
embedded processors'
(https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1275.pdf )
explicitly prohibits the only plausible interpretation of type-name
used in the Keil header:

"Clause 6.7.3 - Type qualifiers, add three new constraint paragraphs:
...
A function type shall not be qualified by an address-space qualifier"

The constraint-violating grammar used by the Keil header also
introduces ambiguity with function type names, because it is unclear
whether a type name such as void (code *) (void) is meant to be an
abstract declaration of a function with a single parameter of type
code *, or a pointer to some type derived from void that is qualified
by code.

According to the ISO C23 standard, the only grammar rule that can
match 'code' in the above macro definition is type-qualifier (via
function-abstract-declarator, parameter-type-list, parameter-list,
parameter-declaration, declaration-specifiers, declaration-specifier
and type-specifier-qualifier). This is fine because the embedded C TR
says type-qualifier can be address-space-name. The following '*' can
then match the pointer rule (via parameter-declaration and
abstract-declarator). The whole type-name can therefore be parsed as
two nested function-abstract-declarators (recursing through
direct-abstract-declarator): a function that has one parameter of
(nonsensical) type 'code *' that returns a function that has no
parameters.

>From my point of view, it is more important for SDCC to be able to
parse declarations like '_Optional typeof(int (char *))' than to be
able to parse the Keil header; however, I'd be interested to hear
others' opinions of the Keil syntax.

Assuming that the Keil syntax must continue to be supported, what's
the best way to treat ambiguous parameter lists in SDDC.y?
1. Ignore the specific token sequences '(__code *)' and '(code *)', or
2. Ignore any bracketed address-space qualifier, or
3. Ignore a parsed parameter list whose first parameter has qualifiers
but no base type or integer type modifiers.

The current candidate patch attached to
https://sourceforge.net/p/sdcc/bugs/3916/ attempts number 3, but it
feels a bit fragile.

Thanks,
Christopher