tool: Expose family() in favour of incomplete is_like_xxx() wrappers

[MarijnS95]

Instead of adding a new is_like_xxx() function every time new information is needed, or new enumeration variants would be added, provide the (now non_exhaustive) enum directly to callers to match what they're interested in.

Also removes is_like_clang_cl() again from #1357 since that did not yet make it into a release, and would only cause unnecessary duplication in our API patterns.

[NobodyXu]

Thanks!

Have a few suggestions, and I will cut release tomorrow as it's too late for me.

[NobodyXu]

Not sure if we should deprecate this now cc @madsmtm what do you think?

[MarijnS95]

Wasn't sure either, might be a bit harsh but otherwise we could forget about it.

[MarijnS95]

At the same time matching against self.family() for single variants is a bit ugly as you've seen.

[MarijnS95]

Have a few suggestions, and I will cut release tomorrow as it's too late for me.

If we incorporate this PR, shall I just go ahead and remove is_like_clang_cl() again since it's only extra duplication?

[NobodyXu]

If we incorporate this PR, shall I just go ahead and remove is_like_clang_cl() again since it's only extra duplication?

Yes i think so

[briansmith]

I suggest we hold off on doing this until the situation gets out of hand. Deprecating the current methods will cause a lot of unneeded churn, especially for projects that don't want to increase their minimum cc-rs version.

[MarijnS95]

@briansmith yeah makes sense, 1.x.x is likely to stay around for a long time. I've removed it so that this PR is ready for the proposed merge and release whenever @NobodyXu is available again (and I'm likely asleep).

[briansmith]

I wonder if it is a good idea to expose family() publicly like this at all, though? I would expect to have more thorough tests for it, first, if so.

[NobodyXu]

I wonder if it is a good idea to expose family() publicly like this at all, though?

Regarding API I think it's ok.

But since it's new API, cc @madsmtm can you take a look at this please?

I would expect to have more thorough tests for it, first, if so.

I remember we do have some tests for compiler family detection

[madsmtm]

Hmm, I don't particularly like exposing ToolFamily, for several reasons:

• I'm not certain that Msvc { clang_cl } is the right nesting, it could also be e.g. Clang { cl }.
• I'm unsure if these three families are really the right separation, e.g. we might want ToolFamily::VxWorks or ToolFamily::Emscripten one day, but if we expose ToolFamily we're forced to put those under ToolFamily::Gnu { vxworks } or ToolFamily::Clang { emscripten }.
  Basically, I'm trying to say that even with #[non_exhaustive] on the enum, it'd still be a semantic breaking change to add a new variant.
• Methods allows us to do the lookup more lazily. E.g. right now we don't use the zig_cc variable internally, so we might actually want to call is_zig_cc lazily instead when the user asks, but that would be harder if we exposed family.

[NobodyXu]

That's indeed true, we might want to reorder ToolFamily later...So perhaps we should close this PR for now and instead has more is_xxx functions?

[madsmtm]

So perhaps we should close this PR for now and instead has more is_xxx functions?

I'd prefer that, yes.

[NobodyXu]

Sorry @MarijnS95, thank you for your work but we are still too worried about the potential API breakage/subpar API, we will close this PR for now and I will cut a new release.