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.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
