Skip to content

Conversation

odain-cbd
Copy link

For iTop project we had to enhance ConstExprEvaluator to parse/evaluate few of our PHP files. Below are listed the added/enhanced Expression usecases.
(https://github.com/Combodo/iTop/tree/feature/4789-nikicfork)

Added:
- Variable
- Isset
- ClassConstFetch
- Cast
- StaticPropertyFetch
- FuncCall
- StaticCall
- NullsafePropertyFetch
- PropertyFetch
- NullsafeMethodCall
- MethodCall

Enhanced:
- ConstFetch
- Coalesce

…om/Combodo/iTop

Added:
	- Variable
	- Isset
	- ClassConstFetch
	- Cast
	- StaticPropertyFetch
	- FuncCall
	- StaticCall
	- NullsafePropertyFetch
	- PropertyFetch
	- NullsafeMethodCall
	- MethodCall

Enhanced:
	- ConstFetch
	- Coalesce
@odain-cbd odain-cbd changed the title Enhance ConstExprEvaluator by adding/enhancing few types Enhance ConstExprEvaluator by evaluating new type of expressions or enhancing some (hopefully) Sep 5, 2025
Copy link
Owner

@nikic nikic left a comment

Choose a reason for hiding this comment

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

The purpose of ConstExprEvaluator is to evaluate "constant expressions", such as the right hand side of const X = .... These constant expressions only support a limited set of operations.

What you seem to be implementing here seems to be a lot broader: It's trying to evaluate all expressions, and accessing global state (like global variables) for that purpose. While this is may be useful for some use cases, it does not fit the purpose of ConstExprEvaluator and this behavior would be highly undesirable in most circumstances (static analyzers do not typically run in the same execution context as the code they're analyzing, so this just ends up accessing global state from the analyzer).

@odain-cbd
Copy link
Author

Thanks for the feedback.

Keeping the purpose of ConstExprEvaluator, I left only below enhancements in this class:
ConstFetch/ ClassConstFetch (added)/ Cast (added)

All non-constant evaluations have been moved in ExprEvaluator class. with associated tests as well.

@odain-cbd odain-cbd requested a review from nikic September 18, 2025 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants