Rfcs: Compile-time checked version of `unimplemented!()`

Created on 19 Feb 2017  路  28Comments  路  Source: rust-lang/rfcs

We all know and love the unimplemented!() macro for keeping track of things we haven't yet gotten around to. However, it is only checked at runtime if a certain code path is encountered. This is fine in many cases, but when writing new code from scratch (especially when porting), refactoring large amounts of code, or building comprehensive new features, you often have segments of code that you haven't implemented yet, but you know you'll have to.

At least for me, it is common to write a chunk of code, but leaving out some annoying edge cases, or maybe logic (like generating a unique identifier) that I want to deal with alter. I know that I will have to complete that block of code before the program does something useful, but I want to do it later. This can be either because I want to complete the main logical flow of the code first, because I haven't figured out how to do that part yet, or simply because I want to see if the rest of my code compiles correctly.

It would be extremely useful to have a variant of unimplemented!() that would generate a compile-time error if present in my code. Something like incomplete!(). However, it should have some properties that I believe mean it can't be implemented without some help from the compiler:

  1. It should be usable as an expression of any type (similar to ! for unimplemented!())
  2. It should only error if the program compiles correctly. Or rather, it should not prevent any other errors from being printed. In essence, all passes of the compiler (including the borrow checker) should run despite the presence of incomplete!().
  3. It should not emit warnings about dead code or unused variables following it (which using unimplemented!() does).

The closest I've been able to come up with on my own is the following

macro_rules! incomplete {
    () => {{
        #[deny(non_snake_case)]
        let _mustBeImplemented = ();
        loop {}
    }}
}

This will only error out after all compiler passes (2), and since it returns !, it is sort of usable in place of any expression (1). However, it does not fit (3), because the infinite loop makes the compiler believe that all code following it is dead. Furthermore, the error it produces is obviously not appropriate for what the macro's intent is.

NOTE: originally posted as https://github.com/rust-lang/rust/issues/39930, but moved here following @sanmai-NL's suggestion.

T-lang T-libs

Most helpful comment

FWIW, I made a crate: https://github.com/jonhoo/incomplete.
It's still not as good as a compiler-supported variant, but it's a start.

All 28 comments

I feel like there might be a misunderstanding of having type !. This marks something that diverges, so execution is statically known not to proceed beyond this point. A function -> ! doesn't return this type, rather it doesn't return at all. This is how the compiler knows that all of the following code is dead.

Your existing incomplete!() might work better for you if you truly make it return anything, like:

macro_rules! incomplete {
    () => {{
        #[deny(non_snake_case)]
        let _mustBeImplemented = ();
        unsafe { ::std::mem::uninitialized() }
    }}
}

Another idea instead of the non_snake_case lint is to push it even later into a link error. e.g. You could declare and call an FFI function you_forgot_to_implement_something(). This will let your code fully pass things like cargo check, but the drawback is that the link error won't be localized to its use.

@cuviper Ah, no, I'm aware of the semantic meaning of -> !, I just didn't think of the

unsafe { ::std::mem::uninitialized() }

trick, and used ! as a substitute (even though I know it technically means something completely different). That's neat. Moving it all the way to linking would be nice, but I think keeping localization information is actually sufficiently important that it wouldn't be a viable option.

I realized that has a type-inference problem if used in statement context, incomplete!(); without any assignment. Here's what I came up with, a little less pretty:

macro_rules! incomplete {
    ($ty:ty) => {{
        #[forbid(non_snake_case)]
        let _mustBeImplemented = ();
        unsafe { ::std::mem::uninitialized::<$ty>() }
    }};
    () => { incomplete!(_) }
}

fn main(){
    let _x: () = incomplete!(); // inferred type
    incomplete!(()); // explicit type `()`
}

Also, I think forbid is needed instead of deny, so the error doesn't get squashed if compiled as a dependency of some other crate.

Oh, yeah, good catch. forbid vs deny I'm slightly less worried about, because it's unlikely you would push a crate in a state where it contains an incomplete!(), but you're right that this could happen if, say, you had a local dependent crate.

