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

[Bruno Ricci via Phabricator via cfe-commits]

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