Replace N macro with operator ""_n - develop

[revl]

Change Description

Because the N macro did not have a project-specific prefix, it clashed with its namesakes from third-party projects like LLVM. This commit introduces a namespace-scoped operator ""_n and replaces all invocations of the N macro with that operator.

Change Type

Select ONE

• Documentation
• Stability bug fix
• Other
• Other - special case

Consensus Changes

• Consensus Changes

API Changes

• API Changes

The N macro in libraries/chain/include/eosio/chain/name.hpp has been replaced with operator ""_n.

Documentation Additions

• Documentation Additions

[larryk85]

Part of the acceptance criteria for this task was to ensure that the implementation of the literal and name type are actually compile time constructing. This is not the case, you can find all of the account name strings and associated runtime conversion operations compiled in. You can also change the names to invalid eosio names and it won't fail at compile time anymore and progress to runtime failures.

This looks to be from abieos which has its own issues with maintaining constexpr'ness. Please make this inline with the one in eosio.cdt, the name there and associated user defined literal will ensure that these are constructed at compile time.

It also appears that you didn't update the eosio-wasm-spec-tests and is failing to build those. The spec tests submodule is used elsewhere so you will also need to ensure that you create an eosio branch in that repo if one doesn't already exist there for these changes.

[swatanabe-b1]

I'd like to point out that the existing N macro is not evaluated at compile time either (it is constexpr, which is not the same thing).

#include <eosio/chain/name.hpp>

// evaluated at compile time (static initialization is mandatory.  [basic.start.static])
eosio::chain::name x = N(test1);
// evaluated at compile time.  Does not compile unless the initializer is a constant expression.
constexpr eosio::chain::name y = N(test2);

int main() {
   // Not necessarily evaluated at compile time.
   x = N(test3);
}

[larryk85]

I'd like to point out that the existing N macro is not evaluated at compile time either (it is constexpr, which is not the same thing).

#include <eosio/chain/name.hpp>

// evaluated at compile time (static initialization is mandatory.  [basic.start.static])
eosio::chain::name x = N(test1);
// evaluated at compile time.  Does not compile unless the initializer is a constant expression.
constexpr eosio::chain::name y = N(test2);

int main() {
   // Not necessarily evaluated at compile time.
   x = N(test3);
}

Indeed, sorry I worded the previous message a bit off, it was quite late and intended for Damon. A part of the acceptance criteria of this was to ensure that we are going to get compile time construction of the names. That was one of the original reasons we wanted to add the _n literal. When writing tests for eosio and new functionality it would be nice to have them fail at compile time and not at runtime.

[swatanabe-b1]

I'm not quite sure I follow the connection between ""_n and compile-time evaluation. Before C++20 consteval, it's easier to force compile-time evaluation with a macro.

[huangminghuang]

Indeed, sorry I worded the previous message a bit off, it was quite late and intended for Damon. A part of the acceptance criteria of this was to ensure that we are going to get compile time construction of the names. That was one of the original reasons we wanted to add the _n literal. When writing tests for eosio and new functionality it would be nice to have them fail at compile time and not at runtime.

All the functions involved in the implementation of ""_n are already constexpr; therefore, it is a compile time evaluated when used in the constexpr context. I tested it on the compiler explore (with only EOS_ASSERT been changed) and it works as expected. Here is the link https://godbolt.org/z/eGc4n1

[revl]

Part of the acceptance criteria for this task was to ensure that the implementation of the literal and name type are actually compile time constructing. This is not the case, you can find all of the account name strings and associated runtime conversion operations compiled in. You can also change the names to invalid eosio names and it won't fail at compile time anymore and progress to runtime failures.

I did spend some time looking into how this can be done. The template-based implementation of string_to_name from abieos would detect errors in the names at compile time. However, unifying eos and abieos implementations is a task that requires more thought than the mechanical RegEx-based replacement presented in this PR.

There's a third implementation of operator ""_n in eosio.cdt/eoslib. However, it exists only in the develop branch of eosio.cdt while in the eosio-cdt-2.1-staging-c branch, that implementation is replaced with #include_next <eosio/name.hpp>, which presumably results in including abieos/include/eosio/name.hpp.

This PR improves the situation by getting rid of the N macro. I think unification of name implementations across all repositories deserves a separate ticket, which I can create and start working on after this PR has been merged. But then I'd have make sure that my changes don't break eos, abieos, and two different branches of eosio.cdt. I'd rather wait until eosio-cdt-2.1-staging-c is merged with develop in eosio.cdt before attempting to clean this up.

It also appears that you didn't update the eosio-wasm-spec-tests and is failing to build those. The spec tests submodule is used elsewhere so you will also need to ensure that you create an eosio branch in that repo if one doesn't already exist there for these changes.

I created a separate PR EOSIO/eosio-wasm-spec-tests#12 for that but I didn't know eosio-wasm-spec-tests was used elsewhere. Thanks for providing this context. I've retargeted the PR to the new eosio branch.