On Thu, 25 Nov 2021, Michael Matz wrote: > Hello, > > On Thu, 25 Nov 2021, Richard Biener via Gcc-patches wrote: > > > It seems to be a style to place gcc_unreachable () after a > > switch that handles all cases with every case returning. > > Those are unreachable (well, yes!), so they will be elided > > at CFG construction time and the middle-end will place > > another __builtin_unreachable "after" them to note the > > path doesn't lead to a return when the function is not declared > > void. > > > > So IMHO those explicit gcc_unreachable () serve no purpose, > > if they could be replaced by a comment. > > Never document in comments what you can document in code (IMO). I think > the code as-is clearly documents the invariants and expectations and > removing the gcc_unreachable() leads to worse sources. > > Can't you simply exempt warning on unreachable __builtin_unreachable()? > It seems an obvious thing that the warning should _not_ warn about, after > all, quite clearly, the author is aware of that being unreachable, it says > so, right there.
gcc_unreachable () is not actually __builtin_unreachable () but instead fancy_abort (__FILE__, __LINE__, __FUNCTION__). Yes, I agree that the warning shouldn't warn about "this is unrechable", but if it's not plain __builtin_unreachable () then we'd need a new function attribute on it which in this particular case means an alternate "fancy_abort" since in general fancy_aborts are of course reachable. We could also handle all noreturn calls this way and not diagnose those if they are unreachable in exchange for some false negatives. Btw, I don't agree with "Never document in comments what you can document in code" in this case, but I take it as a hint that removing gcc_unreachable in those cases should at least leave a comment in there? Richard.
