nickdesaulniers added inline comments.
================ Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:557-558 + dyn_cast_or_null<CXXRecordDecl>(ND->getDeclContext()); + CXXThisScopeRAII ThisScope(*this, ThisContext, Qualifiers(), + ND->isCXXInstanceMember()); + } ---------------- rsmith wrote: > The purpose of creating this is to change the type of `this` in the scope of > the instantiation below, so this change is incorrect. The right fix would be > to change the initializer of `ThisContext` to handle `ND` being null. (I > suspect that actually can't happen at the moment because we happen to only > attach late-parsed attributes to `NamedDecl`s, but I don't think there's any > fundamental reason why that should be the case so it makes some sense to > retain the check here.) > The purpose of creating this is to change the type of this in the scope of > the instantiation below, so this change is incorrect. In this case I agree. The `}` added on L559 should be extended to L564. > so it makes some sense to retain the check here I assume PVS studio is complaining about dereferencing ND on L556, then doing what looks like a `nullptr` check on L558 (reminiscent of `-fno-delete-null-pointer-checks`). I assume `dyn_cast` on L554 can fail at runtime and return `nullptr`? Should that happen, how should we proceed? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62116/new/ https://reviews.llvm.org/D62116 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
