On Mon, Sep 12, 2016 at 5:20 PM, David Blaikie <dblai...@gmail.com> wrote:
> On Mon, Sep 12, 2016 at 5:09 PM Reid Kleckner via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: rnk >> Date: Mon Sep 12 19:01:23 2016 >> New Revision: 281278 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=281278&view=rev >> Log: >> [DebugInfo] Deduplicate debug info limiting logic >> >> We should be doing the same checks when a type is completed as we do >> when a complete type is used during emission. Previously, we duplicated >> the logic, and it got out of sync. This could be observed with >> dllimported classes. >> > > Thanks for having a go at this! > > The logic in completeType (& maybe also completeClassData) still seems to > duplicate some of the logic in shouldOmitDefinition - I think the original > idea I had was a series of cascading entry points (completeType checking > the "oh, the type now has a definition", then falling into > completeRequiredType, etc) > > In theory the "shouldOmitDefinition" could be split into the extra > conditions for each of the parts of that cascade, so the checks wouldn't be > redundantly executed when we were already being called about the completion > or required completion of a type, etc. But I'm not sure the performance > gain would be measurable, etc. > > If not, we might as well just have one entry point that's something like > "revisit type because something interesting happened to it" (it became > complete, it became required to be complete, etc)... but that seems a bit > weird too. > Seems reasonable, but I didn't see a nice, clear, obvious refactoring to do, so I left it here. Seems better than it was before. > >> Also reduce a test case for this slightly. >> > > The UseCompleteType is still there/unnecessary (as is the 'private' in > UnicodeString) - and I'd probably rename it to foo/bar and throw it in a > namespace like the rest for consistency - having semantic names in a test > when they're just remnants from whatever code it was reduced from seems > confusing to me. But I realize I'm a bit more pedantic about that than most. > The private was necessary at some point during the reduction process for some reason.
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits