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

[Sean Silva]

On Thu, Jan 8, 2015 at 11:23 PM, Mikael Persson <mikael.s.persson at gmail.com>
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.
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150109/ca0630c5/attachment.html>