Skip to content

Conversation

@stevefan1999-personal
Copy link
Contributor

@stevefan1999-personal stevefan1999-personal commented Apr 3, 2023

Given that #294 is staggering, I decided to maybe break it down one by one first. I tried to keep the Git profile as low as possible so that most of the Git logs are kept.

I further split the no_std features to not implicitly assume alloc. However, it is still effectively a requirement as not enabling this feature while std is not present will result in a compile error. But I think this is better as it will clarify the intention better.

Disabling std feature also implies error_in_core feature will be enabled and thus requires a nightly compiler to compile. But I guess anyone who works with no_std will have to use nightly either because we are most likely going to write low level code or compilers, who knows.

I changed my mind. This is gated behind an unstable feature in runtime crate now.

I've also pulled in to make the ExpectedSet use BTreeSet instead, so now we can have a stable order to print expected tokens.

Copy link
Owner

@kevinmehall kevinmehall left a comment

Choose a reason for hiding this comment

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

The github-actions failure was also on master, just a diagnostics change due to rustc update. Rebase on master to get the fix I just pushed.


Instead of the alloc feature, should it just use not(feature = "std")? Otherwise it fails to build without it:

$ cargo build -p peg-runtime --no-default-features
   Compiling peg-runtime v0.8.1 (/home/km/projects/rust/peg/peg-runtime)
error[E0412]: cannot find type `BTreeSet` in this scope
  --> peg-runtime/error.rs:15:15
   |
15 |     expected: BTreeSet<&'static str>,
   |               ^^^^^^^^ not found in this scope
   |
help: consider importing this struct
   |
3  | use std::collections::BTreeSet;
   |

error[E0412]: cannot find type `Vec` in this scope
  --> peg-runtime/error.rs:32:54
   |
32 |             let mut errors = self.tokens().collect::<Vec<_>>();
   |                                                      ^^^ not found in this scope
   |
help: consider importing this struct
   |
3  | use std::vec::Vec;
   |

error[E0433]: failed to resolve: use of undeclared type `BTreeSet`
  --> peg-runtime/error.rs:97:27
   |
97 |                 expected: BTreeSet::new(),
   |                           ^^^^^^^^ use of undeclared type `BTreeSet`
   |
help: consider importing this struct
   |
3  | use std::collections::BTreeSet;
   |

Some errors have detailed explanations: E0412, E0433.
For more information about an error, try `rustc --explain E0412`.
error: could not compile `peg-runtime` due to 3 previous errors

Ideally disabling alloc would be supportable by letting the user swap out the error-handling implementation with one that writes to a fixed-size buffer, or doesn't track the expected tokens, or something. But without implementing that (and all the other places that would have to allow replacing data structures to work without alloc), we should probably keep it simple and not make it look like that's a configuration that can be chosen.

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