- User Since
- Mar 14 2013, 3:16 PM (340 w, 3 d)
Fri, Sep 20
Thu, Sep 19
Wed, Sep 18
LGTM, thank you for this!
The review says you abandoned it -- was that accidental?
Tue, Sep 17
Thank you for working on this check!
Fri, Sep 13
Mostly LGTM but a few nits.
LGTM! Thank you for this refactoring -- it was large, but I think the results from it are really good.
Wed, Sep 11
Aside from a minor nit, this LGTM. However, I'm not the most familiar with how cross-compiling works in the first place, so I may be the wrong one to approve this.
Thank you for doing all this work -- in general, this is looking great and really cleans things up!
Tue, Sep 10
Sat, Sep 7
Fri, Sep 6
Aside from the missing documentation bit, I think this LG.
Thu, Sep 5
Wed, Sep 4
I share in the discomfort expressed for this attribute, but I don't have a better solution in mind just yet, so I'm giving some other review feedback in the meantime.
LGTM aside from a small nit. Thank you for this!
Tue, Sep 3
One fear I have with this is in expansions of the offsetof macro, where it is a common implementation strategy to cast a null pointer to be of the correct type when calculating member offsets. Do you think you will be able to distinguish between null pointer additions that the user wrote directly (which is UB) as opposed to null pointer additions that come from the implementation (which is not UB)?
Mon, Sep 2
Sun, Sep 1
Fri, Aug 30
LGTM, thank you for working on this!
LGTM aside from a nit with auto.
Generally LG to me as well
Thu, Aug 29
Wed, Aug 28
Aside from the few remaining nits, I think this is almost ready to go.
I'm wondering whether this should even warn pedantically. There are no format specifiers that apply directly to a _Bool datatype, so the user is left making a choice as to what specifier fits best. I think hhd and hhu are both equally reasonable types, as are the d and u groups directly -- I don't think we should warn on either of those specifiers with a _Bool argument. I could maybe see a case for pedantically diagnosing h, l, and ll prefixes with _Bool because that seems a bit type-confused to me. c as a specifier seems weird (given that false values will potentially terminate the string suddenly depending on terminal behavior, IIRC), but lc seems like type confusion again.
LGTM, but missing a test case.
Tue, Aug 27
Adding @rsmith to see if we can make forward progress on this patch again.
I'm not an LLVM person, but I just wanted to double-check: is this correct even in the face of situations where malloc() and friends return a non-null pointer that is *not* dereferenceable? e.g., when someone calls malloc(0) with certain libc implementations?