[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC
Ilya Biryukov via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 28 02:05:41 PST 2020
ilya-biryukov marked an inline comment as done.
ilya-biryukov added inline comments.
================
Comment at: clang/include/clang/AST/DependencyFlags.h:17-28
+enum class DependencyFlags : uint8_t {
+ Type = 1,
+ Value = 2,
+ Instantiation = 4,
+ UnexpandedPack = 8,
+
+ // Shorthands for commonly used combinations.
----------------
rsmith wrote:
> Hmm. We have a different set of propagated flags for types (dependent / instantiation dependent / unexpanded pack / variably-modified) and for (eg) template arguments and nested name specifiers (dependent / instantiation dependent / unexpanded pack). It would be nice to use the same machinery everywhere without introducing the possibility of meaningless states and giving the wrong names to some states.
>
> I think we should aim for something more type-safe than this: use a different type for each different family of AST nodes, so we don't conflate "dependent" for template arguments with one or both of "type-dependent" and "value-dependent" for expressions, which mean different things.
Yep, sounds good, probably the types would end up being more complicated, but I also like more type-safe approach.
Let me try to come up with something, I'll send a patch today.
================
Comment at: clang/include/clang/AST/DependencyFlags.h:81-85
+inline DependencyFlags turnTypeToValueDependency(DependencyFlags F) {
+ if (!isTypeDependent(F))
+ return F;
+ return (F & ~DependencyFlags::Type) | DependencyFlags::Value;
+}
----------------
rsmith wrote:
> This whole function should be equivalent to just `F & ~DependencyFlags::Type`. Any type-dependent expression must also be value-dependent, so you should never need to set the `::Value` bit. Perhaps we could assert this somewhere.
Good point, I'll try to find a place to assert this with multiple types for different AST categories.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71920/new/
https://reviews.llvm.org/D71920
More information about the cfe-commits
mailing list