Skip to content

Conversation

tormol
Copy link
Collaborator

@tormol tormol commented Jul 14, 2019

I said I'd finish up my changes this weekend, and here they are.
It naturally turned into much more work than expected, mostly because in the process i found unsound trait impls, many inconsistencies, a way to make a const fn for creating AsciiChar from char, and many other small things I felt like changing.

Changelog:

  • Remove unsound impl From<&mut AsciiStr> for &mut [u8] and for &mut str.
  • Remove quickcheck.
  • Rename AsciiStr.trim_left() and AsciiStr.trim_right() to trim_start() and trim_end().
  • Rename AsciiChar::from{,_unchecked}() to from_ascii{,_unchecked}().
  • Remove impls of deprecated std::ascii::AsciiExt trait.
  • Rename some AsciiChar variants, most significantly Null to Nul (for consistency with CStr method names).
  • Remove AsciiStr::new() (in preparation for making it const sooner or changing it).
  • Make from_ascii_unchecked() non-generic (in preparation for making it const sooner).
  • Change Chars iterator returned by AsciiStr::chars() to produce values instead of references.
  • Change AsciiChar::is_whitespace() to return true for form-feed and vertical tab (for consistency with char::is_whitespace().
  • Change many AsciiChar methods to take self by reference for maximum compatibility with u8 and char.
  • Add slice_ascii_str() and get_ascii() to AsAsciiStr and slice_mut_ascii_str() to AsMutAsciiStr.
  • Hide Lines and Split behind impl Trait.
  • Change Chars and CharsMut from being typedefs for [AsciiChar] iterators to being newtypes.
  • Require Rust 1.36 for 1.0.*, and allow later semver-compatible releases to increase it.
  • Make most AsciiChar methods const fn and add AsciiChar::new().
  • Implement AsRef<AsciiStr> for AsciiChar and AsciiStr, and AsMut<AsciiStr> for AsciiStr.
  • Implement ToAsciiChar for i8, u16 and u32.
  • Implement Clone, Copy and Eq for ToAsciiCharError.
  • Add many is_xxx() methods to AsciiChar.

tormol added 11 commits July 11, 2019 16:56
i8 is what c_char corresponds to on x86.
Together with the impl for u8 this means ToAsciiChar is always
implemented for c_char (on any relevant architecture).
* impl From<&mut AsciiStr> for &mut [u8]
* impl From<&mut AsciiStr> for &mut str

They allow writing non-ASCII values to an AsciiStr which when read
out as an AsciiChar will produce values outside the valid niche.
Updating it to 0.8 is difficult, and removing it makes the crate
easier to maintain for Debian.
* Replace Chars and CharsMut type aliases with custom types.
* Add CharsRef.
* Make Chars an Item=AsciiChar iterator instead of Item=&'a AsciiChar.
* Export Split so that rustdoc documents it.

Returning impl trait would make {as,into}_str() methods inaccessible.
Cannot use RangeBounds or SliceIndex because it would conflict
with Index<usize> which returns a different type.
tormol added 9 commits July 15, 2019 01:04
* add const fn AsciiChar::new()
* make all is_xxx() const fn
* make eq_ignore_ascii_case() and to_ascii_{upper,lower}case() const fn
* make as_char() and as_byte() const fn
* make #![no_std] ToAsciiCharError::description() const fn

methods not made const fn:
* AsciiChar::from() require branching
  (Could add a from_byte() methods using an 256-element array,
   but that doesn't seem worth it for now)
* AsciiChar::from_unchecked() require transmuting or equivalents
* make_ascii_{upper,lower}case() require mutating
* as_printable_char() doesn't seem useful enough to bother with a [char; 128]
... for consistency with str::from_utf8[_unchecked]().
* De-genericize AsciiStr::new() and from_ascii_unchecked() so that they can
  hopefully be made const fn sooner (due to needing fewer features).
* Remove AsciiStr::new() because I want to make it similar to AsciiChar::new(),
  but also want to reserve the name for what becomes possible as const fn first.
* Make AsciiStr::split() return impl Trait to make changing it to
  take a Pattern once that is stabilized easier.
* Make AsciiStr::lines() return const fn just in case.
* as_slice()
* as_ptr()
* AsAsciiStrError::valid_up_to()
* #![no_std] AsAsciiStrError::description()

All other methods either require pointer "dereferencing", branching,
calling not yet const fn std methods or return impl Trait.
* Implement identity AsRef and AsMut for AsciiStr.
* Implement AsRe<AsciiStr> for AsciiChar.
* implement Extend and IntoIterator<AsciiString> for
  any iterable with Item=AsRef<AsciiChar>.
* Add methods slice_ascii(), and get_ascii() to AsAsciiStr
* Add methods slice_ascii_mut() to AsMutasciiStr

The main use case is avoiding accidentally panicking by slicing a str
in the middle of a codepoint with AsciiStr::from_ascii(str[a..=b])
or AsciiChar::from(str[n]).

I had hoped to finally use RangeArgument, but almost nothing in std takes it
because they instead went for their own unstable and sealed trait (SliceIndex).
Converting between them is also impossible without going through trait objects.
SliceIndex is actually workable though, albeit with one wart (associated type).
Working with mutable slices is hairy as usual.
* Change is_whitespace() to also return true for AsciiCHar::VT and ::FF.
* Rename a bunch of ctype methods and make them take self by reference:
  * is_digit() => is_ascii_digit()
  * is_hex() => is_ascii_hexdigit()
  + is_control() => is_ascii_control()
  + is_graphic() => is_ascii_graphic()
  + is_blank() => is_ascii_blank()
  + is_print() => is_ascii_printable()
  * is_punctuation() => is_ascii_punctuation()
* Ddd identical _ascii methods when char also has methods without _ascii,
  except that the _ascii methods take self by reference:
  * is_ascii_alphabetic() = is_alphabetic()
  * is_ascii_alphanumeric() = is_alphanumeric()
  * is_ascii_uppercase() = is_uppercase()
  * is_ascii_lowercase() = is_lowercase()
* Add is_digit() which takes base as parameter.
* Add is_ascii_whitespace() which returns true for FF but not VT.
@tomprogrammer
Copy link
Owner

The requirement of rustc 1.36 seems a bit high. Debian for example packages 1.34.2 in Buster, which could be our baseline for rustc compatibility I think.

@tormol
Copy link
Collaborator Author

tormol commented Jul 16, 2019

The idea was to allow us to use as many features as possible without increasing MSRV, but aving at least one 1.0 version usable with packaged rustc makes sense.
With 1.36 we could actually use alloc-in-'#![no_std], but we'll still need a "std" feature for ErrorandCStr`, so that can wait until later.
Because I don't want a patch release to break what previously happened to compile, the minimum supported version becomes 1.33.0 as that is the minimum version that compiles.

Copy link
Owner

@tomprogrammer tomprogrammer left a comment

Choose a reason for hiding this comment

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

I only found some places where I have a different opinion about taking self or &self. Else all changes look fine.

Copy link
Collaborator Author

@tormol tormol left a comment

Choose a reason for hiding this comment

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

Sorry for not responding for three weeks.
Despite being so close to the finish line I didn't have any motivation to pick this PR up again.

tormol added 2 commits August 20, 2019 22:36
* Make methods const fn and #[inline] even if just a different name for another method.
* Make all is_ascii_xxx() methods take self by reference.
Not running clippy on nightly doesn't help much when the
before_script fails if the component isn't available today.
@tomprogrammer
Copy link
Owner

Thanks for the updates here, I think the PR is ready to be merged and published.

@tomprogrammer tomprogrammer merged commit 4e0fce5 into tomprogrammer:master Aug 21, 2019
@tormol tormol deleted the breaking branch August 21, 2019 16:45
@tormol
Copy link
Collaborator Author

tormol commented Aug 21, 2019

Changelog:

Breaking changes:

* Change `AsciiChar.is_whitespace()` to also return true for '\0xb' (vertical tab) and '\0xc' (form feed).
* Remove quickcheck feature.
* Remove `AsciiStr::new()`.
* Rename `AsciiChar::from()` and `AsciiChar::from_unchecked()` to `from_ascii()` and `from_ascii_unchecked()`.
* Rename several `AsciiChar.is_xxx()` methods to `is_ascii_xxx()` (for comsistency with std).
* Rename `AsciiChar::Null` to `Nul` (for consistency with eg. `CStr::from_bytes_with_nul()`).
* Rename `AsciiStr.trim_left()` and `AsciiStr.trim_right()` to `trim_start()` and `trim_end()`.
* Remove impls of the deprecated `std::ascii::AsciiExt` trait.
* Change iterators `Chars`, `CharsMut` and `CharsRef` from type aliases to newtypes.
* Return `impl Trait` from `AsciiStr.lines()` and `AsciiStr.split()`, and remove iterator types `Lines` and `Split`.
* Add `slice_ascii_str()`, `get_ascii()` and `unwrap_ascii()` to the `AsAsciiStr` trait.
* Add `slice_mut_ascii_str()` and `unwrap_ascii_mut()` to the `AsMutAsciiStr` trait.
* Require Rust 1.33.0 for 1.0.\*, and allow later semver-compatible 1.y.0 releases to increase it.

Additions:

* Add `const fn` `AsciiChar::new()` which panicks on invalid values.
* Make most `AsciiChar` methods `const fn`.
* Add multiple `AsciiChar::is_[ascii_]xxx()` methods.
* Implement `AsRef<AsciiStr>` for `AsciiChar`.
* Make `AsciiString`'s `Extend` and `FromIterator` impl generic over all `AsRef<AsciiStr>`.
* Implement inclusive range indexing for `AsciiStr` (and thereby `AsciiString`).
* Mark `AsciiStr` and `AsciiString` `#[repr(transparent)]` (to `[AsciiChar]` and `Vec<AsciiChar>` respectively).

Also, a README update which I had stashed away:

diff --git a/README.md b/README.md
index 192abf0..e054cf1 100644
--- a/README.md
+++ b/README.md
@@ -11,15 +11,16 @@ dependencies section in `Cargo.toml`:
 
 ```toml
 [dependencies]
-ascii = "0.9"
+ascii = "1.0"
\`\`\`
 
 ## Using ascii without libstd
 
 Most of `AsciiChar` and `AsciiStr` can be used without `std` by disabling the
 default features. The owned string type `AsciiString` and the conversion trait
-`IntoAsciiString` as well as all methods referring to these types are
-unavailable. The `Error` trait is also unavailable, but `description()` is made
+`IntoAsciiString` as well as all methods referring to these types and
+`CStr` and `CString` are unavailable.
+The `Error` trait is also unavailable, but `description()` is made
 available as an inherent method for `ToAsciiCharError` and `AsAsciiStrError`.
 
 To use the `ascii` crate in `core`-only mode in your cargo project just add the
@@ -27,7 +28,7 @@ following dependency declaration in `Cargo.toml`:
 
 ```toml
 [dependencies]
-ascii = { version = "0.9", default-features = false }
+ascii = { version = "1.0", default-features = false }
\`\`\`
 
 ## Minimum supported Rust version
@@ -36,7 +37,7 @@ The minimum Rust version for 1.0.\* releases is 1.33.0.
 Later 1.y.0 releases might require newer Rust versions, but the three most
 recent stable releases at the time of publishing will always be supported.  
 For example this means that if the current stable Rust version is 1.38 when
-ascii 1.1.0 is released, then ascii 1.1.* will not require a newer
+ascii 1.1.0 is released, then ascii 1.1.\* will not require a newer
 Rust version than 1.36.
 
 ## History

(to apply, replace "\`\`\`" with "```" and then run git apply).

EDIT: Updated release notes to include is_whitespace() change and remove 0.9.3 items.

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.

2 participants