Skip to content

Conversation

@ringabout
Copy link
Member

@ringabout ringabout commented Nov 11, 2022

Both implicit and explicit (?) conversion will be warned since it is an unsafe conversion. Unsafe features should probably be restricted to cast family.

ref #20761

here is the warning

Warning: unsafe conversion to 'cstring' from 'ptr char'; this will become a compile time error in the future [PtrToCstringConv]

@Araq Araq merged commit 8e1181b into devel Nov 11, 2022
@Araq Araq deleted the pr_warning_implicit branch November 11, 2022 18:32
@github-actions
Copy link
Contributor

Thanks for your hard work on this PR!
The lines below are statistics of the Nim compiler built from 8e1181b

Hint: mm: orc; opt: speed; options: -d:release
165065 lines; 8.517s; 614.188MiB peakmem

narimiran pushed a commit that referenced this pull request Nov 12, 2022
* issue a warning for ptr to cstring conversion[backport]

* add a changelog

(cherry picked from commit 8e1181b)
@narimiran
Copy link
Member

I've backported this, but now 1.6 CIs are failing: https://github.com/nim-lang/Nim/runs/9447983784

@ringabout
Copy link
Member Author

That's weird that the failure didn't appear on the devel branch, but on 1.6 branch. That aside, you can try this fix => #20834

@narimiran
Copy link
Member

That aside, you can try this fix => #20834

Yes, that did the trick.

And with that backported, I think we're ready for 1.6.10RC2. But please let me know if something else should be backported.

@ringabout
Copy link
Member Author

Nice, I think #20761 should be backported later when it is merged or at least part (cast[cstring]) of it should.

capocasa pushed a commit to capocasa/Nim that referenced this pull request Mar 31, 2023
* issue a warning for ptr to cstring conversion[backport]

* add a changelog
bung87 pushed a commit to bung87/Nim that referenced this pull request Jul 29, 2023
* issue a warning for ptr to cstring conversion[backport]

* add a changelog
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