[PATCH] D56329: Fix some warnings on MSVC

[Alexandre Ganea via Phabricator via llvm-commits]

aganea marked an inline comment as done.
aganea added inline comments.


================
Comment at: utils/unittest/googlemock/include/gmock/gmock-matchers.h:145-148
+  // Define virtual destructor to avoid the following MSVC warning:
+  // warning C5046: 'testing::MatcherInterface<T>::~MatcherInterface':
+  //    Symbol involving type with internal linkage not defined
+  virtual ~MatcherInterface() {}
----------------
rnk wrote:
> C5046 seems like some kind of undocumented warning:
> https://developercommunity.visualstudio.com/content/problem/288509/undocumented-warning-c5046-symbol-involving-type-w.html
> 
> Maybe it will go away in new versions of VS if we do nothing? I don't see anything wrong with this code. It has an implicit virtual destructor because its base class has one.
> 
> I guess I'd like to hear more about how this comes up in practice before applying this change, especially since it's imported gmock code.
Yeah it's a warning that came up in the last months, since one of the VS2017 15.8.XX upgrades. The warning I'm seeing is this:
```
46>f:\svn\llvm\utils\unittest\googlemock\include\gmock\gmock-matchers.h(186): warning C5046: 'testing::MatcherInterface<T>::~MatcherInterface': Symbol involving type with internal linkage not defined
46>        with
46>        [
46>            T=`anonymous-namespace'::CustomSubError &
46>        ] (compiling source file F:\svn\llvm\unittests\Support\ErrorTest.cpp)
```
You're probably right, it's most likely a spurious warning. For a similar example, your linked webpage says:

> Jonathan Emmett [MSFT] 
> Thanks for this example. This is indeed a false-positive and we have a fix for this particular issue related to special member functions. It will be addressed in Visual Studio 2019.

Maybe add `-ignore:5046` for `ErrorTest.cpp` in the meanwhile?


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56329/new/

https://reviews.llvm.org/D56329