Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
lukevalenty opened this issue Sep 28, 2024 · 9 comments

Comments

@lukevalenty
Copy link

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
Copy link
Owner

wlav commented Sep 30, 2024

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
Copy link
Author

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

@wlav
Copy link
Owner

wlav commented Sep 30, 2024

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
Copy link
Author

lukevalenty commented Sep 30, 2024

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
Copy link
Owner

wlav commented Sep 30, 2024

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
Copy link
Author

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

@lukevalenty
Copy link
Author

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
Copy link
Owner

wlav commented Oct 1, 2024

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

@wlav
Copy link
Owner

wlav commented Oct 2, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants