David Mak derppening
  • Joined on 2023-08-31
derppening commented on pull request M-Labs/nac3#509 2024-08-27 15:52:31 +08:00
ndstrides: [1] Introduce models

Why are operators provided by some functions but not by others?

derppening commented on pull request M-Labs/nac3#509 2024-08-27 15:52:31 +08:00
ndstrides: [1] Introduce models

What if I want a signed constant then?

derppening commented on pull request M-Labs/nac3#509 2024-08-27 15:52:31 +08:00
ndstrides: [1] Introduce models

Same as with Float and Array.

derppening commented on pull request M-Labs/nac3#509 2024-08-27 15:52:31 +08:00
ndstrides: [1] Introduce models

Same as Array - i64.

derppening commented on pull request M-Labs/nac3#509 2024-08-27 15:52:31 +08:00
ndstrides: [1] Introduce models

I would suggest extending them to SizeT to match the signature of memcpy in C.

derppening commented on pull request M-Labs/nac3#509 2024-08-27 15:52:31 +08:00
ndstrides: [1] Introduce models

FieldIndexCollector? Same for below - Name those as *Collector?

derppening commented on pull request M-Labs/nac3#509 2024-08-27 15:52:31 +08:00
ndstrides: [1] Introduce models

u32.

derppening commented on pull request M-Labs/nac3#509 2024-08-27 15:52:31 +08:00
ndstrides: [1] Introduce models

Move this into the documentation for their respective functions. It is very difficult to refer to these examples when you are trying to look for how to use the APIs.

derppening commented on pull request M-Labs/nac3#509 2024-08-27 15:52:31 +08:00
ndstrides: [1] Introduce models

Consider adding a new() function for GepFieldTraversal.

derppening commented on pull request M-Labs/nac3#509 2024-08-27 15:52:31 +08:00
ndstrides: [1] Introduce models

I think FunctionCall or FuncCall might be a better name.

derppening commented on pull request M-Labs/nac3#509 2024-08-27 15:52:31 +08:00
ndstrides: [1] Introduce models

I don't really agree with the naming of the returning family of functions, as it doesn't indicate to the caller that by invoking returning it actually invokes the function as well.

derppening commented on pull request M-Labs/nac3#509 2024-08-27 15:52:31 +08:00
ndstrides: [1] Introduce models

MapStructFields? Since the only operations its implementing classes do is to add fields based on some criteria, matching what map does to an iterator. Or FieldMapper.

derppening commented on pull request M-Labs/nac3#509 2024-08-27 15:52:31 +08:00
ndstrides: [1] Introduce models

I would name this builder instead.

derppening commented on pull request M-Labs/nac3#509 2024-08-27 15:52:31 +08:00
ndstrides: [1] Introduce models

This name is non-descriptive. Output or OutType would be better.

derppening commented on pull request M-Labs/nac3#509 2024-08-27 15:52:31 +08:00
ndstrides: [1] Introduce models

nit: newline

derppening commented on pull request M-Labs/nac3#509 2024-08-27 15:52:31 +08:00
ndstrides: [1] Introduce models

Why are the type variables not directly bounded by LenKind and Model respectively then, rather than only bounding the impls?

derppening commented on pull request M-Labs/nac3#509 2024-08-27 15:52:31 +08:00
ndstrides: [1] Introduce models

I would suggest naming it llvm_type. Use of get_ is discouraged in Rust AFAIK.

derppening commented on pull request M-Labs/nac3#509 2024-08-27 15:52:31 +08:00
ndstrides: [1] Introduce models

I suspect this might be a candidate for unsafe, as it is a requirement for the caller to enforce this contract.,

derppening commented on pull request M-Labs/nac3#509 2024-08-27 15:52:31 +08:00
ndstrides: [1] Introduce models

I think you can just name it as length().