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

Compilation fails on Windows when building with UNICODE #146

Open
saraedum opened this issue Sep 6, 2022 · 2 comments
Open

Compilation fails on Windows when building with UNICODE #146

saraedum opened this issue Sep 6, 2022 · 2 comments

Comments

@saraedum
Copy link
Member

saraedum commented Sep 6, 2022

Compiling cppast fails on Windows with:

%SRC_DIR%\src\libclang\libclang_parser.cpp(250): error C2664: 'TinyProcessLib::Process::Process(const std::vector<TinyProcessLib::Process::string_type,std::allocator<TinyProcessLib::Process::string_type>> &,const TinyProcessLib::Process::string_type &,std::function<void (const char *,size_t)>,std::function<void (const char *,size_t)>,bool,const TinyProcessLib::Config &) noexcept': cannot convert argument 1 from 'std::basic_string<char,std::char_traits<char>,std::allocator<char>>' to 'const std::vector<TinyProcessLib::Process::string_type,std::allocator<TinyProcessLib::Process::string_type>> &'
%SRC_DIR%\src\libclang\libclang_parser.cpp(251): note: Reason: cannot convert from 'std::basic_string<char,std::char_traits<char>,std::allocator<char>>' to 'const std::vector<TinyProcessLib::Process::string_type,std::allocator<TinyProcessLib::Process::string_type>>'
%SRC_DIR%\src\libclang\libclang_parser.cpp(251): note: No user-defined-conversion operator available that can perform this conversion, or the operator cannot be called

The problem here seems to be that cppast is calling into tiny-process-library with a std::string. However, when UNICODE is set on windows, tiny-process-library is expecting a std::wstring, namely it defines:

#ifdef _WIN32
  typedef unsigned long id_type; // Process id type
  typedef void *fd_type;         // File descriptor type
#ifdef UNICODE
  typedef std::wstring string_type;
#else
  typedef std::string string_type;
#endif

So, I don't know much about Windows but I guess this could essentially be fixed by using std::wstring to store the path to the clang binary on Windows. So, we probably need the same #ifdef to select which string type to use here.

If this sounds like a good plan to you @foonathan, I am happy to produce a PR for this.

@foonathan
Copy link
Collaborator

This just pushes the problem out to the user though. I think it's fine if we assume ASCII paths and just convert the string to wstring by static_cast'ing each char before calling tiny process.

@saraedum saraedum changed the title Compilation fails on Windows Compilation fails on Windows when building with UNICODE Sep 18, 2022
@saraedum
Copy link
Member Author

A patch that interprets string as UTF8 is below. It turned out that I am not building with UNICODE anymore in the end and I also don't have a Windows machine to test this out. So if somebody else runs into this problem, they might want to use that patch as a starting point.

From 64bdb0ce353c5ecca5e6eb741210391ec7e397be Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Julian=20R=C3=BCth?= <julian.rueth@fsfe.org>
Date: Sat, 17 Sep 2022 01:54:55 +0300
Subject: [PATCH] Fix compilation if tpl uses wstring for command lines

we just pretend that the input is UTF-8 encoded. If that's not the case
then there will likely be strange errors due to the wrong encoding.
---
 src/libclang/libclang_parser.cpp |  7 ++++---
 src/libclang/preprocessor.cpp    | 14 ++++++++++++--
 src/libclang/preprocessor.hpp    | 11 +++++++++++
 3 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/src/libclang/libclang_parser.cpp b/src/libclang/libclang_parser.cpp
index b45290b..afa65a4 100644
--- a/src/libclang/libclang_parser.cpp
+++ b/src/libclang/libclang_parser.cpp
@@ -6,6 +6,7 @@
 #include <cstring>
 #include <fstream>
 #include <vector>
+#include <string>
 
 #include <clang-c/CXCompilationDatabase.h>
 #include <process.hpp>
