[TableGen] Make the NUL character invalid in .td files, version 2

[Paul-C-Anagnostopoulos]

Summary

This revision makes the NUL character invalid in TableGen .td files.

The first version used sed to convert @ to NUL in the test file. That didn't work on at least one build. This version uses tr.

[dblaikie]

(can be easier to reuse the same review - that way it's easier to check the diff between the previous committed version of the patch to see what's being fixed)

If the only change is from sed to tr - this seems worth a shot. Keep an eye on the bots & see how it goes!

[Paul-C-Anagnostopoulos]

I couldn't find anything in the actions to "restart review." Did I miss it?

[dblaikie]

In D102254#2751174, @Paul-C-Anagnostopoulos wrote:

I couldn't find anything in the actions to "restart review." Did I miss it?

I think you can update the review again - I don't know that there's any explicit "restart" state transition.

llvm/lib/TableGen/TGLexer.cpp

bool TGLexer::processEOF() {
   return false;
 }
 
 int TGLexer::getNextChar() {
   char CurChar = *CurPtr++;
   switch (CurChar) {
   default:
     return (unsigned char)CurChar;
-  case 0: {
-    // A nul character in the stream is either the end of the current buffer or
-    // a random nul in the file.  Disambiguate that here.
-    if (CurPtr-1 != CurBuf.end())
-      return 0;  // Just whitespace.
 
-    // Otherwise, return end of file.
-    --CurPtr;  // Another call to lex will return EOF again.
+  case 0: {
+    // A NUL character in the stream is either the end of the current buffer or
+    // a spurious NUL in the file.  Disambiguate that here.
+    if (CurPtr - 1 == CurBuf.end()) {
+      --CurPtr; // Arrange for another call to return EOF again.
       return EOF;
     }
+    PrintError(getLoc(),
+               "NUL character is invalid in source; treated as space");
+    return ' ';
+  }
+
   case '\n':
   case '\r':
     // Handle the newline character by ignoring it and incrementing the line
     // count.  However, be careful about 'dos style' files with \n\r in them.
     // Only treat a \n\r or \r\n as a single line.
     if ((*CurPtr == '\n' || (*CurPtr == '\r')) &&
         *CurPtr != CurChar)
       ++CurPtr;  // Eat the two char newline sequence.
if (peekNextChar(0) == '.') {
       return ReturnError(TokStart, "Invalid '..' punctuation");
     }
     return tgtok::dot;
 
   case '\r':
     PrintFatalError("getNextChar() must never return '\r'");
     return tgtok::Error;
 
-  case 0:
   case ' ':
   case '\t':
     // Ignore whitespace.
     return LexToken(FileOrLineStart);
   case '\n':
     // Ignore whitespace, and identify the new line.
     return LexToken(true);
   case '/':
bool TGLexer::LexInclude() {
   CurBuf = SrcMgr.getMemoryBuffer(CurBuffer)->getBuffer();
   CurPtr = CurBuf.begin();
 
   PrepIncludeStack.push_back(
       std::make_unique<std::vector<PreprocessorControlDesc>>());
   return false;
 }
 
+/// SkipBCPLComment - Skip over the comment by finding the next CR or LF.
+/// Or we may end up at the end of the buffer.
 void TGLexer::SkipBCPLComment() {
   ++CurPtr;  // skip the second slash.
-  while (true) {
-    switch (*CurPtr) {
-    case '\n':
-    case '\r':
-      return;  // Newline is end of comment.
-    case 0:
-      // If this is the end of the buffer, end the comment.
-      if (CurPtr == CurBuf.end())
-        return;
-      break;
-    }
-    // Otherwise, skip the character.
-    ++CurPtr;
-  }
+  auto EOLPos = CurBuf.find_first_of("\r\n", CurPtr - CurBuf.data());
+  CurPtr = (EOLPos == StringRef::npos) ? CurBuf.end() : CurBuf.data() + EOLPos;
 }
 
 /// SkipCComment - This skips C-style /**/ comments.  The only difference from C
 /// is that we allow nesting.
 bool TGLexer::SkipCComment() {
   ++CurPtr;  // skip the star.
   unsigned CommentDepth = 1;
 

llvm/test/TableGen/nul-char.td

+// RUN: cat %s | tr '@' '\0' > %t
+// RUN: not llvm-tblgen -DERROR1 %t 2>&1 | FileCheck --check-prefix=ERROR1 %s
+
+// This test file checks that NUL is treated as an invalid character.
+// Each at sign is replaced with a NUL before running the test.
+
+#ifdef ERROR1
+
+// ERROR1: error: NUL character is invalid in source; treated as space
+// ERROR1: error: NUL character is invalid in source; treated as space
+// ERROR1: error: NUL character is invalid in source; treated as space
+// ERROR1: error: NUL character is invalid in source; treated as space
+// ERROR1: error: expected ';' after declaration
+
+def Foo@ {
+  int @ ID = 42;
+}
+
+@
+
+// Comment with a NUL @ there. They are ignored in comments.
+
+def Bar {
+  int Biggie = 12345@789;
+}
+
+#endif
+