[clang-cl] Reenable /Zc:twoPhase by default if targetting MSVC 2017 Update 3 or newer

[zero9178]

Summary

This patch effectively relands https://reviews.llvm.org/D66394, which back then sadly had to be reverted due to build failures. The summary given in the commit is already great, but just to summarize: Since MSVC 2017 Update 3 (or 19.11, so beyond the 19.14 minimum that clang-cl supports), two phase template parsing had been enabled by default. This patch enables two phase lookup by default in clang-cl as well, when the MSVC compatibility version is 19.11 or higher.

The patch previously had to be reverted due to issues when executed with the GCC style driver and -fno-rtti as can be seen here https://bugs.chromium.org/p/chromium/issues/detail?id=996675. https://reviews.llvm.org/D103771, which this patch depends on, implements one of the resolutions given in the report, by defining _HAS_STATIC_RTTI. For older MSVC STL versions that had a missing guard in the functional header, -fno-rtti-data may have to passed instead of -fno-rtti, as soon as std::function is instantiated, regardless of whether two phase lookup is enabled or not.

Depends on https://reviews.llvm.org/D103771

[zero9178]

Rebased onto main

[thakis]

It was reverted due to build failures, but it didn't reland because it's not the default in cl.exe after all. So we shouldn't make it the default in clang-cl either.

[zero9178]

Thanks for the info, seems like I was mislead after testing it again.

[rnk]

If we look back at the original intention of our MSVC compatibility work, we tried to accept as much invalid code as necessary to parse "system headers". System headers were widely interpreted to bethe MSVC STL, ATL, and the Windows SDK. So, even if MSVC defaults to delayed template parsing, if system headers parse with /Zc:twoPhase enabled, maybe we could pick a different default. We'd document the behavior change, of course. Delayed template parsing is an ongoing source of bugs (https://llvm.org/pr50676) and maintenance overhead, so it would be worth diverging from MSVC a bit if possible.

So... maybe we should go ahead with this anyway. As I understand it, all these headers parse correctly in the presence of /Zc:twoPhase, and the RTTI issue has been resolved (?), so we should be OK. I think.

clang/docs/ReleaseNotes.rst

 - ...
 
 - Added support for C++11-style ``[[]]`` attributes on using-declarations, as a
   clang extension.
 
 Windows Support
 ---------------
 
+- clang-cl now defaults to ``/Zc:twoPhase`` if targeting MSVC2017 update 3 or
+  later (``_MSC_VER`` >= 1911). This matches MSVC's behavior. Explicitly pass
+  ``/Zc:twoPhase-`` to restore the old behavior.
+
 C Language Changes in Clang
 ---------------------------
 
 - ...
 
 C++ Language Changes in Clang
 -----------------------------
 

clang/include/clang/Driver/Options.td

 def _SLASH_Zc_threadSafeInit_ : CLFlag<"Zc:threadSafeInit-">,
   HelpText<"Disable thread-safe initialization of static variables">,
   Alias<fno_threadsafe_statics>;
 def _SLASH_Zc_trigraphs : CLFlag<"Zc:trigraphs">,
   HelpText<"Enable trigraphs">, Alias<ftrigraphs>;
 def _SLASH_Zc_trigraphs_off : CLFlag<"Zc:trigraphs-">,
   HelpText<"Disable trigraphs (default)">, Alias<fno_trigraphs>;
 def _SLASH_Zc_twoPhase : CLFlag<"Zc:twoPhase">,
-  HelpText<"Enable two-phase name lookup in templates">,
+  HelpText<"Enable two-phase name lookup in templates (default)">,
   Alias<fno_delayed_template_parsing>;
 def _SLASH_Zc_twoPhase_ : CLFlag<"Zc:twoPhase-">,
-  HelpText<"Disable two-phase name lookup in templates (default)">,
+  HelpText<"Disable two-phase name lookup in templates">,
   Alias<fdelayed_template_parsing>;
 def _SLASH_Z7 : CLFlag<"Z7">,
   HelpText<"Enable CodeView debug information in object files">;
 def _SLASH_Zi : CLFlag<"Zi">, Alias<_SLASH_Z7>,
   HelpText<"Like /Z7">;
 def _SLASH_Zp : CLJoined<"Zp">,
   HelpText<"Set default maximum struct packing alignment">,
   Alias<fpack_struct_EQ>;

clang/lib/Driver/ToolChains/Clang.cpp

 
   // -fthreadsafe-static is default, except for MSVC compatibility versions less
   // than 19.
   if (!Args.hasFlag(options::OPT_fthreadsafe_statics,
                     options::OPT_fno_threadsafe_statics,
                     !IsWindowsMSVC || IsMSVC2015Compatible))
     CmdArgs.push_back("-fno-threadsafe-statics");
 
