fhahn marked an inline comment as done.
fhahn added inline comments.

================
Comment at: clang/lib/CodeGen/CGStmt.cpp:796
+  bool EmitBoolCondBranch = !C || !C->isOne();
+  bool CondIsConst = C;
   const SourceRange &R = S.getSourceRange();
----------------
rjmccall wrote:
> fhahn wrote:
> > lebedev.ri wrote:
> > > I think, if we really want to give this a name, perhaps we want something 
> > > even more specific,
> > > perhaps `CondIsConstImm`/`CondIsConstInt`?
> > I updated the name to `CondIsConstInt`
> Please use an explicit `!= nullptr` check when converting a pointer to `bool` 
> outside of an obvious boolean context.
Updated, thanks!


================
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;
   }
----------------
rjmccall wrote:
> fhahn wrote:
> > 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?)
> The way these options work is that the later standards imply the early 
> standards; that's why there isn't a `CPlusPlus98`.  You just need 
> `CPlusPlus11` here.
Great, thanks for clarifying John! I updated the code.


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

Reply via email to