Skip to content

make build with MSVC #21578

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

Closed
wants to merge 1 commit into from
Closed

make build with MSVC #21578

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/swift/Basic/ImmutablePointerSet.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ ImmutablePointerSet<T> ImmutablePointerSetFactory<T>::EmptyPtrSet =
ImmutablePointerSet<T>(nullptr, {});

template <typename T>
constexpr unsigned ImmutablePointerSetFactory<T>::AllocAlignment =
const unsigned ImmutablePointerSetFactory<T>::AllocAlignment =
(alignof(PtrSet) > alignof(PtrTy)) ? alignof(PtrSet) : alignof(PtrTy);

} // end swift namespace
Expand Down
8 changes: 8 additions & 0 deletions stdlib/public/SwiftShims/SwiftStddef.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,19 @@ typedef __SIZE_TYPE__ __swift_size_t;
#endif

// This selects the signed equivalent of the unsigned type chosen for size_t.
#if defined(_MSC_VER)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I suppose that this should be:

#if defined(_MSC_VER) && !defined(__clang__) ...

#if defined(_M_X86) || defined(_M_ARM)
typedef int ssize_t;
#elif defined(_M_AMD64) || defined(_M_ARM64)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_WIN64 then _WIN32, maybe? Although I'm wondering why MSVC is ever even looking at this header; it should always be compiled with the just-built Clang, no?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoah... its a wild @jrose-apple!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jrose-apple - nope, because this is getting indirectly included into the compiler. It is really sad that we cannot limit this header to just the stdlib/runtime which I would really prefer.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems…wrong. Maybe that's the thing worth tackling: figuring out which runtime / shims headers need to go into the compiler and separating them out.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jrose-apple I agree. I had tried to remove the use of the header, but @jckarter didn't want to lose the re-use of the types defined in the headers IIRC.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I would recommend; the only place we need to avoid using the platform headers is when importing the header to build the standard library.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ssize_t is not available on all platforms as it is not mandated by the standard, it is an extension.

Copy link
Contributor

@jrose-apple jrose-apple Jan 7, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably the same as ptrdiff_t, if SSIZE_MAX isn't defined? Or intptr_t.

EDIT: Or std::make_signed<std::size_t>::type.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SSIZE_MAX is also not available IIRC. Yes, we should be able to use std::make_signed<std::size-t>::type but obviously only in the C++ context.

Copy link
Contributor

@jrose-apple jrose-apple Jan 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I meant to use the absence of SSIZE_MAX as a signifier to use a fallback type, but in practice I think it'd be valid to say that these headers must be included in either a Swift or a C++ context. (Note that Swift should be tested first, since someday we'll probably have C++-in-Swift. Somehow.)

typedef __int64 ssize_t;
#endif
#else
typedef __typeof__(_Generic((__swift_size_t)0, \
unsigned long long int : (long long int)0, \
unsigned long int : (long int)0, \
unsigned int : (int)0, \
unsigned short : (short)0, \
unsigned char : (signed char)0)) __swift_ssize_t;
#endif

#endif // SWIFT_STDLIB_SHIMS_SWIFT_STDDEF_H