Opened 10 years ago
Closed 5 years ago
#12439 closed defect (fixed)
symmetrica fails to compile with clang
Reported by:  ohanar  Owned by:  GeorgSWeber 

Priority:  major  Milestone:  sage7.4 
Component:  packages: standard  Keywords:  
Cc:  tscrim, fbissey  Merged in:  
Authors:  Travis Scrimshaw  Reviewers:  François Bissey 
Report Upstream:  N/A  Work issues:  
Branch:  41f77fb (Commits, GitHub, GitLab)  Commit:  41f77fbdc6970abf6685cb7a32b55298b2bbc5d9 
Dependencies:  Stopgaps: 
Description (last modified by )
There is one function with empty return statements when an int should be returned. Clang doesn't like this.
Attachments (1)
Change History (22)
comment:1 Changed 10 years ago by
 Status changed from new to needs_review
comment:2 Changed 9 years ago by
comment:3 Changed 9 years ago by
 Component changed from build to packages
Changed 9 years ago by
comment:4 Changed 9 years ago by
 Description modified (diff)
comment:5 Changed 9 years ago by
Changed Wreturntype
to Wnoreturntype
:) It now builds with Clang 3.2.
comment:6 Changed 9 years ago by
 Description modified (diff)
comment:7 Changed 9 years ago by
 Milestone changed from sage5.10 to sage5.11
Andrew, do you feel like reviewing this?
comment:8 Changed 8 years ago by
 Milestone changed from sage5.11 to sage5.12
comment:9 Changed 8 years ago by
 Status changed from needs_review to needs_work
This needs to rebased to the latest Symmetrica spkg. Since nobody seems to care, I'm not going to do that.
comment:10 Changed 8 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:11 Changed 8 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:12 Changed 7 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:13 Changed 5 years ago by
 Cc tscrim added
 Milestone changed from sage6.4 to sage7.4
comment:14 Changed 5 years ago by
 Branch set to public/packages/symmetrica_clang12439
 Cc fbissey added
 Commit set to 2133a16b4ee8c8d363f3cc44caaf33086b9b4ae8
 Status changed from needs_work to needs_review
Give this a try. The function that was marked with an int
return type really should have been a void
. I've added the compiler flag to the branch as well.
New commits:
2133a16  Adding patch for function with wrong return type.

comment:15 Changed 5 years ago by
 Status changed from needs_review to needs_work
I am afraid I cannot accept that. You are mistaken in thinking the function should have been void
static int rec01(INT ni, OP vec) /* to compute number of partitions */ { INT erg = OK; if (ni<0) return; if (not EMPTYP(S_V_I(vec,ni))) return; else if (ni<=1) M_I_I(1,S_V_I(vec,ni)); else { INT m,og; og = ni/3+3; m_i_i(0,S_V_I(vec,ni)); for (m=1;m<og;m++) { INT j; j = nim*(3*m1)/2; if (j<0) break; if (m%2==0) { ADDINVERS_APPLY(S_V_I(vec,j)); ADD_APPLY(S_V_I(vec,j),S_V_I(vec,ni)); ADDINVERS_APPLY(S_V_I(vec,j)); } else ADD_APPLY(S_V_I(vec,j),S_V_I(vec,ni)); j = nim*(3*m+1)/2; if (j<0) break; if (m%2==0) { ADDINVERS_APPLY(S_V_I(vec,j)); ADD_APPLY(S_V_I(vec,j),S_V_I(vec,ni)); ADDINVERS_APPLY(S_V_I(vec,j)); } else ADD_APPLY(S_V_I(vec,j),S_V_I(vec,ni)); } } ENDR("internal:rec01"); }
The error here is in thinking that there is only a couple of instances of return;
in the function and that you reach the end of it without a return
. But ENDR
is a macro defined in macro.h
#define ENDR(a) endr_ende: if (erg != OK) EDC(a); return erg;
So you have a final return
of an integer value. It is just hidden. By default gcc
converts the return;
statements to return 0;
in that case and that's what we should do.
And we don't want to add that flag to spkginstall
.
comment:16 Changed 5 years ago by
 Commit changed from 2133a16b4ee8c8d363f3cc44caaf33086b9b4ae8 to 41f77fbdc6970abf6685cb7a32b55298b2bbc5d9
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
41f77fb  Added patch for return values of symmetrica.

comment:17 Changed 5 years ago by
 Status changed from needs_work to needs_review
My fault, I should have read deeper into the code. Fixed.
comment:18 Changed 5 years ago by
 Description modified (diff)
All forgiven :) I am doing a new run of compiling with clang (3.8.0 I have upgraded to sierra and xcode 8.0) on my mac. I'll put that in, that should be the proverbial proof of the pudding.
comment:19 Changed 5 years ago by
 Reviewers set to François Bissey
 Status changed from needs_review to positive_review
Good to go.
comment:20 Changed 5 years ago by
Thank you.
comment:21 Changed 5 years ago by
 Branch changed from public/packages/symmetrica_clang12439 to 41f77fbdc6970abf6685cb7a32b55298b2bbc5d9
 Resolution set to fixed
 Status changed from positive_review to closed
Please fill in your real name as Author.