[llvm] [llvm-lit] Resolve env subcommand required error (PR #98414)
James Henderson via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 12 00:44:25 PDT 2024
jh7370 wrote:
> @jh7370 To clarify the changes I made: Originally, when the tests were failing, there was a check for errors using the line `# CHECK: # error: command failed with exit status: {{.*}}`. However, since the tests have been modified and are now passing, I replaced that check with `# CHECK-NOT: {{^[^#]}}`. This change reflects the fact that the tests are no longer expecting an error, but rather verifying that no unexpected output is generated.
>
> Given this, I think it makes sense to maintain the original format for passing tests, which is `# CHECK-NOT: {{^[^#]}}`, instead of `# CHECK-NOT: # error:`. Does this approach make sense to you, or do you have any thoughts on changing this? If this makes sense, I can revert the last commit and use `# CHECK-NOT: {{^[^#]}}` consistently for the tests that pass. Please let me know if I should clarify anything further.
Just spelling out what the regex means, because I've tripped myself up at least twice trying to interpret it. `^` means "start of line". `[^#]` means "any character except `#`. Combined they mean "match a non-empty line that does not start with `#`". `CHECK-NOT` instructs FileCheck to fail if it sees the specified between the previous thing it matched and the next thing it matches, so the combined effect is to fail the test if, between the previous and next patterns, a non-empty line _without_ a leading `#` appears. In other words, it will NOT fail if it sees e.g. `# error:` in the output, because of the leading `#` (and based on the checks elsewhere in the file, I assume that is indeed the format that errors will be printed in). Could somebody confirm that I'm not making a mistake?
@Harini0924, if you didn't have any other tests to base things on, and you wanted to write this specific test completely from scratch, what would your CHECK lines look like? The way you've been replying, I feel like you don't really have a clear idea of what you are trying to achieve, which makes me nervous. On the other hand, maybe it's just a communication thing.
---
Unrelated, but "Resolve env subcommand required error" isn't really describing what this patch does in a useful manner. I think the key thing is that you're changing env without subcommands to print the environment variables, so I'd change this to "
[llvm-lit] Print environment variables when using env without subcommand".
https://github.com/llvm/llvm-project/pull/98414
More information about the llvm-commits
mailing list