Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[FrameworkBundle] add constraint validators before optimizations#30090
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
xabbuh commentedFeb 6, 2019
| Q | A |
|---|---|
| Branch? | 3.4 |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #24222 |
| License | MIT |
| Doc PR |
nicolas-grekas commentedFeb 7, 2019
I'm not sure, but can that break anything? Like compiler passes that add the tag? Aren't all compiler passes that use Sorry that's a lot of questions, but I'm always suspicious with these kind of changes :) |
xabbuh commentedFeb 7, 2019
That's for sure possible. That would have to be solved by running custom compiler passes with a higher priority. I am afraid there is not much we can do otherwise if we want to solve the linked issue.
That was my initial solution for this (see#24223), but in fact that's not the issue we need to solve here (while I still think we could possibly improve existing compiler passes in such a way). |
nicolas-grekas commentedFeb 7, 2019
So, this issue can happen with any pass, isn't it? |
xabbuh commentedFeb 7, 2019
Not really, because all (if I didn't miss any) our other compiler passes dealing with tags are already registered at the before optimization stage so decorating them works just fine. IMO this is not a new use case. Decorating used to work fine in the past until we accidentally broke that by moving the compiler pass. |
nicolas-grekas left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
OK, let's do this,#20116 contains no details anyway...
fabpot commentedFeb 13, 2019
Thank you@xabbuh. |
…ations (xabbuh)This PR was merged into the 3.4 branch.Discussion----------[FrameworkBundle] add constraint validators before optimizations| Q | A| ------------- | ---| Branch? | 3.4| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#24222| License | MIT| Doc PR |Commits-------05e0e16 add constraint validators before optimizations