-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
fixes #11225; generic sandwich problems; [backport:1.2] #17255
Conversation
doc/manual.rst
Outdated
@@ -5118,6 +5118,50 @@ scope is the default. | |||
``bind`` statements only make sense in templates and generics. | |||
|
|||
|
|||
Delegating mixin statements |
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.
- this example has many bugs (symbols should be exported,
var O
instead ofO
, wrong import, etc) - after fixing those locally, the example works both before and after PR, so it's probably not the best example
- in future work, to avoid these problems (code block out of sync because not tested) we need HAR (see https://github.com/marler8997/har, I ported it to nim, see example har file here: https://github.com/timotheecour/vitanim/blob/master/testcases/har/test1.har) which can be serialized/deserialized to disk analog to
tar
, but human readable. somerunnableExamples
, tests, and some github issues would benefit from it whenever more than 1 test file are needed.
(=> HAR: human readable archive timotheecour/Nim#637)
compiler/cgen.nim
Outdated
@@ -860,7 +860,7 @@ proc containsResult(n: PNode): bool = | |||
for i in 0..<n.safeLen: | |||
if containsResult(n[i]): return true | |||
|
|||
const harmless = {nkConstSection, nkTypeSection, nkEmpty, nkCommentStmt, nkTemplateDef, nkMacroDef} + | |||
const harmless = {nkConstSection, nkTypeSection, nkEmpty, nkCommentStmt, nkTemplateDef, nkMacroDef, nkMixinStmt} + |
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.
shouldn't nkMixinStmt
be stripped away in semantic pass before it reaches backend, eg, so that it doesn't affect each backend
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.
Maybe, but I'm generally in favor of not losing information in the AST. As you can see there are always things that backends explicitly need to ignore anyway.
Merging this for now as most changes are related to handle the nkMixinStmt, nkBindStmt in the backends which is worthwhile no matter what we end up with. |
So if I understand correctly the |
# a.nim
iterator items(a: int): int = discard
proc p*(a: int | uint) =
# for _ in a.items: discard # ok
bind items # doesn't help
for _ in a: discard # still a bug
# b.nim
import a
p 1 still gives
Regarding |
…lang#17255) * fixes nim-lang#11225; generic sandwich problems; [backport:1.2] * progress * delegating these symbols must be done via 'bind'
…lang#17255) * fixes nim-lang#11225; generic sandwich problems; [backport:1.2] * progress * delegating these symbols must be done via 'bind'
potential root cause for #17704 regression |
No description provided.