[PATCH] D148700: [clang] Add support for “regular” keyword attributes

aaron.ballman added a comment. In D148700#4418767 , @rsandifo-arm wrote: > Hi @jyknight , @rsmith > > Do you have any more thoughts on the above? Quick version is: > > 1. Is it OK to have `[[…]]` attributes in the `arm` namespace that affect > semantic

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

jyknight added a comment. In D148700#4418767 , @rsandifo-arm wrote: > Hi @jyknight , @rsmith > > Do you have any more thoughts on the above? Quick version is: > > 1. Is it OK to have `[[…]]` attributes in the `arm` namespace that affect > semantics? I

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

rsandifo-arm added a comment. Hi @jyknight , @rsmith Do you have any more thoughts on the above? Quick version is: 1. Is it OK to have `[[…]]` attributes in the `arm` namespace that affect semantics? 2. Is it OK to raise an error for unrecognised attributes in the `arm` namespace (for a measu

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

aaron.ballman added a comment. In D148700#4401451 , @rsmith wrote: > In D148700#4401353 , @jyknight > wrote: > >> Yes, standard attributes aren't supposed to be used for things which affect >> the type system (a

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

rsandifo-arm added a comment. In D148700#4401451 , @rsmith wrote: > In D148700#4401353 , @jyknight > wrote: > >> Yes, standard attributes aren't supposed to be used for things which affect >> the type system (al

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

rsmith added a comment. In D148700#4401353 , @jyknight wrote: > Yes, standard attributes aren't supposed to be used for things which affect > the type system (although, we certainly have many, already, which do, since > we expose most GCC-syntax attribu

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

jyknight added a comment. > This makes ``CXX11`` and ``C2x`` spellings > unsuitable for attributes that affect the type system, that change the > binary interface of the code, or that have other similar semantic meaning. Yes, standard attributes aren't supposed to be used for things which affect

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

rsandifo-arm added inline comments. Comment at: clang-tools-extra/pseudo/lib/grammar/CMakeLists.txt:5 -# Dependencies should be minimal to avoid long dep paths in the build graph. -# It does use clangBasic headers (tok::TokenKind), but linking is not needed. -# We have no transit

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

thakis added subscribers: sammccall, thakis. thakis added inline comments. Comment at: clang-tools-extra/pseudo/lib/grammar/CMakeLists.txt:5 -# Dependencies should be minimal to avoid long dep paths in the build graph. -# It does use clangBasic headers (tok::TokenKind), but linki

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG301eb6b68f30: [clang] Add support for “regular” keyword attributes (authored by rsandifo-arm). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

aaron.ballman added a comment. In D148700#4371518 , @rsandifo-arm wrote: > Thanks @aaron.ballman and @erichkeane for your patience in reviewing the > patch and steering me in the right direction. > > What do you think about the other two patches in the

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

rsandifo-arm added a comment. Thanks @aaron.ballman and @erichkeane for your patience in reviewing the patch and steering me in the right direction. What do you think about the other two patches in the series: - https://reviews.llvm.org/D148699 (comes before this one) - https://reviews.llvm.org

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Thank you for your patience while we worked through the design details. :-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D14

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

rsandifo-arm marked an inline comment as done. rsandifo-arm added a comment. In D148700#4364229 , @aaron.ballman wrote: > This basically LGTM, but we should add documentation to the internals manual > too: https://clang.llvm.org/docs/InternalsManual.htm

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

rsandifo-arm updated this revision to Diff 524843. rsandifo-arm added a comment. Add internals documentation. Add FIXME to temporary code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148700/new/ https://reviews.llvm.org/D148700 Files: clang-t

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

aaron.ballman added a comment. This basically LGTM, but we should add documentation to the internals manual too: https://clang.llvm.org/docs/InternalsManual.html#spellings Comment at: clang/lib/AST/TypePrinter.cpp:1724-1727 + if (T->getAttrKind() == attr::ArmStreaming) { +

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

rsandifo-arm added a comment. Hi @erichkeane and @aaron.ballman. Does the updated patch look OK? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148700/new/ https://reviews.llvm.org/D148700 ___ cfe-commi

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

rsandifo-arm marked an inline comment as done. rsandifo-arm added a comment. Thanks @erichkeane . Adding the documentation with that kind of disclaimer sounds good to me. I've done that in the updated version, and also fixed the comment problem that Aaron pointed out. Repository: rG LLVM G

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

rsandifo-arm updated this revision to Diff 522703. rsandifo-arm added a comment. Add documentation. Fix a comment cut-&-pasto that Aaron pointed out. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148700/new/ https://reviews.llvm.org/D148700 Files

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

erichkeane added a comment. I don't have any good suggestions unfortunately. Perhaps Aaron does? I went through our list as well, and don't believe I see a good candidate. While I'm generally against an undocumented attribute, I'd be OK with either an immediate follow-up patch that documents

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

rsandifo-arm added a comment. Hi @aaron.ballman and @erichkeane . Do you have any more thoughts on this? In principle, I'm happy to convert an existing attribute over to the new scheme, but in practice, I can't find one that seems to be suitable. If we're going to do that, I think I'll need

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

rsandifo-arm added a subscriber: sdesmalen. rsandifo-arm added a comment. Thanks for the review. Comment at: clang/include/clang/Basic/Attr.td:2427-2430 +def ArmStreaming : TypeAttr, TargetSpecificAttr { + let Spellings = [RegularKeyword<"__arm_streaming">]; + let Documentati

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:2427-2430 +def ArmStreaming : TypeAttr, TargetSpecificAttr { + let Spellings = [RegularKeyword<"__arm_streaming">]; + let Documentation = [Undocumented]; +} I'd feel more comfort

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

rsandifo-arm marked an inline comment as done. rsandifo-arm added a comment. In D148700#4280833 , @erichkeane wrote: > Generally ok with all of this, but want some time to think about it/give > aaron/etc time to see it. Hi @erichkeane & @aaron.ballman

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

rsandifo-arm marked an inline comment as done. rsandifo-arm added inline comments. Comment at: clang/include/clang/Basic/TokenKinds.def:751 +// Keywords defined by Attr.td. +#define KEYWORD_ATTRIBUTE(X) KEYWORD(X, KEYALL) +#include "clang/Basic/AttrTokenKinds.inc" ---

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

rsandifo-arm updated this revision to Diff 515033. rsandifo-arm marked an inline comment as done. rsandifo-arm added a comment. Add #ifndef guard. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148700/new/ https://reviews.llvm.org/D148700 Files:

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

erichkeane added a comment. Generally ok with all of this, but want some time to think about it/give aaron/etc time to see it. Comment at: clang/include/clang/Basic/TokenKinds.def:751 +// Keywords defined by Attr.td. +#define KEYWORD_ATTRIBUTE(X) KEYWORD(X, KEYALL) +#include "

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

rsandifo-arm marked an inline comment as done. rsandifo-arm added inline comments. Comment at: clang/include/clang/Basic/TokenKinds.h:106 +#include "clang/Basic/AttrTokenKinds.inc" +#undef KEYWORD + ); erichkeane wrote: > Does AttrTokenKinds.inc not do this unde

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

rsandifo-arm updated this revision to Diff 514993. rsandifo-arm added a comment. Make AttrTokenKinds.inc use a new KEYWORD_ATTRIBUTE macro, rather than using KEYWORD directly. Move the #undef to the .inc file. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

erichkeane added inline comments. Comment at: clang/include/clang/Basic/TokenKinds.h:106 +#include "clang/Basic/AttrTokenKinds.inc" +#undef KEYWORD + ); Does AttrTokenKinds.inc not do this undef for you? Most of our tablegen'ed/.td files tend to... Edit now t

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

rsandifo-arm created this revision. rsandifo-arm added reviewers: erichkeane, aaron.ballman. Herald added subscribers: kristof.beyls, dschuff. Herald added a project: All. rsandifo-arm requested review of this revision. Herald added subscribers: cfe-commits, aheejin. Herald added projects: clang, c