Skip to content

2026PKUCourseHW5:replace atoi/atof with safe parsing helper#7142

Closed
xiaoxuPekingUniversity wants to merge 4 commits intodeepmodeling:developfrom
xiaoxuPekingUniversity:my-feature
Closed

2026PKUCourseHW5:replace atoi/atof with safe parsing helper#7142
xiaoxuPekingUniversity wants to merge 4 commits intodeepmodeling:developfrom
xiaoxuPekingUniversity:my-feature

Conversation

@xiaoxuPekingUniversity
Copy link
Copy Markdown

This PR replaces atoi/atof in read_pp_upf201.cpp with safer helper functions based on std::stoi and std::stod.
The change improves robustness of string-to-number conversion while keeping the original parsing logic unchanged.

Copilot AI review requested due to automatic review settings March 26, 2026 15:22
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to improve robustness of UPF 2.0.1 header parsing by replacing some atoi/atof usages in read_pp_upf201.cpp with helper functions based on std::stoi/std::stod.

Changes:

  • Added parse_int / parse_double helpers (anonymous namespace) for stricter string-to-number parsing.
  • Switched several numeric fields in read_pseudo_upf201_header from atoi/atof/std::stod to the new helpers.
  • Accidentally added an editor swap file (.swp) to the repository.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated 5 comments.

File Description
source/source_cell/read_pp_upf201.cpp Adds safe parsing helpers and uses them for several UPF201 header numeric fields.
source/source_cell/.read_pp_upf201.cpp.swp Newly added Vim swap file (should not be committed).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread source/source_cell/read_pp_upf201.cpp Outdated
Comment on lines +228 to +233
return value;
}
catch (...)
{
throw std::runtime_error("Invalid floating-point value: " + s);
}
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Catching ... here will also intercept unrelated exceptions (e.g., std::bad_alloc) and lose the original diagnostic. Prefer catching std::invalid_argument and std::out_of_range from std::stoi/std::stod and report those via the project’s error mechanism so the failure reason is preserved.

Copilot uses AI. Check for mistakes.
Comment thread source/source_cell/read_pp_upf201.cpp Outdated
Comment on lines +198 to +203
namespace//辅助函数用于后续类型替换
{
int parse_int(const std::string& s)
{
try
{
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR description says atoi/atof are replaced in read_pp_upf201.cpp, but this file still contains multiple remaining atoi/atof uses (e.g., in read_pseudo_upf201_mesh, PP_NONLOCAL, PP_PSWFC, and SO parsing). Either update the description to reflect the narrower scope (header-only), or complete the replacement consistently across the file.

Copilot uses AI. Check for mistakes.
Comment thread source/source_cell/read_pp_upf201.cpp Outdated
Comment thread source/source_cell/read_pp_upf201.cpp
Comment thread source/source_cell/read_pp_upf201.cpp Outdated
Comment on lines +205 to +209
int value = std::stoi(s, &pos);
if (pos != s.size())
{
throw std::runtime_error("extra characters");
}
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These helpers use std::runtime_error, but this translation unit only includes read_pp.h and does not explicitly include <stdexcept>. Please add the proper standard header in this .cpp to avoid relying on transitive includes.

Copilot uses AI. Check for mistakes.
@xiaoxuPekingUniversity xiaoxuPekingUniversity changed the title No.2:replace atoi/atof with safe parsing helper 2026PKUCourseHW5:replace atoi/atof with safe parsing helper Mar 26, 2026
xiaoxuPekingUniversity and others added 3 commits March 27, 2026 10:56
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@mohanchen
Copy link
Copy Markdown
Collaborator

Nice try! Thanks for your contribution.

@mohanchen mohanchen closed this Mar 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants