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.

Sign up for GitHub

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

Jump to bottom

std::tuple compilation errors with implicit conversion constructors #29493

Closed
llvmbot opened this issue Aug 24, 2016 · 14 comments
Closed

std::tuple compilation errors with implicit conversion constructors #29493

llvmbot opened this issue Aug 24, 2016 · 14 comments
Labels
bugzilla Issues migrated from bugzilla libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Comments

@llvmbot
Copy link
Collaborator

llvmbot commented Aug 24, 2016

Bugzilla Link 29123
Resolution FIXED
Resolved on Oct 09, 2017 17:50
Version 3.9
OS All
Attachments compilation error message
Reporter LLVM Bugzilla Contributor
CC @compnerd,@dlj-NaN,@zmodem,@K-ballo,@mclow,@rengolin,@zygoloid

Extended Description

I encountered this error when trying to use libcxx from 3.9 release and trunk from around August 1st. The same code works with 3.8 release

Repro: compile following example with libc++:
clang -c -std=c++14 repro.cpp -stdlib=libc++

// repro.cpp
// this is minimized code from folly library
#include
template
struct Optional {
// implicit
Optional(const Value& newValue) {}
};

struct dynamic {
// implicit
template dynamic(T t);
};

Optional<std::tuple> get();

void test() { auto x = get(); }
@llvmbot
Copy link
Collaborator Author

llvmbot commented Aug 24, 2016

Darn it.

@zmodem
Copy link
Collaborator

zmodem commented Aug 25, 2016

Eric, can you give an assessment of the severity of this bug and how intrusive a fix might be?

At this stage in the 3.9 process (rc3 was just tagged), I'm reluctant to take patches for anything but regressions from earlier release candidates.

@llvmbot
Copy link
Collaborator Author

llvmbot commented Aug 25, 2016

Eric, can you give an assessment of the severity of this bug and how intrusive a fix might be?

It's a regression from 3.8 and tuple tends to get a lot of usage, and breakage is often noticed by many users. That's why I would like to fix this before 3.9.

The fix should hopefully be small. I'll give a more complete analysis tomorrow.

@llvmbot
Copy link
Collaborator Author

llvmbot commented Aug 28, 2016

So this may be a Clang bug. It seems that GCC accepts the reproducer just fine. I still would like to see this fixed in Clang before 3.9 but that might be a harder change to make.

@llvmbot
Copy link
Collaborator Author

llvmbot commented Aug 29, 2016

Attempted fix: https://reviews.llvm.org/D23978

The bug seems to be caused only when __enable_implicit() is defined as a constexpr function. Defining it as a alias template fixes the bug. I have no idea why this changes anything or why the original error is generated at all.

@zmodem
Copy link
Collaborator

zmodem commented Aug 29, 2016

Attempted fix: https://reviews.llvm.org/D23978

The bug seems to be caused only when __enable_implicit() is defined as a
constexpr function. Defining it as a alias template fixes the bug. I have no
idea why this changes anything or why the original error is generated at all.

Richard, can you shed some light on why the proposed patch makes a difference here?

@zmodem
Copy link
Collaborator

zmodem commented Aug 30, 2016

After speaking with Richard about this, it sounds like we don't have a lot of good options to choose from here (there seems to be both issues with Clang and the C++ standard library).

Taking Eric's D23978 is probably the least bad thing, doesn't make anything worse, and will fix this PR. So can we go ahead and land it?

@rengolin
Copy link
Member

I have no reservations, and Richard's word is good enough for me in this case. :)

We can change that on trunk later, if necessary.

@llvmbot
Copy link
Collaborator Author

llvmbot commented Dec 9, 2016

Update:

I choose not to fix this bug in libc++ for 3.9.0 for two reasons:

  1. This was also caused by a Clang bug, which was fixed shortly after the 3.9.0 release and it has been applied to the upcoming 3.9.1 release. I didn't want to re-write large portions of std::tuple right before a release in order to avoid a Clang bug.
  2. The reproducer has a cyclic SFINAE condition, and I would consider that UB.

Closing this bug as resolved fixed since it compiles with ToT clang.

@llvmbot
Copy link
Collaborator Author

llvmbot commented Oct 10, 2017

*** Bug llvm/llvm-bugzilla-archive#34895 has been marked as a duplicate of this bug. ***

@llvmbot
Copy link
Collaborator Author

llvmbot commented Oct 10, 2017

See https://reviews.llvm.org/D23999 for more information.

@llvmbot
Copy link
Collaborator Author

llvmbot commented Nov 26, 2021

mentioned in issue llvm/llvm-bugzilla-archive#30261

@llvmbot
Copy link
Collaborator Author

llvmbot commented Nov 26, 2021

mentioned in issue llvm/llvm-bugzilla-archive#30301

@llvmbot
Copy link
Collaborator Author

llvmbot commented Nov 26, 2021

mentioned in issue llvm/llvm-bugzilla-archive#34895

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bugzilla Issues migrated from bugzilla libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@rengolin @zmodem @llvmbot