Skip to content

Conversation

@CaseyCarter
Copy link
Contributor

@CaseyCarter CaseyCarter commented Oct 8, 2021

Clang 13 has new warnings to silence:

  • -Wdeprecated-copy: Warns when implicit generation of a copy constructor (respectively copy assignment operator) is deprecated due to the presence of a user-defined copy assignment operator (resp. copy constructor).
  • -Wunused-but-set-variable: Warns when a variable is only ever assigned to. (This presumably ignores class types, or at least classes with non-trivial destructors.)

These changes make us nearly Clang 13-clean. There are also three known-failing libcxx tests that pass with Clang 13 since it no longer emits some false positive -Wdangling warnings. We can't change those tests from XFAIL to PASS until we update our toolset to a VS preview with Clang 13, which will probably happen in the 17.1 preview series.

Clang 13 has new warnings to silence:
* `-Wdeprecated-copy`: Warns when implicit generation of a copy constructor (respectively copy assignment operator) is deprecated due to the presence of a user-defined copy assignment operator (resp. copy constructor).
* `-Wunused-but-set-variable`: Warns when a variable is only ever assigned to. (This presumably ignores class types, or at least classes with non-trivial destructors.)

These changes make us nearly Clang 13-clean. There are also three known-failing libcxx tests that pass with Clang 13 since it no longer emits some false positive `-Wdangling` warnings.
@CaseyCarter CaseyCarter added the enhancement Something can be improved label Oct 8, 2021
@CaseyCarter CaseyCarter requested a review from a team as a code owner October 8, 2021 23:04
@StephanTLavavej StephanTLavavej self-assigned this Oct 13, 2021
@StephanTLavavej

This comment has been minimized.

_Money& _Val; // the monetary amount reference
bool _Intl; // international flag

_Monobj& operator=(const _Monobj&) = delete;
Copy link
Member

Choose a reason for hiding this comment

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

No change requested: I vaguely recall that these were deleted because of obnoxious compiler warnings of the form "whoa, there's a reference data member, I won't be able to generate an assignment operator, and I'm going to warn about this even before you attempt to assign any objects". I believe that these warnings were fixed 🎉 and if we're not seeing them in the test suite then we're good to go.

@StephanTLavavej StephanTLavavej removed their assignment Nov 17, 2021
@mnatsuhara mnatsuhara self-assigned this Dec 1, 2021
@mnatsuhara mnatsuhara removed their assignment Dec 7, 2021
@StephanTLavavej StephanTLavavej self-assigned this Dec 8, 2021
@StephanTLavavej
Copy link
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit f852cb0 into microsoft:main Dec 9, 2021
@StephanTLavavej
Copy link
Member

Thanks for preemptively fixing these warnings! 🛠️ ✔️ 😸

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

Labels

enhancement Something can be improved

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants