This revision was automatically updated to reflect the committed changes.
Closed by commit rC335757: [DebugInfo] Emit ObjC methods as part of interface
(authored by JDevlieghere, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D48241
Files:
lib/CodeGen/CGDebugInfo.cpp
dblaikie added inline comments.
Comment at: lib/CodeGen/CGDebugInfo.cpp:4239
+// Add methods to interface.
+for (auto p : ObjCMethodCache) {
+ if (p.second.empty())
aprantl wrote:
> JDevlieghere wrote:
> > dexonsmith wrote:
> > > Some comment for
aprantl added inline comments.
Comment at: lib/CodeGen/CGDebugInfo.cpp:4239
+// Add methods to interface.
+for (auto p : ObjCMethodCache) {
+ if (p.second.empty())
JDevlieghere wrote:
> dexonsmith wrote:
> > Some comment for "p" here.
> Fixed; I'll
JDevlieghere added inline comments.
Comment at: lib/CodeGen/CGDebugInfo.cpp:4239
+// Add methods to interface.
+for (auto p : ObjCMethodCache) {
+ if (p.second.empty())
dexonsmith wrote:
> Some comment for "p" here.
Fixed; I'll update the rest of
JDevlieghere updated this revision to Diff 152910.
JDevlieghere marked 6 inline comments as done.
JDevlieghere added a comment.
- Feedback Adrian & Duncan
https://reviews.llvm.org/D48241
Files:
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CGDebugInfo.h
test/CodeGenObjC/debug-info-category.m
dexonsmith added inline comments.
Comment at: lib/CodeGen/CGDebugInfo.cpp:3355
+ QualType QTy(ID->getTypeForDecl(), 0);
+ auto it = TypeCache.find(QTy.getAsOpaquePtr());
+ if (it != TypeCache.end()) {
LLVM style rules suggest UpperCamelCase or
aprantl added inline comments.
Comment at: lib/CodeGen/CGDebugInfo.cpp:4246
+ if (it == TypeCache.end())
+continue;
+
What does it mean that a type is not being found in the cache here?
Comment at: lib/CodeGen/CGDebugInfo.h:102
+
JDevlieghere updated this revision to Diff 152896.
JDevlieghere added a comment.
- Re-add test case (forgot to stage it for last patch)
https://reviews.llvm.org/D48241
Files:
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CGDebugInfo.h
test/CodeGenObjC/debug-info-category.m
Index:
JDevlieghere updated this revision to Diff 152883.
JDevlieghere added a comment.
- Update diff to version I used for testing (modulo the removed DWARF version
check)
https://reviews.llvm.org/D48241
Files:
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CGDebugInfo.h
Index:
JDevlieghere added a comment.
I tested this patch on the LLDB test suite and all tests passed.
What I did was:
- I removed the DWARF version check in clang so this was always generated.
- I commented out the code that reads the .apple_objc accelerator tables in
DWARFASTParserClang.cpp (which
JDevlieghere added a comment.
In https://reviews.llvm.org/D48241#1134985, @labath wrote:
> This is not true. (Unlike the .apple_*** tables, ) .debug_names allows you to
> specify a different schema for every entry in the acelerator table. The
> schema is specifing using abbreviations in much
labath added a comment.
I also don't want to get involved too much, but here are my 2c: :)
In https://reviews.llvm.org/D48241#1134342, @JDevlieghere wrote:
> Putting it in a separate column is also a bad idea, because that means the
> column is present for all the entries, including the ones
JDevlieghere updated this revision to Diff 151596.
JDevlieghere marked 3 inline comments as done.
JDevlieghere added a comment.
- Feedback Adrian
https://reviews.llvm.org/D48241
Files:
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CGDebugInfo.h
aprantl added inline comments.
Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4250
+
+ SmallVector EltTys;
+ EltTys.append(InterfaceDecl->getElements().begin(),
```
auto = InterfaceDecl->getElements();
EltTys.append(Elements.begin(), Elements.end());
JDevlieghere updated this revision to Diff 151591.
JDevlieghere added a comment.
- Use type cache &
- Simplify method cache struct
- Add custom test that verifies category methods are emitted
https://reviews.llvm.org/D48241
Files:
clang/lib/CodeGen/CGDebugInfo.cpp
JDevlieghere added a comment.
In https://reviews.llvm.org/D48241#1134240, @dblaikie wrote:
> Not sure I should get too involved in this discussion, knowing so little
> about Objective C - but if it seems sensible, could you provide some examples
> (just in comments/description in this code
aprantl added a comment.
> could you provide some examples (just in comments/description in this code
> review) of what the DWARF used to look like and what it looks like after this
> change?
Thus far an Objective-C interface is a DW_TAG_structure_type that only has
variables and properties
JDevlieghere updated this revision to Diff 151580.
JDevlieghere marked 5 inline comments as done.
JDevlieghere added a comment.
CR feedback
https://reviews.llvm.org/D48241
Files:
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CGDebugInfo.h
aprantl added a comment.
Thanks! Is there also a companion patch for LLVM that disables the objc
accelerator table?
Note that this is not a 100% replacement of the apple_objc accelerator table,
since the apple_objc table also lists all methods defined in categories of that
interface. Is the
dblaikie added a comment.
Not sure I should get too involved in this discussion, knowing so little about
Objective C - but if it seems sensible, could you provide some examples (just
in comments/description in this code review) of what the DWARF used to look
like and what it looks like after
JDevlieghere created this revision.
JDevlieghere added reviewers: aprantl, dexonsmith, dblaikie, labath.
As brought up during the discussion of the DWARF5 accelerator tables,
there is currently no way to associate Objective-C methods with the
interface they belong to, other than the .apple_objc
21 matches
Mail list logo
