[cfe-commits] r58817 - in /cfe/trunk: include/clang/AST/ include/clang/Basic/ include/clang/Parse/ lib/AST/ lib/Basic/ lib/Parse/ lib/Sema/ test/SemaCXX/ www/

[Doug Gregor]

On Fri, Nov 7, 2008 at 5:53 PM, Sebastian Redl
<sebastian.redl at getdesigned.at> wrote:
> Douglas Gregor wrote:
>>
>> Author: dgregor
>> Date: Thu Nov  6 16:13:31 2008
>> New Revision: 58817
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=58817&view=rev
>> Log:
>> Parsing, ASTs, and semantic analysis for the declaration of overloaded
>> operators in C++.
>
> Yay!
>>
>> +DIAG(err_operator_overload_must_be_unary, ERROR,
>> +     "overloaded operator '%0' must be a unary operator (has %1
>> parameter%2)")
>> +DIAG(err_operator_overload_must_be_binary, ERROR,
>> +     "overloaded operator '%0' must be a binary operator (has %1
>> parameter%2)")
>> +DIAG(err_operator_overload_must_be_unary_or_binary, ERROR,
>> +     "overloaded operator '%0' must be a unary or binary operator (has %1
>> parameter%2)")
>> +DIAG(err_operator_overload_must_be_member, ERROR,
>> +     "overloaded operator '%0' must be a non-static member function")
>> +DIAG(err_operator_overload_post_incdec_must_be_int, ERROR,
>> +     "%0parameter of overloaded post-%1 operator must have type 'int'
>> (not '%2')")
>> +
>>
>
> To echo a concern Chris recently raised about my code, these are not
> localizable. I'm not quite sure how to make them localizable, though.

Good point. I've fixed it.

>>
>> +
>> +#ifndef OVERLOADED_OPERATOR
>> +#  define OVERLOADED_OPERATOR(Name,Spelling,Token)
>> +#endif
>>
>
> Wouldn't it make more sense to put an #error directive here? Surely
> including a .def file without defining those macros cannot be intentional.

One might want to pull out just the multi-token operators, so I made
that option easy.

>>
>> +IdentifierInfo *Parser::MaybeParseOperatorFunctionId() {
>> +  if (Tok.isNot(tok::kw_operator))
>> +    return 0;
>> +
>> +  OverloadedOperatorKind Op = OO_None;
>> +  switch (NextToken().getKind()) {
>>
>
> Hmm ... I know very little about how Clang's parser works, but wouldn't it
> be easier to just consume the operator keyword no matter what? What's the
> use case of leaving it in there? (Obviously, the part that parses a
> conversion operator would have to be adapted.)

If the token after the 'operator' does not start an
operator-function-id, we don't parse the 'operator'. Instead, we end
up parsing a conversion-function-id, which consumes the 'operator'.

>>
>> +
>> +  bool CanBeUnaryOperator = false;
>> +  bool CanBeBinaryOperator = false;
>> +  bool MustBeMemberOperator = false;
>> +
>>
>
> Wouldn't it be cleaner to extend the operator def file with this information
> and use it to generate the switch? Or would the resulting generated code
> duplication be worse than the gain? (But you could also generate a lookup
> table.)

Yeah, that's better. Thanks!

  - Doug