[Bug 110678] New: Split "assert (a && b)" statements into "assert(a); assert(b)", for more precise diagnostics

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=110678

            Bug ID: 110678
           Summary: Split "assert (a && b)" statements into "assert(a);
                    assert(b)", for more precise diagnostics
           Product: xorg
           Version: unspecified
          Hardware: Other
                OS: All
            Status: NEW
          Keywords: patch
          Severity: enhancement
          Priority: low
         Component: Driver/intel
          Assignee: chris at chris-wilson.co.uk
          Reporter: adam_richter2004 at yahoo.com
        QA Contact: intel-gfx-bugs at lists.freedesktop.org
                CC: adam_richter2004 at yahoo.com

Created attachment 144271
  --> https://bugs.freedesktop.org/attachment.cgi?id=144271&action=edit
Patch to split assert(a && b) statements into assert(a) and assert(b) in git
version of xf86-video-intel

Many thanks to Adam Jackson via Maarten Maathuis for redirecting me to submit
this patch here.

This patch separates each statement of the form assert(a && b) into "assert(a)"
and "assert(b)" statements.

If you will excuse the boilerplate language, here are some arguments in favor
of this practice.  Thanks for considering this patch.


1. Assertion failures are often sporadic, and users who report them may
   not be in a position to efficiently narrow them down further, so it
   is important to get as much precision from each assertion failure as
   possible.

2. It is a more efficient use of developers time when a bug report
   arrives if the problem has been narrowed down that much more.  A
   new bug report may initially attract more interest, so, if the
   problem has been narrowed down that much more, it may increase the
   chance that developers may invest the time to try to resolve the
   problem, and also reduce unnecessary traffic on the developer mailing
   list about possible causes of the bug that separating the assertion
   was able to rule out.

3. When using a debugger to step over an assertion failure in the
   first part of the statement, the second part is still tested.

4. Providing separate likelihood hints to the compiler in the form
   of separate assert statements does not require the compiler to
   be quite as smart to recognize that it should optimize both branches,
   although I do not know if that makes a difference for any compiler
   commonly used to compile X (that is, I suspect that they are all
   smart enough to realize is that "a && b" is likely true, then "a"
   is likely true and "b" is likely true).

5. In my humble opinion, separated assertions are almost always more
   readable, sometimes eliminating parentheses, often making
   references to the same values line up on the same columns, which
   can make range checks more obvious, and sometimes changing
   multi-line conditions to separate single line conditions, which can
   be recognized separately.

   Consider, for example, how columnization in this makes this range
   check more recongizable and makes it easier to recognize in a glance
   that it is a bounds check and what the bounds are:

                assert(reg.vstride >= 0 && reg.vstride <
ARRAY_SIZE(vstride_for_reg));
                        ... vs. ...

                assert(reg.vstride >= 0);
                assert(reg.vstride < ARRAY_SIZE(vstride_for_reg));

   A possible counter-argument to this might be that, in a sequence of
   assertions, can be informative to group related assertions together,
   which I think is true, but it is possible to get this other means, such
   as by using blank lines to separate express such grouping.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/intel-gfx-bugs/attachments/20190514/4db0a037/attachment.html>