Last-chance diagnostics for incomplete types

[zarko]

Summary

This patch adds a method to ExternalSemaSource to allow it to produce diagnostics when an incomplete type was detected where a complete type was required. For example, one could define an ExternalSemaSource that scans a repository for headers providing a missing definition.

The purpose of the MaybeDiagnoseMissingCompleteType method differs from that of the existing ExternalASTSource::CompleteType method. The former is invoked to diagnose a single SourceLocation-ed missing-complete-type site where there is no chance for recovery. The latter may be called in the course of compiling a valid source file with some of its definitions available in external but otherwise available locations.

[jdennett]

include/clang/Sema/ExternalSemaSource.h

185

Nit: "\p T" -> "T" to refer to the parameter T.

include/clang/Sema/MultiplexExternalSemaSource.h

325

Please document this, even if it's just inheriting docs and then saying how this implements the interface.

[zarko]

Thanks for the comments! I've also rebased this to trunk.

[zarko]

Rebase.

include/clang/Sema/ExternalSemaSource.h

 //===--- ExternalSemaSource.h - External Sema Interface ---------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file defines the ExternalSemaSource interface.
 //
 //===----------------------------------------------------------------------===//
 #ifndef LLVM_CLANG_SEMA_EXTERNAL_SEMA_SOURCE_H
 #define LLVM_CLANG_SEMA_EXTERNAL_SEMA_SOURCE_H
 
 #include "clang/AST/ExternalASTSource.h"
+#include "clang/AST/Type.h"
 #include "clang/Sema/TypoCorrection.h"
 #include "clang/Sema/Weak.h"
 #include "llvm/ADT/MapVector.h"
 #include <utility>
 
 namespace clang {
 
 class CXXConstructorDecl;
public:
   /// external source should take care not to introduce the same instantiations
   /// repeatedly.
   virtual void ReadPendingInstantiations(
                  SmallVectorImpl<std::pair<ValueDecl *, 
                                            SourceLocation> > &Pending) {}
 
   /// \brief Read the set of late parsed template functions for this source.
   ///
   /// The external source should insert its own late parsed template functions
   /// into the map. Note that this routine may be invoked multiple times; the
   /// external source should take care not to introduce the same map entries
   /// repeatedly.
   virtual void ReadLateParsedTemplates(
       llvm::DenseMap<const FunctionDecl *, LateParsedTemplate *> &LPTMap) {}
 
   /// \copydoc Sema::CorrectTypo
   /// \note LookupKind must correspond to a valid Sema::LookupNameKind
   ///
   /// ExternalSemaSource::CorrectTypo is always given the first chance to
   /// correct a typo (really, to offer suggestions to repair a failed lookup).
   /// It will even be called when SpellChecking is turned off or after a
   /// fatal error has already been detected.
   virtual TypoCorrection CorrectTypo(const DeclarationNameInfo &Typo,
                                      int LookupKind, Scope *S, CXXScopeSpec *SS,
                                      CorrectionCandidateCallback &CCC,
                                      DeclContext *MemberContext,
                                      bool EnteringContext,
                                      const ObjCObjectPointerType *OPT) {
     return TypoCorrection();
   }
 
+  /// \brief Produces a diagnostic note if the external source contains a
+  /// complete definition for \p T.
+  ///
+  /// \param Loc the location at which a complete type was required but not
+  /// provided
+  ///
+  /// \param T the \c QualType that should have been complete at \p Loc
+  ///
+  /// \return true if a diagnostic was produced, false otherwise.
+  virtual bool MaybeDiagnoseMissingCompleteType(SourceLocation Loc,
+                                                QualType T) {
+    return false;
+  }
+
   // isa/cast/dyn_cast support
   static bool classof(const ExternalASTSource *Source) {
     return Source->SemaSource;
   }
 }; 
 
 } // end namespace clang
 
 #endif

include/clang/Sema/MultiplexExternalSemaSource.h

   ///
   /// The external source should append its own pending instantiations to the
   /// given vector. Note that this routine may be invoked multiple times; the
   /// external source should take care not to introduce the same instantiations
   /// repeatedly.
   virtual void ReadPendingInstantiations(
               SmallVectorImpl<std::pair<ValueDecl*, SourceLocation> >& Pending);
 
   /// \brief Read the set of late parsed template functions for this source.
   ///
   /// The external source should insert its own late parsed template functions
   /// into the map. Note that this routine may be invoked multiple times; the
   /// external source should take care not to introduce the same map entries
   /// repeatedly.
   virtual void ReadLateParsedTemplates(
       llvm::DenseMap<const FunctionDecl *, LateParsedTemplate *> &LPTMap);
 
   /// \copydoc ExternalSemaSource::CorrectTypo
   /// \note Returns the first nonempty correction.
   virtual TypoCorrection CorrectTypo(const DeclarationNameInfo &Typo,
                                      int LookupKind, Scope *S, CXXScopeSpec *SS,
                                      CorrectionCandidateCallback &CCC,
                                      DeclContext *MemberContext,
                                      bool EnteringContext,
                                      const ObjCObjectPointerType *OPT);
 
+  /// \brief Produces a diagnostic note if one of the attached sources
+  /// contains a complete definition for \p T. Queries the sources in list
+  /// order until the first one claims that a diagnostic was produced.
+  ///
+  /// \param Loc the location at which a complete type was required but not
+  /// provided
+  ///
+  /// \param T the \c QualType that should have been complete at \p Loc
+  ///
+  /// \return true if a diagnostic was produced, false otherwise.
+  virtual bool MaybeDiagnoseMissingCompleteType(SourceLocation Loc, QualType T);
+
   // isa/cast/dyn_cast support
   static bool classof(const MultiplexExternalSemaSource*) { return true; }
   //static bool classof(const ExternalSemaSource*) { return true; }
 }; 
 
 } // end namespace clang
 
 #endif // LLVM_CLANG_SEMA_MULTIPLEX_EXTERNAL_SEMA_SOURCE_H

