On Thu, Apr 23, 2015 at 11:34 AM, Manuel López-Ibáñez <lopeziba...@gmail.com> wrote: > On 04/23/2015 05:12 PM, Jason Merrill wrote: >> >> On 04/20/2015 10:36 PM, Patrick Palka wrote: >> Implementation is pretty straightforward. The only catch is that the >> middle-end doesn't actually assume that REFERENCE_TYPEs are non-NULL so >> code like >> >> int &a = *(int *)0; >> if (&a != 0) >> >> will warn that &a will never be NULL yet the middle-end will fold the >> conditional to false instead of true anyway. But I guess that's not a >> big deal. > > > Is this actually correct? Is it because of undefined behavior?
I would think that the assignment is UB due to the null-pointer dereference but if it's not then we may have to fold the comparison to true always. According to section 8.3.2 of the C++11 standard: A reference shall be initialized to refer to a valid object or function. [ Note: in particular, a null reference cannot exist in a well-defined program, because the only way to create such a reference would be to bind it to the “object” obtained by dereferencing a null pointer, which causes undefined behavior. As described in 9.6, a reference cannot be bound directly to a bit-field. — end note ] So it looks like it's not incorrect to fold the comparison to false. > > It seems also weird we do not warn directly for '*(int *)0' in the C/C++ FE. That wouldn't be too hard to add probably. I'll take a look at this. > > >>> + if (decl_with_nonnull_addr_p (inner)) >> >> >> Using decl_with_nonnull_addr_p doesn't make sense for reference variables, >> since we're using their pointer value rather than their address. > > > Is an extra check needed at all (can &reference ever be false)? Consider it removed. Somehow I didn't catch the redundancy of this check.. > >> >>> + warning_at (location, >>> + OPT_Waddress, >>> + "the address of reference %qD may be assumed to " >>> + "always evaluate to %<true%>", >>> + inner); >> >> >> Perhaps "the compiler can assume that the address of reference %qD will >> always >> evaluate to %<true%>"? > > > The discussion (and perhaps the patch) at > https://gcc.gnu.org/bugzilla/PR65168 may be relevant. > > Jonathan suggests to match what we say for: > > /home/manuel/test.c:3:21: warning: the address of ‘i’ will always evaluate > as ‘true’ [-Waddress] > int i; bool b = !&i; > ^ > > I think this case requires a slightly different text because the address may > not evaluate to 'true' and also because it is not actually the address of > the reference but the object bounded to the reference. Clang says: OK. > > warning: reference cannot be bound to dereferenced null pointer in > well-defined C++ code; pointer may be assumed to always convert to true > [-Wundefined-bool-conversion] > > which is in my opinion even less clear. > > The testcases: > > int fii(int *p) { > int &r=*p; > return !&r; > } > > int fii(int p) { > int &r=p; > return !&r; > } > > should also generate the same warning in my opinion. Good idea. Thanks guys for the feedback. I'll post a new version of this patch in about 24 hours or so. So many issues for such a small patch!
