rjmccall added a comment. In D108479#3140638 <https://reviews.llvm.org/D108479#3140638>, @samitolvanen wrote:
> In D108479#3133578 <https://reviews.llvm.org/D108479#3133578>, @rjmccall > wrote: > >> If we do need to support constant expressions of this > > Yes, we need this also in constant expressions. Okay. I assume just static initializers, and not things like template arguments? >> I think we should have the constant evaluator produce an expression rather >> than an l-value, the way we do with some other builtin calls. That should >> stop the comparison problem more generally. > > Sure, I can take a look at how that would work. Basically, in > `PointerExprEvaluator::VisitBuiltinCallExpr` we should not evaluate the > l-value and just leave it at `Result.set(E)`? Yes, exactly. Since the builtin already requires a constant operand in non-dependent contexts, that should be enough. ================ Comment at: clang/lib/Sema/SemaChecking.cpp:199 +/// Check that the argument to __builtin_function_start is a function. +static bool SemaBuiltinSymbolAddress(Sema &S, CallExpr *TheCall) { + if (checkArgCount(S, TheCall, 1)) ---------------- Please update the function name here. ================ Comment at: clang/lib/Sema/SemaChecking.cpp:208 + if (UnaryOp->getOpcode() == UnaryOperator::Opcode::UO_AddrOf) + E = UnaryOp->getSubExpr(); + ---------------- 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. ================ Comment at: clang/lib/Sema/SemaChecking.cpp:221 + if (ResultType.isNull()) + return true; + ---------------- I think we decided that the result type should be `void*`. Are there other semantic checks from `CheckAddressOfOperand` that still meaningfully apply? 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