This patch introduces a command line option for allowing only selected attributes to be seeded.
Do you mean returning a error if a empty --attributor-seed-allow-list option is present ?
Currently the size of list is being used to tell if a list is present or not.
I think I can use getNumOccurrences() to replace this behaviour .
I think cl::OneOrMore causes a error to be generated if the option is not specified.
That would make it mandatory to have a atleast one --attributor-seed-allow-list parameter for every invocation of
The cl::OneOrMore modifier indicates that the option must be specified at least one time.
As discussed, we can check for the validity of the list in the constructor. Two more comments below.
Use more words to describe this please.
Nit: Maybe initialize it directly here.
Get rid of all lines we don't need, e.g. the last 5.
- Better comments.
- Remove unneeded lines from the test.
I investigated the situation with the argument option that this patch introduces.
LLVM CommandLine library already does not allow a empty list but what happens is when you
pass a empty list for a execution like this opt --attribute-seed-allow-list
CommandLine automatically displays an error and terminates execution.
But when there are other arguments after the option the then that option becomes a member of the list.
It would have been great if we had a way of telling whether a name is valid or not.
Perhaps we could have a list of valid attribute names.
It would have been great if we had a way of telling whether a name is valid or not. Perhaps we could have a list of valid attribute names.
Let's not worry about that too much now. It seems an empty list doesn't make sense and when people pass garbage they'll get something.
One nit below and one idea but otherwise LGTM.
We could set the seeding to false if the list is empty to avoid looking up if it is empty all the time. Feel free to ignore.
Nit: called. \see ...