from:"Daan De Meyer via Phabricator via cfe\-commits"

[PATCH] D114003: LiteralSupport: Don't assert() on invalid input

2021-11-16 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer updated this revision to Diff 387723.
DaanDeMeyer added a comment.

Addressed comments by adding two new errors, one for character literals and one 
for numeric literals.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114003

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/LiteralSupport.cpp


Index: clang/lib/Lex/LiteralSupport.cpp
===
--- clang/lib/Lex/LiteralSupport.cpp
+++ clang/lib/Lex/LiteralSupport.cpp
@@ -693,12 +693,6 @@
 : SM(SM), LangOpts(LangOpts), Diags(Diags),
   ThisTokBegin(TokSpelling.begin()), ThisTokEnd(TokSpelling.end()) {
 
-  // This routine assumes that the range begin/end matches the regex for 
integer
-  // and FP constants (specifically, the 'pp-number' regex), and assumes that
-  // the byte at "*end" is both valid and not part of the regex.  Because of
-  // this, it doesn't have to check for 'overscan' in various places.
-  assert(!isPreprocessingNumberBody(*ThisTokEnd) && "didn't maximally munch?");
-
   s = DigitsBegin = ThisTokBegin;
   saw_exponent = false;
   saw_period = false;
@@ -718,6 +712,16 @@
   isAccum = false;
   hadError = false;
 
+  // This routine assumes that the range begin/end matches the regex for 
integer
+  // and FP constants (specifically, the 'pp-number' regex), and assumes that
+  // the byte at "*end" is both valid and not part of the regex.  Because of
+  // this, it doesn't have to check for 'overscan' in various places.
+  if (isPreprocessingNumberBody(*ThisTokEnd)) {
+Diags.Report(TokLoc, diag::err_lexing_numeric);
+hadError = true;
+return;
+  }
+
   if (*s == '0') { // parse radix
 ParseNumberStartingWithZero(TokLoc);
 if (hadError)
@@ -1432,7 +1436,12 @@
 ++begin;
 
   // Skip over the entry quote.
-  assert(begin[0] == '\'' && "Invalid token lexed");
+  if (begin[0] != '\'') {
+PP.Diag(Loc, diag::err_lexing_char);
+HadError = true;
+return;
+  }
+
   ++begin;
 
   // Remove an optional ud-suffix.
Index: clang/include/clang/Basic/DiagnosticLexKinds.td
===
--- clang/include/clang/Basic/DiagnosticLexKinds.td
+++ clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -269,7 +269,9 @@
 def warn_bad_character_encoding : ExtWarn<
   "illegal character encoding in character literal">,
   InGroup;
-def err_lexing_string : Error<"failure when lexing a string">;
+def err_lexing_string : Error<"failure when lexing a string literal">;
+def err_lexing_char : Error<"failure when lexing a character literal">;
+def err_lexing_numeric : Error<"failure when lexing a numeric literal">;
 def err_placeholder_in_source : Error<"editor placeholder in source file">;
 
 
//===--===//


Index: clang/lib/Lex/LiteralSupport.cpp
===
--- clang/lib/Lex/LiteralSupport.cpp
+++ clang/lib/Lex/LiteralSupport.cpp
@@ -693,12 +693,6 @@
 : SM(SM), LangOpts(LangOpts), Diags(Diags),
   ThisTokBegin(TokSpelling.begin()), ThisTokEnd(TokSpelling.end()) {
 
-  // This routine assumes that the range begin/end matches the regex for integer
-  // and FP constants (specifically, the 'pp-number' regex), and assumes that
-  // the byte at "*end" is both valid and not part of the regex.  Because of
-  // this, it doesn't have to check for 'overscan' in various places.
-  assert(!isPreprocessingNumberBody(*ThisTokEnd) && "didn't maximally munch?");
-
   s = DigitsBegin = ThisTokBegin;
   saw_exponent = false;
   saw_period = false;
@@ -718,6 +712,16 @@
   isAccum = false;
   hadError = false;
 
+  // This routine assumes that the range begin/end matches the regex for integer
+  // and FP constants (specifically, the 'pp-number' regex), and assumes that
+  // the byte at "*end" is both valid and not part of the regex.  Because of
+  // this, it doesn't have to check for 'overscan' in various places.
+  if (isPreprocessingNumberBody(*ThisTokEnd)) {
+Diags.Report(TokLoc, diag::err_lexing_numeric);
+hadError = true;
+return;
+  }
+
   if (*s == '0') { // parse radix
 ParseNumberStartingWithZero(TokLoc);
 if (hadError)
@@ -1432,7 +1436,12 @@
 ++begin;
 
   // Skip over the entry quote.
-  assert(begin[0] == '\'' && "Invalid token lexed");
+  if (begin[0] != '\'') {
+PP.Diag(Loc, diag::err_lexing_char);
+HadError = true;
+return;
+  }
+
   ++begin;
 
   // Remove an optional ud-suffix.
Index: clang/include/clang/Basic/DiagnosticLexKinds.td
===
--- clang/include/clang/Basic/DiagnosticLexKinds.td
+++ clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -269,7 +269,9 @@
 def warn_bad_character_encoding : ExtWarn<
   "illegal character encoding in character literal">,
   InGroup;
-def

[PATCH] D114003: LiteralSupport: Don't assert() on invalid input

2021-11-16 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer updated this revision to Diff 387667.
DaanDeMeyer added a comment.

clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114003

Files:
  clang/lib/Lex/LiteralSupport.cpp


Index: clang/lib/Lex/LiteralSupport.cpp
===
--- clang/lib/Lex/LiteralSupport.cpp
+++ clang/lib/Lex/LiteralSupport.cpp
@@ -693,12 +693,6 @@
 : SM(SM), LangOpts(LangOpts), Diags(Diags),
   ThisTokBegin(TokSpelling.begin()), ThisTokEnd(TokSpelling.end()) {
 
-  // This routine assumes that the range begin/end matches the regex for 
integer
-  // and FP constants (specifically, the 'pp-number' regex), and assumes that
-  // the byte at "*end" is both valid and not part of the regex.  Because of
-  // this, it doesn't have to check for 'overscan' in various places.
-  assert(!isPreprocessingNumberBody(*ThisTokEnd) && "didn't maximally munch?");
-
   s = DigitsBegin = ThisTokBegin;
   saw_exponent = false;
   saw_period = false;
@@ -718,6 +712,16 @@
   isAccum = false;
   hadError = false;
 
+  // This routine assumes that the range begin/end matches the regex for 
integer
+  // and FP constants (specifically, the 'pp-number' regex), and assumes that
+  // the byte at "*end" is both valid and not part of the regex.  Because of
+  // this, it doesn't have to check for 'overscan' in various places.
+  if (isPreprocessingNumberBody(*ThisTokEnd)) {
+Diags.Report(TokLoc, diag::err_lexing_string);
+hadError = true;
+return;
+  }
+
   if (*s == '0') { // parse radix
 ParseNumberStartingWithZero(TokLoc);
 if (hadError)
@@ -1432,7 +1436,12 @@
 ++begin;
 
   // Skip over the entry quote.
-  assert(begin[0] == '\'' && "Invalid token lexed");
+  if (begin[0] != '\'') {
+PP.Diag(Loc, diag::err_lexing_string);
+HadError = true;
+return;
+  }
+
   ++begin;
 
   // Remove an optional ud-suffix.


Index: clang/lib/Lex/LiteralSupport.cpp
===
--- clang/lib/Lex/LiteralSupport.cpp
+++ clang/lib/Lex/LiteralSupport.cpp
@@ -693,12 +693,6 @@
 : SM(SM), LangOpts(LangOpts), Diags(Diags),
   ThisTokBegin(TokSpelling.begin()), ThisTokEnd(TokSpelling.end()) {
 
-  // This routine assumes that the range begin/end matches the regex for integer
-  // and FP constants (specifically, the 'pp-number' regex), and assumes that
-  // the byte at "*end" is both valid and not part of the regex.  Because of
-  // this, it doesn't have to check for 'overscan' in various places.
-  assert(!isPreprocessingNumberBody(*ThisTokEnd) && "didn't maximally munch?");
-
   s = DigitsBegin = ThisTokBegin;
   saw_exponent = false;
   saw_period = false;
@@ -718,6 +712,16 @@
   isAccum = false;
   hadError = false;
 
+  // This routine assumes that the range begin/end matches the regex for integer
+  // and FP constants (specifically, the 'pp-number' regex), and assumes that
+  // the byte at "*end" is both valid and not part of the regex.  Because of
+  // this, it doesn't have to check for 'overscan' in various places.
+  if (isPreprocessingNumberBody(*ThisTokEnd)) {
+Diags.Report(TokLoc, diag::err_lexing_string);
+hadError = true;
+return;
+  }
+
   if (*s == '0') { // parse radix
 ParseNumberStartingWithZero(TokLoc);
 if (hadError)
@@ -1432,7 +1436,12 @@
 ++begin;
 
   // Skip over the entry quote.
-  assert(begin[0] == '\'' && "Invalid token lexed");
+  if (begin[0] != '\'') {
+PP.Diag(Loc, diag::err_lexing_string);
+HadError = true;
+return;
+  }
+
   ++begin;
 
   // Remove an optional ud-suffix.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114003: LiteralSupport: Don't assert() on invalid input

2021-11-16 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer added a comment.

Added some people that were recent reviewers of changes to this file and some 
clangd folks.

I don't have the time to properly test this unfortunately (aside from verifying 
that it fixes all the clangd crashes I'm having), but putting the patch up 
anyway in case anyone's interested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114003

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

[PATCH] D114003: LiteralSupport: Don't assert() on invalid input

2021-11-16 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer created this revision.
DaanDeMeyer added reviewers: eduucaldas, beccadax, sammccall, kadircet.
DaanDeMeyer added a project: clang.
Herald added a subscriber: usaxena95.
DaanDeMeyer requested review of this revision.
Herald added subscribers: cfe-commits, ilya-biryukov.

  When using clangd, it's possible to trigger assertions in
  NumericLiteralParser and CharLiteralParser when switching git branches.
  This commit removes the initial asserts on invalid input and replaces
  those asserts with the error handling mechanism from those respective
  classes instead. This allows clangd to gracefully recover without
  crashing.
  
  See https://github.com/clangd/clangd/issues/888 for more information
  on the clangd crashes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114003

Files:
  clang/lib/Lex/LiteralSupport.cpp


Index: clang/lib/Lex/LiteralSupport.cpp
===
--- clang/lib/Lex/LiteralSupport.cpp
+++ clang/lib/Lex/LiteralSupport.cpp
@@ -693,12 +693,6 @@
 : SM(SM), LangOpts(LangOpts), Diags(Diags),
   ThisTokBegin(TokSpelling.begin()), ThisTokEnd(TokSpelling.end()) {
 
-  // This routine assumes that the range begin/end matches the regex for 
integer
-  // and FP constants (specifically, the 'pp-number' regex), and assumes that
-  // the byte at "*end" is both valid and not part of the regex.  Because of
-  // this, it doesn't have to check for 'overscan' in various places.
-  assert(!isPreprocessingNumberBody(*ThisTokEnd) && "didn't maximally munch?");
-
   s = DigitsBegin = ThisTokBegin;
   saw_exponent = false;
   saw_period = false;
@@ -718,6 +712,16 @@
   isAccum = false;
   hadError = false;
 
+  // This routine assumes that the range begin/end matches the regex for 
integer
+  // and FP constants (specifically, the 'pp-number' regex), and assumes that
+  // the byte at "*end" is both valid and not part of the regex.  Because of
+  // this, it doesn't have to check for 'overscan' in various places.
+  if (isPreprocessingNumberBody(*ThisTokEnd)) {
+  Diags.Report(TokLoc, diag::err_lexing_string);
+  hadError = true;
+  return;
+  }
+
   if (*s == '0') { // parse radix
 ParseNumberStartingWithZero(TokLoc);
 if (hadError)
@@ -1432,7 +1436,12 @@
 ++begin;
 
   // Skip over the entry quote.
-  assert(begin[0] == '\'' && "Invalid token lexed");
+  if (begin[0] != '\'') {
+PP.Diag(Loc, diag::err_lexing_string);
+HadError = true;
+return;
+  }
+
   ++begin;
 
   // Remove an optional ud-suffix.


Index: clang/lib/Lex/LiteralSupport.cpp
===
--- clang/lib/Lex/LiteralSupport.cpp
+++ clang/lib/Lex/LiteralSupport.cpp
@@ -693,12 +693,6 @@
 : SM(SM), LangOpts(LangOpts), Diags(Diags),
   ThisTokBegin(TokSpelling.begin()), ThisTokEnd(TokSpelling.end()) {
 
-  // This routine assumes that the range begin/end matches the regex for integer
-  // and FP constants (specifically, the 'pp-number' regex), and assumes that
-  // the byte at "*end" is both valid and not part of the regex.  Because of
-  // this, it doesn't have to check for 'overscan' in various places.
-  assert(!isPreprocessingNumberBody(*ThisTokEnd) && "didn't maximally munch?");
-
   s = DigitsBegin = ThisTokBegin;
   saw_exponent = false;
   saw_period = false;
@@ -718,6 +712,16 @@
   isAccum = false;
   hadError = false;
 
+  // This routine assumes that the range begin/end matches the regex for integer
+  // and FP constants (specifically, the 'pp-number' regex), and assumes that
+  // the byte at "*end" is both valid and not part of the regex.  Because of
+  // this, it doesn't have to check for 'overscan' in various places.
+  if (isPreprocessingNumberBody(*ThisTokEnd)) {
+  Diags.Report(TokLoc, diag::err_lexing_string);
+  hadError = true;
+  return;
+  }
+
   if (*s == '0') { // parse radix
 ParseNumberStartingWithZero(TokLoc);
 if (hadError)
@@ -1432,7 +1436,12 @@
 ++begin;
 
   // Skip over the entry quote.
-  assert(begin[0] == '\'' && "Invalid token lexed");
+  if (begin[0] != '\'') {
+PP.Diag(Loc, diag::err_lexing_string);
+HadError = true;
+return;
+  }
+
   ++begin;
 
   // Remove an optional ud-suffix.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95746: clang: Exclude efi_main from -Wmissing-prototypes

2021-02-20 Thread Daan De Meyer via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7dd42ecfa2a2: clang: Exclude efi_main from 
-Wmissing-prototypes (authored by DaanDeMeyer).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95746

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/no-warn-missing-prototype.c


Index: clang/test/Sema/no-warn-missing-prototype.c
===
--- clang/test/Sema/no-warn-missing-prototype.c
+++ clang/test/Sema/no-warn-missing-prototype.c
@@ -4,3 +4,7 @@
 int main() {
   return 0;
 }
+
+int efi_main() {
+  return 0;
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -13873,7 +13873,7 @@
   // Don't warn about 'main'.
   if (isa(FD->getDeclContext()->getRedeclContext()))
 if (IdentifierInfo *II = FD->getIdentifier())
-  if (II->isStr("main"))
+  if (II->isStr("main") || II->isStr("efi_main"))
 return false;
 
   // Don't warn about inline functions.


Index: clang/test/Sema/no-warn-missing-prototype.c
===
--- clang/test/Sema/no-warn-missing-prototype.c
+++ clang/test/Sema/no-warn-missing-prototype.c
@@ -4,3 +4,7 @@
 int main() {
   return 0;
 }
+
+int efi_main() {
+  return 0;
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -13873,7 +13873,7 @@
   // Don't warn about 'main'.
   if (isa(FD->getDeclContext()->getRedeclContext()))
 if (IdentifierInfo *II = FD->getIdentifier())
-  if (II->isStr("main"))
+  if (II->isStr("main") || II->isStr("efi_main"))
 return false;
 
   // Don't warn about inline functions.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95746: clang: Exclude efi_main from -Wmissing-prototypes

2021-02-20 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer added a comment.

I should be able to commit it myself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95746

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

[PATCH] D95746: clang: Exclude efi_main from -Wmissing-prototypes

2021-02-20 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer updated this revision to Diff 325224.
DaanDeMeyer added a comment.

Moved the test into the no-warn-missing-prototype test as requested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95746

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/no-warn-missing-prototype.c


Index: clang/test/Sema/no-warn-missing-prototype.c
===
--- clang/test/Sema/no-warn-missing-prototype.c
+++ clang/test/Sema/no-warn-missing-prototype.c
@@ -4,3 +4,7 @@
 int main() {
   return 0;
 }
+
+int efi_main() {
+  return 0;
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -13873,7 +13873,7 @@
   // Don't warn about 'main'.
   if (isa(FD->getDeclContext()->getRedeclContext()))
 if (IdentifierInfo *II = FD->getIdentifier())
-  if (II->isStr("main"))
+  if (II->isStr("main") || II->isStr("efi_main"))
 return false;
 
   // Don't warn about inline functions.


Index: clang/test/Sema/no-warn-missing-prototype.c
===
--- clang/test/Sema/no-warn-missing-prototype.c
+++ clang/test/Sema/no-warn-missing-prototype.c
@@ -4,3 +4,7 @@
 int main() {
   return 0;
 }
+
+int efi_main() {
+  return 0;
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -13873,7 +13873,7 @@
   // Don't warn about 'main'.
   if (isa(FD->getDeclContext()->getRedeclContext()))
 if (IdentifierInfo *II = FD->getIdentifier())
-  if (II->isStr("main"))
+  if (II->isStr("main") || II->isStr("efi_main"))
 return false;
 
   // Don't warn about inline functions.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95746: clang: Exclude efi_main from -Wmissing-prototypes

2021-02-19 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer added a comment.

Added a test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95746

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

[PATCH] D95746: clang: Exclude efi_main from -Wmissing-prototypes

2021-02-19 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer updated this revision to Diff 325114.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95746

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/no-warn-missing-prototype-efi.c


Index: clang/test/Sema/no-warn-missing-prototype-efi.c
===
--- /dev/null
+++ clang/test/Sema/no-warn-missing-prototype-efi.c
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only -Wmissing-prototypes -x c -ffreestanding 
-verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wmissing-prototypes -x c++ -ffreestanding 
-verify %s
+// expected-no-diagnostics
+int efi_main() {
+  return 0;
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -13873,7 +13873,7 @@
   // Don't warn about 'main'.
   if (isa(FD->getDeclContext()->getRedeclContext()))
 if (IdentifierInfo *II = FD->getIdentifier())
-  if (II->isStr("main"))
+  if (II->isStr("main") || II->isStr("efi_main"))
 return false;
 
   // Don't warn about inline functions.


Index: clang/test/Sema/no-warn-missing-prototype-efi.c
===
--- /dev/null
+++ clang/test/Sema/no-warn-missing-prototype-efi.c
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only -Wmissing-prototypes -x c -ffreestanding -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wmissing-prototypes -x c++ -ffreestanding -verify %s
+// expected-no-diagnostics
+int efi_main() {
+  return 0;
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -13873,7 +13873,7 @@
   // Don't warn about 'main'.
   if (isa(FD->getDeclContext()->getRedeclContext()))
 if (IdentifierInfo *II = FD->getIdentifier())
-  if (II->isStr("main"))
+  if (II->isStr("main") || II->isStr("efi_main"))
 return false;
 
   // Don't warn about inline functions.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95746: clang: Exclude efi_main from -Wmissing-prototypes

2021-02-02 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer updated this revision to Diff 320899.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95746

Files:
  clang/lib/Sema/SemaDecl.cpp


Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -13867,7 +13867,7 @@
   // Don't warn about 'main'.
   if (isa(FD->getDeclContext()->getRedeclContext()))
 if (IdentifierInfo *II = FD->getIdentifier())
-  if (II->isStr("main"))
+  if (II->isStr("main") || II->isStr("efi_main"))
 return false;
 
   // Don't warn about inline functions.


Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -13867,7 +13867,7 @@
   // Don't warn about 'main'.
   if (isa(FD->getDeclContext()->getRedeclContext()))
 if (IdentifierInfo *II = FD->getIdentifier())
-  if (II->isStr("main"))
+  if (II->isStr("main") || II->isStr("efi_main"))
 return false;
 
   // Don't warn about inline functions.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95746: clang: Exclude efi_main from -Wmissing-prototypes

2021-01-31 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer added a comment.

CI failures seem unrelated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95746

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

[PATCH] D95746: clang: Exclude efi_main from -Wmissing-prototypes

2021-01-30 Thread Daan De Meyer via Phabricator via cfe-commits

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

When compiling UEFI applications, the main function is named
efi_main() instead of main(). Let's exclude efi_main() from
-Wmissing-prototypes as well to avoid warnings when working
on UEFI applications.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95746

Files:
  clang/lib/Sema/SemaDecl.cpp


Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -13867,7 +13867,7 @@
   // Don't warn about 'main'.
   if (isa(FD->getDeclContext()->getRedeclContext()))
 if (IdentifierInfo *II = FD->getIdentifier())
-  if (II->isStr("main"))
+  if (II->isStr("main") || II->isStr("efi_main"))
 return false;
 
   // Don't warn about inline functions.


Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -13867,7 +13867,7 @@
   // Don't warn about 'main'.
   if (isa(FD->getDeclContext()->getRedeclContext()))
 if (IdentifierInfo *II = FD->getIdentifier())
-  if (II->isStr("main"))
+  if (II->isStr("main") || II->isStr("efi_main"))
 return false;
 
   // Don't warn about inline functions.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91509: [clangd] Add AST config to prebuild ASTs

2020-11-18 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer abandoned this revision.
DaanDeMeyer added a comment.

Abandoning as the end result is too slow even when including a limited number 
of files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91509

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

[PATCH] D91509: [clangd] Add AST config to prebuild ASTs

2020-11-16 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer updated this revision to Diff 305489.
DaanDeMeyer added a comment.

Fix all the bugs after doing some proper testing.

One thing we don't handle yet is header files since those don't appear in the 
compilation database. Maybe we need to add prebuilding for those as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91509

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.h
  clang-tools-extra/clangd/QueryDriverDatabase.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/unittests/ClangdTests.cpp
  clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
  clang-tools-extra/clangd/unittests/TestFS.cpp
  clang-tools-extra/clangd/unittests/TestFS.h

Index: clang-tools-extra/clangd/unittests/TestFS.h
===
--- clang-tools-extra/clangd/unittests/TestFS.h
+++ clang-tools-extra/clangd/unittests/TestFS.h
@@ -65,6 +65,8 @@
   llvm::Optional
   getCompileCommand(PathRef File) const override;
 
+  tooling::CompilationDatabase *lookupCDB(PathRef File) const override;
+
   llvm::Optional getProjectInfo(PathRef File) const override;
 
   std::vector ExtraClangFlags;
Index: clang-tools-extra/clangd/unittests/TestFS.cpp
===
--- clang-tools-extra/clangd/unittests/TestFS.cpp
+++ clang-tools-extra/clangd/unittests/TestFS.cpp
@@ -71,6 +71,11 @@
   FileName, std::move(CommandLine), "")};
 }
 
+tooling::CompilationDatabase *
+MockCompilationDatabase::lookupCDB(PathRef File) const {
+  return nullptr;
+}
+
 const char *testRoot() {
 #ifdef _WIN32
   return "C:\\clangd-test";
Index: clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
===
--- clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
+++ clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
@@ -70,6 +70,10 @@
   return None;
 }
 
+tooling::CompilationDatabase *lookupCDB(PathRef File) const override {
+  return nullptr;
+}
+
 tooling::CompileCommand
 getFallbackCommand(llvm::StringRef File) const override {
   return cmd(File, "-DA=2");
Index: clang-tools-extra/clangd/unittests/ClangdTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdTests.cpp
@@ -1123,6 +1123,10 @@
   FileName, std::move(CommandLine), "")};
 }
 
+tooling::CompilationDatabase *lookupCDB(PathRef File) const override {
+  return nullptr;
+}
+
 std::vector ExtraClangFlags;
 
   private:
Index: clang-tools-extra/clangd/TUScheduler.h
===
--- clang-tools-extra/clangd/TUScheduler.h
+++ clang-tools-extra/clangd/TUScheduler.h
@@ -231,6 +231,8 @@
   /// Returns true if the file was not previously tracked.
   bool update(PathRef File, ParseInputs Inputs, WantDiagnostics WD);
 
+  bool hasFile(PathRef File);
+
   /// Remove \p File from the list of tracked files and schedule removal of its
   /// resources. Pending diagnostics for closed files may not be delivered, even
   /// if requested with WantDiags::Auto or WantDiags::Yes.
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -627,11 +627,15 @@
   FileInputs = Inputs;
 }
 
-log("ASTWorker building file {0} version {1} with command {2}\n[{3}]\n{4}",
+log("ASTWorker building file {0} version {1}", FileName, Inputs.Version);
+vlog("ASTWorker building file {0} version {1} with command {2}\n[{3}]\n{4}",
 FileName, Inputs.Version, Inputs.CompileCommand.Heuristic,
 Inputs.CompileCommand.Directory,
 llvm::join(Inputs.CompileCommand.CommandLine, " "));
 
+if (!InputsAreTheSame)
+  log("ASTWorker inputs are not the same, cached AST is invalidated");
+
 StoreDiags CompilerInvocationDiagConsumer;
 std::vector CC1Args;
 std::unique_ptr Invocation = buildCompilerInvocation(
@@ -1289,6 +1293,8 @@
   return NewFile;
 }
 
+bool TUScheduler::hasFile(PathRef File) { return Files[File] != nullptr; }
+
 void TUScheduler::remove(PathRef File) {
   bool Removed = Files.erase(File);
   if (!Removed)
Index:

[PATCH] D91509: [clangd] Add AST config to prebuild ASTs

2020-11-16 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer updated this revision to Diff 305468.
DaanDeMeyer added a comment.

Fix tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91509

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.h
  clang-tools-extra/clangd/QueryDriverDatabase.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/unittests/ClangdTests.cpp
  clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
  clang-tools-extra/clangd/unittests/TestFS.cpp
  clang-tools-extra/clangd/unittests/TestFS.h

Index: clang-tools-extra/clangd/unittests/TestFS.h
===
--- clang-tools-extra/clangd/unittests/TestFS.h
+++ clang-tools-extra/clangd/unittests/TestFS.h
@@ -65,6 +65,8 @@
   llvm::Optional
   getCompileCommand(PathRef File) const override;
 
+  tooling::CompilationDatabase *lookupCDB(PathRef File) const override;
+
   llvm::Optional getProjectInfo(PathRef File) const override;
 
   std::vector ExtraClangFlags;
Index: clang-tools-extra/clangd/unittests/TestFS.cpp
===
--- clang-tools-extra/clangd/unittests/TestFS.cpp
+++ clang-tools-extra/clangd/unittests/TestFS.cpp
@@ -71,6 +71,11 @@
   FileName, std::move(CommandLine), "")};
 }
 
+tooling::CompilationDatabase *
+MockCompilationDatabase::lookupCDB(PathRef File) const {
+  return nullptr;
+}
+
 const char *testRoot() {
 #ifdef _WIN32
   return "C:\\clangd-test";
Index: clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
===
--- clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
+++ clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
@@ -70,6 +70,10 @@
   return None;
 }
 
+tooling::CompilationDatabase *lookupCDB(PathRef File) const override {
+  return nullptr;
+}
+
 tooling::CompileCommand
 getFallbackCommand(llvm::StringRef File) const override {
   return cmd(File, "-DA=2");
Index: clang-tools-extra/clangd/unittests/ClangdTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdTests.cpp
@@ -1123,6 +1123,10 @@
   FileName, std::move(CommandLine), "")};
 }
 
+tooling::CompilationDatabase *lookupCDB(PathRef File) const override {
+  return nullptr;
+}
+
 std::vector ExtraClangFlags;
 
   private:
Index: clang-tools-extra/clangd/TUScheduler.h
===
--- clang-tools-extra/clangd/TUScheduler.h
+++ clang-tools-extra/clangd/TUScheduler.h
@@ -231,6 +231,8 @@
   /// Returns true if the file was not previously tracked.
   bool update(PathRef File, ParseInputs Inputs, WantDiagnostics WD);
 
+  bool hasFile(PathRef File);
+
   /// Remove \p File from the list of tracked files and schedule removal of its
   /// resources. Pending diagnostics for closed files may not be delivered, even
   /// if requested with WantDiags::Auto or WantDiags::Yes.
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -1289,6 +1289,8 @@
   return NewFile;
 }
 
+bool TUScheduler::hasFile(PathRef File) { return Files[File] == nullptr; }
+
 void TUScheduler::remove(PathRef File) {
   bool Removed = Files.erase(File);
   if (!Removed)
Index: clang-tools-extra/clangd/QueryDriverDatabase.cpp
===
--- clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -277,6 +277,10 @@
 return addSystemIncludes(*Cmd, SystemIncludes);
   }
 
+  tooling::CompilationDatabase *lookupCDB(PathRef File) const override {
+return Base->lookupCDB(File);
+  }
+
   llvm::Optional getProjectInfo(PathRef File) const override {
 return Base->getProjectInfo(File);
   }
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.h
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.h
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.h
@@ -40,6 +40,8 @@
   virtual llvm::Optional
   getCompileCommand(PathRef File) const = 0;
 
+  virtual tooling::CompilationDatabase *lookupCDB(PathRef File) const = 0;
+

[PATCH] D91509: [clangd] Add AST config to prebuild ASTs

2020-11-15 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer updated this revision to Diff 305396.
DaanDeMeyer added a comment.

Fixed formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91509

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.h
  clang-tools-extra/clangd/QueryDriverDatabase.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h

Index: clang-tools-extra/clangd/TUScheduler.h
===
--- clang-tools-extra/clangd/TUScheduler.h
+++ clang-tools-extra/clangd/TUScheduler.h
@@ -231,6 +231,8 @@
   /// Returns true if the file was not previously tracked.
   bool update(PathRef File, ParseInputs Inputs, WantDiagnostics WD);
 
+  bool hasFile(PathRef File);
+
   /// Remove \p File from the list of tracked files and schedule removal of its
   /// resources. Pending diagnostics for closed files may not be delivered, even
   /// if requested with WantDiags::Auto or WantDiags::Yes.
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -1289,6 +1289,8 @@
   return NewFile;
 }
 
+bool TUScheduler::hasFile(PathRef File) { return Files[File] == nullptr; }
+
 void TUScheduler::remove(PathRef File) {
   bool Removed = Files.erase(File);
   if (!Removed)
Index: clang-tools-extra/clangd/QueryDriverDatabase.cpp
===
--- clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -277,6 +277,10 @@
 return addSystemIncludes(*Cmd, SystemIncludes);
   }
 
+  tooling::CompilationDatabase *lookupCDB(PathRef File) const override {
+return Base->lookupCDB(File);
+  }
+
   llvm::Optional getProjectInfo(PathRef File) const override {
 return Base->getProjectInfo(File);
   }
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.h
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.h
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.h
@@ -40,6 +40,8 @@
   virtual llvm::Optional
   getCompileCommand(PathRef File) const = 0;
 
+  virtual tooling::CompilationDatabase *lookupCDB(PathRef File) const = 0;
+
   /// Finds the closest project to \p File.
   virtual llvm::Optional getProjectInfo(PathRef File) const {
 return llvm::None;
@@ -76,6 +78,8 @@
   llvm::Optional
   getCompileCommand(PathRef File) const override;
 
+  virtual tooling::CompilationDatabase *lookupCDB(PathRef File) const override;
+
   /// Returns the path to first directory containing a compilation database in
   /// \p File's parents.
   llvm::Optional getProjectInfo(PathRef File) const override;
@@ -132,6 +136,9 @@
 
   llvm::Optional
   getCompileCommand(PathRef File) const override;
+
+  tooling::CompilationDatabase *lookupCDB(PathRef File) const override;
+
   tooling::CompileCommand getFallbackCommand(PathRef File) const override;
   /// Project info is gathered purely from the inner compilation database to
   /// ensure consistency.
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -68,6 +68,20 @@
 
 llvm::Optional
 DirectoryBasedGlobalCompilationDatabase::getCompileCommand(PathRef File) const {
+  auto *CDB = lookupCDB(File);
+  if (!CDB) {
+return llvm::None;
+  }
+
+  auto Candidates = CDB->getCompileCommands(File);
+  if (!Candidates.empty())
+return std::move(Candidates.front());
+
+  return None;
+}
+
+tooling::CompilationDatabase *
+DirectoryBasedGlobalCompilationDatabase::lookupCDB(PathRef File) const {
   CDBLookupRequest Req;
   Req.FileName = File;
   Req.ShouldBroadcast = true;
@@ -75,14 +89,10 @@
   auto Res = lookupCDB(Req);
   if (!Res) {
 log("Failed to find compilation database for {0}", File);
-return llvm::None;
+return nullptr;
   }
 
-  auto Candidates = Res->CDB->getCompileCommands(File);
-  if (!Candidates.empty())
-return std::move(Candidates.front());
-
-  return None;
+  return Res->CDB;
 }
 
 // For platforms where paths are case-insensitive (but case-preserving),
@@ -270,6 +280,10 @@
   return Cmd;
 }
 
+tooling::CompilationDatabase *OverlayCDB::lookupCDB(PathRef File) const {
+  return Base ? Base->lookupCDB(File) : nullptr;
+}
+
 tooling::CompileCommand OverlayCDB::getFallbackCommand(PathRef File) const

[PATCH] D91509: [clangd] Add AST config to prebuild ASTs

2020-11-15 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer updated this revision to Diff 305394.
DaanDeMeyer retitled this revision from "[clangd] RFC: Add --prebuild-asts 
option" to "[clangd] Add AST config to prebuild ASTs".
DaanDeMeyer edited the summary of this revision.
DaanDeMeyer added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Switched from all or nothing command line option to per file config.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91509

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.h
  clang-tools-extra/clangd/QueryDriverDatabase.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h

Index: clang-tools-extra/clangd/TUScheduler.h
===
--- clang-tools-extra/clangd/TUScheduler.h
+++ clang-tools-extra/clangd/TUScheduler.h
@@ -231,6 +231,8 @@
   /// Returns true if the file was not previously tracked.
   bool update(PathRef File, ParseInputs Inputs, WantDiagnostics WD);
 
+  bool hasFile(PathRef File);
+
   /// Remove \p File from the list of tracked files and schedule removal of its
   /// resources. Pending diagnostics for closed files may not be delivered, even
   /// if requested with WantDiags::Auto or WantDiags::Yes.
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -1289,6 +1289,8 @@
   return NewFile;
 }
 
+bool TUScheduler::hasFile(PathRef File) { return Files[File] == nullptr; }
+
 void TUScheduler::remove(PathRef File) {
   bool Removed = Files.erase(File);
   if (!Removed)
Index: clang-tools-extra/clangd/QueryDriverDatabase.cpp
===
--- clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -277,6 +277,10 @@
 return addSystemIncludes(*Cmd, SystemIncludes);
   }
 
+  tooling::CompilationDatabase *lookupCDB(PathRef File) const override {
+return Base->lookupCDB(File);
+  }
+
   llvm::Optional getProjectInfo(PathRef File) const override {
 return Base->getProjectInfo(File);
   }
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.h
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.h
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.h
@@ -40,6 +40,8 @@
   virtual llvm::Optional
   getCompileCommand(PathRef File) const = 0;
 
+  virtual tooling::CompilationDatabase *lookupCDB(PathRef File) const = 0;
+
   /// Finds the closest project to \p File.
   virtual llvm::Optional getProjectInfo(PathRef File) const {
 return llvm::None;
@@ -76,6 +78,8 @@
   llvm::Optional
   getCompileCommand(PathRef File) const override;
 
+  virtual tooling::CompilationDatabase *lookupCDB(PathRef File) const override;
+
   /// Returns the path to first directory containing a compilation database in
   /// \p File's parents.
   llvm::Optional getProjectInfo(PathRef File) const override;
@@ -132,6 +136,9 @@
 
   llvm::Optional
   getCompileCommand(PathRef File) const override;
+
+  tooling::CompilationDatabase *lookupCDB(PathRef File) const override;
+
   tooling::CompileCommand getFallbackCommand(PathRef File) const override;
   /// Project info is gathered purely from the inner compilation database to
   /// ensure consistency.
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -68,6 +68,20 @@
 
 llvm::Optional
 DirectoryBasedGlobalCompilationDatabase::getCompileCommand(PathRef File) const {
+  auto *CDB = lookupCDB(File);
+  if (!CDB) {
+return llvm::None;
+  }
+
+  auto Candidates = CDB->getCompileCommands(File);
+  if (!Candidates.empty())
+return std::move(Candidates.front());
+
+  return None;
+}
+
+tooling::CompilationDatabase *
+DirectoryBasedGlobalCompilationDatabase::lookupCDB(PathRef File) const {
   CDBLookupRequest Req;
   Req.FileName = File;
   Req.ShouldBroadcast = true;
@@ -75,14 +89,10 @@
   auto Res = lookupCDB(Req);
   if (!Res) {
 log("Failed to find compilation database for {0}", File);
-return llvm::None;
+return nullptr;
   }
 
-  auto Candidates = Res->CDB->getCompileCommands(File);
-  if (!Candidates.empty())
-return std::move(Candidates.front());
-
-  return None;
+  return Res->CDB;
 }
 
 // For platforms

[PATCH] D78869: clang-format: Add ControlStatementsExceptForEachMacros option to SpaceBeforeParens

2020-05-06 Thread Daan De Meyer via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf21c704553a8: clang-format: Add 
ControlStatementsExceptForEachMacros option to… (authored by DaanDeMeyer).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78869

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -972,6 +972,17 @@
"  UNKNOWN_FORACH(Item * item, itemlist) {}\n"
"}");
 
+  FormatStyle Style = getLLVMStyle();
+  Style.SpaceBeforeParens =
+  FormatStyle::SBPO_ControlStatementsExceptForEachMacros;
+  verifyFormat("void f() {\n"
+   "  foreach(Item *item, itemlist) {}\n"
+   "  Q_FOREACH(Item *item, itemlist) {}\n"
+   "  BOOST_FOREACH(Item *item, itemlist) {}\n"
+   "  UNKNOWN_FORACH(Item * item, itemlist) {}\n"
+   "}",
+   Style);
+
   // As function-like macros.
   verifyFormat("#define foreach(x, y)\n"
"#define Q_FOREACH(x, y)\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2900,6 +2900,10 @@
 if ((Left.is(tok::r_paren) && Left.is(TT_AttributeParen)) ||
 (Left.is(tok::r_square) && Left.is(TT_AttributeSquare)))
   return true;
+if (Style.SpaceBeforeParens ==
+FormatStyle::SBPO_ControlStatementsExceptForEachMacros &&
+Left.is(TT_ForEachMacro))
+  return false;
 return Line.Type == LT_ObjCDecl || Left.is(tok::semi) ||
(Style.SpaceBeforeParens != FormatStyle::SBPO_Never &&
 (Left.isOneOf(tok::pp_elif, tok::kw_for, tok::kw_while,
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -329,6 +329,8 @@
 IO.enumCase(Value, "Never", FormatStyle::SBPO_Never);
 IO.enumCase(Value, "ControlStatements",
 FormatStyle::SBPO_ControlStatements);
+IO.enumCase(Value, "ControlStatementsExceptForEachMacros",
+FormatStyle::SBPO_ControlStatementsExceptForEachMacros);
 IO.enumCase(Value, "NonEmptyParentheses",
 FormatStyle::SBPO_NonEmptyParentheses);
 IO.enumCase(Value, "Always", FormatStyle::SBPO_Always);
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1983,6 +1983,17 @@
 ///}
 /// \endcode
 SBPO_ControlStatements,
+/// Same as ``SBPO_ControlStatements`` except this option doesn't apply to
+/// ForEach macros. This is useful in projects where ForEach macros are 
+/// treated as function calls instead of control statements. 
+/// \code
+///void f() {
+///  Q_FOREACH(...) {
+///f();
+///  }
+///}
+/// \endcode
+SBPO_ControlStatementsExceptForEachMacros,
 /// Put a space before opening parentheses only if the parentheses are not
 /// empty i.e. '()'
 /// \code
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2336,6 +2336,19 @@
  }
}
 
+  * ``SBPO_ControlStatementsExceptForEachMacros`` (in configuration: 
``ControlStatementsExceptForEachMacros``)
+Same as ``SBPO_ControlStatements`` except this option doesn't apply to
+ForEach macros. This is useful in projects where ForEach macros are
+treated as function calls instead of control statements.
+
+.. code-block:: c++
+
+   void f() {
+ Q_FOREACH(...) {
+   f();
+ }
+   }
+
   * ``SBPO_NonEmptyParentheses`` (in configuration: ``NonEmptyParentheses``)
 Put a space before opening parentheses only if the parentheses are not
 empty i.e. '()'


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -972,6 +972,17 @@
"  UNKNOWN_FORACH(Item * item, itemlist) {}\n"
"}");
 
+  FormatStyle Style = getLLVMStyle();
+  Style.SpaceBeforeParens =
+  FormatStyle::SBPO_ControlStatementsExceptForEachMacros;
+  verifyFormat("void f() {\n"
+   "  foreach(Item *item, itemlist) {}\n"
+   "

[PATCH] D78869: clang-format: Add ControlStatementsExceptForEachMacros option to SpaceBeforeParens

2020-05-06 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer added a comment.

I pushed the commit to github. Thanks for the help!


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

https://reviews.llvm.org/D78869



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

[PATCH] D78869: clang-format: Add ControlStatementsExceptForEachMacros option to SpaceBeforeParens

2020-05-06 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer added a comment.

I should have commit access now. Is there any documentation on how exactly to 
do a merge? Do I just add the commit to master and push? I apologize if this 
seems obvious, I just want to make sure I'm not inadvertently breaking 
something.


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

https://reviews.llvm.org/D78869



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

[PATCH] D78869: clang-format: Add ControlStatementsExceptForEachMacros option to SpaceBeforeParens

2020-05-05 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer added a comment.

(I don't have commit access so it'd be great if someone could merge this for 
me, at least that's how my other contributions have been handled)


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

https://reviews.llvm.org/D78869



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

[PATCH] D78869: clang-format: Add ControlStatementsExceptForEachMacros option to SpaceBeforeParens

2020-04-26 Thread Daan De Meyer via Phabricator via cfe-commits

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



Comment at: clang/docs/ClangFormatStyleOptions.rst:2311
 
+  * ``SBPO_ControlStatementsExceptForEachMacros`` (in configuration: 
+``ControlStatementsExceptForEachMacros``)

MyDeveloperDay wrote:
> Normally this would be generated from the Format.h after running the dump 
> style script in tools, did you?
I didn't think the docs were automatically generated. I added a comment to 
Format.h and re-generated the docs using the dump_format_style script.


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

https://reviews.llvm.org/D78869



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

[PATCH] D78869: clang-format: Add ControlStatementsExceptForEachMacros option to SpaceBeforeParens

2020-04-26 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer updated this revision to Diff 260161.
DaanDeMeyer added a comment.

- Moved docs to Format.h and re-generated rst file using dump_format_style.py
- Expanded comment with an explanation for why you might want to not have a 
space before the parens of a ForEach macro

This isn't explicitly mentioned in systemd's style guide. I simply did a regex 
search over systemd's codebase. Searching (using regex) for ForEach macro 
usages with a space before the parens returns 7 matches over the entire 
codebase. Searching for ForEach macro usages without a space before the parens 
returns 1753 matches. If needed, I can make a systemd PR that proposes 
explicitly adding this to the style guide.

I only added the relevant changes produced by dump_format_style to this 
revision but there were some other changes produced as well (just to let you 
know).


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

https://reviews.llvm.org/D78869

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -972,6 +972,17 @@
"  UNKNOWN_FORACH(Item * item, itemlist) {}\n"
"}");
 
+  FormatStyle Style = getLLVMStyle();
+  Style.SpaceBeforeParens =
+  FormatStyle::SBPO_ControlStatementsExceptForEachMacros;
+  verifyFormat("void f() {\n"
+   "  foreach(Item *item, itemlist) {}\n"
+   "  Q_FOREACH(Item *item, itemlist) {}\n"
+   "  BOOST_FOREACH(Item *item, itemlist) {}\n"
+   "  UNKNOWN_FORACH(Item * item, itemlist) {}\n"
+   "}",
+   Style);
+
   // As function-like macros.
   verifyFormat("#define foreach(x, y)\n"
"#define Q_FOREACH(x, y)\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2889,6 +2889,10 @@
 if ((Left.is(tok::r_paren) && Left.is(TT_AttributeParen)) ||
 (Left.is(tok::r_square) && Left.is(TT_AttributeSquare)))
   return true;
+if (Style.SpaceBeforeParens ==
+FormatStyle::SBPO_ControlStatementsExceptForEachMacros &&
+Left.is(TT_ForEachMacro))
+  return false;
 return Line.Type == LT_ObjCDecl || Left.is(tok::semi) ||
(Style.SpaceBeforeParens != FormatStyle::SBPO_Never &&
 (Left.isOneOf(tok::pp_elif, tok::kw_for, tok::kw_while,
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -327,6 +327,8 @@
 IO.enumCase(Value, "Never", FormatStyle::SBPO_Never);
 IO.enumCase(Value, "ControlStatements",
 FormatStyle::SBPO_ControlStatements);
+IO.enumCase(Value, "ControlStatementsExceptForEachMacros",
+FormatStyle::SBPO_ControlStatementsExceptForEachMacros);
 IO.enumCase(Value, "NonEmptyParentheses",
 FormatStyle::SBPO_NonEmptyParentheses);
 IO.enumCase(Value, "Always", FormatStyle::SBPO_Always);
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1954,6 +1954,17 @@
 ///}
 /// \endcode
 SBPO_ControlStatements,
+/// Same as ``SBPO_ControlStatements`` except this option doesn't apply to
+/// ForEach macros. This is useful in projects where ForEach macros are 
+/// treated as function calls instead of control statements. 
+/// \code
+///void f() {
+///  Q_FOREACH(...) {
+///f();
+///  }
+///}
+/// \endcode
+SBPO_ControlStatementsExceptForEachMacros,
 /// Put a space before opening parentheses only if the parentheses are not
 /// empty i.e. '()'
 /// \code
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2308,6 +2308,19 @@
  }
}
 
+  * ``SBPO_ControlStatementsExceptForEachMacros`` (in configuration: 
``ControlStatementsExceptForEachMacros``)
+Same as ``SBPO_ControlStatements`` except this option doesn't apply to
+ForEach macros. This is useful in projects where ForEach macros are
+treated as function calls instead of control statements.
+
+.. code-block:: c++
+
+   void f() {
+ Q_FOREACH(...) {
+   f();
+ }
+   }
+
   * ``SBPO_NonEmptyParentheses`` (in configuration: ``NonEmptyParentheses``)
 Put a space before

[PATCH] D78869: clang-format: Add ControlStatementsExceptForEachMacros option to SpaceBeforeParens

2020-04-26 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer updated this revision to Diff 260157.
DaanDeMeyer added a comment.

Fixed formatting


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

https://reviews.llvm.org/D78869

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -972,6 +972,17 @@
"  UNKNOWN_FORACH(Item * item, itemlist) {}\n"
"}");
 
+  FormatStyle Style = getLLVMStyle();
+  Style.SpaceBeforeParens =
+  FormatStyle::SBPO_ControlStatementsExceptForEachMacros;
+  verifyFormat("void f() {\n"
+   "  foreach(Item *item, itemlist) {}\n"
+   "  Q_FOREACH(Item *item, itemlist) {}\n"
+   "  BOOST_FOREACH(Item *item, itemlist) {}\n"
+   "  UNKNOWN_FORACH(Item * item, itemlist) {}\n"
+   "}",
+   Style);
+
   // As function-like macros.
   verifyFormat("#define foreach(x, y)\n"
"#define Q_FOREACH(x, y)\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2889,6 +2889,10 @@
 if ((Left.is(tok::r_paren) && Left.is(TT_AttributeParen)) ||
 (Left.is(tok::r_square) && Left.is(TT_AttributeSquare)))
   return true;
+if (Style.SpaceBeforeParens ==
+FormatStyle::SBPO_ControlStatementsExceptForEachMacros &&
+Left.is(TT_ForEachMacro))
+  return false;
 return Line.Type == LT_ObjCDecl || Left.is(tok::semi) ||
(Style.SpaceBeforeParens != FormatStyle::SBPO_Never &&
 (Left.isOneOf(tok::pp_elif, tok::kw_for, tok::kw_while,
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -327,6 +327,8 @@
 IO.enumCase(Value, "Never", FormatStyle::SBPO_Never);
 IO.enumCase(Value, "ControlStatements",
 FormatStyle::SBPO_ControlStatements);
+IO.enumCase(Value, "ControlStatementsExceptForEachMacros",
+FormatStyle::SBPO_ControlStatementsExceptForEachMacros);
 IO.enumCase(Value, "NonEmptyParentheses",
 FormatStyle::SBPO_NonEmptyParentheses);
 IO.enumCase(Value, "Always", FormatStyle::SBPO_Always);
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1954,6 +1954,7 @@
 ///}
 /// \endcode
 SBPO_ControlStatements,
+SBPO_ControlStatementsExceptForEachMacros,
 /// Put a space before opening parentheses only if the parentheses are not
 /// empty i.e. '()'
 /// \code
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2308,6 +2308,19 @@
  }
}
 
+  * ``SBPO_ControlStatementsExceptForEachMacros`` (in configuration: 
+``ControlStatementsExceptForEachMacros``)
+Same as ``SBPO_ControlStatements`` except this option doesn't apply to
+ForEach macros.
+
+.. code-block:: c++
+
+   void f() {
+ Q_FOREACH(...) {
+   f();
+ }
+   }
+
   * ``SBPO_NonEmptyParentheses`` (in configuration: ``NonEmptyParentheses``)
 Put a space before opening parentheses only if the parentheses are not
 empty i.e. '()'


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -972,6 +972,17 @@
"  UNKNOWN_FORACH(Item * item, itemlist) {}\n"
"}");
 
+  FormatStyle Style = getLLVMStyle();
+  Style.SpaceBeforeParens =
+  FormatStyle::SBPO_ControlStatementsExceptForEachMacros;
+  verifyFormat("void f() {\n"
+   "  foreach(Item *item, itemlist) {}\n"
+   "  Q_FOREACH(Item *item, itemlist) {}\n"
+   "  BOOST_FOREACH(Item *item, itemlist) {}\n"
+   "  UNKNOWN_FORACH(Item * item, itemlist) {}\n"
+   "}",
+   Style);
+
   // As function-like macros.
   verifyFormat("#define foreach(x, y)\n"
"#define Q_FOREACH(x, y)\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2889,6 +2889,10 @@
 if ((Left.is(tok::r_paren) && Left.is(TT_AttributeParen)) ||

[PATCH] D76197: clang-format: Use block indentation for braced initializations

2020-04-26 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer abandoned this revision.
DaanDeMeyer added a comment.

Turns out this is already possible by disabling Cpp11BracedListStyle. The 
proposed change introduced too many changes regardless so I'm abandoning this.


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

https://reviews.llvm.org/D76197



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

[PATCH] D78869: clang-format: Add ControlStatementsExceptForEachMacros option to SpaceBeforeParens

2020-04-25 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer created this revision.
DaanDeMeyer added a reviewer: clang-format.
DaanDeMeyer added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

systemd  recently added a clang-format 
file. One issue I encountered in using clang-format on systemd is that systemd 
does not add a space before the parens of their foreach macros but clang-format 
always adds a space. This does not seem to be configurable in clang-format. 
This revision adds the ControlStatementsExceptForEachMacros option to 
SpaceBeforeParens which puts a space before all control statement parens except 
ForEach macros. This drastically reduces the amount of changes when running 
clang-format on systemd's source code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78869

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -972,6 +972,16 @@
"  UNKNOWN_FORACH(Item * item, itemlist) {}\n"
"}");
 
+  FormatStyle Style = getLLVMStyle();
+  Style.SpaceBeforeParens =
+  FormatStyle::SBPO_ControlStatementsExceptForEachMacros;
+  verifyFormat("void f() {\n"
+   "  foreach(Item *item, itemlist) {}\n"
+   "  Q_FOREACH(Item *item, itemlist) {}\n"
+   "  BOOST_FOREACH(Item *item, itemlist) {}\n"
+   "  UNKNOWN_FORACH(Item * item, itemlist) {}\n"
+   "}", Style);
+
   // As function-like macros.
   verifyFormat("#define foreach(x, y)\n"
"#define Q_FOREACH(x, y)\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2889,6 +2889,10 @@
 if ((Left.is(tok::r_paren) && Left.is(TT_AttributeParen)) ||
 (Left.is(tok::r_square) && Left.is(TT_AttributeSquare)))
   return true;
+if (Style.SpaceBeforeParens ==
+FormatStyle::SBPO_ControlStatementsExceptForEachMacros &&
+Left.is(TT_ForEachMacro))
+  return false;
 return Line.Type == LT_ObjCDecl || Left.is(tok::semi) ||
(Style.SpaceBeforeParens != FormatStyle::SBPO_Never &&
 (Left.isOneOf(tok::pp_elif, tok::kw_for, tok::kw_while,
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -327,6 +327,8 @@
 IO.enumCase(Value, "Never", FormatStyle::SBPO_Never);
 IO.enumCase(Value, "ControlStatements",
 FormatStyle::SBPO_ControlStatements);
+IO.enumCase(Value, "ControlStatementsExceptForEachMacros",
+FormatStyle::SBPO_ControlStatementsExceptForEachMacros);
 IO.enumCase(Value, "NonEmptyParentheses",
 FormatStyle::SBPO_NonEmptyParentheses);
 IO.enumCase(Value, "Always", FormatStyle::SBPO_Always);
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1954,6 +1954,7 @@
 ///}
 /// \endcode
 SBPO_ControlStatements,
+SBPO_ControlStatementsExceptForEachMacros,
 /// Put a space before opening parentheses only if the parentheses are not
 /// empty i.e. '()'
 /// \code
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2308,6 +2308,19 @@
  }
}
 
+  * ``SBPO_ControlStatementsExceptForEachMacros`` (in configuration: 
+``ControlStatementsExceptForEachMacros``)
+Same as ``SBPO_ControlStatements`` except this option doesn't apply to
+ForEach macros.
+
+.. code-block:: c++
+
+   void f() {
+ Q_FOREACH(...) {
+   f();
+ }
+   }
+
   * ``SBPO_NonEmptyParentheses`` (in configuration: ``NonEmptyParentheses``)
 Put a space before opening parentheses only if the parentheses are not
 empty i.e. '()'


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -972,6 +972,16 @@
"  UNKNOWN_FORACH(Item * item, itemlist) {}\n"
"}");
 
+  FormatStyle Style = getLLVMStyle();
+  Style.SpaceBeforeParens =
+  FormatStyle::SBPO_ControlStatementsExceptForEachMacros;
+  verifyFormat("void f() {\n"
+   "  foreach(Item *item, itemlist) {}\n"
+

[PATCH] D76197: clang-format: Use block indentation for braced initializations

2020-03-26 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer added a comment.

Of course, if there's interest in adding this I'll fix all the tests but I 
wanted to make sure there was interest in adding this since it changes 
clang-format's behavior. Can you confirm that this change in behavior is a good 
thing and might be merged if I fix all the tests? If the existing output cannot 
be changed in any way then this revision can be closed since it fundamentally 
requires changing clang-format's output.


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

https://reviews.llvm.org/D76197



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

[PATCH] D76197: clang-format: Use block indentation for braced initializations

2020-03-15 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer updated this revision to Diff 250429.
DaanDeMeyer added a comment.

New implementation that breaks fewer tests.

It seems to be expected that block kind is `BK_Unknown` for C struct braced 
init lists. Is that supposed to be the case?

There's still quite some tests failures but most of them are simply indentation 
changes from 4 to 2 spaces (which is LLVM's indentation size) which might be 
unavoidable if we want this change to happen.


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

https://reviews.llvm.org/D76197

Files:
  clang/lib/Format/FormatToken.h


Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -508,7 +508,7 @@
   return true;
 return is(TT_ArrayInitializerLSquare) || is(TT_ProtoExtensionLSquare) ||
(is(tok::l_brace) &&
-(BlockKind == BK_Block || is(TT_DictLiteral) ||
+(BlockKind == BK_Block || BlockKind == BK_Unknown || 
is(TT_DictLiteral) ||
  (!Style.Cpp11BracedListStyle && NestingLevel == 0))) ||
(is(tok::less) && (Style.Language == FormatStyle::LK_Proto ||
   Style.Language == FormatStyle::LK_TextProto));


Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -508,7 +508,7 @@
   return true;
 return is(TT_ArrayInitializerLSquare) || is(TT_ProtoExtensionLSquare) ||
(is(tok::l_brace) &&
-(BlockKind == BK_Block || is(TT_DictLiteral) ||
+(BlockKind == BK_Block || BlockKind == BK_Unknown || is(TT_DictLiteral) ||
  (!Style.Cpp11BracedListStyle && NestingLevel == 0))) ||
(is(tok::less) && (Style.Language == FormatStyle::LK_Proto ||
   Style.Language == FormatStyle::LK_TextProto));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75022: clang-format: Extend AllowShortLoopsOnASingleLine to do ... while loops.

2020-02-26 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer added a comment.

Is there CI infra that runs for each revision? I verified all the format unit 
tests still pass but I haven't run the entire test suite on my machine.


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

https://reviews.llvm.org/D75022



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

[PATCH] D75022: clang-format: Extend AllowShortLoopsOnASingleLine to do ... while loops.

2020-02-25 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer updated this revision to Diff 246512.
DaanDeMeyer added a comment.

Added extra unit tests


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

https://reviews.llvm.org/D75022

Files:
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -556,6 +556,30 @@
   verifyFormat("for (;;) /* still don't merge */\n"
"  continue;",
AllowsMergedLoops);
+  verifyFormat("do a++;\n"
+   "while (true);", 
+   AllowsMergedLoops);
+  verifyFormat("do /* Don't merge */\n"
+   "  a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
+  verifyFormat("do // Don't merge\n"
+   "  a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
+  verifyFormat("do\n"
+   "  // Don't merge\n"
+   "  a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
+  // Without braces labels are interpreted differently.
+  verifyFormat("{\n"
+   "  do\n"
+   "  label:\n"
+   "a++;\n"
+   "  while (true);\n"
+   "}",
+   AllowsMergedLoops);
 }
 
 TEST_F(FormatTest, FormatShortBracedStatements) {
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -404,7 +404,7 @@
  ? tryMergeSimpleControlStatement(I, E, Limit)
  : 0;
 }
-if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while)) {
+if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while, tok::kw_do)) {
   return Style.AllowShortLoopsOnASingleLine
  ? tryMergeSimpleControlStatement(I, E, Limit)
  : 0;
@@ -449,7 +449,10 @@
   return 0;
 Limit = limitConsideringMacros(I + 1, E, Limit);
 AnnotatedLine  = **I;
-if (Line.Last->isNot(tok::r_paren))
+if (!Line.First->is(tok::kw_do) && Line.Last->isNot(tok::r_paren))
+  return 0;
+// Only merge do while if do is the only statement on the line. 
+if (Line.First->is(tok::kw_do) && !Line.Last->is(tok::kw_do))
   return 0;
 if (1 + I[1]->Last->TotalLength > Limit)
   return 0;


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -556,6 +556,30 @@
   verifyFormat("for (;;) /* still don't merge */\n"
"  continue;",
AllowsMergedLoops);
+  verifyFormat("do a++;\n"
+   "while (true);", 
+   AllowsMergedLoops);
+  verifyFormat("do /* Don't merge */\n"
+   "  a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
+  verifyFormat("do // Don't merge\n"
+   "  a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
+  verifyFormat("do\n"
+   "  // Don't merge\n"
+   "  a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
+  // Without braces labels are interpreted differently.
+  verifyFormat("{\n"
+   "  do\n"
+   "  label:\n"
+   "a++;\n"
+   "  while (true);\n"
+   "}",
+   AllowsMergedLoops);
 }
 
 TEST_F(FormatTest, FormatShortBracedStatements) {
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -404,7 +404,7 @@
  ? tryMergeSimpleControlStatement(I, E, Limit)
  : 0;
 }
-if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while)) {
+if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while, tok::kw_do)) {
   return Style.AllowShortLoopsOnASingleLine
  ? tryMergeSimpleControlStatement(I, E, Limit)
  : 0;
@@ -449,7 +449,10 @@
   return 0;
 Limit = limitConsideringMacros(I + 1, E, Limit);
 AnnotatedLine  = **I;
-if (Line.Last->isNot(tok::r_paren))
+if (!Line.First->is(tok::kw_do) && Line.Last->isNot(tok::r_paren))
+  return 0;
+// Only merge do while if do is the only statement on the line. 
+if (Line.First->is(tok::kw_do) && !Line.Last->is(tok::kw_do))
   return 0;
 if (1 + I[1]->Last->TotalLength > Limit)
   return 0;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75022: clang-format: Extend AllowShortLoopsOnASingleLine to do ... while loops.

2020-02-24 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer updated this revision to Diff 246244.
DaanDeMeyer added a comment.

Added unit tests and fixed the case where stuff follows the do statement (like 
a comment).


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

https://reviews.llvm.org/D75022

Files:
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -556,6 +556,17 @@
   verifyFormat("for (;;) /* still don't merge */\n"
"  continue;",
AllowsMergedLoops);
+  verifyFormat("do a++;\n"
+   "while (true);", 
+   AllowsMergedLoops);
+  verifyFormat("do /* Don't merge */\n"
+   "  a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
+  verifyFormat("do // Don't merge\n"
+   "  a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
 }
 
 TEST_F(FormatTest, FormatShortBracedStatements) {
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -404,7 +404,7 @@
  ? tryMergeSimpleControlStatement(I, E, Limit)
  : 0;
 }
-if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while)) {
+if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while, tok::kw_do)) {
   return Style.AllowShortLoopsOnASingleLine
  ? tryMergeSimpleControlStatement(I, E, Limit)
  : 0;
@@ -449,7 +449,10 @@
   return 0;
 Limit = limitConsideringMacros(I + 1, E, Limit);
 AnnotatedLine  = **I;
-if (Line.Last->isNot(tok::r_paren))
+if (!Line.First->is(tok::kw_do) && Line.Last->isNot(tok::r_paren))
+  return 0;
+// Only merge do while if do is the only statement on the line. 
+if (Line.First->is(tok::kw_do) && !Line.Last->is(tok::kw_do))
   return 0;
 if (1 + I[1]->Last->TotalLength > Limit)
   return 0;


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -556,6 +556,17 @@
   verifyFormat("for (;;) /* still don't merge */\n"
"  continue;",
AllowsMergedLoops);
+  verifyFormat("do a++;\n"
+   "while (true);", 
+   AllowsMergedLoops);
+  verifyFormat("do /* Don't merge */\n"
+   "  a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
+  verifyFormat("do // Don't merge\n"
+   "  a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
 }
 
 TEST_F(FormatTest, FormatShortBracedStatements) {
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -404,7 +404,7 @@
  ? tryMergeSimpleControlStatement(I, E, Limit)
  : 0;
 }
-if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while)) {
+if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while, tok::kw_do)) {
   return Style.AllowShortLoopsOnASingleLine
  ? tryMergeSimpleControlStatement(I, E, Limit)
  : 0;
@@ -449,7 +449,10 @@
   return 0;
 Limit = limitConsideringMacros(I + 1, E, Limit);
 AnnotatedLine  = **I;
-if (Line.Last->isNot(tok::r_paren))
+if (!Line.First->is(tok::kw_do) && Line.Last->isNot(tok::r_paren))
+  return 0;
+// Only merge do while if do is the only statement on the line. 
+if (Line.First->is(tok::kw_do) && !Line.Last->is(tok::kw_do))
   return 0;
 if (1 + I[1]->Last->TotalLength > Limit)
   return 0;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75022: clang-format: Extend AllowShortLoopsOnASingleLine to do ... while loops.

2020-02-24 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer updated this revision to Diff 246150.

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

https://reviews.llvm.org/D75022

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -404,7 +404,7 @@
  ? tryMergeSimpleControlStatement(I, E, Limit)
  : 0;
 }
-if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while)) {
+if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while, tok::kw_do)) {
   return Style.AllowShortLoopsOnASingleLine
  ? tryMergeSimpleControlStatement(I, E, Limit)
  : 0;
@@ -449,7 +449,7 @@
   return 0;
 Limit = limitConsideringMacros(I + 1, E, Limit);
 AnnotatedLine  = **I;
-if (Line.Last->isNot(tok::r_paren))
+if (!Line.First->is(tok::kw_do) && Line.Last->isNot(tok::r_paren))
   return 0;
 if (1 + I[1]->Last->TotalLength > Limit)
   return 0;


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -404,7 +404,7 @@
  ? tryMergeSimpleControlStatement(I, E, Limit)
  : 0;
 }
-if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while)) {
+if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while, tok::kw_do)) {
   return Style.AllowShortLoopsOnASingleLine
  ? tryMergeSimpleControlStatement(I, E, Limit)
  : 0;
@@ -449,7 +449,7 @@
   return 0;
 Limit = limitConsideringMacros(I + 1, E, Limit);
 AnnotatedLine  = **I;
-if (Line.Last->isNot(tok::r_paren))
+if (!Line.First->is(tok::kw_do) && Line.Last->isNot(tok::r_paren))
   return 0;
 if (1 + I[1]->Last->TotalLength > Limit)
   return 0;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53481: [clangd] Support passing a relative path to -compile-commands-dir

2018-10-23 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer added a comment.

You can commit them. Thanks for all the quick responses!


https://reviews.llvm.org/D53481



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

[PATCH] D53527: Fix range length comparison in DraftStore::UpdateDraft when Unicode characters are removed from the document

2018-10-23 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer updated this revision to Diff 170603.
DaanDeMeyer added a comment.

Update diff according to comments.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53527

Files:
  clangd/DraftStore.cpp
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  unittests/clangd/SourceCodeTests.cpp


Index: unittests/clangd/SourceCodeTests.cpp
===
--- unittests/clangd/SourceCodeTests.cpp
+++ unittests/clangd/SourceCodeTests.cpp
@@ -42,6 +42,16 @@
   return range;
 }
 
+TEST(SourceCodeTests, lspLength) {
+  EXPECT_EQ(lspLength(""), 0UL);
+  EXPECT_EQ(lspLength("ascii"), 5UL);
+  // BMP
+  EXPECT_EQ(lspLength("↓"), 1UL);
+  EXPECT_EQ(lspLength("¥"), 1UL);
+  // astral
+  EXPECT_EQ(lspLength(""), 2UL);
+}
+
 TEST(SourceCodeTests, PositionToOffset) {
   // line out of bounds
   EXPECT_THAT_EXPECTED(positionToOffset(File, position(-1, 2)), Failed());
Index: clangd/SourceCode.cpp
===
--- clangd/SourceCode.cpp
+++ clangd/SourceCode.cpp
@@ -67,13 +67,12 @@
   return std::min(Result, U8.size());
 }
 
-// Counts the number of UTF-16 code units needed to represent a string.
 // Like most strings in clangd, the input is UTF-8 encoded.
-static size_t utf16Len(StringRef U8) {
+size_t lspLength(StringRef Code) {
   // A codepoint takes two UTF-16 code unit if it's astral (outside BMP).
   // Astral codepoints are encoded as 4 bytes in UTF-8, starting with 0xxx.
   size_t Count = 0;
-  iterateCodepoints(U8, [&](int U8Len, int U16Len) {
+  iterateCodepoints(Code, [&](int U8Len, int U16Len) {
 Count += U16Len;
 return false;
   });
@@ -123,7 +122,7 @@
   size_t StartOfLine = (PrevNL == StringRef::npos) ? 0 : (PrevNL + 1);
   Position Pos;
   Pos.line = Lines;
-  Pos.character = utf16Len(Before.substr(StartOfLine));
+  Pos.character = lspLength(Before.substr(StartOfLine));
   return Pos;
 }
 
@@ -139,7 +138,7 @@
   if (!Invalid) {
 auto ColumnInBytes = SM.getColumnNumber(FID, Offset) - 1;
 auto LineSoFar = Code.substr(Offset - ColumnInBytes, ColumnInBytes);
-P.character = utf16Len(LineSoFar);
+P.character = lspLength(LineSoFar);
   }
   return P;
 }
Index: clangd/SourceCode.h
===
--- clangd/SourceCode.h
+++ clangd/SourceCode.h
@@ -23,6 +23,10 @@
 
 namespace clangd {
 
+// Counts the number of UTF-16 code units needed to represent a string (LSP
+// specifies string lengths in UTF-16 code units).
+size_t lspLength(StringRef Code);
+
 /// Turn a [line, column] pair into an offset in Code.
 ///
 /// If P.character exceeds the line length, returns the offset at end-of-line.
Index: clangd/DraftStore.cpp
===
--- clangd/DraftStore.cpp
+++ clangd/DraftStore.cpp
@@ -77,8 +77,17 @@
   End, Start),
   llvm::errc::invalid_argument);
 
-if (Change.rangeLength &&
-(ssize_t)(*EndIndex - *StartIndex) != *Change.rangeLength)
+// Since the range length between two LSP positions is dependent on the
+// contents of the buffer we compute the range length between the start and
+// end position ourselves and compare it to the range length of the LSP
+// message to verify the buffers of the client and server are in sync.
+
+// EndIndex and StartIndex are in bytes, but Change.rangeLength is in 
UTF-16
+// code units.
+ssize_t ComputedRangeLength =
+lspLength(Contents.substr(*StartIndex, *EndIndex - *StartIndex));
+
+if (Change.rangeLength && ComputedRangeLength != *Change.rangeLength)
   return make_error(
   formatv("Change's rangeLength ({0}) doesn't match the "
   "computed range length ({1}).",


Index: unittests/clangd/SourceCodeTests.cpp
===
--- unittests/clangd/SourceCodeTests.cpp
+++ unittests/clangd/SourceCodeTests.cpp
@@ -42,6 +42,16 @@
   return range;
 }
 
+TEST(SourceCodeTests, lspLength) {
+  EXPECT_EQ(lspLength(""), 0UL);
+  EXPECT_EQ(lspLength("ascii"), 5UL);
+  // BMP
+  EXPECT_EQ(lspLength("↓"), 1UL);
+  EXPECT_EQ(lspLength("¥"), 1UL);
+  // astral
+  EXPECT_EQ(lspLength(""), 2UL);
+}
+
 TEST(SourceCodeTests, PositionToOffset) {
   // line out of bounds
   EXPECT_THAT_EXPECTED(positionToOffset(File, position(-1, 2)), Failed());
Index: clangd/SourceCode.cpp
===
--- clangd/SourceCode.cpp
+++ clangd/SourceCode.cpp
@@ -67,13 +67,12 @@
   return std::min(Result, U8.size());
 }
 
-// Counts the number of UTF-16 code units needed to represent a string.
 // Like most strings in clangd, the input is UTF-8 encoded.
-static size_t utf16Len(StringRef U8) {
+size_t lspLength(StringRef Code) {
   // A codepoint takes two UTF-16 code unit if it's astral (outside BMP).
   // Astral codepoints are encoded as 4 bytes in UTF-8, starting

[PATCH] D53527: Fix range length comparison in DraftStore::UpdateDraft when Unicode characters are removed from the document

2018-10-22 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer created this revision.
DaanDeMeyer added a reviewer: sammccall.
DaanDeMeyer added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, ioeric, 
ilya-biryukov.

See http://lists.llvm.org/pipermail/clangd-dev/2018-October/000171.html for 
context.

I kept the error (instead of downgrading to a log message) since the range 
lengths differing does indicate either a bug in the client or server range 
calculation or the buffers being out of sync (which both seems serious enough 
to me to be an error). If any existing clients aside from VSCode break they 
should only break when accidentally typing a Unicode character which should 
only be a minor nuisance for a little while until the bug is fixed in the 
respective client.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53527

Files:
  clangd/DraftStore.cpp
  clangd/SourceCode.cpp
  clangd/SourceCode.h


Index: clangd/SourceCode.cpp
===
--- clangd/SourceCode.cpp
+++ clangd/SourceCode.cpp
@@ -67,13 +67,14 @@
   return std::min(Result, U8.size());
 }
 
-// Counts the number of UTF-16 code units needed to represent a string.
-// Like most strings in clangd, the input is UTF-8 encoded.
-static size_t utf16Len(StringRef U8) {
+// Counts the number of UTF-16 code units needed to represent a string (LSP
+// specifies string lengths in UTF-16 code units). Like most strings in clangd,
+// the input is UTF-8 encoded.
+size_t lspLength(StringRef Code) {
   // A codepoint takes two UTF-16 code unit if it's astral (outside BMP).
   // Astral codepoints are encoded as 4 bytes in UTF-8, starting with 0xxx.
   size_t Count = 0;
-  iterateCodepoints(U8, [&](int U8Len, int U16Len) {
+  iterateCodepoints(Code, [&](int U8Len, int U16Len) {
 Count += U16Len;
 return false;
   });
@@ -123,7 +124,7 @@
   size_t StartOfLine = (PrevNL == StringRef::npos) ? 0 : (PrevNL + 1);
   Position Pos;
   Pos.line = Lines;
-  Pos.character = utf16Len(Before.substr(StartOfLine));
+  Pos.character = lspLength(Before.substr(StartOfLine));
   return Pos;
 }
 
@@ -139,7 +140,7 @@
   if (!Invalid) {
 auto ColumnInBytes = SM.getColumnNumber(FID, Offset) - 1;
 auto LineSoFar = Code.substr(Offset - ColumnInBytes, ColumnInBytes);
-P.character = utf16Len(LineSoFar);
+P.character = lspLength(LineSoFar);
   }
   return P;
 }
Index: clangd/SourceCode.h
===
--- clangd/SourceCode.h
+++ clangd/SourceCode.h
@@ -23,6 +23,9 @@
 
 namespace clangd {
 
+/// Calculate the length of Code according to the LSP specification.
+size_t lspLength(StringRef Code);
+
 /// Turn a [line, column] pair into an offset in Code.
 ///
 /// If P.character exceeds the line length, returns the offset at end-of-line.
Index: clangd/DraftStore.cpp
===
--- clangd/DraftStore.cpp
+++ clangd/DraftStore.cpp
@@ -77,8 +77,21 @@
   End, Start),
   llvm::errc::invalid_argument);
 
-if (Change.rangeLength &&
-(ssize_t)(*EndIndex - *StartIndex) != *Change.rangeLength)
+// Since the range length between two LSP positions is dependent on the
+// contents of the buffer we compute the range length between the start and
+// end position ourselves and compare it to the range length of the LSP
+// message to verify the buffers of the client and server are in sync.
+
+// The difference between EndIndex and StartIndex gives the range length in
+// bytes. However, the LSP range length is specified in UTF-16 code units
+// which means directly comparing them will give faulty results. To solve
+// this we convert the computed range length in bytes back to UTF-16 code
+// units by counting the amount of UTF-16 code units in the [StartIndex,
+// EndIndex) substring of the document buffer.
+ssize_t ComputedRangeLength =
+lspLength(Contents.substr(*StartIndex, *EndIndex - *StartIndex));
+
+if (Change.rangeLength && ComputedRangeLength != *Change.rangeLength)
   return make_error(
   formatv("Change's rangeLength ({0}) doesn't match the "
   "computed range length ({1}).",


Index: clangd/SourceCode.cpp
===
--- clangd/SourceCode.cpp
+++ clangd/SourceCode.cpp
@@ -67,13 +67,14 @@
   return std::min(Result, U8.size());
 }
 
-// Counts the number of UTF-16 code units needed to represent a string.
-// Like most strings in clangd, the input is UTF-8 encoded.
-static size_t utf16Len(StringRef U8) {
+// Counts the number of UTF-16 code units needed to represent a string (LSP
+// specifies string lengths in UTF-16 code units). Like most strings in clangd,
+// the input is UTF-8 encoded.
+size_t lspLength(StringRef Code) {
   // A codepoint takes two UTF-16 code unit if it's astral (outside BMP).
   // Astral

[PATCH] D53481: [clangd] Support passing a relative path to -compile-commands-dir

2018-10-22 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer updated this revision to Diff 170384.
DaanDeMeyer added a comment.

Updated diff according to comments.


https://reviews.llvm.org/D53481

Files:
  clangd/tool/ClangdMain.cpp


Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -251,16 +251,24 @@
   // If --compile-commands-dir arg was invoked, check value and override 
default
   // path.
   Optional CompileCommandsDirPath;
-  if (CompileCommandsDir.empty()) {
-CompileCommandsDirPath = None;
-  } else if (!sys::path::is_absolute(CompileCommandsDir) ||
- !sys::fs::exists(CompileCommandsDir)) {
-errs() << "Path specified by --compile-commands-dir either does not "
-  "exist or is not an absolute "
-  "path. The argument will be ignored.\n";
-CompileCommandsDirPath = None;
-  } else {
-CompileCommandsDirPath = CompileCommandsDir;
+  if (!CompileCommandsDir.empty()) {
+if (sys::fs::exists(CompileCommandsDir)) {
+  // We support passing both relative and absolute paths to the
+  // --compile-commands-dir argument, but we assume the path is absolute in
+  // the rest of clangd so we make sure the path is absolute before
+  // continuing.
+  SmallString<128> Path(CompileCommandsDir);
+  if (std::error_code EC = sys::fs::make_absolute(Path)) {
+errs() << "Error while converting the relative path specified by "
+  "--compile-commands-dir to an absolute path: "
+   << EC.message() << ". The argument will be ignored.\n";
+  } else {
+CompileCommandsDirPath = Path.str();
+  }
+} else {
+  errs() << "Path specified by --compile-commands-dir does not exist. The "
+"argument will be ignored.\n";
+}
   }
 
   ClangdServer::Options Opts;


Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -251,16 +251,24 @@
   // If --compile-commands-dir arg was invoked, check value and override default
   // path.
   Optional CompileCommandsDirPath;
-  if (CompileCommandsDir.empty()) {
-CompileCommandsDirPath = None;
-  } else if (!sys::path::is_absolute(CompileCommandsDir) ||
- !sys::fs::exists(CompileCommandsDir)) {
-errs() << "Path specified by --compile-commands-dir either does not "
-  "exist or is not an absolute "
-  "path. The argument will be ignored.\n";
-CompileCommandsDirPath = None;
-  } else {
-CompileCommandsDirPath = CompileCommandsDir;
+  if (!CompileCommandsDir.empty()) {
+if (sys::fs::exists(CompileCommandsDir)) {
+  // We support passing both relative and absolute paths to the
+  // --compile-commands-dir argument, but we assume the path is absolute in
+  // the rest of clangd so we make sure the path is absolute before
+  // continuing.
+  SmallString<128> Path(CompileCommandsDir);
+  if (std::error_code EC = sys::fs::make_absolute(Path)) {
+errs() << "Error while converting the relative path specified by "
+  "--compile-commands-dir to an absolute path: "
+   << EC.message() << ". The argument will be ignored.\n";
+  } else {
+CompileCommandsDirPath = Path.str();
+  }
+} else {
+  errs() << "Path specified by --compile-commands-dir does not exist. The "
+"argument will be ignored.\n";
+}
   }
 
   ClangdServer::Options Opts;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53481: [clangd] Support passing a relative path to -compile-commands-dir

2018-10-21 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer created this revision.
DaanDeMeyer added reviewers: clang-tools-extra, sammccall.
DaanDeMeyer added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, 
ioeric, ilya-biryukov.

This is useful when using clangd with CMake based projects in Visual Studio 
Code since when using CMake the `compile_commands.json` file is usually located 
in a `build` subdirectory which isn't a parent directory of the source files. 
Allowing passing relative paths to -compile-commands-dir allows specifying 
`clangd.arguments = ["-compile-commands-dir=build"]` in VSCode's settings file 
and having it work for each CMake based project that uses the `build` 
subdirectory as the build directory (instead of having to specify the absolute 
path to the compile commands directory for each separate project in VSCode's 
settings).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53481

Files:
  clangd/tool/ClangdMain.cpp


Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -253,14 +253,27 @@
   Optional CompileCommandsDirPath;
   if (CompileCommandsDir.empty()) {
 CompileCommandsDirPath = None;
-  } else if (!sys::path::is_absolute(CompileCommandsDir) ||
- !sys::fs::exists(CompileCommandsDir)) {
-errs() << "Path specified by --compile-commands-dir either does not "
-  "exist or is not an absolute "
-  "path. The argument will be ignored.\n";
+  } else if (!sys::fs::exists(CompileCommandsDir)) {
+errs() << "Path specified by --compile-commands-dir does not exist. The "
+  "argument will be ignored.\n";
 CompileCommandsDirPath = None;
   } else {
-CompileCommandsDirPath = CompileCommandsDir;
+// If the compile-commands-dir arg path is absolute, use it directly. If
+// the path is relative, try to convert it to an absolute path first.
+if (sys::path::is_absolute(CompileCommandsDir)) {
+  CompileCommandsDirPath = CompileCommandsDir;
+} else {
+  SmallString<128> Path(CompileCommandsDir);
+  std::error_code EC = sys::fs::make_absolute(Path);
+  if (EC) {
+errs() << "Error while converting the relative path specified by "
+  "--compile-commands-dir to an absolute path: "
+   << EC.message() << ". The argument will be ignored.\n";
+CompileCommandsDirPath = None;
+  } else {
+CompileCommandsDirPath = Path.str();
+  }
+}
   }
 
   ClangdServer::Options Opts;


Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -253,14 +253,27 @@
   Optional CompileCommandsDirPath;
   if (CompileCommandsDir.empty()) {
 CompileCommandsDirPath = None;
-  } else if (!sys::path::is_absolute(CompileCommandsDir) ||
- !sys::fs::exists(CompileCommandsDir)) {
-errs() << "Path specified by --compile-commands-dir either does not "
-  "exist or is not an absolute "
-  "path. The argument will be ignored.\n";
+  } else if (!sys::fs::exists(CompileCommandsDir)) {
+errs() << "Path specified by --compile-commands-dir does not exist. The "
+  "argument will be ignored.\n";
 CompileCommandsDirPath = None;
   } else {
-CompileCommandsDirPath = CompileCommandsDir;
+// If the compile-commands-dir arg path is absolute, use it directly. If
+// the path is relative, try to convert it to an absolute path first.
+if (sys::path::is_absolute(CompileCommandsDir)) {
+  CompileCommandsDirPath = CompileCommandsDir;
+} else {
+  SmallString<128> Path(CompileCommandsDir);
+  std::error_code EC = sys::fs::make_absolute(Path);
+  if (EC) {
+errs() << "Error while converting the relative path specified by "
+  "--compile-commands-dir to an absolute path: "
+   << EC.message() << ". The argument will be ignored.\n";
+CompileCommandsDirPath = None;
+  } else {
+CompileCommandsDirPath = Path.str();
+  }
+}
   }
 
   ClangdServer::Options Opts;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

37 matches

Mail list logo