This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PING] Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function


[CC Jason since the patch also touches the C++ front end]

The updated patch is here:
  https://gcc.gnu.org/ml/gcc-patches/2015-07/msg00258.html

Thanks
Martin

On 07/04/2015 04:32 PM, Martin Sebor wrote:
I don't think c_validate_addressable is a good name - given that it's
called for lots of things that aren't addressable, in contexts in which
there is no need for them to be addressable, and doesn't do checks of
addressability in contexts where they are actually needed and done
elsewhere (e.g. checks for not taking the address of a register
variable).
The question seems to be something more like: is the expression used
as an
operand something it's OK to use as an operand at all?

Thank you for the review.

I've changed the name to c_reject_gcc_builtin. If you would prefer
a different name still please suggest one. I'm not partial to any
one in particular.

What is the logic for the list of functions above being a complete
list of
the places that need changes?

The logic by which I arrived at the changes was by constructing
test cases exercising expressions where a function is a valid
operand and where its address might need to be obtained when
it's not available, and stepping through the code and modifying
it until I found a suitable place to change to reject it.

How can ifexp be of pointer type?  It's undergone truthvalue conversion
and should always be of type int at this point (but in any case, you
can't
refer to TREE_OPERAND (ifexp, 0) without knowing what sort of expression
it is).

I've removed the redundant test from this function.


+/* For EXPR that is an ADDR_EXPR or whose type is a FUNCTION_TYPE,
+   determines whether its operand can have its address taken issues
+   an error pointing to the location LOC.
+   Operands that cannot have their address taken are builtin functions
+   that have no library fallback (no other kinds of expressions are
+   considered).
+   Returns true when the expression can have its address taken and
+   false otherwise.  */

Apart from the naming issue, the comment says nothing about the semantics
of the function for an argument that's not of that form.

I've reworded the comment to hopefully make the semantics of
the function more clear.

Attached is an updated patch with these changes.

Martin


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]