Skip to content

Replace STD to CORE lib - #37

Open
bharatGoswami8 wants to merge 1 commit into
AS1100K:masterfrom
bharatGoswami8:std_to_core
Open

Replace STD to CORE lib#37
bharatGoswami8 wants to merge 1 commit into
AS1100K:masterfrom
bharatGoswami8:std_to_core

Conversation

@bharatGoswami8

Copy link
Copy Markdown
Contributor
  • Change most possible import from std to core
  • Updated alternative solution

@codecov-commenter

codecov-commenter commented May 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@bharatGoswami8
bharatGoswami8 force-pushed the std_to_core branch 6 times, most recently from 28df195 to d013dd5 Compare May 22, 2026 05:11
@bharatGoswami8
bharatGoswami8 marked this pull request as ready for review May 22, 2026 05:11
@bharatGoswami8

Copy link
Copy Markdown
Contributor Author

@AS1100K, could you please review this PR.

2 similar comments
@bharatGoswami8

Copy link
Copy Markdown
Contributor Author

@AS1100K, could you please review this PR.

@bharatGoswami8

Copy link
Copy Markdown
Contributor Author

@AS1100K, could you please review this PR.

@bharatGoswami8

Copy link
Copy Markdown
Contributor Author

@AS1100K , could you please review this PR.

1 similar comment
@bharatGoswami8

Copy link
Copy Markdown
Contributor Author

@AS1100K , could you please review this PR.

Copilot AI left a comment

Copy link
Copy Markdown

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 reduce reliance on std by switching many imports to core, while also adjusting token construction logic in the proc-macro implementation and tightening CI coverage enforcement.

Changes:

  • Replaced several std::* imports with core::* equivalents across the crate.
  • Updated pasted_to_tokens identifier handling and added additional invalid-identifier test cases (including a new compile-fail doctest).
  • Added a CI step to print coverage summary and enforce 100% line/region coverage thresholds.

Reviewed changes

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

Show a summary per file
File Description
src/test_helpers.rs Switches FromStr import from std to core in internal proc-macro test helpers.
src/segment.rs Switches Peekable import from std to core.
src/lib.rs Switches common imports to core, modifies identifier/literal token creation logic, and adds additional tests/doctests for invalid paste inputs.
src/error.rs Switches iterator/result type references from std to core.
src/attr.rs Switches iter, mem, and FromStr imports from std to core.
.github/workflows/ci.yml Adds an extra llvm-cov step to print coverage summary and enforce coverage thresholds.

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

Comment thread src/lib.rs Outdated
Comment on lines 368 to 382
// Justification for not catching panics from Ident::new/Ident::new_raw:
// After basic validation above, these functions might still panic for edge cases
// (e.g., keywords without raw mode, Unicode edge cases). If we reach this point
// with such input, it indicates a bug in our paste logic (segment parsing,
// modifier handling, or concatenation). Letting it panic surfaces these bugs
// during development/testing. The proc_macro validation is comprehensive and
// handles Unicode, keywords, and other edge cases correctly.
let ident = if raw_mode {
let mut spasted = pasted.clone();
spasted.remove(0);
spasted.remove(0);
TokenTree::Ident(Ident::new_raw(&spasted, span))
} else {
TokenTree::Ident(Ident::new(&pasted, span))
};

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Maybe just Revert this change

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Any alternative can you suggest, i want to remove the dependency of std most of the possible place.

@bharatGoswami8 bharatGoswami8 Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@AS1100K , Could you please check once the updated commit / solution, now we are doing error handing for those cases rather than panic.

Comment thread .github/workflows/ci.yml

@AS1100K AS1100K left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks @bharatGoswami8 for working on this!

I'm sorry that it took so long to review this PR as my university exams were going on during that time, and I didn't found any time looking into the PRs

Comment thread .github/workflows/ci.yml
Comment on lines 105 to +111
- run: cargo +nightly llvm-cov --workspace --branch --exclude-from-report pastey-test-suite --doctests --lcov --output-path lcov.info
env:
RUSTFLAGS: -C instrument-coverage -C llvm-args=-pgo-warn-missing-function
- name: Print coverage summary and thresould check
run: cargo +nightly llvm-cov --workspace --branch --exclude-from-report pastey-test-suite --doctests --fail-under-lines 100 --fail-under-regions 100
env:
RUSTFLAGS: -C instrument-coverage -C llvm-args=-pgo-warn-missing-function

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Can't we just keep the single run command instead of similar two run commands, so that in future it eliminates the chances of the commands not synced

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

no i think because the above one is storing the result in lcov.info which we can not print directly on CI.

Comment thread src/lib.rs Outdated
Comment on lines 368 to 382
// Justification for not catching panics from Ident::new/Ident::new_raw:
// After basic validation above, these functions might still panic for edge cases
// (e.g., keywords without raw mode, Unicode edge cases). If we reach this point
// with such input, it indicates a bug in our paste logic (segment parsing,
// modifier handling, or concatenation). Letting it panic surfaces these bugs
// during development/testing. The proc_macro validation is comprehensive and
// handles Unicode, keywords, and other edge cases correctly.
let ident = if raw_mode {
let mut spasted = pasted.clone();
spasted.remove(0);
spasted.remove(0);
TokenTree::Ident(Ident::new_raw(&spasted, span))
} else {
TokenTree::Ident(Ident::new(&pasted, span))
};

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Maybe just Revert this change

@bharatGoswami8
bharatGoswami8 requested a review from AS1100K June 3, 2026 16:43
@AS1100K

AS1100K commented Jun 9, 2026

Copy link
Copy Markdown
Owner

@bharatGoswami8 Why do we need we need the crate to shift away from using std to core. I don't see any benefit here, as this isn't going to benefit anyway as the proc-macro crates run on the host machine and the proc-macro using std will also work with crates #![no_std]

* Change most possible import from std to core
* Updated alternative solution
@bharatGoswami8

Copy link
Copy Markdown
Contributor Author

@bharatGoswami8 Why do we need we need the crate to shift away from using std to core. I don't see any benefit here, as this isn't going to benefit anyway as the proc-macro crates run on the host machine and the proc-macro using std will also work with crates #![no_std]

Hi @AS1100K ,
When we use some safety qualified compiler and, in those cases, we do need to use the dependency which is already qualified and std still not safety qualified yet, but core lib is partially qualified.

I have reverted major changes related catch_unwind, now only we are using the alternative of std from core.

@bharatGoswami8

Copy link
Copy Markdown
Contributor Author

@AS1100K, could you please review this PR.

@bharatGoswami8

Copy link
Copy Markdown
Contributor Author

@AS1100K, could you please review this PR and merge it.

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.

4 participants