[cfe-dev] Written name of conversion operator

[Douglas Gregor]

On Jun 4, 2010, at 11:26 PM, Abramo Bagnara wrote:

> Il 05/06/2010 00:16, Douglas Gregor ha scritto:
>> 
>> 
>> Something like the latter, since we absolutely must make sure that the common case of (IdentifierInfo*, SourceLocation) doesn't require an extra memory allocation. I would probably end up creating something similar to DeclarationName itself, with a bit-mangled pointer for the name and storage for the first two locations.
>> 
>> 	struct DeclarationNameLoc {
>> 	  void *Ptr;
>> 	  SourceLocations Locs[2];
>> 	};
> 
> Following your guideline I've done a little case study (I've excluded
> some cases I don't know):
> 
> struct DeclarationNameLoc {
>  union {
>    struct {
>      // Already in NamedDecl, *MemberExpr or *DeclRefExpr
>      // SourceLocation NameLoc;
>    } Identifier;

Okay.

>    struct {
>      TypeSourceInfo* TInfo;
>    } CXXConstructorName;
>    struct {
>      // Already in NamedDecl, *MemberExpr or *DeclRefExpr
>      // SourceLocation TildeLoc;
>      TypeSourceInfo* TInfo;
>    } CXXDestructorName;
>    struct {
>      // Already in NamedDecl, *MemberExpr or *DeclRefExpr
>      // SourceLocation OperatorLoc;
>      TypeSourceInfo* TInfo;
>    } CXXConversionFunctionName;

Looks good.

>    struct {
>      // Already in NamedDecl, *MemberExpr or *DeclRefExpr
>      // SourceLocation OperatorLoc;
>      SourceLocation OpLoc;
>      SourceLocation LSquareLoc;
>      // Not so useful
>      // SourceLocation RSquareLoc;
>    } CXXOperatorName;

Since we'll have space for two locations here, I suggest having the location of the first token in the operator (e.g., "delete" in "operator delete [ ]") and the last token in the operator (e.g., "]" in "operator delete [ ]"). That way, we can get the range of the tokens, and from that form the full range of the name.

>    struct {
>      SourceLocation LQuoteLoc;
>      SourceLocation RQuoteLoc;
>      // Already in NamedDecl, *MemberExpr or *DeclRefExpr
>      // SourceLocation NameLoc;
>    } CXXLiteralOperatorName;

We can just store the location of the start of the string literal, then re-parse the literal if we need more detailed location information (which will be very, very rare).

>    struct {
>      // ??
>    } CXXUsingDirective;

Don't worry about this one; it's a hack in DeclarationName that doesn't need any representation in DeclarationNameLoc. 

>    struct {
>      // ??
>    } ObjCZeroArgSelector;
>    struct {
>      // ??
>    } ObjCOneArgSelector;
>    struct {
>      // ??
>    } ObjCMultiArgSelector;
>  };

Unless you're interested in Objective-C, you don't have to worry about this one. Just use a single SourceLocation that will point to the first identifier in the select, and that'll be fine.

What I'd like to do here is to keep the source locations of the identifiers in the selector: for selectors with 0 or 1 arguments, we'll just have a single SourceLocation. For selectors with 2 arguments, we'll have two SourceLocations. For selectors with > 2 arguments, we'll have a pointer to an array SourceLocations containing the locations of all of the identifiers in the selector. This affects ObjCMethodDecls and ObjCMessageExprs, and possibly a few other places. If you're interesting in Objective-C and want to implement this, that would be great; if not, I'll do it once we have a basic DeclarationNameLoc in place.

> };
> 
> The kind of info contained is identified using the available
> DeclarationName.

Sure.

> For the kinds examined we never need to add indirection to extra memory
> and the space needed is at most 8 bytes.

Right. I consider that acceptable for a (big!) improvement in source-location information. Only Objective-C selectors will need external storage (and that's not the common case).

> I believe that DeclarationNameLoc should be added to:
> 
> FunctionDecl
> DeclRefExpr
> DependentScopeDeclRefExpr
> CXXDependentScopeMemberExpr
> MemberExpr
> UnresolvedMemberExpr

Agreed. They don't all have to happen in the same commit :)

> (also it seems there is some intersection between DeclarationNameLoc and
> PseudoDestructorExpr)

PseudoDestructorExpr is probably fine as-is.

> We might add to these classes a method like:
> 
> DeclarationName CLASS::getDeclarationNameLocs(SourceLocation& MainLoc,
> DeclarationNameLoc& Locs);
> 
> to query for complete info.


It would be nice if the thing we call DeclarationNameLoc could bundle the DeclarationName and extra location information together, so that it could provide convenience functions like getSourceRange() (covering the entire name), getNameAsString(), and so on. I suggest that you rename the class that you have currently named DeclarationNameLoc to StoredDeclarationNameLoc (or something like it), then have the function

  DeclarationNameLoc CLASS::getDeclNameLoc();

that gets one of these easy-to-use DeclarationNameLoc classes, and is likely to be used in many places where the AST is manipulated with source-location information.

DeclarationNameLoc might also become the primary storage mechanism for the expressions, rather than having separate DeclarationName/SourceLocation/StoredDeclarationNameLoc members.

	- Doug