A temporary flag for guarding access to an in-development C++20 feature in clang

Clang Frontend
1

This comes from [clang] Implement CTAD for type alias template. by hokein · Pull Request #77890 · llvm/llvm-project · GitHub, but moving discussion here as discussed in the Clang WG meeting.

I’m implementing a missing C++20 CTAD feature in clang. The implementation needs further iterations before it’s production-ready. Aiming for a complete bulletproof version in one patch doesn’t work in practice: increases load on reviewers, makes it difficult to split work, and increases the risk of scope creep and substantial merge conflicts. And a large patch is still likely to have bugs.

Traditionally clang does not markl/guard incomplete features of this size, and this has caused problems for consumers of Clang in the past. To mitigate potential risks for users, I’m considering adding a temporary cc1 flag to control access to this feature.

Some have suggested that features need only be stable on release branches, but this isn’t always a great solution. Not all users consume from these branches, and the developer policy quality guidelines apply equally to HEAD. At Google we release clang frequently, 1-2 weeks behind HEAD. These releases get a lot of real-world testing, and we believe benefit the community substantially by uncovering bugs early. Checking an unstable feature without a safeguard means it will be immediately used (Hyrum’s law) and we need to revert in case of bugs (often including crash-on-invalid). For features known to be unfinished, this is unnecessary churn. A temporary flag is useful to both upstream and downstream, but we are open to other options.

We understand the maintenance burden of a temporary flag, we will be responsible for maintaining this flag. The flag will be removed once the feature is stable (ETA: before the next clang 19 release). I believe a cc1-only flag with a name like -funstable-ctad-alias is something we can safely remove, rather than maintain forever.

What do people think? cc @cor3ntin @AaronBallman

1 Like
2

There is nothing more permanent than a temporary fix. :smiley:

I’m not keen on adding the flag. IMO, the functionality should be landed in small enough chunks which incrementally introduce the feature so that the tree remains stable, and we should not introduce a feature flag for this given how small the footprint is for the feature. Command line options for experimental functionality are appropriate sometimes, but IMO, that should be reserved for larger features with significant surface area and a longer-term need (technical specifications being a good example).

My concern with adding the flag for a smaller feature like this is that an opt-in flag means we lose significant benefit from early testing and unintended impacts (most users won’t know to opt in), and an opt-out flag means we’re making language dialects that we won’t test and don’t scale well as we add more and more opt-out flags. There’s also the maintenance burden aspects of adding the flag only to tear it back out again. I appreciate the offer to be responsible for maintaining the flag, but corporate attention span can be quite short and it’s very easy for this sort of flag to get forgotten about unless we add something to a release checklist.

Checking an unstable feature without a safeguard means it will be immediately used (Hyrum’s law) and we need to revert in case of bugs (often including crash-on-invalid).

We should not be checking in features that are so unstable that we need to revert them; the goal is that the feature is incrementally added in stable chunks. Obviously, this doesn’t always happen because of the complexity involved with some features, but I don’t think that changes our goal.

4 Likes
4

I appreciate the perspective that is important to set a high quality bar for landing feature work, but I think Aaron is overweighting the cost of cc1 flags.

I’m not sure this particular feature deserves a feature flag, but I feel strongly that, as a project, we need a real process for feature flag development. It is not sustainable to run multi-year code reviews and to stabilize patches in pull requests. Our developer policy endorses the practice of incremental development. Landing patches behind a feature flag, shipping that work, enabling the flag in some cases, gathering experience from the field, and incorporating that back into the feature before enabling it ultimately removing the flag is a common-sense software industry best practice. We have run many migrations using this basic pattern in LLVM, such as opaque pointers, the new pass manager, and various debug info settings.

When you ultimately enable the feature by default, the feature flag provides users with an opt-out mechanism that they can use in the field without rolling back an entire toolchain update. This is important for us at Google, and while our use cases don’t always align with everyone else, I suspect that this is important for other users too.

Regarding the cost of these flags, I don’t think you need to rely on promises to clean up cc1 flags. The cc1 interface is hidden, undocumented, known to be unstable, and subject to change at any time. It should evolve in any way that is useful for the project. It does take work to remove flags from this interface and clean up the relevant conditionals, but that is the cost of accepting a cc1 flag, not the cost of testing the cross-product of all possible flag combinations. Anyone using the cc1 flag should expect that it may interact with other features in surprising ways. If I’m wrong and people feel that the cc1 interface is or should be more stable, then I think we need to find some other process for doing feature flag development.

2 Likes
5

