<div dir="ltr"><div class="gmail_extra"><div class="gmail_signature">Hi Nick,</div><div class="gmail_signature"><br></div><div class="gmail_signature">Glad to finally get a sign of life from the clang dev team!</div><div class="gmail_signature"><br></div><div class="gmail_signature">> <span style="font-size:13px">The llvm project requires that patches be submitted by their author (or authors). If you grabbed the original templight code and refactored it, then it may be coauthored between you and the original authors. Our policy requires that an author submit their own code, not mail in someone else's</span></div><div class="gmail_signature"><span style="font-size:13px"><br></span></div><div class="gmail_signature"><span style="font-size:13px">The patches from this email are very out-dated (and I was completely unfamiliar with the process of patch submissions... well, I still am to be honest ;) ). I submitted a much smaller patch after successfully managing to extract all the templight code out of the clang source, to make it a separate tool instead, leaving only very minor "hooks" to be added to Clang itself. That patch can be found at: </span></div><div class="gmail_signature"><span style="font-size:13px"><br></span></div><div class="gmail_signature"><span style="font-size:13px">http</span>://<a href="http://reviews.llvm.org/D5767">reviews.llvm.org/D5767</a></div><div class="gmail_signature"><br></div><div class="gmail_signature">As for the authorship, there are a few things to notice. </div><div class="gmail_signature"><br></div><div class="gmail_signature">First of all, in the new patch that I submitted, all of the code is my own code, because all of the original templight code has been essentially factored out from the original patch, i.e., the "real" templight code now sits in a separate project at: </div><div class="gmail_signature"><br></div><div class="gmail_signature"><a href="https://github.com/mikael-s-persson/templight">https://github.com/mikael-s-persson/templight</a></div><div class="gmail_signature"><br></div><div class="gmail_signature">There is also a more up-to-date clang patch on there, which I will re-submit eventually to Clang when there is more sign of life from the community (I don't like wasting my valuable time throwing patch-updates into a bottomless pit).</div><div class="gmail_signature"><br></div><div class="gmail_signature">The original patch also lacked any specific authorship notice and licensing. But I'm more than happy to add any co-author that is needed to satisfy your submission requirements. All I want is the patch to go through, and I think all other interested parties and original authors feel the same. And after all, we are only talking about of few dozen trivial lines of code here.</div><div class="gmail_signature"><br></div><div class="gmail_signature">And for the record, I'm a bit annoyed by the "<span style="font-size:13px">mail in someone else's code", as it kind of belittles the massive refactoring work I did on the original patch, and the fact that the majority of what it is now is my own code.</span></div><div class="gmail_signature"><br></div><div class="gmail_signature">> <span style="font-size:13px">I think that ActiveTemplateInstantiation should be changed to a form of InstantiationRecord and held from the relevant AST nodes.</span></div><div class="gmail_signature"><span style="font-size:13px"><br></span></div><div class="gmail_signature"><span style="font-size:13px">Maybe so, I don't know, and frankly I don't care. Well, I do care for having good template instantiation code in Clang, but I cannot afford to care too much about design decisions that are not mine to take.</span></div><div class="gmail_signature"><span style="font-size:13px"><br></span></div><div class="gmail_signature"><span style="font-size:13px">> </span><span style="font-size:13px">The changes to ATI to pull it out of Sema and make it part of the AST are complicated enough, I think you should send that out as a first patch.</span><span style="font-size:13px"> </span></div><div class="gmail_signature"><span style="font-size:13px"><br></span></div><div class="gmail_signature"><span style="font-size:13px">I think there is a confusion here. I never pulled the ATI into the AST, I only pulled it out of the Sema class. It has literally just gone from being a nested class within Sema to being a class at global scope (clang namespace, of course!), that's all. There is no change whatsoever in the code except for a change in scope and header-file placement (to avoid including the Sema header, and just because nested classes are a bad idea (I come from Boost library development, the design guidelines are more strict over there)).</span></div><div class="gmail_signature"><span style="font-size:13px"><br></span></div><div class="gmail_signature"><span style="font-size:13px">If you want to work on making ATIs a part of the AST, you're welcome to try / propose it. It is none of my concern, and in fact I would object to it (see below). </span></div><div class="gmail_signature"><br></div><div class="gmail_signature">> <span style="font-size:13px">There's also template instantiation for different reasons which are not currently recorded (ie., due to overload resolution, due to virtual methods). Since that's a change to ATI, that would logically go next.</span></div><div class="gmail_signature"><span style="font-size:13px"><br></span></div><div class="gmail_signature"><span style="font-size:13px">You're right, the categories of template instantiations are too broad. That is something I would be interested in discussing and intend to bring up after my patch has gone through. The cases of overload resolution and virtual methods are not exactly the first ones that come to mind for me though. The additional categories I was going to propose are for differentiating class template instantiations, function declaration instantiations and function definition instantiations. The lack of categorization of those things currently cause templight traces to have ambiguous entries, which I would like to avoid.</span></div><div class="gmail_signature"><span style="font-size:13px"><br></span></div><div class="gmail_signature"><span style="font-size:13px">> </span><span style="font-size:13px">I think we usually use "Callbacks" instead of "Observer", similar to PPCallbacks?</span></div><div class="gmail_signature"><span style="font-size:13px"><br></span></div><div class="gmail_signature">Fine. When we get closer to getting this (D5767) patch through, that can easily be changed, no problem.</div><div class="gmail_signature"><br></div><div class="gmail_signature">> <span style="font-size:13px">At a high level, I think that what we currently do in Sema by tracking template instantiations as a stack </span><span style="font-size:13px">should be available in the AST after instantiation is complete.</span></div><div class="gmail_signature"><span style="font-size:13px"><br></span></div><div class="gmail_signature"><span style="font-size:13px">I'm not sure if this makes sense in terms of the AST, after all, the stack of template instantiations are compilation steps, not actual entities in the code. My understanding is that the AST is only there to record all the entities in the code. I see this as a case of semantic pollution. What other records of compilation steps are recorded in the AST?</span></div><div class="gmail_signature"><span style="font-size:13px"><br></span></div><div class="gmail_signature"><span style="font-size:13px">Also, if the AST is to keep a record of all the template instantiations that occurred, this could significantly increase memory consumption (and thus, speed as well). It is a good thing that the compiler throws away this information because it would and could amount to a lot if it kept it all (I would say approximately 5-10% increase in memory consumption, from my experience). This would be a lot of passive overhead (and therefore, would have to be disabled by default, of course). I certainly would not want such overhead by default in any compiler that I use (and I'm not sure that people who use libclang would want this either).</span></div><div class="gmail_signature"><span style="font-size:13px"><br></span></div><div class="gmail_signature"><span style="font-size:13px">> </span><span style="font-size:13px">Then we could query the AST to ask why something was instantiated instead of tracing the act of instantiation. Is there any use case for templight where this wouldn't work?</span></div><div class="gmail_signature"><span style="font-size:13px"><br></span></div><div class="gmail_signature"><span style="font-size:13px">Yes, pretty much all of templight as a profiler would not work that way, unless the AST also held records of time- and memory- consumption during those instantiation steps. The AST is useful to the templight debugger and is also used in conjunction with the templight traces by the metashell utilities (that Zoltan linked to). The AST describes the code, the templight traces describe the compilation process, they are entirely orthogonal features (yet complementary), as far as I'm concerned.</span></div><div class="gmail_signature"><span style="font-size:13px"><br></span></div><div class="gmail_signature"><span style="font-size:13px">To me, it is important to stress that, although the debugging and interactive querying of the AST is a neat feature, the real "production-critical" feature here is the profiling. This is my primary interest, and those of my peers, i.e., the people who write production template-heavy code that need to optimize the compilation time and memory. Focusing too much on the debugging and AST querying aspects could impede that primary goal, as far as I'm concerned. </span></div><div class="gmail_signature"><span style="font-size:13px"><br></span></div><div class="gmail_signature"><span style="font-size:13px">And as Zoltan mentioned, there could be issues with the safe-mode and dealing with failures to instantiated the templates if we are going to rely on "successful" AST nodes to describe the compilation process. </span></div><div class="gmail_signature"><span style="font-size:13px"><br></span></div><div class="gmail_signature"><span style="font-size:13px">There are basically three use-case categories for templight (from most "novice" to most "professional"): learning, debugging and profiling. Learning and debugging imply that things might be failing a lot, and profiling imply that there is no need to understand the process but a need to identify performance bottlenecks. Your approach does not serve any of these domains particularly well.</span></div><div class="gmail_signature"><span style="font-size:13px"><br></span></div><div class="gmail_signature"><span style="font-size:13px"><br></span></div><div class="gmail_signature"><span style="font-size:13px">Thanks,</span></div><div class="gmail_signature"><span style="font-size:13px">Mikael.</span></div><div class="gmail_signature"><span style="font-size:13px"><br></span></div><div class="gmail_signature"><span style="font-size:13px"><br></span></div><div class="gmail_signature"><span style="font-size:13px"><br></span></div><div class="gmail_signature"><br></div>
</div></div>