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

renderer: expression_t is always allocated as part of structs, ext->numOps always 0 if ext is invalid #1107

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

illwieckz
Copy link
Member

@illwieckz illwieckz commented Apr 29, 2024

As far as I know,

  • expression_t is always allocated as part of structs (texModInfo_t, shaderStage_t),
  • ext->numOps is always 0 if ext is invalid.

@illwieckz illwieckz force-pushed the illwieckz/simplify-expression branch 2 times, most recently from c906c30 to 40bf242 Compare April 29, 2024 21:25
@illwieckz illwieckz marked this pull request as draft April 29, 2024 21:35
@illwieckz illwieckz force-pushed the illwieckz/simplify-expression branch 3 times, most recently from efe082c to 0a88dba Compare April 29, 2024 23:06

// A ext->numOps equals to 0 means empty or invalid expression.
Copy link
Member

Choose a reason for hiding this comment

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

Suggesting there is such a thing as a valid empty expression, but I don't suppose that's the case?

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe an empty expression is invalid, right.

Copy link
Member

Choose a reason for hiding this comment

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

I took a closer look and in fact there are "valid" empty expressions, because the parser is buggy, e.g. ((())). Apparently they evaluate to 0. It's not something that should have been accepted though.

@illwieckz illwieckz force-pushed the illwieckz/simplify-expression branch from c125b4d to 64587e1 Compare May 1, 2024 00:41
@illwieckz illwieckz marked this pull request as ready for review May 4, 2024 01:40
@slipher
Copy link
Member

slipher commented May 4, 2024

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants