pcc added inline comments.
================ Comment at: clang/lib/Sema/SemaType.cpp:7599-7604 + if (!Context.getDiagnostics().isIgnored(diag::warn_memptr_incomplete, + Loc) && + !MPTy->getClass()->getAsCXXRecordDecl()->isBeingDefined() && + !isCompleteType(Loc, QualType(MPTy->getClass(), 0))) + Diag(Loc, diag::warn_memptr_incomplete) + << QualType(MPTy->getClass(), 0); ---------------- rsmith wrote: > It's not reasonable to call `isCompleteType` here; this is non-conforming > behavior, and it's not reasonable for a warning flag to change the semantics > or interpretation of the program. (We actually rely on this in several ways > -- `-Weverything` is not supposed to change the validity of programs, and > reuse of implicit modules with different warning flags relies on the property > that the compiler behavior is as if we compile with `-Weverything` and then > filter the warnings after the fact.) > > It seems fine to do this in the MS ABI case, since we will attempt to > complete the type anyway in that mode. If you want to apply this more > generally, I think there are two options: either add a `-f` flag to carry > this semantic change, or figure out whether the type is completable without > actually completing it (for example, check whether it's a templated class > whose pattern is a definition). That all seems reasonable. I think the right thing to do here is to add a `-fcomplete-member-pointers` flag then -- couple of reasons: - a user of this feature would probably expect to see diagnostics in the case where `RequireCompleteType` would fail in MS mode; - I'm planning to use the extra semantic information that would result from turning this on in IRgen when a new `-fsanitize=cfi-*` flag is enabled, which would mean that we would actually need to call `RequireCompleteType` at Sema time. I was thinking about tying the extra `RequireCompleteType` calls to the new `-fsanitize=cfi-*` flag, but that seems somewhat questionable as well (one of the main reasons is that it would be part of the `-fsanitize=cfi` group, and I don't want to make `-fsanitize=cfi` non-conforming), so we can probably get away with just a recommendation that `-fcomplete-member-pointers` is used with `-fsanitize=cfi`. https://reviews.llvm.org/D47503 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
