[PATCH] D76114: [MC] Recalculate fragment offsets after relaxation

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added inline comments.


================
Comment at: llvm/test/MC/X86/relax-offset.s:15
+.Lint80_keep_stack:
+# CHECK-NOT: error: Assertion `OS.tell() - Start == Layout.getSectionAddressSize(Sec)' failed
----------------
MaskRay wrote:
> NOT pattern should be carefully used. Without a positive check with the same diagnostic, a negative check can easily get stale and don't check anything.
> 
> For this case (a regression test), an error will cause `llvm-mc` to return 1 instead of 0,
> 
> `llvm-mc -filetype=obj -triple=i386 %s -o /dev/null` is sufficient.
> NOT pattern should be carefully used. Without a positive check with the same diagnostic, a negative check can easily get stale and don't check anything.

Just to second this, and add my negative experiences with `CHECK-NOT`; make sure the test is red before your patch is applied, then green after it's applied.  Test Driven Development.  I too have seen CHECK-NOT's not actually test anything.

Also, @MaskRay , this check is for an assertion failure.  Is it common to test for asserts, which may not be enabled when running the tests in release mode?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76114