[PATCH] D60237: [MS] Add metadata for __declspec(allocator)

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.


================
Comment at: clang/lib/CodeGen/CGCall.cpp:4382-4383
+  // Add metadata for calls to MSAllocator functions
+  if (!DisableDebugInfo) {
+    if (TargetDecl && TargetDecl->hasAttr<MSAllocatorAttr>())
+      getDebugInfo()->addHeapAllocSiteMetadata(CI, RetTy, Loc);
----------------
I think you can combine the ifs: `!DisableDebugInfo && TargetDecl ...`


================
Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1965
+                                           SourceLocation Loc) {
+  // FIXME: use the type that return value is cast to
+  llvm::MDNode *node;
----------------
This method takes the type in, so I think it's better to put the FIXME in CGCall.cpp. It should also be capitalized like a complete sentence (https://llvm.org/docs/CodingStandards.html#commenting).


================
Comment at: clang/test/CodeGen/debug-info-codeview-heapallocsite.c:4
+char buf[1024];
+__declspec(allocator) void *myalloc(int s) {
+  void *p = &buf[0];
----------------
I think it would be interesting to add an allocator that returns a pointer to a struct, so something like:
```
struct Foo { int x; };
__declspec(allocator) Foo *alloc_foo();
Foo *call_alloc_foo() { return alloc_foo(); }
```

Then we can check for the right DICompositeType.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60237/new/

https://reviews.llvm.org/D60237