Issue729

Title option parser: prevent defining multiple plugins with the same name but different types
Priority wish Status resolved
Superseder Nosy List florian, jendrik, malte
Assigned To jendrik Keywords
Optional summary

Created on 2017-05-08.11:44:41 by jendrik, last changed by jendrik.

Messages
msg6386 (view) Author: jendrik Date: 2017-05-22.11:58:28
I copied the plans for detecting plugin name collisions to the option parser meta 
issue (issue588) and merged the current fix.
msg6384 (view) Author: malte Date: 2017-05-21.15:41:42
In terms of the code architecture, it is not intuitive to have the name
collision test associated with the DocStore, which is about documentation, while
this issue doesn't have anything to do with documentation conceptually.

It's also not ideal that the current code is non-deterministic in the sense that
if there are multiple name collisions, we have no way to predict which one will
be reported. (Although I expect that it will be deterministic for any given
compile since global object initialization order is probably be fixed by the
compile.)

A better solution would list all conflicts, in a deliberate order, and as
Florian says it should also mention the types. But this would require a more
sophisticated two-state initialization where we first collect all plugins and
then look at them in more detail.

I don't mind going ahead with the current fix if we keep track of the following
longer design TODOs for the option parser:

1) Put the test that no two plug-ins have the same name in a more central place
of the option parser code rather than in the DocStore.

2) Change option parser design to do a two-phase registration of plug-ins, so
that we can first collect all of them, then do global checks against duplicates
etc., and then proceed with the later parts of registration at a stage where we
know that all plug-ins (including type plug-ins) have been initialized.

3) Report name collisions of plug-ins in a well-defined order, e.g.
alphabetically by name and for each name alphabetically listing the types.
msg6383 (view) Author: jendrik Date: 2017-05-21.09:05:21
I made a small pull request at https://bitbucket.org/jendrikseipp/downward/pull-requests/71 .

Malte, could you have a look, please?
msg6375 (view) Author: malte Date: 2017-05-18.14:23:19
No objections.
msg6374 (view) Author: jendrik Date: 2017-05-18.14:20:59
Then we need to decide how to resolve the name clash between PatternGenerator 
"manual" and PatternCollectionGenerator "manual". What about PatternGenerator 
"manual_pattern" and PatternCollectionGenerator "manual_patterns"?
msg6373 (view) Author: malte Date: 2017-05-18.14:17:13
To have some record of the reasons for this design decision, the following
points were raised for/against globally unique plugin names.

For:

1. It may reduce confusion if a plug-in object definition can be interpreted
without knowing about the types of the context in which it is used. For example,
if we write

--search my_search_algorithm(foo=xyzzy(),bar=xyzzy()),

it is nice to know that the two xyzzies refer to the same kind of thing.
(Currently they can be arbitrarily different things if foo and bar have
different types.)

2. It allows us to be closer to Python syntax (or more generally: typical
programming language syntax) than we currently are. For example, rather than the
current
    --heuristic h=ff()
    --search eager_greedy([h],preferred=[h])
we could write
    h=ff()
    eager_greedy([h],preferred=[h])
which can be interpreted as straight Python code. This is particularly
interesting because there are many people interested in eventually seeing the
planner embeddable in a library-like way (e.g. into Python).

3. Two different things with the same name may be seen as an anti-feature
because they make it harder to search for something, find out what is going on, etc.

4. We can eventually use the plug-in names directly to enable/disable things in
CMake without requiring further naming conventions (e.g. "PLUGIN_ASTAR", not
"PLUGIN_SEARCH_ASTAR". Without unique names, we'd have to include type
information here.

Against:

5. We will have to use longer names that lead to redundant-looking command-line
syntax in many cases (e.g. "shrink_strategy=shrink_bisimulation(...)").

6. It is easier to step on someone else's toes by "using up" a name in the
global namespace.


If there are further major points, feel free to add them.
msg6372 (view) Author: florian Date: 2017-05-18.14:10:39
We decided to enforce globally unique names. I updated the issue title to
reflect this.
msg6356 (view) Author: jendrik Date: 2017-05-08.11:51:04
Good point. I added this to our meeting agenda.
msg6355 (view) Author: malte Date: 2017-05-08.11:48:04
I think we should discuss first (with everyone) whether we actually want to
allow using the same name for multiple things. Not allowing this would have
certain advantages too. For example, it would make it easier to pick unique
namespaces for plug-ins, it would make it easier to search for plug-ins by name
(in our documentation or when using --help), and instead of "--heuristic
h=ff()", we could support the syntax "h=ff()", which would make our command-line
syntax simpler and more uniform. (Then our command line would essentially be a
Python program.)
msg6354 (view) Author: jendrik Date: 2017-05-08.11:44:41
Currently, the option parser supports using the same plugin name for different 
plugins of different types (e.g., PatternGenerator "manual" and 
PatternCollectionGenerator "manual"). However, the DocStore which stores the 
documentation ignores the types of plugins when storing information. It should take 
the plugin type into account.
History
Date User Action Args
2017-05-22 11:58:28jendriksetstatus: reviewing -> resolved
messages: + msg6386
2017-05-21 15:41:42maltesetmessages: + msg6384
2017-05-21 09:05:21jendriksetstatus: chatting -> reviewing
messages: + msg6383
2017-05-18 14:25:58jendriksetassignedto: jendrik
2017-05-18 14:23:19maltesetmessages: + msg6375
2017-05-18 14:20:59jendriksetmessages: + msg6374
2017-05-18 14:17:13maltesetmessages: + msg6373
2017-05-18 14:10:39floriansetmessages: + msg6372
title: option parser: allow storing documentation for multiple plugins with the same name but different types -> option parser: prevent defining multiple plugins with the same name but different types
2017-05-08 11:51:04jendriksetpriority: bug -> wish
messages: + msg6356
2017-05-08 11:48:04maltesetstatus: unread -> chatting
messages: + msg6355
2017-05-08 11:44:41jendrikcreate