SIGN IN SIGN UP

fix(extract): a C-family function's return type keeps its pointer and qualifiers

Distilled from #1245 by Andrew Hundt (c51e5cb0, the extract_defs.c
half; the mcp.c and token-reduction parts are not taken).

For C-family functions return_type was the text of the declaration's
`type` node alone. In these grammars the `*`, `&` and `&&` live on the
declarator and cv-qualifiers are sibling nodes, so

    const char *get_name(void)     published   char
    robj **lookupKeys(...)         published   robj
    Text &operator=(const Text &)  published   Text

and no C or C++ function in any index had a pointer return type at all.
The C/C++ resolver's return-type parser already strips a leading
const/volatile and handles trailing `*`, `&`, `&&` -- branches that were
unreachable for functions, so every `Foo *f()` was registered as
returning Foo by value.

c_declared_return_type() renders the declared type at both sites
(functions and methods) for C, C++, CUDA, GLSL, HLSL, ISPC, Slang and
Objective-C -- the list resolve_func_name_c_family uses:

    <leading cv-qualifiers, source order> <base type, verbatim>
    <one space><markers, outermost first>

so `const char *`, `char **`, `Text &`, `Text &&`, `Text *&`,
`volatile unsigned long *`; a pointer-level qualifier follows its star
(`char *const *`); east-const `char const *` normalises to
`const char *`. With no qualifier and no marker the old text is returned
unchanged. It measures, then writes into one exact-size arena
allocation; the declarator walk is a plain child chain, no recursion.
def.return_types is untouched.

Two deliberate differences from the upstream hunk. Upstream copied every
type_qualifier node, which in these grammars includes constexpr,
_Noreturn, mutable and __extension__ -- `constexpr int f()` would have
become `constexpr int`. Only const, volatile, restrict, __restrict,
__restrict__ and _Atomic are kept, and both cases are asserted unchanged.
And the walk stops at the first declarator that is neither pointer nor
reference, so `int (*f(void))(int)` stays `int` rather than becoming a
wrong `int *`.

RED, production reverted and the final tests kept:
  c_function_return_type_preserves_pointer_and_qualifier
      FAIL tests/test_extraction.c:1005: "char" != "const char *"
  cpp_method_return_type_preserves_pointer_and_qualifier
      FAIL tests/test_extraction.c:1072: "char" != "const char *"
  351 passed, 2 failed
c_function_return_type_plain_unchanged is the control: it passes before
and after.

GREEN: extraction 353, c_lsp 762, lang_contract 41, pipeline 281.

Effect on real code -- Redis, production binaries, old against new.
39,132 nodes in both. 1,442 Function nodes change, in return_type only:
void -> void * (270), char -> const char * (101), unsigned char ->
unsigned char * (91), char -> char * (81), robj -> robj * (57), ...
Functions with a `*` in return_type: 0 -> 1,440. With one worker the
complete edge set apart from SEMANTICALLY_RELATED is byte-identical old
against new across six runs (three each) -- CALLS, USAGE, WRITES, IMPORTS
and the rest do not move. SEMANTICALLY_RELATED goes 350 -> 325 because
that pass tokenises the return-type string into its type vector.

One thing measured rather than assumed. With several workers, which
`dict.h` Redis' src/dict.c imports (src/ or deps/hiredis/) depends on
worker merge order. That is main's defect, not this change's: unmodified
main picked src/dict.h in 2 of 10 runs and the hiredis header in 8; with
one worker both binaries always pick src/dict.h. This change picked
src/dict.h in 6 of 10. At ten runs a side that gap is not separable from
chance (Fisher exact, two-sided p = 0.17), and a shifted rate would be
unsurprising for a timing-dependent pick; it is recorded here rather
than explained. The include-target fix is a separate change (#2227).

Known limits, all pre-existing and out of scope: struct and class FIELDS
have the same defect (`const char *data;` publishes char); C prototypes
and declaration-only C++ methods are not extracted as definitions, so
they are not reached; where a macro is misparsed as the type
(`LUA_API const char *f()`), the base was already wrong and now reads
`const LUA_API *`; the resolver's parser does not yet understand a
pointer-level qualifier or a leading _Atomic/restrict and falls back to
a named-type lookup for those, as it effectively did for every pointer
return before.

Co-authored-by: Andrew Hundt <ATHundt@gmail.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
M
Martin Vogel committed
343d7dd5e47b817999098605370f0eab706675f0
Parent: bf4c476