<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jan 8, 2015 at 11:23 PM, Mikael Persson <span dir="ltr"><<a href="mailto:mikael.s.persson@gmail.com" target="_blank">mikael.s.persson@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><br></div><div>Nick Lewycky:</div><span><div>> <span style="font-size:13px">We could then write the tests with FileCheck. I realize this isn't great as a testing apparatus because you don't want tests to be too closely coupled to implementation details, but we'll need something that exercizes the new code in this patch.</span></div><div><span style="font-size:13px"><br></span></div></span><div><span style="font-size:13px">That's a good point, about exercising the new code, I didn't think about that aspect (I assumed no tests were needed because all the changes are "passive" features that only get activated by a tool like templight). Writing such tests might prove to be quite a challenge because there are no clear "invariants" to test for. Part of the problem is that the actual output (traces) depends on a lot more than the code of this patch, e.g., it depends on choices made by clang about when and how to instantiate templates. The typical information in traces (like point-of-instantiation, entity names, etc..) are not necessarily invariant either (e.g., in the future, clang could get "better" at tracking instantiation points, or clang could change entities names slightly).</span></div><div><span style="font-size:13px"><br></span></div><div><span style="font-size:13px">I think that test code for this patch would have to be very minimal about what it requires to judge that a trace is "correct". First, the raw trace files would have to be very minimal, to avoid recording information that might naturally change in the future (like PoI, names, etc.). Second, the test for correctness might have to ignore differences in the ordering of instantiations (when they don't matter, really), which implies additional logic besides a simple file-check. I have some ideas on how such a criteria would be (e.g., comparing the graph topology), but this is not going to be trivial.</span></div><div><span style="font-size:13px"><br></span></div><div><span style="font-size:13px">I'm just worried that this could become the kind of test that you constantly have to "update it to pass it". Which leads to lots of maintenance effort and increased chances of one of these "updates" leading to a weaker / wrong test.</span></div><div><br></div><div><br></div><div>Sean Silva:</div><span><div>> <span style="font-size:13px">(also, minor bikeshed, but naming this *Callbacks instead of *Observer to match with PPCallbacks would be nice)</span></div><div><span style="font-size:13px"><br></span></div></span><div><span style="font-size:13px">That was done in the fixed patch. Do you know how to update the patch (not just upload as attachment) on the phabricator system?</span></div></div></blockquote><div><br></div><div>Sorry, no idea off the top of my head; I don't really use phabricator for submitting patches. Maybe check phabricator's docs or arc's help.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><br></div><div><br></div><div><span style="font-size:13px">Sean Silva:</span><br></div><span>> <span style="font-size:13px">I'm wondering where this non-determinism comes from. I would certain ephemeral computations inside clang to proceed non-deterministically (but are supposed to have a deterministic final result) due to ASLR and the pervasive use of pointers as keys. If you turn off ASLR, does the non-determinism go away?</span><div style="font-size:13px"><br></div></span><div style="font-size:13px">If the non-determinism is no intended and as it seems to be a big concern for you guys, I'll see what I can do in terms of getting some reproducible examples (maybe with some of the more template-heavy Boost library example codes that I've been using for testing out templight). Your intuition about ASLR sounds like it could indeed be the cause. So far, what I've been able to see as non-determinism in the templight traces have involved template instantiations coming out in different orders (i.e., using pointers as keys might have the effect of changing the order of some instantiations) and getting different numbers of memoizations (if I suppress all memoization outputs from the tracers, the total number of instantiation is always the same (I think, but I must confirm this), but when memoizations are outputted, then there is about +-2% variation in the number of instantiations outputted).</div><div style="font-size:13px"><br></div><div style="font-size:13px">Cheers,</div><div style="font-size:13px">Mikael.</div><div style="font-size:13px"><br></div><div class="gmail_extra"><div><br></div>
</div></div>
</blockquote></div><br></div></div>