rust icon indicating copy to clipboard operation
rust copied to clipboard

Published 20 hours ago • verified publisher icon rust-lang

Fix println! ICE when parsing percent prefix number

Open pymongo opened this issue 1 year ago • 3 comments

This PR fixes #125002 ICE occurring, for example, with println!("%100000", 1) or println!("% 100000", 1).

Test Case/Change Explanation

The return type of Num::from_str has been changed to Option<Self> to handle errors when parsing large integers fails.

  1. The first println! in the test case covers the change of the first Num::from_str usage in format_foreign.rs:426.
  2. The second println! in the test case covers the change of the second Num::from_str usage in line 460.
  3. The 3rd to 5th Num::from_str usages behave the same as before.

The 3rd usage would cause an ICE when num > u16::MAX in the previous version, but this commit does not include a fix for the ICE in println!("{:100000$}"). I think we need to emit an error in the compiler and have more discussion in another issue/PR.

pymongo avatar May 11 '24 09:05 pymongo

r? @lcnr

rustbot has assigned @lcnr. They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot avatar May 11 '24 09:05 rustbot

The job mingw-check-tidy failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot) 
Getting action download info
Download action repository 'msys2/[email protected]' (SHA:cc11e9188b693c2b100158c3322424c4cc1dadea)
Download action repository 'actions/checkout@v4' (SHA:0ad4b8fadaa221de15dcec353f45205ec38ea70b)
Download action repository 'actions/upload-artifact@v4' (SHA:65462800fd760344b1a7b4382951275a0abb4808)
Complete job name: PR - mingw-check-tidy
git config --global core.autocrlf false
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
---
COPY scripts/sccache.sh /scripts/
RUN sh /scripts/sccache.sh

COPY host-x86_64/mingw-check/reuse-requirements.txt /tmp/
RUN pip3 install --no-deps --no-cache-dir --require-hashes -r /tmp/reuse-requirements.txt \
    && pip3 install virtualenv
COPY host-x86_64/mingw-check/validate-toolstate.sh /scripts/
COPY host-x86_64/mingw-check/validate-error-codes.sh /scripts/

# NOTE: intentionally uses python2 for x.py so we can test it still works.
# NOTE: intentionally uses python2 for x.py so we can test it still works.
# validate-toolstate only runs in our CI, so it's ok for it to only support python3.
ENV SCRIPT TIDY_PRINT_DIFF=1 python2.7 ../x.py test \
           --stage 0 src/tools/tidy tidyselftest --extra-checks=py:lint
# This file is autogenerated by pip-compile with Python 3.10
# by the following command:
#
#    pip-compile --allow-unsafe --generate-hashes reuse-requirements.in
---

#12 [5/8] COPY host-x86_64/mingw-check/reuse-requirements.txt /tmp/
#12 DONE 0.0s

