<div dir="ltr">Thanks Richard!!</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Nov 7, 2013 at 1:18 AM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Committed with a few changes as r194161. Thanks!<br><div class="gmail_extra"><br><br><div class="gmail_quote">
<div><div class="h5">On Wed, Nov 6, 2013 at 9:20 AM, Rahul Jain <span dir="ltr"><<a href="mailto:1989.rahuljain@gmail.com" target="_blank">1989.rahuljain@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">Ping.</div><div><div><div class="gmail_extra">
<br><br><div class="gmail_quote">On Tue, Oct 29, 2013 at 1:12 PM, Rahul Jain <span dir="ltr"><<a href="mailto:1989.rahuljain@gmail.com" target="_blank">1989.rahuljain@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div><div><div><div><div><div>Hi Richard,<br>
<br></div>Thanks for you valuable inputs. <br><br></div>I have reworked the patch barring just one particular change you suggested. <br>
</div>Here is the patch.<br>
<br>Index: lib/Frontend/CompilerInvocation.cpp<br>
===================================================================<br>--- lib/Frontend/CompilerInvocation.cpp (revision 193583)<div><br>+++ lib/Frontend/CompilerInvocation.cpp (working copy)<br>@@ -1313,6 +1313,8 @@<br>
Opts.MathErrno = !Opts.OpenCL && Args.hasArg(OPT_fmath_errno);<br> Opts.InstantiationDepth =<br> getLastArgIntValue(Args, OPT_ftemplate_depth, 256, Diags);<br>+ Opts.ArrowDepth =<br>+ getLastArgIntValue(Args, OPT_foperator_arrow_depth, 2048, Diags);<br>
Opts.ConstexprCallDepth =<br> getLastArgIntValue(Args, OPT_fconstexpr_depth, 512, Diags);<br> Opts.ConstexprStepLimit =<br></div>Index: lib/Sema/SemaExprCXX.cpp<br>===================================================================<br>
--- lib/Sema/SemaExprCXX.cpp (revision 193583)<div><br>+++ lib/Sema/SemaExprCXX.cpp (working copy)<br>@@ -5189,7 +5189,17 @@<br> SmallVector<SourceLocation, 8> Locations;<br> CTypes.insert(Context.getCanonicalType(BaseType));<br>
<br></div>+ unsigned int ArrowInstantiationCount = 0;<br> while (BaseType->isRecordType()) {<br>+ ArrowInstantiationCount++;<br>+ if (ArrowInstantiationCount >= getLangOpts().ArrowDepth) {<div>
<br>+ Diag(OpLoc, diag::err_arrow_instantiation_depth_exceeded)<br>
+ << getLangOpts().ArrowDepth << Base->getSourceRange();<br>+ Diag(OpLoc, diag::note_arrow_instantiation_depth)<br>+ << getLangOpts().ArrowDepth;<br>+ return ExprError();<br>
</div><div>
+ }<br>+<br> Result = BuildOverloadedArrowExpr(<br> S, Base, OpLoc,<br> // When in a template specialization and on the first loop iteration,<br></div>Index: lib/Driver/Tools.cpp<br>===================================================================<br>
--- lib/Driver/Tools.cpp (revision 193583)<div><br>+++ lib/Driver/Tools.cpp (working copy)<br>@@ -2802,6 +2802,11 @@<br> CmdArgs.push_back(A->getValue());<br> }<br> <br>+ if (Arg *A = Args.getLastArg(options::OPT_foperator_arrow_depth_EQ)) {<br>
+ CmdArgs.push_back("-foperator-arrow-depth");<br>+ CmdArgs.push_back(A->getValue());<br>+ }<br>+<br> if (Arg *A = Args.getLastArg(options::OPT_fconstexpr_depth_EQ)) {<br> CmdArgs.push_back("-fconstexpr-depth");<br>
CmdArgs.push_back(A->getValue());<br></div>Index: include/clang/Basic/LangOptions.def<br>===================================================================<br>--- include/clang/Basic/LangOptions.def (revision 193583)<div>
<br>
+++ include/clang/Basic/LangOptions.def (working copy)<br>@@ -160,6 +160,8 @@<br> ENUM_LANGOPT(SignedOverflowBehavior, SignedOverflowBehaviorTy, 2, SOB_Undefined,<br> "signed integer overflow handling")<br>
<br>+BENIGN_LANGOPT(ArrowDepth, 32, 2048,<br>+ "maximum number of operator->s to follow")<br> BENIGN_LANGOPT(InstantiationDepth, 32, 256,<br> "maximum template instantiation depth")<br>
BENIGN_LANGOPT(ConstexprCallDepth, 32, 512,<br>Index: include/clang/Basic/DiagnosticSemaKinds.td<br>===================================================================<br></div>--- include/clang/Basic/DiagnosticSemaKinds.td (revision 193583)<div>
<br>
+++ include/clang/Basic/DiagnosticSemaKinds.td (working copy)<br>@@ -2569,6 +2569,11 @@<br> // can handle that case properly.<br> def note_ovl_candidate_non_deduced_mismatch_qualified : Note<<br> "candidate template ignored: could not match %q0 against %q1">;<br>
+def err_arrow_instantiation_depth_exceeded : Error<<br>+ "arrow operator instantiation exceeded maximum depth of %0">;<br>+def note_arrow_instantiation_depth : Note<<br>+ "use -foperator-arrow-depth=N to increase arrow operator instantiation "<br>
+ "depth">;<br> <br></div><div> // Note that we don't treat templates differently for this diagnostic.<br> def note_ovl_candidate_arity : Note<"candidate "<br></div>Index: include/clang/Driver/Options.td<br>
===================================================================<br>--- include/clang/Driver/Options.td (revision 193583)<div><br>+++ include/clang/Driver/Options.td (working copy)<br>@@ -775,6 +775,8 @@<br>
def ftemplate_depth_ : Joined<["-"], "ftemplate-depth-">, Group<f_Group>;<br>
def ftemplate_backtrace_limit_EQ : Joined<["-"], "ftemplate-backtrace-limit=">,<br> Group<f_Group>;<br>+def foperator_arrow_depth_EQ : Joined<["-"], "foperator-arrow-depth=">,<br>
+ Group<f_Group>;<br> def ftest_coverage : Flag<["-"], "ftest-coverage">, Group<f_Group>;<br> def fvectorize : Flag<["-"], "fvectorize">, Group<f_Group>,<br>
HelpText<"Enable the loop vectorization passes">;<br></div>Index: include/clang/Driver/CC1Options.td<br>===================================================================<br>--- include/clang/Driver/CC1Options.td (revision 193583)<div>
<br>
+++ include/clang/Driver/CC1Options.td (working copy)<br>@@ -442,6 +442,8 @@<br> HelpText<"Default type visibility">;<br> def ftemplate_depth : Separate<["-"], "ftemplate-depth">,<br>
HelpText<"Maximum depth of recursive template instantiation">;<br>+def foperator_arrow_depth : Separate<["-"], "foperator-arrow-depth">,<br>+ HelpText<"Maximum depth of arrow operator instantiation">;<br>
def fconstexpr_depth : Separate<["-"], "fconstexpr-depth">,<br> HelpText<"Maximum depth of recursive constexpr function calls">;<br> def fconstexpr_steps : Separate<["-"], "fconstexpr-steps">,<br>
</div>
Index: docs/UsersManual.rst<br>===================================================================<br>--- docs/UsersManual.rst (revision 193583)<br>+++ docs/UsersManual.rst (working copy)<br>@@ -1292,6 +1292,11 @@<br>
Sets the limit for recursively nested template instantiations to N. The<br> default is 1024.<br> <br>+.. option:: -foperator-arrow-depth=N<br>+<br>+ Sets the limit for arrow operator instantiations to N. The<br>+ default is 2048.<br>
+<br> .. _objc:<br> <br> Objective-C Language Features<br><br><br><br></div><div>Here is the test case which I have worked out:<br><br>// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s -DMAX=10 -foperator-arrow-depth=10<br>
// RUN: %clang -std=c++11 -fsyntax-only -Xclang -verify %s -DMAX=10 -foperator-arrow-depth=10<br><br><br>template< int n ><br>struct a {<br> a< n+1 > operator->()<br> {<br> return a< n+1 >();<br>
}<br>};<br><br>int foo() {<br> a<0>()->x; \<br>// expected-error{{arrow operator instantiation exceeded maximum depth of 10}} \<br>// expected-note {{use -foperator-arrow-depth=N to increase arrow operator instantiation depth}}<br>
}<br><br></div><div>Please suggest improvements.<br><br></div>A couple of things on which your inputs would be appreciated are:<br><br>1) ""Please don't use the word 'instantiation' here.""<br>
<br><br></div>This comment was implied for note def name/error def name/note string being printed/error string being printed??<br><br></div><div>Please suggest some alternatives for the same.<br></div></div></blockquote>
</div></div></div></div></blockquote><div><br></div></div></div><div>I've reworded this a little.</div><div class="im"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><div><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr"><div>2) "Have you considered including notes pointing at the sequence of
operator-> functions we called, and the corresponding types? This
might not be obvious."<br><br></div><div>By this you mean backtrace right, similar to the option -ftemplate-backtrace-limit=N? If yes, I did think about it, but was concerned about the implementation difficulty. If necessary I will work it out. <br>
</div></div></blockquote></div></div></div></div></blockquote><div><br></div></div><div>I've added this. We now produce:</div><div><br></div><div><div><stdin>:1:81: error: use of 'operator->' on type 'Ptr<int>' would invoke a sequence of more than 256 'operator->' calls</div>
<div>template<typename T> struct Ptr { Ptr<T*> operator->(); }; Ptr<int> p; int n = p->n;</div><div> ~^</div><div><stdin>:1:43: note: 'operator->' declared here produces an object of type 'Ptr<int *>'</div>
<div>template<typename T> struct Ptr { Ptr<T*> operator->(); }; Ptr<int> p; int n = p->n;</div><div> ^</div><div><stdin>:1:43: note: 'operator->' declared here produces an object of type 'Ptr<int **>'</div>
<div><stdin>:1:43: note: 'operator->' declared here produces an object of type 'Ptr<int ***>'</div><div><stdin>:1:43: note: 'operator->' declared here produces an object of type 'Ptr<int ****>'</div>
<div><stdin>:1:43: note: (skipping 248 'operator->'s in backtrace)</div><div><stdin>:1:43: note: 'operator->' declared here produces an object of type 'Ptr<int</div><div> *************************************************************************************************************************************************************************************************************************************************************>'</div>
<div><stdin>:1:43: note: 'operator->' declared here produces an object of type 'Ptr<int</div><div> **************************************************************************************************************************************************************************************************************************************************************>'</div>
<div><stdin>:1:43: note: 'operator->' declared here produces an object of type 'Ptr<int</div><div> ***************************************************************************************************************************************************************************************************************************************************************>'</div>
<div><stdin>:1:43: note: 'operator->' declared here produces an object of type 'Ptr<int</div><div> ****************************************************************************************************************************************************************************************************************************************************************>'</div>
<div><stdin>:1:81: note: use -foperator-arrow-depth=N to increase 'operator->' limit</div><div>template<typename T> struct Ptr { Ptr<T*> operator->(); }; Ptr<int> p; int n = p->n;</div>
<div> ^</div></div><div><br></div><div>I also reduced the depth limit to 256 to match the bracket limit (recursive AST visitors might otherwise hit stack overflows when analyzing a long operator-> chain).</div>
<div><div class="h5">
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div><div class="gmail_extra">
<div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div></div><div>Thanks,<br>
</div><div>Rahul <br>
</div><div><br><br><br></div></div><div><div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Oct 28, 2013 at 11:55 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div>On Sun, Oct 27, 2013 at 2:10 PM, Rahul Jain <span dir="ltr"><<a href="mailto:1989.rahuljain@gmail.com" target="_blank">1989.rahuljain@gmail.com</a>></span> wrote:<br>
</div><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">Hi,<div><br></div><div><div><div>
Thanks a lot Arthur and Richard for your valuable comments. Feels great when a beginner is supported to such an extent.</div>
<div><br></div><div>As per your comments I have reworked the patch. Here is the revised one. Please take a look for feedback.</div>
<div><br></div><div><div>Index: lib/Frontend/CompilerInvocation.cpp</div><div>===================================================================</div><div>--- lib/Frontend/CompilerInvocation.cpp<span style="white-space:pre-wrap"> </span>(revision 193504)</div>
<div>
<div>+++ lib/Frontend/CompilerInvocation.cpp<span style="white-space:pre-wrap"> </span>(working copy)</div><div>@@ -1313,6 +1313,8 @@</div><div> Opts.MathErrno = !Opts.OpenCL && Args.hasArg(OPT_fmath_errno);</div>
<div> Opts.InstantiationDepth =</div><div> getLastArgIntValue(Args, OPT_ftemplate_depth, 256, Diags);</div><div>+ Opts.ArrowDepth =</div></div><div>+ getLastArgIntValue(Args, OPT_foperator_arrow_depth, 2048, Diags);</div>
<div>
<div> Opts.ConstexprCallDepth =</div><div> getLastArgIntValue(Args, OPT_fconstexpr_depth, 512, Diags);</div><div> Opts.ConstexprStepLimit =</div></div><div>Index: lib/Driver/Tools.cpp</div><div>===================================================================</div>
<div>--- lib/Driver/Tools.cpp<span style="white-space:pre-wrap"> </span>(revision 193504)</div><div>+++ lib/Driver/Tools.cpp<span style="white-space:pre-wrap"> </span>(working copy)</div><div>@@ -2802,6 +2802,11 @@</div>
<div> CmdArgs.push_back(A->getValue());</div><div> }</div><div> </div><div>+ if (Arg *A = Args.getLastArg(options::OPT_foperator_arrow_depth_EQ)) {</div><div>+ CmdArgs.push_back("-foperator-arrow-depth");</div>
<div>
<div>+ CmdArgs.push_back(A->getValue());</div><div>+ }</div><div>+</div><div> if (Arg *A = Args.getLastArg(options::OPT_fconstexpr_depth_EQ)) {</div><div> CmdArgs.push_back("-fconstexpr-depth");</div>
<div> CmdArgs.push_back(A->getValue());</div></div><div>Index: lib/Sema/SemaExprCXX.cpp</div><div>===================================================================</div><div>--- lib/Sema/SemaExprCXX.cpp<span style="white-space:pre-wrap"> </span>(revision 193504)</div>
<div>+++ lib/Sema/SemaExprCXX.cpp<span style="white-space:pre-wrap"> </span>(working copy)</div><div>@@ -5189,7 +5189,17 @@</div><div><div> SmallVector<SourceLocation, 8> Locations;</div><div> CTypes.insert(Context.getCanonicalType(BaseType));</div>
<div> </div></div><div>+ unsigned int Arrow_instantiation_count = 0;</div></div></div></div></div></blockquote><div><br></div><div>This should be CamelCase, without underscores.</div><div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr"><div><div> while (BaseType->isRecordType()) {</div><div>+ Arrow_instantiation_count++;</div><div>+ if (Arrow_instantiation_count >= getLangOpts().ArrowDepth) {</div>
<div>+<span style="white-space:pre-wrap"> </span>Diag(OpLoc, diag::err_arrow_instantiation_depth_exceeded)</div><div>+<span style="white-space:pre-wrap"> </span> << getLangOpts().ArrowDepth << Base->getSourceRange();</div>
<div>
<div>+<span style="white-space:pre-wrap"> </span>Diag(OpLoc, diag::note_arrow_instantiation_depth)</div><div>+<span style="white-space:pre-wrap"> </span> << getLangOpts().ArrowDepth;</div><div>+<span style="white-space:pre-wrap"> </span>return ExprError();</div>
</div></div></div></blockquote><div><br></div></div><div>The body of this 'if' should be indented by two more spaces.</div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr"><div><div>
</div><div><div>+ }</div><div>+</div><div> Result = BuildOverloadedArrowExpr(</div><div> S, Base, OpLoc,</div><div> // When in a template specialization and on the first loop iteration,</div>
</div><div>Index: include/clang/Basic/LangOptions.def</div>
<div>===================================================================</div><div>--- include/clang/Basic/LangOptions.def<span style="white-space:pre-wrap"> </span>(revision 193504)</div><div><div>+++ include/clang/Basic/LangOptions.def<span style="white-space:pre-wrap"> </span>(working copy)</div>
<div>@@ -160,6 +160,8 @@</div><div> ENUM_LANGOPT(SignedOverflowBehavior, SignedOverflowBehaviorTy, 2, SOB_Undefined,</div><div> "signed integer overflow handling")</div><div> </div></div><div>+BENIGN_LANGOPT(ArrowDepth, 32, 2048,</div>
<div>
<div>+ "maximum number of operator->s to follow")</div><div> BENIGN_LANGOPT(InstantiationDepth, 32, 256,</div><div> "maximum template instantiation depth")</div><div> BENIGN_LANGOPT(ConstexprCallDepth, 32, 512,</div>
</div><div>Index: include/clang/Basic/DiagnosticSemaKinds.td</div><div>===================================================================</div><div>--- include/clang/Basic/DiagnosticSemaKinds.td<span style="white-space:pre-wrap"> </span>(revision 193504)</div>
<div>+++ include/clang/Basic/DiagnosticSemaKinds.td<span style="white-space:pre-wrap"> </span>(working copy)</div><div>@@ -2569,6 +2569,11 @@</div><div><div> // can handle that case properly.</div><div> def note_ovl_candidate_non_deduced_mismatch_qualified : Note<</div>
<div> "candidate template ignored: could not match %q0 against %q1">;</div><div>+def err_arrow_instantiation_depth_exceeded : Error<</div></div><div>+ "arrow operator instantiation exceeded maximum depth of %0">;</div>
<div>+def note_arrow_instantiation_depth : Note<</div><div>+ "use -foperator-arrow-depth=N to increase arrow operator instantiation "</div><div>+ "depth">;</div></div></div></blockquote><div>
<br></div></div></div><div>Please don't use the word 'instantiation' here.</div><div><br></div><div>Have you considered including notes pointing at the sequence of operator-> functions we called, and the corresponding types? This might not be obvious.</div>
<div><div>
<div><span style="color:rgb(80,0,80)"> </span></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">
<div><div><div> // Note that we don't treat templates differently for this diagnostic.</div>
<div> def note_ovl_candidate_arity : Note<"candidate "</div></div><div>Index: include/clang/Driver/CC1Options.td</div><div>===================================================================</div><div>--- include/clang/Driver/CC1Options.td<span style="white-space:pre-wrap"> </span>(revision 193504)</div>
<div>
<div>+++ include/clang/Driver/CC1Options.td<span style="white-space:pre-wrap"> </span>(working copy)</div><div>@@ -442,6 +442,8 @@</div><div> HelpText<"Default type visibility">;</div><div> def ftemplate_depth : Separate<["-"], "ftemplate-depth">,</div>
<div> HelpText<"Maximum depth of recursive template instantiation">;</div></div><div>+def foperator_arrow_depth : Separate<["-"], "foperator-arrow-depth">,</div><div>
<div>+ HelpText<"Maximum depth of arrow operator instantiation">;</div>
<div> def fconstexpr_depth : Separate<["-"], "fconstexpr-depth">,</div><div> HelpText<"Maximum depth of recursive constexpr function calls">;</div><div> def fconstexpr_steps : Separate<["-"], "fconstexpr-steps">,</div>
<div>Index: include/clang/Driver/Options.td</div><div>===================================================================</div></div><div>--- include/clang/Driver/Options.td<span style="white-space:pre-wrap"> </span>(revision 193504)</div>
<div>
<div>+++ include/clang/Driver/Options.td<span style="white-space:pre-wrap"> </span>(working copy)</div><div>@@ -775,6 +775,8 @@</div><div> def ftemplate_depth_ : Joined<["-"], "ftemplate-depth-">, Group<f_Group>;</div>
<div> def ftemplate_backtrace_limit_EQ : Joined<["-"], "ftemplate-backtrace-limit=">,</div><div> Group<f_Group>;</div></div><div>+def foperator_arrow_depth_EQ : Joined<["-"], "foperator-arrow-depth=">,</div>
<div>+ Group<f_Group>;</div><div><div> def ftest_coverage : Flag<["-"], "ftest-coverage">, Group<f_Group>;</div><div> def fvectorize : Flag<["-"], "fvectorize">, Group<f_Group>,</div>
<div> HelpText<"Enable the loop vectorization passes">;</div></div></div><div><br></div><div><br></div><div>Attaching the gcc testsuite test case too for reference. </div></div></blockquote><div><br></div>
</div></div><div>Please don't mail code from GCC here.</div><div><br></div><div>Instead, you'll need to craft a test case for our test suite. You can take inspiration from</div><div><br></div><div> test/SemaCXX/constexpr-depth.cpp</div>
<div> test/SemaTemplate/instantiation-depth.cpp</div><div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr"><div>How do I add the flag to the documentation?</div></div></blockquote><div><br></div></div><div>Modify docs/UsersManual.rst</div><div><br></div><div>Thank you for working on this!</div><div><div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr"><div>Thanks,</div><div>Rahul</div></div><div><div><div class="gmail_extra"><br><br><div class="gmail_quote">On Sat, Oct 26, 2013 at 12:14 AM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div>On Fri, Oct 25, 2013 at 9:06 AM, Arthur O'Dwyer <span dir="ltr"><<a href="mailto:arthur.j.odwyer@gmail.com" target="_blank">arthur.j.odwyer@gmail.com</a>></span> wrote:<br>
</div><div class="gmail_extra">
<div class="gmail_quote"><div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">IMHO, it would be better to remove the command-line option, and just<br>
set the (hard-coded) limit to 2048 or something else of that general<br>
order of magnitude. I can't think of any valid use for deeply nested<br>
operator->s; it's not like recursive template instantiation, where you<br>
might want to use a higher limit to implement a useful behavior.<br></blockquote><div><br></div></div><div>The "usual" (albeit very rare) use is to get around the recursive template instantiation depth limit.</div>
<div><div>
</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Therefore, setting the limit to something in the thousands ought to be<br>
good enough until someone legitimately runs into the limit; and in the<br>
meantime, we're not polluting the command-line docs with obscure and<br>
useless options.<br></blockquote><div><br></div></div><div>I don't see any problem with setting the default to a higher value, like maybe 2048.</div><div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Also FWIW, I do think a DR ought to be opened to make this an explicit<br>
implementation limit in the Standard... but I'm not volunteering. :)<br>
<br>
my $.02,<br>
–Arthur<br>
<div><div><br>
<br>
On Fri, Oct 25, 2013 at 7:45 AM, Rahul Jain <<a href="mailto:1989.rahuljain@gmail.com" target="_blank">1989.rahuljain@gmail.com</a>> wrote:<br>
><br>
> Hi,<br>
><br>
> This is with reference to the below discussion thread:<br>
><br>
> <a href="http://clang-developers.42468.n3.nabble.com/Endless-operator-gt-chain-causing-infinite-loop-td4035258.html" target="_blank">http://clang-developers.42468.n3.nabble.com/Endless-operator-gt-chain-causing-infinite-loop-td4035258.html</a><br>
><br>
> I have tried to implement a command line argument to handle the issue.<br>
><br>
> The patch for the same is below:<br>
><br>
> Index: lib/Driver/Tools.cpp<br>
> ===================================================================<br>
> --- lib/Driver/Tools.cpp (revision 193400)<br>
> +++ lib/Driver/Tools.cpp (working copy)<br>
> @@ -2802,6 +2802,12 @@<br>
> CmdArgs.push_back(A->getValue());<br>
> }<br>
><br>
> + if (Arg *A = Args.getLastArg(options::OPT_foperatorarrow_depth_,<br>
> + options::OPT_foperatorarrow_depth_EQ)) {<br>
> + CmdArgs.push_back("-foperatorarrow-depth");<br></div></div></blockquote><div><br></div></div><div>Please use foperator_arrow_depth and -foperator-arrow-depth.</div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><div>
> + CmdArgs.push_back(A->getValue());<br>
> + }<br>
> +<br>
> if (Arg *A = Args.getLastArg(options::OPT_fconstexpr_depth_EQ)) {<br>
> CmdArgs.push_back("-fconstexpr-depth");<br>
> CmdArgs.push_back(A->getValue());<br>
> Index: lib/Frontend/CompilerInvocation.cpp<br>
> ===================================================================<br>
> --- lib/Frontend/CompilerInvocation.cpp (revision 193400)<br>
> +++ lib/Frontend/CompilerInvocation.cpp (working copy)<br>
> @@ -1313,6 +1313,8 @@<br>
> Opts.MathErrno = !Opts.OpenCL && Args.hasArg(OPT_fmath_errno);<br>
> Opts.InstantiationDepth =<br>
> getLastArgIntValue(Args, OPT_ftemplate_depth, 256, Diags);<br>
> + Opts.ArrowDepth =<br>
> + getLastArgIntValue(Args, OPT_foperatorarrow_depth, 256, Diags);<br>
> Opts.ConstexprCallDepth =<br>
> getLastArgIntValue(Args, OPT_fconstexpr_depth, 512, Diags);<br>
> Opts.ConstexprStepLimit =<br>
> Index: lib/Sema/SemaExprCXX.cpp<br>
> ===================================================================<br>
> --- lib/Sema/SemaExprCXX.cpp (revision 193400)<br>
> +++ lib/Sema/SemaExprCXX.cpp (working copy)<br>
> @@ -5188,8 +5188,19 @@<br>
> llvm::SmallPtrSet<CanQualType,8> CTypes;<br>
> SmallVector<SourceLocation, 8> Locations;<br>
> CTypes.insert(Context.getCanonicalType(BaseType));<br>
> +<br>
> + static unsigned int arrow_instantiation_count = 0;<br></div></div></blockquote><div><br></div></div></div><div>This should not be static, and the name should follow the coding convention:</div><div><br></div><div>
<a href="http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly" target="_blank">http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly</a></div>
<div>
<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div>
> + while (BaseType->isRecordType()) {<br>
> + arrow_instantiation_count++;<br>
> + if (arrow_instantiation_count >= getLangOpts().ArrowDepth) {<br>
> + Diag(OpLoc,diag::err_arrow_instantiation_depth_exceeded)<br></div></div></blockquote><div><br></div></div><div>Space after comma.</div><div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><div>
> + << getLangOpts().ArrowDepth<br>
> + << Base->getSourceRange();<br>
> + Diag(OpLoc, diag::note_arrow_instantiation_depth)<br>
> + << getLangOpts().ArrowDepth;<br>
> + return ExprError();<br></div></div></blockquote><div><br></div></div><div>Indentation is strange here.</div><div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><div>
> + }<br>
><br>
> - while (BaseType->isRecordType()) {<br>
> Result = BuildOverloadedArrowExpr(<br>
> S, Base, OpLoc,<br>
> // When in a template specialization and on the first loop<br>
> iteration,<br>
> Index: include/clang/Basic/DiagnosticSemaKinds.td<br>
> ===================================================================<br>
> --- include/clang/Basic/DiagnosticSemaKinds.td (revision 193400)<br>
> +++ include/clang/Basic/DiagnosticSemaKinds.td (working copy)<br>
> @@ -2561,6 +2561,11 @@<br>
> // can handle that case properly.<br>
> def note_ovl_candidate_non_deduced_mismatch_qualified : Note<<br>
> "candidate template ignored: could not match %q0 against %q1">;<br>
> +def err_arrow_instantiation_depth_exceeded : Error<<br>
> + "arrow operator instantiation exceeded maximum depth of %0">,<br>
> + DefaultFatal, NoSFINAE;<br></div></div></blockquote><div><br></div></div><div>We don't need this to be fatal.</div><div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><div>
> +def note_arrow_instantiation_depth : Note<<br>
> + "use -foperatorarrow-depth=N to increase arrow operator instantiation<br>
> depth">;<br>
><br>
> // Note that we don't treat templates differently for this diagnostic.<br>
> def note_ovl_candidate_arity : Note<"candidate "<br>
> Index: include/clang/Basic/LangOptions.def<br>
> ===================================================================<br>
> --- include/clang/Basic/LangOptions.def (revision 193400)<br>
> +++ include/clang/Basic/LangOptions.def (working copy)<br>
> @@ -160,6 +160,8 @@<br>
> ENUM_LANGOPT(SignedOverflowBehavior, SignedOverflowBehaviorTy, 2,<br>
> SOB_Undefined,<br>
> "signed integer overflow handling")<br>
><br>
> +BENIGN_LANGOPT(ArrowDepth, 32, 256,<br>
> + "maximum arrow instantiation depth")<br></div></div></blockquote><div><br></div></div><div>"instantiation" is not correct here. Maybe "maximum number of operator->s to follow"?</div>
<div><div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div>
> BENIGN_LANGOPT(InstantiationDepth, 32, 256,<br>
> "maximum template instantiation depth")<br>
> BENIGN_LANGOPT(ConstexprCallDepth, 32, 512,<br>
> Index: include/clang/Driver/CC1Options.td<br>
> ===================================================================<br>
> --- include/clang/Driver/CC1Options.td (revision 193400)<br>
> +++ include/clang/Driver/CC1Options.td (working copy)<br>
> @@ -442,6 +442,8 @@<br>
> HelpText<"Default type visibility">;<br>
> def ftemplate_depth : Separate<["-"], "ftemplate-depth">,<br>
> HelpText<"Maximum depth of recursive template instantiation">;<br>
> +def foperatorarrow_depth : Separate<["-"], "foperatorarrow-depth">,<br>
> + HelpText<"Maximum depth of arrow operator instantiation">;<br>
> def fconstexpr_depth : Separate<["-"], "fconstexpr-depth">,<br>
> HelpText<"Maximum depth of recursive constexpr function calls">;<br>
> def fconstexpr_steps : Separate<["-"], "fconstexpr-steps">,<br>
> Index: include/clang/Driver/Options.td<br>
> ===================================================================<br>
> --- include/clang/Driver/Options.td (revision 193400)<br>
> +++ include/clang/Driver/Options.td (working copy)<br>
> @@ -775,6 +775,8 @@<br>
> def ftemplate_depth_ : Joined<["-"], "ftemplate-depth-">, Group<f_Group>;<br>
> def ftemplate_backtrace_limit_EQ : Joined<["-"],<br>
> "ftemplate-backtrace-limit=">,<br>
> Group<f_Group>;<br>
> +def foperatorarrow_depth_EQ : Joined<["-"], "foperatorarrow-depth=">,<br>
> Group<f_Group>;<br>
> +def foperatorarrow_depth_ : Joined<["-"], "foperatorarrow-depth-">,<br>
> Group<f_Group>;<br></div></div></blockquote><div><br></div></div></div><div>Please remove this variant. We only support the - form for -ftemplate-depth for compatibility with an old g++ misstep.</div><div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><div>
> def ftest_coverage : Flag<["-"], "ftest-coverage">, Group<f_Group>;<br>
> def fvectorize : Flag<["-"], "fvectorize">, Group<f_Group>,<br>
> HelpText<"Enable the loop vectorization passes">;<br>
><br>
><br>
><br>
> Please if someone could help me by reviewing it and suggesting improvements?</div></div></blockquote><div><br></div></div><div>It looks basically right, other than the tweaks above. Thanks!</div><div><br></div><div>
Please also add this flag to the documentation. </div>
</div></div></div>
</blockquote></div><br></div>
</div></div></blockquote></div></div></div><br></div></div>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div></div></div><br></div></div>
</blockquote></div><br></div>