[cfe-dev] Getting Involved, Newbie in need of some hand holding

[Eli Friedman]

On Wed, Jul 1, 2009 at 11:03 AM, Fabio Fracassi<f.fracassi at gmx.net> wrote:
> The first thing I did was creating a test for the feature, so I created a file
> llvm/tools/clang/test/CXX/expr/expr.post/expr.pseudo/p1.cpp with the following
> content:
>
>  3: int main() {
>  4:    typedef int NonClass_type;
>  5:
>  6:    NonClass_type nc;
>  7:    nc.~NonClass_type();
>  8:    nc.NonClass_type::~NonClass_type();
>  9:
> 10:    NonClass_type * pNc;
> 11:    pNc->~NonClass_type();
> 12:    pNc->NonClass_type::~NonClass_type();
> 13: }
>
> which probably contain the first mistakes along the way.
> (Do I test everything relevant? Do lines 8 and 12 belong
> to paragraph 1?, ... )

For paragraph 1, you're missing tests for "The result shall only be
used as the operand for the function call operator (), and the result
of such a call has type void".  Some other interesting things to test
would be forms like "var.ns::ty::~ty()", "var.::ty::~ty()", and
"var.ty<int>::ty2::~ty2()".

There are also some tests to write for paragraph 2, but I'll assume
you haven't written those yet.

If you want, you can fix up the test, mark it as XFAIL, and submit a
patch for it; it's an easy way to get familiar with the process, and
it's nice to have tests for this stuff even if they don't succeed yet.

> ===================================================================
> --- lib/Parse/ParseExpr.cpp     (revision 74335)
> +++ lib/Parse/ParseExpr.cpp     (working copy)
> @@ -915,7 +915,8 @@
>       tok::TokenKind OpKind = Tok.getKind();
>       SourceLocation OpLoc = ConsumeToken();  // Eat the "." or "->" token.
>
> -      if (Tok.isNot(tok::identifier)) {
> +      if (Tok.isNot(tok::identifier) && Tok.isNot(tok::tilde)) {
> +        // ~ is valid for explicit destructor calls
>         Diag(Tok, diag::err_expected_ident);
>         return ExprError();
>       }

That's a start, but not really right even if we're just considering
Parser code.  The issue is that you're not consuming the ~, so the
identifier after it will be treated as an error, and Sema won't get
information for right identifier.  It's probably also easier for the
parser to consume the "()" here immediately, since there isn't any
reasonable way to construct a reference to the destructor in the AST.

You'll probably also need a call to ParseOptionalCXXScopeSpecifier somewhere.

> So I guess this is correct, and calling destructors explicitly (pseudo or
> otherwise) should be handled in ActOnMemberReferenceExpr(...) ?

I would suggest adding a new member to the Action class (in
include/clangParse/Action.h) to handle this case, because it's
significantly different from a normal identifier.

-Eli