-  // -fno-delayed-template-parsing is default, except when targeting MSVC.
-  // Many old Windows SDK versions require this to parse.
-  // FIXME: MSVC introduced /Zc:twoPhase- to disable this behavior in their
-  // compiler. We should be able to disable this by default at some point.
+  // -fno-delayed-template-parsing is default, except when targeting MSVC
+  // earlier than MSVC 2017 15.3 (_MSC_VER 1911). Windows SDK versions
+  // 10.0.15063.0 (Creators Update or Redstone 2) and earlier require this to
+  // parse.
+  bool IsMSVCBefore2017Update3 = !MSVT.empty() && MSVT < VersionTuple(19, 11);
   if (Args.hasFlag(options::OPT_fdelayed_template_parsing,
-                   options::OPT_fno_delayed_template_parsing, IsWindowsMSVC))
+                   options::OPT_fno_delayed_template_parsing,
+                   IsMSVCBefore2017Update3))
     CmdArgs.push_back("-fdelayed-template-parsing");
 
   // -fgnu-keywords default varies depending on language; only pass if
   // specified.
   Args.AddLastArg(CmdArgs, options::OPT_fgnu_keywords,
                   options::OPT_fno_gnu_keywords);
 
   if (Args.hasFlag(options::OPT_fgnu89_inline, options::OPT_fno_gnu89_inline,

clang/test/Driver/cl-options.c

 // DEFAULTNULL-NOT: "-fno-delete-null-pointer-checks"
 // RUN: %clang_cl -c -fdelete-null-pointer-checks -### -- %s 2>&1 | FileCheck -check-prefix=NULL %s
 // NULL-NOT: "-fno-delete-null-pointer-checks"
 // RUN: %clang_cl -c -fno-delete-null-pointer-checks -### -- %s 2>&1 | FileCheck -check-prefix=NONULL %s
 // NONULL: "-fno-delete-null-pointer-checks"
 
 // We recognize -f[no-]delayed-template-parsing.
 // /Zc:twoPhase[-] has the opposite meaning.
-// RUN: %clang_cl -c -### -- %s 2>&1 | FileCheck -check-prefix=DELAYEDDEFAULT %s
-// DELAYEDDEFAULT: "-fdelayed-template-parsing"
-// RUN: %clang_cl -c -fdelayed-template-parsing -### -- %s 2>&1 | FileCheck -check-prefix=DELAYEDON %s
-// RUN: %clang_cl -c /Zc:twoPhase- -### -- %s 2>&1 | FileCheck -check-prefix=DELAYEDON %s
+// RUN: %clang_cl -c -fmsc-version=1910 -### -- %s 2>&1 | FileCheck -check-prefix=OLDDELAYEDDEFAULT %s
+// OLDDELAYEDDEFAULT: "-fdelayed-template-parsing"
+// RUN: %clang_cl -c -fmsc-version=1911 -### -- %s 2>&1 | FileCheck -check-prefix=NEWDELAYEDDEFAULT %s
+// NEWDELAYEDDEFAULT-NOT: "-fdelayed-template-parsing"
+// RUN: %clang_cl -c -fmsc-version=1910 -fdelayed-template-parsing -### -- %s 2>&1 | FileCheck -check-prefix=DELAYEDON %s
+// RUN: %clang_cl -c -fmsc-version=1911 -fdelayed-template-parsing -### -- %s 2>&1 | FileCheck -check-prefix=DELAYEDON %s
+// RUN: %clang_cl -c -fmsc-version=1910 /Zc:twoPhase- -### -- %s 2>&1 | FileCheck -check-prefix=DELAYEDON %s
+// RUN: %clang_cl -c -fmsc-version=1911 /Zc:twoPhase- -### -- %s 2>&1 | FileCheck -check-prefix=DELAYEDON %s
 // DELAYEDON: "-fdelayed-template-parsing"
-// RUN: %clang_cl -c -fno-delayed-template-parsing -### -- %s 2>&1 | FileCheck -check-prefix=DELAYEDOFF %s
-// RUN: %clang_cl -c /Zc:twoPhase -### -- %s 2>&1 | FileCheck -check-prefix=DELAYEDOFF %s
+// RUN: %clang_cl -c -fmsc-version=1910 -fno-delayed-template-parsing -### -- %s 2>&1 | FileCheck -check-prefix=DELAYEDOFF %s
+// RUN: %clang_cl -c -fmsc-version=1911 -fno-delayed-template-parsing -### -- %s 2>&1 | FileCheck -check-prefix=DELAYEDOFF %s
+// RUN: %clang_cl -c -fmsc-version=1910 /Zc:twoPhase -### -- %s 2>&1 | FileCheck -check-prefix=DELAYEDOFF %s
+// RUN: %clang_cl -c -fmsc-version=1911 /Zc:twoPhase -### -- %s 2>&1 | FileCheck -check-prefix=DELAYEDOFF %s
 // DELAYEDOFF-NOT: "-fdelayed-template-parsing"
 
 // RUN: %clang_cl -c -### /std:c++latest -- %s 2>&1 | FileCheck -check-prefix CHECK-LATEST-CHAR8_T %s
 // CHECK-LATEST-CHAR8_T-NOT: "-fchar8_t"
 // RUN: %clang_cl -c -### /Zc:char8_t -- %s 2>&1 | FileCheck -check-prefix CHECK-CHAR8_T %s
 // CHECK-CHAR8_T: "-fchar8_t"
 // RUN: %clang_cl -c -### /Zc:char8_t- -- %s 2>&1 | FileCheck -check-prefix CHECK-CHAR8_T_ %s
 // CHECK-CHAR8_T_: "-fno-char8_t"