Brian Paul <bri...@vmware.com> writes: > On 12/10/2013 03:27 PM, Francisco Jerez wrote: >> Brian Paul <bri...@vmware.com> writes: >>[...] >>> I'm more convinced now that we should simply have an assert there rather >>> than unreachable(). >>> >>> Finally, in release builds we generally don't want to crash in these >>> situations. So IMHO, it's preferable to do something like: >>> >> I have the feeling that this is an orthogonal question to unreachable() >> aborting or not. I'm not convinced that there's a "sensible" default in >> this case, and setting one seems like trying to delay the inevitable to >> me. An invariant of the program has been broken and it's likely that >> something will go very wrong sooner or later. The question boils down >> to: do we want undefined behavior now, or later? I'd rather have it >> now, but it seems like a moot question to me... > > In the cases at hand, _mesa_tex_target_is_layered() and > _mesa_get_texture_layers(), returning false/0 seem like fairly > reasonable defaults to return and hopefully wouldn't lead to a later crash. > > Over the years I've seen several bug reports where someone runs their > app and sees a bunch of _mesa_problem() error messages but the app keeps > running and doesn't crash. I think that's preferable to just crashing > (esp. when Mesa may be running in the X server). That's the reason I'm > hesitant to see __builtin_unreachable() getting used in release builds > in this particular scenario. Having X crash just because we hit a > default switch case seems unnecessarily fragile. > Okay. I'll send an updated version of the texture layers patch in a minute taking into account your feed-back.
> My second concern is this: if someday someone really does want to use > __builtin_unreachable() in the context of unconventional flow control > (per gcc docs), will our new-and-improved unreachable() macro still be > the right tool for them? I worry about changing the behavior of what > seems to be an established convention. > Yes, I'm pretty sure it would still be, after all the unreachable macro doesn't have any effect other than eliminating compiler warnings [what we would still get from the call to abort] and increasing optimization opportunities [which are not a concern in debug builds]. > Sorry for dragging this out. Please go ahead and post a patch showing > what you have in mind. > I already did [1]. Would you mind reviewing it? [1] http://lists.freedesktop.org/archives/mesa-dev/2013-December/049902.html > -Brian
pgpRVgDpW755P.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
