[Mlir-commits] [llvm] [mlir] [Support] Simplify the SourceMgrDiagnosticVerifier interface. NFC (PR #227567)

[Fangrui Song]

MaskRay wrote:

The feature is welcome, but in its current shape it costs much more review than it should.

- #233305–#233309 fix latent bugs in the verifier that #227565 already landed: octal/unchecked offsets, keying by buffer name, unscanned buffers, matching an already-matched directive, the highlight range. They have nothing to do with llvm-mc. Rebase them onto main and land them as standalone fixes, with unit tests in place of the llvm-mc lit tests. They are the best-formed part of the stack and shouldn't wait behind the feature.
- As noted on #227567 and #233698: decide the directive semantics instead of adding one defaulted constructor parameter per PR. If MLIR really needs different behavior, use one options struct.
- Several PRs change MLIR's `-verify-diagnostics` under a `[Support]` tag: prefix word-boundary matching, `:<col>`, preferring unmatched directives. mlir/docs/Diagnostics.md is not updated, and only the first PR had an MLIR reviewer.
- About half of the unittest cases (prefixes, duplicates, columns, invalid offsets, near miss, buffers added later) repeat llvm/test/tools/llvm-mc/verify/basic.s. Keep the unit tests for paths llvm-mc can't reach (`reportUnexpected=false`, unknown buffer, line 0).
- Most comments restate the next line ("Parse the optional ':<col>'.", "A directive that already matched a diagnostic is used up."). Please trim them. The `{{` pre-filter in the custom-prefix PR is an unmeasured optimization; drop it or move it to its own patch.

After that, the llvm-mc part could be three PRs: the interface change; directive options (prefixes, comment scoping, one match per directive); and `llvm-mc --verify` with columns plus the priv-invalid.s conversion.


https://github.com/llvm/llvm-project/pull/227567