Skip to content

[TwigComponent] Fix ComponentTokenParser on 32-bits #2572

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

Merged
merged 1 commit into from
Feb 22, 2025

Conversation

smnandre
Copy link
Member

Q A
Bug fix? yes
New feature? no
Issues Fix #2568
License MIT
  • 64-bit users get the original behavior
  • 32-bit users get the fix to prevent overflow issues

@carsonbot carsonbot added Bug Bug Fix TwigComponent Status: Needs Review Needs to be reviewed labels Feb 12, 2025
Copy link

@smnscp smnscp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @smnandre, I tested the patch on a 32-bit system. Unfortunately, console clear:cache failed again with:

syntax error, unexpected token "-", expecting "{"

That’s due to a compiled template component class with a - in the class name.
My suggestion solves this problem.

@carsonbot carsonbot added Status: Needs Work Additional work is needed and removed Status: Needs Review Needs to be reviewed labels Feb 13, 2025
@carsonbot carsonbot added Status: Needs Review Needs to be reviewed and removed Status: Needs Work Additional work is needed labels Feb 20, 2025
@smnandre
Copy link
Member Author

Hi @smnandre, I tested the patch on a 32-bit system. Unfortunately, console clear:cache failed again with:

syntax error, unexpected token "-", expecting "{"

That’s due to a compiled template component class with a - in the class name. My suggestion solves this problem.

Thanks a lot! I've commited your suggested code. Tell me if this is ok for you and LGTM :)

@smnandre smnandre requested a review from smnscp February 20, 2025 12:35
Copy link

@smnscp smnscp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@carsonbot carsonbot added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Review Needs to be reviewed labels Feb 20, 2025
@smnandre smnandre requested a review from kbond February 20, 2025 13:46
@carsonbot carsonbot added Status: Needs Review Needs to be reviewed and removed Status: Reviewed Has been reviewed by a maintainer labels Feb 20, 2025
@smnandre
Copy link
Member Author

Note: test failures are related to deprecations in Twig, that will require a dedicated PR

@smnandre smnandre requested a review from Kocal February 20, 2025 13:46
@carsonbot carsonbot added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Review Needs to be reviewed labels Feb 20, 2025
@smnandre smnandre force-pushed the twig/fix-crc32-component-parser branch from 312b699 to 8803614 Compare February 22, 2025 01:06
@smnandre smnandre merged commit 004546b into symfony:2.x Feb 22, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Bug Fix Status: Reviewed Has been reviewed by a maintainer TwigComponent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TwigComponent] ComponentTokenParser failing to generate template index on 32bit platform
4 participants