#3916 Syntax error if a function type name is the operand of typeof

[Christopher Bazley]

Function type names cannot be used as the operand of the typeof operator.
Clang and LLVM permit it: https://godbolt.org/z/Txo3Ez3K9
I would expect SDCC to permit it too.
If this is not addressed then this method of declaring pointers to optional-qualified function types will be unavailable to SDCC users, e.g. _Optional typeof (int (int)) *opf;

1. Sample code that reproduces the problem:

int foo(int);
typeof (int (int)) foo; // syntax error
typeof (int (int)) *pfoo; // syntax error
typeof (int (*)(int)) pfoo; // OK

1. Exact command used to run SDCC on this sample code:
   sdcc bug.c

2. SDCC version tested:
   SDCC : mcs51/z80/z180/r2k/r2ka/r3ka/r4k/r5k/r6k/sm83/tlcs90/ez80/z80n/r800/ds390/pic16/pic14/TININative/ds400/hc08/s08/stm8/pdk13/pdk14/pdk15/mos6502/mos65c02/f8/f8l TD- 4.5.15 #15966 (Linux)
   published under GNU General Public License (GPL)

Also tested version 4.5.0 at Compiler Explorer.

1. Copy of the error message or incorrect output:
   bug.c:2: error 1: Syntax error, declaration ignored at ')'
   bug.c:2: warning 278: return type of function omitted, assuming int
   Internal error: validateLink failed in SPEC_SCLS(sym->etype) @ SDCCmem.c:420: expected SPECIFIER, got null-link

[Christopher Bazley]

I retested this with a newer build of SDCC and it still reports a syntax error, but the warning about a missing return type appears to have been fixed:

~/optional-ts/examples $ sdcc -c --std=c23 3916.c
3916.c:2: error 1: Syntax error, declaration ignored at ')'
Internal error: validateLink failed in SPEC_NOUN(p) @ SDCCsymt.c:706: expected SPECIFIER, got null-link
~/optional-ts/examples $ sdcc -c --std=c23 --stack-auto 3916.c
3916.c:2: error 1: Syntax error, declaration ignored at ')'
Internal error: validateLink failed in SPEC_NOUN(p) @ SDCCsymt.c:706: expected SPECIFIER, got null-link
~/optional-ts/examples $ sdcc -v
SDCC : mcs51/z80/z180/r2k/r2ka/r3ka/r4k/r5k/r6k/sm83/tlcs90/ez80/z80n/r800/ds390/pic16/pic14/TININative/ds400/hc08/s08/stm8/pdk13/pdk14/pdk15/mos6502/mos65c02/f8/f8l TD- 4.5.24 #16456 (Mac OS X ppc)
published under GNU General Public License (GPL)

