[cfe-commits] [PATCH] llvm.annotation support

[John McCall]

On Sep 9, 2011, at 10:37 AM, Julien Lerouge wrote:
> On Mon, Sep 05, 2011 at 01:15:39PM -0700, John McCall wrote:
>> 
>> +  case Builtin::BI__builtin_annotation: {
>> +    llvm::Value *AnnVal = EmitScalarExpr(E->getArg(0));
>> +    llvm::Value *F = CGM.getIntrinsic(llvm::Intrinsic::annotation,
>> +                                      AnnVal->getType());
>> +
>> +    // Get the annotation string, go through casts.
>> +    const Expr *AnnotationStrExpr = E->getArg(1)->IgnoreParenCasts();
>> +    llvm::StringRef Str = cast<StringLiteral>(AnnotationStrExpr)->getString();
>> +    return RValue::get(EmitAnnotationCall(F, AnnVal, Str, E->getExprLoc()));
>> +  }
>> 
>> Please have this comment say something like "Sema requires this to be a non-wide string literal, potentially casted", so that it's clear why this cast<> will always succeed.
>> 
>> And then you should make that true by adding the appropriate case to Sema::CheckBuiltinFunctionCall. :)
> 
> Ok, the extra code is below. Thanks, for rewieving this! Let me know if
> there is anything wrong.

The logic looks fine.  Please make CheckBuiltinAnnotationString a static
global function instead of a member function of Sema;  I know you were
probably following the existing practice in that file, but it's honestly not
a very good practice. :)

Do you have commit access?  If not, just send me a complete patch
and I'll commit for you.

John.