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

Configs: use a uniform syntax without Match exceptions #507

Merged
merged 3 commits into from
Jan 13, 2017

Conversation

terpstra
Copy link
Contributor

The old style of specifying Configs used total functions. The only way to
indicate that a key was not matched was to throw an exception. Not only was
this a performance concern, but it also caused confusing error messages
whenever you had a match failure from a lookup within a lookup. The
exception could get handled by an outer-lookup that then reported the wrong
key as missing.

The old style of specifying Configs used total functions.  The only way to
indicate that a key was not matched was to throw an exception.  Not only was
this a performance concern, but it also caused confusing error messages
whenever you had a match failure from a lookup within a lookup.  The
exception could get handled by an outer-lookup that then reported the wrong
key as missing.
@hcook
Copy link
Member

hcook commented Jan 12, 2017

Closes #506

@hcook hcook self-assigned this Jan 12, 2017
Copy link
Member

@hcook hcook left a comment

Choose a reason for hiding this comment

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

No one was using the Map[Any,Any] part of the API anyways.

@jchang0
Copy link
Contributor

jchang0 commented Jan 13, 2017

Can we do some kind of depreciation for CDEMatchError instead of removing it altogether for the time being? (not even sure if it's possible to depreciate a class)

@terpstra
Copy link
Contributor Author

We could, but it is only used in Config definitions, which need to be refactored anyway.

@hcook
Copy link
Member

hcook commented Jan 13, 2017

To expand on @terpstra's comment, all instances of
class BasePlatformConfig extends Config(
(pname,site,here) => pname match {
case Foo => 2
case _ => throw new CDEMatchError
})
become
class BasePlatformConfig extends Config( { (site,here up) =>
case Foo => 2
})
This is a mandatory backwards-incompatible upgrade, one of several coming this week :D

@hcook hcook merged commit 52bb6cd into master Jan 13, 2017
@colinschmidt
Copy link
Contributor

Wait does the config package not use exceptions for control flow?

edwardcwang added a commit to edwardcwang/rocket-chip that referenced this pull request Feb 8, 2017
@hcook hcook deleted the no-throw-configs branch June 1, 2017 01:05
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

4 participants