Reimplement public interface for stability #71

alexcrichton · 2018-03-28T22:40:13Z

More information to come later about this, but this is a result of the
work week discussions we've had about stabilizing procedural macros

dtolnay · 2018-03-29T09:00:44Z

src/lib.rs

    pub fn def_site() -> Span {
-        Span(imp::Span::def_site())
+        Span::_new(imp::Span::def_site())
    }

    /// Creates a new span with the same line/column information as `self` but
    /// that resolves symbols as though it were at `other`.
    pub fn resolved_at(&self, other: Span) -> Span {


Should this and located_at be procmacro2_semver_exempt?

I initially had that yeah but this was used by serde_derive, but I can remove the usage from serde_derive

dtolnay · 2018-03-29T09:01:02Z

src/stable.rs

@@ -1,3 +1,5 @@
+#![allow(dead_code)]


Will need to clean this up.

Perhaps yeah, I personally find the soup of #[cfg] on imports somewhat unappealing and am generally ok with dead code otherwise, so I might personally lean towards leaving this

dtolnay · 2018-03-29T09:04:01Z

src/stable.rs

-                TokenNode::Group(delim, ref stream) => {
-                    let (start, end) = match delim {
+            match *tt {
+                TokenTree::Group(ref g) => {


I would prefer to use the Syn match naming convention here, https://docs.rs/syn/0.12.15/syn/enum.Expr.html#syntax-tree-enums. For example TokenTree::Op(ref tt) => ... tt.op() ... to avoid op.op().

dtolnay · 2018-03-29T09:06:13Z

src/unstable.rs

@@ -158,11 +162,9 @@ impl fmt::Debug for TokenTreeIter {
    }
 }

-#[cfg(procmacro2_semver_exempt)]


Why is this being changed?

Ah this is the same as the #[cfg] soup above

dtolnay · 2018-03-29T09:24:03Z

src/lib.rs

    }
 }

-pub struct TokenTreeIter(imp::TokenTreeIter);
+pub struct TokenTreeIter {


We may want a different name here because if we ever implement IntoIterator for &TokenStream then we will want this name.

dtolnay · 2018-03-29T09:25:13Z

src/lib.rs

    }

-    pub fn as_str(&self) -> &str {
-        self.0.as_str()
+    pub fn stream(&self) -> &TokenStream {


This is sort of an unfortunate signature because you can't do anything with &TokenStream except clone or check if empty. Would it work to implement IntoIterator for &TokenStream? Or is there a cheap way to return TokenStream here?

dtolnay · 2018-03-29T09:31:12Z

src/lib.rs

+        }
+    }
+
+    pub fn set_span(&mut self, span: Span) {


We may need to clarify that this is a shallow set_span. Same for Group::set_span. Is that going to be confusing?

dtolnay · 2018-03-29T09:33:50Z

.travis.yml

@@ -3,7 +3,7 @@ sudo: false

 matrix:
  include:
-    - rust: 1.15.0
+    - rust: 1.23.0


Please restore support for 1.15.0.

More information to come later about this, but this is a result of the work week discussions we've had about stabilizing procedural macros

eddyb · 2018-03-30T22:01:39Z

src/lib.rs

-        Literal(f.into())
+    pub fn f32_unsuffixed(f: f32) -> Literal {
+        assert!(f.is_finite());
+        Literal::_new(imp::Literal::float(f as f64))


This is a bit scary, don't we want to stringify the exact f32 value? Not sure if this specific case is a problem but the compiler tried really hard (before APFloat, mostly) to avoid casting between f32 and f64.

Yeah this'll get fixed one we update upstream to match this api

eddyb · 2018-03-30T22:14:35Z

Aside from #71 (comment) (and a broader question of "what do we even want literals to be?"), I'm curious what the plans for bringing the built-in proc_macro in line with this (or a similar) API are.

Specifically, this design is much nicer for the decoupling being done in rust-lang/rust#49219, so I would prefer if we could first land this new API on rust-lang/rust master, simplifying my PR.

Should it be done in lockstep, just to avoid friction for people who are using proc-macro2 on nightly?

alexcrichton · 2018-03-31T06:34:48Z

@eddyb I plan to land this here and then land referenced prs shortly after. Once that's done I'll send a PR to Rust nightly and we can update proc-macro2 once that lands

That's the current plan at least!

alexcrichton · 2018-03-31T09:36:49Z

Ok! After lots of discussion at the work week I think we've settled on this API as the best way forward for stabilizing the proc_macro crate itself. I'm going to merge this, wait for CI to pass and docs get published, make sure it all looks ok, and then publish a new version on crates.io. This new version I'll then plumb through to quote/syn/etc

This was referenced Mar 28, 2018

Update to the proc-macro2 0.3.0 branch dtolnay/quote#66

Merged

Update to proc-macro2 0.3 and tweak apis dtolnay/syn#381

Merged

alexcrichton force-pushed the v3 branch from 143b98a to 0661745 Compare March 28, 2018 22:48

dtolnay reviewed Mar 29, 2018

View reviewed changes

.travis.yml Outdated

@@ -3,7 +3,7 @@ sudo: false

matrix:

include:

- rust: 1.15.0

- rust: 1.23.0

Copy link

Owner

dtolnay Mar 29, 2018

Choose a reason for hiding this comment

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

Please restore support for 1.15.0.

alexcrichton force-pushed the v3 branch 6 times, most recently from 5993cd2 to 1f0e797 Compare March 29, 2018 18:59

Reimplement public interface for stability

af5bad4

More information to come later about this, but this is a result of the work week discussions we've had about stabilizing procedural macros

alexcrichton force-pushed the v3 branch from 1f0e797 to af5bad4 Compare March 29, 2018 19:15

eddyb reviewed Mar 30, 2018

View reviewed changes

alexcrichton merged commit b2b4df1 into master Mar 31, 2018

alexcrichton deleted the v3 branch March 31, 2018 09:36

This was referenced Mar 31, 2018

Enabling the nightly feature can be a breaking change #67

Closed

Consider moving all APIs not in proc_macro under the procmacro2_semver_exempt cfg option #39

Closed

Repository owner locked and limited conversation to collaborators Dec 30, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reimplement public interface for stability #71

Reimplement public interface for stability #71

alexcrichton commented Mar 28, 2018

dtolnay Mar 29, 2018

alexcrichton Mar 29, 2018

dtolnay Mar 29, 2018

alexcrichton Mar 29, 2018

dtolnay Mar 29, 2018

dtolnay Mar 29, 2018

alexcrichton Mar 29, 2018

dtolnay Mar 29, 2018

dtolnay Mar 29, 2018

dtolnay Mar 29, 2018

dtolnay Mar 29, 2018

eddyb Mar 30, 2018

alexcrichton Mar 31, 2018

eddyb commented Mar 30, 2018 •

edited

Loading

alexcrichton commented Mar 31, 2018

alexcrichton commented Mar 31, 2018

Reimplement public interface for stability #71

Reimplement public interface for stability #71

Conversation

alexcrichton commented Mar 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eddyb commented Mar 30, 2018 • edited Loading

alexcrichton commented Mar 31, 2018

alexcrichton commented Mar 31, 2018

eddyb commented Mar 30, 2018 •

edited

Loading