[cfe-commits] r64153 - in /cfe/trunk: include/clang/AST/ASTContext.h include/clang/AST/DeclTemplate.h include/clang/AST/Type.h include/clang/Parse/Action.h include/clang/Parse/Parser.h lib/AST/ASTContext.cpp lib/AST/DeclTemplate.cpp lib/AST/Type.cpp lib/AST/TypeSerialization.cpp lib/CodeGen/CodeGenTypes.cpp lib/Parse/MinimalAction.cpp lib/Parse/ParseDecl.cpp lib/Parse/ParseExpr.cpp lib/Parse/ParseTemplate.cpp lib/Parse/Parser.cpp lib/Sema/Sema.h lib/Sema/SemaTemplate.cpp test/SemaTemplate/class-template-id.cpp

[Douglas Gregor]

On Feb 9, 2009, at 12:05 PM, Sebastian Redl wrote:

> Douglas Gregor wrote:
>> Author: dgregor
>> Date: Mon Feb  9 12:46:07 2009
>> New Revision: 64153
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=64153&view=rev
>> Log:
>> Start processing template-ids as types when the template-name refers
>> to a class template. For example, the template-id 'vector<int>' now
>> has a nice, sugary type in the type system. What we can do now:
>>
>>  - Parse template-ids like 'vector<int>' (where 'vector' names a
>>    class template) and form proper types for them in the type system.
>>  - Parse icky template-ids like 'A<5>' and 'A<(5 > 0)>' properly,
>>    using (sadly) a bool in the parser to tell it whether '>' should
>>    be treated as an operator or not.
>>
> I suppose it can't be helped.

The only other option I can think of would be to pass it as a boolean  
flag down the stack... that's almost worse :)

>> Modified: cfe/trunk/include/clang/Parse/Parser.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=64153&r1=64152&r2=64153&view=diff
>>
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> =====================================================================
>> --- cfe/trunk/include/clang/Parse/Parser.h (original)
>> +++ cfe/trunk/include/clang/Parse/Parser.h Mon Feb  9 12:46:07 2009
>> @@ -58,6 +58,44 @@
>>
>>   PragmaHandler *PackHandler;
>>
>> +  /// Whether the '>' token acts as an operator or not. This will be
>> +  /// true except when we are parsing an expression within a C++
>> +  /// template argument list, where the '>' closes the template
>> +  /// argument list.
>> +  bool GreaterThanIsOperator;
>> +
>> +  /// \brief RAII object that makes '>' behave like the closing  
>> angle
>> +  /// bracket for a template argument list.
>> +  struct MakeGreaterThanTemplateArgumentListTerminator {
>> +    bool &GreaterThanIsOperator;
>> +    bool OldGreaterThanIsOperator;
>> +
>> +    MakeGreaterThanTemplateArgumentListTerminator(bool &GTIO)
>> +      : GreaterThanIsOperator(GTIO), OldGreaterThanIsOperator 
>> (GTIO) {
>> +      GTIO = false;
>> +    }
>> +
>> +    ~MakeGreaterThanTemplateArgumentListTerminator() {
>> +      GreaterThanIsOperator = OldGreaterThanIsOperator;
>> +    }
>> +  };
>> +
>> +  /// \brief RAII object that makes '>' behave like an
>> +  /// operator. Occurs, for example, inside parentheses.
>> +  struct MakeGreaterThanAnOperator {
>> +    bool &GreaterThanIsOperator;
>> +    bool OldGreaterThanIsOperator;
>> +
>> +    MakeGreaterThanAnOperator(bool &GTIO)
>> +      : GreaterThanIsOperator(GTIO), OldGreaterThanIsOperator 
>> (GTIO) {
>> +      GTIO = true;
>> +    }
>> +
>> +    ~MakeGreaterThanAnOperator() {
>> +      GreaterThanIsOperator = OldGreaterThanIsOperator;
>> +    }
>> +  };
>> +
>>
> I think these can be replaced by
>
> struct GreaterThenIsOperatorScope {
>  bool &GreaterThanIsOperator;
>  bool OldGreaterThanIsOperator;
>
>  GreaterThenIsOperatorScope(bool &GTIO, bool Val)
>    : GreaterThanIsOperator(GTIO), OldGreaterThanIsOperator(GTIO) {
>    GreaterThanIsOperator = Val;
>  }
>
>  ~GreaterThanIsOperatorScope() {
>    GreaterThanIsOperator = OldGreaterThanIsOperator;
>  }
> };

Yeah, that's cleaner. Follow-up patch is coming.

>> +
>> +      // FIXME: What's our recovery strategy for failed template- 
>> argument-lists?
>> +      return;
>> +    }
>>
> Scan for the > ?

Yeah, we do that. I guess the meta-question is, really: how should the  
caller react to a failed attempt to parse a template-id?

>> +  // Therefore, we initially try to parse a type-id.
>> +  if (isTypeIdInParens()) {
>> +    TypeTy *TypeArg = ParseTypeName();
>> +    return Actions.ActOnTypeTemplateArgument(TypeArg);
>> +  }
>>
> If the type is semantically wrong, you could still get NULL here, I
> think. How does ActOnTypeTemplateArgument react to that?
>>
>> +Sema::OwningTemplateArgResult Sema::ActOnTypeTemplateArgument 
>> (TypeTy *Type) {
>> +  return Owned(new (Context) TemplateArg(QualType::getFromOpaquePtr 
>> (Type)));
>> +}
>> +
>>
> Well, that answers the above question. But I think having a null type
> embedded in the AST is not safe.


Oh, I agree. My later patch, which removed ActOnTypeTemplateArgument  
and TemplateArg entirely, makes sure that we have a non-NULL type or  
expression for each argument.

Thanks for the review!

	- Doug