diff options
author | Roman Lebedev <lebedev.ri@gmail.com> | 2018-04-07 10:37:18 +0000 |
---|---|---|
committer | Roman Lebedev <lebedev.ri@gmail.com> | 2018-04-07 10:37:18 +0000 |
commit | 429b00325ffc64168628a8bab38b1df3c4d5a3ae (patch) | |
tree | bd14a322d0a83bf3b2a9f23c5138b01409b3d25b /llvm/unittests/ADT/DenseMapTest.cpp | |
parent | 6669e59f88271595b41829144005f03ab59da7a7 (diff) | |
download | llvm-429b00325ffc64168628a8bab38b1df3c4d5a3ae.zip llvm-429b00325ffc64168628a8bab38b1df3c4d5a3ae.tar.gz llvm-429b00325ffc64168628a8bab38b1df3c4d5a3ae.tar.bz2 |
[unittests] ADT: silence -Wself-assign diagnostics
Summary:
D44883 extends -Wself-assign to also work on C++ classes.
In it's current state (as suggested by @rjmccall), it is not under it's own sub-group.
Since that diag is enabled by `-Wall`, stage2 testing showed that:
* It does not fire on any llvm code
* It does fire for these 3 unittests
* It does fire for libc++ tests
This diff simply silences those new warnings in llvm's unittests.
A similar diff will be needed for libcxx. (`libcxx/test/std/language.support/support.types/byteops/`, maybe something else)
Since i don't think we want to repeat rL322901, let's talk about it.
I've subscribed everyone who i think might be interested...
There are several ways forward:
* Not extend -Wself-assign, close D44883. Not very productive outcome i'd say.
* Keep D44883 in it's current state.
Unless your custom overloaded operators do something unusual for when self-assigning,
the warning is no less of a false-positive than the current -Wself-assign.
Except for tests of course, there you'd want to silence it. The current suggestion is:
```
S a;
a = (S &)a;
```
* Split the diagnostic in two - `-Wself-assign-builtin` (i.e. what is `-Wself-assign` in trunk),
and `-Wself-assign-overloaded` - the new part in D44883.
Since, as i said, i'm not really sure why it would be less of a error than the current `-Wself-assign`,
both would still be in `-Wall`. That way one could simply pass `-Wno-self-assign-overloaded` for all the tests.
Pretty simple to do, and will surely work.
* Split the diagnostic in two - `-Wself-assign-trivial`, and `-Wself-assign-nontrivial`.
The choice of which diag to emit would depend on trivial-ness of that particular operator.
The current `-Wself-assign` would be `-Wself-assign-trivial`.
https://godbolt.org/g/gwDASe - `A`, `B` and `C` case would be treated as trivial, and `D`, `E` and `F` as non-trivial.
Will be the most complicated to implement.
Thoughts?
Reviewers: aaron.ballman, rsmith, rtrieu, rjmccall, dblaikie, atrick, gottesmm
Reviewed By: dblaikie
Subscribers: lebedev.ri, phosek, vsk, rnk, thakis, sammccall, mclow.lists, llvm-commits, rjmccall
Differential Revision: https://reviews.llvm.org/D45082
llvm-svn: 329491
Diffstat (limited to 'llvm/unittests/ADT/DenseMapTest.cpp')
-rw-r--r-- | llvm/unittests/ADT/DenseMapTest.cpp | 4 |
1 files changed, 2 insertions, 2 deletions
diff --git a/llvm/unittests/ADT/DenseMapTest.cpp b/llvm/unittests/ADT/DenseMapTest.cpp index 299d5019..7f52c54 100644 --- a/llvm/unittests/ADT/DenseMapTest.cpp +++ b/llvm/unittests/ADT/DenseMapTest.cpp @@ -247,7 +247,7 @@ TYPED_TEST(DenseMapTest, AssignmentTest) { EXPECT_EQ(this->getValue(), copyMap[this->getKey()]); // test self-assignment. - copyMap = copyMap; + copyMap = static_cast<TypeParam &>(copyMap); EXPECT_EQ(1u, copyMap.size()); EXPECT_EQ(this->getValue(), copyMap[this->getKey()]); } @@ -262,7 +262,7 @@ TYPED_TEST(DenseMapTest, AssignmentTestNotSmall) { EXPECT_EQ(this->getValue(Key), copyMap[this->getKey(Key)]); // test self-assignment. - copyMap = copyMap; + copyMap = static_cast<TypeParam &>(copyMap); EXPECT_EQ(5u, copyMap.size()); for (int Key = 0; Key < 5; ++Key) EXPECT_EQ(this->getValue(Key), copyMap[this->getKey(Key)]); |