Skip to content

adds C++20 module interface sus #472

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.

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

adds C++20 module interface sus #472

wants to merge 1 commit into from

Conversation

cjdb
Copy link
Contributor

@cjdb cjdb commented May 27, 2025

sus is an experimental module interface unit. import sus; replaces including most conventional Subspace headers. Users will need to include headers suffixed with _macros.h in order to use Subspace modules, but should not include any headers otherwise.

`sus` is an experimental module interface unit. `import sus;` replaces
including most conventional Subspace headers. Users will need to include
headers suffixed with `_macros.h` in order to use Subspace modules, but
should not include any headers otherwise.
Comment on lines +239 to +242
target_sources(${SUS_NAME} PUBLIC
FILE_SET CXX_MODULES
FILES "sus.cc"
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see the first add_library has all the .cc files except this one. The target_sources has all the .h and .cc files except this one. And then we repeat target_sources with this one. Can we add a comment so when adding a new file it's clear where to put it in this file?

import sus;

# include "fmt/format.h"
# include "sus/assertions/check.h"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I was expecting only _macros.h headers when using module, how come this is here?

#ifdef TEST_MODULE
import sus;

# include "fmt/format.h"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this just move outside the ifdef entirely?

Comment on lines +31 to +32
#include <sstream>
#include "fmt/format.h"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
#include <sstream>
#include "fmt/format.h"
#include <sstream>
#include "fmt/format.h"

Comment on lines +18 to +19
#include "sus/choice/macros.h"
#include "sus/macros/__private/compiler_bugs.h"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should these move outside the ifdef?

Comment on lines +13 to +14
// limitations under the License.
module;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// limitations under the License.
module;
// limitations under the License.
module;

// See the License for the specific language governing permissions and
// limitations under the License.
module;
#include "sus/assertions/check.h"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add a comment explaining which headers appear here (everything except macros headers)?

using sus::mem::remove_rvalue_reference;

namespace __private {
using sus::mem::__private::HasCloneFromMethod;
Copy link
Collaborator

Choose a reason for hiding this comment

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

How come private stuff has to be in here? Can it be made reachable without being visible? (https://vector-of-bool.github.io/2019/03/31/modules-2.html)

Comment on lines +299 to +306
namespace __private {
using sus::choice_type::__private::find_choice_storage_mut;
using sus::choice_type::__private::find_choice_storage;
using sus::choice_type::__private::TypeList;
using sus::choice_type::__private::MakeStorageType;
using sus::choice_type::__private::StorageIsSafelyConstructibleFromReference;
using sus::choice_type::__private::AllValuesAreUnique;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
namespace __private {
using sus::choice_type::__private::find_choice_storage_mut;
using sus::choice_type::__private::find_choice_storage;
using sus::choice_type::__private::TypeList;
using sus::choice_type::__private::MakeStorageType;
using sus::choice_type::__private::StorageIsSafelyConstructibleFromReference;
using sus::choice_type::__private::AllValuesAreUnique;
}
namespace __private {
using sus::choice_type::__private::find_choice_storage_mut;
using sus::choice_type::__private::find_choice_storage;
using sus::choice_type::__private::TypeList;
using sus::choice_type::__private::MakeStorageType;
using sus::choice_type::__private::StorageIsSafelyConstructibleFromReference;
using sus::choice_type::__private::AllValuesAreUnique;
}

Comment on lines +804 to +806
export using ::sus::panic;
export using ::sus::unreachable;
export using ::sus::unreachable_unchecked;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
export using ::sus::panic;
export using ::sus::unreachable;
export using ::sus::unreachable_unchecked;
export using sus::panic;
export using sus::unreachable;
export using sus::unreachable_unchecked;

This file is consistently not using the leading :: otherwise, and that seems fine for using decls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants