Scintilla / Bugs / #2187 Unable to compile with Xcode 12 Beta

Neil Hodgson - 2020-06-23

Style doesn't support moves as its memory management is a little strange so both move construction and move assignment are = delete. Style contains a FontAlias (non-owning copy of a FontID) and a non-owning const char *fontName. FontAlias has neither move and no copy assignment, just a copy constructor. There may be 120 styles which only use 1 to 5 fonts and not creating a new system font object for each style has been a worthwhile resource saving.

For 5.0, I plan to change Style::font from FontAlias to a reference-counted std::shared_ptr which will have simpler, safer, and more standard semantics. The fontName field could also be made shared_ptr although its more like string interning.

Its likely OK to define some more of the FontAlias special methods as = default as deleting them is done to ensure there are no changes in user code that do anything unexpected. If that doesn't get it working then try switching some special methods in Style to = default.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Chinh Nguyen - 2020-06-23

I will try. I know very little C++ but I'll have a colleague help me.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Neil Hodgson - 2020-06-23

Its unfortunate that the diagnostic isn't saying which particular element of __is_cpp17_move_insertable is missing.

You could try the attached patch which is unlikely to be optimal.

movex.patch

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

I had to make one correction to the patch where there was a noexcept that should've been a noexcept = default.

I got an error after the addition of other.ClearFont() to FontAlias::FontAlias(const FontAlias &other):

/Users/cqn/Desktop/scintilla/src/Style.cxx:25:2: error: 'this' argument to member function 'ClearFont' has type 'const Scintilla::FontAlias', but function is not marked const
        other.ClearFont();
        ^~~~~
In file included from /Users/cqn/Desktop/scintilla/src/Style.cxx:16:
../../src/Style.h:43:7: note: 'ClearFont' declared here
        void ClearFont() noexcept;

And lastly I forgot to give you this error earlier:

/Applications/Xcode-beta.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/memory:1720:13: error: static_assert failed due to requirement '__is_cpp17_move_insertable<std::__1::allocator<Scintilla::PositionCacheEntry>, false>::value' "The specified type does not meet the requirements of Cpp17MoveInsertable"
            static_assert(__is_cpp17_move_insertable<allocator_type>::value,
            ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /Users/cqn/Desktop/scintilla/src/PositionCache.cxx:16:
/Applications/Xcode-beta.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/vector:952:21: note: in instantiation of function template specialization 'std::__1::allocator_traits<std::__1::allocator<Scintilla::PositionCacheEntry> >::__construct_backward_with_exception_guarantees<Scintilla::PositionCacheEntry *>' requested here
    __alloc_traits::__construct_backward_with_exception_guarantees(
                    ^
/Applications/Xcode-beta.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/vector:1094:9: note: in instantiation of member function 'std::__1::vector<Scintilla::PositionCacheEntry, std::__1::allocator<Scintilla::PositionCacheEntry> >::__swap_out_circular_buffer' requested here
        __swap_out_circular_buffer(__v);
        ^
/Applications/Xcode-beta.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/vector:2022:15: note: in instantiation of member function 'std::__1::vector<Scintilla::PositionCacheEntry, std::__1::allocator<Scintilla::PositionCacheEntry> >::__append' requested here
        this->__append(__sz - __cs);
              ^
/Users/cqn/Desktop/scintilla/src/PositionCache.cxx:750:7: note: in instantiation of member function 'std::__1::vector<Scintilla::PositionCacheEntry, std::__1::allocator<Scintilla::PositionCacheEntry> >::resize' requested here
        pces.resize(0x400);

I was able to muddle my way through enough to successfully build and run by commenting out the code that resulted in errors. I just wanted to confirm I could get the rest of my app to build/run.

Neil Hodgson - 2020-06-24

/Users/cqn/Desktop/scintilla/src/Style.cxx:25:2: error: 'this' argument to member function 'ClearFont' has type 'const Scintilla::FontAlias', but function is not marked const other.ClearFont();

other is FontAlias &&other so no const to complain about. The purpose of the argument in a move constructor is to grab its contents then empty it afterwards. The emptying can't be done if it is const.

The diagnostics aren't necessarily bugs in the Scintilla code - there were similar problems with the standard library that came with a release of GCC that were fixed by a GCC update.

PositionCacheEntry is more dangerous as it has an owning pointer to positions. However, since this is the standard library std::unique_ptr it should move correctly if the move operations on PositionCacheEntry are = default.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Neil Hodgson - 2020-06-24

Here is a version of the patch with a move assignment on FontAlias which may make it easier for the compiler to default the move assignment on Style.

movey.patch

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
- Chinh Nguyen - 2020-06-25
  
  That took care of errors and warnings with FontAlias and Style.
  
  Last edit: Chinh Nguyen 2020-06-25
  
  If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

I think Xcode is wrong here. From the C++17 standard:

15
... Given ... and an rvalue rv of type T ...
(15.3) — T is MoveInsertable into X means that the following expression is well-formed:
allocator_traits<A>::construct(m, p, rv)
...
[ Note: A container calls allocator_traits<A>::construct(m, p, args) to construct an element at p
using args, with m == get_allocator(). The default construct in allocator will call ::new((void*)p)
T(args), but specialized allocators may choose a different definition. — end note ]

So T(&&) is required but there is no requirement for an assignment operator.

Neil Hodgson - 2020-09-18

status: open --> open-fixed
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Neil Hodgson - 2020-09-18

Committed fix with [2af468].

Related

Commit: [2af468]

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Neil Hodgson - 2020-11-30

status: open-fixed --> closed-fixed
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Unable to compile with Xcode 12 Beta

Group

Searches

Help

#2187 Unable to compile with Xcode 12 Beta

Discussion

Related