Skip to content
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

Code review snippets to address from @davidben #400

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

Conversation

danakj
Copy link
Collaborator

@danakj danakj commented Oct 13, 2023

cc: @davidben

Nothing here would compile, it's just notes for later.

Also:

  • sus_if_msvc for attributes should be sus_if_msvc_not_cl
    • "language is msvc variant" vs "compiler is msvc"
    • sus_if_gnuc_language
  • test wrapping for each integer size, esp < 32bit
  • not destroy nevervalue? since it's in a union can that be a requirement on NeverValueTypes, and is C++ okay with constructing an object in a place where one already exists and was not destroyed?

Also:
* sus_if_msvc for attributes should be sus_if_msvc_not_cl
  * "language is msvc variant" vs "compiler is msvc"
  * sus_if_gnuc_language
* test wrapping for each integer size, esp < 32bit
* not destroy nevervalue? since it's in a union can that be a
  requirement on NeverValueTypes, and is C++ okay with constructing
  an object in a place where one already exists and was not destroyed?
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.

1 participant