riccibruno added inline comments.
================ Comment at: clang/include/clang/AST/Expr.h:126 + if (TD) + D = D | DependencyFlags::Type; + if (VD) ---------------- 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 ================ Comment at: clang/include/clang/AST/Expr.h:134 + + ExprBits.Dependent = static_cast<unsigned>(D); ExprBits.ValueKind = VK; ---------------- You may want to add an assertion here to make sure that no truncation occurred. ================ Comment at: clang/lib/Serialization/ASTReaderStmt.cpp:530 + E->setDependencies(Deps); + E->setValueKind(static_cast<ExprValueKind>(Record.readInt())); ---------------- 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. 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