#13 [6/8] RUN pip3 install --no-deps --no-cache-dir --require-hashes -r /tmp/reuse-requirements.txt     && pip3 install virtualenv
#13 0.434   Downloading binaryornot-0.4.4-py2.py3-none-any.whl (9.0 kB)
#13 0.452 Collecting boolean-py==4.0
#13 0.459   Downloading boolean.py-4.0-py3-none-any.whl (25 kB)
#13 0.476 Collecting chardet==5.1.0
---
#13 3.692 Building wheels for collected packages: reuse
#13 3.693   Building wheel for reuse (pyproject.toml): started
#13 4.026   Building wheel for reuse (pyproject.toml): finished with status 'done'
#13 4.027   Created wheel for reuse: filename=reuse-1.1.0-cp310-cp310-manylinux_2_35_x86_64.whl size=181117 sha256=f5f58750481f69515c2c0d1d503daf565e2565c370d07fc6aeb95fe3498b4269
#13 4.027   Stored in directory: /tmp/pip-ephem-wheel-cache-e4h_v58t/wheels/c2/3c/b9/1120c2ab4bd82694f7e6f0537dc5b9a085c13e2c69a8d0c76d
#13 4.030 Installing collected packages: boolean-py, binaryornot, setuptools, reuse, python-debian, markupsafe, license-expression, jinja2, chardet
#13 4.053   Attempting uninstall: setuptools
#13 4.053     Found existing installation: setuptools 59.6.0
#13 4.055     Not uninstalling setuptools at /usr/lib/python3/dist-packages, outside environment /usr
#13 4.055     Not uninstalling setuptools at /usr/lib/python3/dist-packages, outside environment /usr
#13 4.055     Can't uninstall 'setuptools'. No files were found to uninstall.
#13 4.754 Successfully installed binaryornot-0.4.4 boolean-py-4.0 chardet-5.1.0 jinja2-3.1.2 license-expression-30.0.0 markupsafe-2.1.1 python-debian-0.1.49 reuse-1.1.0 setuptools-66.0.0
#13 4.755 WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv
#13 5.277 Collecting virtualenv
#13 5.327   Downloading virtualenv-20.26.1-py3-none-any.whl (3.9 MB)
#13 5.507      ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 3.9/3.9 MB 22.1 MB/s eta 0:00:00
#13 5.547 Collecting distlib<1,>=0.3.7
#13 5.554   Downloading distlib-0.3.8-py2.py3-none-any.whl (468 kB)
#13 5.601 Collecting filelock<4,>=3.12.2
#13 5.608   Downloading filelock-3.14.0-py3-none-any.whl (12 kB)
#13 5.608   Downloading filelock-3.14.0-py3-none-any.whl (12 kB)
#13 5.641 Collecting platformdirs<5,>=3.9.1
#13 5.648   Downloading platformdirs-4.2.1-py3-none-any.whl (17 kB)
#13 5.735 Installing collected packages: distlib, platformdirs, filelock, virtualenv
#13 5.900 Successfully installed distlib-0.3.8 filelock-3.14.0 platformdirs-4.2.1 virtualenv-20.26.1
#13 DONE 6.0s

#14 [7/8] COPY host-x86_64/mingw-check/validate-toolstate.sh /scripts/
#14 DONE 0.0s
---
DirectMap4k:      196544 kB
DirectMap2M:     7143424 kB
DirectMap1G:    11534336 kB
##[endgroup]
Executing TIDY_PRINT_DIFF=1 python2.7 ../x.py test            --stage 0 src/tools/tidy tidyselftest --extra-checks=py:lint
+ TIDY_PRINT_DIFF=1 python2.7 ../x.py test --stage 0 src/tools/tidy tidyselftest --extra-checks=py:lint
    Finished `dev` profile [unoptimized] target(s) in 0.03s
##[endgroup]
downloading https://ci-artifacts.rust-lang.org/rustc-builds-alt/be7549f82c194c7a5dc8f34ed0541bfebbaf33ea/rust-dev-nightly-x86_64-unknown-linux-gnu.tar.xz
extracting /checkout/obj/build/cache/llvm-be7549f82c194c7a5dc8f34ed0541bfebbaf33ea-true/rust-dev-nightly-x86_64-unknown-linux-gnu.tar.xz to /checkout/obj/build/x86_64-unknown-linux-gnu/ci-llvm
---
    Finished `release` profile [optimized] target(s) in 26.55s
##[endgroup]
fmt check
tidy check
tidy error: file `tests/ui/macros/issue-125002.rs` must begin with a descriptive name, consider `{reason}-issue-125002.rs`
tidy error: /checkout/tests/ui/macros/issue-125002.rs: missing trailing newline
removing old virtual environment
removing old virtual environment
creating virtual environment at '/checkout/obj/build/venv' using 'python3.10'
Requirement already satisfied: pip in ./build/venv/lib/python3.10/site-packages (24.0)
Collecting black==23.3.0 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 7))
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 1.7/1.7 MB 22.0 MB/s eta 0:00:00
Collecting click==8.1.3 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 34))
  Downloading click-8.1.3-py3-none-any.whl (96 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 96.6/96.6 kB 31.9 MB/s eta 0:00:00
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 96.6/96.6 kB 31.9 MB/s eta 0:00:00
Collecting importlib-metadata==6.7.0 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 38))
  Downloading importlib_metadata-6.7.0-py3-none-any.whl (22 kB)
