llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.42k stars 12.15k forks source link

clang-cl emits bogus error: cannot decompose this type; 'std::tuple_size<const S>::value' is not a valid integral constant expression #32583

Closed StephanTLavavej closed 3 years ago

StephanTLavavej commented 7 years ago
Bugzilla Link 33236
Resolution FIXED
Resolved on Aug 20, 2019 03:01
Version 4.0
OS Windows NT
Blocks llvm/llvm-project#41819
Attachments Self-contained test case
CC @zmodem,@JVApen,@mclow,@zygoloid,@rnk

Extended Description

Note: This is using my implementation of LWG 2770 tuple_size SFINAE, which will ship in VS 2017's first toolset update (aka "15.3").

C:\Temp>type meow.cpp

include

include

struct S { int x; };

int main() { S s{99}; auto [m1] = s; auto& [r1] = s; assert(m1 == 99); assert(&r1 == &s.x);

const S cs{1729};
auto [m2] = cs;
auto& [r2] = cs;
assert(m2 == 1729);
assert(&r2 == &cs.x);

}

C:\Temp>cl Microsoft (R) C/C++ Optimizing Compiler Version 19.11.25326 for x64 Copyright (C) Microsoft Corporation. All rights reserved.

usage: cl [ option... ] filename... [ /link linkoption... ]

C:\Temp>cl /EHsc /nologo /W4 /std:c++latest meow.cpp && meow meow.cpp

C:\Temp>clang-cl -v clang version 4.0.0 (tags/RELEASE_400/final) Target: x86_64-pc-windows-msvc Thread model: posix InstalledDir: S:\WCFB01\vctools\NonShip\ClangLLVM\bin

C:\Temp>clang-cl /EHsc /nologo /W4 /std:c++latest meow.cpp && meow meow.cpp(17,11): error: cannot decompose this type; 'std::tuple_size::value' is not a valid integral constant expression auto& [r2] = cs; ^ 1 error generated.

I believe that this is the same issue as what I reported to the reflectors on April 3, 2017. C1XX has implemented my preferred resolution.

-- Subject: [isocpp-lib] New issue - Core and Library disagree on tuple_size SFINAE (LWG 2770)

LWG 2770's resolution taught tuple_size to SFINAE. However, Core's wording for structured bindings disagrees with what the Library provides.

N4659 23.5.3.6 [tuple.helper]/4: "template class tuple_size; template class tuple_size; template class tuple_size; Let TS denote tuple_size of the cv-unqualified type T. If the expression TS::value is well-formed when treated as an unevaluated operand, then each of the three templates shall meet the UnaryTypeTrait requirements (23.15.1) with a base characteristic of integral_constant<size_t, TS::value> Otherwise, they shall have no member value."

When the Library provides SFINAE, Meow is complete, it's just that Meow::value or Meow::type is non-existent. This is the long-standing practice followed by invoke_result, etc.

However, Core expects different behavior:

11.5 [dcl.struct.bind]/3: "Otherwise, if the qualified-id std::tuple_size names a complete type, the expression std::tuple_size::value shall be a well-formed integral constant expression and"

This wording was introduced in Issaquah as the resolution of GB 20 (thanks to Casey for the programmer-archaeology). He found no discussion, just "ready for vote on Friday".

I encountered this because I implemented tuple_size SFINAE with the traditional approach (it is a complete class with no members), yet libc++ had a test expecting tuple_size to be an incomplete class. I argue that the Core wording should change - it should not inspect the completeness of tuple_size, but should instead inspect the well-formedness of tuple_size::value, which is what the Library is already doing when implementing tuple_size.

Thanks, STL

tstellar commented 3 years ago

mentioned in issue llvm/llvm-project#41819

CaseyCarter commented 3 years ago

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

zmodem commented 5 years ago

r369043, and let's merge it to 9.0

Wohoo! Merged in r369361.

rnk commented 5 years ago

r369043, and let's merge it to 9.0

zmodem commented 5 years ago

I'm heading out, but I hope it lands next week, and we should probably merge it to 9.0 if we still can.

Adding it as a blocker to keep track of it.

StephanTLavavej commented 5 years ago

Thanks Reid! For anyone else following this bug:

CWG 2386 was accepted as a C++17 DR in Feb 2019. I think that resolves all open questions and unblocks Clang. Several customers have noticed this behavior over the years. Merging a fix to 9.0 would be awesome!

rnk commented 5 years ago

I cleaned it up and fixed the test and confirmed that we still have coverage of the error path for ICE handling. The patch is here: https://reviews.llvm.org/D66040

I'm heading out, but I hope it lands next week, and we should probably merge it to 9.0 if we still can.

rnk commented 5 years ago

I'll run some tests and see if this breaks anything...

The only broken test is CXX/dcl.decl/dcl.decomp/p3.cpp, but I'm guessing that's the intended behavior.

rnk commented 5 years ago

I have absolutely zero knowledge about this feature and how it is intended to work, but I assume the "fix" would be something like the following, cleaned up:

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index accca46666b..68fc53309f6 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -1049,8 +1049,8 @@ static IsTupleLike isTupleLike(Sema &S, SourceLocation Loc, QualType T, } Diagnoser(R, Args);

if (R.empty()) {

I'll run some tests and see if this breaks anything...

CaseyCarter commented 5 years ago

I just closed another duplicate of this bug, and had another VS customer report it this week. Consider this a gentle ping to implement CWG 2386 =)

CaseyCarter commented 5 years ago

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

ec04fc15-fa35-46f2-80e1-5d271f2ef708 commented 7 years ago

This seems like something that should be discussed in a joint CWG / LWG session rather than here. I think the status quo is that both core and library think their rule is the right one...

zmodem commented 7 years ago

+Richard and Marshall