Add todo!() macro #56348

matklad · 2018-11-29T19:20:55Z

The primary use-case of todo!() macro is to be a much easier to type
alternative to unimplemented!() macro.

EDIT: hide unpopular proposal about re-purposing unimplemented

However, instead of just replacing `unimplemented!()`, it gives it a more nuanced meaning: a thing which is intentionally left unimplemented and which should not be called at runtime. Usually, you'd like to prevent such cases statically, but sometimes you, for example, have to implement a trait only some methods of which are applicable. There are examples in the wild of code doing this thing, and in this case, the current message of `unimplemented`, "not *yet* implemented" is slightly misleading.

With the addition of TODO, you have three nuanced choices for a
!-returning macro (in addition to a good-old panic we all love):

todo!()
unreachable!()
unimplemented!()

Here's a rough guideline what each one means:

todo: use it during development, as a "hole" or placeholder. It
might be a good idea to add a pre-commit hook which checks that
todo is not accidentally committed.
unreachable!(): use it when your code can statically guarantee
that some situation can not happen. If you use a library and hit
unreachable!() in the library's code, it's definitely a bug in the
library. It's OK to have unreachable!() in the code base,
although, if possible, it's better to replace it with
compiler-verified exhaustive checks.
unimplemented!(): use it when the type checker forces you to
handle some situation, but there's a contract that a callee must not
actually call the code. If you use a library and hit
unimplemented!(), it's probably a bug in your code, though
it could be a bug in the library (or library docs) as well. It is
ok-ish to see an unimplemented!() in real code, but it usually
signifies a clunky, eyebrow-rising API.

rust-highfive · 2018-11-29T19:21:05Z

r? @dtolnay

(rust_highfive has picked a reviewer for you, use r? to override)

matklad · 2018-11-29T19:21:09Z

internals discussion: https://internals.rust-lang.org/t/micro-pre-rfc-todo-macro/8925

matklad · 2018-11-29T19:24:24Z

Example where unimplemented means: "no reasonable implementation exists" as opposed to "will write an impl really soon":

rust/src/bootstrap/builder.rs

Lines 89 to 95 in 3dde9e1

    
               fn make_run(_run: RunConfig) { 
        
                   // It is reasonable to not have an implementation of make_run for rules 
        
                   // who do not want to get called from the root context. This means that 
        
                   // they are likely dependencies (e.g., sysroot creation) or similar, and 
        
                   // as such calling them from ./x.py isn't logical. 
        
                   unimplemented!() 
        
               }

cuviper · 2018-11-29T19:34:07Z

I like that this distinguishes intent -- todo!() means you'll get around to it later, whereas unimplemented!() means you'll leave it that way. To that end, the documentation for the latter should also be updated, and probably remove the "yet" from its panic message. Maybe also audit rustc's own uses to see which ones might better be todo!().

matklad · 2018-11-29T20:00:35Z

To that end, the documentation for the latter should also be updated, and probably remove the "yet" from its panic message.

Oups, was meant to add that to commit message. That obviously should be done, but only when/if we stabilize the todo! macro.

kennytm · 2018-11-29T20:30:13Z

I wonder if we should name it TODO!() so it could match the // TODO comments.

Prior arts for breaking the naming convention: Go, Kotlin

Centril · 2018-11-29T21:15:53Z

src/libcore/macros.rs

@@ -553,6 +553,64 @@ macro_rules! unimplemented {
    ($($arg:tt)+) => (panic!("not yet implemented: {}", format_args!($($arg)*)));
 }

+/// A standardized placeholder for marking unfinished code.


The unimplemented macro also has this sentence in the docs so you probably want to re-frame that documentation.

We can do that only when we stabilize todo: #56348 (comment)

aren't macros insta-stable?

There are other unstable macros already, like concat_idents!

@steveklabnik I believe not necessarily. The dbg!() macro went through nightly first, and is only now landing on stable.

scottmcm · 2018-11-30T00:03:46Z

I think relegating panic! to a parenthetical is being misleading. I don't think "there's a contract that a callee must not actually call the code" is unimplemented!(), I think it's a place for a panic! (or assert!) with a message actually explaining this fact. And then there's no need for another name for the same thing.

Prior art:

matklad · 2018-11-30T06:39:47Z

@scottmcm not sure I entirely understand what you are trying to say, so let me to elaborate on my goals here:

The single reason is that I want to shorten unimplemented to todo. Ideally, I wish it was named todo from the start.

However, we already have unimplemented, so we need to do something about it.

