[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

[Ilya Biryukov via Phabricator via cfe-commits]

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