Collecting mypy-extensions==1.0.0 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 42))
  Downloading mypy_extensions-1.0.0-py3-none-any.whl (4.7 kB)
Collecting packaging==23.1 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 46))
  Downloading packaging-23.1-py3-none-any.whl (48 kB)
Collecting pathspec==0.11.1 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 50))
  Downloading pathspec-0.11.1-py3-none-any.whl (29 kB)
Collecting platformdirs==3.6.0 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 54))
  Downloading platformdirs-3.6.0-py3-none-any.whl (16 kB)
  Downloading platformdirs-3.6.0-py3-none-any.whl (16 kB)
Collecting ruff==0.0.272 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 58))
  Downloading ruff-0.0.272-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (5.9 MB)
Collecting tomli==2.0.1 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 77))
  Downloading tomli-2.0.1-py3-none-any.whl (12 kB)
Collecting typed-ast==1.5.4 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 81))
  Downloading typed_ast-1.5.4-cp310-cp310-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_12_x86_64.manylinux2010_x86_64.whl (877 kB)
  Downloading typed_ast-1.5.4-cp310-cp310-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_12_x86_64.manylinux2010_x86_64.whl (877 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 877.7/877.7 kB 79.3 MB/s eta 0:00:00
Collecting typing-extensions==4.6.3 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 107))
  Downloading typing_extensions-4.6.3-py3-none-any.whl (31 kB)
Collecting zipp==3.15.0 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 114))
  Downloading zipp-3.15.0-py3-none-any.whl (6.8 kB)
Installing collected packages: zipp, typing-extensions, typed-ast, tomli, ruff, platformdirs, pathspec, packaging, mypy-extensions, click, importlib-metadata, black
Successfully installed black-23.3.0 click-8.1.3 importlib-metadata-6.7.0 mypy-extensions-1.0.0 packaging-23.1 pathspec-0.11.1 platformdirs-3.6.0 ruff-0.0.272 tomli-2.0.1 typed-ast-1.5.4 typing-extensions-4.6.3 zipp-3.15.0
some tidy checks failed
Build completed unsuccessfully in 0:00:58
  local time: Sat May 11 09:24:55 UTC 2024
  network time: Sat, 11 May 2024 09:24:56 GMT

rust-log-analyzer avatar May 11 '24 09:05 rust-log-analyzer

r? estebank though feel free to reassign if you're also not familiar with this code

lcnr avatar May 13 '24 14:05 lcnr

@rustbot ready

pymongo avatar May 14 '24 07:05 pymongo

The job x86_64-gnu-llvm-17 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot) 
#16 exporting to docker image format
#16 sending tarball 30.5s done
#16 DONE 37.0s
##[endgroup]
Setting extra environment values for docker:  --env ENABLE_GCC_CODEGEN=1 --env GCC_EXEC_PREFIX=/usr/lib/gcc/
[CI_JOB_NAME=x86_64-gnu-llvm-17]
---
sccache: Starting the server...
##[group]Configure the build
configure: processing command line
configure: 
configure: build.configure-args := ['--build=x86_64-unknown-linux-gnu', '--llvm-root=/usr/lib/llvm-17', '--enable-llvm-link-shared', '--set', 'rust.thin-lto-import-instr-limit=10', '--set', 'change-id=99999999', '--enable-verbose-configure', '--enable-sccache', '--disable-manage-submodules', '--enable-locked-deps', '--enable-cargo-native-static', '--set', 'rust.codegen-units-std=1', '--set', 'dist.compression-profile=balanced', '--dist-compression-formats=xz', '--disable-dist-src', '--release-channel=nightly', '--enable-debug-assertions', '--enable-overflow-checks', '--enable-llvm-assertions', '--set', 'rust.verify-llvm-ir', '--set', 'rust.codegen-backends=llvm,cranelift,gcc', '--set', 'llvm.static-libstdcpp', '--enable-new-symbol-mangling']
configure: target.x86_64-unknown-linux-gnu.llvm-config := /usr/lib/llvm-17/bin/llvm-config
configure: llvm.link-shared     := True
configure: rust.thin-lto-import-instr-limit := 10
configure: change-id            := 99999999
---
   Compiling rustc_builtin_macros v0.0.0 (/checkout/compiler/rustc_builtin_macros)
