Skip to content

[Sema] Fix errors due to implicitly deleted copy constructor. #292

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

Closed
wants to merge 22 commits into from
Closed

Conversation

dcci
Copy link
Member

@dcci dcci commented Dec 7, 2015

No description provided.

@dcci
Copy link
Member Author

dcci commented Dec 7, 2015

This fixes a build failure on FreeBSD with recent clang.

@nadavrot
Copy link
Contributor

nadavrot commented Dec 7, 2015

@dcci What is the error message on FreeBSD?

Can you use "=default " as the body?

@dcci
Copy link
Member Author

dcci commented Dec 7, 2015

Hi Nadav, I tried with =default but I keep getting the same error (which is weird). Anyway, looking at it again I'm not entirely convinced it's the right fix.
As per your request, here's the error message:

/exps/swift/swift/include/swift/Sema/TypeCheckRequestKinds.def:29:1: error: call to implicitly-deleted copy constructor of 'swift::TypeCheckRequest'
TYPE_CHECK_REQUEST(QualifiedLookupInDeclContext, DeclContextLookup)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/exps/swift/swift/include/swift/Sema/TypeCheckRequest.h:154:10: note: expanded from macro 'TYPE_CHECK_REQUEST'
return TypeCheckRequest(TypeCheckRequest::Request, payload);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/exps/swift/swift/include/swift/Sema/TypeCheckRequest.h:79:5: note: copy constructor of 'TypeCheckRequest' is implicitly deleted because field 'Payload' has a deleted copy constructor
} Payload;
^
/exps/swift/swift/include/swift/Sema/TypeCheckRequest.h:73:5: note: explicitly defaulted function was implicitly deleted here
PayloadType(const PayloadType &) = default;
^
/exps/swift/swift/include/swift/Sema/TypeCheckRequestPayloads.def:37:28: note: copy constructor of 'PayloadType' is implicitly deleted because variant field 'InheritedClauseEntry' has a non-trivial
copy constructor
TYPE_CHECK_REQUEST_PAYLOAD(InheritedClauseEntry, std::pair<llvm::PointerUnion<TypeDecl *, ExtensionDecl *>, unsigned>)
^
/exps/swift/swift/include/swift/Sema/TypeCheckRequest.h:76:17: note: expanded from macro 'TYPE_CHECK_REQUEST_PAYLOAD'
VA_ARGS PayloadName;
^

Thanks!

Davide

@dcci
Copy link
Member Author

dcci commented Dec 7, 2015

Clang seems to DTRT:

http://en.cppreference.com/w/cpp/language/copy_constructor
the implicitly-declared or defaulted copy constructor for class T is defined as deleted if any of the following conditions are true:
T is a union and has a variant member with non-trivial copy constructor

@tbartelmess
Copy link

@dcci What version of clang are you using to compile?

@jckarter
Copy link
Contributor

jckarter commented Dec 7, 2015

Does FreeBSD use an outdated Clang by default? There was another PR floating around to make us explicitly use clang35 or later IIRC.

@tbartelmess
Copy link

@jckarter

$ clang++ --version
FreeBSD clang version 3.4.1 (tags/RELEASE_34/dot1-final 208032) 20140512
Target: x86_64-unknown-freebsd10.2
Thread model: posix
$ 

@gribozavr
Copy link
Contributor

Pull request #169 discusses that. I think Clang 3.4 is just too old, and there will be more issues once we get past this one.

@dcci
Copy link
Member Author

dcci commented Dec 7, 2015

This is clang 3.7 on FreeBSD -CURRENT and not 3.4 on stable-10

@gribozavr
Copy link
Contributor

@dcci I know. What I was suggesting in #169 is that we use "clang35" on stable-10 as a special case, and "clang" on FreeBSD versions we don't special case.

@dcci
Copy link
Member Author

dcci commented Dec 8, 2015

