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

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In D132513#3802768 <https://reviews.llvm.org/D132513#3802768>, @jdenny wrote:

> @jhenderson: Thanks for the review!
>
> All: There has been no discussion so far on the directive name collisions, as described in the review summary:
>
>> One issue is that the strings `DEFINE:` and `REDEFINE:` already appear in 5 tests.  This patch adjusts those tests not to use those strings. Alternatively, maybe lit keywords from now on should start with `LIT-` to avoid collisions.  For this patch, we might use `LIT-DEF:` and `LIT-REDEF:`.
>
> If I don't hear any opinions on this within a couple days, I'll go ahead and land the patch with the current names (`DEFINE:` and `REDEFINE:`).  Thanks.

My personal opinion is that it's fine as long as there is pretty much zero risk of a downstream user running into spurious passes due to the change from a FileCheck directive to a lit command. In 99% of cases, I think it'll likely fail due to an invalid definition, so I think we're good, personally. (Plus strictly, `LIT-DEF` could already be in use downstream anyway).


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

https://reviews.llvm.org/D132513