[PATCH] D92101: [Clang][Sema] Attempt to fix CTAD faulty copy of non-local typedefs

Mon Nov 30 05:30:33 PST 2020

martong added inline comments.

================
Comment at: clang/lib/Sema/SemaTemplate.cpp:2356-2359
+      auto *CD = cast<CXXConstructorDecl>(OldParam->getDeclContext());
+      // If the typedef is not a local typedef, then skip the transform.
+      if (OldTypedefDecl->getDeclContext() != CD->getDeclContext())
+        return OldParam;
----------------
rsmith wrote:
> I think it would be equivalent and simpler to check only `OldTypedefDecl->getDeclContext()->isDependentContext()`. If we can resolve the name of the typedef to a dependently-scoped typedef, it must be a member of the current instantiation, and that's what we really care about here. This would also avoid any concerns about whether we're looking at the right declaration of the enclosing class (in the case where multiple definitions from different modules got merged), whether we need to skip over transparent contexts, and so on.
Thanks for the review! I've updated to check for the dependent context.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92101