[PATCH] D132513: [lit] Implement DEFINE and REDEFINE directives

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

Thanks for implementing this! The design looks good to me.
I think it is powerful enough for my impending use case which is quite straightforward to reason about, without making the lit domain specific language too complex.

>> One question I do have: why do we need both DEFINE and REDEFINE? I personally would think that just DEFINE would be enough, with it implicitly replacing any existing substitution. Or is it something to do with substitution order?
>
> I agree that DEFINE would be enough for the core functionality. However, I find that debugging complex substitution expansions can sometimes be painful. I split DEFINE into two directives in order to improve readability and facilitate debugging, as follows.

Having the DEFINE/REDEFINE distinction looks good to me. If a substitution is defined in multiple places, knowing where they are redefined helps (like variable declarations and assignments. Python/Ruby made it difficult.)



================
Comment at: llvm/utils/lit/tests/Inputs/shtest-define/errors/assignment/before-name.txt:8
+# CHECK: Unresolved: 1
+
----------------
delete blank line


================
Comment at: llvm/utils/lit/tests/Inputs/shtest-define/errors/assignment/braces-empty.txt:1
+# DEFINE:%{}=value
+# RUN: echo %{}
----------------
space after `DEFINE:`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132513