This is an archive of the discontinued LLVM Phabricator instance.

[cmake] Disable GCC 9's -Wredundant-move

Authored by labath on Dec 3 2019, 7:26 AM.



This new warning (enabled by -Wextra) fires when a std::move is
redundant, as the default compiler behavior would be to select a move
operation anyway (e.g., when returning a local variable). Unlike
-Wpessimizing-move, it has no performance impact -- it just adds noise.

Currently llvm has about 1500 of these warnings. Unfortunately, the
suggested fix -- removing std::move -- does not work because of some
older compilers we still support. Specifically clang<=3.8 will not use a
move operation if an implicit conversion is needed (Core issue 1579). In
code like "A f(ConvertibleToA a) { return a; }" it will prefer a copy,
or fail to compile if a copy is not possible.

This patch disables that warning to get a meaningful signal out of a GCC
9 build.

Diff Detail

Event Timeline

labath created this revision.Dec 3 2019, 7:26 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 3 2019, 7:26 AM
Herald added a subscriber: mgorny. · View Herald Transcript

Looks like Clang has -Wredundant-move too (part of -Wall). Is it less noisy? I dont see such warnings when boostraping clang. So maybe worth to leave it on for Clang?

labath planned changes to this revision.Dec 3 2019, 7:50 AM

Interesting. I didn't realize that. I'll take a closer look at that and get back here...

labath updated this revision to Diff 231927.Dec 3 2019, 9:16 AM

Leave the warning on for clang (thanks David).

labath added a subscriber: rtrieu.Dec 3 2019, 9:17 AM

After some archaeology, this is what I've learned. Clang used to (a very long time ago) have -Wredundant-move behavior which mostly matched what the current gcc does. Then, in rL243594 (2015), it's scope was reduced to only cover the simple cases (no type conversion involved). It seems the reason for that was that clang at that time did not implement DR1579, and so the thing which the warning suggested to do was actually invalid. Some time later in 2016, DR1579 was implemented (rL274291). However, the warning remained unchanged.

Anyway, it seems that we can leave the warning enabled for clang, at least until it doesn't become more aggressive.

+@rtrieu (author of rL243594) to see whether it's worth re-enabling that warning now.

Ok, thanks for more info.

I think it is fine to disable this warning for gcc and clang too.

aaron.ballman accepted this revision.Dec 6 2019, 1:32 PM

This seems reasonable to me.

This revision is now accepted and ready to land.Dec 6 2019, 1:32 PM
This revision was automatically updated to reflect the committed changes.