[PATCH] PR9992 : -fdelayed-template-parsing support for PCH

Richard Smith richard at metafoo.co.uk
Sun Aug 4 23:46:18 PDT 2013


In future, please include some context in your unified diffs.

LGTM with a FIXME for improving the deserialization approach.

On Sun, Aug 4, 2013 at 9:58 AM, Will Wilson <will at indefiant.com> wrote:

> Thanks for the review. I've done a consistency pass and clang-formatted
> the patch.
>
> While I'd like to look into optimizing the deserialization approach I
> doubt I'll get a chance for some time. Can this will suffice until I (or
> someone else) gets a chance to optimize the approach? Let me know!
>
> FYI, it's been working perfectly on a large body of MSVC code for the past
> week.
>
> - Will.
>
>
> On 28 July 2013 22:32, Richard Smith <richard at metafoo.co.uk> wrote:
>
>> On Sun, Jul 28, 2013 at 11:31 AM, Will Wilson <will at indefiant.com> wrote:
>>
>>> Hi All,
>>>
>>> I've attached a preliminary patch for some feedback. This is my first
>>> foray into the serialization code so I've taken the simplest approach I
>>> could for now.
>>>
>>> I've also moved LateParsedTemplateMap from the Parser to Sema. I did try
>>> and avoid this but plumbing the necessary calls via Sema to the Parser
>>> proved messy, and after considering the options I decided that moving the
>>> state to Sema was the lesser of the two evils.
>>>
>>> Some may consider it a layering violation but as Sema effectively needs
>>> the data to finish its job and the state is nothing more than a couple of
>>> Decl pointers and a sequence of Tokens I thought it reasonable - after all
>>> Sema already deals with Tokens.
>>>
>>
>> I'm not a big fan of the layering here, but I agree that it's preferable
>> to having Serialization know about the Parser.
>>
>> As I said the serialization approach I've taken is about as simplistic as
>>> possible. Lazy lookup/deserialization should really be performed on each
>>> FunctionDecl as they're instantiated rather than as a job lot for the whole
>>> TU (as currently implemented) - but I was hoping for some feedback before
>>> attempting that.
>>>
>>
>> That was the only substantial comment I had on the patch. Please also fix
>> up the formatting (no line break before open brace, space before "&" not
>> after -- clang-format-diff can fix these for you) and inconsistent naming
>> (WriteLateParsedTemplates versus ReadLateParsedTemplateFunctions versus
>> LateParsedTemplatedFunction).
>>
>
>
>
> --
> *Indefiant Ltd.*
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130804/aaf3b6d5/attachment.html>


More information about the cfe-commits mailing list