PR9824: -Wunused-but-set

I'm just looking at PR9824 now & starting with some sanity checking of
existing related warnings.

One result of fixing PR11550 was that given this (taken from
test/SemaCXX/warn-unused-variables.cpp):

  struct S1 { S1(); };
  S1 makeS1();

we warn on this:

  S1 x = makeS1();

under the reasoning that the user could just drop the named variable
and call the function alone: makeS1();

but we don't warn on this:

  S1 x;

even though the equivalent suggestion would be to unname this variable
& simply invoke the default ctor: S1();

Is that reasonable/acceptable/correct? (Should we perhaps have a fixit
in the first case explaining that the user should just call the
function instead? Perhaps that's too hard to write in the presence of
multiple declarators)

From the examples in the bug, GCC doesn't warn about unused-but-set

for code like this:

   int i = 0; // no warning
   i = 1;
   &x; // warning about unused-value here

But it seems it'd be useful to indicate that the initializationg of
zero is completely unused when it's overwritten with 1 in all cases
anyway. Does that seem reasonable?

Ted: you mentioned there's already a dead-store checker in the SA? Was
your intent to basically port that up to the compiler proper? Are
there major performance concerns about it/refactoring that would be
required? Or will it just have to be basically reimplemented from
scratch for it to be compiler-appropriate?

I'm just looking at PR9824 now & starting with some sanity checking of
existing related warnings.

One result of fixing PR11550 was that given this (taken from
test/SemaCXX/warn-unused-variables.cpp):

struct S1 { S1(); };
S1 makeS1();

we warn on this:

S1 x = makeS1();

under the reasoning that the user could just drop the named variable
and call the function alone: makeS1();

but we don't warn on this:

S1 x;

even though the equivalent suggestion would be to unname this variable
& simply invoke the default ctor: S1();

Is that reasonable/acceptable/correct? (Should we perhaps have a fixit
in the first case explaining that the user should just call the
function instead? Perhaps that's too hard to write in the presence of
multiple declarators)

FWIW, both of these are only correct if the destructor is trivial. I'm hoping we already get that right.

I'm just looking at PR9824 now & starting with some sanity checking of
existing related warnings.

One result of fixing PR11550 was that given this (taken from
test/SemaCXX/warn-unused-variables.cpp):

struct S1 { S1(); };
S1 makeS1();

we warn on this:

S1 x = makeS1();

under the reasoning that the user could just drop the named variable
and call the function alone: makeS1();

but we don't warn on this:

S1 x;

even though the equivalent suggestion would be to unname this variable
& simply invoke the default ctor: S1();

Is that reasonable/acceptable/correct? (Should we perhaps have a fixit
in the first case explaining that the user should just call the
function instead? Perhaps that's too hard to write in the presence of
multiple declarators)

FWIW, both of these are only correct if the destructor is trivial. I'm hoping we already get that right.

Sorry, I could've clarified that. We do get that correct.

I'm just looking at PR9824 now & starting with some sanity checking of
existing related warnings.

One result of fixing PR11550 was that given this (taken from
test/SemaCXX/warn-unused-variables.cpp):

  struct S1 { S1(); };
  S1 makeS1();

we warn on this:

  S1 x = makeS1();

under the reasoning that the user could just drop the named variable
and call the function alone: makeS1();

but we don't warn on this:

  S1 x;

even though the equivalent suggestion would be to unname this variable
& simply invoke the default ctor: S1();

Is that reasonable/acceptable/correct?

This feels right to me. Calling a constructor feels more "normal" than
invoking a constructor directly. It's a subjective thing, so clang
shouldn't warn about either I think.