diff options
author | Robin Haberkorn <robin.haberkorn@googlemail.com> | 2025-04-13 00:40:37 +0300 |
---|---|---|
committer | Robin Haberkorn <robin.haberkorn@googlemail.com> | 2025-04-13 01:33:43 +0300 |
commit | 628c73d984fd7663607cc3fd9368f809855906fd (patch) | |
tree | bf9922ec94bbce2df19ef146724bbfe124138998 /src/parser.h | |
parent | 3b06b44fc22cd5c936dd3ce677290e3f65f7f8ce (diff) | |
download | sciteco-628c73d984fd7663607cc3fd9368f809855906fd.tar.gz |
fixed undoing bitfields on Windows
* It turns out that `bool` (_Bool) in bitfields may cause
padding to the next 32-bit word.
This was only observed on MinGW.
I am not entirely sure why, although the C standard does
not guarantee much with regard to bitfield memory layout
and there are 64-bit available due to passing anyway.
Actually, they could also be layed out in a different order.
* I am now consistently using guint instead of `bool` in bitfields
to prevent any potential surprises.
* The way that guint was aliased with bitfield structs
for undoing teco_machine_main_t and teco_machine_qregspec_t flags
was therefore insecure.
It was not guaranteed that the __flags field really "captures"
all of the bit field.
Even with `guint v : 1` fields, this was not guaranteed.
We would have required a static assertion for robustness.
Alternatively, we could have declared a `gsize __flags` variable
as well. This __should__ be safe since gsize should always be
pointer sized and correspond to the platform's alignment.
However, it's also not 100% guaranteed.
Using classic ANSI C enums with bit operations to encode multiple
fields and flags into a single integer also doesn't look very
attractive.
* Instead, we now define scalar types with their own teco_undo_push()
shortcuts for the bitfield structs.
This is in one way simpler and much more robust, but on the other
hand complicates access to the flag variables.
* It's a good question whether a `struct __attribute__((packed))` bitfield
with guint fields would be a reliable replacement for flag enums, that
are communicated with the "outside" (TECO) world.
I am not going to risk it until GCC gives any guarantees, though.
For the time being, bitfields are only used internally where
the concrete memory layout (bit positions) is not crucial.
* This fixes the test suite and therefore probably CI and nightly
builds on Windows.
* Test case: Rub out `@I//` or `@Xq` until before the `@`.
The parser doesn't know that `@` is still set and allows
all sorts of commands where `@` should be forbidden.
* It's unknown how long this has been broken on Windows - quite
possibly since v2.0.
Diffstat (limited to 'src/parser.h')
-rw-r--r-- | src/parser.h | 48 |
1 files changed, 22 insertions, 26 deletions
diff --git a/src/parser.h b/src/parser.h index 050467c..5477150 100644 --- a/src/parser.h +++ b/src/parser.h @@ -16,8 +16,6 @@ */ #pragma once -#include <stdbool.h> - #include <glib.h> #include <Scintilla.h> @@ -52,7 +50,7 @@ typedef struct { * a signed integer, it's ok steal one * bit for the pass_through flag. */ - bool pass_through : 1; + guint pass_through : 1; } teco_loop_context_t; extern GArray *teco_loop_stack; @@ -77,8 +75,8 @@ void undo__remove_index__teco_loop_stack(guint); * FIXME: Maybe use TECO_DECLARE_VTABLE_METHOD()? */ typedef const struct { - bool string_building : 1; - bool last : 1; + guint string_building : 1; + guint last : 1; /** * Called repeatedly to process chunks of input and give interactive feedback. @@ -206,7 +204,7 @@ struct teco_state_t { * This is separate of TECO_KEYMACRO_MASK_START which is set * only in the main machine's start states. */ - bool is_start : 1; + guint is_start : 1; /** * Key macro mask. * This is not a bitmask since it is compared with values set @@ -476,25 +474,17 @@ struct teco_machine_main_t { /** Program counter, i.e. pointer to the next character in the current macro frame */ gsize macro_pc; - /** - * Aliases bitfield with an integer. - * This allows teco_undo_guint(__flags), - * while still supporting easy-to-access flags. - */ - union { - struct { - teco_mode_t mode : 8; - - /** number of `:`-modifiers detected */ - guint modifier_colon : 2; - /** - * Whether the `@`-modifier has been detected. - * This is tracked even in parse-only mode. - */ - bool modifier_at : 1; - }; - guint __flags; - }; + struct teco_machine_main_flags_t { + teco_mode_t mode : 8; + + /** number of `:`-modifiers detected */ + guint modifier_colon : 2; + /** + * Whether the `@`-modifier has been detected. + * This is tracked even in parse-only mode. + */ + guint modifier_at : 1; + } flags; /** The nesting level of braces */ guint brace_level; @@ -529,6 +519,12 @@ struct teco_machine_main_t { }; }; +typedef struct teco_machine_main_flags_t teco_machine_main_flags_t; +TECO_DECLARE_UNDO_SCALAR(teco_machine_main_flags_t); + +#define teco_undo_flags(VAR) \ + (*teco_undo_object_teco_machine_main_flags_t_push(&(VAR))) + void teco_machine_main_init(teco_machine_main_t *ctx, teco_qreg_table_t *qreg_table_locals, gboolean must_undo); @@ -564,7 +560,7 @@ typedef const struct { * Since `@` has syntactic significance, * it is checked even in parse-only mode. */ - bool modifier_at : 1; + guint modifier_at : 1; } teco_machine_main_transition_t; /* |