One choice is to deprecate, another choice is to repurpose. The PR description went with repurposing, because, after grepping rust-lang/rust, I’ve noticed that it already is being used in the C#’s not supported sense.

joshtriplett · 2018-11-30T10:15:58Z

I'm entirely in favor of this.

bors · 2018-12-02T16:17:49Z

☔ The latest upstream changes (presumably #56275) made this pull request unmergeable. Please resolve the merge conflicts.

Thomasdezeeuw · 2018-12-04T14:16:42Z

src/libcore/macros.rs

+///
+/// # Panics
+///
+/// This will always [panic!](macro.panic.html)


Shouldn't this say "panics"?

Thomasdezeeuw · 2018-12-04T14:17:34Z

src/libcore/macros.rs

+///
+/// We want to implement `Foo` on one of our types, but we also want to work on
+/// just `bar()` first. In order for our code to compile, we need to implement
+/// `baz()`, so we can use `unimplemented!`:


This says to use the unimplemented macro.

matklad · 2018-12-05T08:31:36Z

Renamed to TODO!

Centril · 2018-12-05T08:33:52Z

I'm not sure TODO! is better... there's no precedent for that in Rust and it seems harder to type in general?

Ralith · 2018-12-05T08:46:28Z

An all-caps macro strikes me as a large enough digression from Rust style conventions to be immediately confusing to a reader.

matklad · 2018-12-05T09:05:59Z

Don't have an opinion about which capitalization is better, will leave that to libs team to decide. Both Kotlin & Go decided to change naming convention for this macro though.

mark-i-m · 2018-12-05T14:50:24Z

There is some precedent: TODO in comments tends to be all caps.

Ralith · 2018-12-05T19:39:18Z

TODO in comments is not a macro invocation.

mark-i-m · 2018-12-05T22:49:41Z

I know, but it is related to TODOs...

dtolnay · 2018-12-16T22:59:13Z

I am sympathetic to unimplemented!() being complicated to type. At this point I have muscle memory for it but it took a while.

At the same time I agree with @scottmcm in #56348 (comment) and would prefer not to see unimplemented!() become what you wrote in the PR description: "there's a contract that a callee must not actually call the code." That would be panic or assert. The panic or assert message should explain the contract. The point is that the caller broke the contract, not that the code isn't implemented.

I am pretty ambivalent but probably leaning against making this change -- meaning I would continue to use unimplemented!() over todo!() if both were available and meant the same thing. I'll reassign to Boats because I am interested in your take and I see your 👍.

Regarding capitalization, I would advise against weighing Go as precedent. They were hamstrung by requiring a capital first letter on public functions, at which point ToDo is just distracting and Todo is Spanish. In Rust I would want the name to be in lowercase.

r? @withoutboats

matklad · 2019-03-18T15:48:13Z

rebased and created&filled the tracking issue

withoutboats · 2019-03-18T16:04:27Z

@matklad This looks good and I'd like to request just one more change: add a sentence to the docs of both todo and unimplemented documenting that each has the same behavior as the other, so no one wastes time scrutinizing them looking for a difference.

matklad · 2019-03-18T16:09:10Z

I think we should only change unimplemented docs when/if we actually stabilize todo!. Added the note to todo

withoutboats · 2019-03-18T16:10:35Z

that makes sense to me

@bors r+

bors · 2019-03-18T16:10:37Z

📌 Commit c11b37af0ec9d0b5dbe5bed7361740e23cfcd4b4 has been approved by withoutboats

rust-highfive · 2019-03-18T16:26:29Z

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

travis_time:end:14e04bbf:start=1552925416202863259,finish=1552925417399815597,duration=1196952338
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
Setting environment variables from .travis.yml
---
[00:04:40] tidy error: /checkout/src/libcore/macros.rs:565: trailing whitespace
[00:04:42] some tidy checks failed
[00:04:42] 
[00:04:42] 
[00:04:42] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:04:42] 
[00:04:42] 
[00:04:42] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:04:42] Build completed unsuccessfully in 0:00:48
[00:04:42] Build completed unsuccessfully in 0:00:48
[00:04:42] make: *** [tidy] Error 1
[00:04:42] Makefile:67: recipe for target 'tidy' failed
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:1e13e730
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Mon Mar 18 16:15:10 UTC 2019
---
travis_time:end:10f654ac:start=1552925711948310320,finish=1552925711953568162,duration=5257842
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:068dd111
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:22a15fe6
travis_time:start:22a15fe6
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:175b65a0
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

