bitwizeshift / alloy Goto Github PK

Checklist

• I did not find a duplicate of this bug in the Github Issues section.

Description

Dynamic libraries on Windows require all public symbols to be documented with
the __declspec(dllexport) attribute when compiling, and with the __declspec(dllimport)
attribute when consuming.

If these symbols are left undefined, binaries are unable to resolve the public symbols.

The conventional approach is to conditionally define a macro based on whether you
are compiling the library or whether you are a consumer.

Alloy actually already includes headers that define this, usually named api.hpp per library --
but these are just left unused since most testing have been in *nix-like environments and macOS.

All symbols should be appropriately documented, such that -Wunused-attribute warnings will
not fire, and such that MSVC can compile/consume the libraries appropriately.

Steps to reproduce

1. Compile the unit tests using a compiler using the cl back-end (e.g. clang-cl)

Note: Fixing just the unit tests will not cover all APIs, it's just a reproducible step

Expected Behavior

1. The unit tests compile

Actual Behavior

1. The unit tests fail due to unresolved symbols

Checklist

• I did not find a duplicate of this bug in the Github Issues section.

Description

The win32_filesystem_monitor consistently misses firing the first event after being registered.

Steps to reproduce

1. Construct a win32_filesystem_monitor
2. Register a listener to the monitor
3. Register the monitor to a message_pump
4. Watch a directory
5. Modify something in that directory

Expected Behavior

The first modification is reported

Actual Behavior

The second modification and onward is reported.

Extra information

• Operating System: Windows 10
• Compiler: clang-8.0

Checklist

• I did not find a duplicate of this bug in the Github Issues section.

Description

Writing inline functions that are also marked virtual results in the vtable
being emitted to each translation unit that includes that type.

These definitions should probably be moved to a cpp file instead, at the expense
of the function itself potentially not being inlined.

Certain compilers emit warnings on this from -Wweak-vtables

Checklist

• I did not find a duplicate of this bug in the Github Issues section.

Description

Since Github migrated to using the $GITHUB_ENV environment variable, various
parts of the CI build have had to change in order to build correctly. As part of these
changes, it appears that newer GCC build versions are now failing to build entirely
due to different errors depending on debug or release builds:

Debug

C:/ProgramData/chocolatey/lib/mingw/tools/install/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/8.1.0/../../../../x86_64-w64-mingw32/bin/as.exe: CMakeFiles\AlloyCoreTest.dir\src\main.cpp.obj: too many sections (32805)
C:\Users\RUNNER~1\AppData\Local\Temp\ccXSuEW8.s: Assembler messages:
C:\Users\RUNNER~1\AppData\Local\Temp\ccXSuEW8.s: Fatal error: can't write 233 bytes to section .text of CMakeFiles\AlloyCoreTest.dir\src\main.cpp.obj: 'File too big'
C:/ProgramData/chocolatey/lib/mingw/tools/install/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/8.1.0/../../../../x86_64-w64-mingw32/bin/as.exe: CMakeFiles\AlloyCoreTest.dir\src\main.cpp.obj: too many sections (32805)
C:\Users\RUNNER~1\AppData\Local\Temp\ccXSuEW8.s: Fatal error: can't close CMakeFiles\AlloyCoreTest.dir\src\main.cpp.obj: File too big

Link

Release

In file included from D:/a/Alloy/Alloy/lib/alloy-io/include/alloy/io/events/keyboard_events.hpp:34,
                 from D:\a\Alloy\Alloy\build\lib\alloy-io\AlloyIO.SelfContainmentTest\include\alloy\io\events\keyboard_events.hpp.cpp:1:
D:/a/Alloy/Alloy/lib/alloy-io/include/alloy/io/event.hpp:519:8: error: 'alloy::io::event::~event()' redeclared without dllimport attribute after being referenced with dll linkage [-Werror]
 inline alloy::io::event::~event()
        ^~~~~
D:/a/Alloy/Alloy/lib/alloy-io/include/alloy/io/event.hpp:617:13: error: 'void alloy::io::event::reset()' redeclared without dllimport attribute after being referenced with dll linkage [-Werror]
 inline void alloy::io::event::reset()
             ^~~~~
