[llvm] r307440 - ProfData: Fix some unchecked Errors in unit tests
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 10 15:31:29 PDT 2017
Awesome - thanks!
On Mon, Jul 10, 2017 at 2:45 PM Vedant Kumar <vsk at apple.com> wrote:
> Done, thanks for catching this!
>
> r307596
>
> vedant
>
> On Jul 7, 2017, at 2:51 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Fri, Jul 7, 2017 at 2:03 PM David Blaikie via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: dblaikie
>> Date: Fri Jul 7 14:02:59 2017
>> New Revision: 307440
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=307440&view=rev
>> Log:
>> ProfData: Fix some unchecked Errors in unit tests
>>
>> The 'NoError' function was meant to be used as the input to
>> ASSERT/EXPECT_TRUE, but it is easy to forget this (it could be annotated
>> with nodiscard to help this) so many sites that look like they're checked
>> are not (& silently discard the failure). Only one site actually has an
>> Error sneaking out this way and I've replaced that one with a
>> FIXME+consumeError.
>>
>> The rest of the code has been modified to use the EXPECT_THAT_ERROR
>> macros Zach introduced a while back. Between the options available this
>> seems OK/good/something to standardize on - though it's difficult to
>> build a matcher that could handle checking for a specific llvm::Error
>> result, so those remain using the custom ErrorEquals (& the nodiscard
>> added to ensure it is not misused as it was previous to this patch). It
>> could still be generalized a bit further (even not as far as a matcher,
>> but at least support multiple kinds of Error, etc) & added to the
>> general Error utility header.
>>
>> Modified:
>> llvm/trunk/unittests/ProfileData/CMakeLists.txt
>> llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp
>> llvm/trunk/unittests/ProfileData/InstrProfTest.cpp
>>
>> Modified: llvm/trunk/unittests/ProfileData/CMakeLists.txt
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ProfileData/CMakeLists.txt?rev=307440&r1=307439&r2=307440&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/unittests/ProfileData/CMakeLists.txt (original)
>> +++ llvm/trunk/unittests/ProfileData/CMakeLists.txt Fri Jul 7 14:02:59
>> 2017
>> @@ -10,3 +10,5 @@ add_llvm_unittest(ProfileDataTests
>> InstrProfTest.cpp
>> SampleProfTest.cpp
>> )
>> +
>> +target_link_libraries(ProfileDataTests LLVMTestingSupport)
>>
>> Modified: llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp?rev=307440&r1=307439&r2=307440&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp (original)
>> +++ llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp Fri Jul 7
>> 14:02:59 2017
>> @@ -13,6 +13,8 @@
>> #include "llvm/ProfileData/InstrProfReader.h"
>> #include "llvm/ProfileData/InstrProfWriter.h"
>> #include "llvm/Support/raw_ostream.h"
>> +#include "llvm/Testing/Support/Error.h"
>> +#include "llvm/Testing/Support/SupportHelpers.h"
>> #include "gtest/gtest.h"
>>
>> #include <ostream>
>> @@ -21,15 +23,8 @@
>> using namespace llvm;
>> using namespace coverage;
>>
>> -static ::testing::AssertionResult NoError(Error E) {
>> - if (!E)
>> - return ::testing::AssertionSuccess();
>> - return ::testing::AssertionFailure() << "error: " <<
>> toString(std::move(E))
>> - << "\n";
>> -}
>> -
>> -static ::testing::AssertionResult ErrorEquals(coveragemap_error Expected,
>> - Error E) {
>> +LLVM_NODISCARD static ::testing::AssertionResult
>> +ErrorEquals(coveragemap_error Expected, Error E) {
>> coveragemap_error Found;
>> std::string FoundMsg;
>> handleAllErrors(std::move(E), [&](const CoverageMapError &CME) {
>> @@ -209,7 +204,7 @@ struct CoverageMappingTest : ::testing::
>> std::vector<CounterExpression> Expressions;
>> RawCoverageMappingReader Reader(Coverage, Filenames, Data.Filenames,
>> Expressions, Data.Regions);
>> - ASSERT_TRUE(NoError(Reader.read()));
>> + EXPECT_THAT_ERROR(Reader.read(), Succeeded());
>> }
>>
>> void writeAndReadCoverageRegions(bool EmitFilenames = true) {
>> @@ -227,7 +222,7 @@ struct CoverageMappingTest : ::testing::
>> void readProfCounts() {
>> auto Profile = ProfileWriter.writeBuffer();
>> auto ReaderOrErr =
>> IndexedInstrProfReader::create(std::move(Profile));
>> - ASSERT_TRUE(NoError(ReaderOrErr.takeError()));
>> + EXPECT_THAT_ERROR(ReaderOrErr.takeError(), Succeeded());
>> ProfileReader = std::move(ReaderOrErr.get());
>> }
>>
>> @@ -309,7 +304,8 @@ TEST_P(CoverageMappingTest, correct_dese
>> }
>>
>> TEST_P(CoverageMappingTest, load_coverage_for_more_than_two_files) {
>> - NoError(ProfileWriter.addRecord({"func", 0x1234, {0}}));
>> + EXPECT_THAT_ERROR(ProfileWriter.addRecord({"func", 0x1234, {0}}),
>> + Succeeded());
>>
>> const char *FileNames[] = {"bar", "baz", "foo"};
>> static const unsigned N = array_lengthof(FileNames);
>> @@ -320,7 +316,7 @@ TEST_P(CoverageMappingTest, load_coverag
>> // in order to preserve that information during possible sorting of
>> CMRs.
>> addCMR(Counter::getCounter(0), FileNames[I], I, 1, I, 1);
>>
>> - NoError(loadCoverageMapping());
>> + EXPECT_THAT_ERROR(loadCoverageMapping(), Succeeded());
>>
>> for (unsigned I = 0; I < N; ++I) {
>> CoverageData Data = LoadedCoverage->getCoverageForFile(FileNames[I]);
>> @@ -330,15 +326,17 @@ TEST_P(CoverageMappingTest, load_coverag
>> }
>>
>> TEST_P(CoverageMappingTest, load_coverage_with_bogus_function_name) {
>> - NoError(ProfileWriter.addRecord({"", 0x1234, {10}}));
>> + EXPECT_THAT_ERROR(ProfileWriter.addRecord({"", 0x1234, {10}}),
>> Succeeded());
>> startFunction("", 0x1234);
>> addCMR(Counter::getCounter(0), "foo", 1, 1, 5, 5);
>> - ErrorEquals(coveragemap_error::malformed, loadCoverageMapping());
>> + EXPECT_TRUE(ErrorEquals(coveragemap_error::malformed,
>> loadCoverageMapping()));
>> }
>>
>> TEST_P(CoverageMappingTest, load_coverage_for_several_functions) {
>> - NoError(ProfileWriter.addRecord({"func1", 0x1234, {10}}));
>> - NoError(ProfileWriter.addRecord({"func2", 0x2345, {20}}));
>> + EXPECT_THAT_ERROR(ProfileWriter.addRecord({"func1", 0x1234, {10}}),
>> + Succeeded());
>> + EXPECT_THAT_ERROR(ProfileWriter.addRecord({"func2", 0x2345, {20}}),
>> + Succeeded());
>>
>> startFunction("func1", 0x1234);
>> addCMR(Counter::getCounter(0), "foo", 1, 1, 5, 5);
>> @@ -346,7 +344,7 @@ TEST_P(CoverageMappingTest, load_coverag
>> startFunction("func2", 0x2345);
>> addCMR(Counter::getCounter(0), "bar", 2, 2, 6, 6);
>>
>> - NoError(loadCoverageMapping());
>> + EXPECT_THAT_ERROR(loadCoverageMapping(), Succeeded());
>>
>> const auto FunctionRecords = LoadedCoverage->getCoveredFunctions();
>> EXPECT_EQ(2, std::distance(FunctionRecords.begin(),
>> FunctionRecords.end()));
>> @@ -382,14 +380,15 @@ TEST_P(CoverageMappingTest, expansion_ge
>> }
>>
>> TEST_P(CoverageMappingTest, basic_coverage_iteration) {
>> - NoError(ProfileWriter.addRecord({"func", 0x1234, {30, 20, 10, 0}}));
>> + EXPECT_THAT_ERROR(ProfileWriter.addRecord({"func", 0x1234, {30, 20,
>> 10, 0}}),
>> + Succeeded());
>>
>> startFunction("func", 0x1234);
>> addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
>> addCMR(Counter::getCounter(1), "file1", 1, 1, 4, 7);
>> addCMR(Counter::getCounter(2), "file1", 5, 8, 9, 1);
>> addCMR(Counter::getCounter(3), "file1", 10, 10, 11, 11);
>> - NoError(loadCoverageMapping());
>> + EXPECT_THAT_ERROR(loadCoverageMapping(), Succeeded());
>>
>> CoverageData Data = LoadedCoverage->getCoverageForFile("file1");
>> std::vector<CoverageSegment> Segments(Data.begin(), Data.end());
>> @@ -406,7 +405,7 @@ TEST_P(CoverageMappingTest, basic_covera
>> TEST_P(CoverageMappingTest, uncovered_function) {
>> startFunction("func", 0x1234);
>> addCMR(Counter::getZero(), "file1", 1, 2, 3, 4);
>> - NoError(loadCoverageMapping());
>> + EXPECT_THAT_ERROR(loadCoverageMapping(), Succeeded());
>>
>> CoverageData Data = LoadedCoverage->getCoverageForFile("file1");
>> std::vector<CoverageSegment> Segments(Data.begin(), Data.end());
>> @@ -419,7 +418,7 @@ TEST_P(CoverageMappingTest, uncovered_fu
>> startFunction("func", 0x1234);
>> addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
>> addCMR(Counter::getCounter(1), "file1", 1, 1, 4, 7);
>> - NoError(loadCoverageMapping());
>> + EXPECT_THAT_ERROR(loadCoverageMapping(), Succeeded());
>>
>> CoverageData Data = LoadedCoverage->getCoverageForFile("file1");
>> std::vector<CoverageSegment> Segments(Data.begin(), Data.end());
>> @@ -430,13 +429,14 @@ TEST_P(CoverageMappingTest, uncovered_fu
>> }
>>
>> TEST_P(CoverageMappingTest, combine_regions) {
>> - NoError(ProfileWriter.addRecord({"func", 0x1234, {10, 20, 30}}));
>> + EXPECT_THAT_ERROR(ProfileWriter.addRecord({"func", 0x1234, {10, 20,
>> 30}}),
>> + Succeeded());
>>
>> startFunction("func", 0x1234);
>> addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
>> addCMR(Counter::getCounter(1), "file1", 3, 3, 4, 4);
>> addCMR(Counter::getCounter(2), "file1", 3, 3, 4, 4);
>> - NoError(loadCoverageMapping());
>> + EXPECT_THAT_ERROR(loadCoverageMapping(), Succeeded());
>>
>> CoverageData Data = LoadedCoverage->getCoverageForFile("file1");
>> std::vector<CoverageSegment> Segments(Data.begin(), Data.end());
>> @@ -448,13 +448,14 @@ TEST_P(CoverageMappingTest, combine_regi
>> }
>>
>> TEST_P(CoverageMappingTest,
>> restore_combined_counter_after_nested_region) {
>> - NoError(ProfileWriter.addRecord({"func", 0x1234, {10, 20, 40}}));
>> + EXPECT_THAT_ERROR(ProfileWriter.addRecord({"func", 0x1234, {10, 20,
>> 40}}),
>> + Succeeded());
>>
>> startFunction("func", 0x1234);
>> addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
>> addCMR(Counter::getCounter(1), "file1", 1, 1, 9, 9);
>> addCMR(Counter::getCounter(2), "file1", 3, 3, 5, 5);
>> - NoError(loadCoverageMapping());
>> + EXPECT_THAT_ERROR(loadCoverageMapping(), Succeeded());
>>
>> CoverageData Data = LoadedCoverage->getCoverageForFile("file1");
>> std::vector<CoverageSegment> Segments(Data.begin(), Data.end());
>> @@ -468,15 +469,17 @@ TEST_P(CoverageMappingTest, restore_comb
>> // If CodeRegions and ExpansionRegions cover the same area,
>> // only counts of CodeRegions should be used.
>> TEST_P(CoverageMappingTest, dont_combine_expansions) {
>> - NoError(ProfileWriter.addRecord({"func", 0x1234, {10, 20}}));
>> - NoError(ProfileWriter.addRecord({"func", 0x1234, {0, 0}}));
>> + EXPECT_THAT_ERROR(ProfileWriter.addRecord({"func", 0x1234, {10, 20}}),
>> + Succeeded());
>> + EXPECT_THAT_ERROR(ProfileWriter.addRecord({"func", 0x1234, {0, 0}}),
>> + Succeeded());
>>
>> startFunction("func", 0x1234);
>> addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
>> addCMR(Counter::getCounter(1), "file1", 3, 3, 4, 4);
>> addCMR(Counter::getCounter(1), "include1", 6, 6, 7, 7);
>> addExpansionCMR("file1", "include1", 3, 3, 4, 4);
>> - NoError(loadCoverageMapping());
>> + EXPECT_THAT_ERROR(loadCoverageMapping(), Succeeded());
>>
>> CoverageData Data = LoadedCoverage->getCoverageForFile("file1");
>> std::vector<CoverageSegment> Segments(Data.begin(), Data.end());
>> @@ -489,7 +492,8 @@ TEST_P(CoverageMappingTest, dont_combine
>>
>> // If an area is covered only by ExpansionRegions, they should be
>> combinated.
>> TEST_P(CoverageMappingTest, combine_expansions) {
>> - NoError(ProfileWriter.addRecord({"func", 0x1234, {2, 3, 7}}));
>> + EXPECT_THAT_ERROR(ProfileWriter.addRecord({"func", 0x1234, {2, 3, 7}}),
>> + Succeeded());
>>
>> startFunction("func", 0x1234);
>> addCMR(Counter::getCounter(1), "include1", 1, 1, 1, 10);
>> @@ -498,7 +502,7 @@ TEST_P(CoverageMappingTest, combine_expa
>> addExpansionCMR("file", "include1", 3, 1, 3, 5);
>> addExpansionCMR("file", "include2", 3, 1, 3, 5);
>>
>> - NoError(loadCoverageMapping());
>> + EXPECT_THAT_ERROR(loadCoverageMapping(), Succeeded());
>>
>> CoverageData Data = LoadedCoverage->getCoverageForFile("file");
>> std::vector<CoverageSegment> Segments(Data.begin(), Data.end());
>> @@ -510,11 +514,12 @@ TEST_P(CoverageMappingTest, combine_expa
>> }
>>
>> TEST_P(CoverageMappingTest, strip_filename_prefix) {
>> - NoError(ProfileWriter.addRecord({"file1:func", 0x1234, {0}}));
>> + EXPECT_THAT_ERROR(ProfileWriter.addRecord({"file1:func", 0x1234, {0}}),
>> + Succeeded());
>>
>> startFunction("file1:func", 0x1234);
>> addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
>> - NoError(loadCoverageMapping());
>> + EXPECT_THAT_ERROR(loadCoverageMapping(), Succeeded());
>>
>> std::vector<std::string> Names;
>> for (const auto &Func : LoadedCoverage->getCoveredFunctions())
>> @@ -524,11 +529,12 @@ TEST_P(CoverageMappingTest, strip_filena
>> }
>>
>> TEST_P(CoverageMappingTest, strip_unknown_filename_prefix) {
>> - NoError(ProfileWriter.addRecord({"<unknown>:func", 0x1234, {0}}));
>> + EXPECT_THAT_ERROR(ProfileWriter.addRecord({"<unknown>:func", 0x1234,
>> {0}}),
>> + Succeeded());
>>
>> startFunction("<unknown>:func", 0x1234);
>> addCMR(Counter::getCounter(0), "", 1, 1, 9, 9);
>> - NoError(loadCoverageMapping(/*EmitFilenames=*/false));
>> + EXPECT_THAT_ERROR(loadCoverageMapping(/*EmitFilenames=*/false),
>> Succeeded());
>>
>> std::vector<std::string> Names;
>> for (const auto &Func : LoadedCoverage->getCoveredFunctions())
>> @@ -538,8 +544,10 @@ TEST_P(CoverageMappingTest, strip_unknow
>> }
>>
>> TEST_P(CoverageMappingTest, dont_detect_false_instantiations) {
>> - NoError(ProfileWriter.addRecord({"foo", 0x1234, {10}}));
>> - NoError(ProfileWriter.addRecord({"bar", 0x2345, {20}}));
>> + EXPECT_THAT_ERROR(ProfileWriter.addRecord({"foo", 0x1234, {10}}),
>> + Succeeded());
>> + EXPECT_THAT_ERROR(ProfileWriter.addRecord({"bar", 0x2345, {20}}),
>> + Succeeded());
>>
>> startFunction("foo", 0x1234);
>> addCMR(Counter::getCounter(0), "expanded", 1, 1, 1, 10);
>> @@ -549,7 +557,7 @@ TEST_P(CoverageMappingTest, dont_detect_
>> addCMR(Counter::getCounter(0), "expanded", 1, 1, 1, 10);
>> addExpansionCMR("main", "expanded", 9, 1, 9, 5);
>>
>> - NoError(loadCoverageMapping());
>> + EXPECT_THAT_ERROR(loadCoverageMapping(), Succeeded());
>>
>> std::vector<const FunctionRecord *> Instantiations =
>> LoadedCoverage->getInstantiations("expanded");
>> @@ -557,13 +565,14 @@ TEST_P(CoverageMappingTest, dont_detect_
>> }
>>
>> TEST_P(CoverageMappingTest, load_coverage_for_expanded_file) {
>> - NoError(ProfileWriter.addRecord({"func", 0x1234, {10}}));
>> + EXPECT_THAT_ERROR(ProfileWriter.addRecord({"func", 0x1234, {10}}),
>> + Succeeded());
>>
>> startFunction("func", 0x1234);
>> addCMR(Counter::getCounter(0), "expanded", 1, 1, 1, 10);
>> addExpansionCMR("main", "expanded", 4, 1, 4, 5);
>>
>> - NoError(loadCoverageMapping());
>> + EXPECT_THAT_ERROR(loadCoverageMapping(), Succeeded());
>>
>> CoverageData Data = LoadedCoverage->getCoverageForFile("expanded");
>> std::vector<CoverageSegment> Segments(Data.begin(), Data.end());
>> @@ -573,7 +582,8 @@ TEST_P(CoverageMappingTest, load_coverag
>> }
>>
>> TEST_P(CoverageMappingTest, skip_duplicate_function_record) {
>> - NoError(ProfileWriter.addRecord({"func", 0x1234, {1}}));
>> + EXPECT_THAT_ERROR(ProfileWriter.addRecord({"func", 0x1234, {1}}),
>> + Succeeded());
>>
>> startFunction("func", 0x1234);
>> addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
>> @@ -581,7 +591,7 @@ TEST_P(CoverageMappingTest, skip_duplica
>> startFunction("func", 0x1234);
>> addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
>>
>> - NoError(loadCoverageMapping());
>> + EXPECT_THAT_ERROR(loadCoverageMapping(), Succeeded());
>>
>> auto Funcs = LoadedCoverage->getCoveredFunctions();
>> unsigned NumFuncs = std::distance(Funcs.begin(), Funcs.end());
>>
>> Modified: llvm/trunk/unittests/ProfileData/InstrProfTest.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ProfileData/InstrProfTest.cpp?rev=307440&r1=307439&r2=307440&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/unittests/ProfileData/InstrProfTest.cpp (original)
>> +++ llvm/trunk/unittests/ProfileData/InstrProfTest.cpp Fri Jul 7
>> 14:02:59 2017
>> @@ -14,20 +14,15 @@
>> #include "llvm/ProfileData/InstrProfReader.h"
>> #include "llvm/ProfileData/InstrProfWriter.h"
>> #include "llvm/Support/Compression.h"
>> +#include "llvm/Testing/Support/Error.h"
>> +#include "llvm/Testing/Support/SupportHelpers.h"
>> #include "gtest/gtest.h"
>> #include <cstdarg>
>>
>> using namespace llvm;
>>
>> -static ::testing::AssertionResult NoError(Error E) {
>> - if (!E)
>> - return ::testing::AssertionSuccess();
>> - return ::testing::AssertionFailure() << "error: " <<
>> toString(std::move(E))
>> - << "\n";
>> -}
>> -
>> -static ::testing::AssertionResult ErrorEquals(instrprof_error Expected,
>> - Error E) {
>> +LLVM_NODISCARD static ::testing::AssertionResult
>> +ErrorEquals(instrprof_error Expected, Error E) {
>> instrprof_error Found;
>> std::string FoundMsg;
>> handleAllErrors(std::move(E), [&](const InstrProfError &IPE) {
>> @@ -49,7 +44,7 @@ struct InstrProfTest : ::testing::Test {
>>
>> void readProfile(std::unique_ptr<MemoryBuffer> Profile) {
>> auto ReaderOrErr =
>> IndexedInstrProfReader::create(std::move(Profile));
>> - ASSERT_TRUE(NoError(ReaderOrErr.takeError()));
>> + EXPECT_THAT_ERROR(ReaderOrErr.takeError(), Succeeded());
>> Reader = std::move(ReaderOrErr.get());
>> }
>> };
>> @@ -70,7 +65,8 @@ TEST_P(MaybeSparseInstrProfTest, write_a
>> }
>>
>> TEST_P(MaybeSparseInstrProfTest, write_and_read_one_function) {
>> - NoError(Writer.addRecord({"foo", 0x1234, {1, 2, 3, 4}}));
>> + EXPECT_THAT_ERROR(Writer.addRecord({"foo", 0x1234, {1, 2, 3, 4}}),
>> + Succeeded());
>> auto Profile = Writer.writeBuffer();
>> readProfile(std::move(Profile));
>>
>> @@ -87,19 +83,19 @@ TEST_P(MaybeSparseInstrProfTest, write_a
>> }
>>
>> TEST_P(MaybeSparseInstrProfTest, get_instr_prof_record) {
>> - NoError(Writer.addRecord({"foo", 0x1234, {1, 2}}));
>> - NoError(Writer.addRecord({"foo", 0x1235, {3, 4}}));
>> + EXPECT_THAT_ERROR(Writer.addRecord({"foo", 0x1234, {1, 2}}),
>> Succeeded());
>> + EXPECT_THAT_ERROR(Writer.addRecord({"foo", 0x1235, {3, 4}}),
>> Succeeded());
>> auto Profile = Writer.writeBuffer();
>> readProfile(std::move(Profile));
>>
>> Expected<InstrProfRecord> R = Reader->getInstrProfRecord("foo",
>> 0x1234);
>> - ASSERT_TRUE(NoError(R.takeError()));
>> + EXPECT_THAT_ERROR(R.takeError(), Succeeded());
>> ASSERT_EQ(2U, R->Counts.size());
>> ASSERT_EQ(1U, R->Counts[0]);
>> ASSERT_EQ(2U, R->Counts[1]);
>>
>> R = Reader->getInstrProfRecord("foo", 0x1235);
>> - ASSERT_TRUE(NoError(R.takeError()));
>> + EXPECT_THAT_ERROR(R.takeError(), Succeeded());
>> ASSERT_EQ(2U, R->Counts.size());
>> ASSERT_EQ(3U, R->Counts[0]);
>> ASSERT_EQ(4U, R->Counts[1]);
>> @@ -112,18 +108,20 @@ TEST_P(MaybeSparseInstrProfTest, get_ins
>> }
>>
>> TEST_P(MaybeSparseInstrProfTest, get_function_counts) {
>> - NoError(Writer.addRecord({"foo", 0x1234, {1, 2}}));
>> - NoError(Writer.addRecord({"foo", 0x1235, {3, 4}}));
>> + EXPECT_THAT_ERROR(Writer.addRecord({"foo", 0x1234, {1, 2}}),
>> Succeeded());
>> + EXPECT_THAT_ERROR(Writer.addRecord({"foo", 0x1235, {3, 4}}),
>> Succeeded());
>> auto Profile = Writer.writeBuffer();
>> readProfile(std::move(Profile));
>>
>> std::vector<uint64_t> Counts;
>> - ASSERT_TRUE(NoError(Reader->getFunctionCounts("foo", 0x1234, Counts)));
>> + EXPECT_THAT_ERROR(Reader->getFunctionCounts("foo", 0x1234, Counts),
>> + Succeeded());
>> ASSERT_EQ(2U, Counts.size());
>> ASSERT_EQ(1U, Counts[0]);
>> ASSERT_EQ(2U, Counts[1]);
>>
>> - ASSERT_TRUE(NoError(Reader->getFunctionCounts("foo", 0x1235, Counts)));
>> + EXPECT_THAT_ERROR(Reader->getFunctionCounts("foo", 0x1235, Counts),
>> + Succeeded());
>> ASSERT_EQ(2U, Counts.size());
>> ASSERT_EQ(3U, Counts[0]);
>> ASSERT_EQ(4U, Counts[1]);
>> @@ -137,14 +135,15 @@ TEST_P(MaybeSparseInstrProfTest, get_fun
>>
>> // Profile data is copied from general.proftext
>> TEST_F(InstrProfTest, get_profile_summary) {
>> - NoError(Writer.addRecord({"func1", 0x1234, {97531}}));
>> - NoError(Writer.addRecord({"func2", 0x1234, {0, 0}}));
>> - NoError(Writer.addRecord(
>> - {"func3",
>> - 0x1234,
>> - {2305843009213693952, 1152921504606846976, 576460752303423488,
>> - 288230376151711744, 144115188075855872, 72057594037927936}}));
>> - NoError(Writer.addRecord({"func4", 0x1234, {0}}));
>> + EXPECT_THAT_ERROR(Writer.addRecord({"func1", 0x1234, {97531}}),
>> Succeeded());
>> + EXPECT_THAT_ERROR(Writer.addRecord({"func2", 0x1234, {0, 0}}),
>> Succeeded());
>> + EXPECT_THAT_ERROR(Writer.addRecord({"func3",
>> + 0x1234,
>> + {2305843009213693952,
>> 1152921504606846976,
>> + 576460752303423488,
>> 288230376151711744,
>> + 144115188075855872,
>> 72057594037927936}}),
>> + Succeeded());
>> + EXPECT_THAT_ERROR(Writer.addRecord({"func4", 0x1234, {0}}),
>> Succeeded());
>> auto Profile = Writer.writeBuffer();
>> readProfile(std::move(Profile));
>>
>> @@ -195,23 +194,24 @@ TEST_F(InstrProfTest, get_profile_summar
>> }
>>
>> TEST_F(InstrProfTest, test_writer_merge) {
>> - NoError(Writer.addRecord({"func1", 0x1234, {42}}));
>> + EXPECT_THAT_ERROR(Writer.addRecord({"func1", 0x1234, {42}}),
>> Succeeded());
>>
>> InstrProfWriter Writer2;
>> - NoError(Writer2.addRecord({"func2", 0x1234, {0, 0}}));
>> + EXPECT_THAT_ERROR(Writer2.addRecord({"func2", 0x1234, {0, 0}}),
>> Succeeded());
>>
>> - NoError(Writer.mergeRecordsFromWriter(std::move(Writer2)));
>> + EXPECT_THAT_ERROR(Writer.mergeRecordsFromWriter(std::move(Writer2)),
>> + Succeeded());
>>
>> auto Profile = Writer.writeBuffer();
>> readProfile(std::move(Profile));
>>
>> Expected<InstrProfRecord> R = Reader->getInstrProfRecord("func1",
>> 0x1234);
>> - ASSERT_TRUE(NoError(R.takeError()));
>> + EXPECT_THAT_ERROR(R.takeError(), Succeeded());
>> ASSERT_EQ(1U, R->Counts.size());
>> ASSERT_EQ(42U, R->Counts[0]);
>>
>> R = Reader->getInstrProfRecord("func2", 0x1234);
>> - ASSERT_TRUE(NoError(R.takeError()));
>> + EXPECT_THAT_ERROR(R.takeError(), Succeeded());
>> ASSERT_EQ(2U, R->Counts.size());
>> ASSERT_EQ(0U, R->Counts[0]);
>> ASSERT_EQ(0U, R->Counts[1]);
>> @@ -239,15 +239,15 @@ TEST_P(MaybeSparseInstrProfTest, get_ica
>> InstrProfValueData VD3[] = {{(uint64_t)callee1, 1}};
>> Record1.addValueData(IPVK_IndirectCallTarget, 3, VD3, 1, nullptr);
>>
>> - NoError(Writer.addRecord(std::move(Record1)));
>> - NoError(Writer.addRecord({"callee1", 0x1235, {3, 4}}));
>> - NoError(Writer.addRecord({"callee2", 0x1235, {3, 4}}));
>> - NoError(Writer.addRecord({"callee3", 0x1235, {3, 4}}));
>> + EXPECT_THAT_ERROR(Writer.addRecord(std::move(Record1)), Succeeded());
>> + EXPECT_THAT_ERROR(Writer.addRecord({"callee1", 0x1235, {3, 4}}),
>> Succeeded());
>> + EXPECT_THAT_ERROR(Writer.addRecord({"callee2", 0x1235, {3, 4}}),
>> Succeeded());
>> + EXPECT_THAT_ERROR(Writer.addRecord({"callee3", 0x1235, {3, 4}}),
>> Succeeded());
>> auto Profile = Writer.writeBuffer();
>> readProfile(std::move(Profile));
>>
>> Expected<InstrProfRecord> R = Reader->getInstrProfRecord("caller",
>> 0x1234);
>> - ASSERT_TRUE(NoError(R.takeError()));
>> + EXPECT_THAT_ERROR(R.takeError(), Succeeded());
>> ASSERT_EQ(4U, R->getNumValueSites(IPVK_IndirectCallTarget));
>> ASSERT_EQ(3U, R->getNumValueDataForSite(IPVK_IndirectCallTarget, 0));
>> ASSERT_EQ(0U, R->getNumValueDataForSite(IPVK_IndirectCallTarget, 1));
>> @@ -274,11 +274,11 @@ TEST_P(MaybeSparseInstrProfTest, annotat
>> InstrProfValueData VD0[] = {{1000, 1}, {2000, 2}, {3000, 3}, {5000, 5},
>> {4000, 4}, {6000, 6}};
>> Record.addValueData(IPVK_IndirectCallTarget, 0, VD0, 6, nullptr);
>> - NoError(Writer.addRecord(std::move(Record)));
>> + EXPECT_THAT_ERROR(Writer.addRecord(std::move(Record)), Succeeded());
>> auto Profile = Writer.writeBuffer();
>> readProfile(std::move(Profile));
>> Expected<InstrProfRecord> R = Reader->getInstrProfRecord("caller",
>> 0x1234);
>> - ASSERT_TRUE(NoError(R.takeError()));
>> + EXPECT_THAT_ERROR(R.takeError(), Succeeded());
>>
>> LLVMContext Ctx;
>> std::unique_ptr<Module> M(new Module("MyModule", Ctx));
>> @@ -379,15 +379,15 @@ TEST_P(MaybeSparseInstrProfTest, get_ica
>> InstrProfValueData VD3[] = {{(uint64_t)callee1, 1}};
>> Record1.addValueData(IPVK_IndirectCallTarget, 3, VD3, 1, nullptr);
>>
>> - NoError(Writer.addRecord(std::move(Record1), 10));
>> - NoError(Writer.addRecord({"callee1", 0x1235, {3, 4}}));
>> - NoError(Writer.addRecord({"callee2", 0x1235, {3, 4}}));
>> - NoError(Writer.addRecord({"callee3", 0x1235, {3, 4}}));
>> + EXPECT_THAT_ERROR(Writer.addRecord(std::move(Record1), 10),
>> Succeeded());
>> + EXPECT_THAT_ERROR(Writer.addRecord({"callee1", 0x1235, {3, 4}}),
>> Succeeded());
>> + EXPECT_THAT_ERROR(Writer.addRecord({"callee2", 0x1235, {3, 4}}),
>> Succeeded());
>> + EXPECT_THAT_ERROR(Writer.addRecord({"callee3", 0x1235, {3, 4}}),
>> Succeeded());
>> auto Profile = Writer.writeBuffer();
>> readProfile(std::move(Profile));
>>
>> Expected<InstrProfRecord> R = Reader->getInstrProfRecord("caller",
>> 0x1234);
>> - ASSERT_TRUE(NoError(R.takeError()));
>> + EXPECT_THAT_ERROR(R.takeError(), Succeeded());
>> ASSERT_EQ(4U, R->getNumValueSites(IPVK_IndirectCallTarget));
>> ASSERT_EQ(3U, R->getNumValueDataForSite(IPVK_IndirectCallTarget, 0));
>> ASSERT_EQ(0U, R->getNumValueDataForSite(IPVK_IndirectCallTarget, 1));
>> @@ -422,10 +422,10 @@ TEST_P(MaybeSparseInstrProfTest, get_ica
>> InstrProfValueData VD3[] = {{(uint64_t)callee1, 1}};
>> Record1.addValueData(IPVK_IndirectCallTarget, 3, VD3, 1, nullptr);
>>
>> - NoError(Writer.addRecord(std::move(Record1)));
>> - NoError(Writer.addRecord({"callee1", 0x1235, {3, 4}}));
>> - NoError(Writer.addRecord({"callee2", 0x1235, {3, 4}}));
>> - NoError(Writer.addRecord({"callee3", 0x1235, {3, 4}}));
>> + EXPECT_THAT_ERROR(Writer.addRecord(std::move(Record1)), Succeeded());
>> + EXPECT_THAT_ERROR(Writer.addRecord({"callee1", 0x1235, {3, 4}}),
>> Succeeded());
>> + EXPECT_THAT_ERROR(Writer.addRecord({"callee2", 0x1235, {3, 4}}),
>> Succeeded());
>> + EXPECT_THAT_ERROR(Writer.addRecord({"callee3", 0x1235, {3, 4}}),
>> Succeeded());
>>
>> // Set big endian output.
>> Writer.setValueProfDataEndianness(support::big);
>> @@ -437,7 +437,7 @@ TEST_P(MaybeSparseInstrProfTest, get_ica
>> Reader->setValueProfDataEndianness(support::big);
>>
>> Expected<InstrProfRecord> R = Reader->getInstrProfRecord("caller",
>> 0x1234);
>> - ASSERT_TRUE(NoError(R.takeError()));
>> + EXPECT_THAT_ERROR(R.takeError(), Succeeded());
>> ASSERT_EQ(4U, R->getNumValueSites(IPVK_IndirectCallTarget));
>> ASSERT_EQ(3U, R->getNumValueDataForSite(IPVK_IndirectCallTarget, 0));
>> ASSERT_EQ(0U, R->getNumValueDataForSite(IPVK_IndirectCallTarget, 1));
>> @@ -501,20 +501,20 @@ TEST_P(MaybeSparseInstrProfTest, get_ica
>> {uint64_t(callee3), 3}};
>> Record12.addValueData(IPVK_IndirectCallTarget, 4, VD42, 3, nullptr);
>>
>> - NoError(Writer.addRecord(std::move(Record11)));
>> + EXPECT_THAT_ERROR(Writer.addRecord(std::move(Record11)), Succeeded());
>> // Merge profile data.
>> - NoError(Writer.addRecord(std::move(Record12)));
>> + EXPECT_THAT_ERROR(Writer.addRecord(std::move(Record12)), Succeeded());
>>
>> - NoError(Writer.addRecord({callee1, 0x1235, {3, 4}}));
>> - NoError(Writer.addRecord({callee2, 0x1235, {3, 4}}));
>> - NoError(Writer.addRecord({callee3, 0x1235, {3, 4}}));
>> - NoError(Writer.addRecord({callee3, 0x1235, {3, 4}}));
>> - NoError(Writer.addRecord({callee4, 0x1235, {3, 5}}));
>> + EXPECT_THAT_ERROR(Writer.addRecord({callee1, 0x1235, {3, 4}}),
>> Succeeded());
>> + EXPECT_THAT_ERROR(Writer.addRecord({callee2, 0x1235, {3, 4}}),
>> Succeeded());
>> + EXPECT_THAT_ERROR(Writer.addRecord({callee3, 0x1235, {3, 4}}),
>> Succeeded());
>> + EXPECT_THAT_ERROR(Writer.addRecord({callee3, 0x1235, {3, 4}}),
>> Succeeded());
>> + EXPECT_THAT_ERROR(Writer.addRecord({callee4, 0x1235, {3, 5}}),
>> Succeeded());
>> auto Profile = Writer.writeBuffer();
>> readProfile(std::move(Profile));
>>
>> Expected<InstrProfRecord> R = Reader->getInstrProfRecord("caller",
>> 0x1234);
>> - ASSERT_TRUE(NoError(R.takeError()));
>> + EXPECT_THAT_ERROR(R.takeError(), Succeeded());
>> ASSERT_EQ(5U, R->getNumValueSites(IPVK_IndirectCallTarget));
>> ASSERT_EQ(4U, R->getNumValueDataForSite(IPVK_IndirectCallTarget, 0));
>> ASSERT_EQ(0U, R->getNumValueDataForSite(IPVK_IndirectCallTarget, 1));
>> @@ -600,7 +600,7 @@ TEST_P(MaybeSparseInstrProfTest, get_ica
>> // Verify saturation of counts.
>> Expected<InstrProfRecord> ReadRecord1 =
>> Reader->getInstrProfRecord("foo", 0x1234);
>> - ASSERT_TRUE(NoError(ReadRecord1.takeError()));
>> + EXPECT_THAT_ERROR(ReadRecord1.takeError(), Succeeded());
>> ASSERT_EQ(Max, ReadRecord1->Counts[0]);
>>
>> Expected<InstrProfRecord> ReadRecord2 =
>> @@ -643,15 +643,15 @@ TEST_P(MaybeSparseInstrProfTest, get_ica
>> Record12.addValueData(IPVK_IndirectCallTarget, 0, VD1, 255, nullptr);
>> Record12.addValueData(IPVK_IndirectCallTarget, 1, nullptr, 0, nullptr);
>>
>> - NoError(Writer.addRecord(std::move(Record11)));
>> + EXPECT_THAT_ERROR(Writer.addRecord(std::move(Record11)), Succeeded());
>> // Merge profile data.
>> - NoError(Writer.addRecord(std::move(Record12)));
>> + EXPECT_THAT_ERROR(Writer.addRecord(std::move(Record12)), Succeeded());
>>
>> auto Profile = Writer.writeBuffer();
>> readProfile(std::move(Profile));
>>
>> Expected<InstrProfRecord> R = Reader->getInstrProfRecord("caller",
>> 0x1234);
>> - ASSERT_TRUE(NoError(R.takeError()));
>> + EXPECT_THAT_ERROR(R.takeError(), Succeeded());
>> std::unique_ptr<InstrProfValueData[]> VD(
>> R->getValueForSite(IPVK_IndirectCallTarget, 0));
>> ASSERT_EQ(2U, R->getNumValueSites(IPVK_IndirectCallTarget));
>> @@ -790,9 +790,11 @@ TEST_P(MaybeSparseInstrProfTest, value_p
>> }
>>
>> TEST_P(MaybeSparseInstrProfTest, get_max_function_count) {
>> - NoError(Writer.addRecord({"foo", 0x1234, {1ULL << 31, 2}}));
>> - NoError(Writer.addRecord({"bar", 0, {1ULL << 63}}));
>> - NoError(Writer.addRecord({"baz", 0x5678, {0, 0, 0, 0}}));
>> + EXPECT_THAT_ERROR(Writer.addRecord({"foo", 0x1234, {1ULL << 31, 2}}),
>> + Succeeded());
>> + EXPECT_THAT_ERROR(Writer.addRecord({"bar", 0, {1ULL << 63}}),
>> Succeeded());
>> + EXPECT_THAT_ERROR(Writer.addRecord({"baz", 0x5678, {0, 0, 0, 0}}),
>> + Succeeded());
>> auto Profile = Writer.writeBuffer();
>> readProfile(std::move(Profile));
>>
>> @@ -800,18 +802,20 @@ TEST_P(MaybeSparseInstrProfTest, get_max
>> }
>>
>> TEST_P(MaybeSparseInstrProfTest, get_weighted_function_counts) {
>> - NoError(Writer.addRecord({"foo", 0x1234, {1, 2}}, 3));
>> - NoError(Writer.addRecord({"foo", 0x1235, {3, 4}}, 5));
>> + EXPECT_THAT_ERROR(Writer.addRecord({"foo", 0x1234, {1, 2}}, 3),
>> Succeeded());
>> + EXPECT_THAT_ERROR(Writer.addRecord({"foo", 0x1235, {3, 4}}, 5),
>> Succeeded());
>> auto Profile = Writer.writeBuffer();
>> readProfile(std::move(Profile));
>>
>> std::vector<uint64_t> Counts;
>> - ASSERT_TRUE(NoError(Reader->getFunctionCounts("foo", 0x1234, Counts)));
>> + EXPECT_THAT_ERROR(Reader->getFunctionCounts("foo", 0x1234, Counts),
>> + Succeeded());
>> ASSERT_EQ(2U, Counts.size());
>> ASSERT_EQ(3U, Counts[0]);
>> ASSERT_EQ(6U, Counts[1]);
>>
>> - ASSERT_TRUE(NoError(Reader->getFunctionCounts("foo", 0x1235, Counts)));
>> + EXPECT_THAT_ERROR(Reader->getFunctionCounts("foo", 0x1235, Counts),
>> + Succeeded());
>> ASSERT_EQ(2U, Counts.size());
>> ASSERT_EQ(15U, Counts[0]);
>> ASSERT_EQ(20U, Counts[1]);
>> @@ -827,7 +831,7 @@ TEST_P(MaybeSparseInstrProfTest, instr_p
>> FuncNames.push_back("bar2");
>> FuncNames.push_back("bar3");
>> InstrProfSymtab Symtab;
>> - NoError(Symtab.create(FuncNames));
>> + EXPECT_THAT_ERROR(Symtab.create(FuncNames), Succeeded());
>> StringRef R =
>> Symtab.getFuncName(IndexedInstrProf::ComputeHash("func1"));
>> ASSERT_EQ(StringRef("func1"), R);
>> R = Symtab.getFuncName(IndexedInstrProf::ComputeHash("func2"));
>> @@ -848,9 +852,9 @@ TEST_P(MaybeSparseInstrProfTest, instr_p
>> ASSERT_EQ(StringRef(), R);
>>
>> // Now incrementally update the symtab
>> - NoError(Symtab.addFuncName("blah_1"));
>> - NoError(Symtab.addFuncName("blah_2"));
>> - NoError(Symtab.addFuncName("blah_3"));
>> + EXPECT_THAT_ERROR(Symtab.addFuncName("blah_1"), Succeeded());
>> + EXPECT_THAT_ERROR(Symtab.addFuncName("blah_2"), Succeeded());
>> + EXPECT_THAT_ERROR(Symtab.addFuncName("blah_3"), Succeeded());
>> // Finalize it
>> Symtab.finalizeSymtab();
>>
>> @@ -878,7 +882,7 @@ TEST_P(MaybeSparseInstrProfTest, instr_p
>> // Test that we get an error when creating a bogus symtab.
>> TEST_P(MaybeSparseInstrProfTest,
>> instr_prof_bogus_symtab_empty_func_name) {
>> InstrProfSymtab Symtab;
>> - ErrorEquals(instrprof_error::malformed, Symtab.addFuncName(""));
>> + EXPECT_TRUE(ErrorEquals(instrprof_error::malformed,
>> Symtab.addFuncName("")));
>> }
>>
>> // Testing symtab creator interface used by value profile transformer.
>> @@ -901,7 +905,7 @@ TEST_P(MaybeSparseInstrProfTest, instr_p
>> Function::Create(FTy, Function::WeakODRLinkage, "Wbar", M.get());
>>
>> InstrProfSymtab ProfSymtab;
>> - NoError(ProfSymtab.create(*M));
>> + EXPECT_THAT_ERROR(ProfSymtab.create(*M), Succeeded());
>>
>> StringRef Funcs[] = {"Gfoo", "Gblah", "Gbar", "Ifoo", "Iblah", "Ibar",
>> "Pfoo", "Pblah", "Pbar", "Wfoo", "Wblah",
>> "Wbar"};
>> @@ -941,13 +945,17 @@ TEST_P(MaybeSparseInstrProfTest, instr_p
>> for (bool DoCompression : {false, true}) {
>> // Compressing:
>> std::string FuncNameStrings1;
>> - NoError(collectPGOFuncNameStrings(
>> - FuncNames1, (DoCompression && zlib::isAvailable()),
>> FuncNameStrings1));
>> + EXPECT_THAT_ERROR(collectPGOFuncNameStrings(
>> + FuncNames1, (DoCompression &&
>> zlib::isAvailable()),
>> + FuncNameStrings1),
>> + Succeeded());
>>
>> // Compressing:
>> std::string FuncNameStrings2;
>> - NoError(collectPGOFuncNameStrings(
>> - FuncNames2, (DoCompression && zlib::isAvailable()),
>> FuncNameStrings2));
>> + EXPECT_THAT_ERROR(collectPGOFuncNameStrings(
>> + FuncNames2, (DoCompression &&
>> zlib::isAvailable()),
>> + FuncNameStrings2),
>> + Succeeded());
>>
>> for (int Padding = 0; Padding < 2; Padding++) {
>> // Join with paddings :
>> @@ -959,7 +967,7 @@ TEST_P(MaybeSparseInstrProfTest, instr_p
>>
>> // Now decompress:
>> InstrProfSymtab Symtab;
>> - NoError(Symtab.create(StringRef(FuncNameStrings)));
>> + EXPECT_THAT_ERROR(Symtab.create(StringRef(FuncNameStrings)),
>> Succeeded());
>>
>> // Now do the checks:
>> // First sampling some data points:
>> @@ -983,9 +991,11 @@ TEST_P(MaybeSparseInstrProfTest, instr_p
>> }
>>
>> TEST_F(SparseInstrProfTest, preserve_no_records) {
>> - NoError(Writer.addRecord({"foo", 0x1234, {0}}));
>> - NoError(Writer.addRecord({"bar", 0x4321, {0, 0}}));
>> - NoError(Writer.addRecord({"bar", 0x4321, {0, 0, 0}}));
>> + EXPECT_THAT_ERROR(Writer.addRecord({"foo", 0x1234, {0}}), Succeeded());
>> + EXPECT_THAT_ERROR(Writer.addRecord({"bar", 0x4321, {0, 0}}),
>> Succeeded());
>> + // FIXME: I'm guessing this data should be different, but the original
>> author
>> + // should check/update this test so it doesn't produce errors.
>> + consumeError(Writer.addRecord({"bar", 0x4321, {0, 0, 0}}));
>>
>
> Hey Vedant - could you fix this ^ I'm not sure what the right fix was, so
> left it this way to preserve existing behavior.
>
>
>>
>> auto Profile = Writer.writeBuffer();
>> readProfile(std::move(Profile));
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170710/09cb6523/attachment.html>
More information about the llvm-commits
mailing list