netlify / apex Goto Github PK

While I've personally used impl over the years, it's been a one woman fight against "over detail-ification", where most of code implementation is hidden away into a separate namespace making errors almost impossible to read. however with the advent of concepts the detail namespace makes more sense than impl. Add in the ability to do apex::detail directly within a single namespace and life has gotten easier. If Apex is to ever be open sourced, we should endeavor to have it follow the de-facto implementation detail namespace. That is, detail.

Given a type with the following definition:

template <default_initializable T>
struct launderable {
  T const value { };
  int f { };
};

We technically must use std::launder to access the memory location of storage when reconstructing an object in its location. However, there doesn't seem to be a consensus at the ISO level of whether this is needed if a value comes from a union of one type, nor if it's not done in the same given scope. We can't, however, simply just pepper the code with it as this prevent optimizations, and can also lead to Undefined Behavior.

Getting in the details of this will require reading the standard. Until then, maybe don't put const objects in your structs :v

Going forward, we're going to have to start hiding more implementation details and using parts in a more piecemeal fashion. While the prelude header has been a decent success, it turns out we don't need half the declarations found within it in most files, and this

1. makes IWYU go absolutely bonkers when run on our codebase
2. makes it VERY hard to maintain the header.

While creating a huge splayed filesystem of headers used to be considered "bad practice", we've moved to Clang for our C++ stuff, so we can rely on its faster preprocessor for performance.

The current plan to refactor the header is as follows:

• macros are back into their own header
• bare minimum concepts are placed into their own headers
• ranges:: CPOs are placed into their own headers
• using declarations and "elementary" types go into one header

This should break up a lot of the monotony of our current setup and make this way more manageable for our uses. It also means that as time goes on and most of these shims are implemented, we can slowly remove these detail headers and eventually place everything back into apex/core/prelude.hpp

Why this fails on github actions and not locally or within docker is beyond me 🤷‍♀️

Due to lack of C++20 features in Clang 10 (not the devs fault, it's just not implemented or available yet) we do not correctly handle trivial destructors, trivial assignment, and trivial copy and move constructors. Fixing this behavior will require a large refactor, as it means placing all possible trivial states into a base class.

We can then 'undo' this refactor once proper C++20 support is available to us.

The advent of CPOs (customization point objects) for C++ dispatching means that apex::mixin::iterator could technically be implemented entirely by way of CPOs. In addition to ranges related traits (e.g., incrementable_traits, we can also rely on iter::next, iter::prev, etc. to see if a type meets the given interface for a specific iterator), having iter:: specific CPOs would allow us to also eventually wrap iterators into type erased iterator APIs. This will also make hooking into ranges easier in the future.

Describe the bug

Compile problem when using back_emplace in a std::transform() call:

/root/plugin/build/_deps/apex-src/include/apex/core/iterator.hpp:96:20: error: member reference type 'apex::back_emplacer<std::vector<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > > >::container_type *' (aka 'std::vector<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > > *') is a pointer; did you mean to use '->'? [clang-diagnostic-error]
    this->container.emplace_back(std::forward<T>(t));

To Reproduce
See discussion from https://github.com/netlify/netlify-ats-plugin/pull/346
(eliding code due to public repo)

This is a much better approach than using the approaches mentioned by #8 and #9. We can instead use this header to hold the most important bits, even if they are technically supposed to be declared elsewhere. However those 'elsewhere' locations will be include this header anyhow.

A few goals however include:

• Keeping code to an absolute minimum
• Keeping the concepts declared as simple as possible (to reduce compiler output)
• Only including a few core stdlib headers

Once the bare minimum has been defined (these are needed for the ranges::swap, ranges::begin, and ranges::end custom object points, which requires a decent amount of crossover with <iterator> and <ranges>, sadly 😞) we can effectively "lock" the file until needed. Once standard library implementations are available to us we can remove our code entirely, thus reducing the size of the prelude header to a much smaller size.

apex::proxy::item is a proxy object to allow automatic "dereferencing" of a value from some associative container when it might not be able to return said value by reference. Examples of this are

• traffic
• apex::sqlite::context when retrieving apex::sqlite::value
• apex::session::variable, returned from apex::session::environment`
• apex::session::argument's value_type
• trafficserver's cache lookup
• any associative-like type that can be extracted via iteration or via operator []

This solves memory issues and also ensures that a user can't accidentally get the address of a temporary.

The only true downside is that to interact with the type, one must use operator ->, or do something like

auto x = container["key"]
auto&& ref = *x;

However, at some point we could theoretically write a clang-tidy/clang-check to find these issues and prevent them from occuring.

There are a variety of exposition only traits and concepts we need when we're implementing "as yet to be provided by existing implementations" library features. These include things like

• __LegacyInputIterator
• __WeaklyEqualityComparableWith
• cond-value-type
• XREF/xref
• COMMON-REF/common_ref

Adding these into a single header will

• Reduce the overhead of keeping track of where all the exposition only features are located
• Allow us to delete the file when it is no longer needed
• Let us hide all the terrifying nightmare concepts into one place 🙂

The current apex::mixin::iterator type has been a disaster in terms of implementation. However a recent article brought some information to my attention. As a result we'll be killing apex::mixin::iterator and cannablizing it into the apex::mixin::iterable, as this name makes more sense.

However the work to get this implemented will take some time as we do not have a ranges implementation, nor do we have a fully working concepts implementation. As such, this will be minimal and underconstrained. This means code can be expected to break in the near future. However, code breaking means that the code was not correct in the first place, we just couldn't prevent bad code from being written.

Several types depend on so called "customization point objects". These are not really important to the average user and thus should be placed into a single file in one location, and only used when really needed. They technically waste space for those curious about general API usage otherwise.

Now that we are moving to C++20, apex::outcome<T, E> requires (heh) the use of C++ concepts to further constrain the type. More work is needed to make sure that we are using the implementation to its fullest extent, while also making it easier for folks coming from Rust to understand the interface (naming conventions aside, as we tend to use transform instead of map because... the 90s were a thing 😔). Additionally to make sure it is as safe as possible, we need to overconstrain the interface regarding noexcept and requires calls.

Additionally, some work is needed to ensure that the apex::optional and apex::outcome types can interact with each other "simply".

Lastly, we need to implement a way to ensure that it is trivially destructible when possible, the storage is minimal when one type is a reference, and permit references for both types.

The 'handle' name is not too descriptive, and as it turns out the current implementation does not easily permit 'release'-able resources.

For this reason, the type should be renamed to resource, and the concept for a valid resource manager should be named to either resource_manager or resource_storage.

As discovered when working on getting libstdc++ 10 to work with clang-10 and apex, the correct return type of the ranges::end CPO is actually sentinel_for<ranges::iterator_t<T>> auto. We do not currently enforce this, but should for feature parity with the standard. More information can be found here