[PATCH] D56329: Fix some warnings on MSVC

Wed Jan 23 11:10:54 PST 2019

rnk 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() {}
----------------
aganea wrote:
> 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?
Sounds reasonable.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D56329