samitolvanen added inline comments.

================
Comment at: clang/lib/Sema/SemaChecking.cpp:208
+    if (UnaryOp->getOpcode() == UnaryOperator::Opcode::UO_AddrOf)
+      E = UnaryOp->getSubExpr();
+
----------------
rjmccall wrote:
> 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;`.
> > 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;`.

Thanks for the example. So, something like this should also work?
```
$ cat test.cc 
void a() {}
constexpr void (*fn)() = &a;
const void *p = __builtin_function_start(fn);
```

Looking at what happens after we evaluate the expression, `APValue` is an 
l-value containing a `VarDecl`:
```
VarDecl 0xe3427a8 <test.cc:2:1, col:27> col:18 referenced fn 'void (*const)()' 
constexpr cinit
```
Which means I have to look at `VarDecl::getEvaluatedValue` if I want to get the 
function declaration. Or should the evaluation in this case produce a 
`FunctionDecl` directly?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108479

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to