Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unrolling of big collection constructors fails to compile for const variables #1760

Closed
bbannier opened this issue Jun 12, 2024 · 0 comments · Fixed by #1761
Closed

Unrolling of big collection constructors fails to compile for const variables #1760

bbannier opened this issue Jun 12, 2024 · 0 comments · Fixed by #1761
Assignees
Labels
Bug Something isn't working Codegen

Comments

@bbannier
Copy link
Member

bbannier commented Jun 12, 2024

With #1744 we introduced code which unrolls constructors of huge containers. The code we now generate is incorrect for const variables which do not live in a scope (it does work for globals though which are managed more thightly by the module).

module foo;

# Ctrs for collections of more than 10 elements get unrolled.
const CRC16_TABLE: vector<uint16> = [
  0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10
];
$ spicyc -dj foo.spicy
foo_198ff91772b68fab-628dbe1417976318.cc:12:84: error: non-local lambda expression cannot have a capture-default
    const ::hilti::rt::Vector<::hilti::rt::integer::safe<uint16_t>> CRC16_TABLE = [&]() { ...
                                                                                   ^

We probably need to adjust or even drop the capture for declarations emitted outside of a proper C++ scope. We still need to make sure that a constructor can reference other const variables though. A declaration of a const can only refer to other const variables; since const variables are emitted as non-local variables we do not need to capture them.

@bbannier bbannier added Bug Something isn't working Codegen labels Jun 12, 2024
@bbannier bbannier self-assigned this Jun 12, 2024
bbannier added a commit that referenced this issue Jun 12, 2024
When unrolling constructors of collections we previously would always
capture explicitly so that container elements could reference other
variables in the scope. This breaks for `const` variables which are
emitted at non-local scope (into a namespace); the lambda performing the
initialization would also be a non-local and non-local lambdas are not
allowed to capture anything (they do not live in a scope after all).

This means that we cannot capture for such lambdas at all which seems to
be an issue -- after all with that it seems impossible to support
constructors of collections which reference other `const`s. Fortunately
C++ lambdas at non-local scope implicitly capture other non-locals
already and they do not need to be captured explictly. This allows for a
rather simple fix.

Closes #1760.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Codegen
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant