[PATCH] D27827: [ObjC] CodeGen support for @available on macOS

Duncan P. N. Exon Smith via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 20 13:24:43 PST 2017


> On 2017-Feb-20, at 13:11, Alex Lorenz via Phabricator <reviews at reviews.llvm.org> wrote:
> 
> arphaman added inline comments.
> 
> 
> ================
> Comment at: lib/CodeGen/CodeGenFunction.h:2479
> 
> +  llvm::Value *EmitObjCIsOSVersionAtLeast(ArrayRef<llvm::Value *> Args);
> +
> ----------------
> I think it's better to treat this as a builtin in its own right, without including the ObjC part in the names. This function could be renamed to something like `EmitBuiltinAvailable` and the variable that holds the function pointer should be renamed appropriately as well.
> 
> 
> ================
> Comment at: test/CodeGenObjC/availability-check.m:5
> +void use_at_available() {
> +  // CHECK: call i32 @_IsOSVersionAtLeast(i32 10, i32 12, i32 0)
> +  // CHECK-NEXT: icmp ne
> ----------------
> I think the function name should have the lowercase `is`

Keep in mind that we need this to be a reserved identifier, so it either has to start with two underscores or one underscore with an uppercase letter.  In other words, don't forget to add an underscore if you lowercase it.

> 
> 
> ================
> Comment at: test/CodeGenObjC/availability-check.m:7
> +  // CHECK-NEXT: icmp ne
> +  if (__builtin_available(macos 10.12, *))
> +    ;
> ----------------
> Can you add a call to the builtin that has a non-zero sub-minor version as well?
> 
> 
> https://reviews.llvm.org/D27827
> 
> 
> 



More information about the cfe-commits mailing list