Avoid some usages of int_vector_type & vector_type

[eivindjahren]

The goal of this PR is to make some progress towards not using internal vector types. I needed to resolve some bugs and design issues before that is possible.

This resolves severals bugs related to type conversions and having bad states representable:

1. parser.cpp would alias EOF by not using int as the return type for fgetc.
2. smspec would not always switch between representations of time
3. There was a long standing regression with select_box

[Copilot]

Pull request overview

This PR reduces reliance on internal int_vector_type / vector_type containers while fixing several correctness issues around parsing and summary time handling (including unit scaling and rejecting unusable SMSPEC time configurations).

Changes:

• Fix EOF-handling in the parser by avoiding char-based aliasing of EOF when consuming fgetc() results.
• Rework summary time handling to consistently interpret TIME units (DAYS vs HOURS) and correctly support DAY/MONTH/YEAR-only summaries (including rejecting partial/missing time information).
• Refactor several components away from internal vector types toward STL containers (std::vector, std::unordered_map, smart pointers), updating affected tests accordingly.

Reviewed changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
tests/rd_tests/test_sum.py	Adds/extends tests for DAY/MONTH/YEAR-only summaries, sim_length semantics, and rejection of insufficient time metadata.
tests/rd_tests/test_rd_sum.py	Adds tests for TIME unit scaling (DAYS vs HOURS), invalid time unit rejection, resampling preservation of time units, and negative report step validation.
lib/util/util.cpp	Removes util_fread_dev_urandom() implementation.
lib/util/test_work_area.cpp	Switches test-work-area random directory naming away from /dev/urandom to rand()-based generation.
lib/util/parser.cpp	Changes quoter/quote-seeking helpers to take int (supporting EOF) rather than char.
lib/tests/test_layer.cpp	Updates layer tracing tests for new STL-based tracing APIs.
lib/resdata/well_branch_collection.cpp	Replaces vector/index-map storage with std::unordered_map keyed by branch id.
lib/resdata/tests/well_branch_collection.cpp	Updates tests for removed iget-by-index start segment accessor.
lib/resdata/tests/rd_nnc_info_test.cpp	Adjusts NNC info tests (removes instance check).
lib/resdata/tests/rd_layer.cpp	Updates C++ tests to use STL containers and new tracing return types.
lib/resdata/rd_unsmry_loader.cpp	Switches to unified TimeInfo representation and scales TIME values based on SMSPEC unit.
lib/resdata/rd_sum_tstep.cpp	Uses TimeInfo to derive step timing and rejects creating new timesteps for date-only SMSPECs.
lib/resdata/rd_sum_file_data.cpp	Replaces internal vector_type ownership with std::vector<unique_ptr<...>> and updates sorting/indexing logic.
lib/resdata/rd_smspec.cpp	Introduces rd::TimeInfo (TIME vs DAY/MONTH/YEAR) and enforces presence of usable time metadata when reading SMSPEC.
lib/resdata/nnc_info.cpp	Reworks NNC info storage to STL containers and smart pointers; adjusts related APIs.
lib/resdata/layer.cpp	Replaces internal int vectors with STL containers; updates block tracing APIs to return vectors of indices.
lib/resdata/fault_block.cpp	Updates edge tracing to use STL std::vector<int> cell lists.
lib/resdata/fault_block_layer.cpp	Updates content tracing to use returned index tuples instead of output int vectors.
lib/private-include/detail/resdata/rd_unsmry_loader.hpp	Updates loader state to store TimeInfo instead of separate time index/scale fields.
lib/private-include/detail/resdata/rd_sum_file_data.hpp	Replaces vector_type* with std::vector<rd_sum_tstep_ptr> and updates append API.
lib/private-include/detail/resdata/layer_cxx.hpp	Updates C++ layer edge tracing signature to use STL std::vector<int>.
lib/include/resdata/well/well_branch_collection.hpp	Removes iget-by-index start segment accessor from the public header.
lib/include/resdata/rd_sum_tstep.hpp	Removes rd_sum_tstep_free__ and introduces an owning rd_sum_tstep_ptr alias.
lib/include/resdata/rd_smspec.hpp	Adds rd::TimeInfo types and exposes rd_smspec_get_time_info().
lib/include/resdata/nnc_info.hpp	Modernizes header and changes nnc_info_get_size() return type to size_t.
lib/include/resdata/layer.hpp	Updates public layer block-content tracing API to return a vector of (i,j) tuples.
lib/include/ert/util/util.hpp	Removes util_fread_dev_urandom() declaration.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

