wlav / cppyy

Other
411 stars 42 forks source link

🐛 cppyy appears to add an extra `void` template parameter to the end of `std::tuple_cat` template params #261

Open lukevalenty opened 1 month ago

lukevalenty commented 1 month ago

I ran into a strange problem trying to use std::tuple_cat(...). It looks like an extra void template type parameter is being added to the end of the list of template parameters. I minimized the problem down to the following C++ code tested and working in clang 13:   https://godbolt.org/z/54nhqK55T

#include <tuple>
constexpr auto actual = std::tuple_cat();

The cppyy version fails, however:

import cppyy
def test_tuple_cat():
    cppyy.gbl.std.tuple_cat()

The error message below shows the extra void template type parameter:

_____________________________________________________ test_tuple_cat ______________________________________________________

    def test_tuple_cat():
>       std = cppyy.gbl.std.tuple_cat()
E       ValueError: Template method resolution failed:
E         std::tuple<> std::tuple_cat() =>
E           ValueError: nullptr result where temporary expected
E         std::tuple<> std::tuple_cat() =>
E           ValueError: nullptr result where temporary expected

test/pbt/tuple.py:89: ValueError
-------------------------------------------------- Captured stderr call ---------------------------------------------------
input_line_17:7:48: error: expected expression
      new (ret) (std::tuple<>) (std::tuple_cat<, void>());
                                               ^
input_line_17:11:22: error: expected expression
      std::tuple_cat<, void>();
                     ^
input_line_18:7:48: error: expected expression
      new (ret) (std::tuple<>) (std::tuple_cat<, void>());
                                               ^
input_line_18:11:22: error: expected expression
      std::tuple_cat<, void>();
                     ^
================================================= short test summary info =================================================
FAILED test/pbt/tuple.py::test_tuple_cat - ValueError: Template method resolution failed:
==================================================== 1 failed in 0.96s ====================================================

This seems to fail for any usage of std::tuple_cat...the exact args do not appear to matter.

wlav commented 1 month ago

This seems to be a regression that was introduced in the update to LLVM16. It works fine in current cppyy. There are a few changes in the update to the iteration over template parameters, so that offers a clear lead. There's also a current test failing (the use of global lambda`s) that I believe to be related.

lukevalenty commented 1 month ago

@wlav, where's the template parameter iteration code? Maybe I can take a look.

wlav commented 1 month ago

Look for the word "iterator" in this patch file: https://github.com/wlav/cppyy-backend/commit/c5d853105026358840906145a9375af1ca238c66.patch

I copied these over from upstream, but eg. this:

@@ -4456,16 +4458,16 @@ clang::QualType CppyyLegacy::TMetaUtils::ReSubstTemplateArg(clang::QualType inpu
    if (inputTST) {
       bool mightHaveChanged = false;
       llvm::SmallVector<clang::TemplateArgument, 4> desArgs;
-      for(clang::TemplateSpecializationType::iterator I = inputTST->begin(), E = inputTST->end();
-          I != E; ++I) {
-         if (I->getKind() != clang::TemplateArgument::Type) {
-            desArgs.push_back(*I);
+      for (const clang::TemplateArgument &TA : inputTST->template_arguments()) {
+         if (TA.getKind() != clang::TemplateArgument::Type) {
+            desArgs.push_back(TA);
             continue;
          }

-         clang::QualType SubTy = I->getAsType();
+         clang::QualType SubTy = TA.getAsType();
          // Check if the type needs more desugaring and recurse.
-         if (llvm::isa<clang::SubstTemplateTypeParmType>(SubTy)
+         if (llvm::isa<clang::ElaboratedType>(SubTy)
+             || llvm::isa<clang::SubstTemplateTypeParmType>(SubTy)
              || llvm::isa<clang::TemplateSpecializationType>(SubTy)) {
             clang::QualType newSubTy = ReSubstTemplateArg(SubTy,instance);
             mightHaveChanged = SubTy != newSubTy;

have changed types and thus maybe changed behavior.

lukevalenty commented 1 month ago

Thanks for that info.

I created a docker image to make sure that I was using the current released version of cppyy, but it still fails:

# Use the latest Ubuntu LTS as the base image
FROM ubuntu:22.04

# Install required packages and development tools
RUN apt-get update && \
    DEBIAN_FRONTEND=noninteractive apt-get install -y \
    python3 \
    python3-pip \
    vim

# Install necessary Python packages
RUN python3 -m pip install --upgrade pip pytest cppyy 

# Set the working directory
WORKDIR /opt

RUN cat <<EOF > test.py
import cppyy
def test_tuple_cat():
    cppyy.gbl.std.tuple_cat()
EOF

# Run the test
CMD ["pytest", "test.py"]

Build and run the image (make sure the Dockerfile is in its own directory):

docker build -t test_cppyy_empty_tuple_cat .
docker run -it test_cppyy_empty_tuple_cat 

You should see the following output:

================================================================= test session starts ==================================================================
platform linux -- Python 3.10.12, pytest-8.3.3, pluggy-1.5.0
rootdir: /opt
collected 1 item

test.py F                                                                                                                                        [100%]

======================================================================= FAILURES =======================================================================
____________________________________________________________________ test_tuple_cat ____________________________________________________________________

    def test_tuple_cat():
>       cppyy.gbl.std.tuple_cat()
E       ValueError: Template method resolution failed:
E         std::tuple<> std::tuple_cat() =>
E           ValueError: nullptr result where temporary expected
E         std::tuple<> std::tuple_cat() =>
E           ValueError: nullptr result where temporary expected

test.py:3: ValueError
----------------------------------------------------------------- Captured stderr call -----------------------------------------------------------------
input_line_17:7:48: error: expected expression
      new (ret) (std::tuple<>) (std::tuple_cat<, void>());
                                               ^
input_line_17:11:22: error: expected expression
      std::tuple_cat<, void>();
                     ^
input_line_18:7:48: error: expected expression
      new (ret) (std::tuple<>) (std::tuple_cat<, void>());
                                               ^
input_line_18:11:22: error: expected expression
      std::tuple_cat<, void>();
                     ^
=============================================================== short test summary info ================================================================
FAILED test.py::test_tuple_cat - ValueError: Template method resolution failed:
================================================================== 1 failed in 0.33s ===================================================================
wlav commented 1 month ago

I'll go back to that in a bit (digging into #262 atm.). And yes, I was a bit lazy in that I have two differences from the actual release: master just prior to the LLVM16 changes and Mac rather than Linux. So it could be working on Mac b/c of a different set of standard headers as opposed to the LLVM16 changes.

lukevalenty commented 1 month ago

Ok, no worries! I'll dig into the template parameter iteration code. 👍

lukevalenty commented 1 month ago

I'll go back to that in a bit (digging into #262 atm.). And yes, I was a bit lazy in that I have two differences from the actual release: master just prior to the LLVM16 changes and Mac rather than Linux. So it could be working on Mac b/c of a different set of standard headers as opposed to the LLVM16 changes.

That makes sense...maybe you're compiling with clang and libc++ on mac?

wlav commented 1 month ago

Yes, and tuple itself is handled differently now that I think about it. Had some compiler-internal type.

wlav commented 1 month ago

Interestingly, yes, I do get the broken type name of std::tuple_cat<, void> with the released version, but get this: std::tuple_cat<void> with master. This, too, happens in the type name normalization, so the fix for #262 has probably improved that part.

Still invalid code, of course, so still digging where that void comes from.