anonymouspc wrote:
> I’m no SARIF expert, but as someone who has looked into nested diagnostics
> before, I feel like if we want to have nesting support (other than the very
> basic ‘notes nested within warnings/errors’), then we should probably start
> w/ adding support for that to the code that actually emits diagnostics (i.e.
> `DiagnosticsEngine` etc.)
>
> On the other hand if all this aims to do is to properly nest notes in
> errors/warnings (because I know that we’re not doing that at the moment even
> though SARIF supports it—or at least we weren’t a few months ago), then this
> candidly seems like a lot of new code and refactoring just for tbat, but
> admittedly I also haven’t looked into this too much wrt SARIF specifically.
Thanks very much for your code-review, and I wish you a very Happy New Year :).
Like you, I also believe that we should update the places where diagnostics are
constructed, rather than reconstructing a tree by replaying a linear diagnostic
stream.
However, I’ve found that this is currently difficult to do in Clang. To explain
why, let’s compare how GCC and Clang generate diagnostics:
- GCC
- GCC treats diagnostics as "groups".
- The stacktrace is like:
- `c-lex.cc: c_lex_with_flags()` founds a bad stray in program, calls
`error_at()`.
- `global_dc` adds param/location info into this diagnostic.
- `error_at()` constructs a RAII `auto_diagnostic_group`, and the
next-nesting notes are pushed into this group. **(<-- nesting happened here)**
- consumers calls `context::begin_group` and `context::end_group` to fetch
all the information in this group, and recursively calls
`context::(begin|end)_group` on every `m_diagnostics_groups.nesting_depth++`
- Clang
- Clang treats diagnostics as "stream".
- A stacktrace might be like:
- `Sema::CreateOverloadedBinOp()` try to create an operator'+' between
`int` and `std::vector<int>`.
- `for iterator : OverloadSet` try each candidate.
- if a candidate is ok (and best viable), return it.
- if no candidate is viable
- Directly call `CandidateSet.NoteCandidates()`
- For each candidate, use `Diag(...)` to directly print the candidate
note. **(<-- we do not have nesting information here, because this happens in a
plain Sema processing)**
Nesting information can only be obtained at the point where a diagnostic is
constructed — which always corresponds to an AST / Sema call-stack depth. Even
if we modify `DiagnosticsEngine`, we still wouldn’t have access to the precise
nesting structure. (But it seems that your previous unmerged PR #151234, which
introduces `DiagnosticsEngine::NestingLevelRAII`, will make that possible!)
I appreciate for your code-review and I will address the changes in the
upcoming new commits. Thank you!
https://github.com/llvm/llvm-project/pull/174106
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