@@ -245,10 +246,10 @@ libclang_compile_config::libclang_compile_config(const libclang_compilation_data
 
 namespace
 {
+
 bool is_valid_binary(const std::string& binary)
 {
-    tpl::Process process(
-        binary + " -v", "", [](const char*, std::size_t) {}, [](const char*, std::size_t) {});
+    tpl::Process process(detail::widen<tpl::Process::string_type>(binary) + detail::widen<tpl::Process::string_type>(" -v"));
     return process.get_exit_status() == 0;
 }
 
@@ -262,7 +263,7 @@ void add_default_include_dirs(libclang_compile_config& config)
 {
     std::string  verbose_output;
     tpl::Process process(
-        detail::libclang_compile_config_access::clang_binary(config) + " -x c++ -v -", "",
+        detail::widen<tpl::Process::string_type>(detail::libclang_compile_config_access::clang_binary(config)) + detail::widen<tpl::Process::string_type>(" -x c++ -v -"), tpl::Process::string_type(),
         [](const char*, std::size_t) {},
         [&](const char* str, std::size_t n) { verbose_output.append(str, n); }, true);
     process.write("", 1);
diff --git a/src/libclang/preprocessor.cpp b/src/libclang/preprocessor.cpp
index bebfc34..99825f1 100644
--- a/src/libclang/preprocessor.cpp
+++ b/src/libclang/preprocessor.cpp
@@ -10,6 +10,8 @@
 #include <cstring>
 #include <fstream>
 #include <unordered_map>
+#include <locale>
+#include <codecvt>
 
 #include <process.hpp>
 
@@ -373,7 +375,7 @@ std::string write_macro_file(const libclang_compile_config& c, const std::string
 
     auto         cmd = get_macro_command(c, full_path.c_str());
     tpl::Process process(
-        cmd, "", [&](const char* str, std::size_t n) { stream.write(str, std::streamsize(n)); },
+        detail::widen<tpl::Process::string_type>(cmd), tpl::Process::string_type(), [&](const char* str, std::size_t n) { stream.write(str, std::streamsize(n)); },
         diagnostic_logger);
 
     if (auto include_guard = get_include_guard_macro(full_path))
@@ -433,7 +435,7 @@ clang_preprocess_result clang_preprocess_impl(const libclang_compile_config& c,
 
     auto         cmd = get_preprocess_command(c, full_path.c_str(), macro_path);
     tpl::Process process(
-        cmd, "",
+        detail::widen<tpl::Process::string_type>(cmd), tpl::Process::string_type(),
         [&](const char* str, std::size_t n) {
             for (auto ptr = str; ptr != str + n; ++ptr)
                 if (*ptr == '\t')
@@ -1250,3 +1252,11 @@ detail::preprocessor_output detail::preprocess(const libclang_compile_config& co
 
     return result;
 }
+
+template <typename S>
+typename std::enable_if<std::is_same<S, std::wstring>::value, S>::type detail::widen(const std::string& s)
+{
+  // see https://stackoverflow.com/a/18597384/812379
+  std::wstring_convert<std::codecvt_utf8_utf16<wchar_t>> converter;
+  return converter.from_bytes(s);
+}
diff --git a/src/libclang/preprocessor.hpp b/src/libclang/preprocessor.hpp
index 826ccda..860088a 100644
--- a/src/libclang/preprocessor.hpp
+++ b/src/libclang/preprocessor.hpp
@@ -48,6 +48,17 @@ namespace detail
 
     preprocessor_output preprocess(const libclang_compile_config& config, const char* path,
                                    const diagnostic_logger& logger);
+
+    // Convert the input to a string of type S.
+    // If S is a string, return the input unchanged.
+    // Otherwise, pretend that the input is UTF-8 and convert to wstring.
+    template <typename S>
+    typename std::enable_if<std::is_same<S, std::string>::value, S>::type widen(const std::string& s) {
+      return s;
+    }
+
+    template <typename S>
+    typename std::enable_if<std::is_same<S, std::wstring>::value, S>::type widen(const std::string& s);
 } // namespace detail
 } // namespace cppast
 
-- 
2.37.1

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

2 participants