I've also been thinking about whether there's a better way to have the displayed error be more relevant (obviously, having this be a "true" compiler error would be better). I was hoping to use forbid(unknown_lints), but unfortunately that doesn't seem to work correctly. unreachable-code also seems somewhat appropriate, and produces the cleanest message with this:

macro_rules! incomplete {
    ($ty:ty) => {{
        if false {
            loop{};
            #[deny(unreachable_code)]
            () // must be implemented
        }
        unsafe { ::std::mem::uninitialized::<$ty>() }
    }};
    () => { incomplete!(_) }
}

fn main() {
    let _x: () = incomplete!(); // inferred type
    incomplete!(()); // explicit type `()`
}

namely

warning: unreachable expression, #[warn(unreachable_code)] on by default
  --> x.rs:6:13
   |
6  |             () // must be implemented
   |             ^^
...
14 |     let _x: () = incomplete!(); // inferred type
   |                  ------------- in this macro invocation

warning: unreachable expression, #[warn(unreachable_code)] on by default
  --> x.rs:6:13
   |
6  |             () // must be implemented
   |             ^^
...
15 |     incomplete!(()); // explicit type `()`
   |     ---------------- in this macro invocation

though for some reason the warning doesn't seem to be turned into an error (neither does using dead_code instead)...

Hmm, the lint controls don't seem to work on statements -- only on items? Here's a way:

        #[forbid(unused_must_use)]
        fn _incomplete() { Err::<(), ()>(()); }

But it gets an extra note about where the lint level was set:

error: unused result which must be used
  --> <anon>:4:28
   |
4  |         fn _incomplete() { Err::<(), ()>(()); }
   |                            ^^^^^^^^^^^^^^^^^^
...
11 |     let _x: () = incomplete!(); // inferred type
   |                  ------------- in this macro invocation
   |
note: lint level defined here
  --> <anon>:3:18
   |
3  |         #[forbid(unused_must_use)]
   |                  ^^^^^^^^^^^^^^^
...
11 |     let _x: () = incomplete!(); // inferred type
   |                  ------------- in this macro invocation

Maybe there's a lint to trigger that's default deny/forbid.

