[LIbClang] Report the named type for ElaboratedType

[skalinichev]

Summary

Expose ElaboratedType through LibClang interface.

[skalinichev]

Fixed minor formatting issues.

[modocache]

bindings/python/clang/cindex.py

1875 ↗

(On Diff #31503)

Awesome, thanks for updating the Python bindings as well!

As with the C function, I believe this is a typo--shouldn't it be getElaboratedTypeUnderlyingType?

Furthermore, the convention in Python (and in this file) is to use snake_case, so this should probably be get_elaborated_type_underlying_type.

include/clang-c/Index.h

2859 ↗

(On Diff #31503)

Tangential to this diff, but is there a reason these CXTypeKind enum values are undocumented? I can see how many would be familiar to most users, but I don't think "elaborated" is something that many users would immediately recognize. Should these be documented?

3224 ↗

(On Diff #31503)

nit: Other \brief documentation in this file uses periods at the end of sentences. For example, clang_getArraySize, the function immediately prior to this newly added function, has the brief -\brief Return the array size of a constant array.--notice the trailing period.

3226 ↗

(On Diff #31503)

nit: Rather than reference the CXType_Invalid type directly, most of the other documentation in this file states "an invalid type is returned."

3228 ↗

(On Diff #31503)

I believe this is a typo--shouldn't it be clang_getElaboratedTypeUnderlyingType?

tools/libclang/CXType.cpp

987

The documentation of getNamedType() itself states "Retrieve the type named by the qualified-id.". As such, would it make sense to call this function clang_getNamedTypeUnderlyingElaboratedType, instead of clang_getElaboratedTypeUnderlyingType?

[skalinichev]

Address raised issues.

modocache, great review btw, thanks!

[modocache]

Looks good to me! I don't have commit access, though, so you'll probably have to find a committer to review/commit for you, @skalinichev.

[skalinichev]

Ping? Can someone commit it?

[skalinichev]

It just occured to me, why do we need to expose the ElaboratedType if we can retrieve the original type and return it to user. Especially when we already do that for Attributed and Decayed types. So here is a much simplier solution.

[klimek]

Is this still ready for submission or do you want to change something (not clear after your last comment :)

[skalinichev]

Rebased.

Yes, now it's ready.

[klimek]

Ok, so looking over the new patch:
It seems like attributed types and decayed types are somewhat different from elaborated types:

• decayed types don't lose any information if we look through to the original type
• attributed types lose only information that can be considered an implementation detail
• losing the nested name specifiers a user wrote can be considered part of the name (the lookup will be different)

I'm happy to be convinced that this doesn't matter here, and that inconsistencies like in the changed tests (canonical type: elaborated, pointeetype: not elaborated) are benign.

[skalinichev]

Yes, I've noticed that inconsistency too. But, seems like, this is an unrelated issue: TemplateSpecializationType type for some reasons uses id instead of qid (See also all other tests with templates in print-type.cpp, e.g. FieldDecl=baz). To get the type with nested name specifiers for the TemplateSpecializationType c-index-test should use the clang_getCanonicalType. But it doesn't use it for pointers. (Compare c-index-test.c:1289 and c-index-test.c:1335)

Here I'm also adding an another test to show that nested name specifiers are not lost.

[klimek]

Ok, so just for my understanding: the nested name specifier in the changed tests canonicaltype is *not* the user-written type (thus, what the elaborated type would get us), but the full nested name specifier of the canonical type?

[skalinichev]

AFAIUI the canonical type could never be an elaborated type, but just type could.

And yes before we used nested name specifier from the type name as written in the source code (a.k.a. ElaboratedType), now we use nested name specifier of the type that the elaborated type refers to.

[skalinichev]

Ok, after some more testing I found out that elaborated types can have cv specifiers attached, but after using ET->getNamedType() we lose that information. So obviously the previous solution was wrong. Therefore I'm falling back to the original idea here once again: exposing the elaborated type. Also extending tests to account for the found bug.

[kfunk]

tools/libclang/CXType.cpp

990

I'm not happy with this name either. If you look through the libclang API, functions are named like clang_get${CPPCLASSNAME}${CPPMETHOD}. E.g. clang_getEnumConstantDeclValue, clang_getFieldDeclBitWidth, clang_getFunctionTypeCallingConv.

Sergey's initial version was closer to that. Maybe clang_getElaboratedTypeNamedType will just do...?

[skalinichev]

Looking a bit more at the recent commits in libclang, it seems like all recently added functions follow this naming convention:
clang_Cursor_*
clang_Type_*
e.t.c.

So I'm renaming the function to follow that style too.

[skalinichev]

Ok, If there are no objections, I'll commit this revision next week then.

test/Index/print-type.c

 
 int f2(int incompletearray[]);
 
+enum Enum{i}; enum Enum elaboratedEnumType();
+struct Struct{}; struct Struct elaboratedStructType();
+
 // RUN: c-index-test -test-print-type %s | FileCheck %s
 // CHECK: FunctionDecl=f:3:6 (Definition) [type=int *(int *, char *, FooType, int *, void (*)(int))] [typekind=FunctionProto] [canonicaltype=int *(int *, char *, int, int *, void (*)(int))] [canonicaltypekind=FunctionProto] [resulttype=int *] [resulttypekind=Pointer] [args= [int *] [Pointer] [char *] [Pointer] [FooType] [Typedef] [int [5]] [ConstantArray] [void (*)(int)] [Pointer]] [isPOD=0]
 // CHECK: ParmDecl=p:3:13 (Definition) [type=int *] [typekind=Pointer] [isPOD=1] [pointeetype=int] [pointeekind=Int]

 // CHECK: VarDecl=x:10:38 [type=__attribute__((__vector_size__(4 * sizeof(int)))) int] [typekind=Vector] [isPOD=1]
 // CHECK: TypedefDecl=int4_t:11:46 (Definition) [type=int4_t] [typekind=Typedef] [canonicaltype=__attribute__((__vector_size__(4 * sizeof(int)))) int] [canonicaltypekind=Vector] [isPOD=1]
 // CHECK: ParmDecl=incompletearray:13:12 (Definition) [type=int []] [typekind=IncompleteArray] [isPOD=1]
+// CHECK: FunctionDecl=elaboratedEnumType:15:25 [type=enum Enum ()] [typekind=FunctionNoProto] [canonicaltype=enum Enum ()] [canonicaltypekind=FunctionNoProto] [resulttype=enum Enum] [resulttypekind=Enum] [isPOD=0]
+// CHECK: TypeRef=enum Enum:15:6 [type=enum Enum] [typekind=Enum] [isPOD=1]
+// CHECK: StructDecl=Struct:16:8 (Definition) [type=struct Struct] [typekind=Record] [isPOD=1]
+// CHECK: FunctionDecl=elaboratedStructType:16:32 [type=struct Struct ()] [typekind=FunctionNoProto] [canonicaltype=struct Struct ()] [canonicaltypekind=FunctionNoProto] [resulttype=struct Struct] [resulttypekind=Record] [isPOD=0]
+// CHECK: TypeRef=struct Struct:16:8 [type=struct Struct] [typekind=Record] [isPOD=1]

test/Index/print-type.cpp

 };
 int Blob::*member_pointer;
 
+namespace NS { struct Type{}; } NS::Type elaboratedNamespaceType();
+
 // RUN: c-index-test -test-print-type %s -std=c++11 | FileCheck %s
 // CHECK: Namespace=outer:1:11 (Definition) [type=] [typekind=Invalid] [isPOD=0]
 // CHECK: ClassTemplate=Foo:4:8 (Definition) [type=] [typekind=Invalid] [isPOD=0]

 // CHECK: Namespace=inner:14:11 (Definition) [type=] [typekind=Invalid] [isPOD=0]
 // CHECK: StructDecl=Bar:16:8 (Definition) [type=outer::inner::Bar] [typekind=Record] [isPOD=0] [nbFields=3]
 // CHECK: CXXConstructor=Bar:17:3 (Definition) [type=void (outer::Foo<bool> *){{.*}}] [typekind=FunctionProto] [canonicaltype=void (outer::Foo<bool> *){{.*}}] [canonicaltypekind=FunctionProto] [resulttype=void] [resulttypekind=Void] [args= [outer::Foo<bool> *] [Pointer]] [isPOD=0]
-// CHECK: ParmDecl=foo:17:25 (Definition) [type=outer::Foo<bool> *] [typekind=Pointer] [canonicaltype=outer::Foo<bool> *] [canonicaltypekind=Pointer] [isPOD=1] [pointeetype=outer::Foo<bool>] [pointeekind=Unexposed]
+// CHECK: ParmDecl=foo:17:25 (Definition) [type=outer::Foo<bool> *] [typekind=Pointer] [canonicaltype=outer::Foo<bool> *] [canonicaltypekind=Pointer] [isPOD=1] [pointeetype=Foo<bool>] [pointeekind=Unexposed]
 // CHECK: NamespaceRef=outer:1:11 [type=] [typekind=Invalid] [isPOD=0]
 // CHECK: TemplateRef=Foo:4:8 [type=] [typekind=Invalid] [isPOD=0]
 // CHECK: CompoundStmt= [type=] [typekind=Invalid] [isPOD=0]

 // CHECK: StructDecl=Blob:45:8 (Definition) [type=Blob] [typekind=Record] [isPOD=1] [nbFields=2]
 // CHECK: FieldDecl=i:46:7 (Definition) [type=int] [typekind=Int] [isPOD=1]
 // CHECK: VarDecl=member_pointer:49:12 (Definition) [type=int Blob::*] [typekind=MemberPointer] [isPOD=1]
+// CHECK: FunctionDecl=elaboratedNamespaceType:51:42 [type=NS::Type ()] [typekind=FunctionProto] [canonicaltype=NS::Type ()] [canonicaltypekind=FunctionProto] [resulttype=NS::Type] [resulttypekind=Record] [isPOD=0]
+// CHECK: NamespaceRef=NS:51:11 [type=] [typekind=Invalid] [isPOD=0]
+// CHECK: TypeRef=struct NS::Type:51:23 [type=NS::Type] [typekind=Record] [isPOD=1]

tools/libclang/CXType.cpp

     if (const DecayedType *DT = T->getAs<DecayedType>()) {
       return MakeCXType(DT->getOriginalType(), TU);
     }
+
+    /* Handle elaborated types as the original type */
+    if (const auto* ET = T->getAs<ElaboratedType>()) {
+      return MakeCXType(ET->getNamedType(), TU);
+    }
   }
   if (TK == CXType_Invalid)
     TK = GetTypeKind(T);

 
   Decl *D = nullptr;
 
-try_again:
   switch (TP->getTypeClass()) {
   case Type::Typedef:
     D = cast<TypedefType>(TP)->getDecl();

 
   // FIXME: Template type parameters!      
 
-  case Type::Elaborated:
-    TP = cast<ElaboratedType>(TP)->getNamedType().getTypePtrOrNull();
-    goto try_again;
-    
   default:
     break;
   }