This is an archive of the discontinued LLVM Phabricator instance.

[NFC] ASSERT_EQ before accessing items in containers
ClosedPublic

Authored by kbobyrev on Nov 21 2019, 2:01 AM.

Details

Summary

As discussed offline, something different from EXPECT_EQ should be used to check if the container contains enough items before accessing them so that other tests can still be run even if the assertion fails as opposed to having EXPECT_EQ failing and then aborting the run due to the errors caused by out-of-bounds memory access.

Diff Detail

Event Timeline

kbobyrev created this revision.Nov 21 2019, 2:01 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 21 2019, 2:01 AM
ilya-biryukov added inline comments.Nov 21 2019, 2:54 AM
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
1877

We should not break out of the loop here.
Maybe do

if (size() != 1) {
  ADD_FAILURE() << "";
  continue;
}
kbobyrev updated this revision to Diff 230412.Nov 21 2019, 3:02 AM
kbobyrev marked an inline comment as done.
ilya-biryukov accepted this revision.Nov 21 2019, 3:05 AM

LGTM, but please update the other diff as well!

clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
712

Same here, actually. We should not break out of the loop.

This revision is now accepted and ready to land.Nov 21 2019, 3:05 AM
kbobyrev updated this revision to Diff 230416.Nov 21 2019, 3:16 AM
kbobyrev marked an inline comment as done.
kbobyrev edited the summary of this revision. (Show Details)
ilya-biryukov added inline comments.Nov 21 2019, 3:20 AM
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
713

add continue!

kbobyrev updated this revision to Diff 230417.Nov 21 2019, 3:24 AM
kbobyrev marked an inline comment as done.
kbobyrev updated this revision to Diff 231046.Nov 26 2019, 4:44 AM

Rebase on top of master.

This revision was automatically updated to reflect the committed changes.