Skip to content

Conversation

@krinkinmu
Copy link

gcc complains about FieldBackedMapImpl::ListKeys when overloaded-virtual check is enabled. When using -Werror, warnings like this are converted to errors breaking builds.

That's what happened to gcc build of Envoy a while back. The issue in Envoy was fixed by introducing a patch for cel-cpp that was applied to the source during Envoy build (see
envoyproxy/envoy#31814).

Unfortunately during cel-cpp version update in Envoy instead of updating the patch, dropping the parts that are not needed anymore, we dropped the whole cep-cpp patch all together in
envoyproxy/envoy#36661. And now gcc can't build Envoy again.

This change makes the change to silence the gcc warning in the cel-cpp codebase, which hopefully will be a bit more durable than maintaing a patch on the Envoy side.

The change is not very invasive (just a using declaration), plus I can see at least another place in the code that does the same to deal with gcc warning (see

// Import base class signatures to bypass GCC warning/error.
using CelMap::ListKeys;
). With that in mind, would you be open to include this change in the cel-cpp upstream repository?

Thank you.

gcc complains about FieldBackedMapImpl::ListKeys when overloaded-virtual
check is enabled. When using -Werror, warnings like this are converted
to errors breaking builds.

That's what happened to gcc build of Envoy a while back. The issue in
Envoy was fixed by introducing a patch for cel-cpp that was applied to
the source during Envoy build (see
envoyproxy/envoy#31814).

Unfortunately during cel-cpp version update in Envoy instead of
updating the patch, dropping the parts that are not needed anymore, we
dropped the whole cep-cpp patch all together in
envoyproxy/envoy#36661. And now gcc can't build
Envoy again.

This change makes the change to silence the gcc warning in the cel-cpp
codebase, which hopefully will be a bit more durable than maintaing a
patch on the Envoy side.

Signed-off-by: Mikhail Krinkin <[email protected]>
@krinkinmu
Copy link
Author

I see that it was fixed, thank you!

@krinkinmu krinkinmu closed this Nov 5, 2024
copybara-service bot pushed a commit that referenced this pull request Nov 7, 2024
copybara-service bot pushed a commit that referenced this pull request Nov 7, 2024
copybara-service bot pushed a commit that referenced this pull request Nov 7, 2024
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.

1 participant