mikael marked an inline comment as done and an inline comment as not done. mikael added inline comments.
================ Comment at: lib/AST/Type.cpp:2950 + FunctionTypeBits.HasExtQuals = 0; + } } ---------------- mikael wrote: > rjmccall wrote: > > The indentation here is messed up. > > > > You seem to be mixing up "fast qualifiers" with "CVR qualifiers" in a few > > places. That's currently correct, in the sense that the fast qualifiers > > are currently defined to be the CVR qualifiers, but the point of the > > distinction is that we might want to change that (and we have in fact > > explored that in the past). I think you should make `FunctionTypeBits` > > only claim to store the fast qualifiers, which mostly just means updating a > > few field / accessor names and changing things like the > > `getCVRQualifiers()` call right above this to be `getFastQualifiers()`. > > > > If there are places where it's convenient to assume that the CVR qualifiers > > are exactly the fast qualifiers, it's okay to static_assert that, but you > > should make sure to static_assert it in each place you assume it. > I'll change it, but I do have a question related to this: > > Can we make any assumptions with the "fast qualifiers"? I'd like it if we can > assume that they fit in 3 bits. Otherwise I think I also need an assertion > here. I solved it like this in the end: * I defined the FastTypeQuals to get the size of Qualifiers::FastWidth to make it dependent on possible changes to the fast flags. * Then I put a static_assertion to guard the hastConst(), hasVolatile() and hasRestrict() methods. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54862/new/ https://reviews.llvm.org/D54862 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
