> 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