[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:208
+    if (UnaryOp->getOpcode() == UnaryOperator::Opcode::UO_AddrOf)
+      E = UnaryOp->getSubExpr();
+
----------------
samitolvanen wrote:
> rjmccall wrote:
> > samitolvanen wrote:
> > > rjmccall wrote:
> > > > It would be more general to allow any expression that we can constant-evaluate to a specific function / member function reference.  That allows callers to do stuff like `__builtin_function_start((int (A::*)() const) &A::x)` to resolve overloaded function references.
> > > > 
> > > > You should delay this check if the operand is value-dependent.
> > > > It would be more general to allow any expression that we can constant-evaluate to a specific function / member function reference.  That allows callers to do stuff like `__builtin_function_start((int (A::*)() const) &A::x)` to resolve overloaded function references.
> > > 
> > > I looked into using `Expr::EvaluateAsConstantExpr` here and while it works, I'm not sure if allowing arbitrary expressions as the argument provides any value. We can allow resolving overloaded function references without constant-evaluating the expression (and I added tests for this). Did you have any other use cases in mind where this might be useful?
> > I don't see what the advantage of limiting the constant expression would be if we can constant-evaluate it.  `switch` doesn't force you to make case values be integer literals and/or references to enumerators.  What are you trying to achieve with a restriction?
> > 
> > Not having arbitrary restrictions is particularly useful in C++, where templates and `constexpr` machinery can usefully do a lot of abstraction.
> > I don't see what the advantage of limiting the constant expression would be if we can constant-evaluate it.  `switch` doesn't force you to make case values be integer literals and/or references to enumerators.  What are you trying to achieve with a restriction?
> 
> I'm trying to understand the benefit of allowing arbitrary expressions like `__builtin_function_start(100 + a)`, which `EvaluateAsConstantExpr` is happy to evaluate into a reference to `a`.
> 
> I can obviously understand why these should be allowed for `switch`, but here we have a function whose sole purpose is to return the address of a function and I'm wondering why someone would want pass anything more complicated to it.
> 
> > Not having arbitrary restrictions is particularly useful in C++, where templates and `constexpr` machinery can usefully do a lot of abstraction.
> 
> Perhaps I'm just not that familiar with the C++ use cases here. Would you be able to provide me an example I could use as a test case?
> I can obviously understand why these should be allowed for switch, but here we have a function whose sole purpose is to return the address of a function and I'm wondering why someone would want pass anything more complicated to it.

The idea is that you might have a complicated way of picking which function you mean.  I agree that this should probably disallow non-zero offsets.

> Perhaps I'm just not that familiar with the C++ use cases here. Would you be able to provide me an example I could use as a test case?

A simple example: `fn` after `constexpr void (*fn)() = &foo;`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108479/new/

https://reviews.llvm.org/D108479