-
Notifications
You must be signed in to change notification settings - Fork 13.8k
improve c-variadic error reporting #146165
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This comment has been minimized.
This comment has been minimized.
9eaecd8
to
8d11719
Compare
@bors r+ rollup |
☀️ Test successful - checks-actions |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing beeb8e3 (parent) -> 68baa87 (this PR) Test differencesShow 7 test diffsStage 1
Stage 2
Additionally, 3 doctest diffs were found. These are ignored, as they are noisy. Job group index
Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 68baa87ba6f03f8b6af2a368690161f1601e4040 --output-dir test-dashboard And then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
Finished benchmarking commit (68baa87): comparison URL. Overall result: ✅ improvements - no action needed@rustbot label: -perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 3.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary -2.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 466.876s -> 468.245s (0.29%) |
…3, r=workingjubilee Improve C-variadic error messages: part 2 tracking issue: rust-lang#44930 a reimplementation of rust-lang#143546 that builds on rust-lang#146165. This PR - disallows coroutines (e.g. `async fn`) from having a `...` argument - disallows associated functions (both in traits and standard impl blocks) from having a `...` argument - splits up a generic "ill-formed C-variadic function" into specific errors about using an incorrect ABI, not specifying an ABI, or missing the unsafe keyword C-variadic coroutines probably don't make sense? C-variadic functions are for FFI purposes, combining that with async functions seems weird. For associated functions, we're just cutting scope. It's probably fine, but it's probably better to explicitly allow it. So for now, at least give a more targeted error message. Made to be reviewed commit-by-commit. cc `@workingjubilee` r? compiler
…3, r=workingjubilee Improve C-variadic error messages: part 2 tracking issue: rust-lang#44930 a reimplementation of rust-lang#143546 that builds on rust-lang#146165. This PR - disallows coroutines (e.g. `async fn`) from having a `...` argument - disallows associated functions (both in traits and standard impl blocks) from having a `...` argument - splits up a generic "ill-formed C-variadic function" into specific errors about using an incorrect ABI, not specifying an ABI, or missing the unsafe keyword C-variadic coroutines probably don't make sense? C-variadic functions are for FFI purposes, combining that with async functions seems weird. For associated functions, we're just cutting scope. It's probably fine, but it's probably better to explicitly allow it. So for now, at least give a more targeted error message. Made to be reviewed commit-by-commit. cc ``@workingjubilee`` r? compiler
…3, r=workingjubilee Improve C-variadic error messages: part 2 tracking issue: rust-lang#44930 a reimplementation of rust-lang#143546 that builds on rust-lang#146165. This PR - disallows coroutines (e.g. `async fn`) from having a `...` argument - disallows associated functions (both in traits and standard impl blocks) from having a `...` argument - splits up a generic "ill-formed C-variadic function" into specific errors about using an incorrect ABI, not specifying an ABI, or missing the unsafe keyword C-variadic coroutines probably don't make sense? C-variadic functions are for FFI purposes, combining that with async functions seems weird. For associated functions, we're just cutting scope. It's probably fine, but it's probably better to explicitly allow it. So for now, at least give a more targeted error message. Made to be reviewed commit-by-commit. cc `@workingjubilee` r? compiler
Rollup merge of #146342 - folkertdev:c-variadic-errors-take-3, r=workingjubilee Improve C-variadic error messages: part 2 tracking issue: #44930 a reimplementation of #143546 that builds on #146165. This PR - disallows coroutines (e.g. `async fn`) from having a `...` argument - disallows associated functions (both in traits and standard impl blocks) from having a `...` argument - splits up a generic "ill-formed C-variadic function" into specific errors about using an incorrect ABI, not specifying an ABI, or missing the unsafe keyword C-variadic coroutines probably don't make sense? C-variadic functions are for FFI purposes, combining that with async functions seems weird. For associated functions, we're just cutting scope. It's probably fine, but it's probably better to explicitly allow it. So for now, at least give a more targeted error message. Made to be reviewed commit-by-commit. cc `@workingjubilee` r? compiler
tracking issue: #44930
The parts of #143546 that don't require any particular knowledge about c-variadic functions.
This prepares the way for rejecting c-variadic functions that are also coroutines, safe functions, or associated functions.