proxy-wasm / proxy-wasm-cpp-host

WebAssembly for Proxies (C++ host implementation)
Apache License 2.0
84 stars 69 forks source link

Add missing includes / don't include standard headers in namespaces #364

Closed mkauf closed 11 months ago

mkauf commented 1 year ago

There are some missing includes. On some systems, these includes may not be necessary (because they are contained in other system headers).

I have found these while compiling Envoy proxy on Fedora 38.

vm_id_handle.h:

wasm_vm.h:

mkauf commented 1 year ago

I also got other strange compile errors when compiling Envoy proxy:

In file included from external/proxy_wasm_cpp_host/src/null/null.cc:17:
In file included from external/proxy_wasm_cpp_host/include/proxy-wasm/null_vm.h:22:
In file included from external/proxy_wasm_cpp_host/include/proxy-wasm/null_vm_plugin.h:18:
In file included from external/proxy_wasm_cpp_host/include/proxy-wasm/wasm_vm.h:27:
In file included from external/proxy_wasm_cpp_host/include/proxy-wasm/word.h:22:
external/proxy_wasm_cpp_sdk/proxy_wasm_common.h:59:8: error: no type named 'string' in namespace 'proxy_wasm::std'; did you mean '::std::string'?
inline std::string toString(WasmResult r) {
       ^~~~~~~~~~~
       ::std::string
/usr/bin/../lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/stringfwd.h:77:33: note: '::std::string' declared here
  typedef basic_string<char>    string;   

I think that happens because the header proxy_wasm_common.h contains these lines:

#include <cstdint>
#include <string>

And in files like wasm.h, the file proxy_wasm_common.h is included inside a C++ namespace:

namespace proxy_wasm {

#include "proxy_wasm_common.h"

...
}

-> so the header <string> is included inside the namespace proxy_wasm. That's bad practice and probably that's also the cause of the build problem.

sitano commented 1 year ago

also ran into it: