On Thu, Jan 8, 2015 at 11:23 PM, Mikael Persson <[email protected]> wrote:
> > Nick Lewycky: > > 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. > > 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). > > 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. > > 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. > > > Sean Silva: > > (also, minor bikeshed, but naming this *Callbacks instead of *Observer > to match with PPCallbacks would be nice) > > That was done in the fixed patch. Do you know how to update the patch (not > just upload as attachment) on the phabricator system? > 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. > > > Sean Silva: > > 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? > > 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). > > Cheers, > Mikael. > > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