lib/include/resdata/nnc_info.hpp:19

• Public API functions in this header no longer have extern "C" linkage, while related headers (e.g. nnc_vector.hpp) still use it. If downstream code links against the previously unmangled symbols, this change will break ABI/linking. Consider restoring an extern "C" block around the C-style API declarations (alloc/free/etc.) while keeping the C++ convenience APIs outside.

nnc_info_type *nnc_info_alloc(int lgr_nr);
void nnc_info_free(nnc_info_type *nnc_info);
void nnc_info_add_nnc(nnc_info_type *nnc_info, int lgr_nr,
                      int global_cell_number, int nnc_index);

[Copilot]

Pull request overview

Copilot reviewed 31 out of 31 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

lib/resdata/rd_util.cpp:748

• Correct the spelling of “ambiguous” in this comment.

                    {"JLY", 7}, /* ECLIPSE ambigous on July. */

lib/util/parser.cpp:487

• Searching for an empty string currently reads one byte before declaring a match. When skip_string is true, the stream is therefore left one byte past its original position even though a zero-length match should not move it (and an EOF-only path can also attempt an incorrect rewind). Return immediately for the empty-string case, as util_fseek_string does.

    /* An empty string matches at the first position
       after any leading quotation or comment has been skipped. */
    const bool empty_string = string.empty();

[Copilot]

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

lib/util/parser.cpp:549

• For an empty search string, this branch runs only after fgetc() has already consumed a regular character. With skip_string=true the function therefore advances by one byte even though the matched string has length zero; at EOF, the non-skip path also attempts to seek backward by one byte. Preserve the position before the regular character and do not apply the non-empty rewind for this case.

        if (empty_string || c == string_start) {
            /* we got the first character right - lets try in more detail: */
            if (util_fgetc_while_equal(stream, string_rest, case_sensitive)) {

lib/resdata/rd_util.cpp:745

• “ambigous” is misspelled; use “ambiguous.”

                    {"JLY", 7}, /* ECLIPSE ambigous on July. */

[Copilot]

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated 1 comment.

Suppressed comments (4)

lib/include/ert/util/util.hpp:267

• Changing this public function from the existing C-linkage const char * API to a C++ std::string_view function removes the old symbol from the shared library, so already-compiled consumers will fail to link/load. Please retain the old C declaration and definition as a forwarding compatibility wrapper, and expose the string_view implementation separately.

bool util_fseek_string(FILE *stream, std::string_view string, bool skip_string,
                       bool case_sensitive);

lib/include/ert/util/parser.hpp:120

• This signature change also drops the former extern "C" block for every parser function, including unchanged APIs such as basic_parser_alloc, basic_parser_free, and basic_parser_tokenize_buffer. That changes their exported names and breaks existing shared-library consumers. Keep the unchanged APIs under C linkage and preserve a const char * compatibility entry point for seek, with the string_view form isolated as a C++ helper/overload.

bool basic_parser_fseek_string(const basic_parser_type *parser, FILE *stream,
                               const std::string_view string, bool skip_string,

lib/include/resdata/nnc_info.hpp:4

• As with nnc_vector.hpp, removing the former extern "C" section changes the exported names of the entire public NNC-info API. This silently breaks already-compiled clients of the default shared-library build. Please retain C-linkage compatibility entry points (and guard C++ convenience types separately), or explicitly version the API break.

#pragma once
#include <cstddef>
#include <vector>
#include <memory>

lib/include/resdata/nnc_info.hpp:28

• Changing this return type leaves narrowing/sign conversions in existing callers: lib/resdata/tests/rd_nnc_info_test.cpp:93 passes it to test_assert_int_equal, and lib/resdata/rd_grid.cpp:358 compares it with an int loop index. This conflicts with the repository's tracked narrowing-conversion policy. Update the full index/caller chain to size_t (including the assertion), or retain int until that migration is complete.

size_t nnc_info_get_size(const nnc_info_type *nnc_info);

[ajaust]

The change look good. I think it would be great if we could add some tests to the parser if this is not too much work (and out of scope).