[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito updated this revision to Diff 400704.
psigillito added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Removed old approach and started initial changes to add C language


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

Files:
  .arclint


Index: .arclint
===
--- .arclint
+++ .arclint
@@ -2,7 +2,7 @@
   "linters": {
 "clang-format": {
   "type": "script-and-regex",
-  "script-and-regex.script": "cmda utils\\arcanist\\clang-format.sh",
+  "script-and-regex.script": "cmd utils\\arcanist\\clang-format.sh",
   "script-and-regex.regex": 
"/^(?P[[:alpha:]]+)\n(?P[^\n]+)\n(|(?P\\d),(?P\\d)\n(?P.*)\n(?P.*)\n)$/s",
   "include": [
 "(\\.(cc|cpp|h)$)"


Index: .arclint
===
--- .arclint
+++ .arclint
@@ -2,7 +2,7 @@
   "linters": {
 "clang-format": {
   "type": "script-and-regex",
-  "script-and-regex.script": "cmda utils\\arcanist\\clang-format.sh",
+  "script-and-regex.script": "cmd utils\\arcanist\\clang-format.sh",
   "script-and-regex.regex": "/^(?P[[:alpha:]]+)\n(?P[^\n]+)\n(|(?P\\d),(?P\\d)\n(?P.*)\n(?P.*)\n)$/s",
   "include": [
 "(\\.(cc|cpp|h)$)"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito updated this revision to Diff 400706.
psigillito added a comment.

revert changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

Files:
  .arclint


Index: .arclint
===
--- .arclint
+++ .arclint
@@ -2,7 +2,7 @@
   "linters": {
 "clang-format": {
   "type": "script-and-regex",
-  "script-and-regex.script": "cmda utils\\arcanist\\clang-format.sh",
+  "script-and-regex.script": "cmd utils\\arcanist\\clang-format.sh",
   "script-and-regex.regex": 
"/^(?P[[:alpha:]]+)\n(?P[^\n]+)\n(|(?P\\d),(?P\\d)\n(?P.*)\n(?P.*)\n)$/s",
   "include": [
 "(\\.(cc|cpp|h)$)"


Index: .arclint
===
--- .arclint
+++ .arclint
@@ -2,7 +2,7 @@
   "linters": {
 "clang-format": {
   "type": "script-and-regex",
-  "script-and-regex.script": "cmda utils\\arcanist\\clang-format.sh",
+  "script-and-regex.script": "cmd utils\\arcanist\\clang-format.sh",
   "script-and-regex.regex": "/^(?P[[:alpha:]]+)\n(?P[^\n]+)\n(|(?P\\d),(?P\\d)\n(?P.*)\n(?P.*)\n)$/s",
   "include": [
 "(\\.(cc|cpp|h)$)"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito updated this revision to Diff 400708.
psigillito added a comment.

Hopefully this revision works


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

Files:
  .arclint
  clang/include/clang/Format/Format.h
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatTokenLexer.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/tools/clang-format/ClangFormat.cpp

Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -121,6 +121,12 @@
 Verbose("verbose", cl::desc("If set, shows the list of processed files"),
 cl::cat(ClangFormatCategory));
 
+static cl::opt
+SetLanguage("set-language",
+cl::desc("Manually sets the programming language clang-format "
+ "interprets files as"),
+cl::init(""), cl::cat(ClangFormatCategory));
+
 // Use --dry-run to match other LLVM tools when you mean do it but don't
 // actually do it
 static cl::opt
@@ -458,6 +464,12 @@
   Replacements Replaces = sortIncludes(*FormatStyle, Code->getBuffer(), Ranges,
AssumedFileName, &CursorPosition);
 
+  if (SetLanguage.getNumOccurrences() != 0) {
+
+if (SetLanguage.getValue() == "C" || SetLanguage.getValue() == "c") {
+  FormatStyle->Language = FormatStyle::LK_C;
+}
+  }
   // To format JSON insert a variable to trick the code into thinking its
   // JavaScript.
   if (FormatStyle->isJson() && !FormatStyle->DisableFormat) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1218,7 +1218,8 @@
   case tok::kw_public:
   case tok::kw_protected:
   case tok::kw_private:
-if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
+if (Style.Language == FormatStyle::LK_C ||
+Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   nextToken();
 else
@@ -1450,7 +1451,7 @@
   if (!parseEnum())
 break;
   // This only applies for C++.
-  if (!Style.isCpp()) {
+  if (!Style.isCpp() && !Style.isC()) {
 addUnwrappedLine();
 return;
   }
@@ -1493,7 +1494,8 @@
   parseParens();
   // Break the unwrapped line if a K&R C function definition has a parameter
   // declaration.
-  if (!IsTopLevel || !Style.isCpp() || !Previous || FormatTok->is(tok::eof))
+  if (!IsTopLevel || (!Style.isCpp() && !Style.isC()) || !Previous ||
+  FormatTok->is(tok::eof))
 break;
   if (isC78ParameterDecl(FormatTok, Tokens->peekNextToken(), Previous)) {
 addUnwrappedLine();
@@ -1601,7 +1603,7 @@
 break;
   }
 
-  if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+  if ((Style.isCpp() || Style.isC()) && FormatTok->is(TT_StatementMacro)) {
 parseStatementMacro();
 return;
   }
@@ -2647,7 +2649,7 @@
   nextToken();
   // If there are two identifiers in a row, this is likely an elaborate
   // return type. In Java, this can be "implements", etc.
-  if (Style.isCpp() && FormatTok->is(tok::identifier))
+  if ((Style.isCpp() || Style.isC()) && FormatTok->is(tok::identifier))
 return false;
 }
   }
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -97,7 +97,8 @@
   /// For example, 'public:' labels in classes are offset by 1 or 2
   /// characters to the left from their level.
   int getIndentOffset(const FormatToken &RootToken) {
-if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
+if (Style.Language == FormatStyle::LK_C ||
+Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   return 0;
 if (RootToken.isAccessSpecifier(false) ||
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -799,7 +799,8 @@
   if (Previous->is(TT_JsTypeOptionalQuestion))
 Previous = Previous->getPreviousNonComment();
   if ((CurrentToken->is(tok::colon) &&
-   (!Contexts.back().ColonIsDictLiteral || !Style.isCpp())) ||
+   (!Contexts.back().ColonIsDictLiteral ||
+(!Style.isCpp() && !Style.isC( ||
   Style.Language

[PATCH] D117536: Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito created this revision.
psigillito requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

revert old changes, breakout c into own language initial changes

revert arclint changes

revert bad arclint change


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117536

Files:
  .arclint
  clang/include/clang/Format/Format.h
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatTokenLexer.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/tools/clang-format/ClangFormat.cpp

Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -121,6 +121,12 @@
 Verbose("verbose", cl::desc("If set, shows the list of processed files"),
 cl::cat(ClangFormatCategory));
 
+static cl::opt
+SetLanguage("set-language",
+cl::desc("Manually sets the programming language clang-format "
+ "interprets files as"),
+cl::init(""), cl::cat(ClangFormatCategory));
+
 // Use --dry-run to match other LLVM tools when you mean do it but don't
 // actually do it
 static cl::opt
@@ -458,6 +464,12 @@
   Replacements Replaces = sortIncludes(*FormatStyle, Code->getBuffer(), Ranges,
AssumedFileName, &CursorPosition);
 
+  if (SetLanguage.getNumOccurrences() != 0) {
+
+if (SetLanguage.getValue() == "C" || SetLanguage.getValue() == "c") {
+  FormatStyle->Language = FormatStyle::LK_C;
+}
+  }
   // To format JSON insert a variable to trick the code into thinking its
   // JavaScript.
   if (FormatStyle->isJson() && !FormatStyle->DisableFormat) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1218,7 +1218,8 @@
   case tok::kw_public:
   case tok::kw_protected:
   case tok::kw_private:
-if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
+if (Style.Language == FormatStyle::LK_C ||
+Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   nextToken();
 else
@@ -1450,7 +1451,7 @@
   if (!parseEnum())
 break;
   // This only applies for C++.
-  if (!Style.isCpp()) {
+  if (!Style.isCpp() && !Style.isC()) {
 addUnwrappedLine();
 return;
   }
@@ -1493,7 +1494,8 @@
   parseParens();
   // Break the unwrapped line if a K&R C function definition has a parameter
   // declaration.
-  if (!IsTopLevel || !Style.isCpp() || !Previous || FormatTok->is(tok::eof))
+  if (!IsTopLevel || (!Style.isCpp() && !Style.isC()) || !Previous ||
+  FormatTok->is(tok::eof))
 break;
   if (isC78ParameterDecl(FormatTok, Tokens->peekNextToken(), Previous)) {
 addUnwrappedLine();
@@ -1601,7 +1603,7 @@
 break;
   }
 
-  if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+  if ((Style.isCpp() || Style.isC()) && FormatTok->is(TT_StatementMacro)) {
 parseStatementMacro();
 return;
   }
@@ -2647,7 +2649,7 @@
   nextToken();
   // If there are two identifiers in a row, this is likely an elaborate
   // return type. In Java, this can be "implements", etc.
-  if (Style.isCpp() && FormatTok->is(tok::identifier))
+  if ((Style.isCpp() || Style.isC()) && FormatTok->is(tok::identifier))
 return false;
 }
   }
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -97,7 +97,8 @@
   /// For example, 'public:' labels in classes are offset by 1 or 2
   /// characters to the left from their level.
   int getIndentOffset(const FormatToken &RootToken) {
-if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
+if (Style.Language == FormatStyle::LK_C ||
+Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   return 0;
 if (RootToken.isAccessSpecifier(false) ||
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -799,7 +799,8 @@
   if (Previous->is(TT_JsTypeOptionalQuestion))
 Previous = Previous->getPreviousNonComment();
   if ((CurrentToken->is(tok::colon) &&
-   (!Contexts.back().ColonIsDictLiteral || !Style.isCpp())) ||
+   (!Contexts.back().ColonIsDictLi

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito updated this revision to Diff 400715.
psigillito added a comment.

Include multiple commits in review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

Files:
  .arclint
  clang/include/clang/Format/Format.h
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatTokenLexer.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/tools/clang-format/ClangFormat.cpp

Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -121,6 +121,12 @@
 Verbose("verbose", cl::desc("If set, shows the list of processed files"),
 cl::cat(ClangFormatCategory));
 
+static cl::opt
+SetLanguage("set-language",
+cl::desc("Manually sets the programming language clang-format "
+ "interprets files as"),
+cl::init(""), cl::cat(ClangFormatCategory));
+
 // Use --dry-run to match other LLVM tools when you mean do it but don't
 // actually do it
 static cl::opt
@@ -458,6 +464,12 @@
   Replacements Replaces = sortIncludes(*FormatStyle, Code->getBuffer(), Ranges,
AssumedFileName, &CursorPosition);
 
+  if (SetLanguage.getNumOccurrences() != 0) {
+
+if (SetLanguage.getValue() == "C" || SetLanguage.getValue() == "c") {
+  FormatStyle->Language = FormatStyle::LK_C;
+}
+  }
   // To format JSON insert a variable to trick the code into thinking its
   // JavaScript.
   if (FormatStyle->isJson() && !FormatStyle->DisableFormat) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1218,7 +1218,8 @@
   case tok::kw_public:
   case tok::kw_protected:
   case tok::kw_private:
-if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
+if (Style.Language == FormatStyle::LK_C ||
+Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   nextToken();
 else
@@ -1450,7 +1451,7 @@
   if (!parseEnum())
 break;
   // This only applies for C++.
-  if (!Style.isCpp()) {
+  if (!Style.isCpp() && !Style.isC()) {
 addUnwrappedLine();
 return;
   }
@@ -1493,7 +1494,8 @@
   parseParens();
   // Break the unwrapped line if a K&R C function definition has a parameter
   // declaration.
-  if (!IsTopLevel || !Style.isCpp() || !Previous || FormatTok->is(tok::eof))
+  if (!IsTopLevel || (!Style.isCpp() && !Style.isC()) || !Previous ||
+  FormatTok->is(tok::eof))
 break;
   if (isC78ParameterDecl(FormatTok, Tokens->peekNextToken(), Previous)) {
 addUnwrappedLine();
@@ -1601,7 +1603,7 @@
 break;
   }
 
-  if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+  if ((Style.isCpp() || Style.isC()) && FormatTok->is(TT_StatementMacro)) {
 parseStatementMacro();
 return;
   }
@@ -2647,7 +2649,7 @@
   nextToken();
   // If there are two identifiers in a row, this is likely an elaborate
   // return type. In Java, this can be "implements", etc.
-  if (Style.isCpp() && FormatTok->is(tok::identifier))
+  if ((Style.isCpp() || Style.isC()) && FormatTok->is(tok::identifier))
 return false;
 }
   }
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -97,7 +97,8 @@
   /// For example, 'public:' labels in classes are offset by 1 or 2
   /// characters to the left from their level.
   int getIndentOffset(const FormatToken &RootToken) {
-if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
+if (Style.Language == FormatStyle::LK_C ||
+Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   return 0;
 if (RootToken.isAccessSpecifier(false) ||
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -799,7 +799,8 @@
   if (Previous->is(TT_JsTypeOptionalQuestion))
 Previous = Previous->getPreviousNonComment();
   if ((CurrentToken->is(tok::colon) &&
-   (!Contexts.back().ColonIsDictLiteral || !Style.isCpp())) ||
+   (!Contexts.back().ColonIsDictLiteral ||
+(!Style.isCpp() && !Style.isC( ||
   Style.Lan

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito updated this revision to Diff 400717.
psigillito added a comment.

- undo delete


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

Files:
  .arclint
  clang/include/clang/Format/Format.h
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatTokenLexer.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/tools/clang-format/ClangFormat.cpp

Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -121,6 +121,12 @@
 Verbose("verbose", cl::desc("If set, shows the list of processed files"),
 cl::cat(ClangFormatCategory));
 
+static cl::opt
+SetLanguage("set-language",
+cl::desc("Manually sets the programming language clang-format "
+ "interprets files as"),
+cl::init(""), cl::cat(ClangFormatCategory));
+
 // Use --dry-run to match other LLVM tools when you mean do it but don't
 // actually do it
 static cl::opt
@@ -458,6 +464,12 @@
   Replacements Replaces = sortIncludes(*FormatStyle, Code->getBuffer(), Ranges,
AssumedFileName, &CursorPosition);
 
+  if (SetLanguage.getNumOccurrences() != 0) {
+
+if (SetLanguage.getValue() == "C" || SetLanguage.getValue() == "c") {
+  FormatStyle->Language = FormatStyle::LK_C;
+}
+  }
   // To format JSON insert a variable to trick the code into thinking its
   // JavaScript.
   if (FormatStyle->isJson() && !FormatStyle->DisableFormat) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1218,7 +1218,8 @@
   case tok::kw_public:
   case tok::kw_protected:
   case tok::kw_private:
-if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
+if (Style.Language == FormatStyle::LK_C ||
+Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   nextToken();
 else
@@ -1450,7 +1451,7 @@
   if (!parseEnum())
 break;
   // This only applies for C++.
-  if (!Style.isCpp()) {
+  if (!Style.isCpp() && !Style.isC()) {
 addUnwrappedLine();
 return;
   }
@@ -1493,7 +1494,8 @@
   parseParens();
   // Break the unwrapped line if a K&R C function definition has a parameter
   // declaration.
-  if (!IsTopLevel || !Style.isCpp() || !Previous || FormatTok->is(tok::eof))
+  if (!IsTopLevel || (!Style.isCpp() && !Style.isC()) || !Previous ||
+  FormatTok->is(tok::eof))
 break;
   if (isC78ParameterDecl(FormatTok, Tokens->peekNextToken(), Previous)) {
 addUnwrappedLine();
@@ -1601,7 +1603,7 @@
 break;
   }
 
-  if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+  if ((Style.isCpp() || Style.isC()) && FormatTok->is(TT_StatementMacro)) {
 parseStatementMacro();
 return;
   }
@@ -2647,7 +2649,7 @@
   nextToken();
   // If there are two identifiers in a row, this is likely an elaborate
   // return type. In Java, this can be "implements", etc.
-  if (Style.isCpp() && FormatTok->is(tok::identifier))
+  if ((Style.isCpp() || Style.isC()) && FormatTok->is(tok::identifier))
 return false;
 }
   }
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -97,7 +97,8 @@
   /// For example, 'public:' labels in classes are offset by 1 or 2
   /// characters to the left from their level.
   int getIndentOffset(const FormatToken &RootToken) {
-if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
+if (Style.Language == FormatStyle::LK_C ||
+Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   return 0;
 if (RootToken.isAccessSpecifier(false) ||
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -799,7 +799,8 @@
   if (Previous->is(TT_JsTypeOptionalQuestion))
 Previous = Previous->getPreviousNonComment();
   if ((CurrentToken->is(tok::colon) &&
-   (!Contexts.back().ColonIsDictLiteral || !Style.isCpp())) ||
+   (!Contexts.back().ColonIsDictLiteral ||
+(!Style.isCpp() && !Style.isC( ||
   Style.Language == FormatStyle:

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito updated this revision to Diff 400718.
psigillito added a comment.

- annoying arc changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

Files:
  .arclint
  clang/include/clang/Format/Format.h
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatTokenLexer.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/tools/clang-format/ClangFormat.cpp

Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -121,6 +121,12 @@
 Verbose("verbose", cl::desc("If set, shows the list of processed files"),
 cl::cat(ClangFormatCategory));
 
+static cl::opt
+SetLanguage("set-language",
+cl::desc("Manually sets the programming language clang-format "
+ "interprets files as"),
+cl::init(""), cl::cat(ClangFormatCategory));
+
 // Use --dry-run to match other LLVM tools when you mean do it but don't
 // actually do it
 static cl::opt
@@ -458,6 +464,12 @@
   Replacements Replaces = sortIncludes(*FormatStyle, Code->getBuffer(), Ranges,
AssumedFileName, &CursorPosition);
 
+  if (SetLanguage.getNumOccurrences() != 0) {
+
+if (SetLanguage.getValue() == "C" || SetLanguage.getValue() == "c") {
+  FormatStyle->Language = FormatStyle::LK_C;
+}
+  }
   // To format JSON insert a variable to trick the code into thinking its
   // JavaScript.
   if (FormatStyle->isJson() && !FormatStyle->DisableFormat) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1218,7 +1218,8 @@
   case tok::kw_public:
   case tok::kw_protected:
   case tok::kw_private:
-if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
+if (Style.Language == FormatStyle::LK_C ||
+Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   nextToken();
 else
@@ -1450,7 +1451,7 @@
   if (!parseEnum())
 break;
   // This only applies for C++.
-  if (!Style.isCpp()) {
+  if (!Style.isCpp() && !Style.isC()) {
 addUnwrappedLine();
 return;
   }
@@ -1493,7 +1494,8 @@
   parseParens();
   // Break the unwrapped line if a K&R C function definition has a parameter
   // declaration.
-  if (!IsTopLevel || !Style.isCpp() || !Previous || FormatTok->is(tok::eof))
+  if (!IsTopLevel || (!Style.isCpp() && !Style.isC()) || !Previous ||
+  FormatTok->is(tok::eof))
 break;
   if (isC78ParameterDecl(FormatTok, Tokens->peekNextToken(), Previous)) {
 addUnwrappedLine();
@@ -1601,7 +1603,7 @@
 break;
   }
 
-  if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+  if ((Style.isCpp() || Style.isC()) && FormatTok->is(TT_StatementMacro)) {
 parseStatementMacro();
 return;
   }
@@ -2647,7 +2649,7 @@
   nextToken();
   // If there are two identifiers in a row, this is likely an elaborate
   // return type. In Java, this can be "implements", etc.
-  if (Style.isCpp() && FormatTok->is(tok::identifier))
+  if ((Style.isCpp() || Style.isC()) && FormatTok->is(tok::identifier))
 return false;
 }
   }
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -97,7 +97,8 @@
   /// For example, 'public:' labels in classes are offset by 1 or 2
   /// characters to the left from their level.
   int getIndentOffset(const FormatToken &RootToken) {
-if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
+if (Style.Language == FormatStyle::LK_C ||
+Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   return 0;
 if (RootToken.isAccessSpecifier(false) ||
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -799,7 +799,8 @@
   if (Previous->is(TT_JsTypeOptionalQuestion))
 Previous = Previous->getPreviousNonComment();
   if ((CurrentToken->is(tok::colon) &&
-   (!Contexts.back().ColonIsDictLiteral || !Style.isCpp())) ||
+   (!Contexts.back().ColonIsDictLiteral ||
+(!Style.isCpp() && !Style.isC( ||
   Style.Language == For

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito added a comment.

These are just initial changes, there is still a lot of work and test cases to 
write. I figured I would put this out there to see if this is the direction we 
were thinking.




Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2504
 nextToken();
-  addUnwrappedLine();
+addUnwrappedLine();
+return;

MyDeveloperDay wrote:
> 
> I get what your trying to do but what happens in this scenario
> 
> `private::mynamespace::g_value -1;`
> 
> 
Yea, it doesnt parse correctly and is split to a new line after private:. I am 
going to revert all the changes in this code review and start looking at 
implementing C as its own language.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

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

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito added a comment.

Ok, as I was starting to add a new language, the scope of changes just 
continued to grow.

If you think it is worthwhile, I think I can fix this edge case for 
accessSpecifiers by cleaning up my old approach and adding some tests. I dont 
like having to add a big set of operators to check against for handling the 
case where there is a typo and the colon is missing i.e.

  class foo { 
  private
bool jim;
  public:
bool bob;
  };

I think this is probably the most common error so I think we should support it.

I don't think the delete issue in 
https://github.com/llvm/llvm-project/issues/46915 is worth the added 
complexity. Without specifying the language, it is too hard to interpret the 
programmer's intention. For example, these are totally valid as either a delete 
or a function call:

  delete(foo) // foo is a pointer being deleted 
  delete(bar) // bar is a parameter to a function




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

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

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito updated this revision to Diff 402142.
psigillito added a comment.

- revert to corner case handling with private


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

Files:
  .arclint
  clang/include/clang/Format/Format.h
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/FormatTokenLexer.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/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3030,6 +3030,46 @@
"label:\n"
"  signals.baz();\n"
"}");
+  verifyFormat("private[1];");
+  verifyFormat("testArray[public] = 1;");
+  verifyFormat("public();");
+  verifyFormat("myFunc(public);");
+  verifyFormat("std::vector testVec = {private};");
+  verifyFormat("private.p = 1;");
+  verifyFormat("void function(private...){};");
+  verifyFormat("if (private && public)\n");
+  verifyFormat("private &= true;");
+  verifyFormat("int x = private * public;");
+  verifyFormat("public *= private;");
+  verifyFormat("int x = public + private;");
+  verifyFormat("private++;");
+  verifyFormat("++private;");
+  verifyFormat("public += private;");
+  verifyFormat("public = public - private;");
+  verifyFormat("public->foo();");
+  verifyFormat("private--;");
+  verifyFormat("--private;");
+  verifyFormat("public -= 1;");
+  verifyFormat("if (!private && !public)\n");
+  verifyFormat("public != private;");
+  verifyFormat("int x = public / private;");
+  verifyFormat("public /= 2;");
+  verifyFormat("public = public % 2;");
+  verifyFormat("public %= 2;");
+  verifyFormat("if (public < private)\n");
+  verifyFormat("public << private;");
+  verifyFormat("public <<= private;");
+  verifyFormat("if (public > private)\n");
+  verifyFormat("public >> private;");
+  verifyFormat("public >>= private;");
+  verifyFormat("public ^ private;");
+  verifyFormat("public ^= private;");
+  verifyFormat("public | private;");
+  verifyFormat("public |= private;");
+  verifyFormat("auto x = private ? 1 : 2;");
+  verifyFormat("if (public == private)\n");
+  verifyFormat("void foo(public, private)");
+  verifyFormat("public::foo();");
 }
 
 TEST_F(FormatTest, SeparatesLogicalBlocks) {
Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -121,6 +121,12 @@
 Verbose("verbose", cl::desc("If set, shows the list of processed files"),
 cl::cat(ClangFormatCategory));
 
+static cl::opt
+SetLanguage("set-language",
+cl::desc("Manually sets the programming language clang-format "
+ "interprets files as"),
+cl::init(""), cl::cat(ClangFormatCategory));
+
 // Use --dry-run to match other LLVM tools when you mean do it but don't
 // actually do it
 static cl::opt
@@ -458,6 +464,12 @@
   Replacements Replaces = sortIncludes(*FormatStyle, Code->getBuffer(), Ranges,
AssumedFileName, &CursorPosition);
 
+  if (SetLanguage.getNumOccurrences() != 0) {
+
+if (SetLanguage.getValue() == "C" || SetLanguage.getValue() == "c") {
+  FormatStyle->Language = FormatStyle::LK_C;
+}
+  }
   // To format JSON insert a variable to trick the code into thinking its
   // JavaScript.
   if (FormatStyle->isJson() && !FormatStyle->DisableFormat) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1218,7 +1218,8 @@
   case tok::kw_public:
   case tok::kw_protected:
   case tok::kw_private:
-if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
+if (Style.Language == FormatStyle::LK_C ||
+Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   nextToken();
 else
@@ -1450,7 +1451,7 @@
   if (!parseEnum())
 break;
   // This only applies for C++.
-  if (!Style.isCpp()) {
+  if (!Style.isCpp() && !Style.isC()) {
 addUnwrappedLine();
 return;
   }
@@ -1493,7 +1494,8 @@
   parseParens();
   // Break the unwrapped line if a K&R C function definition has a parameter
   // declaration.
-  if (!IsTopLevel || !Style.isCpp() || !Previous || FormatTok->is(tok::eof))
+  if (!IsTopLevel || (!Style.isCpp() && !Style.isC()) || !Previous ||
+  FormatTok->is

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito updated this revision to Diff 402149.
psigillito added a comment.

- revert bad changes and arclint


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3030,6 +3030,46 @@
"label:\n"
"  signals.baz();\n"
"}");
+  verifyFormat("private[1];");
+  verifyFormat("testArray[public] = 1;");
+  verifyFormat("public();");
+  verifyFormat("myFunc(public);");
+  verifyFormat("std::vector testVec = {private};");
+  verifyFormat("private.p = 1;");
+  verifyFormat("void function(private...){};");
+  verifyFormat("if (private && public)\n");
+  verifyFormat("private &= true;");
+  verifyFormat("int x = private * public;");
+  verifyFormat("public *= private;");
+  verifyFormat("int x = public + private;");
+  verifyFormat("private++;");
+  verifyFormat("++private;");
+  verifyFormat("public += private;");
+  verifyFormat("public = public - private;");
+  verifyFormat("public->foo();");
+  verifyFormat("private--;");
+  verifyFormat("--private;");
+  verifyFormat("public -= 1;");
+  verifyFormat("if (!private && !public)\n");
+  verifyFormat("public != private;");
+  verifyFormat("int x = public / private;");
+  verifyFormat("public /= 2;");
+  verifyFormat("public = public % 2;");
+  verifyFormat("public %= 2;");
+  verifyFormat("if (public < private)\n");
+  verifyFormat("public << private;");
+  verifyFormat("public <<= private;");
+  verifyFormat("if (public > private)\n");
+  verifyFormat("public >> private;");
+  verifyFormat("public >>= private;");
+  verifyFormat("public ^ private;");
+  verifyFormat("public ^= private;");
+  verifyFormat("public | private;");
+  verifyFormat("public |= private;");
+  verifyFormat("auto x = private ? 1 : 2;");
+  verifyFormat("if (public == private)\n");
+  verifyFormat("void foo(public, private)");
+  verifyFormat("public::foo();");
 }
 
 TEST_F(FormatTest, SeparatesLogicalBlocks) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2494,14 +2494,25 @@
 }
 
 void UnwrappedLineParser::parseAccessSpecifier() {
+  auto *accessSpecifierCandidate = FormatTok;
   nextToken();
   // Understand Qt's slots.
   if (FormatTok->isOneOf(Keywords.kw_slots, Keywords.kw_qslots))
 nextToken();
   // Otherwise, we don't know what it is, and we'd better keep the next token.
-  if (FormatTok->Tok.is(tok::colon))
+  if (FormatTok->Tok.is(tok::colon)) {
 nextToken();
-  addUnwrappedLine();
+addUnwrappedLine();
+// is not a variable name or namespacename
+  } else if (C_OperatorsFollowingVar.find(FormatTok->Tok.getKind()) ==
+ C_OperatorsFollowingVar.end() &&
+ !FormatTok->Tok.is(tok::coloncolon)) {
+addUnwrappedLine();
+  }
+  // consider the accessSpecifier to be a C identifier
+  else if (accessSpecifierCandidate) {
+accessSpecifierCandidate->Tok.setKind(tok::identifier);
+  }
 }
 
 void UnwrappedLineParser::parseConcept() {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -100,10 +100,31 @@
 if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   return 0;
-if (RootToken.isAccessSpecifier(false) ||
-RootToken.isObjCAccessSpecifier() ||
-(RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
- RootToken.Next && RootToken.Next->is(tok::colon))) {
+
+auto isAccessModifier = [this, &RootToken]() {
+  bool returnValue{false};
+  if (RootToken.isAccessSpecifier(Style.isCpp())) {
+returnValue = true;
+  } else if (RootToken.isObjCAccessSpecifier()) {
+returnValue = true;
+  }
+  // Handle Qt signals
+  else if ((RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
+RootToken.Next && RootToken.Next->is(tok::colon))) {
+returnValue = true;
+  }
+  // Handle malformed access specifier i.e. 'private' without trailing ':'
+  else if ((RootToken.isAccessSpecifier(false) &&
+(!RootToken.Next ||
+ (C_OperatorsFollowingVar.find(RootToken.Next->Tok.getKind()) ==
+  clang::format::C_OperatorsFollowingVar.end() &&
+  !RootToken.Next->Tok.is(tok::coloncolon) {
+returnValue = true;

[PATCH] D118092: review revisions

[psigillito via Phabricator via cfe-commits]

psigillito created this revision.
psigillito requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118092

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp


Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2494,7 +2494,7 @@
 }
 
 void UnwrappedLineParser::parseAccessSpecifier() {
-  auto *accessSpecifierCandidate = FormatTok;
+  auto *AccessSpecifierCandidate = FormatTok;
   nextToken();
   // Understand Qt's slots.
   if (FormatTok->isOneOf(Keywords.kw_slots, Keywords.kw_qslots))
@@ -2504,15 +2504,12 @@
 nextToken();
 addUnwrappedLine();
 // is not a variable name or namespacename
-  } else if (C_OperatorsFollowingVar.find(FormatTok->Tok.getKind()) ==
- C_OperatorsFollowingVar.end() &&
- !FormatTok->Tok.is(tok::coloncolon)) {
+  } else if (!C_OperatorsFollowingVar.count(FormatTok->Tok.getKind()) &&
+ !FormatTok->Tok.is(tok::coloncolon))
 addUnwrappedLine();
-  }
   // consider the accessSpecifier to be a C identifier
-  else if (accessSpecifierCandidate) {
-accessSpecifierCandidate->Tok.setKind(tok::identifier);
-  }
+  else if (AccessSpecifierCandidate)
+AccessSpecifierCandidate->Tok.setKind(tok::identifier);
 }
 
 void UnwrappedLineParser::parseConcept() {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -101,30 +101,26 @@
 Style.isCSharp())
   return 0;
 
-auto isAccessModifier = [this, &RootToken]() {
-  bool returnValue{false};
-  if (RootToken.isAccessSpecifier(Style.isCpp())) {
-returnValue = true;
-  } else if (RootToken.isObjCAccessSpecifier()) {
-returnValue = true;
-  }
+auto IsAccessModifier = [this, &RootToken]() {
+  if (RootToken.isAccessSpecifier(Style.isCpp()))
+return true;
+  else if (RootToken.isObjCAccessSpecifier())
+return true;
   // Handle Qt signals
   else if ((RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
-RootToken.Next && RootToken.Next->is(tok::colon))) {
-returnValue = true;
-  }
+RootToken.Next && RootToken.Next->is(tok::colon)))
+return true;
   // Handle malformed access specifier i.e. 'private' without trailing ':'
   else if ((RootToken.isAccessSpecifier(false) &&
 (!RootToken.Next ||
- (C_OperatorsFollowingVar.find(RootToken.Next->Tok.getKind()) 
==
-  clang::format::C_OperatorsFollowingVar.end() &&
-  !RootToken.Next->Tok.is(tok::coloncolon) {
-returnValue = true;
-  }
-  return returnValue;
+ (!C_OperatorsFollowingVar.count(
+  RootToken.Next->Tok.getKind()) &&
+  !RootToken.Next->Tok.is(tok::coloncolon)
+return true;
+  return false;
 };
 
-if (isAccessModifier()) {
+if (IsAccessModifier()) {
   // The AccessModifierOffset may be overridden by IndentAccessModifiers,
   // in which case we take a negative value of the IndentWidth to simulate
   // the upper indent level.


Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2494,7 +2494,7 @@
 }
 
 void UnwrappedLineParser::parseAccessSpecifier() {
-  auto *accessSpecifierCandidate = FormatTok;
+  auto *AccessSpecifierCandidate = FormatTok;
   nextToken();
   // Understand Qt's slots.
   if (FormatTok->isOneOf(Keywords.kw_slots, Keywords.kw_qslots))
@@ -2504,15 +2504,12 @@
 nextToken();
 addUnwrappedLine();
 // is not a variable name or namespacename
-  } else if (C_OperatorsFollowingVar.find(FormatTok->Tok.getKind()) ==
- C_OperatorsFollowingVar.end() &&
- !FormatTok->Tok.is(tok::coloncolon)) {
+  } else if (!C_OperatorsFollowingVar.count(FormatTok->Tok.getKind()) &&
+ !FormatTok->Tok.is(tok::coloncolon))
 addUnwrappedLine();
-  }
   // consider the accessSpecifier to be a C identifier
-  else if (accessSpecifierCandidate) {
-accessSpecifierCandidate->Tok.setKind(tok::identifier);
-  }
+  else if (AccessSpecifierCandidate)
+AccessSpecifierCandidate->Tok.setKind(tok::identifier);
 }
 
 void UnwrappedLineParser::parseConcept() {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/Un

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito updated this revision to Diff 402733.
psigillito marked 6 inline comments as done.
psigillito added a comment.

Review Changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

Files:
  .arclint
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp

Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2494,7 +2494,7 @@
 }
 
 void UnwrappedLineParser::parseAccessSpecifier() {
-  auto *accessSpecifierCandidate = FormatTok;
+  auto *AccessSpecifierCandidate = FormatTok;
   nextToken();
   // Understand Qt's slots.
   if (FormatTok->isOneOf(Keywords.kw_slots, Keywords.kw_qslots))
@@ -2504,15 +2504,12 @@
 nextToken();
 addUnwrappedLine();
 // is not a variable name or namespacename
-  } else if (C_OperatorsFollowingVar.find(FormatTok->Tok.getKind()) ==
- C_OperatorsFollowingVar.end() &&
- !FormatTok->Tok.is(tok::coloncolon)) {
+  } else if (!C_OperatorsFollowingVar.count(FormatTok->Tok.getKind()) &&
+ !FormatTok->Tok.is(tok::coloncolon))
 addUnwrappedLine();
-  }
   // consider the accessSpecifier to be a C identifier
-  else if (accessSpecifierCandidate) {
-accessSpecifierCandidate->Tok.setKind(tok::identifier);
-  }
+  else if (AccessSpecifierCandidate)
+AccessSpecifierCandidate->Tok.setKind(tok::identifier);
 }
 
 void UnwrappedLineParser::parseConcept() {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -101,30 +101,26 @@
 Style.isCSharp())
   return 0;
 
-auto isAccessModifier = [this, &RootToken]() {
-  bool returnValue{false};
-  if (RootToken.isAccessSpecifier(Style.isCpp())) {
-returnValue = true;
-  } else if (RootToken.isObjCAccessSpecifier()) {
-returnValue = true;
-  }
+auto IsAccessModifier = [this, &RootToken]() {
+  if (RootToken.isAccessSpecifier(Style.isCpp()))
+return true;
+  else if (RootToken.isObjCAccessSpecifier())
+return true;
   // Handle Qt signals
   else if ((RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
-RootToken.Next && RootToken.Next->is(tok::colon))) {
-returnValue = true;
-  }
+RootToken.Next && RootToken.Next->is(tok::colon)))
+return true;
   // Handle malformed access specifier i.e. 'private' without trailing ':'
   else if ((RootToken.isAccessSpecifier(false) &&
 (!RootToken.Next ||
- (C_OperatorsFollowingVar.find(RootToken.Next->Tok.getKind()) ==
-  clang::format::C_OperatorsFollowingVar.end() &&
-  !RootToken.Next->Tok.is(tok::coloncolon) {
-returnValue = true;
-  }
-  return returnValue;
+ (!C_OperatorsFollowingVar.count(
+  RootToken.Next->Tok.getKind()) &&
+  !RootToken.Next->Tok.is(tok::coloncolon)
+return true;
+  return false;
 };
 
-if (isAccessModifier()) {
+if (IsAccessModifier()) {
   // The AccessModifierOffset may be overridden by IndentAccessModifiers,
   // in which case we take a negative value of the IndentWidth to simulate
   // the upper indent level.
Index: .arclint
===
--- .arclint
+++ /dev/null
@@ -1,15 +0,0 @@
-{
-  "linters": {
-"clang-format": {
-  "type": "script-and-regex",
-  "script-and-regex.script": "bash utils/arcanist/clang-format.sh",
-  "script-and-regex.regex": "/^(?P[[:alpha:]]+)\n(?P[^\n]+)\n(|(?P\\d),(?P\\d)\n(?P.*)\n(?P.*)\n)$/s",
-  "include": [
-"(\\.(cc|cpp|h)$)"
-  ],
-  "exclude": [
-"(^clang/test/)"
-  ]
-}
-  }
-}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito updated this revision to Diff 402735.
psigillito added a comment.

undo arclint change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

Files:
  .arclint


Index: .arclint
===
--- /dev/null
+++ .arclint
@@ -0,0 +1,15 @@
+{
+  "linters": {
+"clang-format": {
+  "type": "script-and-regex",
+  "script-and-regex.script": "bash utils/arcanist/clang-format.sh",
+  "script-and-regex.regex": 
"/^(?P[[:alpha:]]+)\n(?P[^\n]+)\n(|(?P\\d),(?P\\d)\n(?P.*)\n(?P.*)\n)$/s",
+  "include": [
+"(\\.(cc|cpp|h)$)"
+  ],
+  "exclude": [
+"(^clang/test/)"
+  ]
+}
+  }
+}


Index: .arclint
===
--- /dev/null
+++ .arclint
@@ -0,0 +1,15 @@
+{
+  "linters": {
+"clang-format": {
+  "type": "script-and-regex",
+  "script-and-regex.script": "bash utils/arcanist/clang-format.sh",
+  "script-and-regex.regex": "/^(?P[[:alpha:]]+)\n(?P[^\n]+)\n(|(?P\\d),(?P\\d)\n(?P.*)\n(?P.*)\n)$/s",
+  "include": [
+"(\\.(cc|cpp|h)$)"
+  ],
+  "exclude": [
+"(^clang/test/)"
+  ]
+}
+  }
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito updated this revision to Diff 402736.
psigillito added a comment.

Base diff on upstream


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

Files:
  .arclint
  clang/lib/Format/FormatToken.h
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3030,6 +3030,46 @@
"label:\n"
"  signals.baz();\n"
"}");
+  verifyFormat("private[1];");
+  verifyFormat("testArray[public] = 1;");
+  verifyFormat("public();");
+  verifyFormat("myFunc(public);");
+  verifyFormat("std::vector testVec = {private};");
+  verifyFormat("private.p = 1;");
+  verifyFormat("void function(private...){};");
+  verifyFormat("if (private && public)\n");
+  verifyFormat("private &= true;");
+  verifyFormat("int x = private * public;");
+  verifyFormat("public *= private;");
+  verifyFormat("int x = public + private;");
+  verifyFormat("private++;");
+  verifyFormat("++private;");
+  verifyFormat("public += private;");
+  verifyFormat("public = public - private;");
+  verifyFormat("public->foo();");
+  verifyFormat("private--;");
+  verifyFormat("--private;");
+  verifyFormat("public -= 1;");
+  verifyFormat("if (!private && !public)\n");
+  verifyFormat("public != private;");
+  verifyFormat("int x = public / private;");
+  verifyFormat("public /= 2;");
+  verifyFormat("public = public % 2;");
+  verifyFormat("public %= 2;");
+  verifyFormat("if (public < private)\n");
+  verifyFormat("public << private;");
+  verifyFormat("public <<= private;");
+  verifyFormat("if (public > private)\n");
+  verifyFormat("public >> private;");
+  verifyFormat("public >>= private;");
+  verifyFormat("public ^ private;");
+  verifyFormat("public ^= private;");
+  verifyFormat("public | private;");
+  verifyFormat("public |= private;");
+  verifyFormat("auto x = private ? 1 : 2;");
+  verifyFormat("if (public == private)\n");
+  verifyFormat("void foo(public, private)");
+  verifyFormat("public::foo();");
 }
 
 TEST_F(FormatTest, SeparatesLogicalBlocks) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2494,14 +2494,22 @@
 }
 
 void UnwrappedLineParser::parseAccessSpecifier() {
+  auto *AccessSpecifierCandidate = FormatTok;
   nextToken();
   // Understand Qt's slots.
   if (FormatTok->isOneOf(Keywords.kw_slots, Keywords.kw_qslots))
 nextToken();
   // Otherwise, we don't know what it is, and we'd better keep the next token.
-  if (FormatTok->Tok.is(tok::colon))
+  if (FormatTok->Tok.is(tok::colon)) {
 nextToken();
-  addUnwrappedLine();
+addUnwrappedLine();
+// is not a variable name or namespacename
+  } else if (!C_OperatorsFollowingVar.count(FormatTok->Tok.getKind()) &&
+ !FormatTok->Tok.is(tok::coloncolon))
+addUnwrappedLine();
+  // consider the accessSpecifier to be a C identifier
+  else if (AccessSpecifierCandidate)
+AccessSpecifierCandidate->Tok.setKind(tok::identifier);
 }
 
 void UnwrappedLineParser::parseConcept() {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -100,10 +100,27 @@
 if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   return 0;
-if (RootToken.isAccessSpecifier(false) ||
-RootToken.isObjCAccessSpecifier() ||
-(RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
- RootToken.Next && RootToken.Next->is(tok::colon))) {
+
+auto IsAccessModifier = [this, &RootToken]() {
+  if (RootToken.isAccessSpecifier(Style.isCpp()))
+return true;
+  else if (RootToken.isObjCAccessSpecifier())
+return true;
+  // Handle Qt signals
+  else if ((RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
+RootToken.Next && RootToken.Next->is(tok::colon)))
+return true;
+  // Handle malformed access specifier i.e. 'private' without trailing ':'
+  else if ((RootToken.isAccessSpecifier(false) &&
+(!RootToken.Next ||
+ (!C_OperatorsFollowingVar.count(
+  RootToken.Next->Tok.getKind()) &&
+  !RootToken.Next->Tok.is(tok::coloncolon)
+return true;
+  return false;
+};
+
+if (IsAccessModifier()) {
   // The AccessModifierOffset may be overridden by IndentAccessModifiers,
   // in which case we take a negative value of th

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito updated this revision to Diff 402737.
psigillito added a comment.

- revert bad arclint changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

Files:
  .arclint
  clang/lib/Format/FormatToken.h
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3030,6 +3030,46 @@
"label:\n"
"  signals.baz();\n"
"}");
+  verifyFormat("private[1];");
+  verifyFormat("testArray[public] = 1;");
+  verifyFormat("public();");
+  verifyFormat("myFunc(public);");
+  verifyFormat("std::vector testVec = {private};");
+  verifyFormat("private.p = 1;");
+  verifyFormat("void function(private...){};");
+  verifyFormat("if (private && public)\n");
+  verifyFormat("private &= true;");
+  verifyFormat("int x = private * public;");
+  verifyFormat("public *= private;");
+  verifyFormat("int x = public + private;");
+  verifyFormat("private++;");
+  verifyFormat("++private;");
+  verifyFormat("public += private;");
+  verifyFormat("public = public - private;");
+  verifyFormat("public->foo();");
+  verifyFormat("private--;");
+  verifyFormat("--private;");
+  verifyFormat("public -= 1;");
+  verifyFormat("if (!private && !public)\n");
+  verifyFormat("public != private;");
+  verifyFormat("int x = public / private;");
+  verifyFormat("public /= 2;");
+  verifyFormat("public = public % 2;");
+  verifyFormat("public %= 2;");
+  verifyFormat("if (public < private)\n");
+  verifyFormat("public << private;");
+  verifyFormat("public <<= private;");
+  verifyFormat("if (public > private)\n");
+  verifyFormat("public >> private;");
+  verifyFormat("public >>= private;");
+  verifyFormat("public ^ private;");
+  verifyFormat("public ^= private;");
+  verifyFormat("public | private;");
+  verifyFormat("public |= private;");
+  verifyFormat("auto x = private ? 1 : 2;");
+  verifyFormat("if (public == private)\n");
+  verifyFormat("void foo(public, private)");
+  verifyFormat("public::foo();");
 }
 
 TEST_F(FormatTest, SeparatesLogicalBlocks) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2494,14 +2494,22 @@
 }
 
 void UnwrappedLineParser::parseAccessSpecifier() {
+  auto *AccessSpecifierCandidate = FormatTok;
   nextToken();
   // Understand Qt's slots.
   if (FormatTok->isOneOf(Keywords.kw_slots, Keywords.kw_qslots))
 nextToken();
   // Otherwise, we don't know what it is, and we'd better keep the next token.
-  if (FormatTok->Tok.is(tok::colon))
+  if (FormatTok->Tok.is(tok::colon)) {
 nextToken();
-  addUnwrappedLine();
+addUnwrappedLine();
+// is not a variable name or namespacename
+  } else if (!C_OperatorsFollowingVar.count(FormatTok->Tok.getKind()) &&
+ !FormatTok->Tok.is(tok::coloncolon))
+addUnwrappedLine();
+  // consider the accessSpecifier to be a C identifier
+  else if (AccessSpecifierCandidate)
+AccessSpecifierCandidate->Tok.setKind(tok::identifier);
 }
 
 void UnwrappedLineParser::parseConcept() {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -100,10 +100,27 @@
 if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   return 0;
-if (RootToken.isAccessSpecifier(false) ||
-RootToken.isObjCAccessSpecifier() ||
-(RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
- RootToken.Next && RootToken.Next->is(tok::colon))) {
+
+auto IsAccessModifier = [this, &RootToken]() {
+  if (RootToken.isAccessSpecifier(Style.isCpp()))
+return true;
+  else if (RootToken.isObjCAccessSpecifier())
+return true;
+  // Handle Qt signals
+  else if ((RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
+RootToken.Next && RootToken.Next->is(tok::colon)))
+return true;
+  // Handle malformed access specifier i.e. 'private' without trailing ':'
+  else if ((RootToken.isAccessSpecifier(false) &&
+(!RootToken.Next ||
+ (!C_OperatorsFollowingVar.count(
+  RootToken.Next->Tok.getKind()) &&
+  !RootToken.Next->Tok.is(tok::coloncolon)
+return true;
+  return false;
+};
+
+if (IsAccessModifier()) {
   // The AccessModifierOffset may be overridden by IndentAccessModifiers,
   // in which case we take a negative valu

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito updated this revision to Diff 402742.
psigillito added a comment.

- revert of .arclint file delete


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3030,6 +3030,46 @@
"label:\n"
"  signals.baz();\n"
"}");
+  verifyFormat("private[1];");
+  verifyFormat("testArray[public] = 1;");
+  verifyFormat("public();");
+  verifyFormat("myFunc(public);");
+  verifyFormat("std::vector testVec = {private};");
+  verifyFormat("private.p = 1;");
+  verifyFormat("void function(private...){};");
+  verifyFormat("if (private && public)\n");
+  verifyFormat("private &= true;");
+  verifyFormat("int x = private * public;");
+  verifyFormat("public *= private;");
+  verifyFormat("int x = public + private;");
+  verifyFormat("private++;");
+  verifyFormat("++private;");
+  verifyFormat("public += private;");
+  verifyFormat("public = public - private;");
+  verifyFormat("public->foo();");
+  verifyFormat("private--;");
+  verifyFormat("--private;");
+  verifyFormat("public -= 1;");
+  verifyFormat("if (!private && !public)\n");
+  verifyFormat("public != private;");
+  verifyFormat("int x = public / private;");
+  verifyFormat("public /= 2;");
+  verifyFormat("public = public % 2;");
+  verifyFormat("public %= 2;");
+  verifyFormat("if (public < private)\n");
+  verifyFormat("public << private;");
+  verifyFormat("public <<= private;");
+  verifyFormat("if (public > private)\n");
+  verifyFormat("public >> private;");
+  verifyFormat("public >>= private;");
+  verifyFormat("public ^ private;");
+  verifyFormat("public ^= private;");
+  verifyFormat("public | private;");
+  verifyFormat("public |= private;");
+  verifyFormat("auto x = private ? 1 : 2;");
+  verifyFormat("if (public == private)\n");
+  verifyFormat("void foo(public, private)");
+  verifyFormat("public::foo();");
 }
 
 TEST_F(FormatTest, SeparatesLogicalBlocks) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2494,14 +2494,22 @@
 }
 
 void UnwrappedLineParser::parseAccessSpecifier() {
+  auto *AccessSpecifierCandidate = FormatTok;
   nextToken();
   // Understand Qt's slots.
   if (FormatTok->isOneOf(Keywords.kw_slots, Keywords.kw_qslots))
 nextToken();
   // Otherwise, we don't know what it is, and we'd better keep the next token.
-  if (FormatTok->Tok.is(tok::colon))
+  if (FormatTok->Tok.is(tok::colon)) {
 nextToken();
-  addUnwrappedLine();
+addUnwrappedLine();
+// is not a variable name or namespacename
+  } else if (!C_OperatorsFollowingVar.count(FormatTok->Tok.getKind()) &&
+ !FormatTok->Tok.is(tok::coloncolon))
+addUnwrappedLine();
+  // consider the accessSpecifier to be a C identifier
+  else if (AccessSpecifierCandidate)
+AccessSpecifierCandidate->Tok.setKind(tok::identifier);
 }
 
 void UnwrappedLineParser::parseConcept() {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -100,10 +100,27 @@
 if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   return 0;
-if (RootToken.isAccessSpecifier(false) ||
-RootToken.isObjCAccessSpecifier() ||
-(RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
- RootToken.Next && RootToken.Next->is(tok::colon))) {
+
+auto IsAccessModifier = [this, &RootToken]() {
+  if (RootToken.isAccessSpecifier(Style.isCpp()))
+return true;
+  else if (RootToken.isObjCAccessSpecifier())
+return true;
+  // Handle Qt signals
+  else if ((RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
+RootToken.Next && RootToken.Next->is(tok::colon)))
+return true;
+  // Handle malformed access specifier i.e. 'private' without trailing ':'
+  else if ((RootToken.isAccessSpecifier(false) &&
+(!RootToken.Next ||
+ (!C_OperatorsFollowingVar.count(
+  RootToken.Next->Tok.getKind()) &&
+  !RootToken.Next->Tok.is(tok::coloncolon)
+return true;
+  return false;
+};
+
+if (IsAccessModifier()) {
   // The AccessModifierOffset may be overridden by IndentAccessModifiers,
   // in which case we take a negative value of th

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/FormatToken.h:125
+/// Operators that can follow a C variable.
+static const std::set C_OperatorsFollowingVar = {
+tok::l_square, tok::r_square,

curdeius wrote:
> HazardyKnusperkeks wrote:
> > And maybe choose a different container: 
> > https://llvm.org/docs/ProgrammersManual.html#set-like-containers-std-set-smallset-setvector-etc
> Not done it seems.
> Please rename and use a different type. Maybe `ImmutableSet`? Or just a 
> sorted `std::vector`?
ok updated to sorted std::vector



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:104
+
+auto isAccessModifier = [this, &RootToken]() {
+  bool returnValue{false};

HazardyKnusperkeks wrote:
> In clang-format all lambdas I've seen start with a capital letter.
Updated. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

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

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito updated this revision to Diff 403861.
psigillito marked 8 inline comments as done.
psigillito added a comment.

- Merge branch 'main' of https://github.com/llvm/llvm-project
- code review syntax cleanup, sorted vector


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3123,6 +3123,46 @@
"label:\n"
"  signals.baz();\n"
"}");
+  verifyFormat("private[1];");
+  verifyFormat("testArray[public] = 1;");
+  verifyFormat("public();");
+  verifyFormat("myFunc(public);");
+  verifyFormat("std::vector testVec = {private};");
+  verifyFormat("private.p = 1;");
+  verifyFormat("void function(private...){};");
+  verifyFormat("if (private && public)\n");
+  verifyFormat("private &= true;");
+  verifyFormat("int x = private * public;");
+  verifyFormat("public *= private;");
+  verifyFormat("int x = public + private;");
+  verifyFormat("private++;");
+  verifyFormat("++private;");
+  verifyFormat("public += private;");
+  verifyFormat("public = public - private;");
+  verifyFormat("public->foo();");
+  verifyFormat("private--;");
+  verifyFormat("--private;");
+  verifyFormat("public -= 1;");
+  verifyFormat("if (!private && !public)\n");
+  verifyFormat("public != private;");
+  verifyFormat("int x = public / private;");
+  verifyFormat("public /= 2;");
+  verifyFormat("public = public % 2;");
+  verifyFormat("public %= 2;");
+  verifyFormat("if (public < private)\n");
+  verifyFormat("public << private;");
+  verifyFormat("public <<= private;");
+  verifyFormat("if (public > private)\n");
+  verifyFormat("public >> private;");
+  verifyFormat("public >>= private;");
+  verifyFormat("public ^ private;");
+  verifyFormat("public ^= private;");
+  verifyFormat("public | private;");
+  verifyFormat("public |= private;");
+  verifyFormat("auto x = private ? 1 : 2;");
+  verifyFormat("if (public == private)\n");
+  verifyFormat("void foo(public, private)");
+  verifyFormat("public::foo();");
 }
 
 TEST_F(FormatTest, SeparatesLogicalBlocks) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2707,14 +2707,28 @@
 }
 
 void UnwrappedLineParser::parseAccessSpecifier() {
+  FormatToken *AccessSpecifierCandidate = FormatTok;
   nextToken();
   // Understand Qt's slots.
   if (FormatTok->isOneOf(Keywords.kw_slots, Keywords.kw_qslots))
 nextToken();
+
+  auto COperatorMatch =
+  std::lower_bound(COperatorsFollowingVar.begin(),
+   COperatorsFollowingVar.end(), FormatTok->Tok.getKind());
   // Otherwise, we don't know what it is, and we'd better keep the next token.
-  if (FormatTok->Tok.is(tok::colon))
+  if (FormatTok->Tok.is(tok::colon)) {
 nextToken();
-  addUnwrappedLine();
+addUnwrappedLine();
+  } else if ((COperatorMatch == COperatorsFollowingVar.end() ||
+  *COperatorMatch != FormatTok->Tok.getKind()) &&
+ !FormatTok->Tok.is(tok::coloncolon)) {
+// Not a variable name nor namespace name.
+addUnwrappedLine();
+  } else if (AccessSpecifierCandidate) {
+// Consider the access specifier to be a C identifier.
+AccessSpecifierCandidate->Tok.setKind(tok::identifier);
+  }
 }
 
 void UnwrappedLineParser::parseConcept() {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -100,10 +100,39 @@
 if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   return 0;
-if (RootToken.isAccessSpecifier(false) ||
-RootToken.isObjCAccessSpecifier() ||
-(RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
- RootToken.Next && RootToken.Next->is(tok::colon))) {
+
+auto IsAccessModifier = [this, &RootToken]() {
+  if (RootToken.isAccessSpecifier(Style.isCpp()))
+return true;
+  else if (RootToken.isObjCAccessSpecifier())
+return true;
+  // Handle Qt signals.
+  else if ((RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
+RootToken.Next && RootToken.Next->is(tok::colon)))
+return true;
+  else if (RootToken.Next &&
+   RootToken.Next->isOneOf(Keywords.kw_slots, Keywords.kw_qslots) &&
+   RootToken.Next->Next && RootToken.Next->Next->is(tok::colon))
+   

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito updated this revision to Diff 403862.
psigillito marked 2 inline comments as done.
psigillito added a comment.

- revert auto formatting comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3123,6 +3123,46 @@
"label:\n"
"  signals.baz();\n"
"}");
+  verifyFormat("private[1];");
+  verifyFormat("testArray[public] = 1;");
+  verifyFormat("public();");
+  verifyFormat("myFunc(public);");
+  verifyFormat("std::vector testVec = {private};");
+  verifyFormat("private.p = 1;");
+  verifyFormat("void function(private...){};");
+  verifyFormat("if (private && public)\n");
+  verifyFormat("private &= true;");
+  verifyFormat("int x = private * public;");
+  verifyFormat("public *= private;");
+  verifyFormat("int x = public + private;");
+  verifyFormat("private++;");
+  verifyFormat("++private;");
+  verifyFormat("public += private;");
+  verifyFormat("public = public - private;");
+  verifyFormat("public->foo();");
+  verifyFormat("private--;");
+  verifyFormat("--private;");
+  verifyFormat("public -= 1;");
+  verifyFormat("if (!private && !public)\n");
+  verifyFormat("public != private;");
+  verifyFormat("int x = public / private;");
+  verifyFormat("public /= 2;");
+  verifyFormat("public = public % 2;");
+  verifyFormat("public %= 2;");
+  verifyFormat("if (public < private)\n");
+  verifyFormat("public << private;");
+  verifyFormat("public <<= private;");
+  verifyFormat("if (public > private)\n");
+  verifyFormat("public >> private;");
+  verifyFormat("public >>= private;");
+  verifyFormat("public ^ private;");
+  verifyFormat("public ^= private;");
+  verifyFormat("public | private;");
+  verifyFormat("public |= private;");
+  verifyFormat("auto x = private ? 1 : 2;");
+  verifyFormat("if (public == private)\n");
+  verifyFormat("void foo(public, private)");
+  verifyFormat("public::foo();");
 }
 
 TEST_F(FormatTest, SeparatesLogicalBlocks) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2707,14 +2707,28 @@
 }
 
 void UnwrappedLineParser::parseAccessSpecifier() {
+  FormatToken *AccessSpecifierCandidate = FormatTok;
   nextToken();
   // Understand Qt's slots.
   if (FormatTok->isOneOf(Keywords.kw_slots, Keywords.kw_qslots))
 nextToken();
+
+  auto COperatorMatch =
+  std::lower_bound(COperatorsFollowingVar.begin(),
+   COperatorsFollowingVar.end(), FormatTok->Tok.getKind());
   // Otherwise, we don't know what it is, and we'd better keep the next token.
-  if (FormatTok->Tok.is(tok::colon))
+  if (FormatTok->Tok.is(tok::colon)) {
 nextToken();
-  addUnwrappedLine();
+addUnwrappedLine();
+  } else if ((COperatorMatch == COperatorsFollowingVar.end() ||
+  *COperatorMatch != FormatTok->Tok.getKind()) &&
+ !FormatTok->Tok.is(tok::coloncolon)) {
+// Not a variable name nor namespace name.
+addUnwrappedLine();
+  } else if (AccessSpecifierCandidate) {
+// Consider the access specifier to be a C identifier.
+AccessSpecifierCandidate->Tok.setKind(tok::identifier);
+  }
 }
 
 void UnwrappedLineParser::parseConcept() {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -100,10 +100,38 @@
 if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   return 0;
-if (RootToken.isAccessSpecifier(false) ||
-RootToken.isObjCAccessSpecifier() ||
-(RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
- RootToken.Next && RootToken.Next->is(tok::colon))) {
+
+auto IsAccessModifier = [this, &RootToken]() {
+  if (RootToken.isAccessSpecifier(Style.isCpp()))
+return true;
+  else if (RootToken.isObjCAccessSpecifier())
+return true;
+  // Handle Qt signals.
+  else if ((RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
+RootToken.Next && RootToken.Next->is(tok::colon)))
+return true;
+  else if (RootToken.Next &&
+   RootToken.Next->isOneOf(Keywords.kw_slots, Keywords.kw_qslots) &&
+   RootToken.Next->Next && RootToken.Next->Next->is(tok::colon))
+return true;
+  // Handle malformed access specifier e.g. 'private'

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito marked 7 inline comments as done.
psigillito added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2716-2718
+  auto COperatorMatch =
+  std::lower_bound(COperatorsFollowingVar.begin(),
+   COperatorsFollowingVar.end(), FormatTok->Tok.getKind());

curdeius wrote:
> Please use `binary_search` and put it inside the `else` branch to avoid it if 
> the first condition is satisfied.
> Something like:
> ```
> if (FormatTok->Tok.is(tok::colon)) {
> ...
> } else if (!binary_search(...) {
> } else if (...) {
> }
> ```
> 
> Also, this code and the code in `UnwrappedLineFormatter` are pretty much 
> similar.
> Can we remove this duplication by e.g. setting the token kind here and 
> checking it in the formatter?
Yes, since we now set the kind to 'identifier' here, the binary_search check in 
the formatter is unnecessary. Updated the formatter to no longer do the check. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

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

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito updated this revision to Diff 404288.
psigillito marked an inline comment as done.
psigillito added a comment.

- review changes, lower_bound to binary_search


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3123,6 +3123,46 @@
"label:\n"
"  signals.baz();\n"
"}");
+  verifyFormat("private[1];");
+  verifyFormat("testArray[public] = 1;");
+  verifyFormat("public();");
+  verifyFormat("myFunc(public);");
+  verifyFormat("std::vector testVec = {private};");
+  verifyFormat("private.p = 1;");
+  verifyFormat("void function(private...){};");
+  verifyFormat("if (private && public)\n");
+  verifyFormat("private &= true;");
+  verifyFormat("int x = private * public;");
+  verifyFormat("public *= private;");
+  verifyFormat("int x = public + private;");
+  verifyFormat("private++;");
+  verifyFormat("++private;");
+  verifyFormat("public += private;");
+  verifyFormat("public = public - private;");
+  verifyFormat("public->foo();");
+  verifyFormat("private--;");
+  verifyFormat("--private;");
+  verifyFormat("public -= 1;");
+  verifyFormat("if (!private && !public)\n");
+  verifyFormat("public != private;");
+  verifyFormat("int x = public / private;");
+  verifyFormat("public /= 2;");
+  verifyFormat("public = public % 2;");
+  verifyFormat("public %= 2;");
+  verifyFormat("if (public < private)\n");
+  verifyFormat("public << private;");
+  verifyFormat("public <<= private;");
+  verifyFormat("if (public > private)\n");
+  verifyFormat("public >> private;");
+  verifyFormat("public >>= private;");
+  verifyFormat("public ^ private;");
+  verifyFormat("public ^= private;");
+  verifyFormat("public | private;");
+  verifyFormat("public |= private;");
+  verifyFormat("auto x = private ? 1 : 2;");
+  verifyFormat("if (public == private)\n");
+  verifyFormat("void foo(public, private)");
+  verifyFormat("public::foo();");
 }
 
 TEST_F(FormatTest, SeparatesLogicalBlocks) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2707,14 +2707,26 @@
 }
 
 void UnwrappedLineParser::parseAccessSpecifier() {
+  FormatToken *AccessSpecifierCandidate = FormatTok;
   nextToken();
   // Understand Qt's slots.
   if (FormatTok->isOneOf(Keywords.kw_slots, Keywords.kw_qslots))
 nextToken();
+
   // Otherwise, we don't know what it is, and we'd better keep the next token.
-  if (FormatTok->Tok.is(tok::colon))
+  if (FormatTok->Tok.is(tok::colon)) {
 nextToken();
-  addUnwrappedLine();
+addUnwrappedLine();
+  } else if (!FormatTok->Tok.is(tok::coloncolon) &&
+ !std::binary_search(COperatorsFollowingVar.begin(),
+ COperatorsFollowingVar.end(),
+ FormatTok->Tok.getKind())) {
+// Not a variable name nor namespace name.
+addUnwrappedLine();
+  } else if (AccessSpecifierCandidate) {
+// Consider the access specifier to be a C identifier.
+AccessSpecifierCandidate->Tok.setKind(tok::identifier);
+  }
 }
 
 void UnwrappedLineParser::parseConcept() {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -100,10 +100,27 @@
 if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   return 0;
-if (RootToken.isAccessSpecifier(false) ||
-RootToken.isObjCAccessSpecifier() ||
-(RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
- RootToken.Next && RootToken.Next->is(tok::colon))) {
+
+auto IsAccessModifier = [this, &RootToken]() {
+  if (RootToken.isAccessSpecifier(Style.isCpp()))
+return true;
+  else if (RootToken.isObjCAccessSpecifier())
+return true;
+  // Handle Qt signals.
+  else if ((RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
+RootToken.Next && RootToken.Next->is(tok::colon)))
+return true;
+  else if (RootToken.Next &&
+   RootToken.Next->isOneOf(Keywords.kw_slots, Keywords.kw_qslots) &&
+   RootToken.Next->Next && RootToken.Next->Next->is(tok::colon))
+return true;
+  // Handle malformed access specifier e.g. 'private' without trailing ':'.
+  else if (!RootToken.Next && RootToken.isAccessSpecifie

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito updated this revision to Diff 404322.
psigillito added a comment.

- wrap vector in function


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3123,6 +3123,46 @@
"label:\n"
"  signals.baz();\n"
"}");
+  verifyFormat("private[1];");
+  verifyFormat("testArray[public] = 1;");
+  verifyFormat("public();");
+  verifyFormat("myFunc(public);");
+  verifyFormat("std::vector testVec = {private};");
+  verifyFormat("private.p = 1;");
+  verifyFormat("void function(private...){};");
+  verifyFormat("if (private && public)\n");
+  verifyFormat("private &= true;");
+  verifyFormat("int x = private * public;");
+  verifyFormat("public *= private;");
+  verifyFormat("int x = public + private;");
+  verifyFormat("private++;");
+  verifyFormat("++private;");
+  verifyFormat("public += private;");
+  verifyFormat("public = public - private;");
+  verifyFormat("public->foo();");
+  verifyFormat("private--;");
+  verifyFormat("--private;");
+  verifyFormat("public -= 1;");
+  verifyFormat("if (!private && !public)\n");
+  verifyFormat("public != private;");
+  verifyFormat("int x = public / private;");
+  verifyFormat("public /= 2;");
+  verifyFormat("public = public % 2;");
+  verifyFormat("public %= 2;");
+  verifyFormat("if (public < private)\n");
+  verifyFormat("public << private;");
+  verifyFormat("public <<= private;");
+  verifyFormat("if (public > private)\n");
+  verifyFormat("public >> private;");
+  verifyFormat("public >>= private;");
+  verifyFormat("public ^ private;");
+  verifyFormat("public ^= private;");
+  verifyFormat("public | private;");
+  verifyFormat("public |= private;");
+  verifyFormat("auto x = private ? 1 : 2;");
+  verifyFormat("if (public == private)\n");
+  verifyFormat("void foo(public, private)");
+  verifyFormat("public::foo();");
 }
 
 TEST_F(FormatTest, SeparatesLogicalBlocks) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2707,14 +2707,26 @@
 }
 
 void UnwrappedLineParser::parseAccessSpecifier() {
+  FormatToken *AccessSpecifierCandidate = FormatTok;
   nextToken();
   // Understand Qt's slots.
   if (FormatTok->isOneOf(Keywords.kw_slots, Keywords.kw_qslots))
 nextToken();
+
   // Otherwise, we don't know what it is, and we'd better keep the next token.
-  if (FormatTok->Tok.is(tok::colon))
+  if (FormatTok->Tok.is(tok::colon)) {
 nextToken();
-  addUnwrappedLine();
+addUnwrappedLine();
+  } else if (!FormatTok->Tok.is(tok::coloncolon) &&
+ !std::binary_search(COperatorsFollowingVar.begin(),
+ COperatorsFollowingVar.end(),
+ FormatTok->Tok.getKind())) {
+// Not a variable name nor namespace name.
+addUnwrappedLine();
+  } else if (AccessSpecifierCandidate) {
+// Consider the access specifier to be a C identifier.
+AccessSpecifierCandidate->Tok.setKind(tok::identifier);
+  }
 }
 
 void UnwrappedLineParser::parseConcept() {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -100,10 +100,27 @@
 if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   return 0;
-if (RootToken.isAccessSpecifier(false) ||
-RootToken.isObjCAccessSpecifier() ||
-(RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
- RootToken.Next && RootToken.Next->is(tok::colon))) {
+
+auto IsAccessModifier = [this, &RootToken]() {
+  if (RootToken.isAccessSpecifier(Style.isCpp()))
+return true;
+  else if (RootToken.isObjCAccessSpecifier())
+return true;
+  // Handle Qt signals.
+  else if ((RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
+RootToken.Next && RootToken.Next->is(tok::colon)))
+return true;
+  else if (RootToken.Next &&
+   RootToken.Next->isOneOf(Keywords.kw_slots, Keywords.kw_qslots) &&
+   RootToken.Next->Next && RootToken.Next->Next->is(tok::colon))
+return true;
+  // Handle malformed access specifier e.g. 'private' without trailing ':'.
+  else if (!RootToken.Next && RootToken.isAccessSpecifier(false))
+return true;
+  return false;
+};
+
+  

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito updated this revision to Diff 404323.
psigillito marked 2 inline comments as done.
psigillito added a comment.

- unnecessary whitespace


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3123,6 +3123,46 @@
"label:\n"
"  signals.baz();\n"
"}");
+  verifyFormat("private[1];");
+  verifyFormat("testArray[public] = 1;");
+  verifyFormat("public();");
+  verifyFormat("myFunc(public);");
+  verifyFormat("std::vector testVec = {private};");
+  verifyFormat("private.p = 1;");
+  verifyFormat("void function(private...){};");
+  verifyFormat("if (private && public)\n");
+  verifyFormat("private &= true;");
+  verifyFormat("int x = private * public;");
+  verifyFormat("public *= private;");
+  verifyFormat("int x = public + private;");
+  verifyFormat("private++;");
+  verifyFormat("++private;");
+  verifyFormat("public += private;");
+  verifyFormat("public = public - private;");
+  verifyFormat("public->foo();");
+  verifyFormat("private--;");
+  verifyFormat("--private;");
+  verifyFormat("public -= 1;");
+  verifyFormat("if (!private && !public)\n");
+  verifyFormat("public != private;");
+  verifyFormat("int x = public / private;");
+  verifyFormat("public /= 2;");
+  verifyFormat("public = public % 2;");
+  verifyFormat("public %= 2;");
+  verifyFormat("if (public < private)\n");
+  verifyFormat("public << private;");
+  verifyFormat("public <<= private;");
+  verifyFormat("if (public > private)\n");
+  verifyFormat("public >> private;");
+  verifyFormat("public >>= private;");
+  verifyFormat("public ^ private;");
+  verifyFormat("public ^= private;");
+  verifyFormat("public | private;");
+  verifyFormat("public |= private;");
+  verifyFormat("auto x = private ? 1 : 2;");
+  verifyFormat("if (public == private)\n");
+  verifyFormat("void foo(public, private)");
+  verifyFormat("public::foo();");
 }
 
 TEST_F(FormatTest, SeparatesLogicalBlocks) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2707,14 +2707,25 @@
 }
 
 void UnwrappedLineParser::parseAccessSpecifier() {
+  FormatToken *AccessSpecifierCandidate = FormatTok;
   nextToken();
   // Understand Qt's slots.
   if (FormatTok->isOneOf(Keywords.kw_slots, Keywords.kw_qslots))
 nextToken();
   // Otherwise, we don't know what it is, and we'd better keep the next token.
-  if (FormatTok->Tok.is(tok::colon))
+  if (FormatTok->Tok.is(tok::colon)) {
 nextToken();
-  addUnwrappedLine();
+addUnwrappedLine();
+  } else if (!FormatTok->Tok.is(tok::coloncolon) &&
+ !std::binary_search(COperatorsFollowingVar.begin(),
+ COperatorsFollowingVar.end(),
+ FormatTok->Tok.getKind())) {
+// Not a variable name nor namespace name.
+addUnwrappedLine();
+  } else if (AccessSpecifierCandidate) {
+// Consider the access specifier to be a C identifier.
+AccessSpecifierCandidate->Tok.setKind(tok::identifier);
+  }
 }
 
 void UnwrappedLineParser::parseConcept() {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -100,10 +100,27 @@
 if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   return 0;
-if (RootToken.isAccessSpecifier(false) ||
-RootToken.isObjCAccessSpecifier() ||
-(RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
- RootToken.Next && RootToken.Next->is(tok::colon))) {
+
+auto IsAccessModifier = [this, &RootToken]() {
+  if (RootToken.isAccessSpecifier(Style.isCpp()))
+return true;
+  else if (RootToken.isObjCAccessSpecifier())
+return true;
+  // Handle Qt signals.
+  else if ((RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
+RootToken.Next && RootToken.Next->is(tok::colon)))
+return true;
+  else if (RootToken.Next &&
+   RootToken.Next->isOneOf(Keywords.kw_slots, Keywords.kw_qslots) &&
+   RootToken.Next->Next && RootToken.Next->Next->is(tok::colon))
+return true;
+  // Handle malformed access specifier e.g. 'private' without trailing ':'.
+  else if (!RootToken.Next && RootToken.isAccessSpecifier(false))
+retur

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito added a comment.

@HazardyKnusperkeks  I do not have commit access. This is my first commit to 
the project. Do I just need to issue an 'arc land' command?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

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

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito added a comment.

@HazardyKnusperkeks 
Ok thanks, someone else can commit on my behalf while I wait for commit access.

name: Philip Sigillito
email: psigill...@gmail.com


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

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

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito added a comment.

Do I need to care about these pre-build checks failing? F21921747: image.png 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

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

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito added a comment.

In D117416#3282937 , 
@HazardyKnusperkeks wrote:

> In D117416#3282853 , @psigillito 
> wrote:
>
>> Do I need to care about these pre-build checks failing? F21921747: image.png 
>> 
>
> The checks passed, the build on linux failed. But as far as I can see it's 
> something in openmp. I assume you did run all the format tests, then 
> everything should be fine.

Yes, I ran the format tests.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

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

[PATCH] D117415: Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito created this revision.
psigillito requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117415

Files:
  .arclint
  clang/lib/Format/FormatToken.h
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3030,6 +3030,8 @@
"label:\n"
"  signals.baz();\n"
"}");
+  // Var that looks like accessSpecifier
+  verifyFormat("private.p = 1;");
 }
 
 TEST_F(FormatTest, SeparatesLogicalBlocks) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2499,9 +2499,17 @@
   if (FormatTok->isOneOf(Keywords.kw_slots, Keywords.kw_qslots))
 nextToken();
   // Otherwise, we don't know what it is, and we'd better keep the next token.
-  if (FormatTok->Tok.is(tok::colon))
+  if (FormatTok->Tok.is(tok::colon)) {
 nextToken();
-  addUnwrappedLine();
+addUnwrappedLine();
+return;
+  }
+  // if followed by an operator probably intended to be a variable, dont treat
+  // as access specifier
+  if (clang::format::OperatorsFollowingVar.find(FormatTok->Tok.getKind()) ==
+  OperatorsFollowingVar.end()) {
+addUnwrappedLine();
+  }
 }
 
 void UnwrappedLineParser::parseConcept() {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -100,7 +100,14 @@
 if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   return 0;
-if (RootToken.isAccessSpecifier(false) ||
+if (RootToken.isAccessSpecifier(Style.Language == FormatStyle::LK_Cpp) ||
+(RootToken.Next &&
+ RootToken.Next->isOneOf(Keywords.kw_slots, Keywords.kw_qslots) &&
+ RootToken.Next->Next && RootToken.Next->Next->is(tok::colon)) ||
+(RootToken.isAccessSpecifier(false) &&
+ (!RootToken.Next ||
+  CppOperatorsFollowingVar.find(RootToken.Next->Tok.getKind()) ==
+  clang::format::CppOperatorsFollowingVar.end())) ||
 RootToken.isObjCAccessSpecifier() ||
 (RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
  RootToken.Next && RootToken.Next->is(tok::colon))) {
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -121,6 +121,32 @@
   TYPE(CSharpGenericTypeConstraintComma)   \
   TYPE(Unknown)
 
+static const std::set CppOperatorsFollowingVar = {
+tok::l_square,  tok::r_square,
+tok::l_paren,   tok::r_paren,
+tok::r_brace,   tok::period,
+tok::ellipsis,  tok::amp,
+tok::ampamp,tok::ampequal,
+tok::star,  tok::starequal,
+tok::plus,  tok::plusplus,
+tok::plusequal, tok::minus,
+tok::arrow, tok::minusminus,
+tok::minusequal,tok::exclaim,
+tok::exclaimequal,  tok::slash,
+tok::slashequal,tok::percent,
+tok::percentequal,  tok::less,
+tok::lessless,  tok::lessequal,
+tok::lesslessequal, tok::spaceship,
+tok::greater,   tok::greatergreater,
+tok::greaterequal,  tok::greatergreaterequal,
+tok::caret, tok::caretequal,
+tok::pipe,  tok::pipepipe,
+tok::pipeequal, tok::question,
+tok::semi,  tok::equal,
+tok::equalequal,tok::comma,
+tok::hash,  tok::hashhash,
+tok::hashat};
+
 /// Determines the semantic type of a syntactic token, e.g. whether "<" is a
 /// template opener or binary operator.
 enum TokenType : uint8_t {
Index: .arclint
===
--- .arclint
+++ .arclint
@@ -2,7 +2,7 @@
   "linters": {
 "clang-format": {
   "type": "script-and-regex",
-  "script-and-regex.script": "bash utils/arcanist/clang-format.sh",
+  "script-and-regex.script": "cmd utils\\arcanist\\clang-format.sh",
   "script-and-regex.regex": "/^(?P[[:alpha:]]+)\n(?P[^\n]+)\n(|(?P\\d),(?P\\d)\n(?P.*)\n(?P.*)\n)$/s",
   "include": [
 "(\\.(cc|cpp|h)$)"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117416: Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito created this revision.
psigillito requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117416

Files:
  .arclint
  clang/lib/Format/FormatToken.h
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3030,6 +3030,8 @@
"label:\n"
"  signals.baz();\n"
"}");
+  // Var that looks like accessSpecifier
+  verifyFormat("private.p = 1;");
 }
 
 TEST_F(FormatTest, SeparatesLogicalBlocks) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2499,9 +2499,17 @@
   if (FormatTok->isOneOf(Keywords.kw_slots, Keywords.kw_qslots))
 nextToken();
   // Otherwise, we don't know what it is, and we'd better keep the next token.
-  if (FormatTok->Tok.is(tok::colon))
+  if (FormatTok->Tok.is(tok::colon)) {
 nextToken();
-  addUnwrappedLine();
+addUnwrappedLine();
+return;
+  }
+  // if followed by an operator probably intended to be a variable, dont treat
+  // as access specifier
+  if (clang::format::OperatorsFollowingVar.find(FormatTok->Tok.getKind()) ==
+  OperatorsFollowingVar.end()) {
+addUnwrappedLine();
+  }
 }
 
 void UnwrappedLineParser::parseConcept() {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -100,7 +100,14 @@
 if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   return 0;
-if (RootToken.isAccessSpecifier(false) ||
+if (RootToken.isAccessSpecifier(Style.Language == FormatStyle::LK_Cpp) ||
+(RootToken.Next &&
+ RootToken.Next->isOneOf(Keywords.kw_slots, Keywords.kw_qslots) &&
+ RootToken.Next->Next && RootToken.Next->Next->is(tok::colon)) ||
+(RootToken.isAccessSpecifier(false) &&
+ (!RootToken.Next ||
+  CppOperatorsFollowingVar.find(RootToken.Next->Tok.getKind()) ==
+  clang::format::CppOperatorsFollowingVar.end())) ||
 RootToken.isObjCAccessSpecifier() ||
 (RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
  RootToken.Next && RootToken.Next->is(tok::colon))) {
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -121,6 +121,32 @@
   TYPE(CSharpGenericTypeConstraintComma)   \
   TYPE(Unknown)
 
+static const std::set CppOperatorsFollowingVar = {
+tok::l_square,  tok::r_square,
+tok::l_paren,   tok::r_paren,
+tok::r_brace,   tok::period,
+tok::ellipsis,  tok::amp,
+tok::ampamp,tok::ampequal,
+tok::star,  tok::starequal,
+tok::plus,  tok::plusplus,
+tok::plusequal, tok::minus,
+tok::arrow, tok::minusminus,
+tok::minusequal,tok::exclaim,
+tok::exclaimequal,  tok::slash,
+tok::slashequal,tok::percent,
+tok::percentequal,  tok::less,
+tok::lessless,  tok::lessequal,
+tok::lesslessequal, tok::spaceship,
+tok::greater,   tok::greatergreater,
+tok::greaterequal,  tok::greatergreaterequal,
+tok::caret, tok::caretequal,
+tok::pipe,  tok::pipepipe,
+tok::pipeequal, tok::question,
+tok::semi,  tok::equal,
+tok::equalequal,tok::comma,
+tok::hash,  tok::hashhash,
+tok::hashat};
+
 /// Determines the semantic type of a syntactic token, e.g. whether "<" is a
 /// template opener or binary operator.
 enum TokenType : uint8_t {
Index: .arclint
===
--- .arclint
+++ .arclint
@@ -2,7 +2,7 @@
   "linters": {
 "clang-format": {
   "type": "script-and-regex",
-  "script-and-regex.script": "bash utils/arcanist/clang-format.sh",
+  "script-and-regex.script": "cmd utils\\arcanist\\clang-format.sh",
   "script-and-regex.regex": "/^(?P[[:alpha:]]+)\n(?P[^\n]+)\n(|(?P\\d),(?P\\d)\n(?P.*)\n(?P.*)\n)$/s",
   "include": [
 "(\\.(cc|cpp|h)$)"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117416: Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito added a comment.

Sorry, this is my first time using Phabricator. I am not sure if it matters 
that .arclint is changed in this review, it is not part of my commit.

This issue only appears to impact the accessSpecifier keywords. For example, a 
c variable declared with the name 'private' will be treated as an access 
specifier.

I tried to make parsing the identifiers more restrictive by requiring either an 
@ prefix for obj-c or a colon after the keyword for cpp. Unfortunately, that 
does not allow an incorrect access specifier (i.e without a colon) to be 
identified as an accessor. I think a missing colon is probably a common typo so 
I dont want to remove identifying a access specifier with a missing colon 
correctly.

I think the correct approach is to check if the var named 'private' is followed 
by an operator or symbol indicating it is variable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

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

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito added a comment.

In D117416#3246838 , @curdeius wrote:

> Thanks for having a try on this.
> However, I don't like this approach too much. You add many changes and a 
> single test. That's not sufficient.
> Also, handling C++ keywords in all cases (e.g. `delete` as a function name) 
> *may* need to distinguish whether we format a C file or a C++ file. It's 
> probably impossible to do this without user input (.h extension is used in 
> both languages for example).
> We'd maybe need to add C as language option and let the user specify the 
> language (`-x c`?).
> That in turn may be painful (because not automatic).
> But, you may have a better solution.
> My 2 cents.

Thanks, I am just starting to understand some of the code base so my changes 
should be taken with a grain of salt. The more I think about this, I do not 
think my change is correct. As you pointed out with the 'delete' keyword, my 
changes would not correctly handle a struct named 'delete' i.e.

  struct delete foo = {0};
  delete.val; 

I think adding a cmd line argument specifying you are using C code is more 
correct. If the user does not specify they are using C, clang-format would 
format the code as it does now.

The only thing I don't like about this is that it might be kind of clumsy to 
only be able to specify your language is C. I can take a stab at implementing 
this if this is the route we want to go.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

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