Skip to content

Commit 2793b1a

Browse files
committed
Address review comments
1 parent ac20f26 commit 2793b1a

2 files changed

Lines changed: 43 additions & 33 deletions

File tree

src/runtime/modules/BuiltinsModule.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ PyResult<PyObject *> print(const PyTuple *args, const PyDict *kwargs, Interprete
114114
if (auto it = kwargs->map().find(file_keyword); it != kwargs->map().end()) {
115115
auto file_ = PyObject::from(it->second);
116116
if (file_.is_err()) { return file_; }
117-
file = file_.unwrap();
117+
if (file_.unwrap() != py_none()) { file = file_.unwrap(); }
118118
}
119119
if (auto it = kwargs->map().find(flush_keyword); it != kwargs->map().end()) {
120120
auto flush_ = PyObject::from(it->second);

src/runtime/modules/IOModule.cpp

Lines changed: 42 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -990,10 +990,15 @@ struct Buffered
990990
{
991991
if (auto err = check_initialized(); err.is_err()) return Err(err.unwrap_err());
992992

993-
if (n < -1) { return Err(value_error("read length must be non-negative or -1")); }
993+
if (n < 0) {
994+
// TODO: determine actual buffer size
995+
n = 4096;
996+
}
994997

995998
if (is_closed()) { return Err(value_error("read of closed file")); }
996999

1000+
if (n == 0) { return PyBytes::create(); }
1001+
9971002
const auto have = readahead();
9981003
if (have > 0) { return static_cast<T *>(this)->readfast(n); }
9991004
Bytes b;
@@ -3118,45 +3123,50 @@ class TextIOWrapper : public TextIOBase
31183123
std::numeric_limits<size_t>::max());
31193124
if (result.is_err()) { return Err(result.unwrap_err()); }
31203125
auto [limit] = result.unwrap();
3126+
return readline_impl(limit);
3127+
}
31213128

3129+
PyResult<PyObject *> readline_impl(size_t limit)
3130+
{
31223131
if (auto result = writeflush(); result.is_err()) { return Err(result.unwrap_err()); }
31233132

31243133
// Lines in the input can end in \'\\n\', \'\\r\', or \'\\r\\n\'
31253134
auto is_line_end = [](std::string_view remaining) {
3126-
if (remaining.starts_with("\n") || remaining.starts_with("\r")) {
3127-
return 1;
3128-
} else if (remaining.starts_with("\r\n")) {
3129-
return 2;
3130-
}
3135+
if (remaining.starts_with("\r\n")) { return 2; }
3136+
if (remaining.starts_with("\n") || remaining.starts_with("\r")) { return 1; }
31313137
return 0;
31323138
};
3133-
while (!m_buffer_bytes.has_value() || m_buffer_bytes->b.empty()) {
3134-
auto chunk = read_chunk(0);
3135-
if (chunk.is_err()) { return Err(chunk.unwrap_err()); }
3136-
if (chunk.unwrap() == 0) { break; }
3137-
}
3138-
3139-
std::string_view remaining{ bit_cast<const char *>(m_buffer_bytes->b.data()) + m_position,
3140-
m_buffer_bytes->b.size() - m_position };
3141-
3142-
// TODO: use utf8 codepoints
3143-
const char *start = remaining.data();
3144-
size_t len = 0;
3145-
limit = std::min(limit, remaining.size());
3146-
for (; len < limit; ++len) {
3147-
if (auto chars = is_line_end(remaining)) {
3148-
// TODO: what if len becomes greater than limit due to \r\n?
3149-
len += chars;
3150-
break;
3139+
std::string line;
3140+
bool found_nl = false;
3141+
while (!found_nl && line.size() < limit) {
3142+
if (!m_buffer_bytes.has_value() || m_buffer_bytes->b.empty()) {
3143+
auto chunk = read_chunk(0);
3144+
if (chunk.is_err()) { return Err(chunk.unwrap_err()); }
3145+
if (!chunk.unwrap()) { break; }
31513146
}
3152-
remaining = remaining.substr(1);
3153-
}
3154-
std::string line{ start, start + len };
3155-
std::rotate(
3156-
m_buffer_bytes->b.begin(), m_buffer_bytes->b.begin() + len, m_buffer_bytes->b.end());
3157-
m_position = 0;
3158-
m_buffer_bytes->b.resize(m_buffer_bytes->b.size() - line.size());
31593147

3148+
std::string_view remaining{ bit_cast<const char *>(m_buffer_bytes->b.data())
3149+
+ m_position,
3150+
m_buffer_bytes->b.size() - m_position };
3151+
3152+
// TODO: use utf8 codepoints
3153+
const char *start = remaining.data();
3154+
size_t len = 0;
3155+
const size_t scan_limit = std::min(limit - line.size(), remaining.size());
3156+
for (; len < scan_limit; ++len) {
3157+
if (auto chars = is_line_end(remaining)) {
3158+
// TODO: what if len becomes greater than limit due to \r\n?
3159+
len += chars;
3160+
found_nl = true;
3161+
break;
3162+
}
3163+
remaining = remaining.substr(1);
3164+
}
3165+
line.insert(line.end(), start, start + len);
3166+
m_buffer_bytes->b.erase(
3167+
m_buffer_bytes->b.begin(), m_buffer_bytes->b.begin() + m_position + len);
3168+
m_position = 0;
3169+
}
31603170
return PyString::create(std::move(line));
31613171
}
31623172

@@ -3167,7 +3177,7 @@ class TextIOWrapper : public TextIOBase
31673177
if (result_.is_err()) { return result_; }
31683178
auto *result = result_.unwrap();
31693179
while (m_position < m_buffer_bytes->b.size()) {
3170-
auto line = readline(nullptr, nullptr);
3180+
auto line = readline_impl(std::numeric_limits<size_t>::max());
31713181
if (line.is_err()) { return line; }
31723182
if (auto appended = result->append(line.unwrap()); appended.is_err()) {
31733183
return Err(appended.unwrap_err());

0 commit comments

Comments
 (0)