[libcxx-commits] [PATCH] D154987: [lit] Implement PYTHON directive and config.prologue

Tue Jul 25 09:33:20 PDT 2023

jdenny added a comment.

In D154987#4531624 <https://reviews.llvm.org/D154987#4531624>, @Endill wrote:

> In D154987#4529228 <https://reviews.llvm.org/D154987#4529228>, @sammccall wrote:
>
>> Could the tests you're targeting be written as actual python files, with an appropriate library of operations? (Some of this library could of course be shared with the shell test implementation).
>
> I think it would accurate to say that the idea this patch implements was born out of use case I have for C++ defect report test suite.

Definitely.  Other feedback/experience for DEFINE, REDEFINE, function substitutions, and `%if` also contributed.

In D154987#4531641 <https://reviews.llvm.org/D154987#4531641>, @awarzynski wrote:

> I am also unsure whether this patch is the right direction for LIT.

Thanks for chiming in.

> One major advantage of LIT is that it is "constrained" and hence:
>
> - the number of "idioms" is rather limited, so it's relatively easy to switch between sub-projects and still be able to parse/contribute new tests,
> - while "creativity" is limited (it's not Python), it allows us to keep the tests relatively coherent.

I haven't found that to be true when test suites invent their own custom lit test "formats".  I admit I haven't investigated their usage generally throughout LLVM.  However, when I've come across one, I have found it to be a significant paradigm shift, and I have had to study the new format's implementation to grasp what the custom rules are.  In my estimation, the difficulty of understanding a lit shell test with PYTHON when you're used to understanding a lit shell test without PYTHON would be much smaller in comparison.  (To be clear, I'm not arguing for avoiding custom lit test formats or replacing them with PYTHON.  I'm just comparing the difficulty of understanding PYTHON with variations that already appear in lit test suites.)

>> That is, a lit with PYTHON should make tests easier to read and should make lit easier to maintain than a lit with its own evolving scripting language.
>
> I am not sure. It will give developers more power, but with extra power comes extra responsibility :) Also, this "power" will require people to understand LIT internals better, right?

What lit internals do you mean?  Do you mean it enables tests to directly access functions and symbols within lit's implementation?  This patch specifically avoids that.  Access to lit-provided features is via a carefully defined `lit` object in python.  Or do you mean it requires test authors to learn various complex subtleties of new user-visible lit features?  (I think people sometimes refer to those subtleties as lit internals.)  That's exactly a problem this patch is trying to avoid by having people use a widely used and well documented language like python instead of new lit scripting language features.

> That wouldn't be a bad thing, but I just want to highlight that there are some advantages of having a dedicated scripting language (which doesn't require one to understand how LIT is implemented).

I previously thought that too.  In practice, I don't think it's working out that way.

We tend to start off with a simple and familiar idea we're used to in other languages (recent examples: variables/macros, functions, if-then-else, for-each).  Then we try to force that into lit's existing shell language, and we find there are many lit-specific subtleties to handle, debate, and ultimately teach.  Then we start using the feature and find it doesn't adapt well to other use cases.  Then people say they regret it.  If we had a dedicated dream team for developing lit features, maybe we could design the perfect set of lit features that give LLVM developers just what they need without adding too much power.  In the real world, I think it's more likely we'll keep evolving a messy collection of features that many people dislike.  Instead, I say let's use python and get back to developing compilers.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154987/new/

https://reviews.llvm.org/D154987