On Wed Dec 4, 2024 at 2:06 AM EST, Bor Grošelj Simić wrote:
> > > According to the current spec patch, you should just check that the hint has
> > > two members here, and one of them is nomem (not an alias of nomem):
> >
> > IIRC I provided feedback on the spec patch to allow for more than two
> > members, so tagged unions can be allocated. I haven't looked at the most
> > recent patch, so I'm not sure if that's correctly specified, but
> > allowing more than two members is the Correct behavior. No strong opinion on
> > nomem aliases, though I'm leaning towards disallowing them.
>
> I think we have a misunderstanding here. I don't see how this is related to
> tagged union allocation, because for example (*(u32 | void) | nomem) is a
> two member tagged union right?.
>

On Tue Dec 3, 2024 at 10:59 PM EST, Bor Grošelj Simić wrote:
> > References: https://todo.sr.ht/~sircmpwn/hare/820
>
> > +// If an invalid UTF-8 sequence is encountered, the position of the decoder is
> > +// set to immediately after the first invalid byte.
>
> What I had in mind with that ticket is that we should document things such as,
> am I allowed to call next() again after an error? Will I even get anything
> meaningful out of the decoder if I inspect its state? Can I replace errors with
> replacement characters and then call next()?

At least to me that all sounds implied from the docs I added, but I
guess it doesn't hurt to be a bit more explicit.

On Tue Dec 3, 2024 at 7:18 PM EST, Ember Sawady wrote:
> another option, which i'd prefer over the current scenario, is to 
> provide a scary-looking warning in the docs for all these functions

This is part of why I proposed moving them to math::fenv:: (if we don't
remove them): that way we can put the scary warning in the README
itself.

On Mon Dec 2, 2024 at 5:22 AM EST, Vlad-Stefan Harbuz wrote:
> Thanks for this! :)
>
> Everything LGTM, but I don't think I've quite understood the usage of shift(). Is this something newly required because of the nested captures?

Tbh this patch is months old so I don't entirely remember, but I'm
pretty sure the behavior prior to this patch was incorrect; it just
happened to not be caught by any tests until now. The jumps/splits need
to be incremented because they target an instruction by its index, but
that index changes when inserting into the slice. See the first change
in +test.ha, which changes an existing test.

shift() still feels kinda hacky to me; there's probably a better way.

On Mon Dec 2, 2024 at 3:33 AM EST, Bor Grošelj Simić wrote:
> Is there a particular reason for this? To me it looked clearer before, and we
> won't be able to escape C based tests in the long term anyway if we plan to
> improve test coverage.

No particular reason, other than it being the only non-Hare test file.
If we'll need C tests in the future anyway then this patch can be
discarded.

On Mon Dec 2, 2024 at 1:07 AM EST, Bor Grošelj Simić wrote:
> I'm not sure about this. This file is obviously in a WIP state, but it is
> possible to use its contents despite what QBE does to compile time constants.
> At least the parts dealing with exceptions and not with rounding modes should
> stay IMO.

The exception stuff has the same issues, where they won't be raised if
QBE optimizes out the arithmetic.

If we do keep these functions (which I don't think we should given the
rt:: functions), what do you think about moving them to a submodule
(math::fenv::)?

On Sun Dec 1, 2024 at 11:37 PM EST, Bor Grošelj Simić wrote:
> > These functions are pretty silly, since it's just as easy and readable
> > to do a direct comparion with zero yourself.
>
> I don't think we should do this. There is more that can be done with a +1/0/-1
> numeric valus than just using it in a comparison.

Like what?

On Mon Dec 2, 2024 at 8:51 PM EST, Bor Grošelj Simić wrote:
> I found one big problem in the design though I'm afraid.
> (nomem | *T) is castable to *T.
> Which means that code that was perfectly valid and safe is now broken in a
> really nasty way.
>
> // We had:
> //equivalent
> let x: *u8 = alloc(2);
> let x = alloc(2): *u8;
>
> // now we have:
> // doesn't compile
> let x: *u8 = alloc(2);

Signed-off-by: Sebastian <sebastian@sebsite.pw>
---
CI will fail because this depends on nomem, as well as some other
patches I've sent earlier. If you want to mess around with this and it
isn't compiling for you, get rid of oldast, oldlex, and oldparse, and
import the normal hare:: modules instead (or just fix the errors
reported by the compiler; there should be very few of them). You'll
still need the harec nomem patch applied though.

I've tested this with various Hare projects (including very large ones
like hare-c and its dependencies: hare-xml and madeline), with
successful results. I've also tested it on some smaller files to ensure
edge-cases are handled correctly.
[message trimmed]

For consistency with their names in stdarg.h

Signed-off-by: Sebastian <sebastian@sebsite.pw>
---
 include/ast.h     |  4 ++--
 include/expr.h    | 10 ++++-----
 include/lex.h     |  8 +++----
 include/types.h   |  4 ++--
 src/check.c       | 56 +++++++++++++++++++++++------------------------
 src/eval.c        | 14 ++++++------
 src/expr.c        |  2 +-
 src/gen.c         | 26 +++++++++++-----------
 src/lex.c         | 10 ++++-----
 src/parse.c       | 32 +++++++++++++--------------
[message trimmed]