[PATCH] Template Instantiation Observer + a few other templight-related changes

[Mikael Persson]

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?


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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150109/fe965af8/attachment.html>