[mlir][PDLL] Add support for c-style comment

[Chia-hungDuan]

Summary

Given that pdll is another format of source file, we may need to add the
copyright disclaimer in c-style comment in some projects.

[jpienaar]

Not sure I follow why c-style is requirement? E.g., copyright in Python files is close to // per line

[rriddle]

In D126315#3535032, @jpienaar wrote:

Not sure I follow why c-style is requirement? E.g., copyright in Python files is close to // per line

I was thinking the same, you could just use // for each line of the copyright.

[Chia-hungDuan]

In D126315#3535032, @jpienaar wrote:

Not sure I follow why c-style is requirement? E.g., copyright in Python files is close to // per line

I think in Tensorflow we are using /* ... */, for example, https://github.com/tensorflow/tensorflow/blob/master/tensorflow/core/transforms/graph_transform_wrapper.cc#L1-L14

Or do we have multiple styles there?

[rriddle]

In D126315#3535051, @Chia-hungDuan wrote:

In D126315#3535032, @jpienaar wrote:

Not sure I follow why c-style is requirement? E.g., copyright in Python files is close to // per line

I think in Tensorflow we are using /* ... */, for example, https://github.com/tensorflow/tensorflow/blob/master/tensorflow/core/transforms/graph_transform_wrapper.cc#L1-L14

Or do we have multiple styles there?

I don't think TF is consistent, it uses // in various languages (even C++ at times): https://github.com/tensorflow/tensorflow/blob/e755ae9c7faa3715dff3a3247872a4499d5a53cd/tensorflow/lite/objc/sources/TFLCommonUtil.h#L1

[Chia-hungDuan]

In D126315#3535058, @rriddle wrote:

In D126315#3535051, @Chia-hungDuan wrote:

In D126315#3535032, @jpienaar wrote:

Not sure I follow why c-style is requirement? E.g., copyright in Python files is close to // per line

I think in Tensorflow we are using /* ... */, for example, https://github.com/tensorflow/tensorflow/blob/master/tensorflow/core/transforms/graph_transform_wrapper.cc#L1-L14

Or do we have multiple styles there?

I don't think TF is consistent, it uses // in various languages (even C++ at times): https://github.com/tensorflow/tensorflow/blob/e755ae9c7faa3715dff3a3247872a4499d5a53cd/tensorflow/lite/objc/sources/TFLCommonUtil.h#L1

Oh I see. So do you prefer to only have one comment style? If so, I could use // style there. I was thinking if we support this, that may be easier for including the copyright disclaimer because in td file, they can use c-style comment. People won't be confused why they need to use the different style. No strong preference here.

[rriddle]

In D126315#3535084, @Chia-hungDuan wrote:

In D126315#3535058, @rriddle wrote:

In D126315#3535051, @Chia-hungDuan wrote:

In D126315#3535032, @jpienaar wrote:

Not sure I follow why c-style is requirement? E.g., copyright in Python files is close to // per line

I think in Tensorflow we are using /* ... */, for example, https://github.com/tensorflow/tensorflow/blob/master/tensorflow/core/transforms/graph_transform_wrapper.cc#L1-L14

Or do we have multiple styles there?

I don't think TF is consistent, it uses // in various languages (even C++ at times): https://github.com/tensorflow/tensorflow/blob/e755ae9c7faa3715dff3a3247872a4499d5a53cd/tensorflow/lite/objc/sources/TFLCommonUtil.h#L1

Oh I see. So do you prefer to only have one comment style? If so, I could use // style there. I was thinking if we support this, that may be easier for including the copyright disclaimer because in td file, they can use c-style comment. People won't be confused why they need to use the different style. No strong preference here.

