[PATCH] D44134: [WebAssembly] Add WebAssemblyException information analysis

[Heejin Ahn via Phabricator via llvm-commits]

aheejin added inline comments.


================
Comment at: lib/Target/WebAssembly/WebAssemblyExceptionInfo.h:38-39
+  ~WebAssemblyException() {
+    for (size_t I = 0, E = SubExceptions.size(); I != E; ++I)
+      delete SubExceptions[I];
+  }
----------------
majnemer wrote:
> aheejin wrote:
> > majnemer wrote:
> > > DeleteContainerPointers(SubExceptions);
> > > 
> > > Even better would be to use a std::vector<unique_ptr<WebAssemblyException>>
> > To change this to `std::vector<unique_ptr<WebAssemblyException>>`, should I RAII `WebAssemblyException` object at the first creation time and use `std::move` whenever I pass it to a function until I figure out which container to insert this?
> Yes, I think that makes sense.
Come to think of it, this can be complicated. `WebAssemblyException` object is created in `recalculate`, which calls `discoverAndMapException`. Within `discoverAndMapException`, if a given exception is found to be a subexception of some other exception, it is inserted into a `SubExceptions` vector in the parent exception. If not, it is not inserted into anywhere in `discoverAndMapException`, but it is inserted into `WebAssemblyExceptionInfo::TopLevelExceptions` after returning from `discoverAndMapException`. So the code should look something like
```
void WebAssembly::discoverAndMapException(std::unique_ptr<WebAssemblyException> WE) {
  ...
  if (WE is a subexception of a parent exception P)
    add WE to P.SubExceptions;
  ...
}

void addTopLeveException(std::unique_ptr<WebAssemblyException> WE);

void WebAssemblyExceptinInfo::recalculate() {
  ...
  std::unique_ptr<WebAssemblyException> o(args);
  for (...) {
    auto *WE = new WeAssemblyException(EHPad);
    discoverAndMapException(std::move(WE), MDT);
    ...
  }
  ...
  if (top level exception)
    addTopLevelException(std::move(WE));
  ...
}
```

But the problem is, once we //move// the object to `discoverAndMapException`, we can't dereference it or move it again after returning from it. So it is complicated because
1. Where you create an object and insert it to a container are different
2. There are multiple places and multiple containers in which an object can be inserted.

I think `DeleteContainerPointers(SubExceptions);` is a simple and sane solution. What do you think?


Repository:
  rL LLVM

https://reviews.llvm.org/D44134