rsandifo-arm marked an inline comment as done. rsandifo-arm added inline comments.
================ Comment at: clang/lib/Sema/SemaDeclAttr.cpp:2902 S.Diag(AL.getRange().getBegin(), diag::err_attribute_wrong_decl_type) - << AL << ExpectedTypeOrNamespace; + << AL << 0 << ExpectedTypeOrNamespace; return; ---------------- erichkeane wrote: > Every where you are doing just a '0' in a diagnostic here it makes it > incredibly unreadable. I'd prefer 1 of 2 solutions: > > 1- Create an enum somewhere that encodes the meaning here, and use those > instead. > 2- use a `/*Whatever*/` comment every place you're passing a raw literal. Yeah, that's fair. Sorry about that. I got some of my own medicine reading the patch back after a while away from it. Most of those `<< 0` come from looking at individual uses to see whether a regular keyword spelling is ever possible. If it wasn't possible, `<< 0` was supposed to be a justification for not covering that line of code in the new test cases. But I now think that was a mistake. Using `isRegularKeywordAttribute()` is useful for readability even if we “know” what its value ahead of time. And using it is more future-proof as well. So this update converts most uses of `<< 0` to `<< Something.isRegulardKeywordAttribute` in cases where the appropriate `Something` is readily available. There are a handful of cases where an immediate is still needed, so I went for option (2) and added a comment to those. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148702/new/ https://reviews.llvm.org/D148702 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits