Move transparent_newtype to use include! by mpbagot · Pull Request #5612 · rust-bitcoin/rust-bitcoin

mpbagot · 2026-02-04T03:27:08Z

The transparent_newtype macro from internals is one of many cross-crate macros which we'd like to remove over time.

Patch 1 adjusts hashes newtype doc comments.
Patch 2 copies transparent_newtype macro to includes directory and replaces all uses of internals::transparent_newtype with include! calls and direct usage of the macro.

Contributes to #5596

tcharding · 2026-02-04T06:17:02Z

NOTE: While this builds correctly with cargo/rustc, vscode users will notice that rust-analyzer lights the IDE up with a whole bunch of errors due to problems resolving an include! path outside of the immediate crate.
This is (possibly) still an open problem that needs resolving for workspace-wide include! usage.

I don't personally use rust-analyzer and I really want to not care but in reality this seems like a show stopper?

FWIW I think we should put the include! call is lib.rs always. I tested it in hashes and it works. FTR I have no clue how the macro call is being resolved, might be worth documenting the calls:

include!("../../includes/newtype.rs"); // Explained in `REPO_DIR/docs/README.md`.

// Defined in `REPO_DIR/includes/newtype.rs`.
transparent_newtype! {
...
}

And writing a README (if we ever do this).

apoelstra · 2026-02-08T10:48:47Z

I'm using helix with rust-analyzer and it ~~seems to work fine for me~~ weirdly I get a "failed to load file ../../includes/newtype.rs" error but it still recognizes the transparent_newtype macro (and does not recognize a typo of it).

This seems fine to me.

At some point, if the language is going to be this hostile as to have no specialization, overboaring coherence rules, no useful syntax for doing blanket impls, and no codegen, we should just switch languages. But my feeling is that "some IDEs show spurious errors" is the least worst of all worlds.

mpbagot · 2026-02-09T00:46:23Z

I've fixed up the docs CI errors, moved the uses of include! to lib.rs in each of the crates and added a short paragraph to the docs.

tcharding

ACK 40fd3a0

tcharding · 2026-02-10T04:28:41Z

Perhaps throw an SPDX licence comment on the new file.

// SPDX-License-Identifier: CC0-1.0

tcharding · 2026-02-10T04:29:20Z

I rekon we can undraft, merge, YOLO!

tcharding

ACK 56c73d9

tcharding

ACK a03d64f

tcharding · 2026-03-03T04:47:07Z

Still needs rebase mate.

mpbagot · 2026-03-03T04:51:02Z

Yep, I know. My local master was out of date. The macro is now causing lint errors in the unsafe parts, so I'm trying to figure that out.

apoelstra · 2026-03-04T22:14:42Z

Sorry, wrong button. Looks like it needs an API update.

mpbagot · 2026-03-05T01:28:15Z

Looks like it needs an API update.

API update is broken on master. It's fixed in #5764 . I can rebase after to fix the CI if you'd like.

tcharding

ACK baa342d

tcharding · 2026-03-08T21:59:33Z

include/newtype.rs