error[E0308]: mismatched types
   --> compiler/rustc_builtin_macros/src/format_foreign.rs:424:37
    |
402 |         let mut width: Option<Num> = None;
...
...
424 |                             width = at.slice_between(end).map(|num| Num::from_str(num, None));
    |                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `Option<Num>`, found `Option<Option<Num>>`
    |
    = note: expected enum `Option<printf::Num>`
               found enum `Option<Option<printf::Num>>`
help: use the `?` operator to extract the `Option<Option<printf::Num>>` value, propagating an `Option::None` value to the caller
    |
424 |                             width = at.slice_between(end).map(|num| Num::from_str(num, None))?;

error[E0308]: mismatched types
   --> compiler/rustc_builtin_macros/src/format_foreign.rs:458:29
    |
    |
402 |         let mut width: Option<Num> = None;
...
...
458 |                     width = at.slice_between(end).map(|num| Num::from_str(num, None));
    |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `Option<Num>`, found `Option<Option<Num>>`
    |
    = note: expected enum `Option<printf::Num>`
               found enum `Option<Option<printf::Num>>`
help: use the `?` operator to extract the `Option<Option<printf::Num>>` value, propagating an `Option::None` value to the caller
    |
458 |                     width = at.slice_between(end).map(|num| Num::from_str(num, None))?;

error[E0308]: mismatched types
   --> compiler/rustc_builtin_macros/src/format_foreign.rs:522:33
    |
    |
403 |         let mut precision: Option<Num> = None;
...
...
522 |                     precision = at.slice_between(end).map(|num| Num::from_str(num, None));
    |                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `Option<Num>`, found `Option<Option<Num>>`
    |
    = note: expected enum `Option<printf::Num>`
               found enum `Option<Option<printf::Num>>`
help: use the `?` operator to extract the `Option<Option<printf::Num>>` value, propagating an `Option::None` value to the caller
    |
522 |                     precision = at.slice_between(end).map(|num| Num::from_str(num, None))?;

For more information about this error, try `rustc --explain E0308`.
error: could not compile `rustc_builtin_macros` (lib) due to 3 previous errors
warning: build failed, waiting for other jobs to finish...

rust-log-analyzer avatar May 14 '24 07:05 rust-log-analyzer

Most of the time you have Option<Option<T>> after a map, what you really wanted is an and_then that does Option<K> -> Option<T> and collapses the Nones.

Thanks! I forgot to use and_then to flatten Option<Option<T>>, the second review change is here https://github.com/pymongo/rust/commit/33d2c87c63152375aad48363585134d0a81bdb9b

pymongo avatar May 18 '24 02:05 pymongo

@bors r+

estebank avatar May 18 '24 06:05 estebank

:pushpin: Commit 582fd1fb539ddea199dbc238946b00465a20620e has been approved by estebank

It is now in the queue for this repository.

bors avatar May 18 '24 06:05 bors

:hourglass: Testing commit 582fd1fb539ddea199dbc238946b00465a20620e with merge 1c90b9fe6eac122b4d3965913b3615f47751a4d3...

bors avatar May 18 '24 08:05 bors

:sunny: Test successful - checks-actions Approved by: estebank Pushing 1c90b9fe6eac122b4d3965913b3615f47751a4d3 to master...

bors avatar May 18 '24 10:05 bors

Finished benchmarking commit (1c90b9fe6eac122b4d3965913b3615f47751a4d3): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary -0.6%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 1.2% [1.2%, 1.2%] 1
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 -2.5% [-2.5%, -2.5%] 1
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) -0.6% [-2.5%, 1.2%] 2

Cycles

Results (primary 1.5%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 1.5% [1.5%, 1.5%] 1
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) 1.5% [1.5%, 1.5%] 1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 669.26s -> 667.741s (-0.23%) Artifact size: 316.04 MiB -> 316.07 MiB (0.01%)

rust-timer avatar May 18 '24 12:05 rust-timer