[Accessibility] When a private type is used in a public way, show its decl.

At the very least, this makes it easier to mark the private type as public
if that's the correct answer.

Swift SVN r19436

include/swift/AST/DiagnosticEngine.h

@@ -392,7 +392,7 @@ namespace swift {
 
     /// \brief The declaration of the currently active diagnostic, if there is
     /// one.
-    Decl *ActiveDiagnosticDecl = nullptr;
+    const Decl *ActiveDiagnosticDecl = nullptr;
 
     /// \brief The source location of the currently active diagnostic, if there
     /// is one.
@@ -515,7 +515,7 @@ namespace swift {
     ///
     /// \returns An in-flight diagnostic, to which additional information can
     /// be attached.
-    InFlightDiagnostic diagnose(Decl *decl, DiagID id,
+    InFlightDiagnostic diagnose(const Decl *decl, DiagID id,
                                 ArrayRef<DiagnosticArgument> args) {
       assert(!ActiveDiagnostic && "Already have an active diagnostic");
       ActiveDiagnosticLoc = SourceLoc();
@@ -534,7 +534,7 @@ namespace swift {
     ///
     /// \returns An in-flight diagnostic, to which additional information can
     /// be attached.
-    InFlightDiagnostic diagnose(Decl *decl, const Diagnostic &diag) {
+    InFlightDiagnostic diagnose(const Decl *decl, const Diagnostic &diag) {
       assert(!ActiveDiagnostic && "Already have an active diagnostic");
       ActiveDiagnosticLoc = SourceLoc();
       ActiveDiagnosticDecl = decl;
@@ -553,7 +553,7 @@ namespace swift {
     /// the types expected by the diagnostic \p ID.
     template<typename ...ArgTypes>
     InFlightDiagnostic
-    diagnose(Decl *decl, Diag<ArgTypes...> id,
+    diagnose(const Decl *decl, Diag<ArgTypes...> id,
              typename detail::PassArgument<ArgTypes>::type... args) {
       ActiveDiagnosticLoc = SourceLoc();
       ActiveDiagnosticDecl = decl;

lib/AST/DiagnosticEngine.cpp

@@ -408,7 +408,7 @@ void DiagnosticEngine::flushActiveDiagnostic() {
         {
           // Figure out which declaration to print. It's the top-most
           // declaration (not a module).
-          Decl *ppDecl = ActiveDiagnosticDecl;
+          const Decl *ppDecl = ActiveDiagnosticDecl;
           auto dc = ActiveDiagnosticDecl->getDeclContext();
 
           // FIXME: Horrible, horrible hackaround. We're not getting a

lib/Frontend/DiagnosticVerifier.cpp

@@ -172,8 +172,7 @@ bool DiagnosticVerifier::verifyFile(unsigned BufferID) {
 
     unsigned MatchCount = 1;
     int LineOffset = 0;
-    if (TextStartIdx > 0) {
+    if (TextStartIdx > 0 && MatchStart[0] == '@') {
-      if (MatchStart[0] == '@') {
       if (MatchStart[1] != '+' && MatchStart[1] != '-') {
         Errors.push_back({ MatchStart.data(),
           "expected '+'/'-' for line offset" });
@@ -181,16 +180,30 @@ bool DiagnosticVerifier::verifyFile(unsigned BufferID) {
       }
       StringRef Offs;
       if (MatchStart[1] == '+')
-          Offs = MatchStart.substr(2, TextStartIdx-2);
+        Offs = MatchStart.slice(2, TextStartIdx).rtrim();
       else
-          Offs = MatchStart.substr(1, TextStartIdx-1);
+        Offs = MatchStart.slice(1, TextStartIdx).rtrim();
-        if (Offs.rtrim().getAsInteger(10, LineOffset)) {
+
+      size_t SpaceIndex = Offs.find(' ');
+      if (SpaceIndex != StringRef::npos && SpaceIndex < TextStartIdx) {
+        size_t Delta = Offs.size() - SpaceIndex;
+        MatchStart = MatchStart.substr(TextStartIdx - Delta);
+        TextStartIdx = Delta;
+        Offs = Offs.slice(0, SpaceIndex);
+      } else {
+        MatchStart = MatchStart.substr(TextStartIdx);
+        TextStartIdx = 0;
+      }
+
+      if (Offs.getAsInteger(10, LineOffset)) {
         Errors.push_back({ MatchStart.data(),
           "expected line offset before '{{'" });
         continue;
       }
+    }
 
-      } else if (MatchStart.substr(0, TextStartIdx).rtrim()
+    if (TextStartIdx > 0) {
+      if (MatchStart.substr(0, TextStartIdx).rtrim()
           .getAsInteger(10, MatchCount)) {
         Errors.push_back(std::make_pair(MatchStart.data(),
                                         "expected match count before '{{'"));

lib/Sema/TypeCheckDecl.cpp

@@ -2173,15 +2173,6 @@ public:
 class TypeAccessibilityDiagnoser : private ASTWalker {
   const ComponentIdentTypeRepr *minAccessibilityType = nullptr;
 
-  const ValueDecl *getValueDecl(const ComponentIdentTypeRepr *TR) {
-    if (const ValueDecl *VD = TR->getBoundDecl())
-      return VD;
-    if (Type ty = TR->getBoundType())
-      return ty->getAnyNominal();
-    assert(TR->isBoundModule());
-    return nullptr;
-  }
-
   bool walkToTypeReprPre(TypeRepr *TR) override {
     auto CITR = dyn_cast<ComponentIdentTypeRepr>(TR);
     if (!CITR)
@@ -2202,6 +2193,15 @@ class TypeAccessibilityDiagnoser : private ASTWalker {
   }
 
 public:
+  static const ValueDecl *getValueDecl(const ComponentIdentTypeRepr *TR) {
+    if (const ValueDecl *VD = TR->getBoundDecl())
+      return VD;
+    if (Type ty = TR->getBoundType())
+      return ty->getAnyNominal();
+    assert(TR->isBoundModule());
+    return nullptr;
+  }
+
   static const TypeRepr *findMinAccessibleType(TypeRepr *TR) {
     TypeAccessibilityDiagnoser diagnoser;
     TR->walk(diagnoser);
@@ -2236,6 +2236,26 @@ static void checkTypeAccessibility(
   diagnose(typeAccess, complainRepr);
 }
 
+/// Highlights the given TypeRepr, and adds a note pointing to the type's
+/// declaration if possible.
+///
+/// Just flushes \p diag as is if \p complainRepr is null.
+static void highlightOffendingType(TypeChecker &TC, InFlightDiagnostic &diag,
+                                   const TypeRepr *complainRepr) {
+  if (!complainRepr) {
+    diag.flush();
+    return;
+  }
+
+  diag.highlight(complainRepr->getSourceRange());
+  diag.flush();
+
+  if (auto CITR = dyn_cast<ComponentIdentTypeRepr>(complainRepr)) {
+    const ValueDecl *VD = TypeAccessibilityDiagnoser::getValueDecl(CITR);
+    TC.diagnose(VD, diag::type_declared_here);
+  }
+}
+
 namespace {
 class DeclChecker : public DeclVisitor<DeclChecker> {
 public:
@@ -2609,8 +2629,7 @@ public:
                                   isExplicit,
                                   anyVar->getAccessibility(),
                                   typeAccess);
-          if (complainRepr)
+          highlightOffendingType(TC, diag, complainRepr);
-            diag.highlight(complainRepr->getSourceRange());
         });
       });
     }
@@ -2767,8 +2786,7 @@ public:
         auto diag = TC.diagnose(TAD, kind,
                                 TAD->getName(), TAD->getAccessibility(),
                                 typeAccess);
-        if (complainRepr)
+        highlightOffendingType(TC, diag, complainRepr);
-          diag.highlight(complainRepr->getSourceRange());
       });
     }
 

test/Sema/accessibility.swift

@@ -18,12 +18,14 @@
   @private func privateReq() {}
 }
 
+// expected-note@+1 {{type declared here}}
 @internal struct InternalStruct: PublicProto, InternalProto, PrivateProto {
   @private func publicReq() {} // expected-error {{method 'publicReq()' must be as accessible as its enclosing type because it matches a requirement in protocol 'PublicProto'}} {{3-11=@internal}}
   @private func internalReq() {} // expected-error {{method 'internalReq()' must have internal accessibility because it matches a requirement in internal protocol 'InternalProto'}} {{3-11=@internal}}
   @private func privateReq() {}
 }
 
+// expected-note@+1 17 {{type declared here}}
 @private struct PrivateStruct: PublicProto, InternalProto, PrivateProto {
   @private func publicReq() {}
   @private func internalReq() {}