[cfe-commits] [PATCH][MS][Review request] - Late Microsoft template parsing
Francois Pichet
pichet2000 at gmail.com
Thu Apr 14 08:22:27 PDT 2011
On Sat, Apr 9, 2011 at 1:59 PM, Francois Pichet <pichet2000 at gmail.com> wrote:
> Hi here is an updated patch
>
>
> On Sat, Mar 26, 2011 at 7:29 PM, Douglas Gregor <dgregor at apple.com> wrote:
>>
>> Nifty. A bunch of comments below.
>>
>> Index: include/clang/Driver/CC1Options.td
>> ===================================================================
>> --- include/clang/Driver/CC1Options.td (revision 127633)
>> +++ include/clang/Driver/CC1Options.td (working copy)
>> @@ -509,6 +509,9 @@
>> HelpText<"Store string literals as writable data">;
>> def fno_bitfield_type_align : Flag<"-fno-bitfield-type-align">,
>> HelpText<"Ignore bit-field types when aligning structures">;
>> +def fdelayed_template_parsing : Flag<"-fdelayed-template-parsing">,
>> + HelpText<"Parse template dependent functions definition at the end of "
>> + "the translation unit">;
>>
>> How about "Parse templated function definitions at the end of the translation unit" ?
>
> done
>
>>
>> Index: include/clang/Parse/Parser.h
>> ===================================================================
>> --- include/clang/Parse/Parser.h (revision 127633)
>> +++ include/clang/Parse/Parser.h (working copy)
>> @@ -908,6 +908,31 @@
>>
>> SourceRange getSourceRange() const;
>> };
>>
>> + /// \brief Contains a late template declaration.
>> + /// Will be parsed at the end of the translation unit.
>> + struct LateParsedTemplate {
>> + explicit LateParsedTemplate(Parser* P, Decl *MD)
>> + : D(MD), C(0), IsExplicitInstantiantion(false) {}
>> +
>> + CachedTokens Toks;
>> +
>> + /// \brief When IsExplicitInstantiantion is false this represents
>> + /// the template function declaration to be late parsed.
>> + Decl *D;
>> +
>> + /// \brief When IsExplicitInstantiantion is true this represents
>> + /// the context where the explicit instantiantion occured.
>> + DeclContext *C;
>>
>> Typo "instantiantion".
>
> done
>
>>
>> +/// \brief Late parse a C++ explicit template instantiation in Microsoft mode.
>> +void Parser::ParseLateExplicitInstantiation(
>> + LateParsedTemplate &LMT) {
>> + assert(!LMT.Toks.empty() && "Empty body!");
>> +
>> + LMT.Toks.push_back(Tok);
>> + PP.EnterTokenStream(LMT.Toks.data(), LMT.Toks.size(), true, false);
>> + ConsumeAnyToken();
>> +
>> + DeclContext *SavedContext = Actions.CurContext;
>> + Actions.CurContext = LMT.C;
>>
>> I'd prefer that you use Sema's ContextRAII object, rather than directly setting CurContext from within the parser. Or, at the very least, have some actions that push/pop the context.
>>
>> Stepping back a little bit... it would seem cleaner not to delay parsing of existing instantiations. Rather, we should just delay the actual instantiation within Sema.
>
> ok I added this in InstantiateFunctionDefinition, I believe this will do it.
>
> // Postpone late parsed functions instantiation.
> if (PatternDecl->isLateTemplateParsed()) {
> PendingInstantiations.push_back(
> std::make_pair(Function, PointOfInstantiation));
> return;
> }
>
>
>>
>> + // Recreate the DeclContext.
>> + DeclContext *SavedContext = Actions.CurContext;
>> + Actions.CurContext = Actions.getContainingDC(FD);
>>
>> I'd really rather see this done via ContextRAII or special action callbacks. The parser shouldn't poke at Sema state directly.
>
> Ok I was not aware of the existence of ContextRAII. I am using it now
> in ParseLateTemplatedFuncDef.
>
>>
>> Index: lib/Sema/SemaDecl.cpp
>> ===================================================================
>> --- lib/Sema/SemaDecl.cpp (revision 127633)
>> +++ lib/Sema/SemaDecl.cpp (working copy)
>> @@ -5544,7 +5544,13 @@
>>
>> // But don't complain if we're in GNU89 mode and the previous definition
>> // was an extern inline function.
>> const FunctionDecl *Definition;
>> - if (FD->hasBody(Definition) &&
>> +
>> + // In support of delayed template parsing, do not generate an error
>> + // if the function already has a fake function definition.
>> + bool IsLateParsingTemplate = getLangOptions().DelayedTemplateParsing &&
>> + dyn_cast_or_null<NullStmt>(FD->getBody());
>> +
>> + if (FD->hasBody(Definition) && !IsLateParsingTemplate &&
>> !canRedefineFunction(Definition, getLangOptions())) {
>> if (getLangOptions().GNUMode && Definition->isInlineSpecified() &&
>> Definition->getStorageClass() == SC_Extern)
>>
>> Can we add a bit to FunctionDecl to specify that it has a late body, rather than using the NullStmt convention? Conventions like that inevitably cause confusion down the line.
>
> Ok I added the flag IsLateTemplateParsed is FunctionDecl.
>
>>
>> +/// IsInsideLocalClassOfTemplateFunction
>> +bool Sema::IsInsideALocalClassWithinATemplateFunction() {
>> + if (CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(CurContext)) {
>> + const FunctionDecl *FD = RD->isLocalClass();
>> + return (FD && FD->getTemplatedKind() != FunctionDecl::TK_NonTemplate);
>> + }
>> + return false;
>>
>> Presumably you want to walk up the DeclContext chain, looking for a local class or stopping when we hit a non-local class, namespace, or the translation unit? Otherwise, one wouldn't notice that we're in a local class if that local class occurs inside a block (for example), or when we're in the body of a member function of a local class.
>
> ok done.
>
>>
>> Testing-wise, how do you propose we validate this feature? You're adding a single test case, but this is a fairly significant compiler mode. Can we run the regression tests also with -fdelayed-template-parsing?
>>
>
> I think the best way to validate the late template parsing is to test
> clang in late template parsing mode against the whole clang test
> suite. That's how I tested that the patch works. Currently there are
> 18 failing tests in that mode but there are normal fail because there
> are failing because of difference in mangling order or because of
> others normal side effects of late template parsing.
>
> I think we can work to add a flag to lit to test in late template
> parsing mode and flag the one that fails.
>
Hi Doug,
You might want to hold on reviewing this patch. I am working on an
update that will include Just-In-Time template parsing (parsing
template code only if it is requested by an actual instantiation).
Also, I am planning to handle the problem of missing typename in
Microsoft templated function by using the actual instantiation
parameters to resolve any expression/type ambiguity.
More information about the cfe-commits
mailing list