<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 29, 2013, at 3:40 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;"> Update the return adjustment tests to make sure we put the covariant-return override after the methods defined in the base which defines the overridden function.<br><br> Also, add back the implicit test expectations so it's a more obvius example of adding a slot to the vftable of a non-primary base subobject.<br></div></blockquote></div><br><div><div>+/// Visit all the methods overridden by the given method recursively,</div><div>+/// in a depth-first pre-order. The Visitor's visitor method returns a bool</div><div>+/// indicating whether to continue the recursion for the given overridden</div><div>+/// method.</div><div><br></div><div>It’s worth spelling out that “false” stops the iteration.</div><div> </div><div>+namespace {</div><div>+  struct OverriddenMethodsCollector {</div><div>+    OverriddenMethodsSetTy *Methods;</div><div>+</div><div>+    bool visit(const CXXMethodDecl *MD) {</div><div>+      return Methods->insert(MD);;</div><div><br></div><div>Double semi-colons, and please add a comment like:</div></div><div>  // Don’t recurse on this method if we’ve already collected it.</div><div><br></div><div><div>+//  2. Each subobject with a vfptr gets its own vftable rather than an address</div><div>+//     point in a single vtable shared between all the subobjects.</div><div><br></div></div><div>I’d make it clear that this is about vftable *symbols* and doesn’t itself</div><div>affect, say, the v-call mechanism.</div><div><br></div><div><div>+//     value.  Therefore, if the offset isn't really constant (i.e. if a virtual</div></div><div><br></div><div>This is just an example of how this can happen, so you should use</div><div>“e.g." instead of “i.e.”.</div><div><br></div><div><div>+//  5. Instead of VTT and constructor vtables, vtordisps are emitted into the</div><div>+//     class layout if a class has</div><div>+//      a) a user-defined ctor/dtor</div><div>+//     and</div><div>+//      b) a method overriding a method in a virtual base.</div></div><div><br></div><div>The main thing that replaces VTT and construction v-tables is having</div><div>a separate vb-table.</div><div><br></div><div><div>+CharUnits</div><div>+VFTableBuilder::ComputeThisOffset(const CXXMethodDecl *MD,</div><div>+                                  BaseSubobject Base,</div><div>+                                  FinalOverriders::OverriderInfo Overrider) {</div><div>+  if (isa<CXXDestructorDecl>(MD))</div><div>+    return CharUnits();</div><div>+</div></div><div><br></div><div>Wait, why don’t destructors require this-offsets?</div><div><br></div><div>Otherwise, this is looking good.</div><div><br></div><div>John.</div></body></html>