-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
checker,cgen: fix duplicates in generic sumtype #20936
base: master
Are you sure you want to change the base?
Conversation
vlib/v/checker/checker.v
Outdated
if print_notice { | ||
c.note('duplicate type: ${msg}', node.pos) | ||
} else { | ||
c.error('duplicate type: ${msg}', node.pos) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm, there is no existing precedent for using an attribute for toggling between an error and a notice 🤔. I am not sure if it is worth the complexity.
Would not either just a notice, or either just an error be simpler?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sometimes this is bad idea to give error. Consider that:
firstapi
:
pub struct Row[T] {
pub mut:
// children components
components []Component[T]
}
@[on_duplicate: 'T is present in sumtype']
@[notice_if_duplicate]
pub type Component[T] = Row[Button]
| Button
| StringSelect
| TextInput
| T
secondapi
:
struct Default[T] {
v T
}
pub fn c[T]() Component[T] {
return Component[T](Default[T]{}.v)
}
user (not dev of firstapi
and secondapi
packages), not knowing that e.g. TextInput
is present in Component
type, uses c[TextInput]()
, and gets an notice, instead of hard error. That also would allow deprecating duplicates in generic sumtypes.
Yet one example:
struct Undefined {}
@[on_duplicate: 'Sentinels are internal']
pub type UndefinedOr[T] = Undefined | T
That would give a error if user will do UndefinedOr[Undefined]
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@medvednikov what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @spytheman here. This complicates the language and gives too many options. I'd just stick with one option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed attribute and made error
by default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are still 2 attributes:
ignore_generic_duplicates
and on_generic_duplicate
.
I do not know how to document/explain that in doc/docs.md
.
Stale |
Sorry, we'll figure something out to merge this. |
I don't think using attributes or custom error messages is a good idea. |
Name[int](123)
, whentype Name[T] = string | int | T
#19159Note: you can control, whether to raise a error, notice or ignore if user used type, which is present as T in generic sumtype