[clang-format] [PR46159] Linux kernel 'C' code uses 'try' as a variable name, allow clang-format to handle such cases

[MyDeveloperDay]

Summary

https://bugs.llvm.org/show_bug.cgi?id=46159

Linux Kernel code using is try as a variable name (as its 'C' code they can), they interpreted as the 'try' keyword by clang-format causing odd formatting (see below)

We have no specific 'C' code handling, as this code could easily be in a '.h' file so need to handle this not based on extension alone.

This revision adds a FormatTokenLexer rule to identify either try blocks "try {" or function try block "try :" and if not, rename the "try" token to be an identifier

static size_t get_kcore_size(int *nphdr, size_t *phdrs_len, size_t *notes_len,
			     size_t *data_offset)
{
	size_t try, size;
	struct kcore_list *m;

	*nphdr = 1; /* PT_NOTE */
	size = 0;

	list_for_each_entry(m, &kclist_head, list) {
		try = kc_vaddr_to_offset((size_t)m->addr + m->size);
		if (try1 > size)
			size = try1;
		*nphdr = *nphdr + 1;
	}
}

clang-format-10 formats it as:

static size_t get_kcore_size(int *nphdr, size_t *phdrs_len, size_t *notes_len,
                             size_t *data_offset) {
  size_t try
    , size;
  struct kcore_list *m;

  *nphdr = 1; /* PT_NOTE */
  size = 0;

  list_for_each_entry(m, &kclist_head, list) {
    try
      = kc_vaddr_to_offset((size_t)m->addr + m->size);
    if (try > size)
      size = try
        ;
    *nphdr = *nphdr + 1;
  }
}

[curdeius]

Nice. Should we test other non-C keywords as well?
Out of curiosity, where does "@try" come from?

clang/lib/Format/FormatTokenLexer.cpp

392

Please rename to something that includes colon or is more general. Just using Brace is misleading.

393

Nit: You can move this if above.

[MyDeveloperDay]

Out of curiosity, where does "@try" come from?

Objective C++ I think

[MyDeveloperDay]

Address review comments

[curdeius]

LGTM.

For a second, I thought that you could simplify the code by removing this @try condition (and calling the function FormatTokenLexer::tryTransformTryUsageForC() only if isCppOnly, but given that Objective is a superset of C, it's probably safer to keep it the way it's done now.

[MyDeveloperDay]

In D80940#2071247, @curdeius wrote:

LGTM.

For a second, I thought that you could simplify the code by removing this @try condition (and calling the function FormatTokenLexer::tryTransformTryUsageForC() only if isCppOnly, but given that Objective is a superset of C, it's probably safer to keep it the way it's done now.

this is the premise of D80079: [clang-format] [NFC] isCpp() is inconsistently used to mean both C++ and Objective C, add language specific isXXX() functions but as we now are learning isCpp() is not what is says on the tin!

clang/lib/Format/FormatTokenLexer.h

private:
   bool tryMergeNSStringLiteral();
   bool tryMergeJSPrivateIdentifier();
   bool tryMergeCSharpStringLiteral();
   bool tryMergeCSharpKeywordVariables();
   bool tryMergeCSharpDoubleQuestion();
   bool tryMergeCSharpNullConditional();
   bool tryTransformCSharpForEach();
   bool tryMergeForEach();
+  bool tryTransformTryUsageForC();
 
   bool tryMergeTokens(ArrayRef<tok::TokenKind> Kinds, TokenType NewType);
 
   // Returns \c true if \p Tok can only be followed by an operand in JavaScript.
   bool precedesOperand(FormatToken *Tok);
 
   bool canPrecedeRegexLiteral(FormatToken *Prev);
 

clang/lib/Format/FormatTokenLexer.cpp

void FormatTokenLexer::tryMergePreviousTokens() {
   if (tryMerge_TMacro())
     return;
   if (tryMergeConflictMarkers())
     return;
   if (tryMergeLessLess())
     return;
   if (tryMergeForEach())
     return;
+  if (Style.isCpp() && tryTransformTryUsageForC())
+    return;
 
   if (Style.isCSharp()) {
     if (tryMergeCSharpKeywordVariables())
       return;
     if (tryMergeCSharpStringLiteral())
       return;
     if (tryMergeCSharpDoubleQuestion())
       return;
bool FormatTokenLexer::tryMergeForEach() {
 
   For->TokenText = StringRef(For->TokenText.begin(),
                              Each->TokenText.end() - For->TokenText.begin());
   For->ColumnWidth += Each->ColumnWidth;
   Tokens.erase(Tokens.end() - 1);
   return true;
 }
 
+bool FormatTokenLexer::tryTransformTryUsageForC() {
+  if (Tokens.size() < 2)
+    return false;
+  auto &Try = *(Tokens.end() - 2);
+  auto &Brace = *(Tokens.end() - 1);
+  if (!Try->is(tok::kw_try))
+    return false;
+  if (Brace->isOneOf(tok::l_brace, tok::colon))
+    return false;
+
+  if (Tokens.size() > 2) {
+    auto &At = *(Tokens.end() - 3);
+    if (At->is(tok::at))
+      return false;
+  }
+
+  Try->Tok.setKind(tok::identifier);
+  return true;
+}
+
 bool FormatTokenLexer::tryMergeLessLess() {
   // Merge X,less,less,Y into X,lessless,Y unless X or Y is less.
   if (Tokens.size() < 3)
     return false;
 
   bool FourthTokenIsLess = false;
   if (Tokens.size() > 3)
     FourthTokenIsLess = (Tokens.end() - 4)[0]->is(tok::less);

clang/unittests/Format/FormatTest.cpp

verifyFormat("class A {\n"
                "    throw;\n"
                "  }\n"
                "};\n");
 
   // Incomplete try-catch blocks.
   verifyIncompleteFormat("try {} catch (");
 }
 
+TEST_F(FormatTest, FormatTryAsAVariable) {
+  verifyFormat("int try;");
+  verifyFormat("int try, size;");
+  verifyFormat("try = foo();");
+  verifyFormat("if (try < size) {\n  return true;\n}");
+
+  verifyFormat("int catch;");
+  verifyFormat("int catch, size;");
+  verifyFormat("catch = foo();");
+  verifyFormat("if (catch < size) {\n  return true;\n}");
+}
+
 TEST_F(FormatTest, FormatSEHTryCatch) {
   verifyFormat("__try {\n"
                "  int a = b * c;\n"
                "} __except (EXCEPTION_EXECUTE_HANDLER) {\n"
                "  // Do nothing.\n"
                "}");
 
   verifyFormat("__try {\n"