Folks,
Over the past week or so, there's been some discussion about making extensive use of smart pointers in the parser (on clang-dev).
Since the change is pervasive (and influences the programming model), we discussed the topic yesterday (with Apple compiler engineers working on clang).
We concluded it makes sense to follow-through with the changes. Our only concern is performance: we don't want to degrade performance of parsing correct code. For example, parsing system headers remains a bottleneck. Since system headers don't contain invalid code, it's important this improvement not degrade performance. Once the change is complete, it's important we do "A/B comparisons" to measure the performance for parsing valid code.
Once we have the data, we can decide if the cost is worth the benefit. We don't anticipate a major slowdown, however given the scope of the change (and frequency of calling "move"), it's something we need to verify.
We'd like to thank Sebastian for tackling this project!
snaroff
steve naroff wrote:
Folks,
Over the past week or so, there's been some discussion about making extensive use of smart pointers in the parser (on clang-dev).
Since the change is pervasive (and influences the programming model), we discussed the topic yesterday (with Apple compiler engineers working on clang).
We concluded it makes sense to follow-through with the changes.
Great! 
Our only concern is performance: we don't want to degrade performance of parsing correct code. For example, parsing system headers remains a bottleneck. Since system headers don't contain invalid code, it's important this improvement not degrade performance. Once the change is complete, it's important we do "A/B comparisons" to measure the performance for parsing valid code.
The tricky part about A/B comparisons is that the changes are interleaved with other changes, which mask the performance cost of this particular change. In many projects, this would be handled by branching for the interface change, with updates from trunk being regularly merged to the branch, and then comparing performance of trunk and branch. With clang's development model, this is not possible; A/B comparisons would have to be performed by carefully creating a version with the relevant changes removed. However, since the basic groundwork is already in trunk, its facilities are being used in other commits, too - see Doug's recent template support commit. This changes the separation from a difficult task into an impossible one.
Sebastian
We think we can drop in some #ifdefs that make the smart pointers just act like raw pointers (eliminating the destructor, removing the pointer to the Action class, etc.) to get a feel for the performance differences.
- Doug
Ran into this issue when looking at encoding of methods:
@interface Test
-(id) test3: (Test [3][4])b;
@end
@implementation Test
-(id) test3: (Test [3][4])b { }
@end
We do type-decay on arrays too early. So, encoding of the method type is not
same as gcc's. I also saw this note in clang:
// Perform the default function/array conversion (C99 6.7.5.3p[7,8]).
// Doing the promotion here has a win and a loss. The win is the type for
// both Decl's and DeclRefExpr's will match (a convenient invariant for the
// code generator). The loss is the orginal type isn't preserved. For example:
//
// void func(int parmvardecl[5]) { // convert "int [5]" to "int *"
// int blockvardecl[5];
// sizeof(parmvardecl); // size == 4
// sizeof(blockvardecl); // size == 20
// }
//
// For expressions, all implicit conversions are captured using the
// ImplicitCastExpr AST node (we have no such mechanism for Decl's).
//
// FIXME: If a source translation tool needs to see the original type, then
// we need to consider storing both types (in ParmVarDecl)...
//
Should we do what FIXME suggests (for objc method params only). Or should we
do type-decay lazily. latter saves us a slot at the expense of more wide-spread change.
My preference is for the FIXME approach. But I know how sensitive we are to adding a
slot to AST node. :).
- Fariborz
Ran into this issue when looking at encoding of methods:
@interface Test
-(id) test3: (Test [3][4])b;
@end
@implementation Test
-(id) test3: (Test [3][4])b { }
@end
We do type-decay on arrays too early. So, encoding of the method type is not
same as gcc’s. I also saw this note in clang:
// Perform the default function/array conversion (C99 6.7.5.3p[7,8]).
// Doing the promotion here has a win and a loss. The win is the type for
// both Decl’s and DeclRefExpr’s will match (a convenient invariant for the
// code generator). The loss is the orginal type isn’t preserved. For example:
//
// void func(int parmvardecl[5]) { // convert “int [5]” to “int *”
// int blockvardecl[5];
// sizeof(parmvardecl); // size == 4
// sizeof(blockvardecl); // size == 20
// }
//
// For expressions, all implicit conversions are captured using the
// ImplicitCastExpr AST node (we have no such mechanism for Decl’s).
//
// FIXME: If a source translation tool needs to see the original type, then
// we need to consider storing both types (in ParmVarDecl)…
//
Yep, that’s my comment/FIXME.
Should we do what FIXME suggests (for objc method params only). Or should we
do type-decay lazily. latter saves us a slot at the expense of more wide-spread change.
My preference is for the FIXME approach. But I know how sensitive we are to adding a
slot to AST node. :).
Ah yes, GCC’s wasteful tree nodes caused me extreme trauma as a young lad:-)
As you say, my preference is for the lazy approach. When I wrote this code, I was clearly too “lazy” to do it. Without some research, it’s hard for me to estimate the difficulty. Since I didn’t really need to original type, I simply added the FIXME.
A variation on the FIXME is to have a subclass of ParmVarDecl (e.g. ObjCParmVarDecl) that contains the original type (to support method encoding). This is still wasteful, however the waste is isolated to ObjC. Note that C++ is already wasting space (with the DefaultArg slot). Conceptually, I like the idea of having ObjC and C++ subclasses of ParmVarDecl.
What do you think?
snaroff
I'm not a fan of having separate ObjC/C++ subclasses for Decls unless there are differences in the data we need to store in the AST. In this case, the difference isn't whether we're in Objective-C or not; it's whether or not the original type is different from the converted type. So, why not have ParmVarWithOriginalTypeDecl be a subclass of ParmVarDecl, and create ParmVarWithOriginalTypeDecls whenever we've actually performed some kind of promotion of the declaration's type? That will capture all of the information in the source code while not special-casing Objective-C or introducing an extra slot into ParmVarDecls that aren't promoted.
I've been meaning to do the same thing with FieldDecl, by creating a subclass BitFieldDecl that's has the extra Stmt* containing the bitfield size. The FieldDecl class's interface won't change... but it will perform a dyn_cast<BitFieldDecl> in FieldDecl::getBitWidth(), so that clients of FieldDecl's won't ever have to know about BitFieldDecl.
- Doug
A variation on the FIXME is to have a subclass of ParmVarDecl (e.g. ObjCParmVarDecl) that contains the original type (to support method encoding). This is still wasteful, however the waste is isolated to ObjC. Note that C++ is already wasting space (with the DefaultArg slot). Conceptually, I like the idea of having ObjC and C++ subclasses of ParmVarDecl.
I'm not a fan of having separate ObjC/C++ subclasses for Decls unless there are differences in the data we need to store in the AST.
This is true for C++, but that's a separate issue I guess.
In this case, the difference isn't whether we're in Objective-C or not; it's whether or not the original type is different from the converted type.
I suppose, but ObjC is the only language that currently cares about the original type (so it can generate meta data consistent with the source).
So, why not have ParmVarWithOriginalTypeDecl be a subclass of ParmVarDecl, and create ParmVarWithOriginalTypeDecls whenever we've actually performed some kind of promotion of the declaration's type? That will capture all of the information in the source code while not special-casing Objective-C or introducing an extra slot into ParmVarDecls that aren't promoted.
From a space perspective, I really like that idea! (and it's straightforward to implement). Conceptually it's a little odd, but that's not very relevant/important...
snaroff
A variation on the FIXME is to have a subclass of ParmVarDecl (e.g. ObjCParmVarDecl) that contains the original type (to support method encoding). This is still wasteful, however the waste is isolated to ObjC. Note that C++ is already wasting space (with the DefaultArg slot). Conceptually, I like the idea of having ObjC and C++ subclasses of ParmVarDecl.
I'm not a fan of having separate ObjC/C++ subclasses for Decls unless there are differences in the data we need to store in the AST. In this case, the difference isn't whether we're in Objective-C or not; it's whether or not the original type is different from the converted type. So, why not have ParmVarWithOriginalTypeDecl be a subclass of ParmVarDecl, and create ParmVarWithOriginalTypeDecls whenever we've actually performed some kind of promotion of the declaration's type? That will capture all of the information in the source code while not special-casing Objective-C or introducing an extra slot into ParmVarDecls that aren't promoted.
I've been meaning to do the same thing with FieldDecl, by creating a subclass BitFieldDecl that's has the extra Stmt* containing the bitfield size. The FieldDecl class's interface won't change... but it will perform a dyn_cast<BitFieldDecl> in FieldDecl::getBitWidth(), so that clients of FieldDecl's won't ever have to know about BitFieldDecl.
Does't c++ mangles the decayed type or the original type (like Objc)? If the latter, then the same issue exists for c++ as well.
In any case, seems like the ParmVarWithOriginalTypeDecls is the way to go.
- Fariborz
C++ mangles the decayed type, since we need to mangle the signature of the function (C++ [defns.signature]; the C++0x version of this paragraph is far clearer about this requirement than the C++03 version).
Besides, the type used for mangling may still be different from the decayed type of the function parameter. For example, given:
void f(const int x) { }
The type of the function is void(int), but the type of the ParmVarDecl is "const int" (so x can't be modified in the body).
- Doug