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.

Sign up for GitHub

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

Jump to bottom

Remove dead code from gen.pyx #13377

Closed
nbruin opened this issue Aug 17, 2012 · 9 comments
Closed

Remove dead code from gen.pyx #13377

nbruin opened this issue Aug 17, 2012 · 9 comments
Assignees
@williamstein
Labels
c: interfaces good first issue p: minor / 4 t: bug
Milestone
sage-5.12

Comments

@nbruin
Copy link
Contributor

nbruin commented Aug 17, 2012

I get two complaints about unreachable code in sage/libs/pari/gen.pyx for toGEN (line 9197 after #12215) and _coerce_c_impl (line 9670 after #12215) and the compiler is correct if I read the code correctly. The edits causing this both are from the pre-trac ticket era (2007 and 2006 respectively)

Component: interfaces

Keywords: easy, sd51

Author: Alex Ghitza

Reviewer: Frédéric Chapoton

Merged: sage-5.12.beta0

Issue created by migration from https://trac.sagemath.org/ticket/13377
@nbruin nbruin added this to the sage-5.11 milestone Aug 17, 2012
@aghitza
Copy link
Contributor

aghitza commented Jul 23, 2013

Attachment: trac_13377-dead_code_gen_pyx.patch.gz

@aghitza
Copy link
Contributor

aghitza commented Jul 23, 2013

Author: Alex Ghitza

@aghitza
Copy link
Contributor

aghitza commented Jul 23, 2013

Changed keywords from easy to easy, sd51

@aghitza
Copy link
Contributor

aghitza commented Jul 23, 2013

comment:1

Removed the code in question.

@JohnCremona
Copy link
Member

comment:2

The description mentions two blocks to be removed but the patch only removes one. Why?

@aghitza
Copy link
Contributor

aghitza commented Jul 25, 2013

comment:3

Ah, forgot to mention that the other block has already disappeared. You can see this by (a) looking at what happens when you apply the patch and rebuild sage--there are no other warnings about unreachable code; and/or (b) reading the very short function _coerce_c_impl and seeing that there are no issues.

@fchapoton
Copy link
Contributor

Reviewer: Frédéric Chapoton

@fchapoton
Copy link
Contributor

comment:4

looks good to me, positive review

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Jul 25, 2013
@jdemeyer
Copy link
Contributor

jdemeyer commented Aug 2, 2013

Merged: sage-5.12.beta0

@jdemeyer jdemeyer closed this as completed Aug 2, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@williamstein williamstein

Labels
c: interfaces good first issue p: minor / 4 t: bug
Projects
None yet
Milestone
sage-5.12
Development

No branches or pull requests
6 participants
@aghitza @williamstein @nbruin @JohnCremona @jdemeyer @fchapoton