David Mak derppening
  • Joined on 2023-08-31
derppening suggested changes for M-Labs/nac3#508 2024-08-26 17:34:56 +08:00
ndstrides: Allow multi-file IRRT + Exceptions in IRRT

The positions of pointers * and references & are still incorrect, and I still see many uses of C-style casts. Please change those before submitting for re-review.

derppening commented on pull request M-Labs/nac3#508 2024-08-26 17:33:43 +08:00
ndstrides: Allow multi-file IRRT + Exceptions in IRRT

Still not reformatted.

derppening commented on pull request M-Labs/nac3#508 2024-08-26 17:30:53 +08:00
ndstrides: Allow multi-file IRRT + Exceptions in IRRT

If you have to do this ugly Makefile trick then let's just keep it the way it is for now.

derppening commented on pull request M-Labs/nac3#508 2024-08-26 15:16:57 +08:00
ndstrides: Allow multi-file IRRT + Exceptions in IRRT

You can keep the _BOOL variable.

I would say I would do it as

if constexpr (IRRT_DEBUG_ASSERT_BOOL) {
    if ((lhs) != (rhs)) {
        raise_debug_assert(SizeT, "LHS = {0}. RHS = {1}",
derppening commented on pull request M-Labs/nac3#508 2024-08-26 15:14:23 +08:00
ndstrides: Allow multi-file IRRT + Exceptions in IRRT

constexpr indicates that this is to be a compile-time constant, and implies inline. This ensures that the variable does not escape this compilation unit and prevents ODR violations. AFAIK…

derppening commented on pull request M-Labs/nac3#508 2024-08-26 15:09:52 +08:00
ndstrides: Allow multi-file IRRT + Exceptions in IRRT

They are functionally the same, but you should use modern C++ regardless.

https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es49-if-you-must-use-a-cast-use-a-named-cast

derppening commented on pull request M-Labs/nac3#508 2024-08-26 15:04:02 +08:00
ndstrides: Allow multi-file IRRT + Exceptions in IRRT

Designated initializer syntax requires C++20.

derppening commented on pull request M-Labs/nac3#508 2024-08-26 15:03:21 +08:00
ndstrides: Allow multi-file IRRT + Exceptions in IRRT

I personally use LLVM, as Microsoft's opening brace style makes the source code unnecessarily bloated.

derppening pushed to misc/update-pyo3 at M-Labs/nac3 2024-08-26 14:56:28 +08:00
c15dd02df4 artiq: Remove all uses to gil-refs APIs
2095579ea2 artiq: Update to pyo3 v0.22 with gil-refs feature
cba3bed311 artiq: Update to pyo3 v0.21
c5ae0e7c36 [standalone] Add tests for tuple equality
b8dab6cf7c [standalone] Add tests for string equality
Compare 18 commits »
derppening commented on pull request M-Labs/nac3#508 2024-08-26 14:56:12 +08:00
ndstrides: Allow multi-file IRRT + Exceptions in IRRT

Same here.

derppening commented on pull request M-Labs/nac3#508 2024-08-26 14:56:12 +08:00
ndstrides: Allow multi-file IRRT + Exceptions in IRRT

Couldn't you just set the exceptions while loading the IRRT? Since IRRT depends on the exception specifications anyways.

derppening commented on pull request M-Labs/nac3#508 2024-08-26 14:56:12 +08:00
ndstrides: Allow multi-file IRRT + Exceptions in IRRT

Fold this into use nac3core::codegen::{...}.

derppening commented on pull request M-Labs/nac3#508 2024-08-26 14:56:12 +08:00
ndstrides: Allow multi-file IRRT + Exceptions in IRRT

Why is this necessary? Why not have multiple IRRT C++ files and link the LLVM bitcode at build-time?

derppening suggested changes for M-Labs/nac3#508 2024-08-26 14:56:12 +08:00
ndstrides: Allow multi-file IRRT + Exceptions in IRRT
derppening commented on pull request M-Labs/nac3#508 2024-08-26 14:56:12 +08:00
ndstrides: Allow multi-file IRRT + Exceptions in IRRT

constexpr

derppening commented on pull request M-Labs/nac3#508 2024-08-26 14:56:12 +08:00
ndstrides: Allow multi-file IRRT + Exceptions in IRRT

Fold this into use nac3core::{codegen::{...}, ...}

derppening commented on pull request M-Labs/nac3#508 2024-08-26 14:56:12 +08:00
ndstrides: Allow multi-file IRRT + Exceptions in IRRT

Probably best to split original.hpp into separate files, since you're already doing that.

derppening commented on pull request M-Labs/nac3#508 2024-08-26 14:56:12 +08:00
ndstrides: Allow multi-file IRRT + Exceptions in IRRT

Why Microsoft style? Wouldn't this introduce inconsistencies with demo.c where opening braces are on the same line as the statement?

derppening commented on pull request M-Labs/nac3#508 2024-08-26 14:56:12 +08:00
ndstrides: Allow multi-file IRRT + Exceptions in IRRT

Same here.

derppening commented on pull request M-Labs/nac3#508 2024-08-26 14:56:12 +08:00
ndstrides: Allow multi-file IRRT + Exceptions in IRRT

* on the left, i.e. uint8_t*.