-
-
Notifications
You must be signed in to change notification settings - Fork 864
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
Segmentation Fault (core dumped) #6501
Comments
Looks like problem is with the |
@canvural Ah you are quite correct thank you. I've updated the description accordingly and will continue my own debugging from there. |
OK, I've narrowed it down to the code currently in the description. If you change the line: * @template R of AttributeOption|ProductOption to: * @template R of OptionInterface then it works as expected. So it appears to be caused by the union type. |
segmentation faults usually are stemming from php-src or php-extension bugs. which versions do you use? you should consider updating if its not up 2 date. |
@staabm It occurs on the PHPStan playground, so same as that one (PHP 8.1.2). |
@staabm It's gonna be a bug in PHPStan coming from infinite recursion... |
The segfault is caused by this commit: phpstan/phpstan-src@f8af5dc (phpstan/phpstan-src#955). |
This is the loop:
|
It happens when Do you have any advice for the right fix @arnaud-lb ? :) Thanks. |
We could revert the commit and then do something like this: diff --git a/src/Type/IntersectionType.php b/src/Type/IntersectionType.php
index 32331d84b..46ae22bb2 100644
--- a/src/Type/IntersectionType.php
+++ b/src/Type/IntersectionType.php
@@ -21,6 +21,7 @@ use PHPStan\Type\Accessory\AccessoryType;
use PHPStan\Type\Accessory\NonEmptyArrayType;
use PHPStan\Type\Generic\TemplateTypeMap;
use PHPStan\Type\Generic\TemplateTypeVariance;
+use PHPStan\Type\Generic\TemplateUnionType;
use function array_map;
use function count;
use function implode;
@@ -96,7 +97,10 @@ class IntersectionType implements CompoundType
public function isSubTypeOf(Type $otherType): TrinaryLogic
{
- if ($otherType instanceof self || $otherType instanceof UnionType) {
+ if (
+ ($otherType instanceof self || $otherType instanceof UnionType)
+ && !$otherType instanceof TemplateUnionType
+ ) {
return $otherType->isSuperTypeOf($this);
}
@@ -110,7 +114,10 @@ class IntersectionType implements CompoundType
public function isAcceptedBy(Type $acceptingType, bool $strictTypes): TrinaryLogic
{
- if ($acceptingType instanceof self || $acceptingType instanceof UnionType) {
+ if (
+ ($acceptingType instanceof self || $acceptingType instanceof UnionType)
+ && !$acceptingType instanceof TemplateUnionType
+ ) {
return $acceptingType->isSuperTypeOf($this);
}
Alternatively, if we don't want IntersectionType to have special cases about TemplateType, we could do something like this: diff --git a/src/Type/Generic/TemplateTypeTrait.php b/src/Type/Generic/TemplateTypeTrait.php
index 387587c5a..1a7b9bf33 100644
--- a/src/Type/Generic/TemplateTypeTrait.php
+++ b/src/Type/Generic/TemplateTypeTrait.php
@@ -3,6 +3,7 @@
namespace PHPStan\Type\Generic;
use PHPStan\TrinaryLogic;
+use PHPStan\Type\Generic\TemplateUnionType;
use PHPStan\Type\IntersectionType;
use PHPStan\Type\MixedType;
use PHPStan\Type\Type;
@@ -121,10 +122,19 @@ trait TemplateTypeTrait
public function isSuperTypeOf(Type $type): TrinaryLogic
{
- if ($type instanceof self || $type instanceof IntersectionType) {
+ if ($type instanceof self) {
return $type->isSubTypeOf($this);
}
+ if ($this instanceof TemplateUnionType && $type instanceof IntersectionType) {
+ $results = [];
+ foreach ($type->getTypes() as $innerType) {
+ $results[] = $innerType->isSuperTypeOf($this);
+ }
+
+ return TrinaryLogic::createYes()->maxMin(...$results);
+ }
+
return $this->getBound()->isSuperTypeOf($type)
->and(TrinaryLogic::createMaybe());
} I would prefer this one, so that the logic related to template types remains encapsulated in TemplateTypeTrait. (I haven't tested any of these thoroughly yet) |
Well I fixed it but I'm not happy about it: phpstan/phpstan-src@d101764 To me this seems like guessing "let's put an if somewhere and see if it works" rather than making a change that makes sense :) But I don't know how to structure the code to prevent problems like this... |
@ondrejmirtes Thank you for the fix. |
Agreed, that's not ideal. I think that it's the correct way to fix this in the current design, though. Infinite recursions can happen when calling isSuperTypeOf from isSubTypeOf (which may call back isSuperTypeOf). Usually we don't call isSuperTypeOf from isSubTypeOf, which avoids infinite recursions. But some types may still need to do it (maybe especially CompoundTypes). From what I've seen, they may call isSuperTypeOf in two cases:
An example of the first case is UnionType doing this in isSubTypeOf:
This is a isSuperTypeOf call with an inner type as $otherType, which is safe. An example of the second case is CallableType doing this in isSubTypeOf:
CallableType relies on the fact that is knows that IntersectionType and UnionType not call back isSubTypeOf on the CallableType. The second case is definitely more dangerous and brittle because it relies on assumptions about the behavior of other types.
It turns out that TemplateUnionType is a UnionType, and I broke this assumption by allowing TemplateUnionType::isSuperTypeOf($otherType) to call $otherType->isSubTypeOf() on IntersectionType. Your commit fixes this by excluding TemplateUnionType from this assumption, which is a valid fix. An alternative fix would be to prevent TemplateType::isSuperTypeOf($otherType) from calling isSubTypeOf() on IntersectionType, like in the second part of my comment above. I wonder if we could be able to make a Rule that detects infinite recursions in Type :) I believe that there is a common issue between the original bug #6210, as well as the currnet one, and many other template-related issues, though. It is that template types inherit from their bound type. This causes template types to pass as their parent type, although they don't always behave like them (on purpose), and can not be substituted for their parent type. All bugs that are resolved by a It appears to be easy to introduce generics bugs in unrelated code because of this. I think that this could be solved by not inheriting from the bound type, and instead represent template types with a bound as |
Thank you for your analysis! :)
What I'd like to do is to disallow any This means that instead of doing Once we do that, we can decouple things so that there doesn't need to be separate Of course the downside is that it's a lot of work, there's a lot of use-cases that need a separate method so I think the Type interface might grow by dozens or maybe even hundreds of methods. It might also be the only instance in the history of programming where it's the right solution 😂 It's one of the things I'd like to see in PHPStan 2.0, so we have a few years to incrementally achieve it :) |
This sounds like a great plan ! |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Bug report
When analysing a file with the following contents, I get a Segmentation Fault (using PHPStan 1.4.4, which didn't appear on 1.4.3).
Code snippet that reproduces the problem
The demo isn't able to give me a shareable link as it errors. I tried to reduce the amount of code from this class to isolate the exact issue but removing various methods appears to make the fault go away, so here is the code:
Expected output
Show analysis output.
Did PHPStan help you today? Did it make you happy in any way?
PHPStan making the world a brighter place one error at a time.
The text was updated successfully, but these errors were encountered: