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

Empty diagnostics when using Python optimization #485

Closed
treiher opened this issue Mar 26, 2021 · 2 comments
Closed

Empty diagnostics when using Python optimization #485

treiher opened this issue Mar 26, 2021 · 2 comments

Comments

@treiher
Copy link

treiher commented Mar 26, 2021

We have observed in RecordFlux that the diagnostics of our langkit-based parser is empty when python -O is used (AdaCore/RecordFlux#603). The root cause seems to be an assertion in the Python binding (librflxlang/__init__.py):

1120     @property
1121     def diagnostics(self):
1122         """
1123         Diagnostics for this unit.
1124         """
1125         count = _unit_diagnostic_count(self._c_value)
1126         result = []
1127         diag = Diagnostic._c_type()
1128         for i in range(count):
1129             assert _unit_diagnostic(self._c_value, i, ctypes.byref(diag))
1130             result.append(diag._wrap())
1131         return result

The function _unit_diagnostic seems to have side effects and is not executed when -O is used. If line 1129 is commented out, the issue also occurs without -O.

6313 _unit_diagnostic = _import_func(
6314     "rflx_unit_diagnostic",
6315     [AnalysisUnit._c_type, ctypes.c_uint, ctypes.POINTER(Diagnostic._c_type)],
6316     ctypes.c_int,
6317 )

We use -O for performance reasons, otherwise the assertions and contracts used in RecordFlux would make the execution significantly slower.

Versions

GNAT: Community 2020
Langkit: 21.0.0

pmderodat added a commit to pmderodat/langkit that referenced this issue Mar 26, 2021
Python's -O command-line option disableds the execution of "assert"
statements. In order for the generated Python bindings to work in such a
mode, we need to keep assertion logic only in "assert" statements. This
commit fixes statements that currently don't respect this principle.

Fixes GitHub issue AdaCore#485
TN: U326-020
@pmderodat
Copy link
Member

Hello @treiher,

Thank you for reporting this. A fix is pending review on #486.

pmderodat added a commit to pmderodat/langkit that referenced this issue Mar 26, 2021
Python's -O command-line option disableds the execution of "assert"
statements. In order for the generated Python bindings to work in such a
mode, we need to keep assertion logic only in "assert" statements. This
commit fixes statements that currently don't respect this principle.

Fixes GitHub issue AdaCore#485
TN: U326-020
pmderodat added a commit to pmderodat/langkit that referenced this issue Mar 26, 2021
Python's -O command-line option disableds the execution of "assert"
statements. In order for the generated Python bindings to work in such a
mode, we need to keep assertion logic only in "assert" statements. This
commit fixes statements that currently don't respect this principle.

Fixes GitHub issue AdaCore#485
TN: U326-020
pmderodat added a commit that referenced this issue Mar 29, 2021
Python's -O command-line option disableds the execution of "assert"
statements. In order for the generated Python bindings to work in such a
mode, we need to keep assertion logic only in "assert" statements. This
commit fixes statements that currently don't respect this principle.

Fixes GitHub issue #485
TN: U326-020
@pmderodat
Copy link
Member

Hello again,

This is now fixed in the master branch. Thanks again for reporting this bug!

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

No branches or pull requests

2 participants