jroelofs wrote:
oh, right. nvm
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
hnrklssn wrote:
> > The ; did you mean to use BAR? version was accidentally emitted in some
> > cases where it didn't make sense to do so, but the tests still passed
> > because the diagnostic did contain cannot use FOO in context asdf.
>
>
>
> There's a `FileCheck` flag to enforce this: `-
jroelofs wrote:
> The ; did you mean to use BAR? version was accidentally emitted in some
> cases where it didn't make sense to do so, but the tests still passed because
> the diagnostic did contain cannot use FOO in context asdf.
There's a `FileCheck` flag to enforce this: `--match-full-line
hnrklssn wrote:
> > >
> >
> >
> > +1, it's why I stopped recommending this approach in my reviews.
>
> While I agree for the most part, there are quite a few diagnostics that are
> absurdly verbose (particularly ones with "did you mean to FLOOP it?"). I
> think partial matches are good/fine
erichkeane wrote:
> >
>
> +1, it's why I stopped recommending this approach in my reviews.
While I agree for the most part, there are quite a few diagnostics that are
absurdly verbose (particularly ones with "did you mean to FLOOP it?"). I think
partial matches are good/fine, but they need
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
erichkeane wrote:
>I don't know the solution, but I think there is some inherent flaw when it
>comes to how -verify tests operate currently, in that you need to trust that
>the author formatted the test to accurately reflect reality.
In part why I push for bookmarks, it makes it clear on the o
hnrklssn 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 o
AaronBallman wrote:
> But for clang -verify you have to add checkers for exactly the emitted
> diagnostics. No more or less. So the only real question is on which line to
> place them.
As someone who does *a lot* of review in Clang, I (strongly) disagree with this
assertion.
You do not have
hnrklssn wrote:
> > But for clang -verify you have to add checkers for exactly the emitted
> > diagnostics. No more or less. So the only real question is on which line to
> > place them.
>
> I don't think it's that simple at least for some tests. We have tests that
> run in `-verify` mode mul
Endilll wrote:
> That was always true, in the sense that there's nothing preventing the author
> from doing the exact same thing: looking at the output from the clang -verify
> failure and adding/removing expected diagnostics until it matches. In fact,
> even without this script that is the mo
hnrklssn wrote:
> As a CFE reviewer, I very much hate the existence of this script (and the
> CodeGen equivalents, but that is a separate discussion).
>
> This existing means I can no longer trust that a test is a reflection of what
> the author intended to write, as they might have just run t
erichkeane wrote:
As a CFE reviewer, I very much hate the existence of this script (and the
CodeGen equivalents, but that is a separate discussion).
This existing means I can no longer trust that a test is a reflection of what
the author intended to write, as they might have just run this and
hnrklssn wrote:
> Hi, `--strip-trailing-cr` isn't a supported option with diff on AIX. Would
> you be able to adapt the tests?
>
> https://lab.llvm.org/buildbot/#/builders/64/builds/985
This should fix it: https://github.com/llvm/llvm-project/pull/108871
https://github.com/llvm/llvm-project/p
jakeegan wrote:
Hi, `--strip-trailing-cr` isn't a supported option with diff on AIX. Would you
be able to adapt the tests?
https://lab.llvm.org/buildbot/#/builders/64/builds/985
https://github.com/llvm/llvm-project/pull/108658
___
cfe-commits mailing
https://github.com/hnrklssn closed
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
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Henrik G. Olsson (hnrklssn)
Changes
This relands commit d4f41befb7256f8e8378ae358b2b3d802454d6a4 which was reverted
by b7e585b95e241d0506b6f71d53ff5b6e72a9c8f4.
This version ignores differences in line endings in the diff tests to make su
https://github.com/hnrklssn created
https://github.com/llvm/llvm-project/pull/108658
This relands commit d4f41befb7256f8e8378ae358b2b3d802454d6a4 which was reverted
by b7e585b95e241d0506b6f71d53ff5b6e72a9c8f4.
This version ignores differences in line endings in the diff tests to make sure
the
18 matches
Mail list logo
