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 compilation error on clang #219

Merged
merged 10 commits into from
Nov 24, 2020

Conversation

eiichiroi
Copy link
Contributor

This pull request fix compilation errors on clang 10.0.0 / macOS High Sierra.

  • fix to include your own header
  • fix to call std::basic_iostream's constructor correctly
  • resolve conflictions between std library and pficommon in test codes

@eiichiroi eiichiroi changed the base branch from master to v4.0 November 10, 2020 06:42
@ysk24ok ysk24ok self-requested a review November 10, 2020 06:56
@eiichiroi
Copy link
Contributor Author

note:
This PR doesn't solve the gcc extension problem.
clang++(with libc++) will output errors on pficommon/system/file.h and pficommon/system/file.cpp.

@eiichiroi eiichiroi force-pushed the fix-compilation-error-on-clang branch from deca998 to d19dc45 Compare November 10, 2020 07:27
@eiichiroi
Copy link
Contributor Author

ad-hoc temporary patch for testing in clang / macOS / libc++

diff --git a/src/system/file.cpp b/src/system/file.cpp
index 397cb46..93f2989 100644
--- a/src/system/file.cpp
+++ b/src/system/file.cpp
@@ -47,7 +47,7 @@ iostream *tmpstream(string &tmpl)
   if (tmpl.length()<6) return NULL;
   int fd=mkstemp(&tmpl[0]);
   if (fd<0) return NULL;
-  return new fd_stream(fd);
+  return NULL;
 }

 ssize_t get_file_size(const string & fn)
diff --git a/src/system/file.h b/src/system/file.h
index 24cec47..3ccdf9c 100644
--- a/src/system/file.h
+++ b/src/system/file.h
@@ -34,12 +34,13 @@

 #include <sys/types.h>
 #include <iostream>
