[PATCH] D123450: [clang-format] Parse Verilog if statements

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/FormatToken.h:1572
+  bool isBlockBegin(const FormatToken , const FormatStyle ) const {
+return Style.isVerilog() ? isVerilogBegin(Tok) : Tok.is(tok::l_brace);
+  }

HazardyKnusperkeks wrote:
> Shouldn't we add TT_MacroBlockBegin?
Adding `TT_MacroBlockBegin` here caused an [[ 
https://github.com/llvm/llvm-project/issues/59335 | assertion failure ]]. See 
also D123450#3768367.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123450

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123450: [clang-format] Parse Verilog if statements

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D123450#3465024 , @MyDeveloperDay 
wrote:

> ``if (Keywords.isBlockBegin(*FormatTok, Style)) {``
>
> I'm not a massive fan of handling l_brace and begin and end as the same 
> thing, I might be tempted to handle them separately, as then its clear what 
> is happening,

+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123450

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123450: [clang-format] Parse Verilog if statements

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2592
 
-  if (FormatTok->is(tok::l_brace)) {
+  if (Keywords.isBlockBegin(*FormatTok, Style)) {
 FormatTok->setFinalizedType(TT_ControlStatementLBrace);

owenpan wrote:
> This is likely the culprit of 
> https://github.com/llvm/llvm-project/issues/57509. @sstwcw can you have a 
> look?
My bad, but see https://reviews.llvm.org/D123450#inline-1282841.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123450

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123450: [clang-format] Parse Verilog if statements

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/FormatToken.h:1550
+  bool isBlockBegin(const FormatToken , const FormatStyle ) const {
+return Tok.is(TT_MacroBlockBegin) ||
+   (Style.isVerilog() ? isVerilogBegin(Tok) : Tok.is(tok::l_brace));

Due of what `isBlockBegin()` is replacing in this patch, `TT_MacroBlockBegin` 
should be added in a separate patch (if it's correct to add it) along with 
corresponding unit tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123450

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123450: [clang-format] Parse Verilog if statements

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2592
 
-  if (FormatTok->is(tok::l_brace)) {
+  if (Keywords.isBlockBegin(*FormatTok, Style)) {
 FormatTok->setFinalizedType(TT_ControlStatementLBrace);

This is likely the culprit of 
https://github.com/llvm/llvm-project/issues/57509. @sstwcw can you have a look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123450

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123450: [clang-format] Parse Verilog if statements

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

`clang/docs/ClangFormatStyleOptions.rst` was out of sync because 
`dump_format_style.py` had not been run. Fixed in cc55d97 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123450

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123450: [clang-format] Parse Verilog if statements

[sstwcw via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9ed2e68c9ae5: [clang-format] Parse Verilog if statements 
(authored by sstwcw).

Changed prior to commit:
  https://reviews.llvm.org/D123450?vs=435777=440021#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123450

Files:
  clang/docs/ClangFormat.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/CMakeLists.txt
  clang/unittests/Format/FormatTestUtils.h
  clang/unittests/Format/FormatTestVerilog.cpp

Index: clang/unittests/Format/FormatTestVerilog.cpp
===
--- /dev/null
+++ clang/unittests/Format/FormatTestVerilog.cpp
@@ -0,0 +1,118 @@
+//===- unittest/Format/FormatTestVerilog.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "FormatTestUtils.h"
+#include "clang/Format/Format.h"
+#include "llvm/Support/Debug.h"
+#include "gtest/gtest.h"
+
+#define DEBUG_TYPE "format-test"
+
+namespace clang {
+namespace format {
+
+class FormatTestVerilog : public ::testing::Test {
+protected:
+  static std::string format(llvm::StringRef Code, unsigned Offset,
+unsigned Length, const FormatStyle ) {
+LLVM_DEBUG(llvm::errs() << "---\n");
+LLVM_DEBUG(llvm::errs() << Code << "\n\n");
+std::vector Ranges(1, tooling::Range(Offset, Length));
+tooling::Replacements Replaces = reformat(Style, Code, Ranges);
+auto Result = applyAllReplacements(Code, Replaces);
+EXPECT_TRUE(static_cast(Result));
+LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+return *Result;
+  }
+
+  static std::string format(llvm::StringRef Code, const FormatStyle ) {
+return format(Code, 0, Code.size(), Style);
+  }
+
+  static void verifyFormat(
+  llvm::StringRef Code,
+  const FormatStyle  = getLLVMStyle(FormatStyle::LK_Verilog)) {
+EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not stable";
+EXPECT_EQ(Code.str(),
+  format(test::messUp(Code, /*HandleHash=*/false), Style));
+  }
+};
+
+TEST_F(FormatTestVerilog, If) {
+  verifyFormat("if (x)\n"
+   "  x = x;");
+  verifyFormat("if (x)\n"
+   "  x = x;\n"
+   "x = x;");
+
+  // Test else
+  verifyFormat("if (x)\n"
+   "  x = x;\n"
+   "else if (x)\n"
+   "  x = x;\n"
+   "else\n"
+   "  x = x;");
+  verifyFormat("if (x) begin\n"
+   "  x = x;\n"
+   "end else if (x) begin\n"
+   "  x = x;\n"
+   "end else begin\n"
+   "  x = x;\n"
+   "end");
+  verifyFormat("if (x) begin : x\n"
+   "  x = x;\n"
+   "end : x else if (x) begin : x\n"
+   "  x = x;\n"
+   "end : x else begin : x\n"
+   "  x = x;\n"
+   "end : x");
+
+  // Test block keywords.
+  verifyFormat("if (x) begin\n"
+   "  x = x;\n"
+   "end");
+  verifyFormat("if (x) begin : x\n"
+   "  x = x;\n"
+   "end : x");
+  verifyFormat("if (x) begin\n"
+   "  x = x;\n"
+   "  x = x;\n"
+   "end");
+  verifyFormat("disable fork;\n"
+   "x = x;");
+  verifyFormat("rand join x x;\n"
+   "x = x;");
+  verifyFormat("if (x) fork\n"
+   "  x = x;\n"
+   "join");
+  verifyFormat("if (x) fork\n"
+   "  x = x;\n"
+   "join_any");
+  verifyFormat("if (x) fork\n"
+   "  x = x;\n"
+   "join_none");
+  verifyFormat("if (x) generate\n"
+   "  x = x;\n"
+   "endgenerate");
+  verifyFormat("if (x) generate : x\n"
+   "  x = x;\n"
+   "endgenerate : x");
+
+  // Test that concatenation braces don't get regarded as blocks.
+  verifyFormat("if (x)\n"
+   "  {x} = x;");
+  verifyFormat("if (x)\n"
+   "  x = {x};");
+  verifyFormat("if (x)\n"
+   "  x = {x};\n"
+   "else\n"
+   "  {x} = {x};");
+}
+
+} // namespace format
+} // end namespace clang
Index: clang/unittests/Format/FormatTestUtils.h
===
--- clang/unittests/Format/FormatTestUtils.h
+++ clang/unittests/Format/FormatTestUtils.h
@@ -19,7 +19,10 @@
 namespace format {
 namespace test 

[PATCH] D123450: [clang-format] Parse Verilog if statements

[sstwcw via Phabricator via cfe-commits]

sstwcw updated this revision to Diff 435777.
sstwcw added a comment.

- add brace


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123450

Files:
  clang/docs/ClangFormat.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/CMakeLists.txt
  clang/unittests/Format/FormatTestUtils.h
  clang/unittests/Format/FormatTestVerilog.cpp

Index: clang/unittests/Format/FormatTestVerilog.cpp
===
--- /dev/null
+++ clang/unittests/Format/FormatTestVerilog.cpp
@@ -0,0 +1,118 @@
+//===- unittest/Format/FormatTestVerilog.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "FormatTestUtils.h"
+#include "clang/Format/Format.h"
+#include "llvm/Support/Debug.h"
+#include "gtest/gtest.h"
+
+#define DEBUG_TYPE "format-test"
+
+namespace clang {
+namespace format {
+
+class FormatTestVerilog : public ::testing::Test {
+protected:
+  static std::string format(llvm::StringRef Code, unsigned Offset,
+unsigned Length, const FormatStyle ) {
+LLVM_DEBUG(llvm::errs() << "---\n");
+LLVM_DEBUG(llvm::errs() << Code << "\n\n");
+std::vector Ranges(1, tooling::Range(Offset, Length));
+tooling::Replacements Replaces = reformat(Style, Code, Ranges);
+auto Result = applyAllReplacements(Code, Replaces);
+EXPECT_TRUE(static_cast(Result));
+LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+return *Result;
+  }
+
+  static std::string format(llvm::StringRef Code, const FormatStyle ) {
+return format(Code, 0, Code.size(), Style);
+  }
+
+  static void verifyFormat(
+  llvm::StringRef Code,
+  const FormatStyle  = getLLVMStyle(FormatStyle::LK_Verilog)) {
+EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not stable";
+EXPECT_EQ(Code.str(),
+  format(test::messUp(Code, /*HandleHash=*/false), Style));
+  }
+};
+
+TEST_F(FormatTestVerilog, If) {
+  verifyFormat("if (x)\n"
+   "  x = x;");
+  verifyFormat("if (x)\n"
+   "  x = x;\n"
+   "x = x;");
+
+  // Test else
+  verifyFormat("if (x)\n"
+   "  x = x;\n"
+   "else if (x)\n"
+   "  x = x;\n"
+   "else\n"
+   "  x = x;");
+  verifyFormat("if (x) begin\n"
+   "  x = x;\n"
+   "end else if (x) begin\n"
+   "  x = x;\n"
+   "end else begin\n"
+   "  x = x;\n"
+   "end");
+  verifyFormat("if (x) begin : x\n"
+   "  x = x;\n"
+   "end : x else if (x) begin : x\n"
+   "  x = x;\n"
+   "end : x else begin : x\n"
+   "  x = x;\n"
+   "end : x");
+
+  // Test block keywords.
+  verifyFormat("if (x) begin\n"
+   "  x = x;\n"
+   "end");
+  verifyFormat("if (x) begin : x\n"
+   "  x = x;\n"
+   "end : x");
+  verifyFormat("if (x) begin\n"
+   "  x = x;\n"
+   "  x = x;\n"
+   "end");
+  verifyFormat("disable fork;\n"
+   "x = x;");
+  verifyFormat("rand join x x;\n"
+   "x = x;");
+  verifyFormat("if (x) fork\n"
+   "  x = x;\n"
+   "join");
+  verifyFormat("if (x) fork\n"
+   "  x = x;\n"
+   "join_any");
+  verifyFormat("if (x) fork\n"
+   "  x = x;\n"
+   "join_none");
+  verifyFormat("if (x) generate\n"
+   "  x = x;\n"
+   "endgenerate");
+  verifyFormat("if (x) generate : x\n"
+   "  x = x;\n"
+   "endgenerate : x");
+
+  // Test that concatenation braces don't get regarded as blocks.
+  verifyFormat("if (x)\n"
+   "  {x} = x;");
+  verifyFormat("if (x)\n"
+   "  x = {x};");
+  verifyFormat("if (x)\n"
+   "  x = {x};\n"
+   "else\n"
+   "  {x} = {x};");
+}
+
+} // namespace format
+} // end namespace clang
Index: clang/unittests/Format/FormatTestUtils.h
===
--- clang/unittests/Format/FormatTestUtils.h
+++ clang/unittests/Format/FormatTestUtils.h
@@ -19,7 +19,10 @@
 namespace format {
 namespace test {
 
-inline std::string messUp(llvm::StringRef Code) {
+// When HandleHash is false, preprocessor directives starting with hash will not
+// be on separate lines.  This is needed because Verilog uses hash for other
+// 

[PATCH] D123450: [clang-format] Parse Verilog if statements

[sstwcw via Phabricator via cfe-commits]

sstwcw updated this revision to Diff 435774.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123450

Files:
  clang/docs/ClangFormat.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/CMakeLists.txt
  clang/unittests/Format/FormatTestUtils.h
  clang/unittests/Format/FormatTestVerilog.cpp

Index: clang/unittests/Format/FormatTestVerilog.cpp
===
--- /dev/null
+++ clang/unittests/Format/FormatTestVerilog.cpp
@@ -0,0 +1,118 @@
+//===- unittest/Format/FormatTestVerilog.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "FormatTestUtils.h"
+#include "clang/Format/Format.h"
+#include "llvm/Support/Debug.h"
+#include "gtest/gtest.h"
+
+#define DEBUG_TYPE "format-test"
+
+namespace clang {
+namespace format {
+
+class FormatTestVerilog : public ::testing::Test {
+protected:
+  static std::string format(llvm::StringRef Code, unsigned Offset,
+unsigned Length, const FormatStyle ) {
+LLVM_DEBUG(llvm::errs() << "---\n");
+LLVM_DEBUG(llvm::errs() << Code << "\n\n");
+std::vector Ranges(1, tooling::Range(Offset, Length));
+tooling::Replacements Replaces = reformat(Style, Code, Ranges);
+auto Result = applyAllReplacements(Code, Replaces);
+EXPECT_TRUE(static_cast(Result));
+LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+return *Result;
+  }
+
+  static std::string format(llvm::StringRef Code, const FormatStyle ) {
+return format(Code, 0, Code.size(), Style);
+  }
+
+  static void verifyFormat(
+  llvm::StringRef Code,
+  const FormatStyle  = getLLVMStyle(FormatStyle::LK_Verilog)) {
+EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not stable";
+EXPECT_EQ(Code.str(),
+  format(test::messUp(Code, /*HandleHash=*/false), Style));
+  }
+};
+
+TEST_F(FormatTestVerilog, If) {
+  verifyFormat("if (x)\n"
+   "  x = x;");
+  verifyFormat("if (x)\n"
+   "  x = x;\n"
+   "x = x;");
+
+  // Test else
+  verifyFormat("if (x)\n"
+   "  x = x;\n"
+   "else if (x)\n"
+   "  x = x;\n"
+   "else\n"
+   "  x = x;");
+  verifyFormat("if (x) begin\n"
+   "  x = x;\n"
+   "end else if (x) begin\n"
+   "  x = x;\n"
+   "end else begin\n"
+   "  x = x;\n"
+   "end");
+  verifyFormat("if (x) begin : x\n"
+   "  x = x;\n"
+   "end : x else if (x) begin : x\n"
+   "  x = x;\n"
+   "end : x else begin : x\n"
+   "  x = x;\n"
+   "end : x");
+
+  // Test block keywords.
+  verifyFormat("if (x) begin\n"
+   "  x = x;\n"
+   "end");
+  verifyFormat("if (x) begin : x\n"
+   "  x = x;\n"
+   "end : x");
+  verifyFormat("if (x) begin\n"
+   "  x = x;\n"
+   "  x = x;\n"
+   "end");
+  verifyFormat("disable fork;\n"
+   "x = x;");
+  verifyFormat("rand join x x;\n"
+   "x = x;");
+  verifyFormat("if (x) fork\n"
+   "  x = x;\n"
+   "join");
+  verifyFormat("if (x) fork\n"
+   "  x = x;\n"
+   "join_any");
+  verifyFormat("if (x) fork\n"
+   "  x = x;\n"
+   "join_none");
+  verifyFormat("if (x) generate\n"
+   "  x = x;\n"
+   "endgenerate");
+  verifyFormat("if (x) generate : x\n"
+   "  x = x;\n"
+   "endgenerate : x");
+
+  // Test that concatenation braces don't get regarded as blocks.
+  verifyFormat("if (x)\n"
+   "  {x} = x;");
+  verifyFormat("if (x)\n"
+   "  x = {x};");
+  verifyFormat("if (x)\n"
+   "  x = {x};\n"
+   "else\n"
+   "  {x} = {x};");
+}
+
+} // namespace format
+} // end namespace clang
Index: clang/unittests/Format/FormatTestUtils.h
===
--- clang/unittests/Format/FormatTestUtils.h
+++ clang/unittests/Format/FormatTestUtils.h
@@ -19,7 +19,10 @@
 namespace format {
 namespace test {
 
-inline std::string messUp(llvm::StringRef Code) {
+// When HandleHash is false, preprocessor directives starting with hash will not
+// be on separate lines.  This is needed because Verilog uses hash for other
+// purposes.
+inline std::string 

[PATCH] D123450: [clang-format] Parse Verilog if statements

[sstwcw via Phabricator via cfe-commits]

sstwcw updated this revision to Diff 435773.
sstwcw added a comment.

use isVerilog


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123450

Files:
  clang/docs/ClangFormat.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/CMakeLists.txt
  clang/unittests/Format/FormatTestUtils.h
  clang/unittests/Format/FormatTestVerilog.cpp

Index: clang/unittests/Format/FormatTestVerilog.cpp
===
--- /dev/null
+++ clang/unittests/Format/FormatTestVerilog.cpp
@@ -0,0 +1,118 @@
+//===- unittest/Format/FormatTestVerilog.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "FormatTestUtils.h"
+#include "clang/Format/Format.h"
+#include "llvm/Support/Debug.h"
+#include "gtest/gtest.h"
+
+#define DEBUG_TYPE "format-test"
+
+namespace clang {
+namespace format {
+
+class FormatTestVerilog : public ::testing::Test {
+protected:
+  static std::string format(llvm::StringRef Code, unsigned Offset,
+unsigned Length, const FormatStyle ) {
+LLVM_DEBUG(llvm::errs() << "---\n");
+LLVM_DEBUG(llvm::errs() << Code << "\n\n");
+std::vector Ranges(1, tooling::Range(Offset, Length));
+tooling::Replacements Replaces = reformat(Style, Code, Ranges);
+auto Result = applyAllReplacements(Code, Replaces);
+EXPECT_TRUE(static_cast(Result));
+LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+return *Result;
+  }
+
+  static std::string format(llvm::StringRef Code, const FormatStyle ) {
+return format(Code, 0, Code.size(), Style);
+  }
+
+  static void verifyFormat(
+  llvm::StringRef Code,
+  const FormatStyle  = getLLVMStyle(FormatStyle::LK_Verilog)) {
+EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not stable";
+EXPECT_EQ(Code.str(),
+  format(test::messUp(Code, /*HandleHash=*/false), Style));
+  }
+};
+
+TEST_F(FormatTestVerilog, If) {
+  verifyFormat("if (x)\n"
+   "  x = x;");
+  verifyFormat("if (x)\n"
+   "  x = x;\n"
+   "x = x;");
+
+  // Test else
+  verifyFormat("if (x)\n"
+   "  x = x;\n"
+   "else if (x)\n"
+   "  x = x;\n"
+   "else\n"
+   "  x = x;");
+  verifyFormat("if (x) begin\n"
+   "  x = x;\n"
+   "end else if (x) begin\n"
+   "  x = x;\n"
+   "end else begin\n"
+   "  x = x;\n"
+   "end");
+  verifyFormat("if (x) begin : x\n"
+   "  x = x;\n"
+   "end : x else if (x) begin : x\n"
+   "  x = x;\n"
+   "end : x else begin : x\n"
+   "  x = x;\n"
+   "end : x");
+
+  // Test block keywords.
+  verifyFormat("if (x) begin\n"
+   "  x = x;\n"
+   "end");
+  verifyFormat("if (x) begin : x\n"
+   "  x = x;\n"
+   "end : x");
+  verifyFormat("if (x) begin\n"
+   "  x = x;\n"
+   "  x = x;\n"
+   "end");
+  verifyFormat("disable fork;\n"
+   "x = x;");
+  verifyFormat("rand join x x;\n"
+   "x = x;");
+  verifyFormat("if (x) fork\n"
+   "  x = x;\n"
+   "join");
+  verifyFormat("if (x) fork\n"
+   "  x = x;\n"
+   "join_any");
+  verifyFormat("if (x) fork\n"
+   "  x = x;\n"
+   "join_none");
+  verifyFormat("if (x) generate\n"
+   "  x = x;\n"
+   "endgenerate");
+  verifyFormat("if (x) generate : x\n"
+   "  x = x;\n"
+   "endgenerate : x");
+
+  // Test that concatenation braces don't get regarded as blocks.
+  verifyFormat("if (x)\n"
+   "  {x} = x;");
+  verifyFormat("if (x)\n"
+   "  x = {x};");
+  verifyFormat("if (x)\n"
+   "  x = {x};\n"
+   "else\n"
+   "  {x} = {x};");
+}
+
+} // namespace format
+} // end namespace clang
Index: clang/unittests/Format/FormatTestUtils.h
===
--- clang/unittests/Format/FormatTestUtils.h
+++ clang/unittests/Format/FormatTestUtils.h
@@ -19,7 +19,10 @@
 namespace format {
 namespace test {
 
-inline std::string messUp(llvm::StringRef Code) {
+// When HandleHash is false, preprocessor directives starting with hash will not
+// be on separate lines.  This is needed because Verilog uses hash for other
+// 

[PATCH] D123450: [clang-format] Parse Verilog if statements

[sstwcw via Phabricator via cfe-commits]

sstwcw updated this revision to Diff 435771.
sstwcw marked 2 inline comments as done.
sstwcw added a comment.

add MacroBlockBegin


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123450

Files:
  clang/docs/ClangFormat.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/CMakeLists.txt
  clang/unittests/Format/FormatTestUtils.h
  clang/unittests/Format/FormatTestVerilog.cpp

Index: clang/unittests/Format/FormatTestVerilog.cpp
===
--- /dev/null
+++ clang/unittests/Format/FormatTestVerilog.cpp
@@ -0,0 +1,118 @@
+//===- unittest/Format/FormatTestVerilog.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "FormatTestUtils.h"
+#include "clang/Format/Format.h"
+#include "llvm/Support/Debug.h"
+#include "gtest/gtest.h"
+
+#define DEBUG_TYPE "format-test"
+
+namespace clang {
+namespace format {
+
+class FormatTestVerilog : public ::testing::Test {
+protected:
+  static std::string format(llvm::StringRef Code, unsigned Offset,
+unsigned Length, const FormatStyle ) {
+LLVM_DEBUG(llvm::errs() << "---\n");
+LLVM_DEBUG(llvm::errs() << Code << "\n\n");
+std::vector Ranges(1, tooling::Range(Offset, Length));
+tooling::Replacements Replaces = reformat(Style, Code, Ranges);
+auto Result = applyAllReplacements(Code, Replaces);
+EXPECT_TRUE(static_cast(Result));
+LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+return *Result;
+  }
+
+  static std::string format(llvm::StringRef Code, const FormatStyle ) {
+return format(Code, 0, Code.size(), Style);
+  }
+
+  static void verifyFormat(
+  llvm::StringRef Code,
+  const FormatStyle  = getLLVMStyle(FormatStyle::LK_Verilog)) {
+EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not stable";
+EXPECT_EQ(Code.str(),
+  format(test::messUp(Code, /*HandleHash=*/false), Style));
+  }
+};
+
+TEST_F(FormatTestVerilog, If) {
+  verifyFormat("if (x)\n"
+   "  x = x;");
+  verifyFormat("if (x)\n"
+   "  x = x;\n"
+   "x = x;");
+
+  // Test else
+  verifyFormat("if (x)\n"
+   "  x = x;\n"
+   "else if (x)\n"
+   "  x = x;\n"
+   "else\n"
+   "  x = x;");
+  verifyFormat("if (x) begin\n"
+   "  x = x;\n"
+   "end else if (x) begin\n"
+   "  x = x;\n"
+   "end else begin\n"
+   "  x = x;\n"
+   "end");
+  verifyFormat("if (x) begin : x\n"
+   "  x = x;\n"
+   "end : x else if (x) begin : x\n"
+   "  x = x;\n"
+   "end : x else begin : x\n"
+   "  x = x;\n"
+   "end : x");
+
+  // Test block keywords.
+  verifyFormat("if (x) begin\n"
+   "  x = x;\n"
+   "end");
+  verifyFormat("if (x) begin : x\n"
+   "  x = x;\n"
+   "end : x");
+  verifyFormat("if (x) begin\n"
+   "  x = x;\n"
+   "  x = x;\n"
+   "end");
+  verifyFormat("disable fork;\n"
+   "x = x;");
+  verifyFormat("rand join x x;\n"
+   "x = x;");
+  verifyFormat("if (x) fork\n"
+   "  x = x;\n"
+   "join");
+  verifyFormat("if (x) fork\n"
+   "  x = x;\n"
+   "join_any");
+  verifyFormat("if (x) fork\n"
+   "  x = x;\n"
+   "join_none");
+  verifyFormat("if (x) generate\n"
+   "  x = x;\n"
+   "endgenerate");
+  verifyFormat("if (x) generate : x\n"
+   "  x = x;\n"
+   "endgenerate : x");
+
+  // Test that concatenation braces don't get regarded as blocks.
+  verifyFormat("if (x)\n"
+   "  {x} = x;");
+  verifyFormat("if (x)\n"
+   "  x = {x};");
+  verifyFormat("if (x)\n"
+   "  x = {x};\n"
+   "else\n"
+   "  {x} = {x};");
+}
+
+} // namespace format
+} // end namespace clang
Index: clang/unittests/Format/FormatTestUtils.h
===
--- clang/unittests/Format/FormatTestUtils.h
+++ clang/unittests/Format/FormatTestUtils.h
@@ -19,7 +19,10 @@
 namespace format {
 namespace test {
 
-inline std::string messUp(llvm::StringRef Code) {
+// When HandleHash is false, preprocessor directives starting with hash will not
+// be on separate lines.  This is needed 

[PATCH] D123450: [clang-format] Parse Verilog if statements

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

As first passes for adding a new language I think this looks pretty good.

``if (Keywords.isBlockBegin(*FormatTok, Style)) {``

I'm not a massive fan of handling l_brace and begin and end as the same thing, 
I might be tempted to handle them separately, as then its clear what is 
happening, but I don't think I'd block it for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123450

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123450: [clang-format] Parse Verilog if statements

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/FormatToken.h:1572
+  bool isBlockBegin(const FormatToken , const FormatStyle ) const {
+return Style.isVerilog() ? isVerilogBegin(Tok) : Tok.is(tok::l_brace);
+  }

Shouldn't we add TT_MacroBlockBegin?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123450

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123450: [clang-format] Parse Verilog if statements

[sstwcw via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/FormatToken.h:374
+  /// Verilog we want to treat the backtick like a hash.
+  tok::TokenKind AliasToken = tok::unknown;
+

HazardyKnusperkeks wrote:
> sstwcw wrote:
> > HazardyKnusperkeks wrote:
> > > Can't we do that with a type?
> > > 
> > > I'm not very happy about the alias, because you can still call 
> > > `Tok.getKind()`.
> > The main problem I seek to solve with the alias thing is with `tok::hash`.  
> > In Verilog they use a backtick instead of a hash.  At first I modified all 
> > places in the code to recognize the backtick.  MyDeveloperDay said "year 
> > really not nice.. its like we can never use tok::hash again!"  Using a type 
> > also requires modifying all instances of tok::hash if I get you right.  How 
> > do I please everyone?
> Then you must hide Tok in the private part, so that no one can ever access 
> `Tok.getKind()` accidentally.
In the new version the type of `Tok` itself gets changed instead.  So an alias 
is not needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123450

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123450: [clang-format] Parse Verilog if statements

[sstwcw via Phabricator via cfe-commits]

sstwcw updated this revision to Diff 422496.
sstwcw edited the summary of this revision.
sstwcw added a comment.

abandon alias


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123450

Files:
  clang/docs/ClangFormat.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/CMakeLists.txt
  clang/unittests/Format/FormatTestUtils.h
  clang/unittests/Format/FormatTestVerilog.cpp

Index: clang/unittests/Format/FormatTestVerilog.cpp
===
--- /dev/null
+++ clang/unittests/Format/FormatTestVerilog.cpp
@@ -0,0 +1,118 @@
+//===- unittest/Format/FormatTestVerilog.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "FormatTestUtils.h"
+#include "clang/Format/Format.h"
+#include "llvm/Support/Debug.h"
+#include "gtest/gtest.h"
+
+#define DEBUG_TYPE "format-test"
+
+namespace clang {
+namespace format {
+
+class FormatTestVerilog : public ::testing::Test {
+protected:
+  static std::string format(llvm::StringRef Code, unsigned Offset,
+unsigned Length, const FormatStyle ) {
+LLVM_DEBUG(llvm::errs() << "---\n");
+LLVM_DEBUG(llvm::errs() << Code << "\n\n");
+std::vector Ranges(1, tooling::Range(Offset, Length));
+tooling::Replacements Replaces = reformat(Style, Code, Ranges);
+auto Result = applyAllReplacements(Code, Replaces);
+EXPECT_TRUE(static_cast(Result));
+LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+return *Result;
+  }
+
+  static std::string format(llvm::StringRef Code, const FormatStyle ) {
+return format(Code, 0, Code.size(), Style);
+  }
+
+  static void verifyFormat(
+  llvm::StringRef Code,
+  const FormatStyle  = getLLVMStyle(FormatStyle::LK_Verilog)) {
+EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not stable";
+EXPECT_EQ(Code.str(),
+  format(test::messUp(Code, /*HandleHash=*/false), Style));
+  }
+};
+
+TEST_F(FormatTestVerilog, If) {
+  verifyFormat("if (x)\n"
+   "  x = x;");
+  verifyFormat("if (x)\n"
+   "  x = x;\n"
+   "x = x;");
+
+  // Test else
+  verifyFormat("if (x)\n"
+   "  x = x;\n"
+   "else if (x)\n"
+   "  x = x;\n"
+   "else\n"
+   "  x = x;");
+  verifyFormat("if (x) begin\n"
+   "  x = x;\n"
+   "end else if (x) begin\n"
+   "  x = x;\n"
+   "end else begin\n"
+   "  x = x;\n"
+   "end");
+  verifyFormat("if (x) begin : x\n"
+   "  x = x;\n"
+   "end : x else if (x) begin : x\n"
+   "  x = x;\n"
+   "end : x else begin : x\n"
+   "  x = x;\n"
+   "end : x");
+
+  // Test block keywords.
+  verifyFormat("if (x) begin\n"
+   "  x = x;\n"
+   "end");
+  verifyFormat("if (x) begin : x\n"
+   "  x = x;\n"
+   "end : x");
+  verifyFormat("if (x) begin\n"
+   "  x = x;\n"
+   "  x = x;\n"
+   "end");
+  verifyFormat("disable fork;\n"
+   "x = x;");
+  verifyFormat("rand join x x;\n"
+   "x = x;");
+  verifyFormat("if (x) fork\n"
+   "  x = x;\n"
+   "join");
+  verifyFormat("if (x) fork\n"
+   "  x = x;\n"
+   "join_any");
+  verifyFormat("if (x) fork\n"
+   "  x = x;\n"
+   "join_none");
+  verifyFormat("if (x) generate\n"
+   "  x = x;\n"
+   "endgenerate");
+  verifyFormat("if (x) generate : x\n"
+   "  x = x;\n"
+   "endgenerate : x");
+
+  // Test that concatenation braces don't get regarded as blocks.
+  verifyFormat("if (x)\n"
+   "  {x} = x;");
+  verifyFormat("if (x)\n"
+   "  x = {x};");
+  verifyFormat("if (x)\n"
+   "  x = {x};\n"
+   "else\n"
+   "  {x} = {x};");
+}
+
+} // namespace format
+} // end namespace clang
Index: clang/unittests/Format/FormatTestUtils.h
===
--- clang/unittests/Format/FormatTestUtils.h
+++ clang/unittests/Format/FormatTestUtils.h
@@ -19,7 +19,10 @@
 namespace format {
 namespace test {
 
-inline std::string messUp(llvm::StringRef Code) {
+// When HandleHash is false, preprocessor directives starting with hash will not
+// be on separate lines.  This is needed 

[PATCH] D123450: [clang-format] Parse Verilog if statements

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/FormatToken.h:374
+  /// Verilog we want to treat the backtick like a hash.
+  tok::TokenKind AliasToken = tok::unknown;
+

sstwcw wrote:
> HazardyKnusperkeks wrote:
> > Can't we do that with a type?
> > 
> > I'm not very happy about the alias, because you can still call 
> > `Tok.getKind()`.
> The main problem I seek to solve with the alias thing is with `tok::hash`.  
> In Verilog they use a backtick instead of a hash.  At first I modified all 
> places in the code to recognize the backtick.  MyDeveloperDay said "year 
> really not nice.. its like we can never use tok::hash again!"  Using a type 
> also requires modifying all instances of tok::hash if I get you right.  How 
> do I please everyone?
Then you must hide Tok in the private part, so that no one can ever access 
`Tok.getKind()` accidentally.



Comment at: clang/lib/Format/FormatToken.h:1533
+switch (Tok.Tok.getKind()) {
+case tok::kw_case:
+case tok::kw_class:

sstwcw wrote:
> HazardyKnusperkeks wrote:
> > So you have a blacklist what is not a keyword? Seems a bit non future 
> > proof, new C++ keywords would have to be added here.
> This is a whitelist of what is a keyword.
My bad. Go on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123450

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123450: [clang-format] Parse Verilog if statements

[sstwcw via Phabricator via cfe-commits]

sstwcw updated this revision to Diff 422049.
sstwcw marked 2 inline comments as done.
sstwcw added a comment.

add comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123450

Files:
  clang/docs/ClangFormat.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/CMakeLists.txt
  clang/unittests/Format/FormatTestUtils.h
  clang/unittests/Format/FormatTestVerilog.cpp

Index: clang/unittests/Format/FormatTestVerilog.cpp
===
--- /dev/null
+++ clang/unittests/Format/FormatTestVerilog.cpp
@@ -0,0 +1,118 @@
+//===- unittest/Format/FormatTestVerilog.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "FormatTestUtils.h"
+#include "clang/Format/Format.h"
+#include "llvm/Support/Debug.h"
+#include "gtest/gtest.h"
+
+#define DEBUG_TYPE "format-test"
+
+namespace clang {
+namespace format {
+
+class FormatTestVerilog : public ::testing::Test {
+protected:
+  static std::string format(llvm::StringRef Code, unsigned Offset,
+unsigned Length, const FormatStyle ) {
+LLVM_DEBUG(llvm::errs() << "---\n");
+LLVM_DEBUG(llvm::errs() << Code << "\n\n");
+std::vector Ranges(1, tooling::Range(Offset, Length));
+tooling::Replacements Replaces = reformat(Style, Code, Ranges);
+auto Result = applyAllReplacements(Code, Replaces);
+EXPECT_TRUE(static_cast(Result));
+LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+return *Result;
+  }
+
+  static std::string format(llvm::StringRef Code, const FormatStyle ) {
+return format(Code, 0, Code.size(), Style);
+  }
+
+  static void verifyFormat(
+  llvm::StringRef Code,
+  const FormatStyle  = getLLVMStyle(FormatStyle::LK_Verilog)) {
+EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not stable";
+EXPECT_EQ(Code.str(),
+  format(test::messUp(Code, /*HandleHash=*/false), Style));
+  }
+};
+
+TEST_F(FormatTestVerilog, If) {
+  verifyFormat("if (x)\n"
+   "  x = x;");
+  verifyFormat("if (x)\n"
+   "  x = x;\n"
+   "x = x;");
+
+  // Test else
+  verifyFormat("if (x)\n"
+   "  x = x;\n"
+   "else if (x)\n"
+   "  x = x;\n"
+   "else\n"
+   "  x = x;");
+  verifyFormat("if (x) begin\n"
+   "  x = x;\n"
+   "end else if (x) begin\n"
+   "  x = x;\n"
+   "end else begin\n"
+   "  x = x;\n"
+   "end");
+  verifyFormat("if (x) begin : x\n"
+   "  x = x;\n"
+   "end : x else if (x) begin : x\n"
+   "  x = x;\n"
+   "end : x else begin : x\n"
+   "  x = x;\n"
+   "end : x");
+
+  // Test block keywords.
+  verifyFormat("if (x) begin\n"
+   "  x = x;\n"
+   "end");
+  verifyFormat("if (x) begin : x\n"
+   "  x = x;\n"
+   "end : x");
+  verifyFormat("if (x) begin\n"
+   "  x = x;\n"
+   "  x = x;\n"
+   "end");
+  verifyFormat("disable fork;\n"
+   "x = x;");
+  verifyFormat("rand join x x;\n"
+   "x = x;");
+  verifyFormat("if (x) fork\n"
+   "  x = x;\n"
+   "join");
+  verifyFormat("if (x) fork\n"
+   "  x = x;\n"
+   "join_any");
+  verifyFormat("if (x) fork\n"
+   "  x = x;\n"
+   "join_none");
+  verifyFormat("if (x) generate\n"
+   "  x = x;\n"
+   "endgenerate");
+  verifyFormat("if (x) generate : x\n"
+   "  x = x;\n"
+   "endgenerate : x");
+
+  // Test that concatenation braces don't get regarded as blocks.
+  verifyFormat("if (x)\n"
+   "  {x} = x;");
+  verifyFormat("if (x)\n"
+   "  x = {x};");
+  verifyFormat("if (x)\n"
+   "  x = {x};\n"
+   "else\n"
+   "  {x} = {x};");
+}
+
+} // namespace format
+} // end namespace clang
Index: clang/unittests/Format/FormatTestUtils.h
===
--- clang/unittests/Format/FormatTestUtils.h
+++ clang/unittests/Format/FormatTestUtils.h
@@ -19,7 +19,10 @@
 namespace format {
 namespace test {
 
-inline 

[PATCH] D123450: [clang-format] Parse Verilog if statements

[sstwcw via Phabricator via cfe-commits]

sstwcw marked 3 inline comments as done.
sstwcw added inline comments.



Comment at: clang/lib/Format/FormatToken.h:374
+  /// Verilog we want to treat the backtick like a hash.
+  tok::TokenKind AliasToken = tok::unknown;
+

HazardyKnusperkeks wrote:
> Can't we do that with a type?
> 
> I'm not very happy about the alias, because you can still call 
> `Tok.getKind()`.
The main problem I seek to solve with the alias thing is with `tok::hash`.  In 
Verilog they use a backtick instead of a hash.  At first I modified all places 
in the code to recognize the backtick.  MyDeveloperDay said "year really not 
nice.. its like we can never use tok::hash again!"  Using a type also requires 
modifying all instances of tok::hash if I get you right.  How do I please 
everyone?



Comment at: clang/lib/Format/FormatToken.h:1157
+VerilogExtraKeywords = std::unordered_set(
+{kw_always,   kw_always_comb,  kw_always_ff,kw_always_latch,
+ kw_assert,   kw_assign,   kw_assume,   kw_automatic,

HazardyKnusperkeks wrote:
> sstwcw wrote:
> > Does anyone know why this part gets aligned unlike the two lists above?
> Have you reformatted the other lines with the same config and revision?
> If yes, my guess would be the missing comment.
It was the missing comment.



Comment at: clang/lib/Format/FormatToken.h:1533
+switch (Tok.Tok.getKind()) {
+case tok::kw_case:
+case tok::kw_class:

HazardyKnusperkeks wrote:
> So you have a blacklist what is not a keyword? Seems a bit non future proof, 
> new C++ keywords would have to be added here.
This is a whitelist of what is a keyword.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123450

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123450: [clang-format] Parse Verilog if statements

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/FormatToken.h:374
+  /// Verilog we want to treat the backtick like a hash.
+  tok::TokenKind AliasToken = tok::unknown;
+

Can't we do that with a type?

I'm not very happy about the alias, because you can still call `Tok.getKind()`.



Comment at: clang/lib/Format/FormatToken.h:1157
+VerilogExtraKeywords = std::unordered_set(
+{kw_always,   kw_always_comb,  kw_always_ff,kw_always_latch,
+ kw_assert,   kw_assign,   kw_assume,   kw_automatic,

sstwcw wrote:
> Does anyone know why this part gets aligned unlike the two lists above?
Have you reformatted the other lines with the same config and revision?
If yes, my guess would be the missing comment.



Comment at: clang/lib/Format/FormatToken.h:1533
+switch (Tok.Tok.getKind()) {
+case tok::kw_case:
+case tok::kw_class:

So you have a blacklist what is not a keyword? Seems a bit non future proof, 
new C++ keywords would have to be added here.



Comment at: clang/lib/Format/FormatToken.h:1593
+
+  std::unordered_set VerilogExtraKeywords;
 };

For consistency reasons add the comment like above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123450

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123450: [clang-format] Parse Verilog if statements

[sstwcw via Phabricator via cfe-commits]

sstwcw added inline comments.



Comment at: clang/lib/Format/FormatToken.h:1157
+VerilogExtraKeywords = std::unordered_set(
+{kw_always,   kw_always_comb,  kw_always_ff,kw_always_latch,
+ kw_assert,   kw_assign,   kw_assume,   kw_automatic,

Does anyone know why this part gets aligned unlike the two lists above?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123450

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123450: [clang-format] Parse Verilog if statements

[sstwcw via Phabricator via cfe-commits]

sstwcw created this revision.
sstwcw added reviewers: MyDeveloperDay, HazardyKnusperkeks, curdeius, owenpan.
Herald added a subscriber: mgorny.
Herald added a project: All.
sstwcw requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch mainly handles treating `begin` as block openers.

While and for statements will be handled in another patch.

We added an alias option in FormatToken to treat a token as another
token.  This way when some language uses an alternative symbol like
backtick for hash we can continue using tok::hash in the code.  For
keywords like `begin`, left braces aren't block openers in if
expressions while they are in structs and enums.  That means we can't
simply treat `begin` and the left brace the same way.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123450

Files:
  clang/docs/ClangFormat.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/CMakeLists.txt
  clang/unittests/Format/FormatTestUtils.h
  clang/unittests/Format/FormatTestVerilog.cpp

Index: clang/unittests/Format/FormatTestVerilog.cpp
===
--- /dev/null
+++ clang/unittests/Format/FormatTestVerilog.cpp
@@ -0,0 +1,118 @@
+//===- unittest/Format/FormatTestVerilog.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "FormatTestUtils.h"
+#include "clang/Format/Format.h"
+#include "llvm/Support/Debug.h"
+#include "gtest/gtest.h"
+
+#define DEBUG_TYPE "format-test"
+
+namespace clang {
+namespace format {
+
+class FormatTestVerilog : public ::testing::Test {
+protected:
+  static std::string format(llvm::StringRef Code, unsigned Offset,
+unsigned Length, const FormatStyle ) {
+LLVM_DEBUG(llvm::errs() << "---\n");
+LLVM_DEBUG(llvm::errs() << Code << "\n\n");
+std::vector Ranges(1, tooling::Range(Offset, Length));
+tooling::Replacements Replaces = reformat(Style, Code, Ranges);
+auto Result = applyAllReplacements(Code, Replaces);
+EXPECT_TRUE(static_cast(Result));
+LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+return *Result;
+  }
+
+  static std::string format(llvm::StringRef Code, const FormatStyle ) {
+return format(Code, 0, Code.size(), Style);
+  }
+
+  static void verifyFormat(
+  llvm::StringRef Code,
+  const FormatStyle  = getLLVMStyle(FormatStyle::LK_Verilog)) {
+EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not stable";
+EXPECT_EQ(Code.str(),
+  format(test::messUp(Code, /*HandleHash=*/false), Style));
+  }
+};
+
+TEST_F(FormatTestVerilog, If) {
+  verifyFormat("if (x)\n"
+   "  x = x;");
+  verifyFormat("if (x)\n"
+   "  x = x;\n"
+   "x = x;");
+
+  // Test else
+  verifyFormat("if (x)\n"
+   "  x = x;\n"
+   "else if (x)\n"
+   "  x = x;\n"
+   "else\n"
+   "  x = x;");
+  verifyFormat("if (x) begin\n"
+   "  x = x;\n"
+   "end else if (x) begin\n"
+   "  x = x;\n"
+   "end else begin\n"
+   "  x = x;\n"
+   "end");
+  verifyFormat("if (x) begin : x\n"
+   "  x = x;\n"
+   "end : x else if (x) begin : x\n"
+   "  x = x;\n"
+   "end : x else begin : x\n"
+   "  x = x;\n"
+   "end : x");
+
+  // Test block keywords.
+  verifyFormat("if (x) begin\n"
+   "  x = x;\n"
+   "end");
+  verifyFormat("if (x) begin : x\n"
+   "  x = x;\n"
+   "end : x");
+  verifyFormat("if (x) begin\n"
+   "  x = x;\n"
+   "  x = x;\n"
+   "end");
+  verifyFormat("disable fork;\n"
+   "x = x;");
+  verifyFormat("rand join x x;\n"
+   "x = x;");
+  verifyFormat("if (x) fork\n"
+   "  x = x;\n"
+   "join");
+  verifyFormat("if (x) fork\n"
+   "  x = x;\n"
+   "join_any");
+  verifyFormat("if (x) fork\n"
+   "  x = x;\n"
+   "join_none");
+  verifyFormat("if (x) generate\n"
+   "  x = x;\n"
+   "endgenerate");
+  verifyFormat("if (x) generate : x\n"
+   "  x = x;\n"
+   "endgenerate : x");
+
+  // Test that