Skip to content

Commit

Permalink
[test] Disable the -Wself-move warning that triggers on exactly what …
Browse files Browse the repository at this point in the history 
…is being tested (NFC)

/Users/jiefu/llvm-project/llvm/unittests/Option/OptionParsingTest.cpp:251:6: error: explicitly moving variable of type 'InputArgList' to itself [-Werror,-Wself-move]
  AL = std::move(AL);
  ~~ ^           ~~
1 error generated.
  • Loading branch information
@DamonFool
DamonFool committed Aug 24, 2023
1 parent d2c37fc commit 4e627a3
Showing 1 changed file with 10 additions and 0 deletions.
10 changes: 10 additions & 0 deletions llvm/unittests/Option/OptionParsingTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,12 @@ TYPED_TEST(OptTableTest, IgnoreCase) {
EXPECT_TRUE(AL.hasArg(OPT_B));
}

#if defined(__clang__)
// Disable the warning that triggers on exactly what is being tested.
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wself-move"
#endif

TYPED_TEST(OptTableTest, InputArgListSelfAssign) {
TypeParam T;
unsigned MAI, MAC;
Expand All @@ -255,6 +261,10 @@ TYPED_TEST(OptTableTest, InputArgListSelfAssign) {
EXPECT_FALSE(AL.hasArg(OPT_SLASH_C));
}

#if defined(__clang__)
#pragma clang diagnostic pop
#endif

TYPED_TEST(OptTableTest, DoNotIgnoreCase) {
TypeParam T;
unsigned MAI, MAC;
Expand Down

6 comments on commit 4e627a3

@andykaylor
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this fix. I was just about to do the same thing. I introduced this problem with https://reviews.llvm.org/D158134.

I see that in a similar case (llvm/unittests/ADT/APIntTest.cpp) this is done before disabling Wself-move:

// Disable the pragma warning from versions of Clang without -Wself-move
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wunknown-pragmas"

Is that needed?

@DamonFool
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It passed on my local machine without ignoring "-Wunknown-pragmas".

You mean not all clang compilers would recognize -Wself-move?
I'm not sure about it.

Let's see if the buildbots would complain about it.
Thanks.

@andykaylor
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like -Wself-move was added nine years ago. I doubt we'll see it causing problems.

@DamonFool
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay.
I'll fix it once the error occurs.
Thanks.

Just wondering: can the llvm-project be built successfully with clang versions 9 years ago?

@andykaylor
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering: can the llvm-project be built successfully with clang versions 9 years ago?

Good question. No, it can't. I guess that makes this a closed issue.

@dwblaikie
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might still be nicer to slightly obfuscate the code to avoid the warning, rather than having to do all the pragma shenanigans?

  A a = ...;
  A &ar = a;
  a = ar;

Please sign in to comment.