[PATCH] D76433: [Syntax] Test both the default and windows target platforms in unittests

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 accepted this revision.
gribozavr2 added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:102
+    std::vector<const char *> Args = {"-target",       Target.data(),
+                                      "-fsyntax-only", "-std=c++11",
+                                      "syntax-test",   FileName};
----------------
Maybe upgrade that to C++17? I hope this is where the world is moving.


================
Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:125
+  void expectTreeDumpEqual(StringRef Code, StringRef Tree,
+                           bool SkipWindows = false) {
+    SCOPED_TRACE(Code);
----------------
s/SkipWindows/RunWithDelayedTemplateParsing/ ?


================
Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:131
+    for (const StringRef Target :
+         {"x86_64-unknown-unknown", "x86_64-pc-win32"}) {
+      if (SkipWindows && Target.equals("x86_64-pc-win32")) {
----------------
Please add a comment explaining that we want to test with delayed template parsing on and off, hence we choose two representative targets where that is normal.


================
Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:809
+      // tree when -fdelayed-template-parsing is active.
+      /*SkipWindows*/ true);
 }
----------------
Need an equal sign in /*SkipWindows=*/.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76433