AaronBallman wrote:
> > In fact, we used to have guidance that only the very first instance of the
> > diagnostic should be spelled out fully, and all subsequent ones should be
> > limited to just the bare minimum needed to identify what the diagnostic is.
> > That's why there are hundreds of instances of things like // expected-note
> > {{here}}.
>
> Side note: In general I think the fact that these tests pass if the check
> implicitly match on a subset of the diagnostic does more harm than good.
+1, it's why I stopped recommending this approach in my reviews.
> > Writing tests is not a mechanical operation, it requires you to think about
> > what you need to test as well as think about how you test it. I think the
> > same is less true for things like testing LLVM IR because that tends to be
> > more routine pattern matching, which typically requires less stylistic
> > choices.
>
> Sometimes it is, sometimes it isn't. Even when it isn't just having the
> checks added to the test file in approximately the final location for me to
> move around is a massive boon compared to manually adding them all.
Sure, it's a time-saver when used as intended. I think the push-back is largely
coming from recognition that not everyone uses tools as they are intended and
we need to balance making it easy to update a large amount of tests (something
which occurs infrequently) against the risks of (even accidental) tool misuse.
> > Given the amount of push-back, I think we should probably revert the
> > changes here so we can discuss with a bit wider of an audience than weighed
> > in on the original PR.
>
> Yup, will revert.
Thank you!
https://github.com/llvm/llvm-project/pull/108658
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
