fhahn added inline comments.
================
Comment at: clang/lib/CodeGen/CGStmt.cpp:801
+ SourceLocToDebugLoc(R.getEnd()),
+ loopMustProgress(CondIsConst));
----------------
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.
================
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;
}
----------------
lebedev.ri wrote:
> 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?
I tried, but it appears as if there's no LangOpt for `C++98`. (this also seems
not directly related to the patch, so I guess we could do that as follow-up as
well?)
================
Comment at: clang/lib/CodeGen/CodeGenFunction.h:542
+ /// condition is a known constant.
+ bool loopMustProgress(bool HasConstantCond) {
if (CGM.getCodeGenOpts().getFiniteLoops() ==
----------------
lebedev.ri wrote:
> 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..
Oh right, the name was confusing. I updated the name
================
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;
}
----------------
lebedev.ri wrote:
> 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?
I tried but I am not sure how to best do that, given there's no explicit C89
lang opt.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D96418/new/
https://reviews.llvm.org/D96418
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
