-
-
Notifications
You must be signed in to change notification settings - Fork 669
fix Issue 18026 - Stack overflow in dmd/dtemplate.d:6248, TemplateInstance::needsCodegen() #14787
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
Changes from all commits
ce7c5df
5002893
1ad92d7
2128aad
2a993e3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6255,33 +6255,46 @@ extern (C++) class TemplateInstance : ScopeDsymbol | |
| this.tnext = null; | ||
| this.tinst = null; | ||
|
|
||
| if (errors || (inst && inst.isDiscardable())) | ||
| // Don't do codegen if the instance has errors, | ||
| // is a dummy instance (see evaluateConstraint), | ||
| // or is determined to be discardable. | ||
| if (errors || inst is null || inst.isDiscardable()) | ||
| { | ||
| minst = null; // mark as speculative | ||
| return false; | ||
| } | ||
|
|
||
| // This should only be called on the primary instantiation. | ||
| assert(this is inst); | ||
|
|
||
| if (global.params.allInst) | ||
| { | ||
| // Do codegen if there is an instantiation from a root module, to maximize link-ability. | ||
|
|
||
| // Do codegen if `this` is instantiated from a root module. | ||
| if (minst && minst.isRoot()) | ||
| return true; | ||
|
|
||
| // Do codegen if the ancestor needs it. | ||
| if (tinst && tinst.needsCodegen()) | ||
| static ThreeState needsCodegenAllInst(TemplateInstance tithis, TemplateInstance tinst) | ||
| { | ||
| minst = tinst.minst; // cache result | ||
| assert(minst); | ||
| assert(minst.isRoot()); | ||
| return true; | ||
| // Do codegen if `this` is instantiated from a root module. | ||
| if (tithis.minst && tithis.minst.isRoot()) | ||
| return ThreeState.yes; | ||
|
|
||
| // Do codegen if the ancestor needs it. | ||
| if (tinst && tinst.inst && tinst.inst.needsCodegen()) | ||
| { | ||
| tithis.minst = tinst.inst.minst; // cache result | ||
| assert(tithis.minst); | ||
| assert(tithis.minst.isRoot()); | ||
| return ThreeState.yes; | ||
| } | ||
| return ThreeState.none; | ||
| } | ||
|
|
||
| if (const needsCodegen = needsCodegenAllInst(this, tinst)) | ||
| return needsCodegen == ThreeState.yes ? true : false; | ||
|
|
||
| // Do codegen if a sibling needs it. | ||
| if (tnext) | ||
| for (; tnext; tnext = tnext.tnext) | ||
| { | ||
| if (tnext.needsCodegen()) | ||
| const needsCodegen = needsCodegenAllInst(tnext, tnext.tinst); | ||
| if (needsCodegen == ThreeState.yes) | ||
| { | ||
| minst = tnext.minst; // cache result | ||
| assert(minst); | ||
|
|
@@ -6291,8 +6304,10 @@ extern (C++) class TemplateInstance : ScopeDsymbol | |
| else if (!minst && tnext.minst) | ||
| { | ||
| minst = tnext.minst; // cache result from non-speculative sibling | ||
| return false; | ||
| // continue searching | ||
| } | ||
| else if (needsCodegen != ThreeState.none) | ||
| break; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Codecov says this is not covered, is it Codecov's issue or really not covered ?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's really not covered, because the interior function never returns I left it in to guard us in case that were to change in the future, and also to align with the non-allinst branch. I think these two branches could be factored because the outer shell is mostly a duplicate now, but I'll leave that for something to look at in far future - maybe one of the strategies will have to go anyway because missing symbols is one of the "big problems" at the moment, and the default strategy to prune prune prune is a massive PITA that isn't helping any cause. |
||
| } | ||
|
|
||
| // Elide codegen because there's no instantiation from any root modules. | ||
|
|
@@ -6317,31 +6332,39 @@ extern (C++) class TemplateInstance : ScopeDsymbol | |
| * => Elide codegen if there is at least one instantiation from a non-root module | ||
| * which doesn't import any root modules. | ||
| */ | ||
|
|
||
| // If the ancestor isn't speculative, | ||
| // 1. do codegen if the ancestor needs it | ||
| // 2. elide codegen if the ancestor doesn't need it (non-root instantiation of ancestor incl. subtree) | ||
| if (tinst) | ||
| static ThreeState needsCodegenRootOnly(TemplateInstance tithis, TemplateInstance tinst) | ||
| { | ||
| const needsCodegen = tinst.needsCodegen(); // sets tinst.minst | ||
| if (tinst.minst) // not speculative | ||
| // If the ancestor isn't speculative, | ||
| // 1. do codegen if the ancestor needs it | ||
| // 2. elide codegen if the ancestor doesn't need it (non-root instantiation of ancestor incl. subtree) | ||
| if (tinst && tinst.inst) | ||
| { | ||
| minst = tinst.minst; // cache result | ||
| return needsCodegen; | ||
| tinst = tinst.inst; | ||
| const needsCodegen = tinst.needsCodegen(); // sets tinst.minst | ||
| if (tinst.minst) // not speculative | ||
| { | ||
| tithis.minst = tinst.minst; // cache result | ||
| return needsCodegen ? ThreeState.yes : ThreeState.no; | ||
| } | ||
| } | ||
|
|
||
| // Elide codegen if `this` doesn't need it. | ||
| if (tithis.minst && !tithis.minst.isRoot() && !tithis.minst.rootImports()) | ||
| return ThreeState.no; | ||
|
|
||
| return ThreeState.none; | ||
| } | ||
|
|
||
| // Elide codegen if `this` doesn't need it. | ||
| if (minst && !minst.isRoot() && !minst.rootImports()) | ||
| return false; | ||
| if (const needsCodegen = needsCodegenRootOnly(this, tinst)) | ||
| return needsCodegen == ThreeState.yes ? true : false; | ||
|
|
||
| // Elide codegen if a (non-speculative) sibling doesn't need it. | ||
| if (tnext) | ||
| for (; tnext; tnext = tnext.tnext) | ||
| { | ||
| const needsCodegen = tnext.needsCodegen(); // sets tnext.minst | ||
| const needsCodegen = needsCodegenRootOnly(tnext, tnext.tinst); // sets tnext.minst | ||
| if (tnext.minst) // not speculative | ||
| { | ||
| if (!needsCodegen) | ||
| if (needsCodegen == ThreeState.no) | ||
| { | ||
| minst = tnext.minst; // cache result | ||
| assert(!minst.isRoot() && !minst.rootImports()); | ||
|
|
@@ -6350,8 +6373,10 @@ extern (C++) class TemplateInstance : ScopeDsymbol | |
| else if (!minst) | ||
| { | ||
| minst = tnext.minst; // cache result from non-speculative sibling | ||
| return true; | ||
| // continue searching | ||
| } | ||
| else if (needsCodegen != ThreeState.none) | ||
| break; | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| // https://issues.dlang.org/show_bug.cgi?id=18026 | ||
| bool f(T)(T x) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| static foreach(i; 0..60000) | ||
| { | ||
| static if(f(i)) | ||
| { | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.