NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land.
LGTM! Yeah I agree that we can eventually support a lot of these incrementally, but it's great to establish a safe baseline first. > If there are other specifiers involved, they may be accidentally removed > (e.g., fix `int [[some_type_attribute]] a[]` to `std::span<int>` a is > incorrect). Note that your patch doesn't actually address //type// attributes, only //declaration// attributes. I'd love to learn if we handle type attributes like `int *_Nonnull` and `int *_Nonnull *_Nonnull` correctly. How do we even handle that correctly? This is no longer about preserving the attribute, it's about the very ability to put the same attribute on a `span` itself or on its template parameter. And in case of `_Nonnull` it doesn't sound like we have anything like that: there is no standard container that acts like `span` but is never null (and even if there was, that's not what `_Nonnull` really means), and we probably also can't do `span<int *_Nonnull>` because that's actually the same type as `span<int *>` (https://godbolt.org/z/oehTMbvrz). So it's likely that we have to give up on unsupported type attributes very early, even before the fixit stage. Maybe on `hasPointerType()` stage; in such cases we aren't even sure that's a pointer, what if it's like an `int *__attribute__((vector_size(8)))`? (This actually can't happen; you can't have vectors of pointers. But something similar totally could.) > So both `const static int * x` and `static const int * x` will be fixed to > `static std::span<const int> x`. Ugh, `const static int` sounds awful because `const`applies to the //pointee//, `static` applies to the //pointer//, then `int` applies to the //pointee// again. I hope we'll never see such code, but it's great that it works correctly! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156192/new/ https://reviews.llvm.org/D156192 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
