<div dir="ltr">Hi,<div><br></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 class="" style="white-space:pre">      </span>(revision 193504)</div>
<div>+++ lib/Frontend/CompilerInvocation.cpp<span class="" style="white-space:pre">     </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>+      getLastArgIntValue(Args, OPT_foperator_arrow_depth, 2048, Diags);</div>
<div>   Opts.ConstexprCallDepth =</div><div>       getLastArgIntValue(Args, OPT_fconstexpr_depth, 512, Diags);</div><div>   Opts.ConstexprStepLimit =</div><div>Index: lib/Driver/Tools.cpp</div><div>===================================================================</div>
<div>--- lib/Driver/Tools.cpp<span class="" style="white-space:pre">    </span>(revision 193504)</div><div>+++ lib/Driver/Tools.cpp<span class="" style="white-space:pre">  </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>+    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>Index: lib/Sema/SemaExprCXX.cpp</div><div>===================================================================</div><div>--- lib/Sema/SemaExprCXX.cpp<span class="" style="white-space:pre">        </span>(revision 193504)</div>
<div>+++ lib/Sema/SemaExprCXX.cpp<span class="" style="white-space:pre">        </span>(working copy)</div><div>@@ -5189,7 +5189,17 @@</div><div>     SmallVector<SourceLocation, 8> Locations;</div><div>     CTypes.insert(Context.getCanonicalType(BaseType));</div>
<div> </div><div>+    unsigned int Arrow_instantiation_count = 0;</div><div>     while (BaseType->isRecordType()) {</div><div>+      Arrow_instantiation_count++;</div><div>+      if (Arrow_instantiation_count >= getLangOpts().ArrowDepth) {</div>
<div>+<span class="" style="white-space:pre">   </span>Diag(OpLoc, diag::err_arrow_instantiation_depth_exceeded)</div><div>+<span class="" style="white-space:pre"> </span>  << getLangOpts().ArrowDepth << Base->getSourceRange();</div>
<div>+<span class="" style="white-space:pre">   </span>Diag(OpLoc, diag::note_arrow_instantiation_depth)</div><div>+<span class="" style="white-space:pre"> </span>  << getLangOpts().ArrowDepth;</div><div>+<span class="" style="white-space:pre">      </span>return ExprError();</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>Index: include/clang/Basic/LangOptions.def</div>
<div>===================================================================</div><div>--- include/clang/Basic/LangOptions.def<span class="" style="white-space:pre">   </span>(revision 193504)</div><div>+++ include/clang/Basic/LangOptions.def<span class="" style="white-space:pre">   </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>+BENIGN_LANGOPT(ArrowDepth, 32, 2048,</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>Index: include/clang/Basic/DiagnosticSemaKinds.td</div><div>===================================================================</div><div>--- include/clang/Basic/DiagnosticSemaKinds.td<span class="" style="white-space:pre">    </span>(revision 193504)</div>
<div>+++ include/clang/Basic/DiagnosticSemaKinds.td<span class="" style="white-space:pre">      </span>(working copy)</div><div>@@ -2569,6 +2569,11 @@</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>+    "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><div> // Note that we don't treat templates differently for this diagnostic.</div>
<div> def note_ovl_candidate_arity : Note<"candidate "</div><div>Index: include/clang/Driver/CC1Options.td</div><div>===================================================================</div><div>--- include/clang/Driver/CC1Options.td<span class="" style="white-space:pre">       </span>(revision 193504)</div>
<div>+++ include/clang/Driver/CC1Options.td<span class="" style="white-space:pre">      </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>+def foperator_arrow_depth : Separate<["-"], "foperator-arrow-depth">,</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>--- include/clang/Driver/Options.td<span class="" style="white-space:pre">  </span>(revision 193504)</div>
<div>+++ include/clang/Driver/Options.td<span class="" style="white-space:pre"> </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>+def foperator_arrow_depth_EQ : Joined<["-"], "foperator-arrow-depth=">,</div>
<div>+                               Group<f_Group>;</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><br></div><div><br></div><div>Attaching the gcc testsuite test case too for reference. </div><div><br></div><div>How do I add the flag to the documentation?</div>
<div><br></div><div>Thanks,</div><div>Rahul</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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="im">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 class="im"><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 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">
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 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">


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 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>
> +    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 class="im">
<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 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>
> +        << 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 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>
> +      }<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 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>
> +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 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>
>  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 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>
>  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>