Pull Request Test Coverage Report for Build 13977619809

Details

• 161 of 189 (85.19%) changed or added relevant lines in 13 files are covered.
• 4 unchanged lines in 2 files lost coverage.
• Overall coverage increased (+0.06%) to 83.173%

Totals
Change from base Build 13977478725:	0.06%
Covered Lines:	22030
Relevant Lines:	26487

💛 - Coveralls

oooo. Concept ACK.

Forgot to say the warning is intentional for now - I think I'll fix that bug first before merging this. (Or anyone else can tonight, I don't think I'll do it today.)

Nice. Here are two more tests I wrote while understanding the extension trait.

    #[test]
    fn five_to_two() {
        let slice = [1, 2, 3, 4, 5];
        let (left, right) = slice.as_chunks::<2>();
        assert_eq!(left, &[[1, 2], [3, 4]]);
        assert_eq!(right, &[5]);

        let (first, rest) = slice.split_first_chunk::<2>().unwrap();
        assert_eq!(first, &[1, 2]);
        assert_eq!(rest, &[3, 4, 5]);

        let (rest, last) = slice.split_last_chunk::<2>().unwrap();
        assert_eq!(rest, &[1, 2, 3]);
        assert_eq!(last, &[4, 5]);
    }

    #[test]
    fn get_array() {
        let slice = [1, 2, 3, 4, 5];

        let chunk = slice.get_array::<2>(0).unwrap();
        assert_eq!(chunk, &[1, 2]);

        // offset is an index of `T` not an index of chunks.
        let chunk = slice.get_array::<2>(1).unwrap();
        assert_eq!(chunk, &[2, 3]);

        let chunk = slice.get_array::<2>(3).unwrap();
        assert_eq!(chunk, &[4, 5]);

        assert!(slice.get_array::<2>(4).is_none());
    }

At first I thought offset was a chunk index not an index of Ts.

Nice!

Pity rust-analyzer does not report error:

@jirijakes Yeah, I've seen someone mentioned that postmonomorphization errors only show up with cargo build, not cargo check and presumable RA uses check (very reasonable) and postmono error is the only feasible option right now (that is still compile time).

@Kixunil, you're right. There is a related issue: rust-lang/rust#99682.

So, cargo check will not report these Hacky errors, only cargo build will. Rust-analyzer can be configured to use build if needed but I don't think it makes much sense.

@jirijakes FTR I've seen a proposal to make check also evaluate these consts which would be much cleaner solution but something that requires work on the compiler side.

@apoelstra do you want me to redo my PR or do it in a followup to save Tobin's ACK?

@Kixunil can you force-push the PR? I think it'll be OK -- I should be able to ACK it and Tobin should be online before I sign off so I can start merging it.

I need to run now (some unexpected stuff came up) but I think I'll be able to do it by the end of the day.

Sorry, this needs another rebase. Process failure on my part -- I closed this tab because I was waiting for Tobin's ACK, and then I merged #4242 because it didn't conflict with any open tabs.

Crap, these ridiculous conflicts are driving me crazy. I was thinking for a while of having a version control system that would store the entire source as a tree (AST + files + directories), not text and make formatting completely machine-generated. It'd make this kind of reformatting impossible (but still settable client-side; obviously, rustfmt would need some more tweaks) and many changes would become trivially mergable - e.g. use items added to the same line.

Out of curiosity, does the idea sound appealing to you? Of course I'm not going to implement this any time soon, at least not in usable form, I'm just wondering if I'm the only one bothered by it. :)

Out of curiosity, does the idea sound appealing to you?

Yes, though I suspect you can get 95% of the way there with custom git merge strategies. And the last 5% probably is not worth the switching cost of learning a new VCS.

In Bitcoin Core there is a notion of a "scripted diff" where if your diff is expressible in shell (e.g. cargo +nightly fmt and somehow we decide on compiler pins) then you can put that shell script into your commit message and CI will verify that the commit is exactly the result of the script. This dramatically simplifies review for stuff that is basically a sed -i across the codebase, but it could also be used as a git merge strategy, where if git sees a conflict between a scripted diff and a normal one, it attempts to apply the normal one and then re-run the script.

Relatedly, it would also be nice to have a git diff driver that collapsed out scripted stuff so you don't have to wade through a bajillion renames to find the one real change..

I suspect you can get 95% of the way there with custom git merge strategies.

Some amount yes but I did a bit of digging into it and I haven't found any documentation on how to implement them (just merge drivers) and the one merge driver I made for merging use is pretty cool but for some reason git refuses to use it with cherry-picks. Also pijul as some cool ideas that I think would work even better with trees.

If we could just use non-nightly rustfmt it might be also worth applying it as a filter. Scripted diffs are nice but I'd love to see some tool that can do stuff like "rename this Rust variable".

Anyway, this all is just entirely theoretical.

Needs rebase after #4248.

Damn, this is exactly the kind of merge conflict that should be resolvable in a smarter version control... Anyway, rebased.

Yeah. Super frustrating that we gotta manually re-apply renames like this.

Queued for re-ACK and I'll also queue for merge since the range-diff from what Tobin ACKed is nearly trivial. (Though he may show up and sneak a new ACK in before my scripts are done running :)).