So, I can confirm my suspicion that this fix isn't quite right.
While this is able to build swift the generated executable crashes while compiling the standard library:
Top of the stack:
Core was generated by `swift'.
Program terminated with signal SIGBUS, Bus error.
#0 llvm::PointerIntPair<swift::Type, 1u, bool, llvm::PointerLikeTypeTraitsswift::Type >::getPointer (this=0x89f07d99f836d848) at /exps/swi
ft/llvm/include/llvm/ADT/PointerIntPair.h:77
77 reinterpret_cast<void*>(Value & PointerBitMask));
(gdb) bt
#0 llvm::PointerIntPair<swift::Type, 1u, bool, llvm::PointerLikeTypeTraitsswift::Type >::getPointer (this=0x89f07d99f836d848) at /exps/swi
ft/llvm/include/llvm/ADT/PointerIntPair.h:77
#1 0x0000000000435c75 in swift::TypeLoc::getType (this=0x89f07d99f836d848) at /exps/swift/swift/include/swift/AST/TypeLoc.h:66
#2 0x00000000014bb7c0 in swift::IterativeTypeChecker::isResolveInheritedClauseEntrySatisfied (this=0x7fffffff1578, payload=...) at /exps/swi
ft/swift/lib/Sema/ITCDecl.cpp:85
#3 0x00000000014ba0cf in swift::IterativeTypeChecker::isSatisfied (this=0x7fffffff1578, request=...) at /exps/swift/swift/include/swift/Sema
/TypeCheckRequestKinds.def:37
#4 0x00000000014ba3b9 in swift::IterativeTypeChecker::satisfy (this=0x7fffffff1578, request=...) at /exps/swift/swift/lib/Sema/IterativeType
Checker.cpp:70

  • @DougGregor who originally wrote this code and may have insights on the right fix.

@GuillaumeBibaut
Copy link
Contributor

I'm getting the same compilation error than @dcci.
I'm on FreeBSD 10.2-RELEASE with clang 3.7 installed and used by the building script. Mainly:

apple/swift/include/swift/Sema/TypeCheckRequestKinds.def:54:1: error: call to implicitly-deleted copy constructor of 'swift::TypeCheckRequest'
TYPE_CHECK_REQUEST(ResolveTypeDecl, TypeDeclResolution)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/home/yom/Projects/apple/swift/include/swift/Sema/TypeCheckRequest.h:154:10: note: expanded from macro 'TYPE_CHECK_REQUEST'
  return TypeCheckRequest(TypeCheckRequest::Request, payload);                 \
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/home/yom/Projects/apple/swift/include/swift/Sema/TypeCheckRequest.h:79:5: note: copy constructor of 'TypeCheckRequest' is implicitly deleted because field 'Payload' has a deleted copy constructor
  } Payload;
    ^
/usr/home/yom/Projects/apple/swift/include/swift/Sema/TypeCheckRequest.h:73:5: note: explicitly defaulted function was implicitly deleted here
    PayloadType(const PayloadType &) = default;
    ^
/usr/home/yom/Projects/apple/swift/include/swift/Sema/TypeCheckRequestPayloads.def:37:28: note: copy constructor of 'PayloadType' is implicitly deleted because variant field 'InheritedClauseEntry' has a non-trivial copy constructor
TYPE_CHECK_REQUEST_PAYLOAD(InheritedClauseEntry, std::pair<llvm::PointerUnion<TypeDecl *, ExtensionDecl *>, unsigned>)
                           ^
/usr/home/yom/Projects/apple/swift/include/swift/Sema/TypeCheckRequest.h:76:17: note: expanded from macro 'TYPE_CHECK_REQUEST_PAYLOAD'
    __VA_ARGS__ PayloadName;
                ^

@dcci
Copy link
Member Author

dcci commented Dec 10, 2015

I think that a way to fix this might be implementing copy-constructor and operator= so that it picks the correct underlying constructor depending on the type. Something like this, modulo mistakes. Thoughts?

diff --git a/include/swift/Sema/TypeCheckRequest.h b/include/swift/Sema/TypeCheckRequest.h
index b5767e5..38c9fb0 100644
--- a/include/swift/Sema/TypeCheckRequest.h
+++ b/include/swift/Sema/TypeCheckRequest.h
@@ -114,7 +114,45 @@ public:
   }

 #include "swift/Sema/TypeCheckRequestPayloads.def"
-
+
+  TypeCheckRequest(const TypeCheckRequest &T) { *this = T; }
+
+  TypeCheckRequest& operator=(const TypeCheckRequest &T) {
+    switch (getPayloadKind(getKind())) {
+    case PayloadKind::Class:
+      Payload.Class = T.Payload.Class;
+      break;
+    case PayloadKind::Enum:
+      Payload.Enum = T.Payload.Enum;
+      break;
+    case PayloadKind::InheritedClauseEntry:
+      // XXX: FIXME
+      new (&Payload.InheritedClauseEntry) std::pair<llvm::PointerUnion<TypeDecl *, ExtensionDecl *>, unsigned>();
+      Payload.InheritedClauseEntry = T.Payload.InheritedClauseEntry;
+      break;
+    case PayloadKind::Protocol:
+      Payload.Protocol = T.Payload.Protocol;
+      break;
+    case PayloadKind::DeclContextLookup:
+      new (&Payload.DeclContextLookup) DeclContextLookupInfo();
+      Payload.DeclContextLookup = T.Payload.DeclContextLookup;
+      break;
+    case PayloadKind::TypeResolution:
+      // XXX: FIXME
+      new (&Payload.InheritedClauseEntry) std::tuple<TypeRepr *, DeclContext *, unsigned>();
+      Payload.TypeResolution = T.Payload.TypeResolution;
+      break;
+    case PayloadKind::TypeDeclResolution:
+      Payload.TypeDeclResolution = T.Payload.TypeDeclResolution;
+      break;
+#if 0
+    default:
+      llvm_unreachable("unknown kind");
+#endif
+    }
+    return *this;
+  }
+
   /// Determine the kind of type check request.
   Kind getKind() const { return TheKind; }

gregomni and others added 16 commits December 10, 2015 23:26
…zed.

Added a check for whether the escape use is due to an
UncheckedTakeEnumDataAddrInst, which accesses the memory to grab the
.Some part of an Optional during “?.” or “!”. In that case, use the
generic used-before-initialized diagnostic, since it isn’t actually
because of closure capture.

Added test cases, including specifically testing that capturing in a
closure in order to unwrap the optional is still ok.
Update the README.md instructions for utils/update-checkout option. Should read `--clone-with-ssh`.
Update README.md to show the correct utils/update-checkout option.
Fix typo in UnicodeNormalization.cpp
Update getting started instructions
[build-script] cmark c flags should derive from cmark build type
…umTag() witness

We were checking if the tag was <= numPayloadCases, not <. Fix this
and swap the order of the basic blocks so that the no-tag case is
the fall through for consistency with getEnumTag().
We weren't actually successfully mutating ParentType,
so fix that, and also ensure we set ParentType together
with the rest of the state.

Discovered while writing resilient enum tests.
Move these to SILDeclRef, maybe not the best place but a good home for now.
Factor out a new requiresForeignToNativeThunk() function, which cleans up
some code duplication introduced by the following patch:

478e1c7

This is a small step towards consolidating duplicated logic for figuring out
method dispatch semantics and emitting curry thunks.
This time, the issue is that TypeNullifier skips bodies of
multi-statement closures. However, ExprRewriter will type
happily pass them on to typeCheckClosureBody(). This could
trigger assertions. Fix this by skipping type checking of
multi-statement closures when diagnosing.

There seems to be a minor QoI regression in some test cases
that already looked pretty dodgy and/or had FIXMEs. However
I think its worth fixing a crash.
eeckstein and others added 5 commits December 11, 2015 10:00
This caused a linker failure on linux for some stdlib unit tests.
rdar://problem/23849414
[SR-184] Better diagnose error via unwrapping optional before initialized.
It's useful for debugging.
@dcci
Copy link
Member Author

dcci commented Dec 16, 2015

I have a new pull request for this (on a different branch) that I'll submit shortly.

@dcci dcci closed this Dec 16, 2015
slavapestov pushed a commit to slavapestov/swift that referenced this pull request Nov 27, 2018
Revert "linux: update header used for `major` macro"
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

Successfully merging this pull request may close these issues.