rsmith added a comment. In https://reviews.llvm.org/D47503#1115505, @rnk wrote:
> [...] And rewrite the side-effecting isCompleteType call to use > RequireCompleteType with the warning diagnostic. We should keep the `isCompleteType` call and diagnose `if (!isCompleteType(...))`. `RequireCompleteType` needs an error diagnostic. (In particular, `RequireCompleteType` will produce an error suggesting a module import if a module containing the definition is known but not imported; `isCompleteType` will just treat the type as incomplete in that case. Don't `RequireCompleteType` unless you require a complete type :) ) ================ Comment at: clang/lib/Sema/SemaType.cpp:2451-2455 + // FIXME: This will cause errors if template instantiation fails. + if (!Context.getDiagnostics().isIgnored(diag::warn_memptr_incomplete, Loc) && + !Class->isDependentType() && + !Class->getAsCXXRecordDecl()->isBeingDefined()) + RequireCompleteType(Loc, Class, diag::warn_memptr_incomplete); ---------------- This doesn't seem right. Calling `RequireCompleteType` will trigger instantiation of the type if it's templated, which can affect the validity (and rarely, the meaning) of the program. Also, passing a warning diagnostic into `RequireCompleteType` doesn't actually work -- there are cases where it will disregard your diagnostic and substitute one of its own, which will be an error. https://reviews.llvm.org/D47503 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