(I'm not sure what the validateLink error is about.)

[Christopher Bazley]

I am attaching a proposed combined bugfix for this issue and issue [#3917], created with assistance from Codex in ChatGPT Work. I hope that it will prove acceptable when reviewed by SDCC's maintainers.

During development of the regression tests for this issue, I found what appears to be an unrelated bug, which I have reported as [bugs:#4073]
I suggest that bug [#4073] should be fixed separately from and after [#3916]/[#3917], because the two topics are really unrelated.

I did the following testing of the proposed patch:

• complete diagnostic-validation results (support/valdiag) for the default targets.
• complete runtime regression (support/regression) results for ucz80.

i.e.
make -j2
make -C support/regression clean-results
make -C support/regression -j2 test-ucz80
make -C support/valdiag clean
make -C support/valdiag -j2

Summary for 'ucz80': 0 failures, 36747 tests, 6407 test cases, 7347989 bytes, 1613643851 ticks
Summary for 'mcs51': 0 failures, 653 tests, 364 test cases, 0 bytes, 0 ticks
Summary for 'mcs51-large': 0 failures, 651 tests, 364 test cases, 0 bytes, 0 ticks
Summary for 'mcs51-stack-auto': 0 failures, 652 tests, 364 test cases, 0 bytes, 0 ticks
Summary for 'ds390': 0 failures, 639 tests, 364 test cases, 0 bytes, 0 ticks
Summary for 'z80': 0 failures, 642 tests, 364 test cases, 0 bytes, 0 ticks
Summary for 'z180': 0 failures, 641 tests, 364 test cases, 0 bytes, 0 ticks
Summary for 'r2k': 0 failures, 641 tests, 364 test cases, 0 bytes, 0 ticks
Summary for 'r4k': 0 failures, 641 tests, 364 test cases, 0 bytes, 0 ticks
Summary for 'sm83': 0 failures, 641 tests, 364 test cases, 0 bytes, 0 ticks
Summary for 'tlcs90': 0 failures, 641 tests, 364 test cases, 0 bytes, 0 ticks
Summary for 'hc08': 0 failures, 640 tests, 364 test cases, 0 bytes, 0 ticks
Summary for 's08': 0 failures, 640 tests, 364 test cases, 0 bytes, 0 ticks
Summary for 'mos6502': 0 failures, 640 tests, 364 test cases, 0 bytes, 0 ticks
Summary for 'stm8': 0 failures, 643 tests, 364 test cases, 0 bytes, 0 ticks
Summary for 'f8': 0 failures, 639 tests, 364 test cases, 0 bytes, 0 ticks
Summary for 'pdk13': 0 failures, 642 tests, 364 test cases, 0 bytes, 0 ticks
Summary for 'pdk14': 0 failures, 640 tests, 364 test cases, 0 bytes, 0 ticks
Summary for 'pdk15': 0 failures, 638 tests, 364 test cases, 0 bytes, 0 ticks

[Philipp Klaus Krause]

I think the approach looks good. However, I do see some test failures:

1) support/regression/tests/typeof.c fails for test-pdk14
2) support/regression/tests/bug-716242.c fails for test-s08 and test-mcs51-small

1) is apparently just about reentrancy in new tests: for some SDCC targets using the stack is quite expensive, so by default local variables and parameters are treated as if they were static (standard-compliant reentrancy can be requestes per-function via __reentrant, per-file via --stack-auto, or for part of a translation unit via #pragma stackauto). However, for function pointers, all parameters must be in registers or on the stack, thus there is a per-port limit on the parameters for pointers to non-reentrant functions. We usually just make most tests for function pointers use reentrant functions.

[Christopher Bazley]

Hi Philipp,

I am attaching a revised candidate patch to fix this issue (replaced 09:54 on 8th Oct). It has been rebased on the fix for [#3028], which must be applied first. Valdiag tests pass in 8 configurations.

The changes since the 6 September attachment are:

1. Preserve pointer memory-space syntax. The grammar can also parse a pointer memory-space qualifier through the parameter-list production. The revised action recognises that case instead of constructing a function type. This addresses the bug-716242.c failures you reported (e.g. ((void (__code *) (void)) p) ()).

2. Handle ellipsis-only parameter lists. These have no parameter entries, so a null list must still set FUNC_HASVARARGS. This applies to both bare function types and pointer-to-function types.

3. Make the indirect-call tests reentrant, as you suggested. They use a named function typedef to attach __reentrant, because SDCC does not currently support that attribute in the abstract type-name position. This patch introduces no new attribute syntax.

4. Add runtime checks with differently typed parameters to test their order. The parameter list is not reversed again: parameter_type_list already supplies it in source order.

5. Check an _Optional function pointer before calling it in the runtime test, while still failing the test if it is null.

6. Move the added qualification test out of the Optional TS examples into bug-3916.c, alongside new ellipsis-only tests. The latter use a named pointer typedef in their _Generic associations to avoid depending on the separate spelling mismatch reported in [#4107].

7. Enable the affected TS examples and update their diagnostic expectations. The [#3028] prerequisite supplies function-parameter adjustment; the already merged optional-call diagnostic is now expected rather than ignored. The gold example uses the same named reentrant-type adaptation described above.

[Christopher Bazley]

By the way, I find it hard to understand what the intent really is with this grammar extension because it seems to be accepted inconsistently by SDCC:

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;
}

See https://godbolt.org/z/4zb61YGa7

[Christopher Bazley]

@spth and @maartenbrock, here's a candidate fix that I think is a bit simpler and more robust: it allows the code from the macro definition in [bugs:#294] as a special case, instead of allowing any malformed type name that is parsed as a function returning a function.

[Christopher Bazley]

@maartenbrock Here's an alternative candidate fix that removes support for the code from the macro definition in [bugs:#294] entirely. ("IMO this Keil syntax need not be supported, esp. if it is against the standard. ")

[Maarten Brock]

Maybe leave a comment in bug-716242.c about the removed tests and a pointer to this bug report.

We also discussed this on the developer mailing list:
https://sourceforge.net/p/sdcc/mailman/message/59421674/
Unfortunately the thread is not tracked on SF correctly.

I don't understand SDCC.y enough to be able to assess your patch further.

I am also not adamant about keeping or rejecting the (__code *) exception. Both have their virtues.

[Philipp Klaus Krause]

AFAIR, @roybaer is somewhat familiar with that parser ( he had getting rid of some conflicts on his todo list for a while).

[Christopher Bazley]

Maybe leave a comment in bug-716242.c about the removed tests and a pointer to this bug report.

Done.

I am also not adamant about keeping or rejecting the (__code *) exception. Both have their virtues.

In that case, I'd prefer to reject it, which is what the attached patch does. Documentation is also updated and version number bumped as requested.