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

Fixed issue with type deduced expressions breaking ternary operators #64

Merged
merged 4 commits into from
Sep 18, 2023

Conversation

thorulf4
Copy link
Contributor

No description provided.

mikucionisaau and others added 2 commits September 18, 2023 09:51
first, when computing a common type and second when assigning.
if (!areInlineIfCompatible(expr.get_type(), expr[1].get_type(), expr[2].get_type())) {

type = getInlineIfCommonType(expr[1].get_type(), expr[2].get_type());
if (!areInlineIfCompatible(type, expr[1].get_type(), expr[2].get_type())) {
Copy link
Member

Choose a reason for hiding this comment

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

It should be possible to convert the common type to the resulting type

Suggested change
if (!areInlineIfCompatible(type, expr[1].get_type(), expr[2].get_type())) {
if (!areAssignmentCompatible(resultType, type)) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what resultType is supposed to be. But areInlineIfCompatible call does areAssignmentCompatible checks

Copy link
Member

Choose a reason for hiding this comment

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

given the expression res = (c) ? (a) : (b) then the resultType is the type of res.

@thorulf4
Copy link
Contributor Author

I've run regression and uppaal tests, both succeed

@thorulf4 thorulf4 merged commit 2ce8a89 into UPPAALModelChecker:main Sep 18, 2023
5 checks passed
@thorulf4 thorulf4 deleted the ternary_operator_fix branch September 18, 2023 11:10
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.

2 participants