Emit a warning if the doctest main function will not be run
#140527
+131
−17
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-review
GuillaumeGomez
force-pushed
the
doctest-main-fn
branch
from
cd960d9 to
c548a27
Compare
notriddle
reviewed
Apr 30, 2025
| @@ -303,7 +303,9 @@ impl<'a, I: Iterator<Item = Event<'a>>> Iterator for CodeBlocks<'_, 'a, I> { | |||
| attrs: vec![], | |||
| args_file: PathBuf::new(), | |||
| }; | |||
| let doctest = doctest::DocTestBuilder::new(&test, krate, edition, false, None, None); | |||
| let doctest = doctest::DocTestBuilder::new( | |||
| &test, krate, edition, false, None, None, None, DUMMY_SP, | |||
There was a problem hiding this comment.
This needs a builder pattern, badly.
There was a problem hiding this comment.
Yep. Do you want it in this PR or in a follow-up?
There was a problem hiding this comment.
Either way is fine, as long as it actually gets done.
notriddle
reviewed
May 1, 2025
| let line = self.get_base_line() + rel_line.offset(); | ||
| let base_line = self.get_base_line(); | ||
| let line = base_line + rel_line.offset(); | ||
| let count = AtomicUsize::new(base_line); |
There was a problem hiding this comment.
Why is an atomic needed here? This code isn't multithreaded.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #140310.
I think we could try to go much further like adding a "link" (ie UI annotations) on the
mainfunction in the doctest. However that will require some more computation, not sure if it's worth it or not. Can still be done in a follow-up if we want it.For now, this PR does two things:
DiagCtxtto the doctest parser to emit the warning.Spanto where the doctest is starting (I hope the way I did it isn't too bad either...).cc @fmease
r? @notriddle