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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits