AaronBallman wrote: > > > It looks like this triggers on quite a few places in LLVM, at least in > > > our internal CI: > > > ``` > > > llvm/llvm-project/llvm/lib/Support/APFloat.cpp:4370](https://cs.corp.google.com/piper///depot/google3/llvm/llvm-project/llvm/lib/Support/APFloat.cpp?l=4370&ws=llvm-integration/4563832&snapshot=44):3: > > > error: calling a 'noreturn' function from a function with the 'pure' > > > attribute is undefined behavior [-Werror,-Wnoreturn-const-pure] > > > 4370 | llvm_unreachable("didn't find the set bit"); > > > | ^ > > > [llvm/llvm-project/llvm/include/llvm/Support/ErrorHandling.h:169](https://cs.corp.google.com/piper///depot/google3/llvm/llvm-project/llvm/include/llvm/Support/ErrorHandling.h?l=169&ws=llvm-integration/4563832&snapshot=44):31: > > > note: expanded from macro 'llvm_unreachable' > > > 169 | #define llvm_unreachable(msg) LLVM_BUILTIN_UNREACHABLE > > > | ^ > > > [llvm/llvm-project/llvm/include/llvm/Support/Compiler.h:485](https://cs.corp.google.com/piper///depot/google3/llvm/llvm-project/llvm/include/llvm/Support/Compiler.h?l=485&ws=llvm-integration/4563832&snapshot=44):57: > > > note: expanded from macro 'LLVM_BUILTIN_UNREACHABLE' > > > 485 | # define LLVM_BUILTIN_UNREACHABLE __builtin_unreachable() > > > | ^ > > > [llvm/llvm-project/llvm/lib/Support/APFloat.cpp:4344](https://cs.corp.google.com/piper///depot/google3/llvm/llvm-project/llvm/lib/Support/APFloat.cpp?l=4344&ws=llvm-integration/4563832&snapshot=44):16: > > > note: function declared 'pure' here > > > 4344 | int IEEEFloat::getExactLog2Abs() const { > > > | ^ > > > 1 error generated. > > > ``` > > > > > > Thanks for pointing this out (odd that no post-commit bots have hit it yet. > > we do have `-Werror` bots). On the one hand, these are true positives. On > > the other hand, I wonder if `unreachable` markings should be exempt from > > triggering the diagnostic here. The point to an explicit unreachable is > > often to document that the code cannot be reached as opposed to a call to > > another non-returning function like `terminate()`. > > Ugh, except that won't help here because some of these calls resolve to: > > ``` > /// This function calls abort(), and prints the optional message to stderr. > /// Use the llvm_unreachable macro (that adds location info), instead of > /// calling this function directly. > [[noreturn]] LLVM_ABI void llvm_unreachable_internal(const char *msg = > nullptr, > const char *file = > nullptr, > unsigned line = 0); > ``` > > instead of `__builtin_unreachable` or the standard interface.
I could look to see if the function name contains `unreachable` on the assumption it's a function intended to mark an unreachable code path, but that still feels a bit too hacky to me. The other alternatives would be to remove the unreachables from the source code and keep the pure markings, or remove the pure markings and keep the unreachable. Neither of those approaches make me particularly happy though. I'll revert the changes, reopen the issue, and explain what the problem is. https://github.com/llvm/llvm-project/pull/206134 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