The use-case of `todo!()` macro is to be a much easier to type alternative to `unimplemented!()` macro.

withoutboats · 2019-03-18T18:31:23Z

@bors r+

bors · 2019-03-18T18:31:24Z

📌 Commit 9d408d9 has been approved by withoutboats

Centril · 2019-03-18T21:05:12Z

@bors rollup

Add todo!() macro The primary use-case of `todo!()` macro is to be a much easier to type alternative to `unimplemented!()` macro. EDIT: hide unpopular proposal about re-purposing unimplemented <details> However, instead of just replacing `unimplemented!()`, it gives it a more nuanced meaning: a thing which is intentionally left unimplemented and which should not be called at runtime. Usually, you'd like to prevent such cases statically, but sometimes you, for example, have to implement a trait only some methods of which are applicable. There are examples in the wild of code doing this thing, and in this case, the current message of `unimplemented`, "not *yet* implemented" is slightly misleading. With the addition of TODO, you have three nuanced choices for a `!`-returning macro (in addition to a good-old panic we all love): * todo!() * unreachable!() * unimplemented!() Here's a rough guideline what each one means: - `todo`: use it during development, as a "hole" or placeholder. It might be a good idea to add a pre-commit hook which checks that `todo` is not accidentally committed. - `unreachable!()`: use it when your code can statically guarantee that some situation can not happen. If you use a library and hit `unreachable!()` in the library's code, it's definitely a bug in the library. It's OK to have `unreachable!()` in the code base, although, if possible, it's better to replace it with compiler-verified exhaustive checks. - `unimplemented!()`: use it when the type checker forces you to handle some situation, but there's a contract that a callee must not actually call the code. If you use a library and hit `unimplemented!()`, it's probably a bug in your code, though it *could* be a bug in the library (or library docs) as well. It is ok-ish to see an `unimplemented!()` in real code, but it usually signifies a clunky, eyebrow-rising API. </details>

@ghost

Rollup of 11 pull requests Successful merges: - #56348 (Add todo!() macro) - #57729 (extra testing of how NLL handles wildcard type `_`) - #57847 (dbg!() without parameters) - #58778 (Implement ExactSizeIterator for ToLowercase and ToUppercase) - #58812 (Clarify distinction between floor() and trunc()) - #58939 (Fix a tiny error in documentation of std::pin.) - #59116 (Be more discerning on when to attempt suggesting a comma in a macro invocation) - #59252 (add self to mailmap) - #59275 (Replaced self-reflective explicit types with clearer `Self` or `Self::…` in stdlib docs) - #59280 (Stabilize refcell_map_split feature) - #59290 (Run branch cleanup after copy prop) Failed merges: r? @ghost

jonhoo · 2019-03-27T16:30:11Z

Just going to leave a link to #39930 and rust-lang/rfcs#1911 here so that we maintain the breadcrumbs for people digging later on.

misha-krainik · 2019-03-28T07:26:56Z

I cannot understand why people think that “todo” is “panic”. Historically //TODO is a note that does not violate your program. But this discussion said todo! have to break your program.

#[macro_export]
#[unstable(feature = "todo_macro", issue = "59277")]
macro_rules! todo {
    () => (panic!("not yet implemented"));
    ($($arg:tt)+) => (panic!("not yet implemented: {}", format_args!($($arg)*)));
}

We have a lot of important RFC. Let's leave the unimplemented!() alone.

CAD97 · 2019-03-28T11:18:46Z

The convention that this is making easier is not unprecedented.

A lot of development flows say that // TODO should not be committed, and should be a local development marker; the committed version is // FIXME.

In addition, this makes no change to // TODO, and you can continue using it per whatever convention you want.

The purpose of this is more to provide todo!() as an alternative to unimplemented!() such that you can have a similar convention as is used between // TODO and // FIXME. (The proposed distinction is that todo!() is for short lived and unimplemented!() is for longer lived, but this does not require that of you.)

misha-krainik · 2019-03-28T11:45:37Z

The convention that this is making easier is not unprecedented.

easy shouldn't be confusing

A lot of development flows say that // TODO should not be committed, and should be a local development marker; the committed version is // FIXME.

A lot of people say that earth is flat. But https://en.wikipedia.org/wiki/Comment_(computer_programming) says "TODO – something to be done."

In addition, this makes no change to // TODO, and you can continue using it per whatever convention you want.

todo! mustn't raise an error.

