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

build failure with -std=c++17 and pari #45

Closed
isuruf opened this issue Nov 14, 2018 · 12 comments
Closed

build failure with -std=c++17 and pari #45

isuruf opened this issue Nov 14, 2018 · 12 comments

Comments

@isuruf
Copy link
Contributor

isuruf commented Nov 14, 2018

paridecl.h has long rank(GEN x);, but there's also std::rank in C++11.

When

#include <string>
using namespace std;

is done in libsrc/eclib/parifact.h, std::rank is pulled in to the namespace with gcc 7.3. (doesn't happen with gcc 4.9)

This leads to a build failure,

<PREFIX>/include/pari/paridecl.h:1442:19: error: return type specified for deduction guide
 long    rank(GEN x);
                   ^
<PREFIX>/include/pari/paridecl.h:1442:1: error: decl-specifier in declaration of deduction guide
 long    rank(GEN x);
 ^~~~
<PREFIX>/include/pari/paridecl.h:1442:19: error: 'long' or 'short' invalid for '__dguide_rank'
 long    rank(GEN x);
                   ^
<PREFIX>/include/pari/paridecl.h:1442:9: error: deduction guide for 'std::rank< <template-parameter-1-1> >' must have trailing return type
 long    rank(GEN x);

cc @saraedum

@saraedum
Copy link

Building the master branch here from source, I could not reproduce this:
make -j4 CXXFLAGS="-std=c++11" worked for me with GCC 8.2.1.

@saraedum
Copy link

Setting -std=c++17 gives me the original error.

@isuruf
Copy link
Contributor Author

isuruf commented Nov 14, 2018

This error might go away if we stopped doing using namespace std, but that's a lot of code to change.

@saraedum
Copy link

I'm not a C++ expert but I think this is the way this type of problem is handled usually. Of course, we could also force an older C++ standard but I guess that's not such a great idea in the long run.

@isuruf
Copy link
Contributor Author

isuruf commented Nov 14, 2018

It might not be enough. Ran into this in lcalc as well.

@isuruf
Copy link
Contributor Author

isuruf commented Nov 14, 2018

It is enough.

Following produces an error

#include <string.h>
using namespace std;

#include <pari/pari.h>
int main() {

}

Following doesn't

#include <string.h>

#include <pari/pari.h>
int main() {

}

@JohnCremona
Copy link
Owner

I have never wanted to not have a global std namespace sine it seemed ridiculous to have to write std::cout instead of cout for every output line -- and many more. I also do not relish the prospect of changing almost every line of code for such a trivial reason.

Instead: since almost no symbols from libpari are used, would it be possible to wrap the header file which include pari.h into a different namespace called pari? Then I would not mind inserting pari:: in a few places.

@isuruf
Copy link
Contributor Author

isuruf commented Jan 9, 2019

@JohnCremona, how about doing using std::cout; and whatever eclib need in a central place? Then you can use cout in other places. This prevents std::rank getting pulled in as rank.

@isuruf
Copy link
Contributor Author

isuruf commented Jan 9, 2019

Instead: since almost no symbols from libpari are used, would it be possible to wrap the header file which include pari.h into a different namespace called pari?

This is not possible since pari is including standard headers and they'll be pulled in to the pari namespace and cause issues.

@JohnCremona
Copy link
Owner

I spent hours, perhaps days, on reorganising the header files earlier this year. I am not going to spend more time on that now, as I have more important things to do.

@isuruf
Copy link
Contributor Author

isuruf commented Jan 9, 2019

Here's a patch that fixes this. Do you think it's reasonable? (If so, I can send it as a PR.)

diff --git a/libsrc/bitspace.cc b/libsrc/bitspace.cc
index 377f03c..8620145 100644
--- a/libsrc/bitspace.cc
+++ b/libsrc/bitspace.cc
@@ -25,7 +25,7 @@
 #include <eclib/interface.h>
 #include <eclib/bitspace.h>
 
-using namespace std;
+
 
 bitspace::bitspace(long d)
 {
diff --git a/libsrc/eclib/curvesort.h b/libsrc/eclib/curvesort.h
index d32a408..ee710e2 100644
--- a/libsrc/eclib/curvesort.h
+++ b/libsrc/eclib/curvesort.h
@@ -26,7 +26,8 @@
                            //flags that this file has been included
 
 #include <string>
-using namespace std;
+#include <eclib/templates.h>
+
 
 #define USE_NEW_CODE_LETTERS
 
diff --git a/libsrc/eclib/interface.h b/libsrc/eclib/interface.h
index 7b3cac6..0facfc3 100644
--- a/libsrc/eclib/interface.h
+++ b/libsrc/eclib/interface.h
@@ -47,7 +47,7 @@
 #include <ext/numeric>
 #endif
 #include <iterator>
-using namespace std;
+
 #include <eclib/templates.h>
 #include <stdint.h>
 
diff --git a/libsrc/eclib/options.h b/libsrc/eclib/options.h
index b35998e..52457de 100644
--- a/libsrc/eclib/options.h
+++ b/libsrc/eclib/options.h
@@ -27,7 +27,7 @@
 
 #include <eclib/GetOpt.h>
 #include <iostream>
-using namespace std;
+
 
 #define DEFAULT_QUIET 0
 #define DEFAULT_VERBOSE 1
diff --git a/libsrc/eclib/parifact.h b/libsrc/eclib/parifact.h
index d577546..e026125 100644
--- a/libsrc/eclib/parifact.h
+++ b/libsrc/eclib/parifact.h
@@ -26,7 +26,7 @@
                            //flags that this file has been included
 
 #include <string>
-using namespace std;
+#include <eclib/templates.h>
 
 string factor(const string n);
 int is_prime(const string p);
diff --git a/libsrc/eclib/templates.h b/libsrc/eclib/templates.h
index 81fba60..89d2da5 100644
--- a/libsrc/eclib/templates.h
+++ b/libsrc/eclib/templates.h
@@ -29,7 +29,45 @@
 #include <set>
 #include <vector>
 #include <iterator>
-using namespace std;
+#include <complex>
+#include <limits>
+#include <map>
+#include <unordered_map>
+#include <algorithm>
+#include <iomanip>
+
+using std::string;
+using std::vector;
+using std::cout;
+using std::cerr;
+using std::cin;
+using std::endl;
+using std::flush;
+using std::ios;
+using std::ostream;
+using std::istream;
+using std::ifstream;
+using std::stringstream;
+using std::istringstream;
+using std::ostringstream;
+using std::ofstream;
+using std::ios_base;
+using std::numeric_limits;
+using std::ostream_iterator;
+using std::complex;
+using std::ws;
+using std::skipws;
+using std::map;
+using std::unordered_map;
+using std::min;
+using std::max;
+using std::ptr_fun;
+using std::pair;
+using std::sort;
+using std::abs;
+using std::binary_function;
+using std::setw;
+
 
 template <class T >
 inline ostream& operator<<(ostream& os, const vector<T>& v)
diff --git a/libsrc/kbessel.cc b/libsrc/kbessel.cc
index add08c6..01cebf0 100644
--- a/libsrc/kbessel.cc
+++ b/libsrc/kbessel.cc
@@ -24,7 +24,8 @@
 
 #include <iostream>  // for debugging only
 #include <cmath>
-using namespace std;
+
+#include <eclib/templates.h>
 #include <eclib/kbessel.h>
 #ifndef M_LN2
 #define M_LN2   0.69314718055994530942
diff --git a/libsrc/parifact.cc b/libsrc/parifact.cc
index 85b6905..4cfffc4 100644
--- a/libsrc/parifact.cc
+++ b/libsrc/parifact.cc
@@ -29,7 +29,7 @@
 //#define DEBUG_GPFACT
 
 #include <iostream>
-using namespace std;
+
 
 string
 factor(const string n)
diff --git a/libsrc/version.cc b/libsrc/version.cc
index 71d369d..20035b6 100644
--- a/libsrc/version.cc
+++ b/libsrc/version.cc
@@ -22,8 +22,8 @@
 //////////////////////////////////////////////////////////////////////////
  
 #include <iostream>
+#include <eclib/templates.h>
 
-using namespace std;
 
 void show_version()
 {

@JohnCremona
Copy link
Owner

Thank you -- that looks very much simpler than I thought it would be. I had also thought that the line "using namespace std;" only appeared in one place in a file (interface.h) which was included by just about every other file, but that turns out to be far from the truth.

Please make a PR and I will test.

@isuruf isuruf changed the title build failure with gcc 7.3 and pari build failure with -std=c++17 and pari Jan 9, 2019
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

3 participants