ilya-biryukov marked 4 inline comments as done. ilya-biryukov added inline comments.
================ Comment at: clang/include/clang/AST/Expr.h:126 + if (TD) + D = D | DependencyFlags::Type; + if (VD) ---------------- riccibruno wrote: > ilya-biryukov wrote: > > Mordante wrote: > > > Just curious why do you prefer `D = D | DependencyFlags::Type;` over `D > > > |= DependencyFlags::Type;` ? The latter seems to be more common. > > Would also prefer `D |=`, but it leads to compilation errors. > > > > The builtin `operator |=` accepts ints and, therefore, fails on > > strongly-typed enums. > > And, AFAIK, there's no way to redefine `operator |=` for non-class types. > You certainly can define a compound assignment operator for an enumeration > type. It is only non-compound-assignment that is special cased and required > to be a member function. > > Example: https://godbolt.org/z/JV5uPw Ah, thanks! So it turns out I was wrong. Will update the patch. ================ Comment at: clang/lib/Serialization/ASTReaderStmt.cpp:530 + E->setDependencies(Deps); + E->setValueKind(static_cast<ExprValueKind>(Record.readInt())); ---------------- riccibruno wrote: > I think it would be nicer to add an abbreviation for the right number of bits > (using `DependencyFlagsBits`) and as you say just serialize/deserialize all > the flags in one go. Will do this in a follow-up. That would be a functional change, I'm aiming to keep this one an NFC. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71920/new/ https://reviews.llvm.org/D71920 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
