[PATCH] D100442: [flang][msvc] Fix compilation of RuntimeGtest tests.

Wed Apr 14 07:19:01 PDT 2021

MehdiChinoune added inline comments.

================
Comment at: flang/unittests/RuntimeGTest/CrashHandlerFixture.cpp:31
   static bool isCrashHanlderRegistered{false};
-  if (not isCrashHanlderRegistered)
+  if (!isCrashHanlderRegistered)
     Fortran::runtime::Terminator::RegisterCrashHandler(CatchCrash);
----------------
awarzynski wrote:
> MehdiChinoune wrote:
> > awarzynski wrote:
> > > Is this change required to fix the build?
> > > Is this change required to fix the build?
> > 
> > Yes
> > 
> > ```
> > [2/34] Building CXX object tools\flang\unittests\RuntimeGTest\CMakeFiles\FlangRuntimeTests.dir\CrashHandlerFixture.cpp.obj
> > FAILED: tools/flang/unittests/RuntimeGTest/CMakeFiles/FlangRuntimeTests.dir/CrashHandlerFixture.cpp.obj
> > C:\PROGRA~2\MICROS~2\2019\COMMUN~1\VC\Tools\MSVC\1428~1.299\bin\Hostx64\x64\cl.exe  /nologo /TP -DBUILD_EXAMPLES -DGTEST_HAS_RTTI=0 -DGTEST_HAS_TR1_TUPLE=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_SILENCE_ALL_CXX17_DEPRECATION_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools\flang\unittests\RuntimeGTest -ID:\dev\llvm-project\flang\unittests\RuntimeGTest -ID:\dev\llvm-project\flang\include -Itools\flang\include -Iinclude -ID:\dev\llvm-project\llvm\include -ID:\dev\llvm-project\llvm\utils\unittest\googletest\include -ID:\dev\llvm-project\llvm\utils\unittest\googlemock\include -ID:\dev\llvm-project\llvm\..\mlir\include -Itools\mlir\include /DWIN32 /D_WINDOWS   /Zc:inline /Zc:__cplusplus /Zc:strictStrings /Oi /Zc:rvalueCast /bigobj /W4 -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd4324 -w14062 -we4238 /Gw /MD /O2 /Ob2  /EHs-c- /GR- -UNDEBUG -std:c++17 /showIncludes /Fotools\flang\unittests\RuntimeGTest\CMakeFiles\FlangRuntimeTests.dir\CrashHandlerFixture.cpp.obj /Fdtools\flang\unittests\RuntimeGTest\CMakeFiles\FlangRuntimeTests.dir\ /FS -c D:\dev\llvm-project\flang\unittests\RuntimeGTest\CrashHandlerFixture.cpp
> > D:\dev\llvm-project\flang\unittests\RuntimeGTest\CrashHandlerFixture.cpp(31): error C2065: 'not': undeclared identifier
> > D:\dev\llvm-project\flang\unittests\RuntimeGTest\CrashHandlerFixture.cpp(31): error C2146: syntax error: missing ')' before identifier 'isCrashHanlderRegistered'
> > D:\dev\llvm-project\flang\unittests\RuntimeGTest\CrashHandlerFixture.cpp(31): error C2059: syntax error: ')'
> > D:\dev\llvm-project\flang\unittests\RuntimeGTest\CrashHandlerFixture.cpp(32): error C2143: syntax error: missing ';' before 'Fortran::runtime::Terminator::RegisterCrashHandler'
> > ```
> I'm not familiar with MSVC, but AFAIK, `not` is a fairly standard C++ keyword. It's not clear to my *why* this change is needed. I appreciate that it fixes things for you, but it feels a bit random.
> 
> Just to clarify, I have no objections to this change, but I would appreciate a bit more justification, ideally with a reference. Something that would clarify *why*. This could be a very short explanation in the commit message.
> but AFAIK, `not` is a fairly standard C++ keyword.

Reference please!

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

https://reviews.llvm.org/D100442