Skip to content

Conversation

@felixmulder
Copy link
Contributor

Review: @smarter

@DarkDimius
Copy link
Contributor

DarkDimius commented Oct 18, 2016

nice title.

@felixmulder felixmulder changed the title Fix #1605: don't methods that have errors Fix #1605: don't inline methods that have errors Oct 18, 2016
@felixmulder
Copy link
Contributor Author

It inlined it ;)

@felixmulder felixmulder force-pushed the topic/fix-inline-untyped branch from 9fff5e9 to 678f2b7 Compare October 18, 2016 14:18
@smarter
Copy link
Member

smarter commented Oct 18, 2016

I think I would put the check at the place where we call makeInlineable in registerInlineInfo instead of deep into makeInlineable itself.

@felixmulder felixmulder force-pushed the topic/fix-inline-untyped branch 2 times, most recently from 50b99e4 to f0cfb25 Compare October 18, 2016 15:19
@felixmulder
Copy link
Contributor Author

@smarter - done

@felixmulder felixmulder force-pushed the topic/fix-inline-untyped branch from f0cfb25 to 5176b04 Compare November 7, 2016 12:13
val inlineCtx = ctx
sym.updateAnnotation(LazyBodyAnnotation { _ =>
implicit val ctx: Context = inlineCtx
val tree1 = treeExpr(ctx)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better use reporter.errorCount

@felixmulder felixmulder force-pushed the topic/fix-inline-untyped branch from 5176b04 to 1fec582 Compare November 7, 2016 13:05
implicit val ctx: Context = inlineCtx
val tree1 = treeExpr(ctx)
makeInlineable(tree1)
ctx.withNoError(treeExpr(ctx))(makeInlineable)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@odersky - updated with your suggestion

@odersky
Copy link
Contributor

odersky commented Nov 7, 2016

LGTM

@odersky odersky merged commit 5cef7a9 into scala:master Nov 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants