This is an archive of the discontinued LLVM Phabricator instance.

[Fix] Make it an error to take the address of (most) enable_if functions.
ClosedPublic

Authored by george.burgess.iv on Oct 9 2015, 2:19 PM.

Details

Summary

For the following function:

int foo(int a) __attribute__((enable_if(a > 0, “”)));

The callee should be able to reasonably assume that foo will be called iff a > 0. However, this is currently not the case. One can grab the address of foo and use the resultant function pointer to call foo with any value of a.

Additionally, given a situation like:

int bar(int a) __attribute__((overloadable, enable_if(a > 0, “”)));
int bar(int a) __attribute__((overloadable));
int baz(int a) { return bar(a); }

One may only indirectly call bar by making a wrapper like baz, because there’s no way to disambiguate which bar you’re referring to in &bar.

This patch fixes both of these issues by making it an error to take the address of a function with an enable_if attribute, unless the condition in said attribute is always true.

Diff Detail

Repository
rL LLVM

Event Timeline

george.burgess.iv retitled this revision from to [Fix] Make it an error to take the address of (most) enable_if functions..
george.burgess.iv updated this object.
george.burgess.iv added a reviewer: rsmith.
george.burgess.iv added a subscriber: cfe-commits.
rsmith added inline comments.Oct 9 2015, 3:10 PM
lib/Sema/SemaExpr.cpp
10246–10253 ↗(On Diff #36990)

Converting the RHS to the type of the LHS seems to only be appropriate for simple assignment operators, not for arbitrary binary operators.

lib/Sema/SemaInit.cpp
4978–4990 ↗(On Diff #36990)

It would seem better to handle this in the assignment rules rather than duplicating it between here and the binary operator case.

george.burgess.iv marked 2 inline comments as done.

Addressed all feedback by rolling two of our overloading checks into one.

lib/Sema/SemaInit.cpp
4978–4990 ↗(On Diff #36990)

I tried that initially, and it failed. Turns out the error in my initial attempt was just me being dumb. :)

Thanks for catching that.

rsmith accepted this revision.Oct 12 2015, 11:40 AM
rsmith edited edge metadata.

LGTM

This revision is now accepted and ready to land.Oct 12 2015, 11:40 AM
This revision was automatically updated to reflect the committed changes.