tlively added a comment.

In D102018#2743818 <https://reviews.llvm.org/D102018#2743818>, @aheejin wrote:

> So what prevented us from using functions when we were using `_Static_assert` 
> and why is it now possible to use functions?

From inside a function, the parameter never looks like a constant because the 
function body does not know how the function will be used. Since the 
static_asserts would have been statements on the function body, they would not 
have been able to assert that the arguments to the function were constant. We 
were using macros to copy the static_asserts to each call site so they could 
see the actual arguments. In contrast, it looks like the diagnose_if attribute 
is evaluated separately for each call site and is able to see the actual 
arguments.

>> The remaining macro intrinsics cannot be made into functions until the 
>> builtin
>> functions they are implemented with can be replaced with normal code patterns
>> because the builtin functions themselves require that their arguments are
>> constants.
>
> Why can't we also use `__REQUIRE_CONSTANT` there? Can't we call 
> `__REQUIRE_CONSTANT` before we call builtins within intrinsic functions?

The new __REQUIRE_CONSTANT would be able to enforce that the intrinsic is only 
called with constant arguments, but that information is not propagated to the 
builtin call in the intrinsic's body. From the builtin's point of view inside 
the function body, its arguments are function parameters, so they may not be 
constant. One fix for this would have been to remove the part of the builtin 
definitions that requires the arguments to be constants, but that seemed more 
hacky than just keeping the original macros for a little bit longer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102018

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

Reply via email to