[PATCH] D96418: [clang] Refactor mustprogress handling, add it to all loops in c++11+.

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

> Well, no, I'm afraid it is actually clear that that code does have UB according to the C++ standard.  Perhaps you mean that it *shouldn't* have UB, or that Clang should define its behavior despite the standard.
>
> I might agree with you that I don't see the value in using this stronger rule in C++, but I think it would help to understand the problem a little better.  I assume this is causing problems for a less trivial test case?  Or do you really have code that's relying on that loop not terminating?

I see. I guess it's good (and bad) that this discovered this UB in our code base. The example I posted is almost exactly what's in our test case. For this particular one, we need a non-terminating thread, so a thread_create function is passed `do_nothing`. Locally, we could get around this by using something like inline assembly to avoid UB.

(Potentially deviating from this patch) While an infinite loop may be UB according to the standard, it's also something fairly common that can be in a lot of code bases. Although it may be an unintended side-effect, it still seems a bit abrupt that we found about this UB from this patch rather than ubsan or a compiler diagnostic. Are there any plans in the future for something along the lines of improving catching this type of UB? Or alternatively, are there plans of maybe defining it in clang (like a language extension) as @rjmccall points out?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96418