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

More C++ tweaks and changes #9478

Merged
merged 12 commits into from
Dec 5, 2024
Merged

More C++ tweaks and changes #9478

merged 12 commits into from
Dec 5, 2024

Conversation

grendello
Copy link
Contributor

@grendello grendello commented Oct 31, 2024

Continued effort to restructure our native C++ runtime, introducing
C++23 features as well as turning as many classes into static ones since
we don't really have objects that need to be created and destroyed during
lifetime of the application:

  • Replace abort_if* preprocessor macros with templated functions. This
    gives us better type safety. Instead using variadic arguments, we now
    provide overloads which take either a plain string without placeholders
    or a lambda function to format the message string on demand.
  • More std::string_view for literal strings
  • More functions decorated with noexcept (this is going to be important
    once exceptions are enabled in the future)
  • Remove some unused code.

@grendello grendello force-pushed the dev/grendel/c++23 branch 2 times, most recently from cc3f7dd to 8187508 Compare November 7, 2024 13:14
@grendello grendello force-pushed the dev/grendel/c++23 branch 5 times, most recently from 88bc1b0 to 614b4aa Compare November 25, 2024 12:19
@grendello grendello marked this pull request as ready for review November 26, 2024 08:42
@grendello grendello requested a review from jonpryor as a code owner November 26, 2024 08:42
@grendello grendello force-pushed the dev/grendel/c++23 branch 2 times, most recently from 52e7070 to db9f5f3 Compare November 29, 2024 08:55
@grendello grendello mentioned this pull request Nov 29, 2024
@@ -603,7 +603,7 @@ Debug::enable_soft_breakpoints (void)
void*
xamarin::android::conn_thread (void *arg)
{
abort_if_invalid_pointer_argument (arg);
abort_if_invalid_pointer_argument (arg, "arg");
Copy link
Member

Choose a reason for hiding this comment

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

I wonder how meaningful this will be "without context": the abort messages which may be logged to the Google Play Store (594d090) will only contain the message Parameter 'arg' must be a valid pointer. Which "arg"?

I think the parameter should also include the method name in the message. so that a "context free" (no adb logcat) message at least lets us know what function we're talking about:

Parameter 'arg' in function 'conn_thread' must be a valid pointer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Implemented in the latest commit

@jonpryor jonpryor merged commit 7b7cd13 into main Dec 5, 2024
58 checks passed
@jonpryor jonpryor deleted the dev/grendel/c++23 branch December 5, 2024 15:43
grendello added a commit that referenced this pull request Dec 5, 2024
* main:
  [native] More C++ tweaks and changes (#9478)
  Add android-platform-support to .gitignore (#9590)
  [ci] Skip JDK install if near match is found in $(JI_JAVA_HOME) (#9585)
  Localized file check-in by OneLocBuild Task: Build definition ID 17928: Build ID 10653360 (#9587)
  Revert "Try dependabot max_length param (#9529)"
  Try dependabot max_length param (#9529)
  LEGO: Pull request (#9575)
  Bump to DevDiv/android-platform-support@52dd010a (#9553)
  Localized file check-in by OneLocBuild Task (#9574)
grendello added a commit that referenced this pull request Dec 5, 2024
* main:
  [native] More C++ tweaks and changes (#9478)
  Add android-platform-support to .gitignore (#9590)
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