-#include <ext/stdio_filebuf.h>
+//#include <ext/stdio_filebuf.h>

 namespace pfi{
 namespace system{
 namespace file{

+/*
 class fd_stream : public std::iostream{
 public:
   explicit fd_stream(int fd)
@@ -49,6 +50,7 @@ public:
 private:
   __gnu_cxx::stdio_filebuf<char> fb;
 };
+*/

 std::iostream *tmpstream(std::string &tmpl);
 ssize_t get_file_size(const std::string & fn);

@ysk24ok
Copy link
Contributor

ysk24ok commented Nov 11, 2020

I've tried build with Clang 11.0.0 on my Macbook Catalina and somehow time_util_test fails.

$ ./waf --checkone time_util_test
Waf: Entering directory `/Users/yusuke-nishioka/repos/pficommon/build'
[47/63] Compiling src/system/time_util_test.cpp
[52/63] Compiling pficommon.pc.in
[53/63] Linking build/src/system/time_util_test
[63/63] Processing build/src/system/time_util_test
Waf: Leaving directory `/Users/yusuke-nishioka/repos/pficommon/build'
test summary
  tests that pass 0/1
  tests that fail 1/1
    /Users/yusuke-nishioka/repos/pficommon/build/src/system/time_util_test
Running main() from ../.unittest-gtest/gtest-1.8.1/fused-src/gtest/gtest_main.cc
[==========] Running 20 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 20 tests from time_util_instance/time_util
[ RUN      ] time_util_instance/time_util.causality/0
1605135597 : 480548
[       OK ] time_util_instance/time_util.causality/0 (0 ms)
[ RUN      ] time_util_instance/time_util.causality/1
1605135597 : 480563
[       OK ] time_util_instance/time_util.causality/1 (0 ms)
[ RUN      ] time_util_instance/time_util.causality/2
1605135597 : 480570
[       OK ] time_util_instance/time_util.causality/2 (0 ms)
[ RUN      ] time_util_instance/time_util.causality/3
1605135597 : 480576
[       OK ] time_util_instance/time_util.causality/3 (0 ms)
[ RUN      ] time_util_instance/time_util.causality/4
1605135597 : 480583
[       OK ] time_util_instance/time_util.causality/4 (0 ms)
[ RUN      ] time_util_instance/time_util.metric/0
[       OK ] time_util_instance/time_util.metric/0 (0 ms)
[ RUN      ] time_util_instance/time_util.metric/1
[       OK ] time_util_instance/time_util.metric/1 (0 ms)
[ RUN      ] time_util_instance/time_util.metric/2
[       OK ] time_util_instance/time_util.metric/2 (0 ms)
[ RUN      ] time_util_instance/time_util.metric/3
[       OK ] time_util_instance/time_util.metric/3 (0 ms)
[ RUN      ] time_util_instance/time_util.metric/4
[       OK ] time_util_instance/time_util.metric/4 (0 ms)
[ RUN      ] time_util_instance/time_util.calendar/0
../src/system/time_util_test.cpp:97: Failure
Expected equality of these values:
  197
  ct.yday
    Which is: 196
../src/system/time_util_test.cpp:98: Failure
Expected equality of these values:
  5
  ct.wday
    Which is: 4
[  FAILED  ] time_util_instance/time_util.calendar/0, where GetParam() = "Asia/Tokyo" (1 ms)
[ RUN      ] time_util_instance/time_util.calendar/1
../src/system/time_util_test.cpp:97: Failure
Expected equality of these values:
  197
  ct.yday
    Which is: 196
../src/system/time_util_test.cpp:98: Failure
Expected equality of these values:
  5
  ct.wday
    Which is: 4
[  FAILED  ] time_util_instance/time_util.calendar/1, where GetParam() = "US/Hawaii" (1 ms)
[ RUN      ] time_util_instance/time_util.calendar/2
[       OK ] time_util_instance/time_util.calendar/2 (0 ms)
[ RUN      ] time_util_instance/time_util.calendar/3
[       OK ] time_util_instance/time_util.calendar/3 (1 ms)
[ RUN      ] time_util_instance/time_util.calendar/4
[       OK ] time_util_instance/time_util.calendar/4 (1 ms)
[ RUN      ] time_util_instance/time_util.isdst/0
../src/system/time_util_test.cpp:133: Failure
Expected equality of these values:
  clt1.sec
    Which is: 553420500
  clt2.sec
    Which is: 553416900
[  FAILED  ] time_util_instance/time_util.isdst/0, where GetParam() = "Asia/Tokyo" (0 ms)
[ RUN      ] time_util_instance/time_util.isdst/1
../src/system/time_util_test.cpp:133: Failure
Expected equality of these values:
  clt1.sec
    Which is: 553490700
  clt2.sec
    Which is: 553488900
[  FAILED  ] time_util_instance/time_util.isdst/1, where GetParam() = "US/Hawaii" (1 ms)
[ RUN      ] time_util_instance/time_util.isdst/2
[       OK ] time_util_instance/time_util.isdst/2 (0 ms)
[ RUN      ] time_util_instance/time_util.isdst/3
[       OK ] time_util_instance/time_util.isdst/3 (0 ms)
[ RUN      ] time_util_instance/time_util.isdst/4
[       OK ] time_util_instance/time_util.isdst/4 (1 ms)
[----------] 20 tests from time_util_instance/time_util (6 ms total)

[----------] Global test environment tear-down
[==========] 20 tests from 1 test case ran. (6 ms total)
[  PASSED  ] 16 tests.
[  FAILED  ] 4 tests, listed below:
[  FAILED  ] time_util_instance/time_util.calendar/0, where GetParam() = "Asia/Tokyo"
[  FAILED  ] time_util_instance/time_util.calendar/1, where GetParam() = "US/Hawaii"
[  FAILED  ] time_util_instance/time_util.isdst/0, where GetParam() = "Asia/Tokyo"
[  FAILED  ] time_util_instance/time_util.isdst/1, where GetParam() = "US/Hawaii"

 4 FAILED TESTS

test failed

@eiichiroi
Copy link
Contributor Author

time_util_test fails because it is wrong itself.

In glib, mktime function fixes the time if the given tm_isdst is different from tz database. (The implementation is likely to be different between glibc and BSD libc.)

In any case, I think that this problem should be resolved with another pull request.

@ysk24ok
Copy link
Contributor

ysk24ok commented Nov 18, 2020

Could you create issues for these problems so that we don't forget to fix?

  • stdio_filebuf problem
  • time_util_test fails

Copy link
Contributor

@ysk24ok ysk24ok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote following simple sample programs to confirm that pfi::network::socketstream and pfi::network::http::httpstream work correctly.

#include <iostream>
#include <string>

#include "pficommon/network/iostream.h"

int main() {
  pfi::network::socketstream ss("localhost", 8000);
  if (!ss) {
    std::cerr << "cannot open connection" << std::endl;
    std::exit(EXIT_FAILURE);
  }
  ss << "GET / HTTP/1.1\r\n\r\n"<< std::flush;
  for (std::string line; getline(ss, line); )
    std::cout << line << std::endl;
}
#include <iostream>
#include <string>

#include "pficommon/network/http.h"

int main() {
  pfi::network::http::httpstream hs("http://example.com/");
  if (!hs) {
    std::cerr << "cannot open connection" << std::endl;
    std::exit(EXIT_FAILURE);
  }
  for (std::string line; getline(hs, line); )
    std::cout << line << std::endl;
}

Both work fine, so LGTM. Let me merge this PR.

Don't forget to create two issues you pended on this PR.

@ysk24ok ysk24ok merged commit 209e5a9 into retrieva:v4.0 Nov 24, 2020
This was referenced Nov 27, 2020
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

Successfully merging this pull request may close these issues.

2 participants