[cfe-commits] r58866 - in /cfe/trunk: include/clang/AST/DeclBase.h include/clang/Basic/DiagnosticKinds.def lib/Parse/ParseDecl.cpp lib/Sema/SemaDecl.cpp test/SemaCXX/destructor.cpp

[Douglas Gregor]

On Nov 11, 2008, at 1:20 AM, Chris Lattner wrote:
> On Nov 7, 2008, at 2:02 PM, Argiris Kirtzidis wrote:
>> +++ cfe/trunk/lib/Parse/ParseDecl.cpp Fri Nov  7 16:02:30 2008
>> @@ -1355,6 +1355,7 @@
>> ///         conversion-function-id  [TODO]
>> ///          '~' class-name
>> ///         template-id             [TODO]
>> +///
>> void Parser::ParseDirectDeclarator(Declarator &D) {
>>  // Parse the first direct-declarator seen.
>>  if (Tok.is(tok::identifier) && D.mayHaveIdentifier()) {
>> @@ -1362,31 +1363,35 @@
>>    // Determine whether this identifier is a C++ constructor name or
>>    // a normal identifier.
>>    if (getLang().CPlusPlus &&
>> -        CurScope->isCXXClassScope() &&
>>        Actions.isCurrentClassName(*Tok.getIdentifierInfo(),
>> CurScope))
>>      D.SetConstructor(Actions.isTypeName(*Tok.getIdentifierInfo(),
>> CurScope),
>>                       Tok.getIdentifierInfo(), Tok.getLocation());
>>    else
>>      D.SetIdentifier(Tok.getIdentifierInfo(), Tok.getLocation());
>>    ConsumeToken();
>> +  } else if (getLang().CPlusPlus &&
>> +             Tok.is(tok::tilde) && D.mayHaveIdentifier()) {
>>    // This should be a C++ destructor.
>>    SourceLocation TildeLoc = ConsumeToken();
>> +    if (Tok.is(tok::identifier)) {
>> +      // Use the next identifier and "~" to form a name for the
>> +      // destructor. This is useful both for diagnostics and for
>> +      // correctness of the parser, since we use presence/absence
>> of the
>> +      // identifier to determine what we parsed.
>> +      // FIXME: We could end up with a template-id here, once we
>> parse
>> +      // templates, and will have to do something different to form
>> the
>> +      // name of the destructor.
>> +      IdentifierInfo *II = Tok.getIdentifierInfo();
>> +      II = &PP.getIdentifierTable().get(std::string("~") + II-
>>> getName());
>
> This is FIXME'd, but this should really be sped up.  Relying on
> std::string's for this is very slow.  This should use SmallString or
> something else similar.

Eh, this is my mess, not Argiris's.

> Stepping back one level, is cramming a ~ into an identifier really the
> right way to go here?  Maybe we should use a smart pointer of some
> kind instead of relying on IdentifierInfo*?  We will eventually need
> something richer like this to handle template-id's, right?


The benefit to cramming the '~' into the identifier is that, once  
we've done it, we get the right name for the destructor throughout the  
compiler because its identifier is a human-readable name. We're  
currently doing the same kind of hack with conversion functions, e.g.,  
"operator int const *", where we paste "operator" and then the type  
name to create the identifier for the declarator.

One possible fix would be to give the the Declarator a dummy name  
(e.g., "<destructor>", "<conversion function">) or make use of the  
flags that say that this isn't a normal named declarator, then make  
NamedDecl::getName() virtual and override it for destructors,  
conversion operators, etc., to form the name on-demand. Since  
getName() isn't invoked unless we're emitting a diagnostic--- 
definitely not in the hot path---it's probably a good tradeoff. We'll  
have to be a little careful throughough, since getIdentifier() and  
getName() will have different strings associated with them.

   - Doug