D:/a/Alloy/Alloy/lib/alloy-io/include/alloy/io/event.hpp:686:22: error: 'static uint32_t alloy::io::event::null_handler(alloy::io::event::operation, const storage_type*, const storage_type*)' redeclared without dllimport attribute after being referenced with dll linkage [-Werror]
 inline std::uint32_t alloy::io::event::null_handler(operation op,
                      ^~~~~

This error appears to be due to using the ALLOY_IO_API preprocessor symbol on a
class that has inline variables. Brief searching on stack overflow indicates that this
warning from GCC is extraneous and does not affect behavior; however it appears that
this also appears that it cannot simply be suppressed with a Wno-... argument since the
-Werror message does not indicate what the warning flag is that triggers this error

Link

It may just be that the fix to this will simply be to update the minimum GCC version.
It may also be that some small code-changes may be required as well.

Expected Behavior

The build succeeds

Actual Behavior

The build fails

Extra information

• Library version: master
• Operating System: Microsoft Windows Server 2019 - 10.0.17763
• Compiler: GNU 8.1.0

Checklist

• I did not find a duplicate of this bug in the Github Issues section.

Description

Building the project with -GNinja for Windows, using either clang, clang-cl, or just normal cl (MSVC) seems to render ctest unable to detect and execute tests.

This appears to work fine for Linux/macOS builds, or MinGW builds. Perhaps CMake is not propagating the correct load-path for the executables?

Preliminary investigations seem to suggest that, for some reason, the $env:Path variable is not being updated with the paths to the binaries needed for the unit tests, and this is causing the executables to fail to execute due to the link-time dependency -- and this results in catch_discover_all_tests to not find any tests.

Steps to reproduce

1. Configure the project with -GNinja for clang, clang-cl, or MSVC
2. Build the project
3. Run ctest . from the main directory

Expected Behavior

All tests execute

Actual Behavior

CTest shows "no tests found!!"

Checklist

• I did not find a duplicate of this feature request in the Github Issues section.

Description

This feature is to enable sanitizers on the Ubuntu build, which requires some code-changes to fix.

The current infrastructure is in-place, but failing to run correctly due to new and delete usages in
the standard-library. It is detecting ::operator new (std::size_t, std::align_t) from alloy::core::allocator
as not being the same alignment used in ::operator delete(std::size_t, std::align_t):

  object passed to delete has wrong type:
  alignment of the allocated type:   default-aligned;
  alignment of the deallocated type: 16 bytes.
    #0 0x4e6062 in operator delete(void*, std::align_val_t) (/home/runner/work/Alloy/Alloy/build/lib/alloy-core/test/alloy-core-test+0x4e6062)
    #1 0x53cfee in alloy::core::allocator::default_resource::do_deallocate(void*, unsigned long, std::align_val_t) /home/runner/work/Alloy/Alloy/build/../lib/alloy-core/include/alloy/core/memory/allocator.hpp:990:3
    #2 0x5fa98c in alloy::core::memory_resource::deallocate(void*, unsigned long, std::align_val_t) /home/runner/work/Alloy/Alloy/build/../lib/alloy-core/include/alloy/core/memory/memory_resource.hpp:268:10
    #3 0x5fa4c4 in alloy::core::allocator::deallocate_bytes(void*, unsigned long, std::align_val_t) /home/runner/work/Alloy/Alloy/build/../lib/alloy-core/include/alloy/core/memory/allocator.hpp:532:22
    #4 0x5fa460 in alloy::core::stl_allocator_adapter<std::byte>::deallocate(std::byte*, unsigned long) /home/runner/work/Alloy/Alloy/build/../lib/alloy-core/include/alloy/core/memory/allocator.hpp:1045:22
    #5 0x5fa424 in std::allocator_traits<alloy::core::stl_allocator_adapter<std::byte> >::deallocate(alloy::core::stl_allocator_adapter<std::byte>&, std::byte*, unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/alloc_traits.h:341:13
    #6 0x5fa3ea in std::_Vector_base<std::byte, alloy::core::stl_allocator_adapter<std::byte> >::_M_deallocate(std::byte*, unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/stl_vector.h:354:4
    #7 0x5fa2fe in std::_Vector_base<std::byte, alloy::core::stl_allocator_adapter<std::byte> >::~_Vector_base() /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/stl_vector.h:335:2
    #8 0x5fa181 in std::vector<std::byte, alloy::core::stl_allocator_adapter<std::byte> >::~vector() /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/stl_vector.h:683:7
    #9 0x5fa0c7 in alloy::core::explicitly_copyable<std::vector<std::byte, alloy::core::stl_allocator_adapter<std::byte> > >::~explicitly_copyable() /home/runner/work/Alloy/Alloy/build/../lib/alloy-core/include/alloy/core/utilities/explicitly_copyable.hpp:59:9
    #10 0x5f9334 in alloy::core::packed_buffer::~packed_buffer() /home/runner/work/Alloy/Alloy/build/../lib/alloy-core/include/alloy/core/memory/packed_buffer.hpp:61:24
    #11 0x5ea00f in alloy::core::test::(anonymous namespace)::____C_A_T_C_H____T_E_S_T____3() /home/runner/work/Alloy/Alloy/build/../lib/alloy-core/test/src/alloy/core/memory/packed_buffer.test.cpp:52:1

It's not entirely clear what could be causing this discrepancy. This fails with both clang and gcc on Ubuntu, but does not appear to occur on mac -- suggesting that this may either be a variation between libc++ and libstdc++, or potentially that this is undefined behavior that is detected in different versions of the compiler.

At any rate, this issue needs to be investigated and fixed before sanitizers can be enabled.

Marking this as both an enhancement and bug report, since there is likely a fix required here.

Checklist

• I did not find a duplicate of this feature request in the Github Issues section.

Description

Code-coverage should be enabled and auto-reported as part of the CI/CD pipeline.

Blocked: until this repo goes public, since I am not paying for private CI

No longer blocked. Using Github Actions, and made the repo public anyway.

Checklist

• I did not find a duplicate of this bug in the Github Issues section.

Description

In extras/win32-bindings, the win32_filesystem_monitor only pumps a single event per watched handle during an invocation of pump, rather than pumping all events at once.

This may be a larger issue for system that would like to watch larger amounts of files at once, since I/O events are typically polled once per update cycle and/or frame.

Steps to reproduce

1. Create a win32_filesystem_monitor
2. Register the monitor to a message_pump
3. Register a listener to the message_pump
4. watch a directory
5. Trigger several filesystem actions
6. Invoke message_pump::pump

Expected Behavior

All events fire

Actual Behavior

A single event per watched directory fires per pump invocation

Extra information

• Operating System: Windows 10
• Compiler: clang-8.0

Checklist

• I did not find a duplicate of this feature request in the Github Issues section.

Description

This repository has been making use of a quickly slapped-together expected implementation for over a year now.
Recently, in a different repository of mine, I released a standalone result type modeled after Rust and Swift's Result types in
bitwizeshift/result.

It would be convenient and more idiomatic to work this into the environment rather than using expected here.

This will require some code-changes, since this would no longer use std::error_code and custom error-categories, but
would rather use an E type as an explicit enumeration.

Checklist

• I did not find a duplicate of this feature request in the Github Issues section.

Description

A lot of the recent code changes to make this build with clang-cl necessitated the use of
compiler #pragmas, which often resulted in code like:

#if defined(__clang__)
# pragma clang diagnostic push
# pragma clang diagnostic ignored <flag>
#elif defined(__GNUC__)
# pragma GCC diagnostic push
# pragma GCC diagnostic ignored <flag>
#endif
...

This duplication occurs because most, if not all, gcc flags are duplicated in clang -- but the opposite
is not true.

C++11 introduced the standardized _Pragma operator which can probably clean this up some
by creating a generic ALLOY_COMPILER_PRAGMA_PUSH and ALLOY_COMPILER_GNULIKE_DIAGNOSTIC_IGNORE(...)
to expand into either the GCC or clang equivalent depending on the build, or nothing on compilers
that do not support these pragmas.