EricWF marked 13 inline comments as done. EricWF added inline comments.
================ Comment at: include/clang/Sema/Sema.h:2758 bool AllowExplicit = false, + bool IsADLCandidate = false, ConversionSequenceList EarlyConversions = None); ---------------- rsmith wrote: > Long lists of bool arguments make me nervous, especially with default > arguments. Can you introduce an enum for the new param at least? Ack. I was thinking the same thing. ================ Comment at: lib/AST/ASTImporter.cpp:7387 return new (Importer.getToContext()) CallExpr( Importer.getToContext(), ToCallee, ToArgs, ToType, E->getValueKind(), ToRParenLoc); ---------------- rsmith wrote: > Do we need to pass through the usesADL flag here too? Seems like it. I'll add tests as well. ================ Comment at: lib/AST/Expr.cpp:1267 : Expr(SC, Empty), NumArgs(NumArgs) { + CallExprBits.UsesADL = false; CallExprBits.NumPreArgs = NumPreArgs; ---------------- riccibruno wrote: > riccibruno wrote: > > I believe that msan can cope with bit level operations just fine. > > What I am afraid is that initializing it here will hide the > > fact that it is not initialized during deserialization if the > > `E->setUsesADL(Record.readInt());` is removed by mistake. > > > > At least not initializing it here will leave a chance for msan > > to catch this. > I don't think they do. Regardless. I'll remove the initialization. I think potentially getting a random value will make it easier to spot bugs. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55534/new/ https://reviews.llvm.org/D55534 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits