Skip to content

Fix/#34 pathresolvers #38

Merged
merged 2 commits into from Mar 3, 2014

3 participants

@odersky
odersky commented Mar 3, 2014

See explanation in commits.

Review by @DarkDimius, @srjd

odersky added some commits Mar 3, 2014
@odersky odersky Add test case for #34
Right now this one fails.
f0d5662
@odersky odersky Fix of #34
The root cause of #34 was that we took a type argument which was an existential type. These are returned as type bounds, which make no sense in the calling context. To avoid that problem in the future, `typeArgs`
got renamed to `argInfos`, so it is clear we get an info, not necessarily a value type. There are
also added method `argTypes`, `argTypesLo`, `argTypesHi`, which return a type, but either throw an exception or return a lower/upper approximation of the argument is an existential type.

There's another issue that the existential type only arose when compiling the same couple fo files the seciond time. We need to chase that one down separately.
f3dacf9
@sjrd
Programming Methods Laboratory EPFL member

Is this trace supposed to stay enabled?

@sjrd
Programming Methods Laboratory EPFL member
sjrd commented on f0d5662 Mar 3, 2014

Aren't test cases supposed to be included in the commit that fixes them?

@sjrd
Programming Methods Laboratory EPFL member
sjrd commented Mar 3, 2014

Otherwise OK for me.

@odersky
odersky commented Mar 3, 2014

The trace can stay enabled. We can include the test case in the commit the next time. for now I would prefer to just get the build back to work, so I am not resubmitting.

@DarkDimius
Programming Methods Laboratory EPFL member

testIssue_34 seems to tests this particular issue. https://github.com/lampepfl/dotty/pull/38/files#diff-28f8d722cae3da74d903fc63040c9d36R74
Though it would be good to come up with minimized test that doesn't depend on "tools/dotc/config/PathResolver.scala" and "tools/dotc/config/Properties.scala", which can be modified in several years making tests useless.

LGTM, as a quickfix for #34

@DarkDimius DarkDimius merged commit 9c73c96 into lampepfl:master Mar 3, 2014

1 check passed

Details default The Travis CI build passed
@odersky
odersky commented Mar 3, 2014

Yes, it would be good to minimize the test case further. The other thing we still need to do is find out why the problem manifested itself only on a resident recompile. That should not be the case. For that reason also it's good to leave the failing test case in a separate commit. Easier to come back to it that way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.