Skip to content

Skip NULL checks for passed external objects#4406

Merged
casella merged 1 commit intomodelica:masterfrom
beutlich:expec-non-null
Jun 22, 2024
Merged

Skip NULL checks for passed external objects#4406
casella merged 1 commit intomodelica:masterfrom
beutlich:expec-non-null

Conversation

@beutlich
Copy link
Copy Markdown
Member

This is in line with modelica-3rdparty/Modelica_DeviceDrivers#153 and specification change/clarification for modelica/ModelicaSpecification#3500.

I labeled with requires Modelica 3.7, but I am actually not sure.

@beutlich beutlich added L: Resources Issue addresses Modelica/Resources (excl. C-Sources) requires Modelica 3.7 Issue that requires Modelica Language Specification 3.7 labels May 22, 2024
Copy link
Copy Markdown
Contributor

@maltelenz maltelenz left a comment

Choose a reason for hiding this comment

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

Seems fine to me.

I would say Modelica 3.7 is not required.

Copy link
Copy Markdown
Contributor

@HansOlsson HansOlsson left a comment

Choose a reason for hiding this comment

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

Seems ok.
However, an alternative would be to have the assert first and then keep the previous checks - that would be slightly more robust.

I don't know if the NULL pointer checks are that costly.

@beutlich
Copy link
Copy Markdown
Member Author

We have it in M_DD without NULL checks for years. Thus it rather is an issue of code style than technical necessity.

@beutlich beutlich force-pushed the expec-non-null branch 2 times, most recently from 4ebac49 to be51800 Compare June 3, 2024 15:53
@casella casella merged commit 1e12cf7 into modelica:master Jun 22, 2024
@beutlich beutlich deleted the expec-non-null branch July 19, 2024 19:58
@beutlich beutlich removed the request for review from bernhard-thiele July 19, 2024 19:58
@beutlich beutlich added this to the MSL4.2.0 milestone Jul 19, 2024
@Esther-Devakirubai
Copy link
Copy Markdown
Contributor

Esther-Devakirubai commented Jul 24, 2024

@casella @beutlich Should this be backported to maint/4.1.0?

@beutlich
Copy link
Copy Markdown
Member Author

@casella @beutlich Should this be backported to maint/4.1.0?

No, not needed.

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

Labels

L: Resources Issue addresses Modelica/Resources (excl. C-Sources) requires Modelica 3.7 Issue that requires Modelica Language Specification 3.7

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants