Possible regression in libclang clang_getTypeSpelling

[gjasny]

Hello,

I inherited a code generator that is based on luaclang and the libclang C interface.
That generator can be used to create pimpl-like proxy interfaces or Google Mock classes for existing interfaces.

While upgrading libclang from v15 to v16 I noticed a change in clang_getTypeSpelling. Before v16 it printed fully qualified namespaces for function argument types, now only the types as written in the declaration itself.

I created a small test case within the libclang Python binding test:

diff --git a/clang/bindings/python/tests/cindex/test_type.py b/clang/bindings/python/tests/cindex/test_type.py
index efe9b0f50be8..4f1ade7ab2f1 100644
--- a/clang/bindings/python/tests/cindex/test_type.py
+++ b/clang/bindings/python/tests/cindex/test_type.py
@@ -466,3 +466,30 @@ class TestType(unittest.TestCase):
         # Variable without a template argument.
         cursor = get_cursor(tu, 'bar')
         self.assertEqual(cursor.get_num_template_arguments(), -1)
+
+    def test_type_spelling_namespace(self):
+        """Ensure Type.spelling prints whole namespace."""
+
+        source ="""
+namespace X {
+    namespace Y {
+        struct Params {
+            int i;
+        };
+        struct Foo {
+            virtual void f(const Params& p) = 0;
+        };
+    }
+}
+    """
+        tu = get_tu(source, lang='cpp')
+        f = get_cursor(tu, 'f')
+        self.assertIsNotNone(f)
+
+        args = f.type.argument_types()
+        self.assertIsNotNone(args)
+        self.assertEqual(len(args), 1)
+
+        t0 = args[0]
+        self.assertIsNotNone(t0)
+        self.assertEqual(t0.spelling, 'const X::Y::Params &')

The test works as expected on the release/15.x branch but fails on the release/16.x branch:

$ ninja check-clang-python
...
======================================================================
FAIL: test_type_spelling_namespace (tests.cindex.test_type.TestType)
Ensure Type.is_function_variadic works.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/gregorj/src/llvm-project/clang/bindings/python/tests/cindex/test_type.py", line 495, in test_type_spelling_namespace
    self.assertEqual(t0.spelling, 'const X::Y::Params &')
AssertionError: 'const Params &' != 'const X::Y::Params &'
- const Params &
+ const X::Y::Params &
?       ++++++

Would the change in behaviour above qualify as a regression? If not, is there a way to restore the old behaviour somehow?

Thanks,
Gregor

[gjasny]

It bisects down to 15f3cd6 (from @mizvekov).

[diseraluca]

We have the same issue for QDoc, where it is blocking our transition to Clang 16.

We independently found 15f3cd6 to be the culprit too.

I've been trying to restore the same behavior but failed in doing so.

PrintingPolicy does not seem to help independently of how we set it up.
The upcoming SuppressElaboration does seem to do the most of the trick, but it is not in 16 (And I haven't seen a patch adding it to libclang either).

Lowering to the C++ AST and playing from there has not been working much either, as the code that libclang internally uses has been equally touched.

The only thing that comes near that I could find was manually replicating much of the work done in "QualTypeNames.cpp", with some special casing, but I still haven't reached parity with the previous behavior.

[shafik]

CC @AaronBallman @erichkeane because I don't know who to tag in here

[mizvekov]

I think this is essentially a duplicate of #58040

This relied on an inconsistent behavior that only worked for the simple case the name had no kw / qualifiers, as soon as you would add one, you see the type name as written, instead of fully qualified.
That patch just made that print as-written for all cases.

The recommended approach is indeed to use clang::TypeName::getFullyQualifiedName from QualTypeNames.cpp.

[diseraluca]

Thank you for your answer @mizvekov .

I apologize as I hadn't found #58040, which would have shed some more light on our problem.

For our case, clang::TypeName::getFullyQualifiedName is not going to cut it as it does a bit too much. For example, it fully qualifies all template parameters, which in turn breaks quite a lot of our downstream code that wasn't expecting that.

I think I will need to improve our, quite old, code, as I understand it was really working because of a bug.
Nonetheless, I was wondering if it might makes sense to expose some more of the code in QualTypeNames.cpp.

For example, for our case, it would have been great to have access to createNestedNameSpecifierForScopeOf(ASTContext, const Type*, ...) which would have helped in transitioning while keeping backwards compatibility, so that we could improve our code with a better foundation and without having some important tasks blocked.

Apart from this, at least on our side, the answer you gave is completely satisfying, and we thank you for that, and consider this bug to be closable.

[mizvekov]

Don't worry, no need for apologies :)

I think exposing createNestedNameSpecifierForScopeOf would be fine, this would be an easy first patch if you want to get involved.

[mizvekov]

Duplicate of #58040