The purpose of this is more to provide todo!() as an alternative to unimplemented!() such that you can have a similar convention as is used between // TODO and // FIXME. (The proposed distinction is that todo!() is for short lived and unimpleunimplementedmented!() is for longer lived, but this does not require that of you.)

If you want to see a shorter version than unimplemented!() without losing clarity, I offer unimpl!() or just keep unimplemented!()

rust-highfive assigned dtolnay Nov 29, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 29, 2018

This comment has been minimized.

Sign in to view

matklad force-pushed the todo-macro branch from 269d0a8 to 7ffa157 Compare November 29, 2018 20:01

Centril reviewed Nov 29, 2018

View reviewed changes

kennytm added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Nov 30, 2018

Thomasdezeeuw reviewed Dec 4, 2018

View reviewed changes

matklad force-pushed the todo-macro branch from 73adb97 to 44d4ddc Compare December 5, 2018 08:30

This comment has been minimized.

Sign in to view

rust-highfive assigned withoutboats and unassigned dtolnay Dec 16, 2018

matklad force-pushed the todo-macro branch from fb0aa26 to 9482cb7 Compare March 18, 2019 15:47

matklad force-pushed the todo-macro branch from 9482cb7 to c11b37a Compare March 18, 2019 16:08

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 18, 2019

Add todo!() macro

9d408d9

The use-case of `todo!()` macro is to be a much easier to type alternative to `unimplemented!()` macro.

matklad force-pushed the todo-macro branch from c11b37a to 9d408d9 Compare March 18, 2019 16:27

Centril mentioned this pull request Mar 19, 2019

Rollup of 12 pull requests #59289

Closed

Centril mentioned this pull request Mar 19, 2019

Rollup of 11 pull requests #59293

Merged

bors merged commit 9d408d9 into rust-lang:master Mar 19, 2019

sftim mentioned this pull request Mar 28, 2019

Fix typo in todo()! comment #59491

Closed

ghost mentioned this pull request Jul 2, 2019

Stabilize todo macro #61879

Merged

matklad deleted the todo-macro branch July 9, 2019 12:33

Centril mentioned this pull request Dec 20, 2019

Differentiate todo! and unimplemented! #67445

Merged

Add todo!() macro #56348

Add todo!() macro #56348

Conversation

matklad commented Nov 29, 2018 • edited Loading

rust-highfive commented Nov 29, 2018

matklad commented Nov 29, 2018

matklad commented Nov 29, 2018

cuviper commented Nov 29, 2018

This comment has been minimized.

matklad commented Nov 29, 2018

kennytm commented Nov 29, 2018

Centril Nov 29, 2018

Choose a reason for hiding this comment

matklad Nov 29, 2018

Choose a reason for hiding this comment

steveklabnik Nov 29, 2018

Choose a reason for hiding this comment

cuviper Nov 29, 2018

Choose a reason for hiding this comment

yoshuawuyts Nov 29, 2018

Choose a reason for hiding this comment

scottmcm commented Nov 30, 2018

matklad commented Nov 30, 2018

joshtriplett commented Nov 30, 2018

bors commented Dec 2, 2018

Thomasdezeeuw Dec 4, 2018

Choose a reason for hiding this comment

Thomasdezeeuw Dec 4, 2018

Choose a reason for hiding this comment

matklad commented Dec 5, 2018

Centril commented Dec 5, 2018

This comment has been minimized.

Ralith commented Dec 5, 2018

matklad commented Dec 5, 2018

mark-i-m commented Dec 5, 2018

Ralith commented Dec 5, 2018

mark-i-m commented Dec 5, 2018

This comment has been minimized.

dtolnay commented Dec 16, 2018

matklad commented Mar 18, 2019

withoutboats commented Mar 18, 2019

matklad commented Mar 18, 2019

withoutboats commented Mar 18, 2019

bors commented Mar 18, 2019

rust-highfive commented Mar 18, 2019

withoutboats commented Mar 18, 2019

bors commented Mar 18, 2019

Centril commented Mar 18, 2019

jonhoo commented Mar 27, 2019 • edited Loading

misha-krainik commented Mar 28, 2019 • edited Loading

CAD97 commented Mar 28, 2019 • edited Loading

misha-krainik commented Mar 28, 2019 • edited Loading

matklad commented Nov 29, 2018 •

edited

Loading

jonhoo commented Mar 27, 2019 •

edited

Loading

misha-krainik commented Mar 28, 2019 •

edited

Loading

CAD97 commented Mar 28, 2019 •

edited

Loading

misha-krainik commented Mar 28, 2019 •

edited

Loading