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.* >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
