[PATCH] D96418: [clang] Refactor mustprogress handling, add it to all loops in c++11+.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Thank you!
Some more thoughts.



================
Comment at: clang/lib/CodeGen/CGStmt.cpp:801
+                 SourceLocToDebugLoc(R.getEnd()),
+                 loopMustProgress(CondIsConst));
 
----------------
fhahn wrote:
> lebedev.ri wrote:
> > Now this doesn't make sense. Why must loop progress if it's condition is a constant?
> > I think this should be renamed?
> I'm not sure I follow here. Whether the condition is constant or not is important for C11 and later. Something like ` while(true) {}` does not have to make progress, but `while (Foo) {}` must make progress. I am not sure what it should be renamed to?
Right.
But now this says that "if the condition is a constant integer 1 or 0, the loop must make progress".
Isn't that the opposite what this should say, as per your comment?
Shouldn't it be "loop must progress unless the condition is a constant integer 1"?


================
Comment at: clang/lib/CodeGen/CodeGenFunction.h:535-536
+    // Hence each function is 'mustprogress' in C++11 or later.
     return getLangOpts().CPlusPlus11 || getLangOpts().CPlusPlus14 ||
            getLangOpts().CPlusPlus17 || getLangOpts().CPlusPlus20;
   }
----------------
Since every new standard version will have to be added here,
can we invert this and just check for the know fixed set
of the versions where this doesn't apply?


================
Comment at: clang/lib/CodeGen/CodeGenFunction.h:542
+  /// condition is a known constant.
+  bool loopMustProgress(bool HasConstantCond) {
     if (CGM.getCodeGenOpts().getFiniteLoops() ==
----------------
Ah, i see.
My problem is that it's impossible to tell whether `loopMustProgress` is a member variable
that gets assigned `CondIsConstInt` value, or a "filter" function.

Maybe this (and it's `function` friend) should be something like `checkIfLoopMustProgress` or something..


================
Comment at: clang/lib/CodeGen/CodeGenFunction.h:562
+    // Loops with non-constant conditions must make progress in C11 and later.
+    return getLangOpts().C11 || getLangOpts().C17 || getLangOpts().C2x;
   }
----------------
Since every new standard version will have to be added here,
can we invert this and just check for the know fixed set
of the versions where this doesn't apply?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96418/new/

https://reviews.llvm.org/D96418