Re: Attribute spelling policy
On Mon, Oct 23, 2017 at 11:16 AM, Arthur O'Dwyer wrote: > On Mon, Oct 23, 2017 at 7:33 AM, Aaron Ballman via cfe-commits > wrote: >> >> On Mon, Oct 23, 2017 at 9:48 AM, Hal Finkel wrote: >> > On 10/21/2017 10:14 AM, Aaron Ballman via cfe-commits wrote: >> >> >> >> Attributes come with multiple spelling flavors, but when it comes to >> >> adding new attributes that are not present in other compiler tools >> >> such as GCC or MSVC, we have done a poor job of being consistent with >> >> which spelling flavors we adopt the attributes under. Some of our >> >> attributes are specified with only an __attribute__ spelling (about >> >> 100), while others are specified with both __attribute__ and >> >> [[clang::XXX]] (about 30), and still others are specified as only >> >> [[clang::XXX]] attributes (only 1). This puts additional burden on >> >> developers to remember which attributes are spelled with what syntax >> >> and the various rules surrounding how to write attributes with that >> >> spelling. >> >> >> >> I am proposing that we take a more principled approach when adding new >> >> attributes so that we provide a better user experience. Specifically, >> >> when adding an attribute that other vendors do not support, the >> >> attribute should be given an __attribute__ and [[clang::]] spelling >> >> unless there's good reason not to. This is not a novel proposal -- GCC >> >> supports all of their documented __attribute__ spellings under a >> >> [[gnu::XXX]] spelling, and I am proposing we do the same with our >> >> vendor namespace. >> > >> > For attributes that both Clang and GCC support, where GCC provides a >> > [[gnu::X]] syntax, do you propose that our policy will be to support the >> > same? >> >> Yes; we should use the [[gnu::X]] spelling instead of adding the same >> attribute under [[clang::X]] unless we think our semantics need to >> significantly differ from GCC's. > > > This seems like a departure from what you wrote originally, unless you just > misspoke or I misinterpreted. > You originally said, "[New attributes] should be given an __attribute__ and > [[clang::]] spelling unless there's good reason not to." I would 100% agree > with that policy, from a user's point of view. It would be awesome to know > exactly how an attribute is spelled without having to look it up in the > manual. Agreed. > But when you used the word "instead" just now, you implied that the policy > would be more like "New attributes should be given an __attribute__ and > [[clang::]] spelling unless there's good reason not to, or unless Clang is > copying work originally done by the GNU project, in which case the new > attribute should be given __attribute__ and [[gnu::]] spellings but not a > [[clang::]] spelling." Is that really what you meant, or was your original > statement closer to what you meant? That is really what I meant, but to be extra-double-clear: * If we're adding a new attribute that has never existed under a previous [[vendor::attr]] spelling, it should be __attribute__ and [[clang::attr]]. * If we're adding a new-to-clang attribute that has existed under a previous [[vendor::attr]] spelling: * if we intend for the attribute semantics to match the other vendor, it should be [[vendor::attr]]. * if we intend for the attribute semantics to differ from the other vendor, it should be [[clang::attr]]. * whether it's also __attribute__ may depend on the attribute being added; for instance, we may or may not want to add an __attribute__ spelling for a [[gsl::attr]]. > As a user, I would strongly prefer to write all my attributes in a > consistent way. If that way has to be __attribute__((foo)), then okay. But > in C++11-and-later, it would be nice to write all my attributes as > [[clang::foo]], and not have to keep consulting the manual to find out which > of [[foo]], [[clang::foo]], and [[gnu::foo]] is the proper spelling for this > particular attribute. You will always have to figure out whether it's [[foo]] or [[vendor::foo]] because [[foo]] is provided by the standard. However, the point still stands of it being difficult to remember [[gnu::foo]] vs [[clang::foo]]. However, I think that's a momentary bit of confusion compared to the confusion of "where do I write this attribute so that it works" that you get with __attribute__. > Of course the worst possible outcome would be where [[clang::foo]] and > [[gnu::foo]] both exist and have different semantics. I hope that doesn't > happen! But if it has already happened, then it becomes even more important > that a user of Clang never need to use the [[gnu::]] spelling for any Clang > attribute (because accidentally using [[gnu::]] on the wrong attribute could > cause bugs, in the hypothetical case that [[clang::foo]] and [[gnu::foo]] > have different semantics). > > Basically, as a user, I would like to see: > - Attribute names never deliberately collide with other vendors While that would be ideal, it also seems unlikely to be enforceable. One point
Re: Attribute spelling policy
On Mon, Oct 23, 2017 at 7:33 AM, Aaron Ballman via cfe-commits < cfe-commits@lists.llvm.org> wrote: > On Mon, Oct 23, 2017 at 9:48 AM, Hal Finkel wrote: > > On 10/21/2017 10:14 AM, Aaron Ballman via cfe-commits wrote: > >> > >> Attributes come with multiple spelling flavors, but when it comes to > >> adding new attributes that are not present in other compiler tools > >> such as GCC or MSVC, we have done a poor job of being consistent with > >> which spelling flavors we adopt the attributes under. Some of our > >> attributes are specified with only an __attribute__ spelling (about > >> 100), while others are specified with both __attribute__ and > >> [[clang::XXX]] (about 30), and still others are specified as only > >> [[clang::XXX]] attributes (only 1). This puts additional burden on > >> developers to remember which attributes are spelled with what syntax > >> and the various rules surrounding how to write attributes with that > >> spelling. > >> > >> I am proposing that we take a more principled approach when adding new > >> attributes so that we provide a better user experience. Specifically, > >> when adding an attribute that other vendors do not support, the > >> attribute should be given an __attribute__ and [[clang::]] spelling > >> unless there's good reason not to. This is not a novel proposal -- GCC > >> supports all of their documented __attribute__ spellings under a > >> [[gnu::XXX]] spelling, and I am proposing we do the same with our > >> vendor namespace. > > > > For attributes that both Clang and GCC support, where GCC provides a > > [[gnu::X]] syntax, do you propose that our policy will be to support the > > same? > > Yes; we should use the [[gnu::X]] spelling instead of adding the same > attribute under [[clang::X]] unless we think our semantics need to > significantly differ from GCC's. > This seems like a departure from what you wrote originally, unless you just misspoke or I misinterpreted. You originally said, "[New attributes] should be given an __attribute__ and [[clang::]] spelling unless there's good reason not to." I would 100% agree with that policy, from a user's point of view. It would be awesome to know exactly how an attribute is spelled without having to look it up in the manual. But when you used the word "instead" just now, you implied that the policy would be more like "New attributes should be given an __attribute__ and [[clang::]] spelling unless there's good reason not to, *or unless Clang is copying work originally done by the GNU project*, in which case the new attribute should be given __attribute__ and [[gnu::]] spellings but not a [[clang::]] spelling." Is that really what you meant, or was your original statement closer to what you meant? As a user, I would strongly prefer to write all my attributes in a consistent way. If that way has to be __attribute__((foo)), then okay. But in C++11-and-later, it would be nice to write all my attributes as [[clang::foo]], and not have to keep consulting the manual to find out which of [[foo]], [[clang::foo]], and [[gnu::foo]] is the proper spelling for this particular attribute. Of course the worst possible outcome would be where [[clang::foo]] and [[gnu::foo]] both exist and have different semantics. I hope that doesn't happen! But if it has already happened, then it becomes even more important that a user of Clang never need to use the [[gnu::]] spelling for any Clang attribute (because accidentally using [[gnu::]] on the wrong attribute could cause bugs, in the hypothetical case that [[clang::foo]] and [[gnu::foo]] have different semantics). Basically, as a user, I would like to see: - Attribute names never deliberately collide with other vendors - Every attribute supported by Clang is available under __attribute__ - Every attribute supported by Clang is available under [[clang::]] - Any attribute that mimics a GNU feature is available under [[gnu::]] (and also under the two above) - Any attribute that mimics an MSVC feature is available under [[msvc::]] (and also under the first two above) - Any attribute that mimics a GNU-and-MSVC feature (thus, a feature available on all three platforms) is available under all of [[gnu::]], [[msvc::]], and [[clang::]] (and also under __attribute__) Then, as a user, I don't need to care about the historical accident of which project's employees invented a feature first; I can just use the [[xxx::]] prefix corresponding to my primary compiler, and either it will work or it will error out (in which case I'll know that my primary compiler does not support that feature). my $.02, –Arthur ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: Attribute spelling policy
On Mon, Oct 23, 2017 at 9:48 AM, Hal Finkel wrote: > > On 10/21/2017 10:14 AM, Aaron Ballman via cfe-commits wrote: >> >> Attributes come with multiple spelling flavors, but when it comes to >> adding new attributes that are not present in other compiler tools >> such as GCC or MSVC, we have done a poor job of being consistent with >> which spelling flavors we adopt the attributes under. Some of our >> attributes are specified with only an __attribute__ spelling (about >> 100), while others are specified with both __attribute__ and >> [[clang::XXX]] (about 30), and still others are specified as only >> [[clang::XXX]] attributes (only 1). This puts additional burden on >> developers to remember which attributes are spelled with what syntax >> and the various rules surrounding how to write attributes with that >> spelling. >> >> I am proposing that we take a more principled approach when adding new >> attributes so that we provide a better user experience. Specifically, >> when adding an attribute that other vendors do not support, the >> attribute should be given an __attribute__ and [[clang::]] spelling >> unless there's good reason not to. This is not a novel proposal -- GCC >> supports all of their documented __attribute__ spellings under a >> [[gnu::XXX]] spelling, and I am proposing we do the same with our >> vendor namespace. > > > For attributes that both Clang and GCC support, where GCC provides a > [[gnu::X]] syntax, do you propose that our policy will be to support the > same? Yes; we should use the [[gnu::X]] spelling instead of adding the same attribute under [[clang::X]] unless we think our semantics need to significantly differ from GCC's. >> Assuming this approach is reasonable to the community, >> I will add a >> CLANG spelling that behaves similar to the GCC spelling in that it >> automatically provides both the GNU and CXX11 spellings as >> appropriate. There are some attributes for which a [[clang::XXX]] >> spelling is not appropriate: >>* attributes that appertain to function declarations but require >> accessing the function parameters, such as disable_if or >> requires_capability > > > Is this restriction related to the change that p0542 proposes to make to the > interpretation of attributes that appear after functions as part of the > contracts proposal? Of sorts. P0542 describes the same problem we're running into with disable_if and others, but its solution is highly concerning because it removes the ability to specify any attributes on the function type and it introduces an irregularity in the way attributes are specified. I really don't like the direction taken in P0542, but I agree with the problem and think it needs to be solved. ~Aaron > > Thanks again, > Hal > >>* attributes with GNU spellings whose use is discouraged or >> deprecated, such as no_sanitize_memory >>* attributes that are part of other vendor specifications, like CUDA or >> OpenCL >> These deviations are reasonable, but should be documented in Attr.td >> near the Spelling definition for the attribute so that it's explicitly >> understood why the spelling differs. >> >> Additionally, I intend for the proposed CLANG spelling to be extended >> in the future to more easily expose [[clang::XXX]] spellings for >> attributes intended to be used in C (with >> -fdouble-square-bracket-attributes) as well as C++. >> >> As always, feedback is welcome! >> >> ~Aaron >> ___ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > > -- > Hal Finkel > Lead, Compiler Technology and Programming Languages > Leadership Computing Facility > Argonne National Laboratory > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: Attribute spelling policy
On 10/21/2017 10:14 AM, Aaron Ballman via cfe-commits wrote: Attributes come with multiple spelling flavors, but when it comes to adding new attributes that are not present in other compiler tools such as GCC or MSVC, we have done a poor job of being consistent with which spelling flavors we adopt the attributes under. Some of our attributes are specified with only an __attribute__ spelling (about 100), while others are specified with both __attribute__ and [[clang::XXX]] (about 30), and still others are specified as only [[clang::XXX]] attributes (only 1). This puts additional burden on developers to remember which attributes are spelled with what syntax and the various rules surrounding how to write attributes with that spelling. I am proposing that we take a more principled approach when adding new attributes so that we provide a better user experience. Specifically, when adding an attribute that other vendors do not support, the attribute should be given an __attribute__ and [[clang::]] spelling unless there's good reason not to. This is not a novel proposal -- GCC supports all of their documented __attribute__ spellings under a [[gnu::XXX]] spelling, and I am proposing we do the same with our vendor namespace. For attributes that both Clang and GCC support, where GCC provides a [[gnu::X]] syntax, do you propose that our policy will be to support the same? Assuming this approach is reasonable to the community, I will add a CLANG spelling that behaves similar to the GCC spelling in that it automatically provides both the GNU and CXX11 spellings as appropriate. There are some attributes for which a [[clang::XXX]] spelling is not appropriate: * attributes that appertain to function declarations but require accessing the function parameters, such as disable_if or requires_capability Is this restriction related to the change that p0542 proposes to make to the interpretation of attributes that appear after functions as part of the contracts proposal? Thanks again, Hal * attributes with GNU spellings whose use is discouraged or deprecated, such as no_sanitize_memory * attributes that are part of other vendor specifications, like CUDA or OpenCL These deviations are reasonable, but should be documented in Attr.td near the Spelling definition for the attribute so that it's explicitly understood why the spelling differs. Additionally, I intend for the proposed CLANG spelling to be extended in the future to more easily expose [[clang::XXX]] spellings for attributes intended to be used in C (with -fdouble-square-bracket-attributes) as well as C++. As always, feedback is welcome! ~Aaron ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits -- Hal Finkel Lead, Compiler Technology and Programming Languages Leadership Computing Facility Argonne National Laboratory ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: Attribute spelling policy
Very much a +1 on having attributes map in via `__attribute__` and `[[clang::]]` uniformly when appropriate. On Sat, Oct 21, 2017 at 8:14 AM, Aaron Ballman via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Attributes come with multiple spelling flavors, but when it comes to > adding new attributes that are not present in other compiler tools > such as GCC or MSVC, we have done a poor job of being consistent with > which spelling flavors we adopt the attributes under. Some of our > attributes are specified with only an __attribute__ spelling (about > 100), while others are specified with both __attribute__ and > [[clang::XXX]] (about 30), and still others are specified as only > [[clang::XXX]] attributes (only 1). This puts additional burden on > developers to remember which attributes are spelled with what syntax > and the various rules surrounding how to write attributes with that > spelling. > > I am proposing that we take a more principled approach when adding new > attributes so that we provide a better user experience. Specifically, > when adding an attribute that other vendors do not support, the > attribute should be given an __attribute__ and [[clang::]] spelling > unless there's good reason not to. This is not a novel proposal -- GCC > supports all of their documented __attribute__ spellings under a > [[gnu::XXX]] spelling, and I am proposing we do the same with our > vendor namespace. > > Assuming this approach is reasonable to the community, I will add a > CLANG spelling that behaves similar to the GCC spelling in that it > automatically provides both the GNU and CXX11 spellings as > appropriate. There are some attributes for which a [[clang::XXX]] > spelling is not appropriate: > * attributes that appertain to function declarations but require > accessing the function parameters, such as disable_if or > requires_capability > * attributes with GNU spellings whose use is discouraged or > deprecated, such as no_sanitize_memory > * attributes that are part of other vendor specifications, like CUDA or > OpenCL > These deviations are reasonable, but should be documented in Attr.td > near the Spelling definition for the attribute so that it's explicitly > understood why the spelling differs. > > Additionally, I intend for the proposed CLANG spelling to be extended > in the future to more easily expose [[clang::XXX]] spellings for > attributes intended to be used in C (with > -fdouble-square-bracket-attributes) as well as C++. > > As always, feedback is welcome! > > ~Aaron > ___ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > -- Saleem Abdulrasool compnerd (at) compnerd (dot) org ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Attribute spelling policy
Attributes come with multiple spelling flavors, but when it comes to adding new attributes that are not present in other compiler tools such as GCC or MSVC, we have done a poor job of being consistent with which spelling flavors we adopt the attributes under. Some of our attributes are specified with only an __attribute__ spelling (about 100), while others are specified with both __attribute__ and [[clang::XXX]] (about 30), and still others are specified as only [[clang::XXX]] attributes (only 1). This puts additional burden on developers to remember which attributes are spelled with what syntax and the various rules surrounding how to write attributes with that spelling. I am proposing that we take a more principled approach when adding new attributes so that we provide a better user experience. Specifically, when adding an attribute that other vendors do not support, the attribute should be given an __attribute__ and [[clang::]] spelling unless there's good reason not to. This is not a novel proposal -- GCC supports all of their documented __attribute__ spellings under a [[gnu::XXX]] spelling, and I am proposing we do the same with our vendor namespace. Assuming this approach is reasonable to the community, I will add a CLANG spelling that behaves similar to the GCC spelling in that it automatically provides both the GNU and CXX11 spellings as appropriate. There are some attributes for which a [[clang::XXX]] spelling is not appropriate: * attributes that appertain to function declarations but require accessing the function parameters, such as disable_if or requires_capability * attributes with GNU spellings whose use is discouraged or deprecated, such as no_sanitize_memory * attributes that are part of other vendor specifications, like CUDA or OpenCL These deviations are reasonable, but should be documented in Attr.td near the Spelling definition for the attribute so that it's explicitly understood why the spelling differs. Additionally, I intend for the proposed CLANG spelling to be extended in the future to more easily expose [[clang::XXX]] spellings for attributes intended to be used in C (with -fdouble-square-bracket-attributes) as well as C++. As always, feedback is welcome! ~Aaron ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
