Expose ElaboratedType through LibClang interface.
Details
Details
Diff Detail
Diff Detail
Event Timeline
| 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? | |
Comment Actions
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.
Comment Actions
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.
Comment Actions
Is this still ready for submission or do you want to change something (not clear after your last comment :)
Comment Actions
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.
Comment Actions
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.
Comment Actions
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?
Comment Actions
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.
Comment Actions
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.
| 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...? | |
Comment Actions
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.
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?