Allow core/ and modules/ to use base/

[Kentaro Hara]

Hi

Currently we have a restriction that wtf/ and platform/ can use base/ but core/ and modules/ cannot. If core/ and modules/ want to use base/, we need to create a thin wrapper in wtf/ or platform/. This restriction was introduced because we wanted to prevent developers from using base:: things randomly in core/ and modules/.

However, this restriction is getting a bit inconvenient. For example, it's silly that wtf/AutoReset.h is just fowarding to base::AutoReset. We have to add a lot of 'using' statements to wtf/Partitions.h just to forward the functions to base/'s functions.

Maybe would it make sense to relax the restriction and allow core/ and modules/ to use base/? We have already allowed core/ and modules/ to use base/'s macros such as DCHECK.

What do you think?

--

Kentaro Hara, Tokyo, Japan

[Elliott Sprehn]

I don't want to relax this rule, it's simple to follow and easy to enforce. Allowing direct usage of base also allows random functions to sneak through, for example things using std::string, std::vector and std::map.

I put a bunch of thought into this early on with Onion Soup and decided it was better to have a simple rule that base was not allowed instead of having a complex whitelist of types and functions that require a fancy presubmit.

[Kentaro Hara]

I'm not proposing to expose random things in base/. I'm just proposing to keep the whitelisting in DEPs but allow core/ and modules/ to use them as well as wtf/ and platform/. Does that sound too bad?

I'm wondering if adding lots of 'using' statements in wtf/ would be a win.

[Elliott Sprehn]

haraken@ and I discussed this offline. The problem here is that DEPS just controls what headers you include, not what types you can use in the code, so transitive deps end up being allowed as well. The current presubmit simply forbids "base::" entirely in core/, if we relax that then including a header that was allowed by DEPS would then bring in things like base::StringPiece. To fix this we then end up adding a complex presubmit that has a whitelist of class names, and we still have to deal with the problem of Oilpan/WTF compatibility for all of the classes that were transitively brought in.

The current policy was chosen for it's simplicity, no base, except things that have either a thoughtful wrapper class making them Oilpan and WTF friendly, or a typedef for something so simple that it's already compatible (ex. enum, only holds ints, etc.)

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CABg10jxvr51HCywx4VC5wXxF3kQSUQR_eNXCk5qtpBcSG%2B-Keg%40mail.gmail.com.