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

LOG macro conflicts #154

Open
kyessenov opened this issue May 15, 2023 · 4 comments
Open

LOG macro conflicts #154

kyessenov opened this issue May 15, 2023 · 4 comments
Assignees

Comments

@kyessenov
Copy link
Collaborator

kyessenov commented May 15, 2023

Abseil has defined LOG as well: https://github.com/abseil/abseil-cpp/blob/master/absl/log/log.h

proxy-wasm-api.h defining the macro unconditionally causes issues importing abseil into wasm.

@mpwarres
Copy link
Contributor

That's an unfortunate collision, though I guess it isn't surprising. We can't simply switch to something like PROXY_WASM_LOG since that would break existing code. But maybe we could have a conditional define similar to PROXY_WASM_PROTOBUF, which (if defined) would cause proxy_wasm_api.h to define the LOG macros as PROXY_WASM_LOG instead of LOG.

@kyessenov supposing we added such a USE_PROXY_WASM_LOG macro, would that address your issue?

@kyessenov
Copy link
Collaborator Author

Yeah, that would work. The problem is mainly "null VM" code since it imports both parts of Envoy with absl and proxy-wasm header. I think it's fine to disable the log macros in those modules since they are not standalone and coupled with Envoy.

@PiotrSikora
Copy link
Member

Does anybody use LOG directly? I think it's an internal implementation detail and end users use LOG_TRACE, etc. If that's the case, then we should be able to rename it without any extra defines.

@kyessenov
Copy link
Collaborator Author

@PiotrSikora Well, it's public so we can't be sure. I'm not sure about the desire for backwards compatibility for the SDK - it might be OK to break it since it doesn't seem to be versioned in any way.

kyessenov added a commit to google/cel-cpp that referenced this issue May 18, 2023
LOG causes macro pollution and conflicts with ProxyWasm SDK (see proxy-wasm/proxy-wasm-cpp-sdk#154).

PiperOrigin-RevId: 533164114
kyessenov added a commit to google/cel-cpp that referenced this issue May 18, 2023
LOG causes macro pollution and conflicts with ProxyWasm SDK (see proxy-wasm/proxy-wasm-cpp-sdk#154).

PiperOrigin-RevId: 533164114
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