[PATCH] D35789: [XRay][compiler-rt] Do not print the warning when the binary is not XRay instrumented.

[David Blaikie via llvm-commits]

On Sun, Jul 30, 2017 at 10:07 PM Dean Michael Berris via Phabricator <
reviews at reviews.llvm.org> wrote:

> dberris marked an inline comment as done.
> dberris added inline comments.
>
>
> ================
> Comment at: test/xray/TestCases/Linux/quiet-start.cc:5-8
> +// RUN: XRAY_OPTIONS="patch_premain=true verbosity=1" %run %t 2>&1 | \
> +// RUN:    FileCheck %s --check-prefix NOISY
> +// RUN: XRAY_OPTIONS="patch_premain=true verbosity=0" %run %t 2>&1 | \
> +// RUN:    FileCheck %s --check-prefix QUIET
> ----------------
> dblaikie wrote:
> > Does this test the default (the comment says it checks what happens when
> there are no XRay instrumentation sleds - but the code seems to explicitly
> specify verbosity, rather than relying on any default based on the
> presence/absence of sleds? (unless that would override the XRAY_OPTIONS?))
> >
> > Seems like it might be better to phrase this whole change, etc in terms
> of tying it into the verbosity level & mention as an aside that this means
> it defaults to off. (& I don't think it's necessary to test that default if
> the compiler-rt verbosity work already handles the default, tests &
> documents it, etc))
> Added a test that will use an empty XRAY_OPTIONS environment variable, so
> that we're testing all the defaults.
>

If the default verbosity is already tested elsewhere, I wouldn't bother
testing them here. Good to keep things focused. (alternatively I wouldn't
bother testing the explicit handling if that's tested elsewhere & might use
the default in this test, etc)

- Dave


>
>
> https://reviews.llvm.org/D35789
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170731/a99c729d/attachment.html>