Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline
Changeset View
Standalone View
clang/include/clang/Basic/Attr.td
| Show First 20 Lines • Show All 765 Lines • ▼ Show 20 Lines | |||||
| def Artificial : InheritableAttr { | def Artificial : InheritableAttr { | ||||
| let Spellings = [GCC<"artificial">]; | let Spellings = [GCC<"artificial">]; | ||||
| let Subjects = SubjectList<[InlineFunction]>; | let Subjects = SubjectList<[InlineFunction]>; | ||||
| let Documentation = [ArtificialDocs]; | let Documentation = [ArtificialDocs]; | ||||
| let SimpleHandler = 1; | let SimpleHandler = 1; | ||||
| } | } | ||||
| def DebugTrampoline: InheritableAttr { | |||||
| let Spellings = [Clang<"debug_trampoline">]; | |||||
aaron.ballman: Why does this not have a `[[]]` spelling? New attributes typically should be using the `Clang`… | |||||
Ok, I'll update it to the Clang spelling.
What about debugger_trampoline_target? debugger_trampoline_mangled_target? augusto2112: > Why does this not have a `[[]]` spelling? New attributes typically should be using the… | |||||
| let Subjects = SubjectList<[Function, ObjCMethod]>; | |||||
ObjC method decls as well? aaron.ballman: ObjC method decls as well? | |||||
Added objc methods (and tests) as well. augusto2112: Added objc methods (and tests) as well. | |||||
| let Documentation = [DebugTrampolineDocs]; | |||||
Not Done ReplyInline ActionsThis is quite fragile, for a few reasons.
It seems to me that this should be accepting an Expr argument that's either a CallExpr/MemberCallExpr or is a DeclRefExpr that names a function, so you could do: void bar();
void baz(int), baz(float);
__attribute__((trampoline(bar))) void foo() { bar(); }
// or
__attribute__((trampoline(baz(12))) void foo() { baz(12); }That said, this is still fragile in that it's easy to write the attribute and then the function body drifts out of sync with the attribute over time. Given that this only impacts debug information, that sort of latent issue is likely to hide in the code for a long time. Should we consider a warning if the function named by the attribute is not called within the function carrying the attribute? aaron.ballman: This is quite fragile, for a few reasons.
1) It's too easy for the user to get undiagnosed… | |||||
I understand the concerns you brought up as I didn't make it clear initially that this isn't supposed to be used by users, but meant to be used in compiler generated code. Given that, do you still think this should be an Expr? I think that'd be more error prone for tools to generate the correct C++ code to write, when compared with the mangled name. augusto2112: I understand the concerns you brought up as I didn't make it clear initially that this isn't… | |||||
Not Done ReplyInline ActionsHyrem's law exists, and attributes are the proof of that one :) Anything we provide in the attributes list gets used by users SOMEWHERE, and we need to do what we can to not have our interface be user hostile. Also, this means that whatever code is generated needs to have the same mangled name... and then doesn't this get defeated by the inliner? Can you name something that will later be removed? What if you're naming something without linkage? erichkeane: Hyrem's law exists, and attributes are the proof of that one :) Anything we provide in the… | |||||
I think we need to make this attribute fit the use case (compiler generated code which can provide an unambiguous target the debugger should trampoline to) instead of making it half fit a second use case (people using this attribute in their real code), which would make it a bad attribute that doesn't really fit either use case. I can make it super clear, either by name, documentation, examples, what this is meant for, and we could discuss a different attribute, similar to what @DavidSpickett was talking about, which means "step through this function, and check after every function call if we're in regular code, stop if yes", which would we intended for people to use, without the drawback this one would have for people (implementation drifting, users accidentally naming the wrong function, etc). Also, I'm not 100% how using expressions here would work. Lets say you have: struct S {};
void foo(S s) {
}
void foo(int i) {}
__attribute__((trampoline_mangled_target(foo(?))))
void bar() {
S s;
foo(s);
}What would be in place of the "?".
If the function was inlined, there's debug info to indicate that, and the debugger should handle it accordingly. If the function is completely deleted, the debugger would step over it, which is already the behavior you get when your function is deleted in optimized code.
You mean calling a static function defined in the same function? If we have debug info for it, I believe it should still work. augusto2112: > Hyrem's law exists, and attributes are the proof of that one :) Anything we provide in the… | |||||
| let SimpleHandler = 1; | |||||
| } | |||||
| def XRayInstrument : InheritableAttr { | def XRayInstrument : InheritableAttr { | ||||
| let Spellings = [Clang<"xray_always_instrument">, | let Spellings = [Clang<"xray_always_instrument">, | ||||
| Clang<"xray_never_instrument">]; | Clang<"xray_never_instrument">]; | ||||
| let Subjects = SubjectList<[Function, ObjCMethod]>; | let Subjects = SubjectList<[Function, ObjCMethod]>; | ||||
| let Accessors = [Accessor<"alwaysXRayInstrument", | let Accessors = [Accessor<"alwaysXRayInstrument", | ||||
| [Clang<"xray_always_instrument">]>, | [Clang<"xray_always_instrument">]>, | ||||
| Accessor<"neverXRayInstrument", | Accessor<"neverXRayInstrument", | ||||
| [Clang<"xray_never_instrument">]>]; | [Clang<"xray_never_instrument">]>]; | ||||
| ▲ Show 20 Lines • Show All 3,378 Lines • Show Last 20 Lines | |||||
Why does this not have a [[]] spelling? New attributes typically should be using the Clang spelling unless there's a compelling reason not to.
Also, I'm a bit worried that the identifier trampoline is pretty generic and can mean different things to different folks, so we might want a more debug info-specific name for this (https://en.wikipedia.org/wiki/Trampoline_(computing)).