jdenny added inline comments.
================
Comment at: include/clang/AST/Attr.h:210-212
+ unsigned Idx;
+ bool HasThis;
+ bool IsValid;
----------------
aaron.ballman wrote:
> I think it might be best to mash these together using bit-fields:
> ```
> unsigned Idx : 30;
> unsigned HasThis : 1;
> unsigned IsValid : 1;
> ```
Good point. Thanks.
================
Comment at: include/clang/AST/Attr.h:238-243
+ ParamIdx &operator=(const ParamIdx &I) {
+ Idx = I.Idx;
+ HasThis = I.HasThis;
+ IsValid = I.IsValid;
+ return *this;
+ }
----------------
aaron.ballman wrote:
> Is this necessary? There should be an implicit copy assignment operator for
> this class, and it's a bit strange to have a copy assignment but no copy
> constructor.
Oops. Thanks.
================
Comment at: include/clang/AST/Attr.h:291-292
+template <typename DerivedT, typename ValueT>
+class ParamIdxItrBase : public std::iterator<std::input_iterator_tag, ValueT,
+ ptrdiff_t, ValueT *, ValueT &> {
+ DerivedT &der() { return *static_cast<DerivedT *>(this); }
----------------
aaron.ballman wrote:
> `std::iterator` was deprecated in C++17, so you should manually expose these
> fields.
Now that sizeof(ParamIdx) == sizeof(unsigned), I can no longer find a reason to
avoid ParamIdx arrays, and so I can no longer find a good justification for the
iterator classes.
https://reviews.llvm.org/D43248
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
