[RFC] Improving C++ Standard filtering in the lit tests

Runtimes C++
1

In libc++ the test suite supports all C++ language versions. Tests have filters which to determine which language versions they are not available in. For example, a feature that is introduced in C++26 will be filtered like

// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20, c++23

This list grows with every released C++ Standard. A real issue is that some features added to C++ are deprecated and eventually removed. In libc++ we use annotations for deprecation and test these. For example a feature deprecated in C++17 and removed in C++26 looks like

// UNSUPPORTED: c++03, c++11, c++14, c++26

This test will break once we add support for C++29. Then we need to manually update the test to

// UNSUPPORTED: c++03, c++11, c++14, c++26, c++29

In the libc++ header code C++ language versions are guarded with #ifdef’s like

#if _LIBCPP_STD_VER <= 17

or

#if _LIBCPP_STD_VER >= 17

I propose to add allow a similar syntax in lit. This means the first example becomes

// UNSUPPORTED: <=c++23

and the second example becomes

// UNSUPPORTED: <=c++14, >=c++26

If we decide we like this feature I would propose to write new tests in a different way. To me, the examples above require mental gymnastics. For the first example I propose we write

// REQUIRES: >=c++26

The second example needs mental gymnastics to determine the feature was deprecated in C++17 and removed in C++26, I propose to simplify this to

// REQUIRES: >=c++17
// UNSUPPORTED: >=c++26

I feel the latter two examples are much easier to understand and thus to write and to teach to new new contributors.

Note the REQUIRES syntax is already supported in lit. At the moment, we can’t use it for language filtering since it requires updating tests when a new C++ Standard is introduced. The new syntax >=c++xy does not have this limitation.

The proposal is to add these features, but not to do a mass conversion, I only want to convert the deprecation/removal tests, since they will break with C++29.

Since the characters < and > are not supported by lit, this requires a minor change in lit.

The implementation works by adding several new feature flags in the tests,
<=c++03 … <=c++26 and >=c++03 … >= c++26. It would be possible to add <c++03 … <c++26 and >c++03 … >c++26 too. The motivation not to do this is that I prefer to have a uniform syntax in our tests. So always use

// REQUIRES: >=c++26

Adding >c++23 would allow

// REQUIRES: >c++23

By not adding >c++23 this syntax won’t work and we guide developers to do the right thing.

Implementing this is quite trivial; a proof-of-concept is available. I’ve tested the number of executed tests remains stable before and after the change. (This patch needs work before being reviewable, including adding documentation.)

2 Likes
2

I’m very much in favour of this. It would make the version requirements so much simpler. I don’t think we need <=c++03 and >=c++03 though. The first is equivalent to c++03, and the second one is always true.

P.S. If a feature is removed in some version we can just write REQUIRES: c++17, c++20, c++23. No need to update tests every time we add a new version.

3

Thanks for your feedback!

True. I want to look at generating these values instead of hard-coding them. In that case I might want to not add special rules for these.

Good point! Since we rarely use REQUIRES it didn’t occur to me when I wrote that specific patch.

4

  1. Proposed changes to core lit (which this is, because it involves expression evaluation) should be posted more centrally, maybe to LLVM Project.

  2. I’m thinking that setting a single dialect feature to true (e.g. c++17) is not ideal. A given dialect should support all prior features that have not been removed. I’d suggest that a test run for C++17 should also set the features for all prior versions (c++03, c++11, c++14, c++17). Then for a feature added in C++11, deprecated in C++17, and removed in C++26, the test would have REQUIRES: c++11 && !c++26. This can be implemented with no changes to lit itself. Supporting new versions would not require any changes to existing tests of removed features.

AFAICT the proof of concept modifies lit to accept angle brackets but doesn’t do anything to evaluate them? I admit it has been a while since I looked deeply at lit.

5

Thanks for your feedback!

I deemed that change small enough for a normal review. Do you feel that change really RFC worthy?

This would be a huge change for the existing libc++ lit tests. I don’t know whether this breaks existing test, if we want to move in this direction I can test that.This change basically changes c++17 to mean <=c++17. It would change the syntax for a typical test for a feature introduced to C++20 to

REQUIRES: !c++17 && c++20

I’m not convinced this syntax helps maintainability.

Lit does not need to do anything to evaluate them. Basically this changes
# identifier :: [-+=._a-zA-Z0-9]+
to
# identifier :: [-+<=>._a-zA-Z0-9]+
The libc++ lit suite adds the new '>=c++xy` features and the “main” lit just needs to evaluate them.

6

Oh, changing the syntax of feature identifiers I didn’t catch that before. The syntax made me think you were doing integer comparisons on the numeric part of the name, or something. Yes, that’s a simple enough change to lit.

No, it would just be REQUIRES: c++20.
My c++17 would, in your syntax, mean >=c++17 not <=c++17. Because I’m using REQUIRES not UNSUPPORTED.

I don’t see that your proposal is simpler. You suggest changing a new-in-C++26 test, currently
UNSUPPORTED: c++03, c++11, c++14, c++17, c++20, c++23
to
UNSUPPORTED: <=c++23
while my suggestion changes it to
REQUIRES: c++26
and I don’t know about you, but I find the positive requirement easier to understand. The first one is “not supported up through and including C++23” which just reads a little weirdly to me. The second one is “supported in C++26 and later.” which IMO is clearer. Both tactics change the same number of actual test lines and preserve the doesn’t-change-with-every-new-standard requirement.

It is true that my suggestion would require a big-bang change to all the tests, because c++17 would suddenly mean >=c++17 and that would be a problem because you guys are using UNSUPPORTED instead of REQUIRES everywhere. So, on those grounds, the <= idea has appeal.

Semantically the two approaches are equivalent, so I don’t have any objection.

7

I put up this example since in libc++ we typically use negatives or double negatives.

This looks closer to my final proposal to replace UNSUPPORTED: <=c++23

So I indeed very much like to use more REQUIRES, I just don’t want to break backwards compatibility and possibly need to modify a lot of tests.