nim-lang / Nim

Nim is a statically typed compiled systems programming language. It combines successful concepts from mature languages like Python, Ada and Modula. Its design focuses on efficiency, expressiveness, and elegance (in that order of priority).
https://nim-lang.org
Other
16.63k stars 1.47k forks source link

stricter skip for conversions in array indices in transf #24424

Closed metagn closed 2 weeks ago

metagn commented 2 weeks ago

fixes #17958

In transf, conversions in subscript expressions are skipped (with skipConv's rules). This is because array indexing can produce conversions to the range type that is the array's index type, which causes a RangeDefect rather than an IndexDefect (and also --rangeChecks and --indexChecks are both considered). However this causes problems when explicit conversions are used, between types of different bitsizes, because those also get skipped.

To fix this, we only skip the conversion if:

And skipConv rules also still apply (int/float classification).

Another idea would be to prevent the implicit conversion to the array index type from being generated. But there is no good way to do this: matching to the base type instead prevents types like uint32 from implicitly converting (i.e. it can convert to range[0..3] but not int), and analyzing whether this is an array bound check is easier in transf, since sigmatch just produces a type conversion.

The rules for skipping the conversion could also receive some other tweaks: We could add a rule that changing bitsizes also doesn't skip the conversion, but this breaks the uint32 case. We could simplify it to only removing implicit skips to specifically fix #17958, but this is wrong in general.

We could also add something like nkChckIndex that generates index errors instead but this is weird when it doesn't have access to the collection type and it might be overkill.

metagn commented 2 weeks ago

Just removing the skip produces RangeDefect instead of IndexDefect because of implicit conversions, will restrict this to explicit conversions.

github-actions[bot] commented 2 weeks ago

Thanks for your hard work on this PR! The lines below are statistics of the Nim compiler built from 76c5f16ac5e06cc4cdd24c41067ffcf35bdf77dc

Hint: mm: orc; opt: speed; options: -d:release 177412 lines; 8.772s; 652.547MiB peakmem