[PATCH] D96462: [LV] Add remarks that explicitly mention error handling in candidate loops

[Florian Hahn via Phabricator via llvm-commits]

fhahn added reviewers: thegameg, anemet.
fhahn added a comment.

In D96462#2560783 <https://reviews.llvm.org/D96462#2560783>, @jdoerfert wrote:

> In D96462#2556207 <https://reviews.llvm.org/D96462#2556207>, @fhahn wrote:
>
>> Thanks for working on improving the remarks! Unfortunately we have to be careful to keep them generic with respect to the frontend. I don't think we should be  using a function name string to detect using `assert` and include other implementation specific parts (like `NDEBUG`) in the message. For example, the user could define their own `__assert_fail` function or the IR could be coming from a Rust or Swift frontend.
>
> I'd argue, whatever the frontend or producer is, something called `assert_fail` could be reasonable attributed to "an assertion". If it is implicit or not is a different story but it tells the user more than just: "No unique exit block". Especially if the code doesn't have an obvious side exit (but only an implicit or explicit assertion).

I think it's great you are working on making the remarks more user-friendly!

I'd just like to make sure it works well across a range of frontends. Assuming `__assert_fail` is an assertion in C/C++ is probably fine (unlikely to have many cases where it is not an assertion), but I am still reluctant to encode such frontend-specific logic here. I think a key question we should think about is how other frontends can opt-in to the same remark. I don't have a real suggestion for that at the moment, but if there's anything we can do other than hard-coding assertion function names for x languages here, that would be great.

Another interesting question is how to best present more context information to the user about remarks. Some of those remarks could benefit from slightly more context/information than can reasonably fit into the remark itself and some way to link to slightly more in-depth documentation would IMO be very valuable. I think such a place would also be slightly better suited to go into frontend/language specific details as well. We could also try to 'guess' the language (based on the file from `!dbg`) and point to language-specific info, if appropriate (e.g. in the `assertion` case)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96462