I think it'd be nice if we can just avoid it, or at least I'd prefer if we find value in having them in general vs just for the copyright. In .td files I've seen we mostly just use them for parameter comment like things, but for PDLL I'd almost prefer we just use the IDE for those types of things (e.g. https://reviews.llvm.org/D126033)

[Chia-hungDuan]

In D126315#3535102, @rriddle wrote:

In D126315#3535084, @Chia-hungDuan wrote:

In D126315#3535058, @rriddle wrote:

In D126315#3535051, @Chia-hungDuan wrote:

In D126315#3535032, @jpienaar wrote:

Not sure I follow why c-style is requirement? E.g., copyright in Python files is close to // per line

I think in Tensorflow we are using /* ... */, for example, https://github.com/tensorflow/tensorflow/blob/master/tensorflow/core/transforms/graph_transform_wrapper.cc#L1-L14

Or do we have multiple styles there?

I don't think TF is consistent, it uses // in various languages (even C++ at times): https://github.com/tensorflow/tensorflow/blob/e755ae9c7faa3715dff3a3247872a4499d5a53cd/tensorflow/lite/objc/sources/TFLCommonUtil.h#L1

Oh I see. So do you prefer to only have one comment style? If so, I could use // style there. I was thinking if we support this, that may be easier for including the copyright disclaimer because in td file, they can use c-style comment. People won't be confused why they need to use the different style. No strong preference here.

I think it'd be nice if we can just avoid it, or at least I'd prefer if we find value in having them in general vs just for the copyright. In .td files I've seen we mostly just use them for parameter comment like things, but for PDLL I'd almost prefer we just use the IDE for those types of things (e.g. https://reviews.llvm.org/D126033)

Got it! I'll use the // style first. Thanks!

mlir/lib/Tools/PDLL/Parser/Lexer.h

Token formToken(Token::Kind kind, const char *tokStart) {
     return Token(kind, StringRef(tokStart, curPtr - tokStart));
   }
 
   /// Return the next character in the stream.
   int getNextChar();
 
   /// Lex methods.
   void lexComment();
+  LogicalResult lexCComment();
   Token lexDirective(const char *tokStart);
   Token lexIdentifier(const char *tokStart);
   Token lexNumber(const char *tokStart);
   Token lexString(const char *tokStart, bool isStringBlock);
 
   llvm::SourceMgr &srcMgr;
   int curBufferID;
   StringRef curBuffer;

mlir/lib/Tools/PDLL/Parser/Lexer.cpp

switch (curChar) {
       case '(':
         return formToken(Token::l_paren, tokStart);
       case ')':
         return formToken(Token::r_paren, tokStart);
       case '/':
         if (*curPtr == '/') {
           lexComment();
           continue;
+        } else if (*curPtr == '*') {
+          if (failed(lexCComment()))
+            return emitError(curPtr - 1, "unterminated comment");
+          continue;
         }
         return emitError(tokStart, "unexpected character");
 
       // Ignore whitespace characters.
       case 0:
       case ' ':
       case '\t':
       case '\n':
switch (*curPtr++) {
         LLVM_FALLTHROUGH;
       default:
         // Skip over other characters.
         break;
     }
   }
 }
 
+/// Skip a c-style comment, which is in the form of /**/.
+LogicalResult Lexer::lexCComment() {
+  // Advance over the '*'.
+  assert(*curPtr == '*');
+  ++curPtr;
+  unsigned nestedDepth = 1;
+
+  while (true) {
+    switch (*curPtr++) {
+    case 0:
+      return failure();
+    case '*':
+      if (curPtr[0] == '/') {
+        // This is the end of comment.
+        ++curPtr;
+        if (--nestedDepth == 0)
+          return success();
+      }
+      break;
+    case '/':
+      if (curPtr[0] == '*') {
+        // Start of a nested comment.
+        ++nestedDepth;
+        ++curPtr;
+      }
+      break;
+    }
+  }
+
+  llvm_unreachable("EOF is handled in the switch above");
+}
+
 Token Lexer::lexDirective(const char *tokStart) {
   // Match the rest with an identifier regex: [0-9a-zA-Z_]*
   while (isalnum(*curPtr) || *curPtr == '_') ++curPtr;
 
   StringRef str(tokStart, curPtr - tokStart);
   return Token(Token::directive, str);
 }
 

mlir/test/mlir-pdll/Parser/comment-failure.pdll

+// RUN: not mlir-pdll %s -I %S -split-input-file 2>&1 | FileCheck %s
+
+/*
+  /* This is malformed C-style comment */
+// CHECK: unterminated comment
+Pattern {
+  erase _: Op;
+}

mlir/test/mlir-pdll/Parser/comment.pdll

+// RUN: mlir-pdll %s -I %S -split-input-file 2>&1 | FileCheck %s
+
+/* This is C-style comment in single line */
+// CHECK: Module
+// CHECK: EraseStmt
+// CHECK:   `-DeclRefExpr {{.*}} Type<Op>
+Pattern {
+  erase _: Op;
+}
+
+// -----
+
+/*
+  This is C-style comment in multiple lines
+*/
+// CHECK: Module
+// CHECK: EraseStmt
+// CHECK:   `-DeclRefExpr {{.*}} Type<Op>
+Pattern {
+  erase _: Op;
+}
+
+// -----
+
+/*
+  /* This is nested C-style comment in multiple lines */
+*/
+// CHECK: Module
+// CHECK: EraseStmt
+// CHECK:   `-DeclRefExpr {{.*}} Type<Op>
+Pattern {
+  erase _: Op;
+}