[clang] NNS: don't print trailing scope resolution operator in diagnostics (#130529)

This clears up the printing of a NestedNameSpecifier so a trailing '::'
is not printed, unless it refers into the global scope.

This fixes a bunch of diagnostics where the trailing :: was awkward.
This also prints the NNS quoted consistenty.

There is a drive-by improvement to error recovery, where now we print
the actual type instead of `<dependent type>`.

This will clear up further uses of NNS printing in further patches.

Author: mizvekov

clang/docs/ReleaseNotes.rst

@@ -161,8 +161,8 @@ related warnings within the method body.
   ``__attribute__((model("large")))`` on non-TLS globals in x86-64 compilations.
   This forces the global to be considered small or large in regards to the
   x86-64 code model, regardless of the code model specified for the compilation.
-- Clang now emits a warning ``-Wreserved-init-priority`` instead of a hard error 
-  when ``__attribute__((init_priority(n)))`` is used with values of n in the 
+- Clang now emits a warning ``-Wreserved-init-priority`` instead of a hard error
+  when ``__attribute__((init_priority(n)))`` is used with values of n in the
   reserved range [0, 100]. The warning will be treated as an error by default.
 
 - There is a new ``format_matches`` attribute to complement the existing
@@ -234,7 +234,8 @@ Improvements to Clang's diagnostics
 - Diagnostics on chained comparisons (``a < b < c``) are now an error by default. This can be disabled with
   ``-Wno-error=parentheses``.
 - The ``-Wshift-bool`` warning has been added to warn about shifting a boolean. (#GH28334)
-
+- Fixed diagnostics adding a trailing ``::`` when printing some source code
+  constructs, like base classes.
 - The :doc:`ThreadSafetyAnalysis` now supports ``-Wthread-safety-pointer``,
   which enables warning on passing or returning pointers to guarded variables
   as function arguments or return value respectively. Note that

clang/include/clang/AST/NestedNameSpecifier.h

@@ -223,7 +223,8 @@ class NestedNameSpecifier : public llvm::FoldingSetNode {
   /// `ns::SomeTemplate<int, MyClass>` instead of
   /// `ns::SomeTemplate<Container::value_type, T>`.
   void print(raw_ostream &OS, const PrintingPolicy &Policy,
-             bool ResolveTemplateArguments = false) const;
+             bool ResolveTemplateArguments = false,
+             bool PrintFinalScopeResOp = true) const;
 
   void Profile(llvm::FoldingSetNodeID &ID) const {
     ID.AddPointer(Prefix.getOpaqueValue());

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -599,16 +599,17 @@ def err_using_typename_non_type : Error<
   "'typename' keyword used on a non-type">;
 def err_using_dependent_value_is_type : Error<
   "dependent using declaration resolved to type without 'typename'">;
-def err_using_decl_nested_name_specifier_is_not_class : Error<
-  "using declaration in class refers into '%0', which is not a class">;
+def err_using_decl_nested_name_specifier_is_not_class
+    : Error<"using declaration in class refers into %0, which is not a class">;
 def warn_cxx17_compat_using_decl_non_member_enumerator : Warning<
   "member using declaration naming non-class '%0' enumerator is "
   "incompatible with C++ standards before C++20">, InGroup<CXXPre20Compat>,
   DefaultIgnore;
 def err_using_decl_nested_name_specifier_is_current_class : Error<
   "using declaration refers to its own class">;
-def err_using_decl_nested_name_specifier_is_not_base_class : Error<
-  "using declaration refers into '%0', which is not a base class of %1">;
+def err_using_decl_nested_name_specifier_is_not_base_class
+    : Error<
+          "using declaration refers into %0, which is not a base class of %1">;
 def err_using_decl_constructor_not_in_direct_base : Error<
   "%0 is not a direct base of %1, cannot inherit constructors">;
 def err_using_decl_can_not_refer_to_class_member : Error<
@@ -1733,8 +1734,8 @@ def err_no_matching_local_friend_suggest : Error<
   "cannot define friend function %0 in a local class definition; did you mean %3?">;
 def err_partial_specialization_friend : Error<
   "partial specialization cannot be declared as a friend">;
-def err_qualified_friend_def : Error<
-  "friend function definition cannot be qualified with '%0'">;
+def err_qualified_friend_def
+    : Error<"friend function definition cannot be qualified with %0">;
 def err_friend_def_in_local_class : Error<
   "friend function cannot be defined in a local class">;
 def err_friend_specialization_def : Error<
@@ -1743,14 +1744,16 @@ def err_friend_not_first_in_declaration : Error<
   "'friend' must appear first in a non-function declaration">;
 def err_using_decl_friend : Error<
   "cannot befriend target of using declaration">;
-def warn_template_qualified_friend_unsupported : Warning<
-  "dependent nested name specifier '%0' for friend class declaration is "
-  "not supported; turning off access control for %1">,
-  InGroup<UnsupportedFriend>;
-def warn_template_qualified_friend_ignored : Warning<
-  "dependent nested name specifier '%0' for friend template declaration is "
-  "not supported; ignoring this friend declaration">,
-  InGroup<UnsupportedFriend>;
+def warn_template_qualified_friend_unsupported
+    : Warning<
+          "dependent nested name specifier %0 for friend class declaration is "
+          "not supported; turning off access control for %1">,
+      InGroup<UnsupportedFriend>;
+def warn_template_qualified_friend_ignored
+    : Warning<"dependent nested name specifier %0 for friend template "
+              "declaration is "
+              "not supported; ignoring this friend declaration">,
+      InGroup<UnsupportedFriend>;
 def ext_friend_tag_redecl_outside_namespace : ExtWarn<
   "unqualified friend declaration referring to type outside of the nearest "
   "enclosing namespace is a Microsoft extension; add a nested name specifier">,
@@ -5556,9 +5559,10 @@ def ext_template_spec_extra_headers : ExtWarn<
 def note_explicit_template_spec_does_not_need_header : Note<
   "'template<>' header not required for explicitly-specialized class %0 "
   "declared here">;
-def err_template_qualified_declarator_no_match : Error<
-  "nested name specifier '%0' for declaration does not refer into a class, "
-  "class template or class template partial specialization">;
+def err_template_qualified_declarator_no_match
+    : Error<"nested name specifier %0 for declaration does not refer into a "
+            "class, "
+            "class template or class template partial specialization">;
 def err_specialize_member_of_template : Error<
   "cannot specialize %select{|(with 'template<>') }0a member of an "
   "unspecialized template">;
@@ -5858,13 +5862,13 @@ def note_typename_member_refers_here : Note<
     "referenced member %0 is declared here">;
 def note_typename_refers_here : Note<
     "referenced %0 is declared here">;
-def err_typename_missing : Error<
-  "missing 'typename' prior to dependent type name '%0%1'">;
-def err_typename_missing_template : Error<
-  "missing 'typename' prior to dependent type template name '%0%1'">;
-def ext_typename_missing : ExtWarn<
-  "missing 'typename' prior to dependent type name '%0%1'">,
-  InGroup<DiagGroup<"typename-missing">>;
+def err_typename_missing
+    : Error<"missing 'typename' prior to dependent type name %0">;
+def err_typename_missing_template
+    : Error<"missing 'typename' prior to dependent type template name %0">;
+def ext_typename_missing
+    : ExtWarn<"missing 'typename' prior to dependent type name %0">,
+      InGroup<DiagGroup<"typename-missing">>;
 def ext_typename_outside_of_template : ExtWarn<
   "'typename' occurs outside of a template">, InGroup<CXX11>;
 def warn_cxx98_compat_typename_outside_of_template : Warning<
@@ -5878,9 +5882,10 @@ def note_using_value_decl_missing_typename : Note<
 def warn_cxx17_compat_implicit_typename : Warning<"use of implicit 'typename' is "
   "incompatible with C++ standards before C++20">, InGroup<CXX20Compat>,
   DefaultIgnore;
-def ext_implicit_typename : ExtWarn<"missing 'typename' prior to dependent "
-  "type name %0%1; implicit 'typename' is a C++20 extension">,
-  InGroup<CXX20>;
+def ext_implicit_typename
+    : ExtWarn<"missing 'typename' prior to dependent "
+              "type name %0; implicit 'typename' is a C++20 extension">,
+      InGroup<CXX20>;
 
 def err_template_kw_refers_to_non_template : Error<
   "%0%select{| following the 'template' keyword}1 "
@@ -5890,12 +5895,13 @@ def note_template_kw_refers_to_non_template : Note<
 def err_template_kw_refers_to_dependent_non_template : Error<
   "%0%select{| following the 'template' keyword}1 "
   "cannot refer to a dependent template">;
-def err_template_kw_refers_to_type_template : Error<
-  "'%0%1' is expected to be a non-type template, but instantiated to a %select{class|type alias}2 template">;
+def err_template_kw_refers_to_type_template
+    : Error<"%0 is expected to be a non-type template, but instantiated to a "
+            "%select{class|type alias}1 template">;
 def note_referenced_type_template : Note<
   "%select{class|type alias}0 template declared here">;
-def err_template_kw_missing : Error<
-  "missing 'template' keyword prior to dependent template name '%0%1'">;
+def err_template_kw_missing
+    : Error<"missing 'template' keyword prior to dependent template name %0">;
 def ext_template_outside_of_template : ExtWarn<
   "'template' keyword outside of a template">, InGroup<CXX11>;
 def warn_cxx98_compat_template_outside_of_template : Warning<
@@ -7888,8 +7894,8 @@ def err_nogetter_property_incdec : Error<
   "no getter method %1 for %select{increment|decrement}0 of property">;
 def err_no_subobject_property_setting : Error<
   "expression is not assignable">;
-def err_qualified_objc_access : Error<
-  "%select{property|instance variable}0 access cannot be qualified with '%1'">;
+def err_qualified_objc_access : Error<"%select{property|instance variable}0 "
+                                      "access cannot be qualified with %1">;
 
 def ext_freestanding_complex : Extension<
   "complex numbers are an extension in a freestanding C99 implementation">;
@@ -9839,8 +9845,8 @@ def note_non_usual_function_declared_here : Note<
 // C++ literal operators
 def err_literal_operator_outside_namespace : Error<
   "literal operator %0 must be in a namespace or global scope">;
-def err_literal_operator_id_outside_namespace : Error<
-  "non-namespace scope '%0' cannot have a literal operator member">;
+def err_literal_operator_id_outside_namespace
+    : Error<"non-namespace scope %0 cannot have a literal operator member">;
 def err_literal_operator_default_argument : Error<
   "literal operator cannot have a default argument">;
 def err_literal_operator_bad_param_count : Error<

clang/lib/AST/ASTDiagnostic.cpp

@@ -461,8 +461,9 @@ void clang::FormatASTNodeDiagnosticArgument(
     }
     case DiagnosticsEngine::ak_nestednamespec: {
       NestedNameSpecifier *NNS = reinterpret_cast<NestedNameSpecifier*>(Val);
-      NNS->print(OS, Context.getPrintingPolicy());
-      NeedQuotes = false;
+      NNS->print(OS, Context.getPrintingPolicy(),
+                 /*ResolveTemplateArguments=*/false,
+                 /*PrintFinalScopeResOp=*/false);
       break;
     }
     case DiagnosticsEngine::ak_declcontext: {

clang/lib/AST/NestedNameSpecifier.cpp

@@ -248,7 +248,8 @@ bool NestedNameSpecifier::containsErrors() const {
 /// Print this nested name specifier to the given output
 /// stream.
 void NestedNameSpecifier::print(raw_ostream &OS, const PrintingPolicy &Policy,
-                                bool ResolveTemplateArguments) const {
+                                bool ResolveTemplateArguments,
+                                bool PrintFinalScopeResOp) const {
   if (getPrefix())
     getPrefix()->print(OS, Policy);
 
@@ -269,7 +270,8 @@ void NestedNameSpecifier::print(raw_ostream &OS, const PrintingPolicy &Policy,
     break;
 
   case Global:
-    break;
+    OS << "::";
+    return;
 
   case Super:
     OS << "__super";
@@ -331,7 +333,8 @@ void NestedNameSpecifier::print(raw_ostream &OS, const PrintingPolicy &Policy,
   }
   }
 
-  OS << "::";
+  if (PrintFinalScopeResOp)
+    OS << "::";
 }
 
 LLVM_DUMP_METHOD void NestedNameSpecifier::dump(const LangOptions &LO) const {

clang/lib/Sema/SemaDecl.cpp

@@ -351,7 +351,7 @@ ParsedType Sema::getTypeName(const IdentifierInfo &II, SourceLocation NameLoc,
             Diag(QualifiedLoc, diag::warn_cxx17_compat_implicit_typename);
           else
             Diag(QualifiedLoc, diag::ext_implicit_typename)
-                << SS->getScopeRep() << II.getName()
+                << NestedNameSpecifier::Create(Context, SS->getScopeRep(), &II)
                 << FixItHint::CreateInsertion(QualifiedLoc, "typename ");
         }
 
@@ -795,9 +795,9 @@ void Sema::DiagnoseUnknownTypeName(IdentifierInfo *&II,
       DiagID = diag::ext_typename_missing;
 
     Diag(SS->getRange().getBegin(), DiagID)
-      << SS->getScopeRep() << II->getName()
-      << SourceRange(SS->getRange().getBegin(), IILoc)
-      << FixItHint::CreateInsertion(SS->getRange().getBegin(), "typename ");
+        << NestedNameSpecifier::Create(Context, SS->getScopeRep(), II)
+        << SourceRange(SS->getRange().getBegin(), IILoc)
+        << FixItHint::CreateInsertion(SS->getRange().getBegin(), "typename ");
     SuggestedType = ActOnTypenameType(S, SourceLocation(),
                                       *SS, *II, IILoc).get();
   } else {

clang/lib/Sema/SemaExpr.cpp

@@ -2940,6 +2940,9 @@ ExprResult Sema::BuildQualifiedDeclarationNameExpr(
   }
 
   if (const TypeDecl *TD = R.getAsSingle<TypeDecl>()) {
+    QualType Ty = Context.getTypeDeclType(TD);
+    QualType ET = getElaboratedType(ElaboratedTypeKeyword::None, SS, Ty);
+
     // Diagnose a missing typename if this resolved unambiguously to a type in
     // a dependent context.  If we can recover with a type, downgrade this to
     // a warning in Microsoft compatibility mode.
@@ -2948,8 +2951,7 @@ ExprResult Sema::BuildQualifiedDeclarationNameExpr(
       DiagID = diag::ext_typename_missing;
     SourceLocation Loc = SS.getBeginLoc();
     auto D = Diag(Loc, DiagID);
-    D << SS.getScopeRep() << NameInfo.getName().getAsString()
-      << SourceRange(Loc, NameInfo.getEndLoc());
+    D << ET << SourceRange(Loc, NameInfo.getEndLoc());
 
     // Don't recover if the caller isn't expecting us to or if we're in a SFINAE
     // context.
@@ -2960,11 +2962,9 @@ ExprResult Sema::BuildQualifiedDeclarationNameExpr(
     D << FixItHint::CreateInsertion(Loc, "typename ");
 
     // Recover by pretending this was an elaborated type.
-    QualType Ty = Context.getTypeDeclType(TD);
     TypeLocBuilder TLB;
     TLB.pushTypeSpec(Ty).setNameLoc(NameInfo.getLoc());
 
-    QualType ET = getElaboratedType(ElaboratedTypeKeyword::None, SS, Ty);
     ElaboratedTypeLoc QTL = TLB.push<ElaboratedTypeLoc>(ET);
     QTL.setElaboratedKeywordLoc(SourceLocation());
     QTL.setQualifierLoc(SS.getWithLocInContext(Context));
@@ -15433,7 +15433,7 @@ ExprResult Sema::BuildBinOp(Scope *S, SourceLocation OpLoc,
         Diag(OE->getQualifier() ? OE->getQualifierLoc().getBeginLoc()
                                 : OE->getNameLoc(),
              diag::err_template_kw_missing)
-          << OE->getName().getAsString() << "";
+            << OE->getName().getAsIdentifierInfo();
         return ExprError();
       }
     }
@@ -21025,18 +21025,24 @@ ExprResult Sema::CheckPlaceholderExpr(Expr *E) {
     NamedDecl *Temp = *ULE->decls_begin();
     const bool IsTypeAliasTemplateDecl = isa<TypeAliasTemplateDecl>(Temp);
 
-    if (NestedNameSpecifierLoc Loc = ULE->getQualifierLoc(); Loc.hasQualifier())
-      Diag(NameInfo.getLoc(), diag::err_template_kw_refers_to_type_template)
-          << Loc.getNestedNameSpecifier() << NameInfo.getName().getAsString()
-          << Loc.getSourceRange() << IsTypeAliasTemplateDecl;
-    else
-      Diag(NameInfo.getLoc(), diag::err_template_kw_refers_to_type_template)
-          << "" << NameInfo.getName().getAsString() << ULE->getSourceRange()
-          << IsTypeAliasTemplateDecl;
+    NestedNameSpecifier *NNS = ULE->getQualifierLoc().getNestedNameSpecifier();
+    TemplateName TN(dyn_cast<TemplateDecl>(Temp));
+    if (TN.isNull())
+      TN = Context.getAssumedTemplateName(NameInfo.getName());
+    TN = Context.getQualifiedTemplateName(NNS,
+                                          /*TemplateKeyword=*/true, TN);
+
+    Diag(NameInfo.getLoc(), diag::err_template_kw_refers_to_type_template)
+        << TN << ULE->getSourceRange() << IsTypeAliasTemplateDecl;
     Diag(Temp->getLocation(), diag::note_referenced_type_template)
         << IsTypeAliasTemplateDecl;
 
-    return CreateRecoveryExpr(NameInfo.getBeginLoc(), NameInfo.getEndLoc(), {});
+    QualType TST =
+        Context.getTemplateSpecializationType(TN, ULE->template_arguments());
+    QualType ET =
+        Context.getElaboratedType(ElaboratedTypeKeyword::None, NNS, TST);
+    return CreateRecoveryExpr(NameInfo.getBeginLoc(), NameInfo.getEndLoc(), {},
+                              ET);
   }
 
   // Overloaded expressions.

clang/lib/Sema/SemaTemplate.cpp

@@ -363,12 +363,12 @@ bool Sema::DiagnoseUnknownTemplateName(const IdentifierInfo &II,
 
   // The code is missing a 'template' keyword prior to the dependent template
   // name.
-  NestedNameSpecifier *Qualifier = (NestedNameSpecifier*)SS->getScopeRep();
-  Diag(IILoc, diag::err_template_kw_missing)
-    << Qualifier << II.getName()
-    << FixItHint::CreateInsertion(IILoc, "template ");
+  NestedNameSpecifier *Qualifier = (NestedNameSpecifier *)SS->getScopeRep();
   SuggestedTemplate
     = TemplateTy::make(Context.getDependentTemplateName(Qualifier, &II));
+  Diag(IILoc, diag::err_template_kw_missing)
+      << SuggestedTemplate.get()
+      << FixItHint::CreateInsertion(IILoc, "template ");
   SuggestedKind = TNK_Dependent_template_name;
   return true;
 }
@@ -660,7 +660,7 @@ void Sema::diagnoseExprIntendedAsTemplateName(Scope *S, ExprResult TemplateName,
   // was missing.
   if (MissingTemplateKeyword) {
     Diag(NameInfo.getBeginLoc(), diag::err_template_kw_missing)
-        << "" << NameInfo.getName().getAsString() << SourceRange(Less, Greater);
+        << NameInfo.getName() << SourceRange(Less, Greater);
     return;
   }
 
@@ -3762,16 +3762,17 @@ TypeResult Sema::ActOnTemplateIdType(
     //   elaborated-type-specifier (7.1.5.3).
     if (!LookupCtx && isDependentScopeSpecifier(SS)) {
       // C++2a relaxes some of those restrictions in [temp.res]p5.
+      NestedNameSpecifier *NNS =
+          NestedNameSpecifier::Create(Context, SS.getScopeRep(), TemplateII);
       if (AllowImplicitTypename == ImplicitTypenameContext::Yes) {
         if (getLangOpts().CPlusPlus20)
           Diag(SS.getBeginLoc(), diag::warn_cxx17_compat_implicit_typename);
         else
           Diag(SS.getBeginLoc(), diag::ext_implicit_typename)
-              << SS.getScopeRep() << TemplateII->getName()
+              << NNS
               << FixItHint::CreateInsertion(SS.getBeginLoc(), "typename ");
       } else
-        Diag(SS.getBeginLoc(), diag::err_typename_missing_template)
-            << SS.getScopeRep() << TemplateII->getName();
+        Diag(SS.getBeginLoc(), diag::err_typename_missing_template) << NNS;
 
       // FIXME: This is not quite correct recovery as we don't transform SS
       // into the corresponding dependent form (and we don't diagnose missing

clang/test/CXX/basic/basic.lookup/basic.lookup.qual/class.qual/p2.cpp

@@ -190,7 +190,7 @@ namespace InhCtor {
   }
   struct DerivedFromNS : NS::NS {
     // No special case unless the NNS names a class.
-    using InhCtor::NS::NS; // expected-error {{using declaration in class refers into 'InhCtor::NS::', which is not a class}}
+    using InhCtor::NS::NS; // expected-error {{using declaration in class refers into 'InhCtor::NS', which is not a class}}
 
   };
 

clang/test/CXX/class.access/class.access.dcl/p1.cpp

@@ -331,7 +331,7 @@ namespace test4 {
     // expected-warning@-2 {{access declarations are deprecated; use using declarations instead}}
 #else
     // expected-error@-4 {{ISO C++11 does not allow access declarations; use using declarations instead}}
-    // expected-error@-5 {{using declaration refers into 'Subclass::', which is not a base class of 'C'}}
+    // expected-error@-5 {{using declaration refers into 'Subclass', which is not a base class of 'C'}}
 #endif
 
     int bar();

clang/test/CXX/class.access/class.friend/p3-cxx0x.cpp

@@ -36,7 +36,7 @@ class A {
 public:
   class foo {};
   static int y;
-  template <typename S> friend class B<S>::ty; // expected-warning {{dependent nested name specifier 'B<S>::' for friend class declaration is not supported}}
+  template <typename S> friend class B<S>::ty; // expected-warning {{dependent nested name specifier 'B<S>' for friend class declaration is not supported}}
 };
 
 template<typename T> class B { typedef int ty; };