[cfe-dev] Patch proposal for __builtin_offsetof construct.

[Roberto Amadini]

Douglas Gregor ha scritto:
> On Apr 16, 2010, at 3:35 AM, Roberto Amadini wrote:
>
>   
>> Hi,
>>
>> I'm the student who is trying to fix the problem mentioned in
>> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2010-February/007833.html .
>>     
[...]
>> Index: include/clang/AST/Expr.h
>> ===================================================================
>> --- include/clang/AST/Expr.h	(revisione 100962)
>> +++ include/clang/AST/Expr.h	(copia locale)
>> @@ -991,6 +985,78 @@
>>     
[...]
>> +public:
>> +  // __builtin_offsetof(type, identifier(.identifier|[expr])*)
>> +  // FIXME: Type-dependent case ignored.
>> +  struct OffsetOfNode {
>> +    SourceLocation LocStart, LocEnd;
>> +    bool isBrackets;  // true if [expr], false if .ident
>> +    union {
>> +      FieldDecl *MemberDecl;
>> +      Expr *IndexExpr;
>> +    } U;
>> +  };
>>     
>
> You could save a word of storage per OffsetOfNode by mangling the "isBrackets" bit into the lowest bit of the FieldDecl or Expr pointer. llvm::PointerUnion<FieldDecl *, Expr *> will do this for you.
>
> But, see below for more comments on this representation.
>   
In Order to have Expr*'s in contiguous memory, I replaced *IndexExpr
field with an unsigned integer ExprIndex, which represents the index of the
corresponding Expr* in the array of subscript expressions: in this way, 
I can't
use a PointerUnion, or not?
>> Index: lib/AST/StmtProfile.cpp
>> ===================================================================
>> --- lib/AST/StmtProfile.cpp	(revisione 100962)
>> +++ lib/AST/StmtProfile.cpp	(copia locale)
>> @@ -261,6 +261,10 @@
>>   ID.AddInteger(S->getOpcode());
>> }
>>
>> +void StmtProfiler::VisitOffsetOfExpr(OffsetOfExpr *S) {
>> +  VisitExpr(S);
>> +}
>> +
>>     
>
> To profile an OffsetOfExpr, we'll need to visit the type and all of the OffsetOfNodes as well. That way, we can distinguish between different OffsetOfExprs on the same base type.
>
>   
Ok, but I have a doubt: VisitExpr(S) visits recursively all the 
sub-expressions,
thus I think that I haven't to explicitly visit such expressions while 
I'm iterating on
OffsetOfNodes array for declarations visit... Or not?
> Overall, this is a great start on __builtin_offsetof, thanks! I look forward to seeing a revised patch.
>
> 	- Doug
>   
Thanks for your comments!  I tried to follow your advices...
I attached a new patch which refers to revision no. 102050.

Regards,
Roberto.
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: OffsetOfExpr0422.patch
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20100422/92105b9a/attachment.ksh>