Opened 3 years ago
Closed 3 years ago
#26958 closed defect (fixed)
Inheriting from MPolynomialRing_libsingular crashes sage
Reported by:  etn40ff  Owned by:  

Priority:  major  Milestone:  sage8.7 
Component:  algebra  Keywords:  polynomial rings 
Cc:  Merged in:  
Authors:  Salvatore Stella  Reviewers:  Jeroen Demeyer 
Report Upstream:  N/A  Work issues:  
Branch:  57c8182 (Commits, GitHub, GitLab)  Commit:  57c81821cf0afc9060f2826002a56b9ff3563653 
Dependencies:  Stopgaps: 
Description
The following crashes sage with a SIGSEGV:
sage: from sage.rings.polynomial.multi_polynomial_libsingular import MPolynomialRing_libsingular sage: class Foo(MPolynomialRing_libsingular): ....: pass sage: Foo(QQ, 2, ['x','y'], 'degrevlex')
Strangely enough this bug is triggered only when inheriting; indeed the following works as expected:
sage: from sage.rings.polynomial.multi_polynomial_libsingular import MPolynomialRing_libsingular sage: MPolynomialRing_libsingular(QQ, 2, ['x','y'], 'degrevlex') Multivariate Polynomial Ring in x, y over Rational Field
As it turns out this bug is due to a spurious call to __init_extra__
of Algebras(...).parent_class
. Univariate polynomial rings avoid this call by setting the attribute _no_generic_basering_coercion = True
; now multivariate polynomial rings do so too.
Change History (9)
comment:1 Changed 3 years ago by
 Branch set to u/etn40ff/26958
 Commit set to d628f1459d21ecd34c14366336a7b56fc677b78c
 Status changed from new to needs_review
comment:2 Changed 3 years ago by
 Reviewers set to Jeroen Demeyer
 Status changed from needs_review to positive_review
 Type changed from PLEASE CHANGE to defect
Doesn't hurt I suppose...
comment:3 Changed 3 years ago by
 Status changed from positive_review to needs_work
Although maybe a minor comment about the code: can you replace
cdef public bool _no_generic_basering_coercion
by
cdef readonly _no_generic_basering_coercion
and remove the bool
cimport (which serves no purpose)?
comment:4 Changed 3 years ago by
 Commit changed from d628f1459d21ecd34c14366336a7b56fc677b78c to 57c81821cf0afc9060f2826002a56b9ff3563653
Branch pushed to git repo; I updated commit sha1. New commits:
57c8182  Avoid importing bool

comment:5 Changed 3 years ago by
Done, thanks for the review.
A side note worth noting for #25558: after this patch it is possible to inherit from MPolynomialRing_libsingular
but it is not possible to change its element class because the element constructor uses a function, new_MP
, which hardcodes its output.
comment:6 Changed 3 years ago by
 Status changed from needs_work to needs_review
comment:7 Changed 3 years ago by
 Status changed from needs_review to positive_review
comment:8 Changed 3 years ago by
 Milestone changed from sage8.6 to sage8.7
Retarging tickets optimistically to the next milestone. If you are responsible for this ticket (either its reporter or owner) and don't believe you are likely to complete this ticket before the next release (8.7) please retarget this ticket's milestone to sagepending or sagewishlist.
comment:9 Changed 3 years ago by
 Branch changed from u/etn40ff/26958 to 57c81821cf0afc9060f2826002a56b9ff3563653
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
Fix 26958