Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

[release/3.1] Fix build with clang 10 #28026

Merged
merged 1 commit into from
Mar 25, 2020
Merged

Conversation

omajid
Copy link
Member

@omajid omajid commented Mar 11, 2020

This fixes 3 different sets of build issues that are seen when compiling with clang 10.

  • Clang 10 fails to compile slist.h because the code contained is actually invalid. The assignment operator being used doesn't exist.

    This is a backport of SList::Init: add missing constructor runtime#33096

  • Clang 10 has moved exception-handling mismatches in function declarations under the -fms-compatibility flag (instead of the -fms-extensions flag). Our declarations of atoll and other similar functions are missing the exception declaration throw(). This mismatch in exception declarations makes clang 10 unable to build this code. Fix it by defining THROW_DECL as throw() which is supported at least as far back as clang 3.3.

    This is a backport of Fix re-declarations of builtin functions with clang 10 runtime#32837

  • Clang 10 has enabled additional warnings. Lets turn of -Werror globally in this release branch by making the -ignorewarnings switch to ./build.sh be the default.

@omajid
Copy link
Member Author

omajid commented Mar 13, 2020

cc @jkotas @janvorli @danmosemsft. Can someone please review this?

@jkotas
Copy link
Member

jkotas commented Mar 13, 2020

Would it be better to fix most of these problems once and for all by disabling all warnings by default (for release branches)?

@omajid
Copy link
Member Author

omajid commented Mar 13, 2020

Would it be better to fix most of these problems once and for all by disabling all warnings by default (for release branches)?

I think so. I don't know if it would have helped with the first issue which is actually invalid code.

I generally defer to https://flameeyes.blog/2009/02/25/future-proof-your-code-dont-use-werror/ which says:

So please, don’t use -Werror in released code, make it optional, use it during development, but not in released code.

@jkotas
Copy link
Member

jkotas commented Mar 16, 2020

@omajid Could you please amend this PR with a change to disable werror, and undo changes that will be no longer necessary after that?

This fixes 3 different sets of build issues seen when compiling with
Clang 10.

- Clang 10 fails to compile slist.h because the code contained is
  actually invalid. The assignment operator being used doesn't exist.

  This is a backport of dotnet/runtime#33096

- Clang 10 has moved exception-handling mismatches in function
  declarations under the -fms-compatibility flag (instead of the
  -fms-extensions flag). Our declarations of atoll and other similar
  functions are missing the exception declaration `throw()`. This
  mismatch in exception declarations makes clang 10 unable to build this
  code. Fix it by defining THROW_DECL as `throw()` which is supported at
  least as far back as clang 3.3.

  This is a backport of dotnet/runtime#32837

- Clang 10 has enabled additional warnings. Lets turn of -Werror
  globally in this release branch by making the `-ignorewarnings` switch
  to `./build.sh` be the default.
@omajid omajid force-pushed the 3.1-clang10 branch 2 times, most recently from b72c44b to ee39a13 Compare March 16, 2020 23:38
@omajid
Copy link
Member Author

omajid commented Mar 16, 2020

@jkotas Done. Turns out only misleading-identation issue was a warning.

As for THROW_DECL, I could, alternatively, pass -fms-compatibility along with -fms-extensions instead of changing the definition of THROW_DECL.

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.

Thanks!

@jeffschwMSFT jeffschwMSFT added the Servicing-consider Issue for next servicing release review label Mar 17, 2020
@jeffschwMSFT jeffschwMSFT added this to the 3.1.x milestone Mar 17, 2020
@leecow leecow added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Mar 17, 2020
@leecow leecow modified the milestones: 3.1.x, 3.1.4 Mar 17, 2020
@Anipik Anipik merged commit aaa1e0e into dotnet:release/3.1 Mar 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants