From fb8e1214c11dc0871fde7ae5972c08017b40b9bb Mon Sep 17 00:00:00 2001 From: ouankou Date: Fri, 13 Feb 2026 04:52:05 +0000 Subject: [PATCH 1/4] streamline: consolidate duplicate trimWhitespace functions - Remove trimWhitespaceCopy wrapper function - Replace 9 call sites with trimWhitespace - Uses more complete whitespace char set (form feed, vertical tab) - Add trimWhitespace overload taking pos/count to avoid substring copies - Remove redundant check in trimWhitespace overload --- src/OpenMPIR.cpp | 47 ++++++++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/src/OpenMPIR.cpp b/src/OpenMPIR.cpp index 053237d0..35f3ac15 100644 --- a/src/OpenMPIR.cpp +++ b/src/OpenMPIR.cpp @@ -62,6 +62,20 @@ std::string trimWhitespace(const std::string &text) { return text.substr(begin, end - begin + 1); } +std::string trimWhitespace(const std::string &text, size_t pos, size_t count) { + const char *whitespace = " \t\n\r\f\v"; + size_t end_pos = pos + count; + if (end_pos > text.size()) { + end_pos = text.size(); + } + size_t begin = text.find_first_not_of(whitespace, pos); + if (begin == std::string::npos || begin >= end_pos) { + return std::string(); + } + size_t end = text.find_last_not_of(whitespace, end_pos - 1); + return text.substr(begin, end - begin + 1); +} + std::string normalizeRawExpression(const char *expr, bool strip_trailing_colon = false) { if (!expr) { @@ -181,15 +195,6 @@ std::string normalizeClauseExpression(OpenMPClauseKind kind, namespace { -std::string trimWhitespaceCopy(const std::string &value) { - const std::string::size_type begin = value.find_first_not_of(" \t\r\n"); - if (begin == std::string::npos) { - return std::string(); - } - const std::string::size_type end = value.find_last_not_of(" \t\r\n"); - return value.substr(begin, end - begin + 1); -} - bool isIdentifierChar(char ch) { const unsigned char uch = static_cast(ch); return std::isalnum(uch) != 0 || ch == '_'; @@ -284,7 +289,7 @@ bool hasIncompleteTrailingOperator(const std::string &expression) { } bool isValidDistDataBaseExpression(const std::string &expression) { - const std::string trimmed_expression = trimWhitespaceCopy(expression); + const std::string trimmed_expression = trimWhitespace(expression); if (trimmed_expression.empty()) { return false; } @@ -349,7 +354,7 @@ bool splitMapExpressionDistDataSuffix(const std::string &expression, return false; } - const std::string trimmed_expression = trimWhitespaceCopy(expression); + const std::string trimmed_expression = trimWhitespace(expression); *array_section_expression = trimmed_expression; dist_data_arguments->clear(); if (trimmed_expression.empty() || trimmed_expression.back() != ')') { @@ -408,7 +413,7 @@ bool splitMapExpressionDistDataSuffix(const std::string &expression, } const std::string prefix = - trimWhitespaceCopy(trimmed_expression.substr(0, open_paren_pos)); + trimWhitespace(trimmed_expression, 0, open_paren_pos); if (prefix.empty()) { return false; } @@ -442,14 +447,14 @@ bool splitMapExpressionDistDataSuffix(const std::string &expression, } const std::string base_expression = - trimWhitespaceCopy(prefix.substr(0, token_begin)); + trimWhitespace(prefix, 0, token_begin); if (!isValidDistDataBaseExpression(base_expression)) { return false; } *array_section_expression = base_expression; - *dist_data_arguments = trimWhitespaceCopy(trimmed_expression.substr( - open_paren_pos + 1, trimmed_expression.size() - open_paren_pos - 2)); + *dist_data_arguments = trimWhitespace(trimmed_expression, open_paren_pos + 1, + trimmed_expression.size() - open_paren_pos - 2); return true; } @@ -863,7 +868,7 @@ void OpenMPMapClause::addItem(const std::string &expr, bool has_dist_data = splitMapExpressionDistDataSuffix( expr, &array_section_expression, &dist_data_arguments); - const std::string trimmed_expression = trimWhitespaceCopy(expr); + const std::string trimmed_expression = trimWhitespace(expr); std::string item_expression = trimmed_expression; if (has_dist_data) { item_expression = array_section_expression + " dist_data(" + @@ -881,7 +886,7 @@ void OpenMPMapClause::addItem(const std::string &expr, const std::vector policy_texts = splitTopLevelCommaSeparated(dist_data_arguments); for (const std::string &raw_policy : policy_texts) { - const std::string policy_text = trimWhitespaceCopy(raw_policy); + const std::string policy_text = trimWhitespace(raw_policy); if (policy_text.empty()) { continue; } @@ -907,12 +912,12 @@ void OpenMPMapClause::addItem(const std::string &expr, } } if (close_pos == std::string::npos || - trimWhitespaceCopy(policy_text.substr(close_pos + 1)).size() != 0) { + trimWhitespace(policy_text, close_pos + 1, policy_text.size() - close_pos - 1).size() != 0) { continue; } - policy_name = trimWhitespaceCopy(policy_text.substr(0, open_pos)); - policy_argument = trimWhitespaceCopy( - policy_text.substr(open_pos + 1, close_pos - open_pos - 1)); + policy_name = trimWhitespace(policy_text, 0, open_pos); + policy_argument = trimWhitespace( + policy_text, open_pos + 1, close_pos - open_pos - 1); } std::string normalized_name = policy_name; From 0ff2c01e9bbe1d1a24b5ebbb9df28a2154095d2b Mon Sep 17 00:00:00 2001 From: ouankou Date: Fri, 13 Feb 2026 05:20:26 +0000 Subject: [PATCH 2/4] fix: add bounds checking and overflow protection to trimWhitespace - Add check for pos >= text.size() to prevent out_of_range exception - Add overflow check for pos + count calculation (end_pos < pos) - Ensures robust handling of edge cases --- src/OpenMPIR.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/OpenMPIR.cpp b/src/OpenMPIR.cpp index 35f3ac15..3dbe4ee7 100644 --- a/src/OpenMPIR.cpp +++ b/src/OpenMPIR.cpp @@ -63,9 +63,12 @@ std::string trimWhitespace(const std::string &text) { } std::string trimWhitespace(const std::string &text, size_t pos, size_t count) { + if (pos >= text.size()) { + return std::string(); + } const char *whitespace = " \t\n\r\f\v"; size_t end_pos = pos + count; - if (end_pos > text.size()) { + if (end_pos > text.size() || end_pos < pos) { end_pos = text.size(); } size_t begin = text.find_first_not_of(whitespace, pos); From 63bfefc2da3e79cfbf150535bec83cbbf68c0dd0 Mon Sep 17 00:00:00 2001 From: ouankou Date: Sat, 14 Feb 2026 07:38:42 +0000 Subject: [PATCH 3/4] Codebase improvements: add check target, clean up, fix namespace --- CMakeLists.txt | 5 +++++ src/OpenMPIR.h | 14 ++++++-------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 72737549..e6e1179d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -83,6 +83,11 @@ if(OMPPARSER_ENABLE_WASM) add_subdirectory(wasm) endif() +add_custom_target(check + COMMAND ${CMAKE_CTEST_COMMAND} --output-on-failure + DEPENDS tester omp_roundtrip test_locations + COMMENT "Running all tests...") + set(ompparser_targets ompparser) # Install headers and libraries diff --git a/src/OpenMPIR.h b/src/OpenMPIR.h index 4fedae67..31af4025 100644 --- a/src/OpenMPIR.h +++ b/src/OpenMPIR.h @@ -22,8 +22,6 @@ #include #include -using namespace std; - enum OpenMPBaseLang { Lang_C, Lang_Cplusplus, Lang_Fortran, Lang_unknown }; enum OpenMPExprParseMode { @@ -211,7 +209,7 @@ class OpenMPDirective : public SourceLocation { * should only have one OpenMPClause object for each instance of kind and full * parameters */ - map *> clauses; + std::map *> clauses; // Owned storage for clause objects to ensure automatic cleanup std::vector> clause_storage; @@ -270,7 +268,7 @@ class OpenMPDirective : public SourceLocation { OpenMPDirectiveKind getKind() { return kind; }; - map *> *getAllClauses() { + std::map *> *getAllClauses() { return &clauses; }; @@ -320,8 +318,8 @@ class OpenMPDirective : public SourceLocation { // atomic directive class OpenMPAtomicDirective : public OpenMPDirective { protected: - map *> clauses_atomic_after; - map *> clauses_atomic_clauses; + std::map *> clauses_atomic_after; + std::map *> clauses_atomic_clauses; std::vector>> atomic_clause_vector_storage; @@ -343,11 +341,11 @@ class OpenMPAtomicDirective : public OpenMPDirective { } return clauses_atomic_clauses[kind]; }; - map *> * + std::map *> * getAllClausesAtomicAfter() { return &clauses_atomic_after; }; - map *> *getAllAtomicClauses() { + std::map *> *getAllAtomicClauses() { return &clauses_atomic_clauses; }; }; From 3e5696a12c3a79a127973b60d663ba13707cd540 Mon Sep 17 00:00:00 2001 From: ouankou Date: Thu, 26 Feb 2026 20:28:33 -0800 Subject: [PATCH 4/4] cmake: gate check target behind BUILD_TESTING --- CMakeLists.txt | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e6e1179d..16ae2f92 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -83,10 +83,12 @@ if(OMPPARSER_ENABLE_WASM) add_subdirectory(wasm) endif() -add_custom_target(check - COMMAND ${CMAKE_CTEST_COMMAND} --output-on-failure - DEPENDS tester omp_roundtrip test_locations - COMMENT "Running all tests...") +if(BUILD_TESTING) + add_custom_target(check + COMMAND ${CMAKE_CTEST_COMMAND} --output-on-failure + DEPENDS tester omp_roundtrip test_locations + COMMENT "Running all tests...") +endif() set(ompparser_targets ompparser)