[PATCH] D47503: Sema: Add a warning for member pointers with incomplete base types.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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