Features Download
From: Douglas Gregor <dgregor-2kanFRK1NckAvxtiuMwx3w <at> public.gmane.org>
Subject: Re: [PATCH] Sema and AST for C++0x Lambda Expressions
Newsgroups: gmane.comp.compilers.clang.scm
Date: Saturday 19th November 2011 19:48:37 UTC (over 6 years ago)
On Nov 15, 2011, at 10:45 PM, John Freeman wrote:

> I finished integrating your LambdaScopeInfo changes with my work since my
last patch submission. Divergent branches aren't fun. :/

No, they aren't. Sorry for going rogue :)

> I saw you switched from using CheckFieldDecl to directly using
FieldDecl::Create. Was that intentional? I had the impression that the
extra semantic checking was preferred. Same question about switching from
ActOnIdExpression to directly using name lookup.

We don't need most of the checking in CheckFieldDecl, since a proper AST
already maintains most of those invariants. Plus, all of the field-centric
diagnostics would be wrong.

Similarly with ActOnIdExpression; I'd much rather that we just did
unqualified name lookup and inspect the result appropriately.

> A few problems concerning the CXXMethodDecl (henceforth called Method):
> A. I was trying to use ActOnStartOfFunctionDef to enter the DeclContext
owned by Method. You had used PushDeclContext directly. I believe that will
be necessary to avoid creating an extraneous FunctionScopeInfo, but do we
want any of the extra checking in ActOnStartOfFunctionDef?

The necessary checking should be factored out of ActOnStartOfFunctionDef so
that both routines can use it (separate patches).

> B. A conundrum:
> - PushDeclContext requires that the lexical DeclContext for Method be
> - CXXRecordDecl::addDecl requires that the lexical DeclContext for Method
be the CXXRecordDecl (henceforth called Class).
> How about this solution? :
> 1. Method.setLexicalDeclContext(CurContext).
> 2. PushDeclContext.
> 3. Add Method to the LambdaScopeInfo so that it can be retrieved later
(we can't look it up in Class because we haven't called addDecl yet).
> 4. Parse body.
> 5. Retrieve Method from the LambdaScopeInfo.
> 6. ActOnFinishFunctionBody (calls PopDeclContext).
> 7. Method.setLexicalDeclContext(Class).
> 8. Class.addDecl(Method).

This seems okay, but why not just perform the addDecl early on, and push
both the Class context and the Method context? (in other words, handle this
like an inline member function of a local class)

> C. In a similar question to part A on ActOnStartOfFunctionDef vs
PushDeclContext, concerning step 6 above, do we want to use PopDeclContext
directly instead of ActOnFinishFunctionBody, or do we want the extra
semantic checks?

Again, I think the answer is to refactor. Blocks have some end-of-body
checking; that's very much in the same realm.

This is very much a refactor-and-then-implement sort of feature, if I
haven't made that obvious :)

	- Doug
CD: 3ms