Skip to content

Delete -fms-extensions from coreclr native build - #102834

Merged
jkotas merged 4 commits into
dotnet:mainfrom
am11:feature/std=c++11+no-ms-extensions
Jun 15, 2024
Merged

Delete -fms-extensions from coreclr native build#102834
jkotas merged 4 commits into
dotnet:mainfrom
am11:feature/std=c++11+no-ms-extensions

Conversation

@am11

@am11 am11 commented May 29, 2024

Copy link
Copy Markdown
Member

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.
@ghost ghost added the area-Infrastructure-coreclr Only use for closed issues label May 29, 2024
@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
@dotnet-policy-service

Copy link
Copy Markdown
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
Comment thread src/coreclr/utilcode/stacktrace.cpp
Comment thread src/native/libs/System.Native/pal_networking.c Outdated
Comment thread src/coreclr/inc/check.h
Comment thread src/coreclr/nativeaot/Runtime/eventpipe/ds-rt-aot.cpp Outdated
Comment thread src/coreclr/pal/src/thread/process.cpp Outdated
Comment thread src/coreclr/tools/superpmi/superpmi-shared/standardpch.h
Comment thread src/coreclr/vm/common.h

inline void RetailBreak()
{
#ifdef TARGET_X86

Copy link
Copy Markdown
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
Copy Markdown
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.

Comment thread src/coreclr/vm/eventtrace.cpp Outdated
Comment thread src/coreclr/vm/spinlock.h Outdated
Comment thread src/coreclr/utilcode/md5.cpp Outdated
Comment thread src/coreclr/vm/common.h Outdated
@am11
am11 force-pushed the feature/std=c++11+no-ms-extensions branch from 700888f to 78b29e9 Compare May 30, 2024 16:22
Comment thread src/coreclr/minipal/minipal.h Outdated
@am11

am11 commented Jun 13, 2024

Copy link
Copy Markdown
Member Author

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.

@jkotas jkotas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Infrastructure-coreclr Only use for closed issues community-contribution Indicates that the PR has been added by a community member

4 participants