[PATCH] D98086: [FileCheck] Fix numeric error propagation

[Joel E. Denny via Phabricator via llvm-commits]

jdenny added inline comments.


================
Comment at: llvm/test/FileCheck/match-time-error-propagation/invalid-excluded-pattern.txt:13
+RUN: echo > %t.chk \
+RUN:      'CHECK-NOT: [[#0x8000000000000000+0x8000000000000000]] [[UNDEF-VAR]]'
+RUN: echo > %t.in '10000000000000000'
----------------
thopre wrote:
> Did you intend to have two undefined variables (UNDEF and VAR)? Why not just UNDEFVAR? I don't think the parser looks anything after the minus sign.
Good catch.  I was trying to hyphenate.   I'll go with UNDEFVAR.

(BTW, the parser doesn't complain when UNDEF is defined.  It just ignores `-VAR`.  We should fix that.)



================
Comment at: llvm/test/FileCheck/match-time-error-propagation/matched-excluded-pattern.txt:10-18
+; TODO: Capturing from an excluded pattern probably shouldn't be permitted
+; because it seems useless: it's captured only if the pattern matches, but then
+; FileCheck fails.  The helpfulness of reporting overflow from that capture is
+; perhaps questionable then, but it doesn't seem harmful either.  Anyway, the
+; goal of this test is simply to exercise the error propagation mechanism for a
+; matched excluded pattern.  In the future, if we have a more interesting error
+; to exercise in that case, we should instead use it in this test, and then we
----------------
thopre wrote:
> FYI I'd like to make a patch to use APInt in FileCheck to allow numbers bigger than 64bits. I've found the need while working on __builtin_isnan for long double. No timeline and I don't expect to have time on it soon but that would remove the only post-match error unless a new one is added before that. On the other hand I haven't even started that work so I think this testcase should be kept.
Thanks for mentioning it.  Inequality constraints will fit here, right?  If so, hopefully they'll come first so it's easy to maintain this handling.


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

https://reviews.llvm.org/D98086