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

Delete -fms-extensions from coreclr native build #102834

Merged
merged 4 commits into from
Jun 15, 2024

Conversation

am11
Copy link
Member

@am11 am11 commented May 29, 2024

For stdcpp conformance, remove the remaining special extensions.

  • clean up unnecessary __llvm special handling.
  • Add class, enum, struct prefixes to cases where the variable name is clashing with type name and it changes meaning (-Werror=changes-meaning)
    • In some trivial cases, I just renamed the variables..
  • On x86, replace __asm {} syntax with __asm ("")
  • Two small illumos related build fixes under src/native which helped validating the rest of the changes on the platform.

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label May 29, 2024
Copy link
Contributor

Tagging subscribers to this area: @hoyosjs
See info in area-owners.md if you want to be subscribed.

@am11 am11 force-pushed the feature/std=c++11+no-ms-extensions branch from 27819e3 to b621127 Compare May 30, 2024 06:59
@am11 am11 marked this pull request as ready for review May 30, 2024 07:10
@am11 am11 requested review from jkoritzinsky and jkotas May 30, 2024 07:11
src/coreclr/inc/check.h Show resolved Hide resolved
src/coreclr/nativeaot/Runtime/eventpipe/ds-rt-aot.cpp Outdated Show resolved Hide resolved
src/coreclr/pal/src/thread/process.cpp Outdated Show resolved Hide resolved
@@ -204,11 +204,7 @@ EXTERN_C AppDomain* STDCALL GetAppDomain();

inline void RetailBreak()
{
#ifdef TARGET_X86
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 why it was defined differently for x86. @jkotas do you have any idea?

Copy link
Member

Choose a reason for hiding this comment

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

Historically, inline int 3 gave a better experience under the debugger - the debugger stopped in your code, not inside a Windows DebugBreak API.

src/coreclr/vm/eventtrace.cpp Outdated Show resolved Hide resolved
src/coreclr/vm/spinlock.h Outdated Show resolved Hide resolved
src/coreclr/vm/common.h Outdated Show resolved Hide resolved
@am11 am11 force-pushed the feature/std=c++11+no-ms-extensions branch from 700888f to 78b29e9 Compare May 30, 2024 16:22
@am11 am11 requested a review from janvorli June 13, 2024 05:05
@am11
Copy link
Member Author

am11 commented Jun 13, 2024

OSX failure is infra issue (tests are passing but helix job is unable to exit cleanly):

/private/tmp/helix/working/A8E70893/w/A68408BE/e /private/tmp/helix/working/A8E70893/w/A68408BE/e
  Discovering: System.Text.RegularExpressions.Unit.Tests (method display = ClassAndMethod, method display options = None)
  Discovered:  System.Text.RegularExpressions.Unit.Tests (found 25 test cases)
  Starting:    System.Text.RegularExpressions.Unit.Tests (parallel test collections = on [12 threads], stop on fail = off)
  Finished:    System.Text.RegularExpressions.Unit.Tests
=== TEST EXECUTION SUMMARY ===
   System.Text.RegularExpressions.Unit.Tests  Total: 830, Errors: 0, Failed: 0, Skipped: 0, Time: 0.352s
/private/tmp/helix/working/A8E70893/w/A68408BE/e
----- end Wed Jun 12 17:28:07 EDT 2024 ----- exit code 0 ----------------------------------------------------------
exit code 0 means Exited Successfully
Unable to obtain kernel buffer: Operation not permitted
usage: sudo dmesg
----- start =============== XUnitLogChecker Output =====================================================
No dumps found in /cores.

rest of failures are known issues (per Build Analysis).

@jkotas, PTAL.

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thank you!

@jkotas jkotas merged commit 3099f31 into dotnet:main Jun 15, 2024
175 of 179 checks passed
@am11 am11 deleted the feature/std=c++11+no-ms-extensions branch June 15, 2024 06:14
@github-actions github-actions bot locked and limited conversation to collaborators Jul 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Infrastructure-coreclr community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants