Chromium specific plugin

[Tomek Jurkiewicz]

Hello,

I'm looking for the right tool to prevent a potentially dangerous construct: std::optional<signin::Tribool>. This can lead to confusing boolean evaluations and is also an equivalent of std:optional<std::optional<bool>>, which is ill-defined.

I'm wondering if this is a valid use case for a plugin. My team is interested in an automated way to prevent these usages, as there's a recurring debate about why not to use this construct. It would be helpful to document this knowledge within an automated tool.

I admit that the scope is quite local and limited to the signin/sync teams of Chrome, so I'm unsure if this is the right approach.

●●●●
Tomasz Jurkiewicz
Software Engineer

[Hans Wennborg]

+dcheng who's the plugin expert.

I don't think we'd want a separate plugin for this, but maybe it could fit within the existing style plugin.

If it's simple enough to check for, maybe this could be a regular presubmit check though?

[Nico Weber]

How often does this happen?

Maybe signing::Tribool could be internal to the (de)serialization code and the public API could be optional<bool>?

If it's limited to a few directories, maybe a presubmit is good enough?

(No strong opinion, but clang plugins are pretty heavy hammers.)

[Tomek Jurkiewicz]

How often? Honestly, rarely, but if there's a lightweight solution to that, then I think it's worth it. I'm exactly fishing for opinions here.

 

I'm unsure if that can be replaced with std::optional<bool>, namely because static_cast<bool>(std::optional<bool>(false)) evaluates to true, a construct which the owning team wants to avoid in code 

●●●●
Tomasz Jurkiewicz
Software Engineer

[Nico Weber]

Is that something that happens in practice? If that's a common buggy pattern, that's something we could add an upstream clang warning for, since nothing in that line is chromium-specific.

(As https://chromium.googlesource.com/chromium/src.git/+/HEAD/docs/writing_clang_plugins.md says, "Don't write a clang plugin" :) )

[Tomek Jurkiewicz]

Let me put this this way: it's a common question that's being asked within the team. What I'm looking for is the right way to automate this.

I understand that the plugin might be an overkill, at the same time I fail to find any guidance to do it another way. What I need is just to check if new code introduces a specific type. The check could be limited to a team-owned code base and seems like it's just a right query (clang-query of form varDecl(hasType(.))?), which is upstream clang warning. Do you have any guidance on this? Sorry for the noob question, I'm a little bit lost and confused with this excess of options and information.

●●●●
Tomasz Jurkiewicz
Software Engineer