[clang] [clang-tools-extra] [clang] [Sema] Preserve nested name specifier prefix in MemberPointerType (PR #118236)

[Matheus Izvekov via cfe-commits]

================
@@ -5347,15 +5347,18 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
 
         case NestedNameSpecifier::TypeSpec:
         case NestedNameSpecifier::TypeSpecWithTemplate:
-          ClsType = QualType(NNS->getAsType(), 0);
-          // Note: if the NNS has a prefix and ClsType is a nondependent
-          // TemplateSpecializationType, then the NNS prefix is NOT included
-          // in ClsType; hence we wrap ClsType into an ElaboratedType.
-          // NOTE: in particular, no wrap occurs if ClsType already is an
-          // Elaborated, DependentName, or DependentTemplateSpecialization.
-          if (isa<TemplateSpecializationType>(NNS->getAsType()))
+          const Type *NNSType = NNS->getAsType();
+          ClsType = QualType(NNSType, 0);
+          // If ClsType is an Elaborated, DependentName, or
+          // DependentTemplateSpecialization, it already stores the NNS prefix.
+          // Otherwise, wrap it in an Elaborated type to have a place to store
+          // the NNS prefix.
+          if (!(isa<ElaboratedType>(NNSType) ||
+                isa<DependentNameType>(NNSType) ||
+                isa<DependentTemplateSpecializationType>(NNSType))) {
----------------
mizvekov wrote:

This seems to be testing for conditions which seem impossible, the NNS would be malformed otherwise.

For example, what would it mean for the NNS type to be an ElaboratedType? If the NNS has a prefix, which would take precedence, that prefix or the ElaboratedType's prefix? If there is no prefix, what sense would it convey beyond using the Elaborated prefix as its prefix instead?

For DependentNameType, that would be redundant with how we represent these as Identifier + prefix, so we should prefer that over forming a NNS with such type.

The DTST is an exception here, as we lack a NNS kind for an identifier + list of template arguments, but then such DTST should not store a NNS itself, otherwise again this would be a bug somewhere else, as that would be redundant / confusing with the NNS' own prefix as explained above.

So if we are getting either an ElaboratedType or a DependentNameType here, there seems to be a bug elsewhere.

Regarding the explanation for DependentTemplateSpecialization: even if the type node itself can have a nested name, it should not be present when the node is used as the type for a NNS node itself.

We should handle a DTST here by rebuilding it, adding the NNSPrefix as the DTST's nested name.

https://github.com/llvm/llvm-project/pull/118236