Minimal example of a bug from cppyy

[Vipul-Cariappa]

That needs to be fixed at CppInterOp
reference: look at comments at compiler-research/cppyy-backend#116

---

I have written a test case that is a minimal example of a failing test at cppyy (suspected).
I am not sure how to fix this. More experienced people here could take a look at it.

[Vipul-Cariappa]

The assertion fails here with

"Container" != "my_namespace::Container"

I need the fully qualified type name.

[vgvassilev]

Wouldn't GetQualifiedName or GetQualifiedCompleteName help?

[faze-geek]

Well the following diff in GetVariableType passed the above test for me. I may be wrong due to lack of context.

--- a/lib/Interpreter/CppInterOp.cpp
+++ b/lib/Interpreter/CppInterOp.cpp
@@ -1216,7 +1216,10 @@ namespace Cpp {
     auto D = (Decl *) var;
 
     if (auto DD = llvm::dyn_cast_or_null<DeclaratorDecl>(D)) {
-      return DD->getType().getAsOpaquePtr();
+      QualType QT = DD->getType();
+
+      // Ensure the type includes the namespace.
+      QT = QT.getCanonicalType();
+      return QT.getAsOpaquePtr();
     }

[vgvassilev]

If the change makes the test pass that's a good sign.

[faze-geek]

Well, let me run all the tests and get back to you.

[faze-geek]

@vgvassilev I ran all the tests now, all tests pass but the typedef int I; failed in TypeReflectionTest.cpp, so changing the above diff makes all tests pass. What is your opinion here ?

TCppType_t GetVariableType(TCppScope_t var)
{
    auto D = (Decl *) var;

    if (auto DD = llvm::dyn_cast_or_null<DeclaratorDecl>(D)) {
        QualType QT = DD->getType();

        // Check if the type is a typedef type
        if (QT->isTypedefNameType()) {
            return QT.getAsOpaquePtr();
        }

        // Else, return the canonical type
        QT = QT.getCanonicalType();
        return QT.getAsOpaquePtr();
    }

    return 0;
}

[Vipul-Cariappa]

@faze-geek, I did not test your changes locally. But if all existing tests and the test from this PR pass, you can open a PR of this branch (create a new branch from this branch). I will take a look at it. We may extend the test with other combinations.

Thanks for looking into this. 😄

[faze-geek]

Sure ! Will make the changes soon.

[github-actions]

clang-tidy made some suggestions

[github-actions]

warning: no header providing "std::string" is directly included [misc-include-cleaner]

unittests/CppInterOp/VariableReflectionTest.cpp:6:

+ #include <string>

[mcbarton]

@Vipul-Cariappa has this bug been fixed now?