would a patch for #981 be accepted?

Hi,

Is http://tracker.bro.org/bro/ticket/981 a bug, or did I misunderstand
how the language is supposed to work? If I write a patch, would it be
accepted (assuming it's of good quality)? I'm not sure if I'll be able
to write the patch yet, but I'm trying to get a sense of how to move
forward.


I think that's a legit bug. Robin? Jon?

  .Seth

Provided that &default for tables is not supposed to modify table membership when accessing indices that don't exist, the example you gave looked correct to me (i.e. not a bug)

Since foo[0] and foo[1] are never actually entries in the table, you're always working directly with the value associated with the &default attribute whenever you access the table at those indices.

- Jon

I'm wondering if it should modify the table here. I can see how the
current behaviour is misleading, it violates the "principle of least
surprise". :slight_smile:

Can we change tables so that if &default is a non-constant, the first
time one accesses a non-existing index, that slot gets assigned a
deep-copy of the &default value? The downside would be that if
somebody is relying on the current behaviour, he might access lots of
non-existing entries with the assumption that the table won't change
(i.e., he won't run into memory trouble).

An alternative would be to have the lookup return a copy of the
default, but not modify the table. With that "foo[0]$x = 0" would
still not work as expected, but at least it wouldn't have any
surprising side effects.

Robin

What about modifying the table on writes to empty slots but not reads of
empty slots? Is that feasible, or would it require too much code change?

Don't think that's going to work because the write operates on the
returned record; at that point it's not clear anymore that the value
came out of the table.

Robin

Provided that &default for tables is not supposed to modify table
membership when accessing indices that don't exist,

I'm wondering if it should modify the table here. I can see how the
current behaviour is misleading, it violates the "principle of least
surprise". :slight_smile:

Does seem more intuitive to me.

Can we change tables so that if &default is a non-constant, the first
time one accesses a non-existing index, that slot gets assigned a
deep-copy of the &default value?

Probably.

The downside would be that if
somebody is relying on the current behaviour, he might access lots of
non-existing entries with the assumption that the table won't change
(i.e., he won't run into memory trouble).

Maybe we can provide a script-layer flag that, when on, generates warnings for script locations that end up assigning &default values to non-existing indices? That at least would help someone pinpoint locations they need to change to work w/ new behavior.

- Jon

I think this is reasonable and the way I'd expect it to work at least. I've never seen anyone doing things with &default that would violate it either. On the upside I've never seen anyone give a non-constant &default value.

  .Seth

Can we change tables so that if &default is a non-constant, the first
time one accesses a non-existing index, that slot gets assigned a
deep-copy of the &default value?

I'm not a fan of this, as "&default is a non-constant" is a more nuanced
semantic notion (and could get messy for complicated constructors) than
the sort that I think language rules should draw upon. Rather, my thinking
is that &default should apply to rvalues but not lvalues. I believe (but
have not confirmed) that there's enough context in the parser to tell whether
the access is in an lvalue context or an rvalue context. If so, then the
former shoud be a run-time error; if the user wants to update a value in the
table that's not already there, they need to first put the value in the table.

An alternative would be to have the lookup return a copy of the
default, but not modify the table.

That makes sense to me in any case. In fact, I'm surprised this isn't
what already happens.

    Vern