> On 2017-Feb-20, at 13:11, Alex Lorenz via Phabricator 
> <revi...@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
> 
> 
> 

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

Reply via email to