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

Fix a few cython "referenced before assignment" warnings #33176

Closed
orlitzky opened this issue Jan 15, 2022 · 11 comments
Closed

Fix a few cython "referenced before assignment" warnings #33176

orlitzky opened this issue Jan 15, 2022 · 11 comments

Comments

@orlitzky
Copy link
Contributor

Our build displays several warnings like

warning: sage/libs/ntl/ntl_ZZ.pyx:274:23: local variable 'ans' referenced before assignment

for variables that are passed by reference to a C function that overwrites them (meaning that there is no real bug). Here we fix a few easy ones by initializing the variable.

Component: build

Author: Michael Orlitzky

Branch/Commit: 93f2e90

Reviewer: Markus Wageringel, Marc Mezzarobba

Issue created by migration from https://trac.sagemath.org/ticket/33176

@orlitzky orlitzky added this to the sage-9.5 milestone Jan 15, 2022
@orlitzky
Copy link
Contributor Author

Author: Michael Orlitzky

@orlitzky
Copy link
Contributor Author

Commit: d33a9e7

@orlitzky
Copy link
Contributor Author

Branch: u/mjo/ticket/33176

@orlitzky
Copy link
Contributor Author

New commits:

48c2b15Trac #33176: fix Cython warnings in sage.libs.ntl.
38444daTrac #33176: update API usage in sage.libs.gap.element.
12bfe01Trac #33176: fix a Cython warning in sage.libs.singular.groebner_strategy.
d33a9e7Trac #33176: fix Cython warning in sage.matrix.matrix_modn_dense_template.

@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Jan 28, 2022
@mwageringel
Copy link

comment:3
        cdef long mini, minval = 0

Did you intend to set mini to 0 as well, here? There is still a warning about it

[sagelib-9.6.beta2] warning: sage/libs/ntl/ntl_ZZ_pX.pyx:1175:44: local variable 'mini' referenced before assignment

Otherwise, the changes look good to me.

@mwageringel
Copy link

comment:4

Related discussion in #33201.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 23, 2022

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

92acd43Trac #33176: fix Cython warnings in sage.libs.ntl.
5594ba8Trac #33176: update API usage in sage.libs.gap.element.
01176dfTrac #33176: fix a Cython warning in sage.libs.singular.groebner_strategy.
93f2e90Trac #33176: fix Cython warning in sage.matrix.matrix_modn_dense_template.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 23, 2022

Changed commit from d33a9e7 to 93f2e90

@orlitzky
Copy link
Contributor Author

comment:6

Replying to @mwageringel:

        cdef long mini, minval = 0

Did you intend to set mini to 0 as well, here? There is still a warning about it

I think so. The first time it's used (if it's used) is when it's passed by reference to,

Z_pX_min_val_coeff(minval, mini, self.x, pZZ.x)

According to ntlwrap_impl.h, that function overwrites both minval and mini. I've updated that one commit and rebased it onto 9.6-beta2.

@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 Apr 17, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Sep 19, 2022
@mezzarobba
Copy link
Member

Reviewer: Markus Wageringel, Marc Mezzarobba

@vbraun
Copy link
Member

vbraun commented Nov 7, 2022

Changed branch from u/mjo/ticket/33176 to 93f2e90

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

No branches or pull requests

5 participants