<div dir="ltr"><div><font color="#000000" face="Calibri, Arial, Helvetica, sans-serif">Hi,</font></div><div><font color="#000000" face="Calibri, Arial, Helvetica, sans-serif"><span style="font-size:16px"><br></span></font></div><div><font color="#000000" face="Calibri, Arial, Helvetica, sans-serif">Thank you guys for coming to the round table. I am trying to summarize what we were talking about.</font></div><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif"><div><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><br></span></div><span style="font-size:16px">- Error handling: rollback mechanism</span></span><div>We agreed that this is something that would be incredibly problematic to implement mostly because the AST is not designed to be trimmed. In normal cases, the AST just grows, but nodes and interconnections between them are never deleted.<br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><br></span></div><div><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">- Replace `StructuralEquivalency` with ODRHash</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">  - Step 0: StructuralEquivalency should not diagnose when called from the</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">    Importer, diags should be coming from the importer.</span></div><div>We agreed that the class StructuralEquivalenceContext is used for multiple things in the ASTImporter. These are 1) ODR violation discovery 2) identifying the existing part of a redecl chain for a Decl. Actually, ODR checking is based on tokens if we compare definitions, so structural eq is not the best thing to use for that.</div><div>We have concerns whether we could use the ODRHash, but we agreed that it would be worth to prototype it.</div><div><br style=""><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">- ODRViolation handling</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">  - Class(Var)</span><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">TemplateSpecializationDecl: Problem! We can't have more than 1 spec</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">    (</span><a href="https://reviews.llvm.org/D66999" target="_blank" style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">https://reviews.llvm.org/D66999</a><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">)</span></div><div>We agreed that this is fine to be conservative in case of specializations, in these cases we will always use the existing definition and will report back an error. (In case of simple classes and other non-specialization decls ,however, the liberal strategy can be used.)</div><div><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">  - Renaming strategy in case of ODR violation</span><br></div><div><div>Raphael is worried about name mangling issues that could cause.<br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"></div></div><div><br></div><div><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">- Strategies for AccumulateChildErrors</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">  Clients of the ASTImporter should be able to choose an</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">  appropriate error handling strategy for their needs.  For instance,</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">  they may not want to mark an entire namespace as erroneous merely</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">  because there is an ODR error with two typedefs.  As another example,</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">  the client may allow EnumConstantDecls with same names but with</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">  different values in two distinct translation units.</span></div><div>We agreed that this is fine to use this approach, probably there is no need to make this behavior configurable.<br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif"><br><div><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">- VarTemplateSpecializationDecl: ODR violation is not even detected</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"></span></div><div><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif">We didn't have time to discuss this.</span></div><div></div><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">Smaller issues/tasks/FIXMEs (techn debt)</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">- VisitFunctionDecl:</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">  - Member function templates are not handled similarly to simple fun</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">    specializations</span></div><div>We agreed that this is a big technical debt<br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">  - Merge function definitions of members</span></div><div>We discussed that out-of class default definitions can cause this situation, and at some time we should finish this.<br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">  - Merge exception specifications</span></div><div><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><br></span></div><div><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">- Handling of inheritable attributes (</span><a href="https://reviews.llvm.org/D68634" target="_blank" style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">https://reviews.llvm.org/D68634</a><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">)</span></div><div><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">  - Use PrevDecl in `GetImportedOrCreateDecl` ?</span></div><div>We agreed that handling of attributes cannot be done with one patch because there are many different inheritable attributes and each must be handled differently.</div><div><br style=""><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">- ObjC/ObjC++ support and stabilization</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">  - No test cases (not interesting for E///, we don't have objc/c++ code)</span></div><div>We agreed that there is a technical debt here, but we did not show much interest in fixing that.</div><div><br style=""><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">- ClassTemplateSpecializationDec</span><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">l: merge instantiated default arguments,</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">  exceptions specifications</span></div><div>We understood the problem (that took like 20 minutes and AST dumping) and recognized that the way we merge initializers of fields is good, and we should continue doing that with default arg and exceptions spec instantiations. <br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><br></span></div><div><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif">We did not have time to discuss these other issues:</span></div><div><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">- Structural Eq:</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">  - Polluted cache of nonequivalent declarations</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">  - Some diagnostics are completely missing, this is misleading</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">- Several minor issues/fixmes with VarTemplateDecl</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">- Check visibility/linkage for ClassTemplateDecl, VarTemplateDecl</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">- Fix import of equivalent but repeated FriendDecls</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">- Handle redecl chain of TypeDefNameDecl</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">- Add Decls to their context in a unified way, and only if the "From"DC</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">  contains it (`AddDeclToContexts`)</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">- VisitVarDecl:</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">  - Check for ODR error if the two definitions have different initializers?</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">  - Diagnose ODR error if the two initializers are different</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">- Remove obsolate FIXMEs and TODOs</span><br style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px">- Import default arguments of templates</span><br></div><div><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif;font-size:16px"><br></span></div><div><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif">Cheers,</span></div><div><span style="color:rgb(0,0,0);font-family:Calibri,Arial,Helvetica,sans-serif">Gabor</span></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Oct 17, 2019 at 4:11 PM Gábor Márton <<a href="mailto:martongabesz@gmail.com">martongabesz@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hi,<div><br></div><div><div style="margin:0px;padding:0px;border:0px;font-variant-numeric:inherit;font-variant-east-asian:inherit;font-stretch:inherit;font-size:12pt;line-height:inherit;font-family:Calibri,Arial,Helvetica,sans-serif;vertical-align:baseline;color:black">At the upcoming LLVM Dev Conf, we will have a round table discussion for ASTImporter, right after the ASTImporter Tutorial.</div><div style="margin:0px;padding:0px;border:0px;font-variant-numeric:inherit;font-variant-east-asian:inherit;font-stretch:inherit;font-size:12pt;line-height:inherit;font-family:Calibri,Arial,Helvetica,sans-serif;vertical-align:baseline;color:black"><span style="margin:0px;padding:0px;border:0px;font:inherit;vertical-align:baseline;color:inherit">The time slot for the round table is </span><span style="margin:0px;padding:0px;border:0px;font:inherit;vertical-align:baseline;color:inherit">Wednesday, Oct 23 2:55-4:00.</span></div><div style="margin:0px;padding:0px;border:0px;font-variant-numeric:inherit;font-variant-east-asian:inherit;font-stretch:inherit;font-size:12pt;line-height:inherit;font-family:Calibri,Arial,Helvetica,sans-serif;vertical-align:baseline;color:black"><span style="margin:0px;padding:0px;border:0px;font:inherit;vertical-align:baseline;color:inherit">I have gathered things about possible future work and improvements, bring your own topic to discuss!</span></div></div><div style="margin:0px;padding:0px;border:0px;font-variant-numeric:inherit;font-variant-east-asian:inherit;font-stretch:inherit;font-size:12pt;line-height:inherit;font-family:Calibri,Arial,Helvetica,sans-serif;vertical-align:baseline;color:black"><span style="margin:0px;padding:0px;border:0px;font:inherit;vertical-align:baseline;color:inherit"><br></span></div><div style="margin:0px;padding:0px;border:0px;font-variant-numeric:inherit;font-variant-east-asian:inherit;font-stretch:inherit;font-size:12pt;line-height:inherit;font-family:Calibri,Arial,Helvetica,sans-serif;vertical-align:baseline;color:black"><span style="margin:0px;padding:0px;border:0px;font:inherit;vertical-align:baseline;color:inherit">Thanks and see you at the conference,</span></div><div style="margin:0px;padding:0px;border:0px;font-variant-numeric:inherit;font-variant-east-asian:inherit;font-stretch:inherit;font-size:12pt;line-height:inherit;font-family:Calibri,Arial,Helvetica,sans-serif;vertical-align:baseline;color:black"><span style="margin:0px;padding:0px;border:0px;font:inherit;vertical-align:baseline;color:inherit">Gabor</span></div><div style="margin:0px;padding:0px;border:0px;font-variant-numeric:inherit;font-variant-east-asian:inherit;font-stretch:inherit;font-size:12pt;line-height:inherit;font-family:Calibri,Arial,Helvetica,sans-serif;vertical-align:baseline;color:black"><span style="margin:0px;padding:0px;border:0px;font:inherit;vertical-align:baseline;color:inherit"><br></span></div><div style="margin:0px;padding:0px;border:0px;font-variant-numeric:inherit;font-variant-east-asian:inherit;font-stretch:inherit;font-size:12pt;line-height:inherit;font-family:Calibri,Arial,Helvetica,sans-serif;vertical-align:baseline;color:black"><span style="margin:0px;padding:0px;border:0px;font:inherit;vertical-align:baseline;color:inherit">Big stuff<br>- Error handling: rollback mechanism<br>- Replace `StructuralEquivalency` with ODRHash<br>  - Step 0: StructuralEquivalency should not diagnose when called from the<br>    Importer, diags should be coming from the importer.<br>- ODRViolation handling<br>  - Class(Var)TemplateSpecializationDecl: Problem! We can't have more than 1 spec<br>    (<a href="https://reviews.llvm.org/D66999" target="_blank">https://reviews.llvm.org/D66999</a>)<br>  - VarTemplateSpecializationDecl: ODR violation is not even detected<br>  - Renaming strategy<br>- Strategies for AccumulateChildErrors<br>  Clients of the ASTImporter should be able to choose an<br>  appropriate error handling strategy for their needs.  For instance,<br>  they may not want to mark an entire namespace as erroneous merely<br>  because there is an ODR error with two typedefs.  As another example,<br>  the client may allow EnumConstantDecls with same names but with<br>  different values in two distinct translation units.<br><br>Smaller issues/tasks/FIXMEs (techn debt)<br>- VisitFunctionDecl:<br>  - Member function templates are not handled similarly to simple fun<br>    specializations<br>  - Merge function definitions of members<br>  - Merge exception specifications<br>- Handling of inheritable attributes (<a href="https://reviews.llvm.org/D68634" target="_blank">https://reviews.llvm.org/D68634</a>)<br>  - Use PrevDecl in `GetImportedOrCreateDecl` ?<br>- ObjC/ObjC++ support and stabilization<br>  - No test cases (not interesting for E///, we don't have objc/c++ code)<br>- ClassTemplateSpecializationDecl: merge instantiated default arguments,<br>  exceptions specifications<br>- Structural Eq:<br>  - Polluted cache of nonequivalent declarations<br>  - Some diagnostics are completely missing, this is misleading<br>- Several minor issues/fixmes with VarTemplateDecl<br>- Check visibility/linkage for ClassTemplateDecl, VarTemplateDecl<br>- Fix import of equivalent but repeated FriendDecls<br>- Handle redecl chain of TypeDefNameDecl<br>- Add Decls to their context in a unified way, and only if the "From"DC<br>  contains it (`AddDeclToContexts`)<br>- VisitVarDecl:<br>  - Check for ODR error if the two definitions have different initializers?<br>  - Diagnose ODR error if the two initializers are different<br>- Remove obsolate FIXMEs and TODOs<br>- Import default arguments of templates<br></span></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jul 22, 2019 at 6:28 PM Gábor Márton <<a href="mailto:martongabesz@gmail.com" target="_blank">martongabesz@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hi,<div><br></div><div>I am planning to submit a talk about ASTImporter to the upcoming Dev Meeting at October 22-23, 2019.</div><div>I'd like to talk about</div><div>- the API, </div><div>- how it is used in CTU analysis and in LLDB,</div><div>- internal subtleties and difficulties (Error handling, ExternalASTSource, ...)</div><div>The goal would be to attract more developers to use and improve ASTImporter related code, so perhaps this will be a tutorial.</div><div><br></div><div>Independently from the talk, I'd like to have a round table discussion if there is enough interest.</div><div>Some topics could cover future development ideas and existing problems we have.</div><div>Please get back to me if you are interested and think about the topics you have in mind, also don't forget to buy your ticket to the DevMeeting.</div><div><br></div><div>Thanks,</div><div>Gabor</div></div>
</blockquote></div>
</blockquote></div>