Good enough?

        fn _incomplete() { '_: loop {} }
error: invalid label name `'_`, #[deny(lifetime_underscore)] on by default
  --> <anon>:3:28
   |
3  |         fn _incomplete() { '_: loop {} }
   |                            ^^
...
10 |     let _x: () = incomplete!(); // inferred type
   |                  ------------- in this macro invocation
   |
   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
   = note: for more information, see issue #36892 <https://github.com/rust-lang/rust/issues/36892>

Still hacky to abuse lints, of course -- not as good as it could be as a real compiler macro.

That last one is pretty good. I liked the unreachable code one primarily because the message is very clear, highlighting the () // must be implemented, but being able to get rid of the note about the linting is nice. Compiler support would be best though, you're right. Hence this issue ;)

FWIW, I made a crate: https://github.com/jonhoo/incomplete.
It's still not as good as a compiler-supported variant, but it's a start.

Actually, come to think of it, maybe all I really want is a lint like missing_docs, so that I can choose

#![warn(unimplemented)]

or

#![deny(unimplemented)]

I noticed that the compile_error! macro in https://github.com/rust-lang/rfcs/pull/1695 just landed. Unfortunately, that terminates compilation too early, and so doesn't actually fit well for this issue either..

fn my_fn() -> usize {
    unimplemented()!;
    0
}

produces:

warning: unreachable expression

Does this suffice to point out left-over unimplemented! invocations at compile time?

@sanmai-NL You mean unimplemented!(), right? I don't think that solution is adequate for what I had in mind, for a couple of reasons:

  • it's not an error if there are no other errors
  • it'll give me lots of additional errors for any statements below the unimplemented!()

I believe this is either unimplemented!() itself when executed in CTFE, which is tracked in https://github.com/rust-lang/rust/issues/51999 or it is compile_error!. So I'll close in favor of those.

@Centril I don't think that's quite accurate. compile_error is close, but it fails too early (https://github.com/rust-lang/rfcs/issues/1911#issuecomment-290578068). Specifically, the request here is to have a device that will only prevent compilation if there are no other compilation errors. The idea is that I want something that reminds me to fill in a section later, but first fix all the other compilation errors. This is very handy when refactoring a somewhat large code-base. For example, consider the case where I change a function to return a Result where it wasn't previously. In of the callers, handling the Err case will be more complicated, so I mark it as incomplete! (or whatever we want to call it). I can then continue going through all the other parts of the code that need to be changed, and only when everything else has been fixed does the compiler remind me to go back and remind me that I still need to fill in that case.

@jonhoo so... the only difference between incomplete!() and const { panic!(...) } is that the latter will unconditionally cause a compilation error while the former will only do it if there are zero errors otherwise? That distinction feels somewhat niche...? (reopening anyways... but I'd recommend fleshing it out on IRLO to get some more eyes on it...)

Isn't the goal here #![deny(unimplemented)] more than incomplete!() now? You could kinda do that with grep -r unimplemented . right?

@burdges No, there are reasonable cases where I want to run my code when it still has unimplemented! in it. incomplete! on the other hand are things I _know_ I have to do before running, but want to defer writing while I handle other (more fundamental) errors. In part because those errors may cause me to further change the design such that the incomplete! block would also change.

@Centril yes, it's perhaps a little niche, but it would be super handy when doing bigger refactoring. If I'm not mistaken, const { panic!() } will fail to compile before borrow checking?

const { panic!() } will fail to compile before borrow checking?

cc @oli-obk re. this question

No, after checking. all const eval happens after borrow checking and Mir computation for that item. other items may be in different states though.

On topic of this feature request. this is doable in clippy and feels niche enough to me to not be in the compiler

@oli-obk If this were in clippy it wouldn't actually be useful, because I don't think most developers regularly run clippy. You do however regularly run the compiler. It's an interesting point that const { panic!() } runs at the very end. it might be that that would actually be how to implement this macro!

(note: the const { .. } bit is fictive and not actually proposed by any RFC, but you can use a const item for that instead I think...)

It only runs in the end for the containing item. you might error out immediately on that and never see your other errors.

I don't see how being in clippy would make this less useful. Rls will show it, and if your workflow uses this kind of check, then your workflow will just contain clippy from now on 馃槂

That said. This kind of scheme does not fit into queries very well. Any improvements for queries will likely end up reporting these errors earlier again. I do think the grep method is the most reliable one, or doing it in clippy, which doesn't need to uphold the query system

As a hack you could create this macro on nightly yourself by using broken inline assembly.

Rls will only show it if clippy is enabled :) I'm working on a 50k LOC codebase that wasn't originally written with clippy enabled, so it's not actually feasible to have clippy running all the time (at least not yet) because it generates too much noise. You might be right that for most projects a clippy lint is the way to go, but what would the macro then ever consist of? It would have to also be possible to compile outside of clippy?

I quite like the idea of grep -r unimplemented . coupled with reminder text, so

unimplemented!() // TODO: ...

which permits seeing reminders for only a fragment of the tree too.

I'm working on a 50k LOC codebase that wasn't originally written with clippy enabled, so it's not actually feasible to have clippy running all the time (at least not yet) because it generates too much noise.

You can run it all the time and just enable the lints you feel confident about. E.g. leave all lints disabled and just enable the one about incomplete!()

You might be right that for most projects a clippy lint is the way to go, but what would the macro then ever consist of?

It would just forward to unimplemented and the lint would check for the name.

It would have to also be possible to compile outside of clippy?

yes indeed. Without clippy it would just be the same as writing unimplemented!

Even easier would be to simply find all unimplemented!("incomplete"), which would work on regular code without an additional macro.

Could this be implemented by linking to an external function that doesn't exist so the failure is pushed back until an exe is produced? See the [panic-never] crate

Was this page helpful?
0 / 5 - 0 ratings

Related issues

marinintim picture marinintim  路  3Comments

burdges picture burdges  路  3Comments

steveklabnik picture steveklabnik  路  4Comments

silversolver1 picture silversolver1  路  3Comments

3442853561 picture 3442853561  路  4Comments