fix incorrect typing in closure-iterator C code-gen #1355
Conversation
Summary ======= Fix the C code generator emitting incorrectly typed code for closure closure-iterators, which recent stricter C compilers report errors for. Details ======= When computing the environment for closure iterators, and the closure iterator closes over some outer locals, there's already a symbol in the hidden parameter slot, using the environment type of the enclosing routine. The symbol is retyped during closure iterator environment computation, but the *node* referencing it was not updated. `cgen` used the MIR locals type for the definition, but the *node's* type (which was still the enclosing procedure's environment type) for the cast, resulting in a pointer type mismatch. The node referencing the symbol is now also updated, and an assertion to make sure the types match is added in `cgen`.
zerbina
added
bug
No worries, it's somewhat non-obvious. The problem affects all closure closure-iterators (regardless of their complexity), so there's already test coverage for the bug (e.g., in Now, adding a test where the GCC or Clang warning is promoted to an error is possible, but GCC, at least, has the quirk that promoting a warning to an error only works if C compiler warnings are not disabled across the board (which the NimSkull compiler does by default, by passing This is exactly what I did in this PR, but said approach has the significant downside that as soon as the NimSkull compiler starts requiring some mission-critical flags to be passed along to the C compiler, the tests will cease to work, so I'd rather not embrace this approach further. |
|
Oh wow, that's a much bigger problem -- thanks for the detailed explanation. |
|
/merge |
|
Merge requested by: @saem Contents after the first section break of the PR description has been removed and preserved below:
|
Summary
Fix the C code generator emitting incorrectly typed code for closure
closure-iterators, which recent, stricter C compilers report errors
for.
Fixes #1353.
Details
When computing the environment for a closure iterator where the
iterator closes over some outer locals, there's already a symbol in
the hidden parameter slot, using the environment type of the enclosing
routine.
The symbol is retyped during closure iterator environment computation,
but the node referencing it was not updated.
cgenused the MIRlocal's type for the definition, but the node's type (which was still
the enclosing procedure's environment type) for the cast, resulting in
a pointer type mismatch.
The node referencing the symbol is now also updated, and an assertion
to make sure the types match is added in
cgen.