I agree with this being the intent, but there are various ways in which we don’t adhere to it:

  1. We sometimes do document use of -Xclang. See Clang Compiler User’s Manual — Clang 19.0.0git documentation and Clang Compiler User’s Manual — Clang 19.0.0git documentation for some examples. We also include -Xclang in command line help and in the dump of the clang-cl options at Clang Compiler User’s Manual — Clang 19.0.0git documentation. If we really don’t want users to use -Xclang, then perhaps we should remove it from the --help output.

  2. I could have missed it, but I don’t think we document that cc1 options are not stable and that users should not rely on them. We probably should have a statement to that effect somewhere.

6

I think this should be a case by case decision instead of a general policy. Sometimes it is good to have opt-in features, e.g., -fcoroutines-ts. It is also true that too many opt-in flags would decrease testing quality. But it is hard to find the line. I don’t think we should define the line early. Let’s try to make a case by case decision.

3 Likes
7

I agree with @ChuanqiXu that this should be a case-by-case basis, and generally we’d want a flag because the feature is huge (concepts), not standard (tses) or maybe we have a high degree of concern about deploying the feature in production.

This fits neither. It’s a small feature on top of existing CTAD work, one that doesn’t affect existing code.
We routinely do larger changes to clang without flags. This is not a multi year effort, nor even a multi month effort.
We landed higher impact features this month, no flag.

In practice, flags tend to reduce field experience. If a work-in-progress breaks existing code, we need to know, and know as soon as possible. Hiding things behind flags just reduce our visibility. Individual users rarely enable flags (or read the doc letting them know there is a flag).
Switching a feature on by default before a release would for example, lead to surprises.

On the flip side, flags tend to just… reduce the pressure to fix bugs, as users can “just disable the flag”. We put C++17 conformance behind a flag years ago and no one is actively trying to fix that, getting us stuck in a local maximum, serving no one.

Lastly, if we were to do that frequently, we clearly do not have the resources (human and material) to test the cross product of combinations.
And while making sure the compiler is stable and conforming is a great use of time, trying to understand why it crashes when -funstable-ctad-alias is enabled and -funstable-ctad-inherited-ctrs is not (for example)… is just a waste of already spread-thin resources.

TLl;DR: I’m not keen on adding the flag either. Let’s spend some time to make sure unit tests cover our bases and if the patch has to be reverted once or twice… so be it.

3 Likes
8

I can confirm that users do not know this. The name “-Xclang” is completely innocuous, and doesn’t really raise warning bells to anyone using it.

In response to a problem arising from that misunderstanding, I once (a long time ago) proposed in D55150 to have clang’s -Xclang and -mllvm arguments produce a default-on warning message. The reaction to that proposal was not positive. I still think this is quite unfortunate.

9

That is my experience as well. While working at Coverity, I witnessed many uses of -Xclang in the field. We eventually had to resort to supporting such uses and adapting to inconsistent behavior across Clang versions.

I see I messed up one of the links I intended in my previous message. One of those was supposed to be for the Clang Modules documentation. See the “Using Prebuilt Modules” section and the many -cc1 invocations listed there. That documentation should be updated to use proper driver invocations.

1 Like
10

There is nothing more permanent than a temporary fix. :smiley:

True, quick fixes are much preferred, and it is our common practice. However, it also puts additional time pressure on patch authors. They need to address all upcoming issues promptly, or else their patch will be reverted.

I agree that this feature may not be considered a good example for a feature flag, given its narrow scope. However, its implementation is not trivial (the current patch is even large for an incomplete version). In my opinion, having a temporary flag is a beneficial approach to developing this feature incrementally. It enables faster and easier development and review iterations.

On the flip side, flags tend to just… reduce the pressure to fix bugs, as users can “just disable the flag”. We put C++17 conformance behind a flag years ago and no one is actively trying to fix that, getting us stuck in a local maximum, serving no one.

This was never my intention for this flag.
We’re eager to implement and use this feature (it has been a long-requested feature from our internal users). This means we will conduct experiments, tests and polish it until it is production-ready.

This fits neither. It’s a small feature on top of existing CTAD work, one that doesn’t affect existing code.
We routinely do larger changes to clang without flags. This is not a multi year effort, nor even a multi month effort.
We landed higher impact features this month, no flag.

The size of a feature is not the only determining factor, I think it is more about features that are still in development. I tend to be cautious on shipping an unfinished feature (as it doesn’t work very well for us).

TLl;DR: I’m not keen on adding the flag either. Let’s spend some time to make sure unit tests cover our bases and if the patch has to be reverted once or twice… so be it.

Based on the feedback received from the community, I will follow the suggestion.

It is worth mentioning that we seem to have differing understandings and expectations regarding -cc1 flags. I share the thought that these flags are unstable, and not visible to end users which would make them easier to maintain or modify. As Reid mentioned above, if this is not the case, we should have better clarifications, and find other ways for feature developments.

1 Like
11

This is a good point, even though I always assumed cc1 flags not to be reliable, practice definitely differs.

+1