Skip to content

Code review snippets to address from @davidben #400

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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?
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant