[email protected]

[c8ef]

@bazel-io skip_check unstable_url

[bazel-io]

Hello @bazelbuild/bcr-maintainers, modules without existing maintainers (jsoncons) have been updated in this PR. Please review the changes.

[c8ef]

@bazel-io skip_check unstable_url

Require module maintainers' approval for newly pushed changes.

Require module maintainers' approval for newly pushed changes.

[fmeum]

Enabling header processing uncovered some issues. Does this need a newer C++ standard? You can add --cxxopt=-std=c++14 to the build_flags.

[c8ef]

I don't think enabling header processing for jsoncons is appropriate. It requires each header to be self-contained (ref: https://github.com/google/re2/blob/6dcd83d60f7944926bfd308cc13979fc53dd69ca/.bazelrc#L7). However, using size_t for jsoncons header is not.

[fmeum]

I don't think enabling header processing for jsoncons is appropriate. It requires each header to be self-contained (ref: https://github.com/google/re2/blob/6dcd83d60f7944926bfd308cc13979fc53dd69ca/.bazelrc#L7). However, using size_t for jsoncons header is not.

Does jsoncons require the use to manually include the extension header? What's the significance of size_t?

But yes, if the module has headers that intentionally aren't self-contained, we can revert the commit that enabled header processing. This is just pretty unusual, which is why I didn't expect it to result in false positives.

[c8ef]

Most CI failures stem from this: https://github.com/danielaparker/jsoncons/blob/master/include/jsoncons/ser_context.hpp. It uses size_t without including any other headers. It appears that it should not be included directly.

[fmeum]

To be honest, that does sound like a real bug in jsoncons, it should just include cstddef.h for this. We can merge this as is, but could you file a bug with upstream?