<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">On Jul 30, 2013, at 2:48 AM, Timur Iskhodzhanov <<a href="mailto:timurrrr@google.com">timurrrr@google.com</a>> wrote:<br><div><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">2013/7/30 John McCall <<a href="mailto:rjmccall@apple.com">rjmccall@apple.com</a>>:<br><blockquote type="cite">On Jul 29, 2013, at 3:40 AM, Timur Iskhodzhanov <<a href="mailto:timurrrr@google.com">timurrrr@google.com</a>> wrote:<br><br>Update the return adjustment tests to make sure we put the covariant-return<br>override after the methods defined in the base which defines the overridden<br>function.<br><br>Also, add back the implicit test expectations so it's a more obvius example<br>of adding a slot to the vftable of a non-primary base subobject.<br><br><br>+/// Visit all the methods overridden by the given method recursively,<br>+/// in a depth-first pre-order. The Visitor's visitor method returns a bool<br>+/// indicating whether to continue the recursion for the given overridden<br>+/// method.<br><br>It’s worth spelling out that “false” stops the iteration.<br></blockquote><br>Done<br><br><blockquote type="cite">+namespace {<br>+ struct OverriddenMethodsCollector {<br>+ OverriddenMethodsSetTy *Methods;<br>+<br>+ bool visit(const CXXMethodDecl *MD) {<br>+ return Methods->insert(MD);;<br><br>Double semi-colons, and please add a comment like:<br> // Don’t recurse on this method if we’ve already collected it.<br><br>+// 2. Each subobject with a vfptr gets its own vftable rather than an<br>address<br>+// point in a single vtable shared between all the subobjects.<br><br>I’d make it clear that this is about vftable *symbols*<br></blockquote><br>Done<br><br><blockquote type="cite">and doesn’t itself affect, say, the v-call mechanism.<br></blockquote><br>Hm, not sure what you've meant here - they somewhat do affect the<br>vcall mechanism.<br></div></blockquote><div><br></div><div>How so? Itanium’s v-tables are still logically broken up into separate</div><div>components, such that an access through any one v-table pointer only</div><div>ever accesses a particular component. You could very easily change</div><div>Itanium to allocate all the v-tables in separate symbols without any need</div><div>to change the code for virtual calls. It’s just that Itanium is trying to</div><div>optimize the number of separate symbols required.</div><div><br></div><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><blockquote type="cite">+CharUnits<br>+VFTableBuilder::ComputeThisOffset(const CXXMethodDecl *MD,<br>+ BaseSubobject Base,<br>+ FinalOverriders::OverriderInfo Overrider)<br>{<br>+ if (isa<CXXDestructorDecl>(MD))<br>+ return CharUnits();<br>+<br><br>Wait, why don’t destructors require this-offsets?<br></blockquote><br>Nope, as the virtual dtor's final overrider is emitted for the MDC,<br>which has 0 offset.<br>I've added a comment for that!<br></div></blockquote><div><br></div>Sure, but — oh, I see, we just need to return the offset from the complete</div><div>object here, and if we need a thunk to adjust to that, then the calling logic</div><div>will handle that.</div><div><br></div><div><blockquote type="cite" dir="auto"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><blockquote type="cite">Otherwise, this is looking good.<br></blockquote><br>Thanks!<br>Committed as r187409.<br><br>I also had to move one of the visitors out of the<br>VFTableBuilder::ComputeThisOffset body to avoid compiler warning.<br></div></blockquote><div><br></div>Yeah, C++03 has some pointless restrictions about template arguments.</div><div><br></div><div>Also, we’ve found that MSVC tends to miscompile local classes, so pulling</div><div>things out is for the best.</div><div><br></div><div>John.</div></body></html>