Allow SimpleLiteral to be a vararg #143
Conversation
Allow SimpleLiteral to be a vararg
|
|
||
| public SimpleLiteral(T[] values) { | ||
| public SimpleLiteral(T... values) { |
There was a problem hiding this comment.
Won't this create issues for SyntaxParser at line 538? Those lines over there should be changed as well. Also, should the other constructor be removed as well or can it make a difference? I'm not experienced enough with Java generics to know if that constructor should exist or not (because it already has a varargs parameter).
There was a problem hiding this comment.
It shouldn't, but it'll remove that whole Array newInstance. Epic. I will get around to changing that soon.
…pleLiteral Vararg generics are just not the way to go in Java and the code was not working when you'd use it as expected. The original changes were therefore reverted, but there was still a small change left in the SimpleLiteral class.
|
Sorry to hijack this pull request like this. My original intention was to just add the changes I proposed, but then I saw that the proposal itself is just not a good idea because of the way varargs and generics are handled (there's a reason the original author Syst3ms) added a specific constructor that needed a class argument as well). That said, I still made some minor changes, hence why I'm still merging the pull request. |
Allow SimpleLiteral to be a vararg for accepting a single value.