Skip to content

Commit 7f7e417

Browse files
Grufoonyvincentlaucsb
Grufoony
and
vincentlaucsb
authored
Remove extra ';' in csv.hpp (#291)
* Remove extra ';' Fix the closing brace of the constructor in the csv.hpp file. * Enable pedantic warnings if CSV_DEVELOPER true * Remove extraneous semi-colon * Update CMakeLists.txt * Update test_error_handling.cpp --------- Co-authored-by: Vincent La <vincela9@gmail.com>
1 parent 50583f8 commit 7f7e417

10 files changed

Lines changed: 54 additions & 38 deletions

File tree

‎CMakeLists.txt‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,14 +71,16 @@ if (CSV_DEVELOPER)
7171
# Allow for performance profiling
7272
if (MSVC)
7373
target_link_options(csv PUBLIC /PROFILE)
74+
# Treat warnings as errors
75+
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /WX")
7476
endif()
7577

76-
# More error messages.
78+
# More error messages, treat warnings as errors.
7779
if (UNIX)
7880
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} \
79-
-Wall -Wextra -Wsign-compare \
81+
-Wall -Wextra -Wpedantic -Wsign-compare \
8082
-Wwrite-strings -Wpointer-arith -Winit-self \
81-
-Wconversion -Wno-sign-conversion")
83+
-Wconversion -Wno-sign-conversion -Werror")
8284
endif()
8385

8486
# Generate a single header library

‎include/internal/basic_csv_parser.hpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ namespace csv {
9292
IBasicCSVParser() = default;
9393
IBasicCSVParser(const CSVFormat&, const ColNamesPtr&);
9494
IBasicCSVParser(const ParseFlagMap& parse_flags, const WhitespaceMap& ws_flags
95-
) : _parse_flags(parse_flags), _ws_flags(ws_flags) {};
95+
) : _parse_flags(parse_flags), _ws_flags(ws_flags) {}
9696

9797
virtual ~IBasicCSVParser() {}
9898

@@ -213,15 +213,15 @@ namespace csv {
213213
StreamParser(TStream& source,
214214
const CSVFormat& format,
215215
const ColNamesPtr& col_names = nullptr
216-
) : IBasicCSVParser(format, col_names), _source(source) {};
216+
) : IBasicCSVParser(format, col_names), _source(source) {}
217217

218218
StreamParser(
219219
TStream& source,
220220
internals::ParseFlagMap parse_flags,
221221
internals::WhitespaceMap ws_flags) :
222222
IBasicCSVParser(parse_flags, ws_flags),
223223
_source(source)
224-
{};
224+
{}
225225

226226
~StreamParser() {}
227227

