nickdesaulniers added inline comments.

================
Comment at: clang/lib/AST/Expr.cpp:3462-3468
       ->isConstantInitializer(Ctx, false, Culprit);
   case CXXDefaultArgExprClass:
     return cast<CXXDefaultArgExpr>(this)->getExpr()
       ->isConstantInitializer(Ctx, false, Culprit);
   case CXXDefaultInitExprClass:
     return cast<CXXDefaultInitExpr>(this)->getExpr()
       ->isConstantInitializer(Ctx, false, Culprit);
----------------
efriedma wrote:
> nickdesaulniers wrote:
> > @efriedma should a few more of these cases be changed from passing `false` 
> > as the `ForRef` param of `isConstantInitializer` to `IsForRef`?  If I 
> > change the rest of the cases except for `MaterializeTemporaryExprClass` 
> > then tests are still green.
> One of the reasons I didn't go with this approach is that I really didn't 
> want to think about this question...
> 
> We probably don't have good test coverage for the C++ constructs, since we 
> don't really use isConstantInitializer() outside of C++03 mode.
It looks like I should be able to test this against all of Android, and against 
Google's internal C++ codebase.  Let me work through that, and see if I can 
shake out additional tests cases to add here so that we have lower risk of 
regression.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151587

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to