Enforce user literals parsing

intel / safe-arithmetic

Safe arithmetic library for C++20 and above. Safe arithmetic ensures correctness of arithmetic operations at compile-time. It protects against overflow, underflow, divide by zero, and out-of-bounds index access. This provides both functional correctness as well as greater protection against related security threats.

https://intel.github.io/safe-arithmetic/

Boost Software License 1.0

86 stars 10 forks source link

Enforce user literals parsing #50

Closed rykovv closed 5 days ago

rykovv commented 3 months ago

Closes #11

Current implementation contains a 🐛 bug. Compilation will fail for invalid values captured by user literals. Such weakness puts in danger the whole purpose of the safety library. Stronger user literals parsing is needed.

This PR adds strong support for positive decimal and hexadecimal numbers. The support is provided through creation of three concepts: hex_integer and decimal_integer. These concepts match provided values with intended types and dispatch appropriate parsing function. So, additional safety is provided.

elbeno commented 3 months ago

UDLs will never receive the negative sign? Unary minus is an operator, not part of a literal. https://godbolt.org/z/M1hneGfj7

rykovv commented 3 months ago

See the point. My understanding wasn't correct. Hence, think it can be split in following steps:

Remove netagive decimal integer concept
Create additional minus struct but derived from unary_op in dsl/minus.hpp
Specialize for Primitive. Will need to swap min with max in ival and make both negative.
Does specialization for mask make sense?
Add unary minus operator overload

Am I missing something?

lukevalenty commented 3 months ago

See the point. My understanding wasn't correct. Hence, think it can be split in following steps:

Remove netagive decimal integer concept

Create additional minus struct but derived from unary_op in dsl/minus.hpp

Specialize for Primitive. Will need to swap min with max in ival and make both negative.

Does specialization for mask make sense?

Add unary minus operator overload

Am I missing something?

I think you can just remove the negative decimal integer concept and it's fine. The library should already handle unary minus.

rykovv commented 3 months ago

I think you can just remove the negative decimal integer concept and it's fine. The library should already handle unary minus.

Gotcha. Haven't seen anything in dsl/minus.hpp, but found later in var.

rykovv commented 3 months ago

Restructuring the PR.

rykovv commented 3 months ago

@elbeno are there any specific requirements for clang-format? Is the style based on LLVM or Google?

rykovv commented 1 month ago

@elbeno which parameters should I use to properly apply clang-format? Couldn't find clues in git-clang-format.py

elbeno commented 1 month ago

@elbeno which parameters should I use to properly apply clang-format? Couldn't find clues in git-clang-format.py

Build the target(s) fix-clang-format and check-clang-format. Note that cmake finds clang-format/clang-tidy alongside the compiler it's using. The CI on this repo currently uses clang-17 for the formatting. (Which is old; this repo needs to be updated to take advantage of clang-18. But one thing at a time.)

rykovv commented 4 weeks ago

@elbeno I'm getting the code formatted with clang-format / clang-tidy, alas with wrong formatting rules (default). I don't see .clang-format file anywhere in the repo, but rather included in .gitignore. If the .clang-format file is not part of the repo, how a common style is enforced?

rykovv commented 3 weeks ago

I've noticed unary negation is not supported by unsigned types. Would be a topic for another PR.

elbeno commented 3 weeks ago

This repo pulls in various CI stuff -- including .clang-format -- from https://github.com/intel/cicd-repo-infrastructure. Running cmake should produce a .clang-format file (symlinked).

elbeno commented 3 weeks ago

With a clean repo, what I normally do is:

$ cmake -Bbuild
$ TOOLCHAIN_ROOT=/usr/lib/llvm-17 cmake --preset clang --fresh
$ ninja -C build <target>

The first cmake run sets up the symlinks from the CICD repo, including the presets file. This is only necessary once. The second cmake run is the "real" one that uses llvm-17 (installed from the script at apt.llvm.org) and the symlnked presets file. After that, you can build any target you like.

rykovv commented 3 weeks ago

This method doesn't work in Windows. Looks like presets expect clang++ binaries to be in Linux-based directories (/usr/bin). You've mentioned something about WSL (Windows Subsystem for Linux). Is there a way to get everything setup there? Otherwise, I'll need to install a Linux distro for C++ dev.

elbeno commented 3 weeks ago

WSL will work.

rykovv commented 3 weeks ago

Will try to get it working with WSL

rykovv commented 2 weeks ago

So, still struggling to get it working:

Ran

$ sudo apt install libc-bin
$ sudo apt install build-essential
$ llvm.sh 17 # script from apt.llvm.org
$ sudo apt install clang-tools-17 clang-format-17 clang-tidy-17

$ cd safe-arithmetic
$ cmake -Bbuild -GNinja
$ TOOLCHAIN_ROOT=/usr/lib/llvm-17 cmake --preset clang --fresh
$ ninja -C build check-clang-format # failed

The last command faild with error: Format.cmake: cannot run because clang-format and/or python not found So, I setup up the symbolic links

$ ln -s /usr/bin/clang-format-17 /usr/bin/clang-format
$ ln -s /usr/bin/clang-tidy-17 /usr/bin/clang-tidy
$ ln -s /usr/bin/python3 /usr/bin/python

That didn't help. So, ninja -C build check-clang-format is still not working.

On the other hand, I've tried ninja -C build all It also failed

/safe-arithmetic/test/safe/big_integer/detail/minus.cpp
In file included from /root/safe-arithmetic/test/safe/big_integer/detail/minus.cpp:1:
In file included from /root/safe-arithmetic/test/safe/big_integer/detail/properties.hpp:3:
/root/safe-arithmetic/include/safe/big_integer/detail/storage.hpp:6:10: fatal error: 'compare' file not found
    6 | #include <compare>
      |          ^~~~~~~~~
1 error generated.