lib/Sema/MultiplexExternalSemaSource.cpp

   for (size_t I = 0, E = Sources.size(); I < E; ++I) {
     if (TypoCorrection C = Sources[I]->CorrectTypo(Typo, LookupKind, S, SS, CCC,
                                                    MemberContext,
                                                    EnteringContext, OPT))
       return C;
   }
   return TypoCorrection();
 }
+
+bool MultiplexExternalSemaSource::MaybeDiagnoseMissingCompleteType(
+    SourceLocation Loc, QualType T) {
+  for (size_t I = 0, E = Sources.size(); I < E; ++I) {
+    if (Sources[I]->MaybeDiagnoseMissingCompleteType(Loc, T))
+      return true;
+  }
+  return false;
+}

lib/Sema/SemaType.cpp

          Tag->isBeingDefined() ? diag::note_type_being_defined
                                : diag::note_forward_declaration)
       << QualType(Tag, 0);
 
   // If the Objective-C class was a forward declaration, produce a note.
   if (IFace && !IFace->getDecl()->isInvalidDecl())
     Diag(IFace->getDecl()->getLocation(), diag::note_forward_class);
 
+  // If we have external information that we can use to suggest a fix,
+  // produce a note.
+  if (ExternalSource)
+    ExternalSource->MaybeDiagnoseMissingCompleteType(Loc, T);
+
   return true;
 }
 
 bool Sema::RequireCompleteType(SourceLocation Loc, QualType T,
                                unsigned DiagID) {
   TypeDiagnoserDiag Diagnoser(DiagID);
   return RequireCompleteType(Loc, T, Diagnoser);
 }

unittests/Sema/ExternalSemaSourceTest.cpp

 #include "clang/Tooling/Tooling.h"
 #include "gtest/gtest.h"
 
 using namespace clang;
 using namespace clang::tooling;
 
 namespace {
 
+// \brief Counts the number of times MaybeDiagnoseMissingCompleteType
+// is called. Returns the result it was provided on creation.
+class CompleteTypeDiagnoser : public clang::ExternalSemaSource {
+public:
+  CompleteTypeDiagnoser(bool MockResult) : CallCount(0), Result(MockResult) {}
+
+  virtual bool MaybeDiagnoseMissingCompleteType(SourceLocation L, QualType T) {
+    ++CallCount;
+    return Result;
+  }
+
+  int CallCount;
+  bool Result;
+};
+
 // \brief Counts the number of err_using_directive_member_suggest diagnostics
 // correcting from one namespace to another while still passing all diagnostics
 // along a chain of consumers.
 class NamespaceDiagnosticWatcher : public clang::DiagnosticConsumer {
   DiagnosticConsumer *Chained;
   std::string FromNS;
   std::string ToNS;
 
TEST(ExternalSemaSource, ExternalTypoCorrectionOrdering) {
   ASSERT_TRUE(clang::tooling::runToolOnCodeWithArgs(
       Installer.take(), "namespace AAA { } using namespace AAB;", Args));
   ASSERT_LE(1, First.CallCount);
   ASSERT_LE(1, Second.CallCount);
   ASSERT_EQ(0, Third.CallCount);
   ASSERT_EQ(1, Watcher.SeenCount);
 }
 
+// We should only try MaybeDiagnoseMissingCompleteType if we can't otherwise
+// solve the problem.
+TEST(ExternalSemaSource, TryOtherTacticsBeforeDiagnosing) {
+  llvm::OwningPtr<ExternalSemaSourceInstaller> Installer(
+      new ExternalSemaSourceInstaller);
+  CompleteTypeDiagnoser Diagnoser(false);
+  Installer->PushSource(&Diagnoser);
+  std::vector<std::string> Args(1, "-std=c++11");
+  // This code hits the class template specialization/class member of a class
+  // template specialization checks in Sema::RequireCompleteTypeImpl.
+  ASSERT_TRUE(clang::tooling::runToolOnCodeWithArgs(
+      Installer.take(),
+      "template <typename T> struct S { class C { }; }; S<char>::C SCInst;",
+      Args));
+  ASSERT_EQ(0, Diagnoser.CallCount);
+}
+
+// The first ExternalSemaSource where MaybeDiagnoseMissingCompleteType returns
+// true should be the last one called.
+TEST(ExternalSemaSource, FirstDiagnoserTaken) {
+  llvm::OwningPtr<ExternalSemaSourceInstaller> Installer(
+      new ExternalSemaSourceInstaller);
+  CompleteTypeDiagnoser First(false);
+  CompleteTypeDiagnoser Second(true);
+  CompleteTypeDiagnoser Third(true);
+  Installer->PushSource(&First);
+  Installer->PushSource(&Second);
+  Installer->PushSource(&Third);
+  std::vector<std::string> Args(1, "-std=c++11");
+  ASSERT_FALSE(clang::tooling::runToolOnCodeWithArgs(
+      Installer.take(), "class Incomplete; Incomplete IncompleteInstance;",
+      Args));
+  ASSERT_EQ(1, First.CallCount);
+  ASSERT_EQ(1, Second.CallCount);
+  ASSERT_EQ(0, Third.CallCount);
+}
+
 } // anonymous namespace