Skip to content
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

[ci skip] document that multiple yield in inline iterator cause code bloat #10553

Merged
merged 2 commits into from
Feb 5, 2019

Conversation

timotheecour
Copy link
Member

@timotheecour timotheecour commented Feb 4, 2019

  • this documents that multiple yield in inline iterator cause code bloat

it's probably by design and not a bug but should be documented as it's a gotcha; see example below:

/cc @Araq

iterator myiter(): int =
  yield 10
  yield 11

proc main()=
  for a in myiter():
    echo "done"
main()

generates with --embedsrc -d:release:

N_LIB_PRIVATE N_NIMCALL(void, main_audQMEKlkQTCwd9cxcZvVCg)(void) {
	{
		NI a;
		a = (NI)0;
//    for a in myiter():
		a = ((NI) 10);
//      echo "done"
		echoBinSafe(TM_12xtM2ZmWTyc1IoNCQwzsA_2, 1);
//    for a in myiter():
		a = ((NI) 11);
//      echo "done"
		echoBinSafe(TM_12xtM2ZmWTyc1IoNCQwzsA_2, 1);
	}
}

links

iterator myParentDirs(p: string): string =
  # XXX os's parentDirs is stupid (multiple yields) and triggers an old bug...

note

@Araq
Copy link
Member

Araq commented Feb 5, 2019

Well it's not like you cannot fight the code bloat in the codegen with some usages of goto so there is no inherent advantage of D's way. And what you call a "minor annoyance" for the iterator implementor gets ugly, quickly.

@Araq Araq merged commit 2b30998 into nim-lang:devel Feb 5, 2019
@timotheecour timotheecour deleted the pr_doc_iterator_yield_codesize branch February 5, 2019 20:18
@timotheecour timotheecour mentioned this pull request Feb 5, 2019
37 tasks
@timotheecour
Copy link
Member Author

timotheecour commented Feb 5, 2019

Well it's not like you cannot fight the code bloat in the codegen with some usages of goto

we should definitely, if it's possible to use goto

so there is no inherent advantage of D's way

we've talked about this; D ranges (which again, are feasible in nim without compiler support AFAIK) are 1st class; and I'll eventually make more progress in that direction when i find time

And what you call a "minor annoyance" for the iterator implementor gets ugly, quickly.

I did write * a bit more complex, not minor annoyance; yes that's the tradeoff. Without the above mentioned goto in inline iterator implementation to avoid code bloat, the "corrected" inline iterator that avoids multiple yield also has to model iteration state though (see #10557) ; with the "corrected" implementation using goto, well, let's see when we have that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants