[PATCH] D62690: Modify StepsLeft counter behaivior for constexprs

[wasiher via Phabricator via cfe-commits]

wasiher created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When I was trying to remove limits from constexpr steps (I wanted to give compiler as much time as it it needs), I  found -fconstexpr-steps option.
Naively i decided that -fconstexpr-steps=-1 is the best solution for this, because clang worked fine with my big constexpr function, and was not stopping. But when I checked source code, I understood that it is integer overflow, and compilation will stop  with error some day even if I will try to compile "while(1);" like code.

In the source code step limit is accesed as signed interger:
Opts.ConstexprStepLimit = getLastArgIntValue(Args, OPT_fconstexpr_steps, 1048576, Diags);

But later unsigned variable StepsLeft based on ConstexprStepLimit used as counter.

So maybe it will be better to make StepsLeft signed int?
In the negative case StepsLeft decrement will be disabled.

I also thought about making zero value to have same behavior, but such behavior was not declared anywhere before, and zero value probably can be used by someone as disabler of constexpr for example. But in the current implementation of clang negative value is not prohibited, but usage probably would not produce right expectation for user.


Repository:
  rC Clang

https://reviews.llvm.org/D62690

Files:
  clang/lib/AST/ExprConstant.cpp


Index: clang/lib/AST/ExprConstant.cpp
===================================================================
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -721,7 +721,7 @@
     /// StepsLeft - The remaining number of evaluation steps we're permitted
     /// to perform. This is essentially a limit for the number of statements
     /// we will evaluate.
-    unsigned StepsLeft;
+    int StepsLeft;
 
     /// BottomFrame - The frame in which evaluation started. This must be
     /// initialized after CurrentCall and CallStackDepth.
@@ -898,7 +898,8 @@
         FFDiag(S->getBeginLoc(), diag::note_constexpr_step_limit_exceeded);
         return false;
       }
-      --StepsLeft;
+      if (StepsLeft > 0)
+        --StepsLeft;
       return true;
     }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D62690.202225.patch
Type: text/x-patch
Size: 786 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190530/147395f0/attachment.bin>