+        internals::_emit_alloc! {
+            $(#[$($from_box_attr)*])*
+            #[inline]
+            #[allow(clippy::ptr_as_ptr)]


FTR I had to look this lint up. I wrestled with it for a bit but couldn't come up with anything other than this solution.

Perhaps use this?

#[allow(clippy::ptr_as_ptr)] // Cannot use `cast()` because size of `Self` is not know at compile time.

FTR I like to comment on less well known attributes to save the next guy investigating why the attribute is there or checking if it has become stale.

tcharding

ACK baa342d

tcharding

ACK c0c49f1

apoelstra · 2026-03-29T16:00:40Z

c0c49f1 needs rebase

apoelstra · 2026-03-29T16:00:53Z

I have gotten my local CI to like the include/ directory so when this is rebased I can test and merge it.

tcharding

ACK 1839853

apoelstra · 2026-03-30T04:00:27Z

In ce15f30:

Tobin has complained about this formatter patch in multiple other places (including in #5918 which is closed). Should we fix that here?

apoelstra · 2026-03-30T04:01:16Z

Also, for those new to the project -- this kind of shit is one reason we don't enforce rustfmt here.

mpbagot · 2026-03-30T04:33:45Z

Tobin has complained about this formatter patch in multiple other places (including in #5918 which is closed). Should we fix that here?

I made a variation on it in #5919 that he ACK'ed after I added some skips. I've cherry-picked that in here in place of the plain format patch. If you'd still prefer to have it elsewhere to keep the non-automated changes separate, then I can open another PR for the formatting changes.

tcharding

ACK 88b9a60

tcharding · 2026-03-31T04:06:24Z

Its easy enough to review, I personally don't mind how or where it goes in.

In hashes, a handful of the transparent_newtype invocations for hashes use doc comments that are missing backticks around names or are generally unclean. These should be normalised to suit typical style. Adjust doc comments on sha256t, sha256d, muhash and siphash hash newtypes.

The transparent_newtype macro from internals is one of many cross-crate macros which we'd like to remove over time. Copy transparent_newtype macro to include directory. Replace all uses of internals::transparent_newtype with include! calls and direct usage of the macro.

mpbagot · 2026-03-31T14:37:05Z

Rebased because the formatting is fixed by #5927

tcharding

ACK ac8f61f

apoelstra

ACK ac8f61f; successfully ran local tests

github-actions bot added C-bitcoin PRs modifying the bitcoin crate C-hashes PRs modifying the hashes crate C-internals PRs modifying the internals crate C-io PRs modifying the io crate C-primitives labels Feb 4, 2026

mpbagot changed the title ~~Move transparent_newtype to use inlude!~~ Move transparent_newtype to use include! Feb 4, 2026

mpbagot mentioned this pull request Feb 5, 2026

Move remaining pow types to units #5608

Open

mpbagot force-pushed the feature/newtype-macro-includes branch from b798d04 to 40fd3a0 Compare February 9, 2026 00:15

github-actions bot added the doc label Feb 9, 2026

tcharding previously approved these changes Feb 10, 2026

View reviewed changes

mpbagot dismissed tcharding’s stale review via 56c73d9 February 10, 2026 05:16

mpbagot force-pushed the feature/newtype-macro-includes branch from 40fd3a0 to 56c73d9 Compare February 10, 2026 05:16

mpbagot marked this pull request as ready for review February 10, 2026 05:16

tcharding previously approved these changes Feb 12, 2026

View reviewed changes

mpbagot dismissed tcharding’s stale review via a03d64f February 16, 2026 06:52

mpbagot force-pushed the feature/newtype-macro-includes branch from 56c73d9 to a03d64f Compare February 16, 2026 06:52

tcharding approved these changes Feb 17, 2026

View reviewed changes

mpbagot force-pushed the feature/newtype-macro-includes branch from a03d64f to f9eddd2 Compare March 3, 2026 04:41

mpbagot force-pushed the feature/newtype-macro-includes branch from f9eddd2 to ca6f2c5 Compare March 3, 2026 04:55

apoelstra closed this Mar 4, 2026

apoelstra reopened this Mar 4, 2026

mpbagot force-pushed the feature/newtype-macro-includes branch from ca6f2c5 to baa342d Compare March 7, 2026 00:47

tcharding approved these changes Mar 8, 2026

View reviewed changes

tcharding reviewed Mar 8, 2026

View reviewed changes

tcharding approved these changes Mar 8, 2026

View reviewed changes

tcharding added the one ack PRs that have one ACK, so one more can progress them label Mar 8, 2026

mpbagot force-pushed the feature/newtype-macro-includes branch from baa342d to c0c49f1 Compare March 9, 2026 00:46

tcharding approved these changes Mar 9, 2026

View reviewed changes

tcharding mentioned this pull request Mar 9, 2026

Release tracking PR: The whole stack up to bitcoin 0.34.0-beta #5796

Draft

mpbagot force-pushed the feature/newtype-macro-includes branch 2 times, most recently from bbb1a9b to 1839853 Compare March 29, 2026 17:20

tcharding approved these changes Mar 29, 2026

View reviewed changes

mpbagot force-pushed the feature/newtype-macro-includes branch from 1839853 to 88b9a60 Compare March 30, 2026 04:28

tcharding approved these changes Mar 31, 2026

View reviewed changes

tcharding mentioned this pull request Mar 31, 2026

hashes: fmt skip a bunch of stuff #5927

Merged

mpbagot added 2 commits April 1, 2026 01:34

mpbagot force-pushed the feature/newtype-macro-includes branch from 88b9a60 to ac8f61f Compare March 31, 2026 14:34

tcharding approved these changes Apr 1, 2026

View reviewed changes

apoelstra approved these changes Apr 1, 2026

View reviewed changes

apoelstra merged commit dde0c21 into rust-bitcoin:master Apr 1, 2026
29 checks passed

Conversation

mpbagot commented Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tcharding commented Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

apoelstra commented Feb 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mpbagot commented Feb 9, 2026

Uh oh!

tcharding left a comment

Choose a reason for hiding this comment

Uh oh!

tcharding commented Feb 10, 2026

Uh oh!

tcharding commented Feb 10, 2026

Uh oh!

tcharding left a comment

Choose a reason for hiding this comment

Uh oh!

tcharding left a comment

Choose a reason for hiding this comment

Uh oh!

tcharding commented Mar 3, 2026

Uh oh!

mpbagot commented Mar 3, 2026

Uh oh!

apoelstra commented Mar 4, 2026

Uh oh!

mpbagot commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tcharding left a comment

Choose a reason for hiding this comment

Uh oh!

tcharding Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

tcharding Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

tcharding left a comment

Choose a reason for hiding this comment

Uh oh!

tcharding left a comment

Choose a reason for hiding this comment

Uh oh!

apoelstra commented Mar 29, 2026

Uh oh!

apoelstra commented Mar 29, 2026

Uh oh!

tcharding left a comment

Choose a reason for hiding this comment

Uh oh!

apoelstra commented Mar 30, 2026

Uh oh!

apoelstra commented Mar 30, 2026

Uh oh!

mpbagot commented Mar 30, 2026

Uh oh!

tcharding left a comment

Choose a reason for hiding this comment

Uh oh!

tcharding commented Mar 31, 2026

Uh oh!

mpbagot commented Mar 31, 2026

Uh oh!

tcharding left a comment

Choose a reason for hiding this comment

Uh oh!

apoelstra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

mpbagot commented Feb 4, 2026 •

edited

Loading

tcharding commented Feb 4, 2026 •

edited

Loading

apoelstra commented Feb 8, 2026 •

edited

Loading

mpbagot commented Mar 5, 2026 •

edited

Loading