v2: fix CleanC self-host C output#12
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01829eb8dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| base_name := typ.name() | ||
| if base_name != '' { | ||
| candidates << base_name | ||
| if base_name.contains('.') { | ||
| candidates << base_name.replace('.', '__') | ||
| } else if g.cur_module != '' && g.cur_module != 'main' && g.cur_module != 'builtin' | ||
| && !base_name.contains('__') { | ||
| candidates << '${g.cur_module}__${base_name}' |
There was a problem hiding this comment.
When generating a cast in a non-main module, this unqualified base_name candidate is checked before the current-module-qualified name, and c_type_is_fn_pointer_alias() also accepts function-pointer aliases recorded from main. If a local non-function alias such as bar.SignalHandler = int is cast inside bar, while main.SignalHandler is a function-pointer alias, the cast target can be misidentified as the short SignalHandler function alias instead of bar__SignalHandler, producing the wrong C type. The existing collision test only covers bar.SignalHandler(...) from main, not the unqualified same-module cast path.
Useful? React with 👍 / 👎.
| ast.FnLiteral {} | ||
| else { | ||
| return false |
There was a problem hiding this comment.
This only handles bare identifiers and function literals, so a comparator passed as a qualified function value such as array__sort_with_compare(mut items, other.compare_items) falls through and is emitted without the new (FnSortCB) cast. That leaves the same incompatible function-pointer type in the generated C for imported/module-qualified comparator helpers that this change fixes for local comparators, so strict C builds can still fail for those inputs.
Useful? React with 👍 / 👎.
| g.sb.write_string('(FnSortCB)') | ||
| g.expr(comparator) |
There was a problem hiding this comment.
This cast is applied solely from the C function name and argument index, before checking the actual parameter type. In skip-builtin/custom-runtime code that defines or calls array__sort_with_compare/array__sorted_with_compare with a non-FnSortCB second parameter, any identifier argument is now emitted as (FnSortCB)arg, producing invalid or wrong C for an otherwise valid call. The cast should be gated by the resolved expected parameter type rather than the helper name alone.
Useful? React with 👍 / 👎.
GGRei
force-pushed
the
v2-cleanc-selfhost-c-output
branch
from
b50d6e5 to
1ff5fb7
Compare
|
Rebased this branch on top of latest master after vlang#27390, and adapted the CleanC changes to the updated upstream structure. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ff5fb7a7f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| c_mod_name := g.resolve_module_name(mod_name).replace('.', '_') | ||
| c_name := '${c_mod_name}__${sel.rhs.name}' | ||
| return c_name in g.fn_return_types || c_name in g.fn_param_is_ptr |
There was a problem hiding this comment.
When the comparator is a module selector that resolves to a function-pointer value rather than a function declaration, e.g. an imported __global/const callback other.callback passed to array__sort_with_compare, this helper returns false because it only recognizes names recorded in fn_return_types/fn_param_is_ptr. Local identifier callback values are now cast, but the module-selector equivalent is emitted without (FnSortCB), so strict C still sees the incompatible callback typedef when code stores the comparator in another module.
Useful? React with 👍 / 👎.
1 participant
Summary
Fixes the remaining V2 CleanC C-output issues that prevented
cmd/v2/test_v2_self.shfrom completing cleanly with GCC and Clang.The changes cover:
gettidemission, avoiding undeclared libc callstrace_callshandling forno_gettidand musl buildsFnSortCBcasts forsort_with_compare/sorted_with_compare, including anonymous and capturing comparatorsValidation
VJOBS=2 ./v test vlib/v2/gen/cleanc: passed, 4 passed / 4 skippedVJOBS=1 bash cmd/v2/test_v2_self.sh: passedVJOBS=1 V2CC=clang bash cmd/v2/test_v2_self.sh: passed