[PATCH] D19754: Allow 'nodebug' on local variables

[David Blaikie via cfe-commits]

On Thu, May 5, 2016 at 8:50 AM, Robinson, Paul <paul.robinson at sony.com>
wrote:

> This would be a great conversation to have at the social, sadly I will
> have to miss it this month.
>

Yeah, I don't often make it along to them, unfortunately.


>
> >> dblaikie wrote:
> >>> Doesn't look like the const case is any different from the non-const
> >>> case, is it?
> >> Given a white-box analysis of the compiler internals, you are correct;
> >> this is in contrast to the static const/non-const handling, which *does*
> >> use different paths.
> >> I am unwilling to trust that the const/non-const paths for locals will
> >> forever use the same path.  You could also look at it as "the test is
> >> the spec."
> >
> > But even then - what about any other property of a variable? What if it's
> > in a nested scope instead of a top level scope? What if it's declared in
> > a condition (if (int x = ...)). What if it's volatile? We could pick
> > arbitrary properties that /could/ have an effect on debug info but we
> > generally believe to be orthogonal to the feature at hand
>
> What you are describing is what testing literature refers to as criteria
> for equivalence classes.  There is some level of judgment to that, yes.
>

Yep yep, to be sure. I'm just generally trying to encourage the community
behavior towards being both selective & thorough about testing.


> > & I'd say 'const' is in the same group of things.
>
> I would have thought exactly that for the static-storage case, but it is
> demonstrably not true.


Could you provide the example you had in mind there? I suspect there's a
bit of a false equivalence there - that, once we understand the language
rules/constructs better, we'll see it doesn't apply here to the local
variable case & wouldn't be used to inform our equivalence classification.
(I'll skip other parts where you refer to that issue in this email until we
discuss it further here)


> Therefore, const/not is a valid distinction for
> the equivalence classes in the static case.  Needing a separate const
> test for the static case, it seems completely appropriate to have the
> same for the auto case.  In other words, in my judgment the storage-class
> doesn't seem relevant to the equivalence class criteria for the test.
>
> > (a const local variable still needs storage, etc, - the address can be
> > escaped and accessed from elsewhere and determined to be unique
>
> All of those objections apply equally to the static case, and yet the
> static case must have separate tests.
>
> > - and both variables in this example could be optimized away entirely
> > by the compiler because they're unused, so the const is no worse off
> > there in that theoretical concern)
>
> Again not different for the (file-)static case.  If a file-static
> variable is not used in the CU there's no reason for it to be emitted.
> As it happens I *did* need uses to get the static cases to work, and
> (currently) don't need uses to get the local cases to work, so in the
> interest of not including elements in the test case that are irrelevant
> to the feature-under-test, I didn't add uses of the locals.
>

But I think that points out that the parallel doesn't apply here - uses are
critical to the static variable case (well, as you saw - uses of the type,
at least, and separately, definitions of the static members are also
necessary to test that codepath too). I don't think there's an equivalence
between this and the local variable case until we see the variable
disappear due to lack of use. Then we'll get into the territory of "what
kind of use is enough" to preserve the variable to demonstrate the debug
info is present/not present - but even then, the const isn't so relevant.
The const would just allow the variable to be optimized away more
frequently & thus not be present to demonstrate the attribute's
functionality.

Sorry, I'm perhaps having a hard time explaining this well.


>
> This is an argument for doing the test exactly as I did: first run it
> to prove debug info IS emitted in the absence of the attribute, then
> again to prove debug info is suppressed in the presence of the attribute.
> That way if optimization or lack-of-use means the variable is not emitted,
> the test can be adjusted to make sure that condition does not apply,
> proving that the lack of debug info is properly because of the attribute
> and not because of some other irrelevant circumstance.
>

Yeah, I think that's a separate and interesting part of testing too -
making the test case resilient to other changes (in this case - the
possibility of frontend optimizations that might remove the variable
entirely before we have a chance to test the attribute).

Another way I would suggest to approach this would be to make it impossible
for the compiler to mess this up - but that also "complicates" the test a
bit too:

void f1(int&);
void f2() {
  ...attribute... int x;
  f1(x);
}

That way the compiler can't optimize away x, has to put it somewhere in
memory, etc.


>
> --paulr
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160505/e2029c43/attachment.html>