-Wunused-variable for user-defined types

Hi all,

I find clang’s unused variable warning very useful to keep the code from accumulating garbage over time. For built-in types it works great, but I’ve noticed that for user-defined types it often misses unused variables. The current logic for user-defined types is to warn on unused variable if 1) the type has a trivial destructor 2) the constructor is either elided or trivial, or the type is marked with warn_unused attribute [1].
Does anyone object to extending 2) to allow constexpr constructors? The reasoning is that constexpr constructors are not supposed to have any side-effects.
What about a check that constructor does not have side-effects, even if it’s not marked as constexpr? This can start by using the same code as the one for constexpr checking. Is this the right thing in the compiler, or should this go into clang-tidy?

[1] http://clang.llvm.org/doxygen/SemaDecl_8cpp_source.html#l01426

Thanks,
Eugene

Hi all,

I find clang's unused variable warning very useful to keep the code from
accumulating garbage over time. For built-in types it works great, but I've
noticed that for user-defined types it often misses unused variables. The
current logic for user-defined types is to warn on unused variable if 1)
the type has a trivial destructor 2) the constructor is either elided or
trivial, or the type is marked with warn_unused attribute [1].
Does anyone object to extending 2) to allow constexpr constructors? The
reasoning is that constexpr constructors are not supposed to have any
side-effects.

Not quite true, unfortunately - a constexpr function (or constructor) /can/
be used in constexpr contexts but only has to have no side effects for the
/particular/ codepaths that are called from constexpr contexts.

eg:

constexpr int func(bool b) { if (b) printf("stuff") return 3; }

constexpr int x = func(false);
const int y = func(true);

is valid code, as I understand it. But we might be able to check the
particular caller, to see if it's constexpr - but that could have
performance implications.

What about a check that constructor does not have side-effects, even if
it's not marked as constexpr? This can start by using the same code as the
one for constexpr checking. Is this the right thing in the compiler, or
should this go into clang-tidy?

Might be a bit too much to do.

The aggresive way to do this would be to assume all types are "Value" types
and warn on all unused variables. Then add an attribute to annotate types
that have side effects (and even specifically types with scoped side
effects - such as MutexLock, so you could warn on "MutexLock(n);" but not
warn on "MutexLock(n), func();")

Hi all,

I find clang's unused variable warning very useful to keep the code from
accumulating garbage over time. For built-in types it works great, but I've
noticed that for user-defined types it often misses unused variables. The
current logic for user-defined types is to warn on unused variable if 1)
the type has a trivial destructor 2) the constructor is either elided or
trivial, or the type is marked with warn_unused attribute [1].
Does anyone object to extending 2) to allow constexpr constructors? The
reasoning is that constexpr constructors are not supposed to have any
side-effects.

Not quite true, unfortunately - a constexpr function (or constructor)
/can/ be used in constexpr contexts but only has to have no side effects
for the /particular/ codepaths that are called from constexpr contexts.

eg:

constexpr int func(bool b) { if (b) printf("stuff") return 3; }

constexpr int x = func(false);
const int y = func(true);

is valid code, as I understand it. But we might be able to check the
particular caller, to see if it's constexpr - but that could have
performance implications.

In some cases, we'll be checking whether the initialization is a constant
expression anyway, and it seems reasonable to extend the warning to cover
those cases. I'm not sure whether there'll be a noticeable performance
penalty for doing this to all local variables; I suspect not, since at
least CodeGen is likely to trigger the constant-evaluation of their
initializers.

What about a check that constructor does not have side-effects, even if

it's not marked as constexpr? This can start by using the same code as the
one for constexpr checking. Is this the right thing in the compiler, or
should this go into clang-tidy?

Might be a bit too much to do.

The aggresive way to do this would be to assume all types are "Value"
types and warn on all unused variables. Then add an attribute to annotate
types that have side effects (and even specifically types with scoped side
effects - such as MutexLock, so you could warn on "MutexLock(n);" but not
warn on "MutexLock(n), func();")

Maybe under a separate warning flag?

Hi all,

I find clang's unused variable warning very useful to keep the code from
accumulating garbage over time. For built-in types it works great, but I've
noticed that for user-defined types it often misses unused variables. The
current logic for user-defined types is to warn on unused variable if 1)
the type has a trivial destructor 2) the constructor is either elided or
trivial, or the type is marked with warn_unused attribute [1].
Does anyone object to extending 2) to allow constexpr constructors? The
reasoning is that constexpr constructors are not supposed to have any
side-effects.

Not quite true, unfortunately - a constexpr function (or constructor)
/can/ be used in constexpr contexts but only has to have no side effects
for the /particular/ codepaths that are called from constexpr contexts.

eg:

constexpr int func(bool b) { if (b) printf("stuff") return 3; }

constexpr int x = func(false);
const int y = func(true);

is valid code, as I understand it. But we might be able to check the
particular caller, to see if it's constexpr - but that could have
performance implications.

In some cases, we'll be checking whether the initialization is a constant
expression anyway, and it seems reasonable to extend the warning to cover
those cases. I'm not sure whether there'll be a noticeable performance
penalty for doing this to all local variables; I suspect not, since at
least CodeGen is likely to trigger the constant-evaluation of their
initializers.

Yep - I was wondering if we'd be doing the work in many cases anyway & able
to leverage that. Sounds good.

What about a check that constructor does not have side-effects, even if

it's not marked as constexpr? This can start by using the same code as the
one for constexpr checking. Is this the right thing in the compiler, or
should this go into clang-tidy?

Might be a bit too much to do.

The aggresive way to do this would be to assume all types are "Value"
types and warn on all unused variables. Then add an attribute to annotate
types that have side effects (and even specifically types with scoped side
effects - such as MutexLock, so you could warn on "MutexLock(n);" but not
warn on "MutexLock(n), func();")

Maybe under a separate warning flag?

Right, yes.

Not quite true, unfortunately - a constexpr function (or constructor)
/can/ be used in constexpr contexts but only has to have no side effects
for the /particular/ codepaths that are called from constexpr contexts.

eg:

constexpr int func(bool b) { if (b) printf("stuff") return 3; }

constexpr int x = func(false);
const int y = func(true);

is valid code, as I understand it. But we might be able to check the
particular caller, to see if it's constexpr - but that could have
performance implications.

Good point! I missed that.

In some cases, we'll be checking whether the initialization is a constant
expression anyway, and it seems reasonable to extend the warning to cover
those cases. I'm not sure whether there'll be a noticeable performance
penalty for doing this to all local variables; I suspect not, since at
least CodeGen is likely to trigger the constant-evaluation of their
initializers.

Yep - I was wondering if we'd be doing the work in many cases anyway &
able to leverage that. Sounds good.

For the unused variable warning we don't need to check every local variable
-- only the unused ones. But this does make the code change a lot more
involved. How about a change that's only triggered by constexpr
constructors with no arguments? My understanding is such constructors
should not have side-effects (7.1.5/5).

Eugene