tra added inline comments.
================ Comment at: clang/include/clang/Sema/Sema.h:10407-10409 bool IsAllowedCUDACall(const FunctionDecl *Caller, const FunctionDecl *Callee) { + if (llvm::any_of(ExprEvalContexts, ---------------- hliao wrote: > tra wrote: > > One more thing. The idea of this function is that we're checking if the > > `Caller` is allowed to call the `Callee`. > > However here, you're checking the current context, which may not > > necessarily be the same as the caller's. I.e. someone could potentially > > call it way after the context is gone. > > > > Currently all uses of this function obtain the caller from `CurContext`, > > but if we start relying on other properties of the current context other > > than the caller function, then we may neet to pass the context explicitly, > > or only pass the Callee and check if it's callable from the current context. > > > > ``` > > > as the expression within `decltype` may be quite complicated, the idea here > is to relax that rule within `decltype` context, not only for a particular > pair of caller/callee. I understand the idea, but in this case the argument was more about the code style. Currently the contract is that the function's decision is derived from its arguments (and could, perhaps, be a static method). With this patch you start relying on the context, but it's not obvious from the function signature. Replacing Caller with context, or removing the caller altogether would bring the function signature closer to what the function does. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61458/new/ https://reviews.llvm.org/D61458 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits