Thanks Richard. Updated patch including context & FIXME attached ready for committing.
Cheers, Will. On 5 August 2013 08:46, Richard Smith <[email protected]> wrote: > 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 <[email protected]> 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 <[email protected]> wrote: >> >>> On Sun, Jul 28, 2013 at 11:31 AM, Will Wilson <[email protected]>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.* >> > > -- *Indefiant Ltd.*
late_template_pch_support_formatted.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
