Getting Involved, Newbie in need of some hand holding

Hello clang,

I have been interested in the standardisation of the upcomming C++0x for quite
some time now, and while reading up on it I discovered LLVM and CLang.
I was imediately intrigued by the project, and I'd like to get involved.

So I started reading all the Documentation (short of API-Reference) that I
could find, and think that I now have a tentative understanding how the
larger parts fit togher.

Reading the Open Projects and the C++ Status Pages I took on the first
"project" which looked managable and open, which landed me with
Pseudo Destructor calls.

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?, ... )
Anyway, having this test I can clearly see that this feature
is not yet supported, I recive the following output from
clang-cc -fsyntax-only -verify:

Errors seen but not expected:
  Line 7: expected identifier
  Line 8: member reference base type 'NonClass_type' (aka 'int') is not a
structure or union
  Line 11: expected identifier
  Line 12: member reference base type 'NonClass_type' (aka 'int') is not a
structure or union

So searching for these errors I get the source locations where they occur,
and discovered that Line 7 and 11 are not correctly parsed so I changed this
with the following patch:

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