‎include/internal/csv_reader.hpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ namespace csv {
125125
#endif
126126

127127
iterator() = default;
128-
iterator(CSVReader* reader) : daddy(reader) {};
128+
iterator(CSVReader* reader) : daddy(reader) {}
129129
iterator(CSVReader*, CSVRow&&);
130130

131131
/** Access the CSVRow held by the iterator */
@@ -222,7 +222,7 @@ namespace csv {
222222
CSV_CONST iterator end() const noexcept;
223223

224224
/** Returns true if we have reached end of file */
225-
bool eof() const noexcept { return this->parser->eof(); };
225+
bool eof() const noexcept { return this->parser->eof(); }
226226
///@}
227227

228228
/** @name CSV Metadata */

‎include/internal/csv_row.hpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ namespace csv {
3939
class CSVField {
4040
public:
4141
/** Constructs a CSVField from a string_view */
42-
constexpr explicit CSVField(csv::string_view _sv) noexcept : sv(_sv) { };
42+
constexpr explicit CSVField(csv::string_view _sv) noexcept : sv(_sv) {}
4343

4444
operator std::string() const {
4545
return std::string("<CSVField> ") + std::string(this->sv);
@@ -254,7 +254,7 @@ namespace csv {
254254
}
255255

256256
/** Returns true if field is a floating point value */
257-
CONSTEXPR_14 bool is_float() noexcept { return type() == DataType::CSV_DOUBLE; };
257+
CONSTEXPR_14 bool is_float() noexcept { return type() == DataType::CSV_DOUBLE; }
258258

259259
/** Return the type of the underlying CSV data */
260260
CONSTEXPR_14 DataType type() noexcept {

‎include/internal/csv_writer.hpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ namespace csv {
201201
*/
202202

203203
DelimWriter(OutputStream& _out, bool _quote_minimal = true)
204-
: out(&_out), quote_minimal(_quote_minimal) {};
204+
: out(&_out), quote_minimal(_quote_minimal) {}
205205

206206
/** Construct a DelimWriter over the file
207207
*
@@ -215,7 +215,7 @@ namespace csv {
215215
quote_minimal(_quote_minimal) {
216216
if (!owned_out->is_open())
217217
throw std::runtime_error("Failed to open file for writing: " + filename);
218-
};
218+
}
219219

220220
/** Destructor will flush remaining data
221221
*

‎include/internal/thread_safe_deque.hpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ namespace csv {
2424
template<typename T>
2525
class ThreadSafeDeque {
2626
public:
27-
ThreadSafeDeque(size_t notify_size = 100) : _notify_size(notify_size) {};
27+
ThreadSafeDeque(size_t notify_size = 100) : _notify_size(notify_size) {}
2828
ThreadSafeDeque(const ThreadSafeDeque& other) {
2929
this->data = other.data;
3030
this->_notify_size = other._notify_size;

‎single_include/csv.hpp‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4238,7 +4238,7 @@ namespace csv {
42384238
class CSVField {
42394239
public:
42404240
/** Constructs a CSVField from a string_view */
4241-
constexpr explicit CSVField(csv::string_view _sv) noexcept : sv(_sv) { };
4241+
constexpr explicit CSVField(csv::string_view _sv) noexcept : sv(_sv) {}
42424242

42434243
operator std::string() const {
42444244
return std::string("<CSVField> ") + std::string(this->sv);
@@ -4453,7 +4453,7 @@ namespace csv {
44534453
}
44544454

44554455
/** Returns true if field is a floating point value */
4456-
CONSTEXPR_14 bool is_float() noexcept { return type() == DataType::CSV_DOUBLE; };
4456+
CONSTEXPR_14 bool is_float() noexcept { return type() == DataType::CSV_DOUBLE; }
44574457

44584458
/** Return the type of the underlying CSV data */
44594459
CONSTEXPR_14 DataType type() noexcept {
@@ -4746,7 +4746,7 @@ namespace csv {
47464746
template<typename T>
47474747
class ThreadSafeDeque {
47484748
public:
4749-
ThreadSafeDeque(size_t notify_size = 100) : _notify_size(notify_size) {};
4749+
ThreadSafeDeque(size_t notify_size = 100) : _notify_size(notify_size) {}
47504750
ThreadSafeDeque(const ThreadSafeDeque& other) {
47514751
this->data = other.data;
47524752
this->_notify_size = other._notify_size;
@@ -4923,7 +4923,7 @@ namespace csv {
49234923
IBasicCSVParser() = default;
49244924
IBasicCSVParser(const CSVFormat&, const ColNamesPtr&);
49254925
IBasicCSVParser(const ParseFlagMap& parse_flags, const WhitespaceMap& ws_flags
4926-
) : _parse_flags(parse_flags), _ws_flags(ws_flags) {};
4926+
) : _parse_flags(parse_flags), _ws_flags(ws_flags) {}
49274927

49284928
virtual ~IBasicCSVParser() {}
49294929

@@ -5044,15 +5044,15 @@ namespace csv {
50445044
StreamParser(TStream& source,
50455045
const CSVFormat& format,
50465046
const ColNamesPtr& col_names = nullptr
5047-
) : IBasicCSVParser(format, col_names), _source(source) {};
5047+
) : IBasicCSVParser(format, col_names), _source(source) {}
50485048

50495049
StreamParser(
50505050
TStream& source,
50515051
internals::ParseFlagMap parse_flags,
50525052
internals::WhitespaceMap ws_flags) :
50535053
IBasicCSVParser(parse_flags, ws_flags),
50545054
_source(source)
5055-
{};
5055+
{}
50565056

50575057
~StreamParser() {}
50585058

@@ -5237,7 +5237,7 @@ namespace csv {
52375237
#endif
52385238

52395239
iterator() = default;
5240-
iterator(CSVReader* reader) : daddy(reader) {};
5240+
iterator(CSVReader* reader) : daddy(reader) {}
52415241
iterator(CSVReader*, CSVRow&&);
52425242

52435243
/** Access the CSVRow held by the iterator */
@@ -5334,7 +5334,7 @@ namespace csv {
53345334
CSV_CONST iterator end() const noexcept;
53355335

53365336
/** Returns true if we have reached end of file */
5337-
bool eof() const noexcept { return this->parser->eof(); };
5337+
bool eof() const noexcept { return this->parser->eof(); }
53385338
///@}
53395339

53405340
/** @name CSV Metadata */
@@ -6777,7 +6777,7 @@ namespace csv {
67776777
*/
67786778

67796779
DelimWriter(OutputStream& _out, bool _quote_minimal = true)
6780-
: out(&_out), quote_minimal(_quote_minimal) {};
6780+
: out(&_out), quote_minimal(_quote_minimal) {}
67816781

67826782
/** Construct a DelimWriter over the file
67836783
*
@@ -6791,7 +6791,7 @@ namespace csv {
67916791
quote_minimal(_quote_minimal) {
67926792
if (!owned_out->is_open())
67936793
throw std::runtime_error("Failed to open file for writing: " + filename);
6794-
};
6794+
}
67956795

67966796
/** Destructor will flush remaining data
67976797
*

‎single_include_test/csv.hpp‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4238,7 +4238,7 @@ namespace csv {
42384238
class CSVField {
42394239
public:
42404240
/** Constructs a CSVField from a string_view */
4241-
constexpr explicit CSVField(csv::string_view _sv) noexcept : sv(_sv) { };
4241+
constexpr explicit CSVField(csv::string_view _sv) noexcept : sv(_sv) {}
42424242

42434243
operator std::string() const {
42444244
return std::string("<CSVField> ") + std::string(this->sv);
@@ -4453,7 +4453,7 @@ namespace csv {
44534453
}
44544454

44554455
/** Returns true if field is a floating point value */
4456-
CONSTEXPR_14 bool is_float() noexcept { return type() == DataType::CSV_DOUBLE; };
4456+
CONSTEXPR_14 bool is_float() noexcept { return type() == DataType::CSV_DOUBLE; }
44574457

44584458
/** Return the type of the underlying CSV data */
44594459
CONSTEXPR_14 DataType type() noexcept {
@@ -4746,7 +4746,7 @@ namespace csv {
47464746
template<typename T>
47474747
class ThreadSafeDeque {
47484748
public:
4749-
ThreadSafeDeque(size_t notify_size = 100) : _notify_size(notify_size) {};
4749+
ThreadSafeDeque(size_t notify_size = 100) : _notify_size(notify_size) {}
47504750
ThreadSafeDeque(const ThreadSafeDeque& other) {
47514751
this->data = other.data;
47524752
this->_notify_size = other._notify_size;
@@ -4923,7 +4923,7 @@ namespace csv {
49234923
IBasicCSVParser() = default;
49244924
IBasicCSVParser(const CSVFormat&, const ColNamesPtr&);
49254925
IBasicCSVParser(const ParseFlagMap& parse_flags, const WhitespaceMap& ws_flags
4926-
) : _parse_flags(parse_flags), _ws_flags(ws_flags) {};
4926+
) : _parse_flags(parse_flags), _ws_flags(ws_flags) {}
49274927

49284928
virtual ~IBasicCSVParser() {}
49294929

@@ -5044,15 +5044,15 @@ namespace csv {
50445044
StreamParser(TStream& source,
50455045
const CSVFormat& format,
50465046
const ColNamesPtr& col_names = nullptr
5047-
) : IBasicCSVParser(format, col_names), _source(source) {};
5047+
) : IBasicCSVParser(format, col_names), _source(source) {}
50485048

50495049
StreamParser(
50505050
TStream& source,
50515051
internals::ParseFlagMap parse_flags,
50525052
internals::WhitespaceMap ws_flags) :
50535053
IBasicCSVParser(parse_flags, ws_flags),
50545054
_source(source)
5055-
{};
5055+
{}
50565056

50575057
~StreamParser() {}
50585058

@@ -5237,7 +5237,7 @@ namespace csv {
52375237
#endif
52385238

52395239
iterator() = default;
5240-
iterator(CSVReader* reader) : daddy(reader) {};
5240+
iterator(CSVReader* reader) : daddy(reader) {}
52415241
iterator(CSVReader*, CSVRow&&);
52425242

52435243
/** Access the CSVRow held by the iterator */
@@ -5334,7 +5334,7 @@ namespace csv {
53345334
CSV_CONST iterator end() const noexcept;
53355335

53365336
/** Returns true if we have reached end of file */
5337-
bool eof() const noexcept { return this->parser->eof(); };
5337+
bool eof() const noexcept { return this->parser->eof(); }
53385338
///@}
53395339

53405340
/** @name CSV Metadata */
@@ -6777,7 +6777,7 @@ namespace csv {
67776777
*/
67786778

67796779
DelimWriter(OutputStream& _out, bool _quote_minimal = true)
6780-
: out(&_out), quote_minimal(_quote_minimal) {};
6780+
: out(&_out), quote_minimal(_quote_minimal) {}
67816781

67826782
/** Construct a DelimWriter over the file
67836783
*
@@ -6791,7 +6791,7 @@ namespace csv {
67916791
quote_minimal(_quote_minimal) {
67926792
if (!owned_out->is_open())
67936793
throw std::runtime_error("Failed to open file for writing: " + filename);
6794-
};
6794+
}
67956795

67966796
/** Destructor will flush remaining data
67976797
*

‎tests/CMakeLists.txt‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,16 @@ FetchContent_Declare(
88

99
FetchContent_MakeAvailable(Catch2)
1010

11+
# Catch2's benchmark source files have -Wconversion hits; suppress on its targets
12+
# so our global -Wconversion/-Werror don't fail Catch2's own compilation.
13+
if(NOT MSVC)
14+
foreach(_catch2_target Catch2 Catch2WithMain)
15+
if(TARGET ${_catch2_target})
16+
target_compile_options(${_catch2_target} PRIVATE -Wno-conversion)
17+
endif()
18+
endforeach()
19+
endif()
20+
1121
add_executable(csv_test "")
1222
target_sources(csv_test
1323
PRIVATE
@@ -35,6 +45,12 @@ target_sources(csv_test
3545
target_link_libraries(csv_test csv)
3646
target_link_libraries(csv_test Catch2::Catch2WithMain)
3747

48+
# Suppress warnings originating from Catch2 headers (they are not our code)
49+
get_target_property(_catch2_inc Catch2::Catch2 INTERFACE_INCLUDE_DIRECTORIES)
50+
if(_catch2_inc)
51+
target_include_directories(csv_test SYSTEM PRIVATE ${_catch2_inc})
52+
endif()
53+
3854
if(MSVC)
3955
# Workaround to enable debugging unit tests in Visual Studio
4056
add_custom_command(

‎tests/test_error_handling.cpp‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,6 @@ TEST_CASE("Worker thread exceptions propagate to main thread", "[error_handling]
8888
}
8989

9090
SECTION("Exception during iteration is catchable") {
91-
bool caught = false;
92-
9391
try {
9492
// Create empty stringstream - will hit EOF immediately
9593
std::stringstream ss("");
@@ -102,7 +100,6 @@ TEST_CASE("Worker thread exceptions propagate to main thread", "[error_handling]
102100
}
103101
catch (const std::exception& e) {
104102
(void)e; // Suppress unused variable warning
105-
caught = true;
106103
}
107104

108105
// Either catches exception or completes successfully - should NOT terminate
@@ -122,9 +119,9 @@ TEST_CASE("Fields at chunk boundaries are not corrupted", "[chunking][data_integ
122119
std::ofstream out(test_file);
123120
out << "id,name,value,timestamp\n";
124121

125-
// Write enough data to cross at least one chunk boundary
122+
// Write enough data to cross two chunk boundaries
126123
// Approximate: 50 bytes per row = ~200K rows for 10MB
127-
const size_t rows_to_write = 250000;
124+
const size_t rows_to_write = 420000;
128125

129126
for (size_t i = 0; i < rows_to_write; i++) {
130127
out << i << ",name" << i << ",value" << i << "," << (1000000 + i) << "\n";
@@ -192,6 +189,7 @@ TEST_CASE("Fields at chunk boundaries are not corrupted", "[chunking][data_integ
192189

193190
// Verify we found critical rows (they weren't lost due to corruption)
194191
REQUIRE(found_critical_200k);
192+
REQUIRE(found_critical_400k);
195193

196194
// Verify we read all rows
197195
REQUIRE(row_count >= 200000);

0 commit comments

Comments
 (0)