Skip to content

Commit

Permalink
Fix missing memory_tracker_type_to_str (#4785)
Browse files Browse the repository at this point in the history
I forgot to add a switch case for ENUMERATION_CREATE. This changes the
implementations for both memory_type_to_str and
memory_tracker_type_to_str to generate compiler errors when a valid enum
case is not covered.

I've also added unit tests to cover conversions for all type values as
well.

---
TYPE: NO_HISTORY
DESC: Fix missing memory_tracker_type_to_str
  • Loading branch information
davisp authored Mar 5, 2024
1 parent d1355d2 commit 0a349d6
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 0 deletions.
8 changes: 8 additions & 0 deletions tiledb/common/memory_tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ std::string memory_type_to_str(MemoryType type) {
auto val = std::to_string(static_cast<uint32_t>(type));
throw std::logic_error("Invalid memory type: " + val);
}

auto val = std::to_string(static_cast<uint32_t>(type));
throw std::logic_error("Invalid memory type: " + val);
}

std::string memory_tracker_type_to_str(MemoryTrackerType type) {
Expand All @@ -100,6 +103,8 @@ std::string memory_tracker_type_to_str(MemoryTrackerType type) {
return "ArrayRead";
case MemoryTrackerType::ARRAY_WRITE:
return "ArrayWrite";
case MemoryTrackerType::ENUMERATION_CREATE:
return "EnumerationCreate";
case MemoryTrackerType::FRAGMENT_INFO_LOAD:
return "FragmentInfoLoad";
case MemoryTrackerType::QUERY_READ:
Expand All @@ -120,6 +125,9 @@ std::string memory_tracker_type_to_str(MemoryTrackerType type) {
auto val = std::to_string(static_cast<uint32_t>(type));
throw std::logic_error("Invalid memory tracker type: " + val);
}

auto val = std::to_string(static_cast<uint32_t>(type));
throw std::logic_error("Invalid memory tracker type: " + val);
}

uint64_t MemoryTrackerResource::get_count() {
Expand Down
16 changes: 16 additions & 0 deletions tiledb/common/memory_tracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,14 @@ enum class MemoryType {
TILE_WRITER_DATA
};

/**
* Return a string representation of type
*
* @param type The MemoryType to convert.
* @return A string representation.
*/
std::string memory_type_to_str(MemoryType type);

/** The type of MemoryTracker. */
enum class MemoryTrackerType {
ANONYMOUS,
Expand All @@ -137,6 +145,14 @@ enum class MemoryTrackerType {
SCHEMA_EVOLUTION
};

/**
* Return a string representation of type
*
* @param type The MemoryTrackerType to convert.
* @return A string representation.
*/
std::string memory_tracker_type_to_str(MemoryTrackerType type);

class MemoryTrackerResource : public tdb::pmr::memory_resource {
public:
// Disable all default generated constructors.
Expand Down
6 changes: 6 additions & 0 deletions tiledb/common/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,9 @@ commence(unit_test experimental)
unit_is_not_experimental.cc>
)
conclude(unit_test)

commence(unit_test memory_tracker_types)
this_target_sources(main.cc unit_memory_tracker_types.cc)
this_target_object_libraries(baseline)
conclude(unit_test)

72 changes: 72 additions & 0 deletions tiledb/common/test/unit_memory_tracker_types.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/**
* @file unit_memory_tracker_types.cc
*
* @section LICENSE
*
* The MIT License
*
* @copyright Copyright (c) 2024 TileDB, Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*
* @section DESCRIPTION
*
* This file tests the memory tracker to_str functions.
*/

#include <iostream>

#include <test/support/tdb_catch.h>
#include "tiledb/common/memory_tracker.h"

using namespace tiledb::common;

TEST_CASE("memory_type_to_str") {
auto max = static_cast<int>(tiledb::sm::MemoryType::TILE_WRITER_DATA);
size_t failures = 0;
for (int8_t i = 0; i < 127; i++) {
auto val = static_cast<tiledb::sm::MemoryType>(i);
if (i <= max) {
REQUIRE_NOTHROW(tiledb::sm::memory_type_to_str(val));
} else {
REQUIRE_THROWS(tiledb::sm::memory_type_to_str(val));
failures += 1;
}
}
// Technically, we could eventually have more than 127 enumeration values
// and this test would pass when it shouldn't.
REQUIRE(failures > 0);
}

TEST_CASE("memory_tracker_type_to_str") {
auto max = static_cast<int>(tiledb::sm::MemoryTrackerType::SCHEMA_EVOLUTION);
size_t failures = 0;
for (int8_t i = 0; i < 127; i++) {
auto val = static_cast<tiledb::sm::MemoryTrackerType>(i);
if (i <= max) {
REQUIRE_NOTHROW(tiledb::sm::memory_tracker_type_to_str(val));
} else {
REQUIRE_THROWS(tiledb::sm::memory_tracker_type_to_str(val));
failures += 1;
}
}
// Technically, we could eventually have more than 127 enumeration values
// and this test would pass when it shouldn't.
REQUIRE(failures > 0);
}

0 comments on commit 0a349d6

Please sign in to comment.