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

Add explicit keyword for some constructors in diplomat_runtime.hpp #349

Open
makotokato opened this issue Oct 31, 2023 · 1 comment
Open
Labels
B-cpp C++ backend
Milestone

Comments

@makotokato
Copy link
Contributor

Mozilla has a static analysis for C++ code by own's clang-plugin. ExplicitImplicitChecker is a checker for implicit type conversion in constructor.

The constructors of diplomat::Ok, diplomat::Err, diplomat::result and diplomat::span has no explicit keyword. So it means that it can cause implicit type conversion in constructor.

So I would like to request to add explicit keyword for some class's constructors.

I guess that https://google.github.io/styleguide/cppguide.html#Explicit_Constructors will be same things.

@Manishearth
Copy link
Contributor

Manishearth commented Oct 31, 2023

I'm not opposed to some of these. Ok/Err should be explicit. result and span ... prbably not; it's an intentional choice there. result is supposed to be implicitly constructable from Ok/Err and span is supposed to also be implicitly constructible just like std::span is. The Google style guide you link to talks about interchangeablility as an example.

It would be nice if we could have a CI job that matched the settings Mozilla uses to build so we don't discover these things during integration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-cpp C++ backend
Projects
None yet
Development

No branches or pull requests

2 participants