[cfe-dev] UnqualifiedId copy-constructor fixme

[Douglas Gregor]

On Jan 19, 2010, at 1:08 PM, Erik Verbruggen wrote:

> Hello,
> 
> While digging through DeclSpec.h, I noticed that the UnqualifiedId class has a private assignment operator, but a public (and implemented) copy-constructor with a big FIXME:
> 
> /// \brief Do not use this copy constructor. It is temporary, and only
> /// exists because we are holding FieldDeclarators in a SmallVector when we
> /// don't actually need them.
> ///
> /// FIXME: Kill this copy constructor.
> 
> Now it looks like the only place where this is used, is actually Parser::ParseObjCMethodDecl where it is used to hold the parameter declarations in the optional parameter list (line 860 in rev. 93892). Quite easy to kill. However, on line 874 of that file, those parameters are passed to the ActOnMethodDeclaration action callback, which might do something with them... (I guess that when you'd use clang to refactor parameter names, you'd need to use this method to find them).

Very strange that we're dropping these parameters in semantic analysis. At the very least, they should be stored in the AST so that, as you mentioned, refactoring can find them.

> So, is this copy constructor still eligible for "simple" removal? If so, the attached patch will do that. Or should the parameter declarations still be passed to ActOnMethodDeclaration, but with a different (non-copying) data-structure?


The parameter declarations should still be passed to ActOnMethodDeclaration, so we can't just go with this "simple" removal. The right answer is probably to actually build ParmVarDecls for those parameters (using ActOnParamDeclarator) and then passing the resulting DeclTy's to ActOnMethodDeclaration, which can store them in the method declaration.

	- Doug