Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Headers not usable alone #550

Closed
Arfrever opened this issue Jan 3, 2021 · 10 comments
Closed

Headers not usable alone #550

Arfrever opened this issue Jan 3, 2021 · 10 comments

Comments

@Arfrever
Copy link

Arfrever commented Jan 3, 2021

Some headers of OpenCC are not usable alone, i.e. without manually including other headers (from either OpenCC or standard library).

Problem minimally existed in OpenCC 1.0.6 (1 affected header), and became worse in 1.1.0 and 1.1.1 (4 affected headers, including main opencc.h).

In directory of installed headers of OpenCC 1.0.6 (usually /usr/include/opencc):

$ (for header in *; do g++ -o /tmp/opencc_test -x c++ -I/usr/include/opencc - <<<"#include \"${header}\""$'\nint main(){}' && echo -e "\e[1;32m=== GOOD: ${header} ===\e[0m" || echo -e "\e[1;31m=== BAD:  ${header} ===\e[0m"; done)
=== GOOD: BinaryDict.hpp ===
=== GOOD: Common.hpp ===
=== GOOD: Config.hpp ===
=== GOOD: Conversion.hpp ===
=== GOOD: ConversionChain.hpp ===
=== GOOD: Converter.hpp ===
=== GOOD: DartsDict.hpp ===
=== GOOD: Dict.hpp ===
=== GOOD: DictConverter.hpp ===
=== GOOD: DictEntry.hpp ===
=== GOOD: DictGroup.hpp ===
=== GOOD: Exception.hpp ===
=== GOOD: Export.hpp ===
=== GOOD: Lexicon.hpp ===
=== GOOD: MaxMatchSegmentation.hpp ===
=== GOOD: Optional.hpp ===
=== GOOD: PhraseExtract.hpp ===
=== GOOD: Segmentation.hpp ===
=== GOOD: Segments.hpp ===
=== GOOD: SerializableDict.hpp ===
In file included from <stdin>:1:
SimpleConverter.hpp:34:21: error: variable ‘opencc::OPENCC_EXPORT opencc::SimpleConverter’ has initializer but incomplete type
   34 | class OPENCC_EXPORT SimpleConverter {
      |                     ^~~~~~~~~~~~~~~
SimpleConverter.hpp:35:1: error: expected primary-expression before ‘public’
   35 | public:
      | ^~~~~~
SimpleConverter.hpp:35:1: error: expected ‘}’ before ‘public’
SimpleConverter.hpp:34:37: note: to match this ‘{’
   34 | class OPENCC_EXPORT SimpleConverter {
      |                                     ^
SimpleConverter.hpp:35:1: error: expected ‘,’ or ‘;’ before ‘public’
   35 | public:
      | ^~~~~~
SimpleConverter.hpp:42:19: error: expected class-name before ‘(’ token
   42 |   ~SimpleConverter();
      |                   ^
SimpleConverter.hpp:48:8: error: ‘string’ in namespace ‘std’ does not name a type
   48 |   std::string Convert(const std::string& input) const;
      |        ^~~~~~
SimpleConverter.hpp:1:1: note: ‘std::string’ is defined in header ‘<string>’; did you forget to ‘#include <string>’?
  +++ |+#include <string>
    1 | /*
SimpleConverter.hpp:54:8: error: ‘string’ in namespace ‘std’ does not name a type
   54 |   std::string Convert(const char* input) const;
      |        ^~~~~~
SimpleConverter.hpp:54:3: note: ‘std::string’ is defined in header ‘<string>’; did you forget to ‘#include <string>’?
   54 |   std::string Convert(const char* input) const;
      |   ^~~
SimpleConverter.hpp:61:8: error: ‘string’ in namespace ‘std’ does not name a type
   61 |   std::string Convert(const char* input, size_t length) const;
      |        ^~~~~~
SimpleConverter.hpp:61:3: note: ‘std::string’ is defined in header ‘<string>’; did you forget to ‘#include <string>’?
   61 |   std::string Convert(const char* input, size_t length) const;
      |   ^~~
SimpleConverter.hpp:70:3: error: ‘size_t’ does not name a type
   70 |   size_t Convert(const char* input, char* output) const;
      |   ^~~~~~
SimpleConverter.hpp:1:1: note: ‘size_t’ is defined in header ‘<cstddef>’; did you forget to ‘#include <cstddef>’?
  +++ |+#include <cstddef>
    1 | /*
SimpleConverter.hpp:80:3: error: ‘size_t’ does not name a type
   80 |   size_t Convert(const char* input, size_t length, char* output) const;
      |   ^~~~~~
SimpleConverter.hpp:80:3: note: ‘size_t’ is defined in header ‘<cstddef>’; did you forget to ‘#include <cstddef>’?
SimpleConverter.hpp:82:1: error: expected unqualified-id before ‘private’
   82 | private:
      | ^~~~~~~
SimpleConverter.hpp:86:1: error: expected declaration before ‘}’ token
   86 | } // namespace opencc
      | ^
=== BAD:  SimpleConverter.hpp ===
=== GOOD: TextDict.hpp ===
=== GOOD: UTF8StringSlice.hpp ===
=== GOOD: UTF8Util.hpp ===
=== GOOD: opencc.h ===

In directory of installed headers of OpenCC 1.1.1 (usually /usr/include/opencc):

$ (for header in *; do g++ -o /tmp/opencc_test -x c++ -I/usr/include/opencc - <<<"#include \"${header}\""$'\nint main(){}' && echo -e "\e[1;32m=== GOOD: ${header} ===\e[0m" || echo -e "\e[1;31m=== BAD:  ${header} ===\e[0m"; done)
In file included from <stdin>:1:
BinaryDict.hpp:37:10: error: ‘BinaryDictPtr’ does not name a type; did you mean ‘BinaryDict’?
   37 |   static BinaryDictPtr NewFromFile(FILE* fp);
      |          ^~~~~~~~~~~~~
      |          BinaryDict
=== BAD:  BinaryDict.hpp ===
=== GOOD: Common.hpp ===
=== GOOD: Config.hpp ===
=== GOOD: Conversion.hpp ===
=== GOOD: ConversionChain.hpp ===
=== GOOD: Converter.hpp ===
In file included from <stdin>:1:
DartsDict.hpp:47:10: error: ‘DartsDictPtr’ does not name a type; did you mean ‘DartsDict’?
   47 |   static DartsDictPtr NewFromDict(const Dict& thatDict);
      |          ^~~~~~~~~~~~
      |          DartsDict
DartsDict.hpp:49:10: error: ‘DartsDictPtr’ does not name a type; did you mean ‘DartsDict’?
   49 |   static DartsDictPtr NewFromFile(FILE* fp);
      |          ^~~~~~~~~~~~
      |          DartsDict
=== BAD:  DartsDict.hpp ===
=== GOOD: Dict.hpp ===
=== GOOD: DictConverter.hpp ===
=== GOOD: DictEntry.hpp ===
=== GOOD: DictGroup.hpp ===
=== GOOD: Exception.hpp ===
=== GOOD: Export.hpp ===
=== GOOD: Lexicon.hpp ===
=== GOOD: MarisaDict.hpp ===
=== GOOD: MaxMatchSegmentation.hpp ===
=== GOOD: Optional.hpp ===
=== GOOD: PhraseExtract.hpp ===
=== GOOD: Segmentation.hpp ===
=== GOOD: Segments.hpp ===
=== GOOD: SerializableDict.hpp ===
=== GOOD: SerializedValues.hpp ===
In file included from <stdin>:1:
SimpleConverter.hpp:42:30: error: ‘string’ in namespace ‘std’ does not name a type
   42 |   SimpleConverter(const std::string& configFileName);
      |                              ^~~~~~
SimpleConverter.hpp:20:1: note: ‘std::string’ is defined in header ‘<string>’; did you forget to ‘#include <string>’?
   19 | #include "Export.hpp"
  +++ |+#include <string>
   20 | 
SimpleConverter.hpp:50:8: error: ‘string’ in namespace ‘std’ does not name a type
   50 |   std::string Convert(const std::string& input) const;
      |        ^~~~~~
SimpleConverter.hpp:50:3: note: ‘std::string’ is defined in header ‘<string>’; did you forget to ‘#include <string>’?
   50 |   std::string Convert(const std::string& input) const;
      |   ^~~
SimpleConverter.hpp:56:8: error: ‘string’ in namespace ‘std’ does not name a type
   56 |   std::string Convert(const char* input) const;
      |        ^~~~~~
SimpleConverter.hpp:56:3: note: ‘std::string’ is defined in header ‘<string>’; did you forget to ‘#include <string>’?
   56 |   std::string Convert(const char* input) const;
      |   ^~~
SimpleConverter.hpp:64:8: error: ‘string’ in namespace ‘std’ does not name a type
   64 |   std::string Convert(const char* input, size_t length) const;
      |        ^~~~~~
SimpleConverter.hpp:64:3: note: ‘std::string’ is defined in header ‘<string>’; did you forget to ‘#include <string>’?
   64 |   std::string Convert(const char* input, size_t length) const;
      |   ^~~
SimpleConverter.hpp:73:3: error: ‘size_t’ does not name a type
   73 |   size_t Convert(const char* input, char* output) const;
      |   ^~~~~~
SimpleConverter.hpp:20:1: note: ‘size_t’ is defined in header ‘<cstddef>’; did you forget to ‘#include <cstddef>’?
   19 | #include "Export.hpp"
  +++ |+#include <cstddef>
   20 | 
SimpleConverter.hpp:84:3: error: ‘size_t’ does not name a type
   84 |   size_t Convert(const char* input, size_t length, char* output) const;
      |   ^~~~~~
SimpleConverter.hpp:84:3: note: ‘size_t’ is defined in header ‘<cstddef>’; did you forget to ‘#include <cstddef>’?
=== BAD:  SimpleConverter.hpp ===
=== GOOD: TextDict.hpp ===
=== GOOD: UTF8StringSlice.hpp ===
=== GOOD: UTF8Util.hpp ===
In file included from opencc.h:25,
                 from <stdin>:1:
SimpleConverter.hpp:42:30: error: ‘string’ in namespace ‘std’ does not name a type
   42 |   SimpleConverter(const std::string& configFileName);
      |                              ^~~~~~
SimpleConverter.hpp:1:1: note: ‘std::string’ is defined in header ‘<string>’; did you forget to ‘#include <string>’?
  +++ |+#include <string>
    1 | /*
SimpleConverter.hpp:50:8: error: ‘string’ in namespace ‘std’ does not name a type
   50 |   std::string Convert(const std::string& input) const;
      |        ^~~~~~
SimpleConverter.hpp:50:3: note: ‘std::string’ is defined in header ‘<string>’; did you forget to ‘#include <string>’?
   50 |   std::string Convert(const std::string& input) const;
      |   ^~~
SimpleConverter.hpp:56:8: error: ‘string’ in namespace ‘std’ does not name a type
   56 |   std::string Convert(const char* input) const;
      |        ^~~~~~
SimpleConverter.hpp:56:3: note: ‘std::string’ is defined in header ‘<string>’; did you forget to ‘#include <string>’?
   56 |   std::string Convert(const char* input) const;
      |   ^~~
SimpleConverter.hpp:64:8: error: ‘string’ in namespace ‘std’ does not name a type
   64 |   std::string Convert(const char* input, size_t length) const;
      |        ^~~~~~
SimpleConverter.hpp:64:3: note: ‘std::string’ is defined in header ‘<string>’; did you forget to ‘#include <string>’?
   64 |   std::string Convert(const char* input, size_t length) const;
      |   ^~~
SimpleConverter.hpp:73:3: error: ‘size_t’ does not name a type
   73 |   size_t Convert(const char* input, char* output) const;
      |   ^~~~~~
SimpleConverter.hpp:1:1: note: ‘size_t’ is defined in header ‘<cstddef>’; did you forget to ‘#include <cstddef>’?
  +++ |+#include <cstddef>
    1 | /*
SimpleConverter.hpp:84:3: error: ‘size_t’ does not name a type
   84 |   size_t Convert(const char* input, size_t length, char* output) const;
      |   ^~~~~~
SimpleConverter.hpp:84:3: note: ‘size_t’ is defined in header ‘<cstddef>’; did you forget to ‘#include <cstddef>’?
=== BAD:  opencc.h ===
@BYVoid
Copy link
Owner

BYVoid commented Feb 25, 2021

Could you please suggest why should people use headers alone? The ABI of C++ is very unstable. I do not encourage people to include C++ header directly.

@Arfrever
Copy link
Author

Problem was originally reported in Gentoo for building of pyzy, which apparently has in https://github.com/pyzy/pyzy/blob/6d9c3cdff364e0da75e1c26222240f26370ebf73/src/SimpTradConverter.cc#L29-L30:

#ifdef HAVE_OPENCC
#  include <opencc.h>

I will try to create patch for OpenCC.

@BYVoid
Copy link
Owner

BYVoid commented Feb 26, 2021

Ah sorry I misunderstood you. It looks like a real problem if #include <opencc.h> does not work.

Looking forward to your patch or pull request.

@Arfrever
Copy link
Author

Arfrever commented Mar 2, 2021

Apparently problem in SimpleConverter.hpp and opencc.h was already fixed in commit c08b4b2 (#417) (5 days after 1.1.1 release).

Problems in BinaryDict.hpp and DartsDict.hpp remain.
Patch for problems in BinaryDict.hpp and DartsDict.hpp: OpenCC-headers.patch.txt
This patch makes CMake generate opencc_config.h header from opencc_config.h.in, and opencc_config.h header will be installed.
opencc_config.h header may define OPENCC_ENABLE_DARTS, which is now used in Common.hpp header, which conditionally defines BinaryDictPtr and DartsDictPtr types, which are used in BinaryDict.hpp and DartsDict.hpp headers.

@BYVoid
Copy link
Owner

BYVoid commented Mar 2, 2021

Hi @Arfrever,

Thanks for the patch. However make test failed on my side with the patch:

Scanning dependencies of target CommandLineConvertTest
[ 98%] Building CXX object test/CMakeFiles/CommandLineConvertTest.dir/CommandLineConvertTest.cpp.o
In file included from /Users/byvoid/dev/opencc/test/CommandLineConvertTest.cpp:21:
/Users/byvoid/dev/opencc/test/../src/Common.hpp:31:10: fatal error: 'opencc_config.h' file not found
#include "opencc_config.h"
         ^~~~~~~~~~~~~~~~~
1 error generated.

I did not look into it very much. Could you please help check if this error reproduces?

@Arfrever
Copy link
Author

Arfrever commented Mar 2, 2021

test/CMakeLists.txt had include_directories(../src), but addition of similar setting for ${PROJECT_BINARY_DIR}/src was forgotten.
For consistency and simplicity, include_directories(..) in subdirectories (src/benchmark/CMakeLists.txt, src/tools/CMakeLists.txt) and include_directories(../src) in test/CMakeLists.txt are changed to include_directories("${PROJECT_SOURCE_DIR}/src") in src/CMakeLists.txt and test/CMakeLists.txt.

Patch: OpenCC-headers.patch.txt

@Arfrever
Copy link
Author

Arfrever commented Mar 3, 2021

Improved patch: OpenCC-headers.patch.txt
("${PROJECT_BINARY_DIR}/src/opencc_config.h" added to ${LIBOPENCC_HEADERS} variable.)

@BYVoid
Copy link
Owner

BYVoid commented Mar 4, 2021

Hi @Arfrever,

Thanks for the patch again. make test worked without any problem. However, this does not work with Node.js binding, which requires gyp. The current configuration does not generate opencc_config.h for node (make node-test).

@Arfrever
Copy link
Author

Arfrever commented Mar 4, 2021

It seems that Darts support is not used in Node.js binding.
I can add node/opencc_config.h file, which is the same as CMake-generated ${PROJECT_BINARY_DIR}/src/opencc_config.h in case of disabled Darts support.

When I try node-gyp configure, it fails for me with Error: Cannot find module 'nan', so I cannot test this idea.

Possibly working patch: OpenCC-headers.patch.txt
(I do not know if change in node/node_opencc.gypi is actually required.)

BYVoid pushed a commit that referenced this issue Mar 4, 2021
From d18eac4ee14b871c2c9ebeb9fc9ee797e6b2109a Mon Sep 17 00:00:00 2001
From: Arfrever Frehtes Taifersar Arahesis <Arfrever@Apache.Org>
Date: Tue, 2 Mar 2021 00:00:00 +0000
@BYVoid
Copy link
Owner

BYVoid commented Mar 4, 2021

Thanks. Submitted.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants