Repository navigation
Conversation
alejandro-vaz
left a comment
alejandro-vaz
left a comment
There was a problem hiding this comment.
it works but I don't think this is the way
I was thinking more of having conditional visibility based on attributes
something similar to what the add-syntax crate does but just about visibility
|
I found a crate called |
|
yeah but we can't add the crate as dependency this is smallvec, each dependency is a potential risk, and depending on small crates with an unknown dependency graph is extremely risky for the ecosystem whatever we do, it has to be self-hosted |
|
i think we can write our own proc-marco, do the same thing as |
|
yeah I think that's the way |
0909ccf to
70aec69
Compare
|
wait wait wait let's think through it again I don't like the idea we have another crate to publish |
|
don't we have any way to do the same thing without having a completely new crate to publish?? |
|
hi, I've updated the PR to implement the proc-macro we discussed — it works, and there are no external dependencies. but proc macros have to live in their own crate, That feels like a lot of overhead for something small. |
I also find it quite odd, but I'm not sure how else to implement it. |
maybe just back to the original approach instead, copy the api twice？ |
|
I don't like copying things twice, it's just a bad practice can we hack ourselves into it with a simple macro-rules macro that injects a like macro_rules! public {
($attribute:meta) => {{
#[cfg($attribute)]
pub
}}
}or something weird like that, idk |
70aec69 to
5035f99
Compare
| // http://www.apache.org/licenses/LICENSE-2.0> or the MIT license | ||
| // <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your | ||
| // option. This file may not be copied, modified, or distributed | ||
| // except according to those terms. |
There was a problem hiding this comment.
we can move this file to src/macros.rs and convert that file to a proc-macro crate itself and then take the normal smallvec crate and re-export all those macros
There was a problem hiding this comment.
now that I think about it, this means we have to publish a new crate
I don't like that idea
|
|
||
| /// Makes the wrapped function part of the public API when the `#[cfg]` | ||
| /// predicate is true, and keeps it private otherwise. | ||
| macro_rules! pub_if { |
There was a problem hiding this comment.
I think we should rename it to something simpler like public
|
|
||
| #[cfg(not($($cond)*))] | ||
| $(#[$attr])* | ||
| pub(crate) const fn $($rest)* |
There was a problem hiding this comment.
we can remove the pub(crate) and just leave it private as-is
|
|
||
| #[cfg(not($($cond)*))] | ||
| $(#[$attr])* | ||
| pub(crate) fn $($rest)* |
| /// Makes the wrapped function part of the public API when the `#[cfg]` | ||
| /// predicate is true, and keeps it private otherwise. |
There was a problem hiding this comment.
this is not exported so we can simply remove the doc comment
| pub fn with_capacity_in(capacity: usize, allocator: Heap) -> Self { | ||
| Self::try_with_capacity_in(capacity, allocator).unwrap_or_else(SmallVecError::handle) | ||
| pub_if! { | ||
| #[cfg(any(feature = "allocator-api", feature = "allocator-api2"))] |
There was a problem hiding this comment.
this is the same as #[cfg(feature = "allocator-api")]
| .try_grow_raw(LocatedLength::new(0, false), capacity, &this.allocator) | ||
| }?; | ||
| pub_if! { | ||
| #[cfg(any(feature = "allocator-api", feature = "allocator-api2"))] |
| allocator | ||
| impl<Item, const INLINE: usize, Heap: Allocator> SmallVec<Item, INLINE, Heap> { | ||
| pub_if! { | ||
| #[cfg(any(feature = "allocator-api", feature = "allocator-api2"))] |
|
the only thing I don't like about this approach is that processing the token stream disables IDE help |
2 participants
issue #725
when no allocator feature is enabled, hide them from public API