Breed names can cause shadowing of built-in procedures

[mrerrormessage]

Consider what the meaning of is-string? is in the following file. As it happens, is-string? "abc" => true, while is-string? one-of strings => false.

breeds [ strings string ]

This also affects link breeds (I believe). I'm unsure how to resolve this, as it seems like it has a lot of potential to break models. I think the following are viable alternatives:

1. Specify that the behavior in these cases is undefined and do whatever is most convenient for us (the status quo, stated explicitly).
2. Decide that we will always override the built-in primitives with breed primitives (or vice-versa).
3. Decide that this condition is an error and forbid certain breed names. This seems like the most NetLogo-y decision by far, IMO. But it will break some models

I was unable to find an issue like this for NetLogo, but I think it's quite possible that Tortoise and/or Headless have current or past issues to the same effect.

Should also add tests around procedure names shadowing breed procedure names, for instance:

breed [kittens kitten]
to kittens-at end

The above fails to compile in NetLogo hexy, but is not tested.

[SethTisue]

I vote for #3 (and I wouldn't be surprised if the new compiler already behaves that way, the StructureParser rewrite was supposed to fix all issues like this all at once)

[qiemem]

Vote for 3 as well. Do we know of any models this will break? I feel like such models are already asking from trouble...

[mrerrormessage]

The new compiler may behave that way in some circumstances, but not all. Notably it doesn't whine at all about directed-links-breed [ directed-links directed-link ].

A lot of tests use directed-links-breed [ directed-links directed-link ], which is a more subtle error as is-directed-link? now has two meanings. There are probably a handful of models (though none that I see in the models library) which use the same idiom and will break in similar ways.

[nicolaspayette]

Maybe it would be wise to change

NetLogo/src/main/org/nlogo/headless/HeadlessWorkspace.scala

Line 72 in acb5fb5

"directed-link-breed [directed-links directed-link]\n" +

?

[mrerrormessage]

Yeah, I changed that on a branch. It broke a bunch of tests (unsurprisingly). I'm trying to work out good names for the breeds. I initially used directed-lnks directed-lnk, but I don't like that because it looked too much like a typo. I'm thinking directed-edges directed-edge (undirected-edges undirected-edge) would communicate the intention well without typo confusion.

[nicolaspayette]

I'm thinking directed-edges directed-edge (undirected-edges undirected-edge)

Seems like a sensible choice.

[TheBizzle]

I support the third option. Anything else seems prone to breaking the principle of least surprise.

[mrerrormessage]

@nicolaspayette , Travis runs on this branch have uncovered a model in the models library which defines a breed prim overriding a built-in. Here is the model and the error:

• models/Sample Models/Social Science/Rebellion.nlogo: "org.nlogo.core.CompilerException: Defining a breed [AGENTS AGENT] redefines IS-AGENT?, a primitive reporter"

@qiemem , the following nw models will also need to be changed. I'm happy to do it, but wanted to let you know what models were changing and what changes I was making.

• extensions/nw/test/applet-test.nlogo: "org.nlogo.core.CompilerException: Defining a breed [DIRECTED-LINKS DIRECTED-LINK] redefines IS-DIRECTED-LINK?, a primitive reporter"
• extensions/nw/models/Network Builder.nlogo: "org.nlogo.core.CompilerException: Defining a breed [UNDIRECTED-LINKS UNDIRECTED-LINK] redefines IS-UNDIRECTED-LINK?, a primitive reporter"

I'm planning to change the breed names from links/link to edges/edge. Let me know if you'd prefer a different change.

[qiemem]

I'll make the change.

Now that I see the change, I kind of wish the primitives were named is-directed? and is-undirected?. Naming link breeds (un)directed-links is pretty common. I don't suppose you would want to change the primitive names via the autoconverter, would you?

[mrerrormessage]

Perhaps a rename would be worth discussing, but I still think this change is worth making, even (and perhaps especially) if it forces people to abandon undirected-link-breed [undirected-links undirected-link]. There are three types of models this affects:

• Case 1 - this is the only link breed declared. In this case the user can use the default link primitives.
• Case 2 - there are other undirected link breeds. In this case, the ambiguous meaning of the is-undirected-link? could genuinely affect models. The user would have no way of knowing what that primitive did. We should be prompting these users to change their models as they currently have unspecified behavior.
• Case 3 - there are other breeds, but no other undirected link breeds. In this case the user has to change the breed name, but is-undirected-link? continues to work as it worked before.

Having said all that, we should probably add some documentation in the link programming guide which covers this transition as well as making sure all our examples no longer have undirected-link-breed [undirected-links undirected-link].

[Idloj]

@mrerrormessage , Is this intended?

[mrerrormessage]

Nope, good catch!