Page MenuHomePhabricator

Adding clone_attrs attribute.
Needs ReviewPublic

Authored by plotfi on Jun 21 2022, 1:14 AM.

Details

Summary

This is just experimental for the time being. Posting to phab to get some comments.

Diff Detail

Event Timeline

plotfi created this revision.Jun 21 2022, 1:14 AM
Herald added a project: Restricted Project. · View Herald Transcript
plotfi requested review of this revision.Jun 21 2022, 1:14 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 21 2022, 1:14 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

This is pretty incomplete. Please ignore for now.

Can you add some details to the patch summary as to what problem you're trying to solve with these changes?

The idea itself has some interesting edge cases to think about:

[[some_attr]] void decl();
[[clone_attrs_from(decl)]] void other();
[[some_new_attr]] void decl();

int main() {
  other(); // Does other see some_new_attr as well?
}

or

[[hot]] void decl();
[[clone_attrs_from(decl), cold]] void other_decl(); // Do you properly catch the mutual exclusions?

or

[[some_attr_requiring_the_func_return_a_pointer]] int *decl();
[[clone_attrs_from(decl)]] int other(); // Do you properly catch the semantic restrictions?

or

struct [[some_struct_attr]] S { ... };
[[clone_attrs_from(S)]] void other(); // Do you properly catch the semantic restrictions?

or

[[some_attr]] void decl();
[[clone_attrs_from(decl)]] void decl(); // Uh oh, recursive.

(I'm sure there are more situations I'm missing, but you get the idea about needing to be very careful with the edge cases.)

Thanks for the feedback on corner cases @aaron.ballman, this will give me more concrete things to think about here.

At the moment I mainly thinking about a case:

typedef int foo;
enum : foo {} __attribute__((__clone_attrs_from__(foo)));

This is mostly because of how NS_OPTIONS enums on Darwin are defined through a macro.
We have been looking into breaking up how the typedef and the enum are defined for better behavior with C++.
The only challenge is that if an NS_OPTIONS macro is used to declare an enum and an attribute is specified either before or after, it will then only apply to either the typedef or the enum.

Thats the reason for a clone_attrs attribute.

I'll update description soon.

Can you add some details to the patch summary as to what problem you're trying to solve with these changes?

The idea itself has some interesting edge cases to think about:

[[some_attr]] void decl();
[[clone_attrs_from(decl)]] void other();
[[some_new_attr]] void decl();

int main() {
  other(); // Does other see some_new_attr as well?
}

or

[[hot]] void decl();
[[clone_attrs_from(decl), cold]] void other_decl(); // Do you properly catch the mutual exclusions?

or

[[some_attr_requiring_the_func_return_a_pointer]] int *decl();
[[clone_attrs_from(decl)]] int other(); // Do you properly catch the semantic restrictions?

or

struct [[some_struct_attr]] S { ... };
[[clone_attrs_from(S)]] void other(); // Do you properly catch the semantic restrictions?

or

[[some_attr]] void decl();
[[clone_attrs_from(decl)]] void decl(); // Uh oh, recursive.

(I'm sure there are more situations I'm missing, but you get the idea about needing to be very careful with the edge cases.)

Thanks for the feedback on corner cases @aaron.ballman, this will give me more concrete things to think about here.

At the moment I mainly thinking about a case:

typedef int foo;
enum : foo {} __attribute__((__clone_attrs_from__(foo)));

This is mostly because of how NS_OPTIONS enums on Darwin are defined through a macro.
We have been looking into breaking up how the typedef and the enum are defined for better behavior with C++.
The only challenge is that if an NS_OPTIONS macro is used to declare an enum and an attribute is specified either before or after, it will then only apply to either the typedef or the enum.

Thats the reason for a clone_attrs attribute.

Ah, thank you for the explanation. That is an interesting case. Another potential idea to consider is the idea of "inheriting" attributes from a related interface. e.g., a derived class which inherits attributes from the base, or an enum with a fixed underlying type inheriting attributes from the fixed type, etc. I've not given it a lot of thought, but it seems like there's a relationship there that might make sense for attributes in general. As an example:

struct [[gnu::packed]] S {
  char c;
  int i;
};

struct T {
  char c;
  int i;
};

template <typename U>
struct [[inherits_attributes]] V : U {
  double d;
};

Where instantiating V<S> would result in both V and S being packed, but instantiating V<T> would not pack either. (Again, I've not put a ton of thought into it, but it's a possible alternative design worth considering.)

I'll update description soon.

Can you add some details to the patch summary as to what problem you're trying to solve with these changes?

The idea itself has some interesting edge cases to think about:

[[some_attr]] void decl();
[[clone_attrs_from(decl)]] void other();
[[some_new_attr]] void decl();

int main() {
  other(); // Does other see some_new_attr as well?
}

or

[[hot]] void decl();
[[clone_attrs_from(decl), cold]] void other_decl(); // Do you properly catch the mutual exclusions?

or

[[some_attr_requiring_the_func_return_a_pointer]] int *decl();
[[clone_attrs_from(decl)]] int other(); // Do you properly catch the semantic restrictions?

or

struct [[some_struct_attr]] S { ... };
[[clone_attrs_from(S)]] void other(); // Do you properly catch the semantic restrictions?

or

[[some_attr]] void decl();
[[clone_attrs_from(decl)]] void decl(); // Uh oh, recursive.

(I'm sure there are more situations I'm missing, but you get the idea about needing to be very careful with the edge cases.)

plotfi added a subscriber: nuriamari.