[PATCH] D137787: [CodeGen] Relax assertion on generating destructor call

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

In D137787#3924727 <https://reviews.llvm.org/D137787#3924727>, @Hahnfeld wrote:

> Yes, I fully agree that having a test is desirable, I just didn't manage so far. Maybe it takes somebody with deep AST knowledge to explain under what circumstances `DtorDecl->getParent()` is *not* the canonical `Decl`. Maybe this could help crafting a test case, even outside of modules maybe

Modules can be "activated" at a random point of the translation unit. This would have influence of which `Decl*` becomes the canonical declaration as this is usually the first seen declaration. We have seen such issues over time with the modules system where depending on time when we load the module; the version of the standard library; the modulemap organization, etc. I am inclined to say this change somewhat makes sense to me as it seems somewhat consistent with what we've seen over the years but there are several questions I'd like to ask:

- Is the failure also not reproducible with clang9 (on which is based cling)?
- Is the failure also not reproducible with clang9 built on top of the downstream patches?

I believe if you can reduce the headers further it would make it easier to figure out in which context this assertion needs to be relaxed. Sorry for not being very helpful here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137787