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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri accepted this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

Seems fine to me.
Might be good for someone else (@aaron.ballman ?) to also take a look, not sure.

Thank you.



================
Comment at: clang/lib/CodeGen/CGStmt.cpp:801
+                 SourceLocToDebugLoc(R.getEnd()),
+                 loopMustProgress(CondIsConst));
 
----------------
fhahn wrote:
> lebedev.ri wrote:
> > 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"?
> Ok, so with 'saying' do you mean `loopMustProgress` is a bad name? I changed the name as suggested below, but if you meant something else, please let me know.
I'm saying that i was completely misreading this code because i thought `CondIsConstInt` was being stored into `loopMustProgress`, not that `loopMustProgress()` was being called with `CondIsConstInt` arg.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96418