Skip to content

[TwigComponent] Fix ComponentTokenParser on 32-bits - #2572

Merged
smnandre merged 1 commit into
symfony:2.xfrom
smnandre:twig/fix-crc32-component-parser
Feb 22, 2025
Merged

[TwigComponent] Fix ComponentTokenParser on 32-bits#2572
smnandre merged 1 commit into
symfony:2.xfrom
smnandre:twig/fix-crc32-component-parser

Conversation

@smnandre

Copy link
Copy Markdown
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

@smnscp smnscp left a comment

Copy link
Copy Markdown

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.

Comment thread src/TwigComponent/src/Twig/ComponentTokenParser.php Outdated
@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
Copy Markdown
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

@smnscp smnscp left a comment

Copy link
Copy Markdown

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
Copy Markdown
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
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