8000 [DependencyInjection] Removed unreachable code by unkind · Pull Request #12072 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[DependencyInjection] Removed unreachable code #12072

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

Merged

Conversation

unkind
Copy link
Contributor
@unkind unkind commented Sep 28, 2014
Q A
Bug fix? not really
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

According to undefined $e, this is an unreachable code. As I can see, circular references are handled by parent method.

@hacfi
Copy link
Contributor
hacfi commented Sep 29, 2014

@unkind Did you find this because your ide reported this? The code might be wrong but removing this entirely is not a good idea imho!

@unkind
Copy link
Contributor Author
unkind commented Sep 29, 2014

I cannot find any case when interpreter hits this code, feel free to correct me if you think it breaks BC somehow.

@hacfi
Copy link
Contributor
hacfi commented Sep 29, 2014

After debugging this I think @unkind is right.

3 lines before parent::get is called which does the same check:

if (isset($this->loading[$id])) {

@stof
Copy link
Member
stof commented Sep 29, 2014

this indeed looks like dead code. 👍

@fabpot
Copy link
Member
fabpot commented Sep 29, 2014

Good catch, thanks @unkind.

@fabpot fabpot merged commit ecedea2 into symfony:master Sep 29, 2014
fabpot added a commit that referenced this pull request Sep 29, 2014
This PR was merged into the 2.6-dev branch.

Discussion
----------

[DependencyInjection] Removed unreachable code

| Q             | A
| ------------- | ---
| Bug fix?      | not really
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

According to undefined $e, this is an unreachable code. As I can see, circular references are handled by parent method.

Commits
-------

ecedea2 [DependencyInjection] Removed unreachable code
@unkind unkind deleted the refactoring-dic-remove-unreachable-code branch September 29, 2014 11:45
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.

4 participants
0