[PATCH] D55552: [Sema] Better static assert diagnostics for expressions involving temporaries.

[Clement Courbet via Phabricator via cfe-commits]

courbet updated this revision to Diff 178010.
courbet added a comment.

Handle remaining static_assert categories inside the prettyprinter.


Repository:
  rC Clang

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

https://reviews.llvm.org/D2

Files:
  include/clang/AST/Stmt.h
  lib/AST/StmtPrinter.cpp
  lib/Sema/SemaTemplate.cpp
  test/SemaCXX/static-assert-cxx17.cpp
  test/SemaCXX/static-assert.cpp

Index: test/SemaCXX/static-assert.cpp
===
--- test/SemaCXX/static-assert.cpp
+++ test/SemaCXX/static-assert.cpp
@@ -76,6 +76,8 @@
   static const Tp value = v;
   typedef Tp value_type;
   typedef integral_constant type;
+  constexpr operator value_type() const noexcept { return value; }
+  constexpr value_type operator()() const noexcept { return value; }
 };
 
 template 
@@ -103,6 +105,7 @@
 } // namespace std
 
 struct ExampleTypes {
+  explicit ExampleTypes(int);
   using T = int;
   using U = float;
 };
@@ -119,6 +122,18 @@
 // expected-error@-1{{static_assert failed due to requirement 'std::is_const::value == false' "message"}}
 static_assert(!(std::is_const::value == true), "message");
 // expected-error@-1{{static_assert failed due to requirement '!(std::is_const::value == true)' "message"}}
+static_assert(std::is_const(), "message");
+// expected-error@-1{{static_assert failed due to requirement 'std::is_const()' "message"}}
+static_assert(!(std::is_const()()), "message");
+// expected-error@-1{{static_assert failed due to requirement '!(std::is_const()())' "message"}}
+static_assert(std::is_same()), int>::value, "message");
+// expected-error@-1{{static_assert failed due to requirement 'std::is_same, int>::value' "message"}}
+static_assert(std::is_const::value, "message");
+// expected-error@-1{{static_assert failed due to requirement 'std::is_const::value' "message"}}
+static_assert(std::is_const::value, "message");
+// expected-error@-1{{static_assert failed due to requirement 'std::is_const::value' "message"}}
+static_assert(std::is_const::value, "message");
+// expected-error@-1{{static_assert failed due to requirement 'std::is_const::value' "message"}}
 
 struct BI_tag {};
 struct RAI_tag : BI_tag {};
Index: test/SemaCXX/static-assert-cxx17.cpp
===
--- test/SemaCXX/static-assert-cxx17.cpp
+++ test/SemaCXX/static-assert-cxx17.cpp
@@ -54,3 +54,42 @@
 }
 template void foo5();
 // expected-note@-1{{in instantiation of function template specialization 'foo5' requested here}}
+
+struct ExampleTypes {
+  explicit ExampleTypes(int);
+  using T = int;
+  using U = float;
+};
+
+template struct X {
+  int i=0;
+  int j=0;
+  constexpr operator bool() const { return false; }
+};
+
+template void foo6() {
+static_assert(X());
+// expected-error@-1{{static_assert failed due to requirement 'X()'}}
+static_assert(X{});
+// expected-error@-1{{static_assert failed due to requirement 'X{}'}}
+static_assert(X{1,2});
+// expected-error@-1{{static_assert failed due to requirement 'X{1, 2}'}}
+static_assert(X({1,2}));
+// expected-error@-1{{static_assert failed due to requirement 'X({1, 2})'}}
+static_assert(typename T::T{0});
+// expected-error@-1{{static_assert failed due to requirement 'int{0}'}}
+static_assert(typename T::T(0));
+// expected-error@-1{{static_assert failed due to requirement 'int(0)'}}
+static_assert(sizeof(X) == 0);
+// expected-error@-1{{static_assert failed due to requirement 'sizeof(X) == 0'}}
+static_assert((const X*)nullptr);
+// expected-error@-1{{static_assert failed due to requirement '(const X *)nullptr'}}
+static_assert(static_cast*>(nullptr));
+// expected-error@-1{{static_assert failed due to requirement 'static_cast *>(nullptr)'}}
+static_assert((const X[]){} == nullptr);
+// expected-error@-1{{static_assert failed due to requirement '(X const[0]){} == nullptr'}}
+static_assert(sizeof(X().X::~X())>) == 0);
+// expected-error@-1{{static_assert failed due to requirement 'sizeof(X) == 0'}}
+}
+template void foo6();
+// expected-note@-1{{in instantiation of function template specialization 'foo6' requested here}}
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -3059,8 +3059,9 @@
 // for actual types.
 class FailedBooleanConditionPrinterHelper : public PrinterHelper {
 public:
-  explicit FailedBooleanConditionPrinterHelper(const PrintingPolicy &P)
-  : Policy(P) {}
+  FailedBooleanConditionPrinterHelper(const ASTContext &Context,
+  const PrintingPolicy &P)
+  : Context(Context), Policy(P) {}
 
   bool handledStmt(Stmt *E, raw_ostream &OS) override {
 const auto *DR = dyn_cast(E);
@@ -3081,6 +3082,7 @@
   }
 
 private:
+  const ASTContext &Context;
   const PrintingPolicy Policy;
 };
 
@@ -312

[PATCH] D55552: [Sema] Better static assert diagnostics for expressions involving temporaries.

[Clement Courbet via Phabricator via cfe-commits]

courbet updated this revision to Diff 178011.
courbet added a comment.

clang-format patch


Repository:
  rC Clang

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

https://reviews.llvm.org/D2

Files:
  include/clang/AST/Stmt.h
  lib/AST/StmtPrinter.cpp
  lib/Sema/SemaTemplate.cpp
  test/SemaCXX/static-assert-cxx17.cpp
  test/SemaCXX/static-assert.cpp

Index: test/SemaCXX/static-assert.cpp
===
--- test/SemaCXX/static-assert.cpp
+++ test/SemaCXX/static-assert.cpp
@@ -76,6 +76,8 @@
   static const Tp value = v;
   typedef Tp value_type;
   typedef integral_constant type;
+  constexpr operator value_type() const noexcept { return value; }
+  constexpr value_type operator()() const noexcept { return value; }
 };
 
 template 
@@ -103,6 +105,7 @@
 } // namespace std
 
 struct ExampleTypes {
+  explicit ExampleTypes(int);
   using T = int;
   using U = float;
 };
@@ -119,6 +122,18 @@
 // expected-error@-1{{static_assert failed due to requirement 'std::is_const::value == false' "message"}}
 static_assert(!(std::is_const::value == true), "message");
 // expected-error@-1{{static_assert failed due to requirement '!(std::is_const::value == true)' "message"}}
+static_assert(std::is_const(), "message");
+// expected-error@-1{{static_assert failed due to requirement 'std::is_const()' "message"}}
+static_assert(!(std::is_const()()), "message");
+// expected-error@-1{{static_assert failed due to requirement '!(std::is_const()())' "message"}}
+static_assert(std::is_same()), int>::value, "message");
+// expected-error@-1{{static_assert failed due to requirement 'std::is_same, int>::value' "message"}}
+static_assert(std::is_const::value, "message");
+// expected-error@-1{{static_assert failed due to requirement 'std::is_const::value' "message"}}
+static_assert(std::is_const::value, "message");
+// expected-error@-1{{static_assert failed due to requirement 'std::is_const::value' "message"}}
+static_assert(std::is_const::value, "message");
+// expected-error@-1{{static_assert failed due to requirement 'std::is_const::value' "message"}}
 
 struct BI_tag {};
 struct RAI_tag : BI_tag {};
Index: test/SemaCXX/static-assert-cxx17.cpp
===
--- test/SemaCXX/static-assert-cxx17.cpp
+++ test/SemaCXX/static-assert-cxx17.cpp
@@ -54,3 +54,44 @@
 }
 template void foo5();
 // expected-note@-1{{in instantiation of function template specialization 'foo5' requested here}}
+
+struct ExampleTypes {
+  explicit ExampleTypes(int);
+  using T = int;
+  using U = float;
+};
+
+template 
+struct X {
+  int i = 0;
+  int j = 0;
+  constexpr operator bool() const { return false; }
+};
+
+template 
+void foo6() {
+  static_assert(X());
+  // expected-error@-1{{static_assert failed due to requirement 'X()'}}
+  static_assert(X{});
+  // expected-error@-1{{static_assert failed due to requirement 'X{}'}}
+  static_assert(X{1, 2});
+  // expected-error@-1{{static_assert failed due to requirement 'X{1, 2}'}}
+  static_assert(X({1, 2}));
+  // expected-error@-1{{static_assert failed due to requirement 'X({1, 2})'}}
+  static_assert(typename T::T{0});
+  // expected-error@-1{{static_assert failed due to requirement 'int{0}'}}
+  static_assert(typename T::T(0));
+  // expected-error@-1{{static_assert failed due to requirement 'int(0)'}}
+  static_assert(sizeof(X) == 0);
+  // expected-error@-1{{static_assert failed due to requirement 'sizeof(X) == 0'}}
+  static_assert((const X *)nullptr);
+  // expected-error@-1{{static_assert failed due to requirement '(const X *)nullptr'}}
+  static_assert(static_cast *>(nullptr));
+  // expected-error@-1{{static_assert failed due to requirement 'static_cast *>(nullptr)'}}
+  static_assert((const X[]){} == nullptr);
+  // expected-error@-1{{static_assert failed due to requirement '(X const[0]){} == nullptr'}}
+  static_assert(sizeof(X().X::~X())>) == 0);
+  // expected-error@-1{{static_assert failed due to requirement 'sizeof(X) == 0'}}
+}
+template void foo6();
+// expected-note@-1{{in instantiation of function template specialization 'foo6' requested here}}
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -3059,8 +3059,9 @@
 // for actual types.
 class FailedBooleanConditionPrinterHelper : public PrinterHelper {
 public:
-  explicit FailedBooleanConditionPrinterHelper(const PrintingPolicy &P)
-  : Policy(P) {}
+  FailedBooleanConditionPrinterHelper(const ASTContext &Context,
+  const PrintingPolicy &P)
+  : Context(Context), Policy(P) {}
 
   bool handledStmt(Stmt *E, raw_ostream &OS) override {
 const auto *DR = dyn_cast(E);
@@ -3081,6 +3082,7 @@
   }
 
 private:
+  const ASTContext &Context;
   const PrintingPolicy Policy;
 };
 
@@ -3122,8 +3124,9 @@
   std::string Description;
   {
 llvm::raw_string_ostream Out

[PATCH] D55552: [Sema] Better static assert diagnostics for expressions involving temporaries.

[Clement Courbet via Phabricator via cfe-commits]

courbet updated this revision to Diff 178012.
courbet added a comment.

revert now unneeded changes.


Repository:
  rC Clang

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

https://reviews.llvm.org/D2

Files:
  include/clang/AST/Stmt.h
  lib/AST/StmtPrinter.cpp
  lib/Sema/SemaTemplate.cpp
  test/SemaCXX/static-assert-cxx17.cpp
  test/SemaCXX/static-assert.cpp

Index: test/SemaCXX/static-assert.cpp
===
--- test/SemaCXX/static-assert.cpp
+++ test/SemaCXX/static-assert.cpp
@@ -76,6 +76,8 @@
   static const Tp value = v;
   typedef Tp value_type;
   typedef integral_constant type;
+  constexpr operator value_type() const noexcept { return value; }
+  constexpr value_type operator()() const noexcept { return value; }
 };
 
 template 
@@ -103,6 +105,7 @@
 } // namespace std
 
 struct ExampleTypes {
+  explicit ExampleTypes(int);
   using T = int;
   using U = float;
 };
@@ -119,6 +122,18 @@
 // expected-error@-1{{static_assert failed due to requirement 'std::is_const::value == false' "message"}}
 static_assert(!(std::is_const::value == true), "message");
 // expected-error@-1{{static_assert failed due to requirement '!(std::is_const::value == true)' "message"}}
+static_assert(std::is_const(), "message");
+// expected-error@-1{{static_assert failed due to requirement 'std::is_const()' "message"}}
+static_assert(!(std::is_const()()), "message");
+// expected-error@-1{{static_assert failed due to requirement '!(std::is_const()())' "message"}}
+static_assert(std::is_same()), int>::value, "message");
+// expected-error@-1{{static_assert failed due to requirement 'std::is_same, int>::value' "message"}}
+static_assert(std::is_const::value, "message");
+// expected-error@-1{{static_assert failed due to requirement 'std::is_const::value' "message"}}
+static_assert(std::is_const::value, "message");
+// expected-error@-1{{static_assert failed due to requirement 'std::is_const::value' "message"}}
+static_assert(std::is_const::value, "message");
+// expected-error@-1{{static_assert failed due to requirement 'std::is_const::value' "message"}}
 
 struct BI_tag {};
 struct RAI_tag : BI_tag {};
Index: test/SemaCXX/static-assert-cxx17.cpp
===
--- test/SemaCXX/static-assert-cxx17.cpp
+++ test/SemaCXX/static-assert-cxx17.cpp
@@ -54,3 +54,44 @@
 }
 template void foo5();
 // expected-note@-1{{in instantiation of function template specialization 'foo5' requested here}}
+
+struct ExampleTypes {
+  explicit ExampleTypes(int);
+  using T = int;
+  using U = float;
+};
+
+template 
+struct X {
+  int i = 0;
+  int j = 0;
+  constexpr operator bool() const { return false; }
+};
+
+template 
+void foo6() {
+  static_assert(X());
+  // expected-error@-1{{static_assert failed due to requirement 'X()'}}
+  static_assert(X{});
+  // expected-error@-1{{static_assert failed due to requirement 'X{}'}}
+  static_assert(X{1, 2});
+  // expected-error@-1{{static_assert failed due to requirement 'X{1, 2}'}}
+  static_assert(X({1, 2}));
+  // expected-error@-1{{static_assert failed due to requirement 'X({1, 2})'}}
+  static_assert(typename T::T{0});
+  // expected-error@-1{{static_assert failed due to requirement 'int{0}'}}
+  static_assert(typename T::T(0));
+  // expected-error@-1{{static_assert failed due to requirement 'int(0)'}}
+  static_assert(sizeof(X) == 0);
+  // expected-error@-1{{static_assert failed due to requirement 'sizeof(X) == 0'}}
+  static_assert((const X *)nullptr);
+  // expected-error@-1{{static_assert failed due to requirement '(const X *)nullptr'}}
+  static_assert(static_cast *>(nullptr));
+  // expected-error@-1{{static_assert failed due to requirement 'static_cast *>(nullptr)'}}
+  static_assert((const X[]){} == nullptr);
+  // expected-error@-1{{static_assert failed due to requirement '(X const[0]){} == nullptr'}}
+  static_assert(sizeof(X().X::~X())>) == 0);
+  // expected-error@-1{{static_assert failed due to requirement 'sizeof(X) == 0'}}
+}
+template void foo6();
+// expected-note@-1{{in instantiation of function template specialization 'foo6' requested here}}
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -3059,8 +3059,7 @@
 // for actual types.
 class FailedBooleanConditionPrinterHelper : public PrinterHelper {
 public:
-  explicit FailedBooleanConditionPrinterHelper(const PrintingPolicy &P)
-  : Policy(P) {}
+  FailedBooleanConditionPrinterHelper(const PrintingPolicy &P) : Policy(P) {}
 
   bool handledStmt(Stmt *E, raw_ostream &OS) override {
 const auto *DR = dyn_cast(E);
@@ -3123,7 +3122,8 @@
   {
 llvm::raw_string_ostream Out(Description);
 FailedBooleanConditionPrinterHelper Helper(getPrintingPolicy());
-FailedCond->printPretty(Out, &Helper, getPrintingPolicy());
+FailedCond->printPretty(Out, &Helper, getPrintingPolicy(), 0,

[PATCH] D55229: [COFF] Statically link certain runtime library functions

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

If we're going to change the default, please at least add a flag to allow 
callers to opt into dlimport.  We can then make that dependent on 
`-static-objc` or similar.




Comment at: CodeGenObjC/gnu-init.m:103
+// Make sure we do not have dllimport on the load function
+// CHECK-WIN: declare dso_local void @__objc_load
 

compnerd wrote:
> rnk wrote:
> > compnerd wrote:
> > > rnk wrote:
> > > > mgrang wrote:
> > > > > I had to remove dllimport in this and the next unit test. I am not 
> > > > > sure of the wider implications of this change. Maybe controlling this 
> > > > > via a flag (like -flto-visibility-public-std) is a better/more 
> > > > > localized option?
> > > > These are the ones that I think @compnerd really cares about since the 
> > > > objc runtime is typically dynamically linked and frequently called. We 
> > > > might want to arrange things such that we have a separate codepath for 
> > > > ObjC runtime helpers. I'm surprised we don't already have such a code 
> > > > path.
> > > I think that @theraven would care more about this code path than I.  I 
> > > think that this change may be wrong, because the load here is supposed to 
> > > be in the ObjC runtime, and this becoming local to the module would break 
> > > the global registration.
> > We just set dso_local whenever something isn't dllimport, even if it's a 
> > lie sometimes. I think everything would work as intended with a linker 
> > thunk. It's "true" as far as LLVM is concerned for the purposes of emitting 
> > relocations.
> Ah, okay, then, this might be okay.  However, the use of `dllimport` here 
> would force that the runtime to be called.  Then again it is possible to 
> statically link ...
`__objc_load` is a function defined in objc.dll.  It absolutely does want to be 
dlimport, or the linker won't be able to find it.



Comment at: CodeGenObjCXX/msabi-stret.mm:16
 
-// CHECK: declare dllimport void @objc_msgSend_stret(i8*, i8*, ...)
+// CHECK: declare dso_local void @objc_msgSend_stret(i8*, i8*, ...)
 // CHECK-NOT: declare dllimport void @objc_msgSend(i8*, i8*, ...)

Similarly, the `objc_msgSend` family are defined in objc.dll and so need to be 
`dlimport`ed.


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

https://reviews.llvm.org/D55229



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

[PATCH] D55413: [ExprConstant] Handle compound assignment when LHS has integral type and RHS has floating point type

[S. B. Tam via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/ExprConstant.cpp:3441
+  Value);
+}
 APSInt LHS = HandleIntToIntCast(Info, E, PromotedLHSType,

rjmccall wrote:
> cpplearner wrote:
> > rjmccall wrote:
> > > Can we more fundamentally restructure this entire handler so that, if the 
> > > compound assignment's computation result type is an arithmetic type, we 
> > > just promote both operands to that type, do the arithmetic there, and 
> > > then coerce back down?  This is C++, so the LHS type imposes almost no 
> > > restrictions on the RHS type; also, this is Clang, where we support way 
> > > too many arithmetic types for our own good.
> > It seems the conditional statement is unavoidable, because `APSInt` and 
> > `APFloat` can't be handled at the same time (i.e. you need to choose among 
> > `Handle{Int,Float}To{Int,Float}Cast`, and between 
> > `handleIntIntBinOp`/`handleFloatFloatBinOp`). Maybe it's possible to add a 
> > layer that can accept both `APSInt` and `APFloat`, but it seems like an 
> > overkill if it's only used in the compound assignment case.
> But we can have `HandleValueTo{Int,Float}Cast` functions that start with an 
> arbitrary `APValue` and do the switch on that type, and we can have a 
> `HandleValueValueBinOp` function that asserts that its operands have the same 
> type and does the switch, and those two together should be good enough for 
> what we need here.
That's what I mean by "add a layer", and I think it's unworthy.

Maybe it makes more sense as part of a larger scale rewrite, but that's more 
than I can deal with.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55413



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

[PATCH] D55640: [clang-tidy] Implement a check for large Objective-C type encodings 🔍

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

I wonder if we want to have an option to elide ObjC type info for all non-POD 
C++ types.  Nothing that you do with the type encoding is likely to be correct 
(for example, you can see the pointer field in a `std::shared_ptr`, but you 
can't see that changes to it need to update reference counts) so it probably 
does more harm than good.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55640



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

[PATCH] D55359: [clangd] Avoid emitting Queued status when we are able to acquire the Barrier.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55359



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

[PATCH] D55646: [ASTImporter] Make ODR diagnostics warning by default

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 created this revision.
Herald added subscribers: cfe-commits, Szelethus, martong, dkrupp.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.

- Make ODR diagnostics warning by default

After this change all odr-related diagnostics are considered warnings belonging
to Diagnostic Group "OneDefinitionRule". They can be propagated to error level
with '-Werror=odr'.

- Remove unused ErrorOnTagTypeMismatch member


Repository:
  rC Clang

https://reviews.llvm.org/D55646

Files:
  include/clang/AST/ASTStructuralEquivalence.h
  include/clang/Basic/DiagnosticASTKinds.td
  include/clang/Basic/DiagnosticGroups.td
  lib/AST/ASTImporter.cpp
  lib/AST/ASTStructuralEquivalence.cpp
  lib/Sema/SemaType.cpp
  test/ASTMerge/category/test.m
  test/ASTMerge/class-template-partial-spec/test.cpp
  test/ASTMerge/class-template/test.cpp
  test/ASTMerge/enum/test.c
  test/ASTMerge/function/test.c
  test/ASTMerge/interface/test.m
  test/ASTMerge/namespace/test.cpp
  test/ASTMerge/property/test.m
  test/ASTMerge/struct/test.c
  test/ASTMerge/typedef/test.c
  test/ASTMerge/var/test.c
  test/Modules/elaborated-type-specifier-from-hidden-module.m
  test/Modules/redefinition-c-tagtypes.m

Index: test/Modules/redefinition-c-tagtypes.m
===
--- test/Modules/redefinition-c-tagtypes.m
+++ test/Modules/redefinition-c-tagtypes.m
@@ -15,7 +15,7 @@
   int b;
 #else
   int c; // expected-note {{field has name 'c' here}}
-  // expected-error@redefinition-c-tagtypes.m:12 {{type 'struct NS' has incompatible definitions}}
+  // expected-warning@redefinition-c-tagtypes.m:12 {{type 'struct NS' has incompatible definitions}}
   // expected-note@Inputs/F.framework/PrivateHeaders/NS.h:3 {{field has name 'b' here}}
 #endif
 };
@@ -26,7 +26,7 @@
   SND = 43,
 #else
   SND = 44, // expected-note {{enumerator 'SND' with value 44 here}}
-  // expected-error@redefinition-c-tagtypes.m:23 {{type 'enum NSE' has incompatible definitions}}
+  // expected-warning@redefinition-c-tagtypes.m:23 {{type 'enum NSE' has incompatible definitions}}
   // expected-note@Inputs/F.framework/PrivateHeaders/NS.h:8 {{enumerator 'SND' with value 43 here}}
 #endif
   TRD = 55
@@ -42,7 +42,7 @@
   MinXOther = MinX,
 #else
   MinXOther = TRD, // expected-note {{enumerator 'MinXOther' with value 55 here}}
-  // expected-error@redefinition-c-tagtypes.m:39 {{type 'enum NSMyEnum' has incompatible definitions}}
+  // expected-warning@redefinition-c-tagtypes.m:39 {{type 'enum NSMyEnum' has incompatible definitions}}
   // expected-note@Inputs/F.framework/PrivateHeaders/NS.h:18 {{enumerator 'MinXOther' with value 11 here}}
 #endif
 };
Index: test/Modules/elaborated-type-specifier-from-hidden-module.m
===
--- test/Modules/elaborated-type-specifier-from-hidden-module.m
+++ test/Modules/elaborated-type-specifier-from-hidden-module.m
@@ -7,7 +7,7 @@
 struct S2 { int x; };
 struct S3 *z;
 // Incompatible definition.
-struct S3 { float y; }; // expected-error {{has incompatible definitions}} // expected-note {{field has name}}
+struct S3 { float y; }; // expected-warning {{has incompatible definitions}} // expected-note {{field has name}}
 // expected-note@Inputs/elaborated-type-structs.h:3 {{field has name}}
 
 @import ElaboratedTypeStructs.Structs;
Index: test/ASTMerge/var/test.c
===
--- test/ASTMerge/var/test.c
+++ test/ASTMerge/var/test.c
@@ -1,12 +1,12 @@
 // RUN: %clang_cc1 -emit-pch -o %t.1.ast %S/Inputs/var1.c
 // RUN: %clang_cc1 -emit-pch -o %t.2.ast %S/Inputs/var2.c
-// RUN: not %clang_cc1 -ast-merge %t.1.ast -ast-merge %t.2.ast -fsyntax-only -fdiagnostics-show-note-include-stack %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -ast-merge %t.1.ast -ast-merge %t.2.ast -fsyntax-only -fdiagnostics-show-note-include-stack %s 2>&1 | FileCheck %s
 
-// CHECK: var2.c:2:9: error: external variable 'x1' declared with incompatible types in different translation units ('double *' vs. 'float **')
+// CHECK: var2.c:2:9: warning: external variable 'x1' declared with incompatible types in different translation units ('double *' vs. 'float **')
 // CHECK: var1.c:2:9: note: declared here with type 'float **'
-// CHECK: var2.c:3:5: error: external variable 'x2' declared with incompatible types in different translation units ('int' vs. 'double')
+// CHECK: var2.c:3:5: warning: external variable 'x2' declared with incompatible types in different translation units ('int' vs. 'double')
 // CHECK: In file included from{{.*}}var1.c:3:
 // CHECK: var1.h:1:8: note: declared here with type 'double'
-// CHECK: error: external variable 'xarray3' declared with incompatible types in different translation units ('int [17]' vs. 'int [18]')
+// CHECK: warning: external variable 'xarray3' declared with incompatible types in different translation units ('int [17]' vs. 'int [18]')
 // CHECK: var1.c:7:5: note: declare

[PATCH] D55483: Introduce the callback attribute and emit !callback metadata

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: include/clang/Basic/Builtins.def:942
 LIBBUILTIN(alloca, "v*z", "f", "stdlib.h", ALL_GNU_LANGUAGES)
+LIBBUILTIN(qsort_r, "",   "fC<3,-1,-1,4>", "stdlib.h", 
ALL_GNU_LANGUAGES)
 // POSIX string.h

qsort_r callback argument order is different on Linux, macOS and FreeBSD so 
those constants can't be hardcoded: 
Linux:
`void qsort_r(void *base, size_t nmemb, size_t size, int (*compar)(const void 
*, const void *, void *), void *arg);`
FreeBSD:
`void qsort_r(void *base, size_t nmemb, size_t size, void *thunk, int 
(*compar)(void *, const void *, const void*));`
macos:
`void qsort_r(void *base, size_t nel, size_t width, void *thunk, int 
(*compar)(void *, const void *, const void *));`



Comment at: test/CodeGen/callback_qsort_r.c:1
+// RUN: %clang %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -D_GNU_SOURCE %s -S -c -emit-llvm -o - -O1 | FileCheck %s 
--check-prefix=IR

This should use a linux triple otherwise the qsort_r declaration is wrong. 
Ideally it should also handle macos+FreeBSD with the inverted argument order.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55483



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

[PATCH] D55646: [ASTImporter] Make ODR diagnostics warning by default

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 added a comment.

In order to be able to handle ODR-related diagnostics with command line 
options, these diagnostics were moved from Error category to Warning. What are 
your thoughts?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55646



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

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: lib/Frontend/PrecompiledPreamble.cpp:459
+if (moveOnNoError(VFS->status(RB.first), Status)) {
+  OverriddenFiles[Status.getUniqueID()] = PreambleHash;
+} else {

NIT: remove braces around single-statement branches, they are usually omitted 
in the LLVM code


Repository:
  rC Clang

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

https://reviews.llvm.org/D41005



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

r349015 - [asan] Don't check ODR violations for particular types of globals

[Vitaly Buka via cfe-commits]

Author: vitalybuka
Date: Thu Dec 13 01:47:39 2018
New Revision: 349015

URL: http://llvm.org/viewvc/llvm-project?rev=349015&view=rev
Log:
[asan] Don't check ODR violations for particular types of globals

Summary:
private and internal: should not trigger ODR at all.
unnamed_addr: current ODR checking approach fail and rereport false violation if
a linker merges such globals
linkonce_odr, weak_odr: could cause similar problems and they are already not
instrumented for ELF.

Reviewers: eugenis, kcc

Subscribers: kubamracek, hiraditya, llvm-commits

Differential Revision: https://reviews.llvm.org/D55621

Added:
cfe/trunk/test/CodeGen/asan-static-odr.cpp

Added: cfe/trunk/test/CodeGen/asan-static-odr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/asan-static-odr.cpp?rev=349015&view=auto
==
--- cfe/trunk/test/CodeGen/asan-static-odr.cpp (added)
+++ cfe/trunk/test/CodeGen/asan-static-odr.cpp Thu Dec 13 01:47:39 2018
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - -triple x86_64-linux %s 
| FileCheck %s --check-prefixes=CHECK,ALIAS1
+
+// No alias on Windows but indicators should work.
+// RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - -triple 
x86_64-windows-msvc %s | FileCheck %s --check-prefixes=CHECK,ALIAS0
+
+static int global;
+
+int main() {
+  return global;
+}
+
+// CHECK-NOT: __odr_asan_gen
+// CHECK-NOT: private alias
+// CHECK: [[VAR:@.*global.*]] ={{.*}} global { i32, [60 x i8] } 
zeroinitializer, align 32
+// CHECK: @0 = internal global {{.*}} [[VAR]] to i64), {{.*}}, i64 -1 }]
+// CHECK: call void @__asan_register_globals(i64 ptrtoint ([1 x { i64, i64, 
i64, i64, i64, i64, i64, i64 }]* @0 to i64), i64 1)
+// CHECK: call void @__asan_unregister_globals(i64 ptrtoint ([1 x { i64, i64, 
i64, i64, i64, i64, i64, i64 }]* @0 to i64), i64 1)


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

[PATCH] D55621: [asan] Don't check ODR violations for particular types of globals

[Vitaly Buka via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC349015: [asan] Don't check ODR violations for 
particular types of globals (authored by vitalybuka, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55621?vs=177947&id=178019#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D55621

Files:
  test/CodeGen/asan-static-odr.cpp


Index: test/CodeGen/asan-static-odr.cpp
===
--- test/CodeGen/asan-static-odr.cpp
+++ test/CodeGen/asan-static-odr.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - -triple x86_64-linux %s 
| FileCheck %s --check-prefixes=CHECK,ALIAS1
+
+// No alias on Windows but indicators should work.
+// RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - -triple 
x86_64-windows-msvc %s | FileCheck %s --check-prefixes=CHECK,ALIAS0
+
+static int global;
+
+int main() {
+  return global;
+}
+
+// CHECK-NOT: __odr_asan_gen
+// CHECK-NOT: private alias
+// CHECK: [[VAR:@.*global.*]] ={{.*}} global { i32, [60 x i8] } 
zeroinitializer, align 32
+// CHECK: @0 = internal global {{.*}} [[VAR]] to i64), {{.*}}, i64 -1 }]
+// CHECK: call void @__asan_register_globals(i64 ptrtoint ([1 x { i64, i64, 
i64, i64, i64, i64, i64, i64 }]* @0 to i64), i64 1)
+// CHECK: call void @__asan_unregister_globals(i64 ptrtoint ([1 x { i64, i64, 
i64, i64, i64, i64, i64, i64 }]* @0 to i64), i64 1)


Index: test/CodeGen/asan-static-odr.cpp
===
--- test/CodeGen/asan-static-odr.cpp
+++ test/CodeGen/asan-static-odr.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - -triple x86_64-linux %s | FileCheck %s --check-prefixes=CHECK,ALIAS1
+
+// No alias on Windows but indicators should work.
+// RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - -triple x86_64-windows-msvc %s | FileCheck %s --check-prefixes=CHECK,ALIAS0
+
+static int global;
+
+int main() {
+  return global;
+}
+
+// CHECK-NOT: __odr_asan_gen
+// CHECK-NOT: private alias
+// CHECK: [[VAR:@.*global.*]] ={{.*}} global { i32, [60 x i8] } zeroinitializer, align 32
+// CHECK: @0 = internal global {{.*}} [[VAR]] to i64), {{.*}}, i64 -1 }]
+// CHECK: call void @__asan_register_globals(i64 ptrtoint ([1 x { i64, i64, i64, i64, i64, i64, i64, i64 }]* @0 to i64), i64 1)
+// CHECK: call void @__asan_unregister_globals(i64 ptrtoint ([1 x { i64, i64, i64, i64, i64, i64, i64, i64 }]* @0 to i64), i64 1)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54438: [analyzer] Reimplement dependencies between checkers

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Hmmm, I left plenty of room for improvement in the tblgen generation, we could 
generate compile-time errors on cycles within the dependency graph, try to 
include the generated file only once, but these clearly are very low-prio 
issues, so I'll do it later. I'll have to revisit the entire thing soon enough.


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

https://reviews.llvm.org/D54438



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

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D48116#1322878 , @nik wrote:

> In D48116#1144732 , @ilya-biryukov 
> wrote:
>
> > Have you considered doing the same filtering in ASTUnit's 
> > `StoredDiagnosticConsumer`? It should not be more difficult and allows to 
> > avoid changing the clang's diagnostic interfaces. That's what we do in 
> > clangd.
>
>
> Hmm, it's a bit strange that StoredDiagnosticConsumer should also start to 
> filter things.


I

> For filtering in StoredDiagnosticConsumer one needs to pass the new bool 
> everywhere along where "bool CaptureDiagnostics" is already passed on (to end 
> up in the StoredDiagnosticConsumer constructor) . Also, 
> ASTUnit::CaptureDiagnostics is then not enough anymore since the new bool is 
> also needed in getMainBufferWithPrecompiledPreamble(). One could also (2) 
> convert  "bool CaptureDiagnostics" to an enum with enumerators like 
> CaptureNothing, CaptureAll, CaptureAllWithoutNonErrorsFromIncludes to make 
> this a bit less invasive.
>  If changing clang's diagnostic interface should be avoided, I tend to go 
> with (2). Ilya?

Yeah, LG. The changes in the `ASTUnit` look strictly better than changes in 
clang - the latter seems to already provide enough to do the filtering.
If you avoid changing the `StoredDiagnosticConsumer` (or writing a filtering 
wrapper for `DiagnosticConsumer`), you'll end up having some diagnostics inside 
headers generated **after** preamble was built, right?
That might be either ok or not, depending on the use-case.


Repository:
  rC Clang

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

https://reviews.llvm.org/D48116



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

r349019 - [OpenCL] Add generic AS to 'this' pointer

[Mikael Nilsson via cfe-commits]

Author: mikael
Date: Thu Dec 13 02:15:27 2018
New Revision: 349019

URL: http://llvm.org/viewvc/llvm-project?rev=349019&view=rev
Log:
[OpenCL] Add generic AS to 'this' pointer

Address spaces are cast into generic before invoking the constructor.

Added support for a trailing Qualifiers object in FunctionProtoType.

Note: This recommits the previously reverted patch, 
  but now it is commited together with a fix for lldb.

Differential Revision: https://reviews.llvm.org/D54862

Added:
cfe/trunk/test/CodeGenOpenCLCXX/addrspace-of-this.cl
Modified:
cfe/trunk/include/clang/AST/CanonicalType.h
cfe/trunk/include/clang/AST/DeclCXX.h
cfe/trunk/include/clang/AST/Type.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/AST/ASTDumper.cpp
cfe/trunk/lib/AST/DeclCXX.cpp
cfe/trunk/lib/AST/ItaniumMangle.cpp
cfe/trunk/lib/AST/MicrosoftMangle.cpp
cfe/trunk/lib/AST/Type.cpp
cfe/trunk/lib/AST/TypePrinter.cpp
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/CGClass.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CGValue.h
cfe/trunk/lib/Index/USRGeneration.cpp
cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp
cfe/trunk/lib/Parse/ParseDecl.cpp
cfe/trunk/lib/Parse/ParseOpenMP.cpp
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Sema/SemaLambda.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/lib/Sema/SemaTemplate.cpp
cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp
cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/lib/Sema/TreeTransform.h
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp
cfe/trunk/test/CodeGenOpenCLCXX/template-address-spaces.cl
cfe/trunk/test/SemaOpenCLCXX/address-space-templates.cl
cfe/trunk/tools/libclang/CIndex.cpp

Modified: cfe/trunk/include/clang/AST/CanonicalType.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/CanonicalType.h?rev=349019&r1=349018&r2=349019&view=diff
==
--- cfe/trunk/include/clang/AST/CanonicalType.h (original)
+++ cfe/trunk/include/clang/AST/CanonicalType.h Thu Dec 13 02:15:27 2018
@@ -510,7 +510,7 @@ struct CanProxyAdaptor;

Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=349019&r1=349018&r2=349019&view=diff
==
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Thu Dec 13 02:15:27 2018
@@ -2182,7 +2182,10 @@ public:
   /// 'this' type.
   QualType getThisType(ASTContext &C) const;
 
-  unsigned getTypeQualifiers() const {
+  static QualType getThisType(const FunctionProtoType *FPT,
+  const CXXRecordDecl *Decl);
+
+  Qualifiers getTypeQualifiers() const {
 return getType()->getAs()->getTypeQuals();
   }
 

Modified: cfe/trunk/include/clang/AST/Type.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=349019&r1=349018&r2=349019&view=diff
==
--- cfe/trunk/include/clang/AST/Type.h (original)
+++ cfe/trunk/include/clang/AST/Type.h Thu Dec 13 02:15:27 2018
@@ -256,28 +256,24 @@ public:
   }
 
   bool hasConst() const { return Mask & Const; }
-  void setConst(bool flag) {
-Mask = (Mask & ~Const) | (flag ? Const : 0);
-  }
+  bool hasOnlyConst() const { return Mask == Const; }
   void removeConst() { Mask &= ~Const; }
   void addConst() { Mask |= Const; }
 
   bool hasVolatile() const { return Mask & Volatile; }
-  void setVolatile(bool flag) {
-Mask = (Mask & ~Volatile) | (flag ? Volatile : 0);
-  }
+  bool hasOnlyVolatile() const { return Mask == Volatile; }
   void removeVolatile() { Mask &= ~Volatile; }
   void addVolatile() { Mask |= Volatile; }
 
   bool hasRestrict() const { return Mask & Restrict; }
-  void setRestrict(bool flag) {
-Mask = (Mask & ~Restrict) | (flag ? Restrict : 0);
-  }
+  bool hasOnlyRestrict() const { return Mask == Restrict; }
   void removeRestrict() { Mask &= ~Restrict; }
   void addRestrict() { Mask |= Restrict; }
 
   bool hasCVRQualifiers() const { return getCVRQualifiers(); }
   unsigned getCVRQualifiers() const { return Mask & CVRMask; }
+  unsigned getCVRUQualifiers() const { return Mask & (CVRMask | UMask); }
+
   void setCVRQualifiers(unsigned mask) {
 assert(!(mask & ~CVRMask) && "bitmask contains non-CVR bits");
 Mask = (Mask & ~CVRMask) | mask;
@@ -1526,7 +1522,9 @@

[PATCH] D55191: [clangd] Refine the way of checking a declaration is referenced by the written code.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM. Many thanks for the fix, really important to get those cases right.




Comment at: unittests/clangd/XRefsTests.cpp:1228
 
+TEST(FindReferences, ExplicitSymbols) {
+  const char *Tests[] = {

hokein wrote:
> ilya-biryukov wrote:
> > I'm missing what does this test actually tests.
> > The absence of implicit references (I guess constructor expressions)?
> This test tests the cases where symbol is being marked implicit incorrectly, 
> which will result in no result in xref.
Thanks for clarification! The tested cases were so simple that I though they 
worked before.
(Which shows this change is very important, IMHO)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55191



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

[PATCH] D55455: Remove stat cache chaining as it's no longer needed after PTH support has been removed

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Yay! Anything that makes `FileManager` and friends simpler LG


Repository:
  rC Clang

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

https://reviews.llvm.org/D55455



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

[PATCH] D55245: [clang-tidy] Add the abseil-duration-subtraction check

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Did you rebase the check onto the new master with your refactorings in?




Comment at: test/clang-tidy/abseil-duration-subtraction.cpp:12
+  // CHECK-FIXES: absl::ToDoubleSeconds(d - absl::Seconds(1))
+  x = absl::ToDoubleSeconds(d) - absl::ToDoubleSeconds(d1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the 
duration domain [abseil-duration-subtraction]

hwright wrote:
> JonasToth wrote:
> > hwright wrote:
> > > JonasToth wrote:
> > > > hwright wrote:
> > > > > JonasToth wrote:
> > > > > > From this example starting:
> > > > > > 
> > > > > > - The RHS should be a nested expression with function calls, as the 
> > > > > > RHS is transformed to create the adversary example i mean in the 
> > > > > > transformation function above.
> > > > > > 
> > > > > > ```
> > > > > > absl::ToDoubleSeconds(d) - 
> > > > > > absl::ToDoubleSeconds(absl::ToDoubleSeconds(d) - 
> > > > > > absl::ToDoubleSeconds(d1));
> > > > > > ```
> > > > > > I think you need the proper conversion function, as the result of 
> > > > > > the expression is `double` and you need a `Duration`, right?
> > > > > > 
> > > > > > But in principle starting from this idea the transformation might 
> > > > > > break.
> > > > > I think there may be some confusion here (and that's entirely my 
> > > > > fault. :) )
> > > > > 
> > > > > We should never get this expression as input to the check, since it 
> > > > > doesn't compile (as you point out):
> > > > > ```
> > > > > absl::ToDoubleSeconds(d) - 
> > > > > absl::ToDoubleSeconds(absl::ToDoubleSeconds(d) - 
> > > > > absl::ToDoubleSeconds(d1));
> > > > > ```
> > > > > 
> > > > > Since `absl::ToDoubleSeconds` requires that its argument is an 
> > > > > `absl::Duration`, but the expression `absl::ToDoubleSeconds(d) - 
> > > > > absl::ToDoubleSeconds(d1)` results in a `double`, we can't get this 
> > > > > as input.
> > > > > 
> > > > > There may be other expressions which could be input, but in practice 
> > > > > they don't really happen.  I've added a contrived example to the 
> > > > > tests, but at some point the tests get too complex and confuse the 
> > > > > fix matching infrastructure.
> > > > Your last sentence is the thing ;) Murphies Law will hit this check, 
> > > > too. In my opinion wrong transformations are very unfortunate and 
> > > > should be avoided if possible (in this case possible).
> > > > You can simply require that the expression of type double does not 
> > > > contain any duration subtraction calls.
> > > > 
> > > > This is even possible in the matcher-part of the check.
> > > I've written a test (which the testing infrastructure fails to handle 
> > > well, so I haven't included it in the diff), and it produces these 
> > > results:
> > > 
> > > ```
> > >//
> > >//
> > > -  x = absl::ToDoubleSeconds(d) - (absl::ToDoubleSeconds(d1) - 5);
> > > +  x = absl::ToDoubleSeconds(d - absl::Seconds(absl::ToDoubleSeconds(d1) 
> > > - 5));
> > >//
> > >//
> > > -  x = absl::ToDoubleSeconds(d - absl::Seconds(absl::ToDoubleSeconds(d1) 
> > > - 5));
> > > +  x = absl::ToDoubleSeconds(d - absl::Seconds(absl::ToDoubleSeconds(d1 - 
> > > absl::Seconds(5;
> > > ```
> > > 
> > > Those results are correct.  There is a cosmetic issue of round tripping 
> > > through the `double` conversion in the 
> > > `absl::Seconds(absl::ToDoubleSeconds(...))` phrase, but untangling that 
> > > is 1) difficult (because of order of operations issues) and thus; 2) 
> > > probably the subject of a separate check.
> > > 
> > > This is still such a rare case (as in, we've not encountered it in 
> > > Google's codebase), that I'm not really concerned.  But if it's worth it 
> > > to explicitly exclude it from the traversal matcher, I can do that.
> > Can you say what the direct issue is? I would bet its the overlapping?
> > A note in the documentation would be ok from my side. When the conflicting 
> > transformations are tried to be applied clang-tidy does not crash but print 
> > a nice diagnostic and continue its life?
> Another example I've verified:
> ```
> -  x = absl::ToDoubleSeconds(d) - 
> absl::ToDoubleSeconds(absl::Seconds(absl::ToDoubleSeconds(d1) - 5));
> +  x = absl::ToDoubleSeconds(d - absl::Seconds(absl::ToDoubleSeconds(d1) - 
> 5));
> ```
> 
> This a nested case, and while `clang-tidy` finds both of them, it only 
> applies the outer most one (presumably the one it finds first in its AST 
> traversal):
> ```
> note: this fix will not be applied because it overlaps with another fix
> ```
> 
> The new code can then be checked again to fix the internal instance.
> 
> It's not possible to express this case in a test because testing 
> infrastructure uses regular expressions, and the repeated strings in the test 
> expectation cause it to get a bit confused.
> 
> Given all the of the above, I'm unsure what content would go in the 
> documentation which is specific to this check.
Yes, that should be the 

[PATCH] D55648: [CodeComplete] Fill preferred type on binary expressions

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: kadircet.

Repository:
  rC Clang

https://reviews.llvm.org/D55648

Files:
  include/clang/Sema/Sema.h
  lib/Parse/ParseExpr.cpp
  lib/Sema/SemaCodeComplete.cpp
  unittests/Sema/CodeCompleteTest.cpp

Index: unittests/Sema/CodeCompleteTest.cpp
===
--- unittests/Sema/CodeCompleteTest.cpp
+++ unittests/Sema/CodeCompleteTest.cpp
@@ -14,31 +14,39 @@
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaDiagnostic.h"
 #include "clang/Tooling/Tooling.h"
-#include "gtest/gtest.h"
 #include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+#include 
 
 namespace {
 
 using namespace clang;
 using namespace clang::tooling;
+using ::testing::Each;
 using ::testing::UnorderedElementsAre;
 
 const char TestCCName[] = "test.cc";
-using VisitedContextResults = std::vector;
 
-class VisitedContextFinder: public CodeCompleteConsumer {
+struct CompletionContext {
+  std::vector VisitedNamespaces;
+  std::string PreferredType;
+};
+
+class VisitedContextFinder : public CodeCompleteConsumer {
 public:
-  VisitedContextFinder(VisitedContextResults &Results)
+  VisitedContextFinder(CompletionContext &ResultCtx)
   : CodeCompleteConsumer(/*CodeCompleteOpts=*/{},
  /*CodeCompleteConsumer*/ false),
-VCResults(Results),
+ResultCtx(ResultCtx),
 CCTUInfo(std::make_shared()) {}
 
   void ProcessCodeCompleteResults(Sema &S, CodeCompletionContext Context,
   CodeCompletionResult *Results,
   unsigned NumResults) override {
-VisitedContexts = Context.getVisitedContexts();
-VCResults = getVisitedNamespace();
+ResultCtx.VisitedNamespaces =
+getVisitedNamespace(Context.getVisitedContexts());
+ResultCtx.PreferredType = Context.getPreferredType().getAsString();
   }
 
   CodeCompletionAllocator &getAllocator() override {
@@ -47,7 +55,9 @@
 
   CodeCompletionTUInfo &getCodeCompletionTUInfo() override { return CCTUInfo; }
 
-  std::vector getVisitedNamespace() const {
+private:
+  std::vector getVisitedNamespace(
+  CodeCompletionContext::VisitedContextSet VisitedContexts) const {
 std::vector NSNames;
 for (const auto *Context : VisitedContexts)
   if (const auto *NS = llvm::dyn_cast(Context))
@@ -55,27 +65,25 @@
 return NSNames;
   }
 
-private:
-  VisitedContextResults& VCResults;
+  CompletionContext &ResultCtx;
   CodeCompletionTUInfo CCTUInfo;
-  CodeCompletionContext::VisitedContextSet VisitedContexts;
 };
 
 class CodeCompleteAction : public SyntaxOnlyAction {
 public:
-  CodeCompleteAction(ParsedSourceLocation P, VisitedContextResults &Results)
-  : CompletePosition(std::move(P)), VCResults(Results) {}
+  CodeCompleteAction(ParsedSourceLocation P, CompletionContext &ResultCtx)
+  : CompletePosition(std::move(P)), ResultCtx(ResultCtx) {}
 
   bool BeginInvocation(CompilerInstance &CI) override {
 CI.getFrontendOpts().CodeCompletionAt = CompletePosition;
-CI.setCodeCompletionConsumer(new VisitedContextFinder(VCResults));
+CI.setCodeCompletionConsumer(new VisitedContextFinder(ResultCtx));
 return true;
   }
 
 private:
   // 1-based code complete position ;
   ParsedSourceLocation CompletePosition;
-  VisitedContextResults& VCResults;
+  CompletionContext &ResultCtx;
 };
 
 ParsedSourceLocation offsetToPosition(llvm::StringRef Code, size_t Offset) {
@@ -88,21 +96,49 @@
   static_cast(Offset - StartOfLine + 1)};
 }
 
-VisitedContextResults runCodeCompleteOnCode(StringRef Code) {
-  VisitedContextResults Results;
-  auto TokenOffset = Code.find('^');
-  assert(TokenOffset != StringRef::npos &&
- "Completion token ^ wasn't found in Code.");
-  std::string WithoutToken = Code.take_front(TokenOffset);
-  WithoutToken += Code.drop_front(WithoutToken.size() + 1);
-  assert(StringRef(WithoutToken).find('^') == StringRef::npos &&
- "expected exactly one completion token ^ inside the code");
-
+CompletionContext runCompletion(StringRef Code, size_t Offset) {
+  CompletionContext ResultCtx;
   auto Action = llvm::make_unique(
-  offsetToPosition(WithoutToken, TokenOffset), Results);
+  offsetToPosition(Code, Offset), ResultCtx);
   clang::tooling::runToolOnCodeWithArgs(Action.release(), Code, {"-std=c++11"},
 TestCCName);
-  return Results;
+  return ResultCtx;
+}
+
+struct ParsedAnnotations {
+  std::vector Points;
+  std::string Code;
+};
+
+ParsedAnnotations parseAnnotations(StringRef AnnotatedCode) {
+  ParsedAnnotations R;
+  while (!AnnotatedCode.empty()) {
+size_t NextPoint = AnnotatedCode.find('^');
+if (NextPoint == StringRef::npos) {
+  R.Code += AnnotatedCode;
+  AnnotatedCode = "";
+  break;
+}
+R.Code += AnnotatedCode.substr(0, NextPoint);
+R.Points.push_back(R.Code.size());
+
+AnnotatedCode = AnnotatedCode.su

[PATCH] D55315: [clangd] Only reduce priority of a thread for indexing.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/index/Background.cpp:107
   while (ThreadPoolSize--) {
 ThreadPool.emplace_back([this] { run(); });
   }

NIT: remove braces around a single statement



Comment at: clangd/index/Background.cpp:142
 }
+setCurrentThreadPriority(Priority);
 (*Task)();

NIT: maybe add newlines around the three lines (`setCurrentThreadPriority` and 
Task execution). These are pretty important.
 



Comment at: clangd/index/Background.cpp:142
 }
+setCurrentThreadPriority(Priority);
 (*Task)();

ilya-biryukov wrote:
> NIT: maybe add newlines around the three lines (`setCurrentThreadPriority` 
> and Task execution). These are pretty important.
>  
Maybe only lower the priority iff it's `!= Normal`. Would avoid system calls in 
most cases (I believe you mentioned it in the other comment before too)



Comment at: clangd/index/Background.cpp:206
+auto I = Tasks.end();
+if (Priority == ThreadPriority::Normal) {
+  I = Tasks.begin();

A short comment about the structure of the queue would be in order, i.e. 
mention that we first store all "normal" tasks, followed by "low" tasks. And 
that we don't expect the number of "normal" tasks to grow beyond single-digit 
numbers, so it's ok to do linear search here and insert in that position



Comment at: clangd/index/Background.cpp:208
+  I = Tasks.begin();
+  while (I->second == ThreadPriority::Normal)
+I++;

Maybe change to `llvm::find_if(Tasks, [](Task &T) { return T.second == 
ThreadPriority::Low); } `?
More concise and avoids explicit loop.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55315



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

[PATCH] D55417: [clangd] Change disk-backed storage to be atomic

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/index/BackgroundIndexStorage.cpp:42
+  llvm::Twine TempPath(OutPath, ".tmp.");
+  TempPath.concat(std::to_string(rand()));
+  std::error_code EC;

There's a helper in LLVM that will do this, `llvm::createUniqueFile()`, I 
believe it also tries multiple times in case of clashes.



Comment at: clangd/index/BackgroundIndexStorage.cpp:55
+  // Then move to real location.
+  EC = llvm::sys::fs::rename(TempPath, OutPath);
+  if (EC)

We **should not** remove if no error occurred, as the same name at this point 
can be taken by a different action.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55417



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

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

> You also get into trouble because there are many header files that it add 
> LLVM_NODISCARD to but they don't include "Compiler.h" where LLVM is defined 
> (so you end up with lots of errors) 
>  3>C:\llvm\include\llvm/ADT/iterator_range.h(46): error C3646: 'IteratorT': 
> unknown override specifier (compiling source file 
> C:\llvm\tools\clang\utils\TableGen\ClangCommentHTMLNamedCharacterReferenceEmitter.cpp)
>  I'm wondering if there is anything I can add to the checker, to check to see 
> if the macro being used for the ReplacementString is defined "in scope"

There is `clang-tidy/utils/IncludeInserter.cpp` that helps you ensuring a 
necessary include is made. I am not sure if checking "in scope" is possible for 
macros in clang-tidy is practical, as it works on the AST that does not contain 
the macros. You can hook into `PPCallbacks` and check your macro exists and is 
defined. But will it be stable, as you can pass in `__attribute__(...)` which 
would not be a macro.
Maybe `IncludeInserter.cpp` is the best for now.

> Of course as I'm not building with C++17 I could have used [[nodiscard]] 
> instead of LLVM_NODISCARD, and that would of improved this I guess
>  I do get 100's of nodiscard warnings
>  the majority come from Diag() calls e.g.
> 
>   Diag(clang::diag::err_drv_expecting_fopenmp_with_fopenmp_targets);
>   
>   96>C:\llvm\tools\clang\lib\Driver\Driver.cpp(591): warning C4834: 
> discarding return value of function with 'nodiscard' attribute
>   96>C:\llvm\tools\clang\lib\Driver\Driver.cpp(688): warning C4834: 
> discarding return value of function with 'nodiscard' attribute
>   96>C:\llvm\tools\clang\lib\Driver\Driver.cpp(749): warning C4834: 
> discarding return value of function with 'nodiscard' attribute
>   96>C:\llvm\tools\clang\lib\Driver\Driver.cpp(795): warning C4834: 
> discarding return value of function with 'nodiscard' attribute

A heuristic could be, if the destructor is "trivial". Some classes do their 
business logic in the destructor (emitting logs, registering the diagnostic, 
...).
This pattern might be quite common for such tasks. Not annotating constructors 
might work, too. They are "special functions" in the language sense
and if the user calls the constructor and discards its result it probably means 
the destructor does something interesting.
Not sure if that is worth a heuristic...

> But I do get some other ones which look interesting, (I don't know these 
> areas of the code well enough to know if these are real issues!)
> 
> 90>C:\llvm\tools\clang\lib\Frontend\DiagnosticRenderer.cpp(476): warning 
> C4834: discarding return value of function with 'nodiscard' attribute
> 
>   static bool checkRangeForMacroArgExpansion(CharSourceRange Range,
>  const SourceManager &SM,
>  SourceLocation ArgumentLoc) {
> SourceLocation BegLoc = Range.getBegin(), EndLoc = Range.getEnd();
> while (BegLoc != EndLoc) {
>   if (!checkLocForMacroArgExpansion(BegLoc, SM, ArgumentLoc))
> return false;
>   BegLoc.getLocWithOffset(1);
> }
>   
> return checkLocForMacroArgExpansion(BegLoc, SM, ArgumentLoc);
>   }

Thats the offender, is it?

>   BegLoc.getLocWithOffset(1);

Looks like a bug to me, `getLocWithOffset` does not modify the sourcelocation 
itself. Maybe the `checkLocForMacroArgExpansion` modifies it? But otherwise 
that looks like a potential infinite loop.

> also a couple like this
> 
> 90>C:\llvm\tools\clang\lib\Frontend\SerializedDiagnosticReader.cpp(133): 
> warning C4834: discarding return value of function with 'nodiscard' attribute
>  90>C:\llvm\tools\clang\lib\Frontend\SerializedDiagnosticReader.cpp(175): 
> warning C4834: discarding return value of function with 'nodiscard' attribute
> 
>   unsigned BlockOrCode = 0;
>   llvm::ErrorOr Res = skipUntilRecordOrBlock(Stream, BlockOrCode);
>   if (!Res)
> Res.getError();
>

AFAIK `llvm::Error` must be consumed because if it goes out of scope unhandled 
it will `assert`. Not sure how to handle that.

> I could see that this level of noise might put people off, although this is 
> alot higher level of noise than I saw in both protobuf or in opencv which I 
> tried.
> 
> I wonder if it would be enough to put in some kind of exclusion regex list?
> 
> Reruning the build after having removed the LLVM_NODISCARD from Diag(...) to 
> see how much it quietens down
> 
> I'll continue looking to see if I find anything interesting.

Thanks for the evaluation. clang-tidy already has some quiet chatty checks and 
I think we should try to give reasonable SNR.
If we find some kind of heuristic we can apply we should do it, otherwise we 
could land it anyway.


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

https://reviews.llvm.org/D55433



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

[PATCH] D55649: [clangd] Enable cross-namespace completions by default in clangd

[Eric Liu via Phabricator via cfe-commits]

ioeric created this revision.
ioeric added reviewers: hokein, ilya-biryukov, kadircet.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay.

Code completion will suggest symbols from any scope (incl. inaccessible
scopes) when there's no qualifier explicitly specified. As we are assigning
relatively low scores for cross-namespace completion items, the overall code
completion quality doesn't regress. The feature has been tried out by a few
folks, and the feedback is generally positive, so I think it should be ready to
be enabled by default.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D55649

Files:
  clangd/tool/ClangdMain.cpp


Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -141,7 +141,7 @@
 "not defined in the scopes (e.g. "
 "namespaces) visible from the code completion point. Such completions "
 "can insert scope qualifiers."),
-cl::init(false), cl::Hidden);
+cl::init(true));
 
 static cl::opt
 ShowOrigins("debug-origin", cl::desc("Show origins of completion items"),


Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -141,7 +141,7 @@
 "not defined in the scopes (e.g. "
 "namespaces) visible from the code completion point. Such completions "
 "can insert scope qualifiers."),
-cl::init(false), cl::Hidden);
+cl::init(true));
 
 static cl::opt
 ShowOrigins("debug-origin", cl::desc("Show origins of completion items"),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55650: [clangd] Fix an assertion failure in background index.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: kadircet.
Herald added subscribers: arphaman, jkorous, MaskRay, ioeric, ilya-biryukov.

When indexing a file which contains an uncompilable error, we will
trigger an assertion failure -- the IndexFileIn data is not set, but we
access them in the backgound index.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D55650

Files:
  clangd/index/Background.cpp
  clangd/index/IndexAction.cpp
  unittests/clangd/BackgroundIndexTests.cpp


Index: unittests/clangd/BackgroundIndexTests.cpp
===
--- unittests/clangd/BackgroundIndexTests.cpp
+++ unittests/clangd/BackgroundIndexTests.cpp
@@ -66,6 +66,25 @@
   BackgroundIndexTest() { preventThreadStarvationInTests(); }
 };
 
+TEST_F(BackgroundIndexTest, NoCrashOnErrorFile) {
+  MockFSProvider FS;
+  FS.Files[testPath("root/A.cc")] = "error file";
+  llvm::StringMap Storage;
+  size_t CacheHits = 0;
+  MemoryShardStorage MSS(Storage, CacheHits);
+  OverlayCDB CDB(/*Base=*/nullptr);
+  BackgroundIndex Idx(Context::empty(), "", FS, CDB,
+  [&](llvm::StringRef) { return &MSS; });
+
+  tooling::CompileCommand Cmd;
+  Cmd.Filename = testPath("root/A.cc");
+  Cmd.Directory = testPath("root");
+  Cmd.CommandLine = {"clang++", "-DA=1", testPath("root/A.cc")};
+  CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
+
+  ASSERT_TRUE(Idx.blockUntilIdleForTest());
+}
+
 TEST_F(BackgroundIndexTest, IndexTwoFiles) {
   MockFSProvider FS;
   // a.h yields different symbols when included by A.cc vs B.cc.
Index: clangd/index/IndexAction.cpp
===
--- clangd/index/IndexAction.cpp
+++ clangd/index/IndexAction.cpp
@@ -134,22 +134,30 @@
   void EndSourceFileAction() override {
 WrapperFrontendAction::EndSourceFileAction();
 
+SymbolSlab Symbols;
+RefSlab Refs;
+IncludeGraph IGraph;
 const auto &CI = getCompilerInstance();
-if (CI.hasDiagnostics() &&
-CI.getDiagnostics().hasUncompilableErrorOccurred()) {
-  errs() << "Skipping TU due to uncompilable errors\n";
-  return;
+bool HasUncompilableErrors =
+CI.hasDiagnostics() &&
+CI.getDiagnostics().hasUncompilableErrorOccurred();
+if (!CI.hasDiagnostics() || !HasUncompilableErrors) {
+  Symbols = Collector->takeSymbols();
+  Refs = Collector->takeRefs();
+  IGraph = std::move(IG);
 }
-SymbolsCallback(Collector->takeSymbols());
-if (RefsCallback != nullptr)
-  RefsCallback(Collector->takeRefs());
-if (IncludeGraphCallback != nullptr) {
-#ifndef NDEBUG
+assert(SymbolsCallback && "SymbolsCallback must be set.");
+SymbolsCallback(std::move(Symbols));
+if (RefsCallback)
+  RefsCallback(std::move(Refs));
+if (IncludeGraphCallback) {
   // This checks if all nodes are initialized.
-  for (const auto &Node : IG)
-assert(Node.getKeyData() == Node.getValue().URI.data());
-#endif
-  IncludeGraphCallback(std::move(IG));
+  assert(std::all_of(IGraph.begin(), IGraph.end(),
+ [](const StringMapEntry &Node) {
+   return Node.getKeyData() ==
+  Node.getValue().URI.data();
+ }));
+  IncludeGraphCallback(std::move(IGraph));
 }
   }
 
Index: clangd/index/Background.cpp
===
--- clangd/index/Background.cpp
+++ clangd/index/Background.cpp
@@ -382,6 +382,9 @@
 return createStringError(inconvertibleErrorCode(), "Execute() failed");
   Action->EndSourceFile();
 
+  assert(Index.Symbols && Index.Refs && Index.Sources
+ && "Symbols, Refs and Sources must be set.");
+
   log("Indexed {0} ({1} symbols, {2} refs, {3} files)",
   Inputs.CompileCommand.Filename, Index.Symbols->size(),
   Index.Refs->numRefs(), Index.Sources->size());


Index: unittests/clangd/BackgroundIndexTests.cpp
===
--- unittests/clangd/BackgroundIndexTests.cpp
+++ unittests/clangd/BackgroundIndexTests.cpp
@@ -66,6 +66,25 @@
   BackgroundIndexTest() { preventThreadStarvationInTests(); }
 };
 
+TEST_F(BackgroundIndexTest, NoCrashOnErrorFile) {
+  MockFSProvider FS;
+  FS.Files[testPath("root/A.cc")] = "error file";
+  llvm::StringMap Storage;
+  size_t CacheHits = 0;
+  MemoryShardStorage MSS(Storage, CacheHits);
+  OverlayCDB CDB(/*Base=*/nullptr);
+  BackgroundIndex Idx(Context::empty(), "", FS, CDB,
+  [&](llvm::StringRef) { return &MSS; });
+
+  tooling::CompileCommand Cmd;
+  Cmd.Filename = testPath("root/A.cc");
+  Cmd.Directory = testPath("root");
+  Cmd.CommandLine = {"clang++", "-DA=1", testPath("root/A.cc")};
+  CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
+
+  ASSERT_TRUE(Idx.blockUntilIdleForTest());
+}
+
 TEST_F(BackgroundIndexTest, IndexTwoFiles) {
   MockFSProvider FS;
  

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In fact, the existing `modernize-make-unique` can be configured to support 
`absl::make_unique`, you'd just need to configure the check option 
`MakeSmartPtrFunction` to `absl::make_unique` (this is what we do inside 
google).

The biggest missing part of the `modernize-make-unique` is `absl::WrapUnique`, 
I think we should extend `MakeSmartPtrCheck` class (maybe add hooks) to support 
it.


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

https://reviews.llvm.org/D55044



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

[clang-tools-extra] r349031 - [clangd] Move the utility function to anonymous namespace, NFC.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Thu Dec 13 05:07:29 2018
New Revision: 349031

URL: http://llvm.org/viewvc/llvm-project?rev=349031&view=rev
Log:
[clangd] Move the utility function to anonymous namespace, NFC.

Modified:
clang-tools-extra/trunk/clangd/index/Background.cpp

Modified: clang-tools-extra/trunk/clangd/index/Background.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=349031&r1=349030&r2=349031&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Background.cpp Thu Dec 13 05:07:29 2018
@@ -86,6 +86,39 @@ IncludeGraph getSubGraph(const URI &U, c
 
   return IG;
 }
+
+// Creates a filter to not collect index results from files with unchanged
+// digests.
+// \p FileDigests contains file digests for the current indexed files, and all
+// changed files will be added to \p FilesToUpdate.
+decltype(SymbolCollector::Options::FileFilter)
+createFileFilter(const llvm::StringMap &FileDigests,
+ llvm::StringMap &FilesToUpdate) {
+  return [&FileDigests, &FilesToUpdate](const SourceManager &SM, FileID FID) {
+StringRef Path;
+if (const auto *F = SM.getFileEntryForID(FID))
+  Path = F->getName();
+if (Path.empty())
+  return false; // Skip invalid files.
+SmallString<128> AbsPath(Path);
+if (std::error_code EC =
+SM.getFileManager().getVirtualFileSystem()->makeAbsolute(AbsPath)) 
{
+  elog("Warning: could not make absolute file: {0}", EC.message());
+  return false; // Skip files without absolute path.
+}
+sys::path::remove_dots(AbsPath, /*remove_dot_dot=*/true);
+auto Digest = digestFile(SM, FID);
+if (!Digest)
+  return false;
+auto D = FileDigests.find(AbsPath);
+if (D != FileDigests.end() && D->second == Digest)
+  return false; // Skip files that haven't changed.
+
+FilesToUpdate[AbsPath] = *Digest;
+return true;
+  };
+}
+
 } // namespace
 
 BackgroundIndex::BackgroundIndex(
@@ -281,38 +314,6 @@ void BackgroundIndex::update(StringRef M
   }
 }
 
-// Creates a filter to not collect index results from files with unchanged
-// digests.
-// \p FileDigests contains file digests for the current indexed files, and all
-// changed files will be added to \p FilesToUpdate.
-decltype(SymbolCollector::Options::FileFilter)
-createFileFilter(const llvm::StringMap &FileDigests,
- llvm::StringMap &FilesToUpdate) {
-  return [&FileDigests, &FilesToUpdate](const SourceManager &SM, FileID FID) {
-StringRef Path;
-if (const auto *F = SM.getFileEntryForID(FID))
-  Path = F->getName();
-if (Path.empty())
-  return false; // Skip invalid files.
-SmallString<128> AbsPath(Path);
-if (std::error_code EC =
-SM.getFileManager().getVirtualFileSystem()->makeAbsolute(AbsPath)) 
{
-  elog("Warning: could not make absolute file: {0}", EC.message());
-  return false; // Skip files without absolute path.
-}
-sys::path::remove_dots(AbsPath, /*remove_dot_dot=*/true);
-auto Digest = digestFile(SM, FID);
-if (!Digest)
-  return false;
-auto D = FileDigests.find(AbsPath);
-if (D != FileDigests.end() && D->second == Digest)
-  return false; // Skip files that haven't changed.
-
-FilesToUpdate[AbsPath] = *Digest;
-return true;
-  };
-}
-
 Error BackgroundIndex::index(tooling::CompileCommand Cmd,
  BackgroundIndexStorage *IndexStorage) {
   trace::Span Tracer("BackgroundIndex");


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

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D55044#1329604 , @hokein wrote:

> In fact, the existing `modernize-make-unique` can be configured to support 
> `absl::make_unique`, you'd just need to configure the check option 
> `MakeSmartPtrFunction` to `absl::make_unique` (this is what we do inside 
> google).


See https://reviews.llvm.org/D52670#change-eVDYJhWSHWeW (the 
`getModuleOptions()` part) on how to do that

> The biggest missing part of the `modernize-make-unique` is 
> `absl::WrapUnique`, I think we should extend `MakeSmartPtrCheck` class (maybe 
> add hooks) to support it.




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

https://reviews.llvm.org/D55044



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

[clang-tools-extra] r349032 - [clangd] Avoid emitting Queued status when we are able to acquire the Barrier.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Thu Dec 13 05:09:50 2018
New Revision: 349032

URL: http://llvm.org/viewvc/llvm-project?rev=349032&view=rev
Log:
[clangd] Avoid emitting Queued status when we are able to acquire the Barrier.

Reviewers: ilya-biryukov

Subscribers: javed.absar, ioeric, MaskRay, jkorous, arphaman, kadircet, 
cfe-commits

Differential Revision: https://reviews.llvm.org/D55359

Modified:
clang-tools-extra/trunk/clangd/TUScheduler.cpp
clang-tools-extra/trunk/clangd/Threading.cpp
clang-tools-extra/trunk/clangd/Threading.h
clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp

Modified: clang-tools-extra/trunk/clangd/TUScheduler.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.cpp?rev=349032&r1=349031&r2=349032&view=diff
==
--- clang-tools-extra/trunk/clangd/TUScheduler.cpp (original)
+++ clang-tools-extra/trunk/clangd/TUScheduler.cpp Thu Dec 13 05:09:50 2018
@@ -634,9 +634,11 @@ void ASTWorker::run() {
 } // unlock Mutex
 
 {
-  // FIXME: only emit this status when the Barrier couldn't be acquired.
-  emitTUStatus({TUAction::Queued, Req.Name});
-  std::lock_guard BarrierLock(Barrier);
+  std::unique_lock Lock(Barrier, std::try_to_lock);
+  if (!Lock.owns_lock()) {
+emitTUStatus({TUAction::Queued, Req.Name});
+Lock.lock();
+  }
   WithContext Guard(std::move(Req.Ctx));
   trace::Span Tracer(Req.Name);
   emitTUStatus({TUAction::RunningAction, Req.Name});

Modified: clang-tools-extra/trunk/clangd/Threading.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Threading.cpp?rev=349032&r1=349031&r2=349032&view=diff
==
--- clang-tools-extra/trunk/clangd/Threading.cpp (original)
+++ clang-tools-extra/trunk/clangd/Threading.cpp Thu Dec 13 05:09:50 2018
@@ -28,6 +28,15 @@ void Notification::wait() const {
 
 Semaphore::Semaphore(std::size_t MaxLocks) : FreeSlots(MaxLocks) {}
 
+bool Semaphore::try_lock() {
+  std::unique_lock Lock(Mutex);
+  if (FreeSlots > 0) {
+--FreeSlots;
+return true;
+  }
+  return false;
+}
+
 void Semaphore::lock() {
   trace::Span Span("WaitForFreeSemaphoreSlot");
   // trace::Span can also acquire locks in ctor and dtor, we make sure it

Modified: clang-tools-extra/trunk/clangd/Threading.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Threading.h?rev=349032&r1=349031&r2=349032&view=diff
==
--- clang-tools-extra/trunk/clangd/Threading.h (original)
+++ clang-tools-extra/trunk/clangd/Threading.h Thu Dec 13 05:09:50 2018
@@ -42,6 +42,7 @@ class Semaphore {
 public:
   Semaphore(std::size_t MaxLocks);
 
+  bool try_lock();
   void lock();
   void unlock();
 

Modified: clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp?rev=349032&r1=349031&r2=349032&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp Thu Dec 13 
05:09:50 2018
@@ -698,13 +698,11 @@ TEST_F(TUSchedulerTests, TUStatus) {
   EXPECT_THAT(CaptureTUStatus.AllStatus,
   ElementsAre(
   // Statuses of "Update" action.
-  TUState(TUAction::Queued, "Update"),
   TUState(TUAction::RunningAction, "Update"),
   TUState(TUAction::BuildingPreamble, "Update"),
   TUState(TUAction::BuildingFile, "Update"),
 
   // Statuses of "Definitions" action
-  TUState(TUAction::Queued, "Definitions"),
   TUState(TUAction::RunningAction, "Definitions"),
   TUState(TUAction::Idle, /*No action*/ "")));
 }


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

[PATCH] D55359: [clangd] Avoid emitting Queued status when we are able to acquire the Barrier.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL349032: [clangd] Avoid emitting Queued status when we are 
able to acquire the Barrier. (authored by hokein, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55359

Files:
  clang-tools-extra/trunk/clangd/TUScheduler.cpp
  clang-tools-extra/trunk/clangd/Threading.cpp
  clang-tools-extra/trunk/clangd/Threading.h
  clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp


Index: clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
@@ -698,13 +698,11 @@
   EXPECT_THAT(CaptureTUStatus.AllStatus,
   ElementsAre(
   // Statuses of "Update" action.
-  TUState(TUAction::Queued, "Update"),
   TUState(TUAction::RunningAction, "Update"),
   TUState(TUAction::BuildingPreamble, "Update"),
   TUState(TUAction::BuildingFile, "Update"),
 
   // Statuses of "Definitions" action
-  TUState(TUAction::Queued, "Definitions"),
   TUState(TUAction::RunningAction, "Definitions"),
   TUState(TUAction::Idle, /*No action*/ "")));
 }
Index: clang-tools-extra/trunk/clangd/Threading.cpp
===
--- clang-tools-extra/trunk/clangd/Threading.cpp
+++ clang-tools-extra/trunk/clangd/Threading.cpp
@@ -28,6 +28,15 @@
 
 Semaphore::Semaphore(std::size_t MaxLocks) : FreeSlots(MaxLocks) {}
 
+bool Semaphore::try_lock() {
+  std::unique_lock Lock(Mutex);
+  if (FreeSlots > 0) {
+--FreeSlots;
+return true;
+  }
+  return false;
+}
+
 void Semaphore::lock() {
   trace::Span Span("WaitForFreeSemaphoreSlot");
   // trace::Span can also acquire locks in ctor and dtor, we make sure it
Index: clang-tools-extra/trunk/clangd/TUScheduler.cpp
===
--- clang-tools-extra/trunk/clangd/TUScheduler.cpp
+++ clang-tools-extra/trunk/clangd/TUScheduler.cpp
@@ -634,9 +634,11 @@
 } // unlock Mutex
 
 {
-  // FIXME: only emit this status when the Barrier couldn't be acquired.
-  emitTUStatus({TUAction::Queued, Req.Name});
-  std::lock_guard BarrierLock(Barrier);
+  std::unique_lock Lock(Barrier, std::try_to_lock);
+  if (!Lock.owns_lock()) {
+emitTUStatus({TUAction::Queued, Req.Name});
+Lock.lock();
+  }
   WithContext Guard(std::move(Req.Ctx));
   trace::Span Tracer(Req.Name);
   emitTUStatus({TUAction::RunningAction, Req.Name});
Index: clang-tools-extra/trunk/clangd/Threading.h
===
--- clang-tools-extra/trunk/clangd/Threading.h
+++ clang-tools-extra/trunk/clangd/Threading.h
@@ -42,6 +42,7 @@
 public:
   Semaphore(std::size_t MaxLocks);
 
+  bool try_lock();
   void lock();
   void unlock();
 


Index: clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
@@ -698,13 +698,11 @@
   EXPECT_THAT(CaptureTUStatus.AllStatus,
   ElementsAre(
   // Statuses of "Update" action.
-  TUState(TUAction::Queued, "Update"),
   TUState(TUAction::RunningAction, "Update"),
   TUState(TUAction::BuildingPreamble, "Update"),
   TUState(TUAction::BuildingFile, "Update"),
 
   // Statuses of "Definitions" action
-  TUState(TUAction::Queued, "Definitions"),
   TUState(TUAction::RunningAction, "Definitions"),
   TUState(TUAction::Idle, /*No action*/ "")));
 }
Index: clang-tools-extra/trunk/clangd/Threading.cpp
===
--- clang-tools-extra/trunk/clangd/Threading.cpp
+++ clang-tools-extra/trunk/clangd/Threading.cpp
@@ -28,6 +28,15 @@
 
 Semaphore::Semaphore(std::size_t MaxLocks) : FreeSlots(MaxLocks) {}
 
+bool Semaphore::try_lock() {
+  std::unique_lock Lock(Mutex);
+  if (FreeSlots > 0) {
+--FreeSlots;
+return true;
+  }
+  return false;
+}
+
 void Semaphore::lock() {
   trace::Span Span("WaitForFreeSemaphoreSlot");
   // trace::Span can also acquire locks in ctor and dtor, we make sure it
Index: clang-tools-extra/trunk/clangd/TUScheduler.cpp
===
--- clang-tools-extra/trunk/clangd/TUScheduler.cpp
+++ clang-tools-extra/trunk/clangd/TUScheduler.cpp
@@ -634,9 +634,11 @@
 } // unlo

[clang-tools-extra] r349033 - [clangd] Refine the way of checking a declaration is referenced by the written code.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Thu Dec 13 05:17:04 2018
New Revision: 349033

URL: http://llvm.org/viewvc/llvm-project?rev=349033&view=rev
Log:
[clangd] Refine the way of checking a declaration is referenced by the written 
code.

Summary:
The previous solution (checking the AST) is not a reliable way to
determine whether a declaration is explicitly referenced by the source
code, we are still missing a few cases.

Reviewers: ilya-biryukov

Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits

Differential Revision: https://reviews.llvm.org/D55191

Modified:
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp

Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=349033&r1=349032&r2=349033&view=diff
==
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Thu Dec 13 05:17:04 2018
@@ -139,21 +139,19 @@ public:
   SourceLocation Loc,
   index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
 if (Loc == SearchedLocation) {
-  // Check whether the E has an implicit AST node (e.g. ImplicitCastExpr).
-  auto hasImplicitExpr = [](const Expr *E) {
-if (!E || E->child_begin() == E->child_end())
+  auto isImplicitExpr = [](const Expr *E) {
+if (!E)
   return false;
-// Use the first child is good enough for most cases -- normally the
-// expression returned by handleDeclOccurence contains exactly one
-// child expression.
-const auto *FirstChild = *E->child_begin();
-return isa(FirstChild) ||
-   isa(FirstChild) ||
-   isa(FirstChild) ||
-   isa(FirstChild);
+// We assume that a constructor expression is implict (was inserted by
+// clang) if it has an invalid paren/brace location, since such
+// experssion is impossible to write down.
+if (const auto *CtorExpr = dyn_cast(E))
+  return CtorExpr->getNumArgs() > 0 &&
+ CtorExpr->getParenOrBraceRange().isInvalid();
+return isa(E);
   };
 
-  bool IsExplicit = !hasImplicitExpr(ASTNode.OrigE);
+  bool IsExplicit = !isImplicitExpr(ASTNode.OrigE);
   // Find and add definition declarations (for GoToDefinition).
   // We don't use parameter `D`, as Parameter `D` is the canonical
   // declaration, which is the first declaration of a redeclarable

Modified: clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp?rev=349033&r1=349032&r2=349033&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp Thu Dec 13 05:17:04 
2018
@@ -1225,6 +1225,53 @@ TEST(FindReferences, WithinAST) {
   }
 }
 
+TEST(FindReferences, ExplicitSymbols) {
+  const char *Tests[] = {
+  R"cpp(
+  struct Foo { Foo* [self]() const; };
+  void f() {
+if (Foo* T = foo.[^self]()) {} // Foo member call expr.
+  }
+  )cpp",
+
+  R"cpp(
+  struct Foo { Foo(int); };
+  Foo f() {
+int [b];
+return [^b]; // Foo constructor expr.
+  }
+  )cpp",
+
+  R"cpp(
+  struct Foo {};
+  void g(Foo);
+  Foo [f]();
+  void call() {
+g([^f]());  // Foo constructor expr.
+  }
+  )cpp",
+
+  R"cpp(
+  void [foo](int);
+  void [foo](double);
+
+  namespace ns {
+  using ::[fo^o];
+  }
+  )cpp",
+  };
+  for (const char *Test : Tests) {
+Annotations T(Test);
+auto AST = TestTU::withCode(T.code()).build();
+std::vector> ExpectedLocations;
+for (const auto &R : T.ranges())
+  ExpectedLocations.push_back(RangeIs(R));
+EXPECT_THAT(findReferences(AST, T.point()),
+ElementsAreArray(ExpectedLocations))
+<< Test;
+  }
+}
+
 TEST(FindReferences, NeedsIndex) {
   const char *Header = "int foo();";
   Annotations Main("int main() { [[f^oo]](); }");


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

[PATCH] D55191: [clangd] Refine the way of checking a declaration is referenced by the written code.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE349033: [clangd] Refine the way of checking a declaration 
is referenced by the written… (authored by hokein, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D55191?vs=177190&id=178038#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55191

Files:
  clangd/XRefs.cpp
  unittests/clangd/XRefsTests.cpp


Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -139,21 +139,19 @@
   SourceLocation Loc,
   index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
 if (Loc == SearchedLocation) {
-  // Check whether the E has an implicit AST node (e.g. ImplicitCastExpr).
-  auto hasImplicitExpr = [](const Expr *E) {
-if (!E || E->child_begin() == E->child_end())
+  auto isImplicitExpr = [](const Expr *E) {
+if (!E)
   return false;
-// Use the first child is good enough for most cases -- normally the
-// expression returned by handleDeclOccurence contains exactly one
-// child expression.
-const auto *FirstChild = *E->child_begin();
-return isa(FirstChild) ||
-   isa(FirstChild) ||
-   isa(FirstChild) ||
-   isa(FirstChild);
+// We assume that a constructor expression is implict (was inserted by
+// clang) if it has an invalid paren/brace location, since such
+// experssion is impossible to write down.
+if (const auto *CtorExpr = dyn_cast(E))
+  return CtorExpr->getNumArgs() > 0 &&
+ CtorExpr->getParenOrBraceRange().isInvalid();
+return isa(E);
   };
 
-  bool IsExplicit = !hasImplicitExpr(ASTNode.OrigE);
+  bool IsExplicit = !isImplicitExpr(ASTNode.OrigE);
   // Find and add definition declarations (for GoToDefinition).
   // We don't use parameter `D`, as Parameter `D` is the canonical
   // declaration, which is the first declaration of a redeclarable
Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -1225,6 +1225,53 @@
   }
 }
 
+TEST(FindReferences, ExplicitSymbols) {
+  const char *Tests[] = {
+  R"cpp(
+  struct Foo { Foo* [self]() const; };
+  void f() {
+if (Foo* T = foo.[^self]()) {} // Foo member call expr.
+  }
+  )cpp",
+
+  R"cpp(
+  struct Foo { Foo(int); };
+  Foo f() {
+int [b];
+return [^b]; // Foo constructor expr.
+  }
+  )cpp",
+
+  R"cpp(
+  struct Foo {};
+  void g(Foo);
+  Foo [f]();
+  void call() {
+g([^f]());  // Foo constructor expr.
+  }
+  )cpp",
+
+  R"cpp(
+  void [foo](int);
+  void [foo](double);
+
+  namespace ns {
+  using ::[fo^o];
+  }
+  )cpp",
+  };
+  for (const char *Test : Tests) {
+Annotations T(Test);
+auto AST = TestTU::withCode(T.code()).build();
+std::vector> ExpectedLocations;
+for (const auto &R : T.ranges())
+  ExpectedLocations.push_back(RangeIs(R));
+EXPECT_THAT(findReferences(AST, T.point()),
+ElementsAreArray(ExpectedLocations))
+<< Test;
+  }
+}
+
 TEST(FindReferences, NeedsIndex) {
   const char *Header = "int foo();";
   Annotations Main("int main() { [[f^oo]](); }");


Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -139,21 +139,19 @@
   SourceLocation Loc,
   index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
 if (Loc == SearchedLocation) {
-  // Check whether the E has an implicit AST node (e.g. ImplicitCastExpr).
-  auto hasImplicitExpr = [](const Expr *E) {
-if (!E || E->child_begin() == E->child_end())
+  auto isImplicitExpr = [](const Expr *E) {
+if (!E)
   return false;
-// Use the first child is good enough for most cases -- normally the
-// expression returned by handleDeclOccurence contains exactly one
-// child expression.
-const auto *FirstChild = *E->child_begin();
-return isa(FirstChild) ||
-   isa(FirstChild) ||
-   isa(FirstChild) ||
-   isa(FirstChild);
+// We assume that a constructor expression is implict (was inserted by
+// clang) if it has an invalid paren/brace location, since such
+// experssion is impossible to write down.
+if (const auto *CtorExpr = dyn_cast(E))
+  return CtorExpr->getNumArgs() > 0 &&
+ CtorExpr->getParenOrBraceRange().isInvalid();
+return isa(E);
   };
 
-  bool IsExplicit = !

[PATCH] D55245: [clang-tidy] Add the abseil-duration-subtraction check

[Hyrum Wright via Phabricator via cfe-commits]

hwright updated this revision to Diff 178039.
hwright marked an inline comment as done.
hwright added a comment.

Rebase and update documentation


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

https://reviews.llvm.org/D55245

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/DurationComparisonCheck.cpp
  clang-tidy/abseil/DurationRewriter.cpp
  clang-tidy/abseil/DurationRewriter.h
  clang-tidy/abseil/DurationSubtractionCheck.cpp
  clang-tidy/abseil/DurationSubtractionCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-duration-subtraction.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-duration-subtraction.cpp

Index: test/clang-tidy/abseil-duration-subtraction.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-duration-subtraction.cpp
@@ -0,0 +1,64 @@
+// RUN: %check_clang_tidy %s abseil-duration-subtraction %t -- -- -I %S/Inputs
+
+#include "absl/time/time.h"
+
+void f() {
+  double x;
+  absl::Duration d, d1, d2;
+
+  x = absl::ToDoubleSeconds(d) - 1.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleSeconds(d - absl::Seconds(1))
+  x = absl::ToDoubleSeconds(d) - absl::ToDoubleSeconds(d1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleSeconds(d - d1);
+  x = absl::ToDoubleSeconds(d) - 6.5 - 8.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleSeconds(d - absl::Seconds(6.5)) - 8.0;
+  x = absl::ToDoubleHours(d) - 1.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleHours(d - absl::Hours(1))
+  x = absl::ToDoubleMinutes(d) - 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleMinutes(d - absl::Minutes(1))
+  x = absl::ToDoubleMilliseconds(d) - 9;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleMilliseconds(d - absl::Milliseconds(9))
+  x = absl::ToDoubleMicroseconds(d) - 9;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleMicroseconds(d - absl::Microseconds(9))
+  x = absl::ToDoubleNanoseconds(d) - 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleNanoseconds(d - absl::Nanoseconds(42))
+
+  // We can rewrite the argument of the duration conversion
+#define THIRTY absl::Seconds(30)
+  x = absl::ToDoubleSeconds(THIRTY) - 1.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleSeconds(THIRTY - absl::Seconds(1))
+#undef THIRTY
+
+  // Some other contexts
+  if (absl::ToDoubleSeconds(d) - 1.0 > 10) {}
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: if (absl::ToDoubleSeconds(d - absl::Seconds(1)) > 10) {}
+
+  // A nested occurance
+  x = absl::ToDoubleSeconds(d) - absl::ToDoubleSeconds(absl::Seconds(5));
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleSeconds(d - absl::Seconds(5))
+  x = absl::ToDoubleSeconds(d) - absl::ToDoubleSeconds(absl::Seconds(absl::ToDoubleSeconds(d1)));
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleSeconds(d - absl::Seconds(absl::ToDoubleSeconds(d1)))
+
+  // These should not match
+  x = 5 - 6;
+  x = 4 - absl::ToDoubleSeconds(d) - 6.5 - 8.0;
+  x = absl::ToDoubleSeconds(d) + 1.0;
+  x = absl::ToDoubleSeconds(d) * 1.0;
+  x = absl::ToDoubleSeconds(d) / 1.0;
+
+#define MINUS_FIVE(z) absl::ToDoubleSeconds(z) - 5
+  x = MINUS_FIVE(d);
+#undef MINUS_FIVE
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -8,6 +8,7 @@
abseil-duration-division
abseil-duration-factory-float
abseil-duration-factory-scale
+   abseil-duration-subtraction
abseil-faster-strsplit-delimiter
abseil-no-internal-dependencies
abseil-no-namespace
Index: docs/clang-tidy/checks/abseil-duration-subtraction.rst
===
--- /dev/null
+++ do

[PATCH] D55245: [clang-tidy] Add the abseil-duration-subtraction check

[Hyrum Wright via Phabricator via cfe-commits]

hwright marked an inline comment as done.
hwright added a comment.

I've updated the documentation, and the rebased to `master`.


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

https://reviews.llvm.org/D55245



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

[PATCH] D55646: [ASTImporter] Make ODR diagnostics warning by default

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

The primary reason why want these to be Warnings instead of Errors because 
there are many false positive ODR Errors at the moment. These are not fatal 
errors but there is a maximum error count which once reached the whole process 
(analysis) is aborted.
Usually, such false positives are related to a wrong import of redeclaration 
chains or existing errors in the structural match logic.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55646



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

[PATCH] D55648: [CodeComplete] Fill preferred type on binary expressions

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

It seems like we have some assumptions about what people might be trying to do 
in most of the cases, but i think it is OK since this never performs worse than 
the older version.




Comment at: lib/Parse/ParseExpr.cpp:396
 
 // Code completion for the right-hand side of an assignment expression
 // goes through a special hook that takes the left-hand side into account.

maybe update the comment as well to reflect it is not only for assignments.



Comment at: lib/Sema/SemaCodeComplete.cpp:4928
+if (LHSType->isIntegralOrEnumerationType())
+  return S.getASTContext().IntTy;
+return QualType();

why not LHSType ?



Comment at: lib/Sema/SemaCodeComplete.cpp:4935
+return S.getASTContext().BoolTy;
+  case tok::pipe:
+  case tok::pipeequal:

maybe also state the assumption for bitwise operators, or maybe just move to 
right after shift operators



Comment at: unittests/Sema/CodeCompleteTest.cpp:295
+void test(Cls c) {
+  // we assume arithmetic and comparions ops take the same time.
+  c + ^c; c - ^c; c * ^c; c / ^c; c % ^c;

s/time/type/ ?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55648



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

[PATCH] D55595: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

One thing: Could you please check the utility-scripts in clang-tidy that create 
new checks, if they need adjustments? Not sure if we have something in there 
that relies on the old structure.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55595



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

[PATCH] D55363: [clangd] Expose FileStatus in LSP.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Sorry if I missed any important design discussions here, but wanted to clear up 
what information we are trying to convey to the user with the status messages?
E.g. why seeing "building preamble", "building file" or "queued" in the status 
bar can be useful to the user? Those messages mention internals of clangd, I'm 
not sure how someone unfamiliar with internals of clangd should interpret this 
information.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55363



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

[PATCH] D55245: [clang-tidy] Add the abseil-duration-subtraction check

[Jonas Toth via Phabricator via cfe-commits]

JonasToth accepted this revision.
JonasToth added a comment.

LGTM with the doc-nit.




Comment at: docs/clang-tidy/checks/abseil-duration-subtraction.rst:33
+Note: As with other ``clang-tidy`` checks, it is possible that multiple fixes
+may overlap (as in the case of nested expressions), so not all occurences can
+be transformed in one run. Running ``clang-tidy`` multiple times will fix these

I think it the doc should be explicit that this is the case for nesting the 
subtraction expression the issue can occur. Otherwise it might be a bit 
confusing.


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

https://reviews.llvm.org/D55245



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

[PATCH] D55595: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added a comment.

In D55595#1329647 , @JonasToth wrote:

> One thing: Could you please check the utility-scripts in clang-tidy that 
> create new checks, if they need adjustments? Not sure if we have something in 
> there that relies on the old structure.


As far as I see these scripts do not touch 'tool' or 'plugin' folders at all, 
only the specific module, docs and tests.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55595



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

[PATCH] D55437: [Index] Index declarations in lambda expression.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

The `RecursiveASTVisitor` seems to have the code that properly traverses 
parameters and return types of lambdas. Any ideas why it's not getting called 
here?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55437



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

[PATCH] D55595: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin

[Jonas Toth via Phabricator via cfe-commits]

JonasToth accepted this revision.
JonasToth added a comment.

LGTM, chatted with steveire on IRC: blocking this for a better cmake based 
solution makes no sense. So your free to commit :)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55595



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

[PATCH] D55650: [clangd] Fix an assertion failure in background index.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

I think the description is misleading, you are saying that we were triggering 
an assertion and you are fixing that. But in reality, we were not triggering 
any assertions, this patch is adding the assertions right?




Comment at: clangd/index/Background.cpp:385
 
+  assert(Index.Symbols && Index.Refs && Index.Sources
+ && "Symbols, Refs and Sources must be set.");

I don't think it is a good idea to assert and die just for one file, since we 
are actually trying to index all the files. We should rather check these exists 
and propagate the error in case of failure. You can have a look at 
https://reviews.llvm.org/D55224, same lines.

Also with your changes in the endsourcefileaction this assertion is not 
possible to be triggered, since you will fill them with "empty" versions.



Comment at: clangd/index/IndexAction.cpp:144
+CI.getDiagnostics().hasUncompilableErrorOccurred();
+if (!CI.hasDiagnostics() || !HasUncompilableErrors) {
+  Symbols = Collector->takeSymbols();

I think this is same as `if (!HasUncompilableErrors)` so you can keep the old 
one just by negating it.



Comment at: clangd/index/IndexAction.cpp:150
+assert(SymbolsCallback && "SymbolsCallback must be set.");
+SymbolsCallback(std::move(Symbols));
+if (RefsCallback)

I don't think it is a good idea to respond with an "empty" symbol slab rather 
than not replying. They have different semantics. The former implies everything 
went well, just couldn't find any symbols in the file, whereas the latter 
implies there was an error.

So I believe we should keep this code as it was.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55650



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

[PATCH] D55649: [clangd] Enable cross-namespace completions by default in clangd

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clangd/tool/ClangdMain.cpp:144
 "can insert scope qualifiers."),
-cl::init(false), cl::Hidden);
+cl::init(true));
 

why not keep it hidden ?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55649



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

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: test/clang-tidy/modernize-use-nodiscard.cpp:126
+template
+class Bar
+{

MyDeveloperDay wrote:
> curdeius wrote:
> > JonasToth wrote:
> > > I think the template tests should be improved.
> > > What happens for `T empty()`, `typename T::value_type empty()` and so on. 
> > > THe same for the parameters for functions/methods (including function 
> > > templates).
> > > 
> > > Thinking of it, it might be a good idea to ignore functions where the 
> > > heuristic depends on the template-paramters.
> > It might be a good idea to add a note in the documentation about handling 
> > of function templates and functions in class templates.
> I think I need some help to determine if the parameter is a template 
> parameter (specially a const T & or a const_reference)
> 
> I'm trying to remove functions which have any type of Template parameter (at 
> least for now)
> 
> I've modified the hasNonConstReferenceOrPointerArguments matcher to use 
> isTemplateTypeParamType()
> 
> but this doesn't seem to work though an Alias or even just with a const &
> 
> ```
>   return llvm::any_of(Node.parameters(), [](const ParmVarDecl *Par) {
> QualType ParType = Par->getType();
> 
> if (ParType->isTemplateTypeParmType())
>   return true;
> 
> if (ParType->isPointerType() || isNonConstReferenceType(ParType))
>   return true;
> 
> return false;
>   });
> ```
> 
> mostly the tests cases work for  T and T&  (see below)
> 
> but it does not seem to work for const T&, or const_reference, where it still 
> wants to add the [[nodiscard]]
> 
> Could anyone give me any pointers, or somewhere I can look to learn?  I was 
> thinking I needed to look at the getUnqualifiedDeSugared() but it didn't seem 
> to work the way I expected.
> 
> ```
> template
> class Bar
> {
> public:
> using value_type = T;
> using reference = value_type&;
> using const_reference = const value_type&;
> 
> bool empty() const;
> // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'empty' should be 
> marked NO_DISCARD [modernize-use-nodiscard]
> // CHECK-FIXES: NO_DISCARD bool empty() const;
> 
> // we cannot assume that the template parameter isn't a pointer
> bool f25(value_type) const;
> // CHECK-MESSAGES-NOT: warning:
> // CHECK-FIXES: bool f25(value_type) const;
> 
> bool f27(reference) const;
> // CHECK-MESSAGES-NOT: warning:
> // CHECK-FIXES: bool f27(reference) const
> 
> typename T::value_type f35() const;
> // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f35' should be 
> marked NO_DISCARD [modernize-use-nodiscard]
> // CHECK-FIXES: NO_DISCARD typename T::value_type f35() const
> 
> T f34() const;
> // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f34' should be 
> marked NO_DISCARD [modernize-use-nodiscard]
> // CHECK-FIXES: NO_DISCARD T f34() const
> 
> bool f31(T) const;
> // CHECK-MESSAGES-NOT: warning:
> // CHECK-FIXES: bool f31(T) const
> 
> bool f33(T&) const;
> // CHECK-MESSAGES-NOT: warning:
> // CHECK-FIXES: bool f33(T&) const
> 
> // -  FIXME TESTS BELOW FAIL - //
> bool f26(const_reference) const;
> // CHECK-MESSAGES-NOT: warning:
> // CHECK-FIXES: bool f26(const_reference) const;
> 
> bool f32(const T&) const;
> // CHECK-MESSAGES-NOT: warning:
> // CHECK-FIXES: bool f32(const T&) const
> };
> 
> ```
> 
> 
> 
> 
> 
> 
Is this resolved, as you marked it done?


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

https://reviews.llvm.org/D55433



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

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: test/clang-tidy/modernize-use-nodiscard.cpp:183
+// CHECK-MESSAGES-NOT: warning:
+// CHECK-FIXES: bool f33(T&) const
+

No warning -> No fix -> You can ellide the `CHECK-FIXES` here and elsewhere. 
FileCheck is not confused by that :)
You don't need to specify that you dont expect a warning, too, because every 
warning that is not handled by `CHECK-MESSAGES`/`CHECK-NOTES` will result in a 
failed test.


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

https://reviews.llvm.org/D55433



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

[clang-tools-extra] r349038 - [clang-tidy] Share the forced linking code between clang-tidy tool and plugin

[Ivan Donchevskii via cfe-commits]

Author: yvvan
Date: Thu Dec 13 06:37:17 2018
New Revision: 349038

URL: http://llvm.org/viewvc/llvm-project?rev=349038&view=rev
Log:
[clang-tidy] Share the forced linking code between clang-tidy tool and plugin

Extract code that forces linking to the separate header and include it in both 
plugin and standalone tool

Differential Revision: https://reviews.llvm.org/D55595

Added:
clang-tools-extra/trunk/clang-tidy/ClangTidyForceLinker.h
Modified:
clang-tools-extra/trunk/clang-tidy/plugin/ClangTidyPlugin.cpp
clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp

Added: clang-tools-extra/trunk/clang-tidy/ClangTidyForceLinker.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyForceLinker.h?rev=349038&view=auto
==
--- clang-tools-extra/trunk/clang-tidy/ClangTidyForceLinker.h (added)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyForceLinker.h Thu Dec 13 
06:37:17 2018
@@ -0,0 +1,108 @@
+//===- ClangTidyForceLinker.h - clang-tidy 
===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "llvm/Support/Compiler.h"
+
+namespace clang {
+namespace tidy {
+
+// This anchor is used to force the linker to link the CERTModule.
+extern volatile int CERTModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED CERTModuleAnchorDestination =
+CERTModuleAnchorSource;
+
+// This anchor is used to force the linker to link the AbseilModule.
+extern volatile int AbseilModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED AbseilModuleAnchorDestination =
+AbseilModuleAnchorSource;
+
+// This anchor is used to force the linker to link the BoostModule.
+extern volatile int BoostModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED BoostModuleAnchorDestination =
+BoostModuleAnchorSource;
+
+// This anchor is used to force the linker to link the BugproneModule.
+extern volatile int BugproneModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED BugproneModuleAnchorDestination =
+BugproneModuleAnchorSource;
+
+// This anchor is used to force the linker to link the LLVMModule.
+extern volatile int LLVMModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED LLVMModuleAnchorDestination =
+LLVMModuleAnchorSource;
+
+// This anchor is used to force the linker to link the CppCoreGuidelinesModule.
+extern volatile int CppCoreGuidelinesModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED CppCoreGuidelinesModuleAnchorDestination =
+CppCoreGuidelinesModuleAnchorSource;
+
+// This anchor is used to force the linker to link the FuchsiaModule.
+extern volatile int FuchsiaModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED FuchsiaModuleAnchorDestination =
+FuchsiaModuleAnchorSource;
+
+// This anchor is used to force the linker to link the GoogleModule.
+extern volatile int GoogleModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED GoogleModuleAnchorDestination =
+GoogleModuleAnchorSource;
+
+// This anchor is used to force the linker to link the AndroidModule.
+extern volatile int AndroidModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED AndroidModuleAnchorDestination =
+AndroidModuleAnchorSource;
+
+// This anchor is used to force the linker to link the MiscModule.
+extern volatile int MiscModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED MiscModuleAnchorDestination =
+MiscModuleAnchorSource;
+
+// This anchor is used to force the linker to link the ModernizeModule.
+extern volatile int ModernizeModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED ModernizeModuleAnchorDestination =
+ModernizeModuleAnchorSource;
+
+#if CLANG_ENABLE_STATIC_ANALYZER
+// This anchor is used to force the linker to link the MPIModule.
+extern volatile int MPIModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED MPIModuleAnchorDestination =
+MPIModuleAnchorSource;
+#endif
+
+// This anchor is used to force the linker to link the PerformanceModule.
+extern volatile int PerformanceModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED PerformanceModuleAnchorDestination =
+PerformanceModuleAnchorSource;
+
+// This anchor is used to force the linker to link the PortabilityModule.
+extern volatile int PortabilityModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED PortabilityModuleAnchorDestination =
+PortabilityModuleAnchorSource;
+
+// This anchor is used to force the linker to link the ReadabilityModule.
+extern volatile int ReadabilityModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED ReadabilityModuleAnchorDestination =
+ReadabilityModuleAnchorSource;
+
+// This anchor is used to force the linker to link the ObjCModule.
+extern volatile int ObjCModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED ObjCModuleAnchorDestinat

[PATCH] D55595: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin

[Ivan Donchevskii via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE349038: [clang-tidy] Share the forced linking code between 
clang-tidy tool and plugin (authored by yvvan, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D55595?vs=177832&id=178048#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55595

Files:
  clang-tidy/ClangTidyForceLinker.h
  clang-tidy/plugin/ClangTidyPlugin.cpp
  clang-tidy/tool/ClangTidyMain.cpp

Index: clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -16,6 +16,7 @@
 //===--===//
 
 #include "../ClangTidy.h"
+#include "../ClangTidyForceLinker.h"
 #include "clang/Config/config.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "llvm/Support/Process.h"
@@ -482,98 +483,6 @@
   return 0;
 }
 
-// This anchor is used to force the linker to link the CERTModule.
-extern volatile int CERTModuleAnchorSource;
-static int LLVM_ATTRIBUTE_UNUSED CERTModuleAnchorDestination =
-CERTModuleAnchorSource;
-
-// This anchor is used to force the linker to link the AbseilModule.
-extern volatile int AbseilModuleAnchorSource;
-static int LLVM_ATTRIBUTE_UNUSED AbseilModuleAnchorDestination =
-AbseilModuleAnchorSource;
-
-// This anchor is used to force the linker to link the BoostModule.
-extern volatile int BoostModuleAnchorSource;
-static int LLVM_ATTRIBUTE_UNUSED BoostModuleAnchorDestination =
-BoostModuleAnchorSource;
-
-// This anchor is used to force the linker to link the BugproneModule.
-extern volatile int BugproneModuleAnchorSource;
-static int LLVM_ATTRIBUTE_UNUSED BugproneModuleAnchorDestination =
-BugproneModuleAnchorSource;
-
-// This anchor is used to force the linker to link the LLVMModule.
-extern volatile int LLVMModuleAnchorSource;
-static int LLVM_ATTRIBUTE_UNUSED LLVMModuleAnchorDestination =
-LLVMModuleAnchorSource;
-
-// This anchor is used to force the linker to link the CppCoreGuidelinesModule.
-extern volatile int CppCoreGuidelinesModuleAnchorSource;
-static int LLVM_ATTRIBUTE_UNUSED CppCoreGuidelinesModuleAnchorDestination =
-CppCoreGuidelinesModuleAnchorSource;
-
-// This anchor is used to force the linker to link the FuchsiaModule.
-extern volatile int FuchsiaModuleAnchorSource;
-static int LLVM_ATTRIBUTE_UNUSED FuchsiaModuleAnchorDestination =
-FuchsiaModuleAnchorSource;
-
-// This anchor is used to force the linker to link the GoogleModule.
-extern volatile int GoogleModuleAnchorSource;
-static int LLVM_ATTRIBUTE_UNUSED GoogleModuleAnchorDestination =
-GoogleModuleAnchorSource;
-
-// This anchor is used to force the linker to link the AndroidModule.
-extern volatile int AndroidModuleAnchorSource;
-static int LLVM_ATTRIBUTE_UNUSED AndroidModuleAnchorDestination =
-AndroidModuleAnchorSource;
-
-// This anchor is used to force the linker to link the MiscModule.
-extern volatile int MiscModuleAnchorSource;
-static int LLVM_ATTRIBUTE_UNUSED MiscModuleAnchorDestination =
-MiscModuleAnchorSource;
-
-// This anchor is used to force the linker to link the ModernizeModule.
-extern volatile int ModernizeModuleAnchorSource;
-static int LLVM_ATTRIBUTE_UNUSED ModernizeModuleAnchorDestination =
-ModernizeModuleAnchorSource;
-
-#if CLANG_ENABLE_STATIC_ANALYZER
-// This anchor is used to force the linker to link the MPIModule.
-extern volatile int MPIModuleAnchorSource;
-static int LLVM_ATTRIBUTE_UNUSED MPIModuleAnchorDestination =
-MPIModuleAnchorSource;
-#endif
-
-// This anchor is used to force the linker to link the PerformanceModule.
-extern volatile int PerformanceModuleAnchorSource;
-static int LLVM_ATTRIBUTE_UNUSED PerformanceModuleAnchorDestination =
-PerformanceModuleAnchorSource;
-
-// This anchor is used to force the linker to link the PortabilityModule.
-extern volatile int PortabilityModuleAnchorSource;
-static int LLVM_ATTRIBUTE_UNUSED PortabilityModuleAnchorDestination =
-PortabilityModuleAnchorSource;
-
-// This anchor is used to force the linker to link the ReadabilityModule.
-extern volatile int ReadabilityModuleAnchorSource;
-static int LLVM_ATTRIBUTE_UNUSED ReadabilityModuleAnchorDestination =
-ReadabilityModuleAnchorSource;
-
-// This anchor is used to force the linker to link the ObjCModule.
-extern volatile int ObjCModuleAnchorSource;
-static int LLVM_ATTRIBUTE_UNUSED ObjCModuleAnchorDestination =
-ObjCModuleAnchorSource;
-
-// This anchor is used to force the linker to link the HICPPModule.
-extern volatile int HICPPModuleAnchorSource;
-static int LLVM_ATTRIBUTE_UNUSED HICPPModuleAnchorDestination =
-HICPPModuleAnchorSource;
-
-// This anchor is used to force the linker to link the ZirconModule.
-extern volatile int ZirconModuleAnchorSource;
-static int LLVM_ATT

[PATCH] D55315: [clangd] Only reduce priority of a thread for indexing.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 178052.
kadircet marked 6 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55315

Files:
  clangd/Threading.cpp
  clangd/Threading.h
  clangd/index/Background.cpp
  clangd/index/Background.h

Index: clangd/index/Background.h
===
--- clangd/index/Background.h
+++ clangd/index/Background.h
@@ -13,6 +13,7 @@
 #include "Context.h"
 #include "FSProvider.h"
 #include "GlobalCompilationDatabase.h"
+#include "Threading.h"
 #include "index/FileIndex.h"
 #include "index/Index.h"
 #include "index/Serialization.h"
@@ -110,14 +111,14 @@
   // queue management
   using Task = std::function;
   void run(); // Main loop executed by Thread. Runs tasks from Queue.
-  void enqueueTask(Task T);
+  void enqueueTask(Task T, ThreadPriority Prioirty);
   void enqueueLocked(tooling::CompileCommand Cmd,
  BackgroundIndexStorage *IndexStorage);
-  std::mutex QueueMu;
+  std::mutex TasksMu;
   unsigned NumActiveTasks = 0; // Only idle when queue is empty *and* no tasks.
-  std::condition_variable QueueCV;
+  std::condition_variable TasksCV;
   bool ShouldStop = false;
-  std::deque Queue;
+  std::list> Tasks;
   std::vector ThreadPool; // FIXME: Abstract this away.
   GlobalCompilationDatabase::CommandChanged::Subscription CommandsChanged;
 };
Index: clangd/index/Background.cpp
===
--- clangd/index/Background.cpp
+++ clangd/index/Background.cpp
@@ -22,6 +22,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/SHA1.h"
@@ -102,14 +103,8 @@
   })) {
   assert(ThreadPoolSize > 0 && "Thread pool size can't be zero.");
   assert(this->IndexStorageFactory && "Storage factory can not be null!");
-  while (ThreadPoolSize--) {
+  while (ThreadPoolSize--)
 ThreadPool.emplace_back([this] { run(); });
-// Set priority to low, since background indexing is a long running task we
-// do not want to eat up cpu when there are any other high priority threads.
-// FIXME: In the future we might want a more general way of handling this to
-// support tasks with various priorities.
-setThreadPriority(ThreadPool.back(), ThreadPriority::Low);
-  }
 }
 
 BackgroundIndex::~BackgroundIndex() {
@@ -120,86 +115,109 @@
 
 void BackgroundIndex::stop() {
   {
-std::lock_guard Lock(QueueMu);
+std::lock_guard Lock(TasksMu);
 ShouldStop = true;
   }
-  QueueCV.notify_all();
+  TasksCV.notify_all();
 }
 
 void BackgroundIndex::run() {
   WithContext Background(BackgroundContext.clone());
   while (true) {
 Optional Task;
+ThreadPriority Priority;
 {
-  std::unique_lock Lock(QueueMu);
-  QueueCV.wait(Lock, [&] { return ShouldStop || !Queue.empty(); });
+  std::unique_lock Lock(TasksMu);
+  TasksCV.wait(Lock, [&] { return ShouldStop || !Tasks.empty(); });
   if (ShouldStop) {
-Queue.clear();
-QueueCV.notify_all();
+Tasks.clear();
+TasksCV.notify_all();
 return;
   }
   ++NumActiveTasks;
-  Task = std::move(Queue.front());
-  Queue.pop_front();
+  std::tie(Task, Priority) = std::move(Tasks.front());
+  Tasks.pop_front();
 }
+
+if (Priority != ThreadPriority::Normal)
+  setCurrentThreadPriority(Priority);
 (*Task)();
+if (Priority != ThreadPriority::Normal)
+  setCurrentThreadPriority(ThreadPriority::Normal);
+
 {
-  std::unique_lock Lock(QueueMu);
+  std::unique_lock Lock(TasksMu);
   assert(NumActiveTasks > 0 && "before decrementing");
   --NumActiveTasks;
 }
-QueueCV.notify_all();
+TasksCV.notify_all();
   }
 }
 
 bool BackgroundIndex::blockUntilIdleForTest(
 llvm::Optional TimeoutSeconds) {
-  std::unique_lock Lock(QueueMu);
-  return wait(Lock, QueueCV, timeoutSeconds(TimeoutSeconds),
-  [&] { return Queue.empty() && NumActiveTasks == 0; });
+  std::unique_lock Lock(TasksMu);
+  return wait(Lock, TasksCV, timeoutSeconds(TimeoutSeconds),
+  [&] { return Tasks.empty() && NumActiveTasks == 0; });
 }
 
 void BackgroundIndex::enqueue(const std::vector &ChangedFiles) {
-  enqueueTask([this, ChangedFiles] {
-trace::Span Tracer("BackgroundIndexEnqueue");
-// We're doing this asynchronously, because we'll read shards here too.
-// FIXME: read shards here too.
-
-log("Enqueueing {0} commands for indexing", ChangedFiles.size());
-SPAN_ATTACH(Tracer, "files", int64_t(ChangedFiles.size()));
-
-// We shuffle the files because processing them in a random order should
-// quickly give us good coverage of headers in th

[PATCH] D55649: [clangd] Enable cross-namespace completions by default in clangd

[Eric Liu via Phabricator via cfe-commits]

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



Comment at: clangd/tool/ClangdMain.cpp:144
 "can insert scope qualifiers."),
-cl::init(false), cl::Hidden);
+cl::init(true));
 

kadircet wrote:
> why not keep it hidden ?
We hid the flag because the feature was experimental. Now that it's the 
default, I think it makes sense to expose the flag.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55649



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

[PATCH] D55654: [clang] [Driver] [NetBSD] Add -D_REENTRANT when using sanitizers

[Michał Górny via Phabricator via cfe-commits]

mgorny created this revision.
mgorny added reviewers: krytarowski, vitalybuka, joerg, bkramer.
Herald added subscribers: jfb, cryptoad.

Requested by @krytarowski with the following rationale:

  We intend to support only reentrant interfaces in interceptors and if
  we use -lpthread without _REENTRANT defined, things are not guaranteed
  to work. This defined _REENTRANT is especially important for 
  and sanitization of interfaces around struct FILE. Some APIs have
  alternative modes depending on whether there is _REENTRANT enabled in
  the preprocessor namespace. We intend to sanitize only the _REENTRANT
  ones.


Repository:
  rC Clang

https://reviews.llvm.org/D55654

Files:
  lib/Driver/ToolChains/NetBSD.cpp
  lib/Driver/ToolChains/NetBSD.h


Index: lib/Driver/ToolChains/NetBSD.h
===
--- lib/Driver/ToolChains/NetBSD.h
+++ lib/Driver/ToolChains/NetBSD.h
@@ -76,6 +76,10 @@
 
   SanitizerMask getSupportedSanitizers() const override;
 
+  void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
+ llvm::opt::ArgStringList &CC1Args,
+ Action::OffloadKind DeviceOffloadKind) const 
override;
+
 protected:
   Tool *buildAssembler() const override;
   Tool *buildLinker() const override;
Index: lib/Driver/ToolChains/NetBSD.cpp
===
--- lib/Driver/ToolChains/NetBSD.cpp
+++ lib/Driver/ToolChains/NetBSD.cpp
@@ -457,3 +457,18 @@
   }
   return Res;
 }
+
+void NetBSD::addClangTargetOptions(const ArgList &,
+   ArgStringList &CC1Args,
+   Action::OffloadKind) const {
+  const SanitizerArgs &SanArgs = getSanitizerArgs();
+  if (SanArgs.needsAsanRt() || SanArgs.needsHwasanRt() ||
+  SanArgs.needsDfsanRt() || SanArgs.needsLsanRt() ||
+  SanArgs.needsMsanRt() || SanArgs.needsTsanRt() ||
+  SanArgs.needsUbsanRt() || SanArgs.needsSafeStackRt() ||
+  SanArgs.needsCfiRt() || SanArgs.needsCfiDiagRt() ||
+  SanArgs.needsStatsRt() || SanArgs.needsEsanRt() ||
+  SanArgs.needsScudoRt()) {
+CC1Args.push_back("-D_REENTRANT");
+  }
+}


Index: lib/Driver/ToolChains/NetBSD.h
===
--- lib/Driver/ToolChains/NetBSD.h
+++ lib/Driver/ToolChains/NetBSD.h
@@ -76,6 +76,10 @@
 
   SanitizerMask getSupportedSanitizers() const override;
 
+  void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
+ llvm::opt::ArgStringList &CC1Args,
+ Action::OffloadKind DeviceOffloadKind) const override;
+
 protected:
   Tool *buildAssembler() const override;
   Tool *buildLinker() const override;
Index: lib/Driver/ToolChains/NetBSD.cpp
===
--- lib/Driver/ToolChains/NetBSD.cpp
+++ lib/Driver/ToolChains/NetBSD.cpp
@@ -457,3 +457,18 @@
   }
   return Res;
 }
+
+void NetBSD::addClangTargetOptions(const ArgList &,
+   ArgStringList &CC1Args,
+   Action::OffloadKind) const {
+  const SanitizerArgs &SanArgs = getSanitizerArgs();
+  if (SanArgs.needsAsanRt() || SanArgs.needsHwasanRt() ||
+  SanArgs.needsDfsanRt() || SanArgs.needsLsanRt() ||
+  SanArgs.needsMsanRt() || SanArgs.needsTsanRt() ||
+  SanArgs.needsUbsanRt() || SanArgs.needsSafeStackRt() ||
+  SanArgs.needsCfiRt() || SanArgs.needsCfiDiagRt() ||
+  SanArgs.needsStatsRt() || SanArgs.needsEsanRt() ||
+  SanArgs.needsScudoRt()) {
+CC1Args.push_back("-D_REENTRANT");
+  }
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55483: Introduce the callback attribute and emit !callback metadata

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert marked 2 inline comments as done.
jdoerfert added inline comments.



Comment at: include/clang/Basic/Builtins.def:942
 LIBBUILTIN(alloca, "v*z", "f", "stdlib.h", ALL_GNU_LANGUAGES)
+LIBBUILTIN(qsort_r, "",   "fC<3,-1,-1,4>", "stdlib.h", 
ALL_GNU_LANGUAGES)
 // POSIX string.h

arichardson wrote:
> qsort_r callback argument order is different on Linux, macOS and FreeBSD so 
> those constants can't be hardcoded: 
> Linux:
> `void qsort_r(void *base, size_t nmemb, size_t size, int (*compar)(const void 
> *, const void *, void *), void *arg);`
> FreeBSD:
> `void qsort_r(void *base, size_t nmemb, size_t size, void *thunk, int 
> (*compar)(void *, const void *, const void*));`
> macos:
> `void qsort_r(void *base, size_t nel, size_t width, void *thunk, int 
> (*compar)(void *, const void *, const void *));`
Really good point. I'll remove qsort_r in the next version I guess. If we find 
another similar function we can extend the encoding to sth like:  
`C:(<3,-1,-1,4>|<5,4,-1,-1>)` which represents both possibilities. The 
`Builtin::Context::performsCallback(...)` will then need to check which is 
appropriate based on the type of the actual declaration.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55483



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

[PATCH] D55417: [clangd] Change disk-backed storage to be atomic

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 178059.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Use creauteUniqueFile.
- Don't remove on success.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55417

Files:
  clangd/index/BackgroundIndexStorage.cpp


Index: clangd/index/BackgroundIndexStorage.cpp
===
--- clangd/index/BackgroundIndexStorage.cpp
+++ clangd/index/BackgroundIndexStorage.cpp
@@ -9,6 +9,8 @@
 
 #include "Logger.h"
 #include "index/Background.h"
+#include "llvm/ADT/ScopeExit.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
@@ -33,6 +35,33 @@
   return ShardRootSS.str();
 }
 
+template 
+llvm::Error writeAtomically(llvm::StringRef OutPath, WriterFunction &&Writer) {
+  // Write to a temporary file first.
+  llvm::SmallString<128> TempPath;
+  int FD;
+  auto EC = llvm::sys::fs::createUniqueFile(
+  llvm::Twine(OutPath, ".tmp."), FD, TempPath);
+  if (EC)
+return llvm::errorCodeToError(EC);
+  // Make sure temp file is destroyed on failure.
+  auto RemoveOnFail =
+  llvm::make_scope_exit([TempPath] { llvm::sys::fs::remove(TempPath); });
+  llvm::raw_fd_ostream OS(FD, /*shouldClose=*/true);
+  Writer(OS);
+  OS.close();
+  if (OS.has_error())
+return llvm::errorCodeToError(OS.error());
+  // Then move to real location.
+  EC = llvm::sys::fs::rename(TempPath, OutPath);
+  if (EC)
+return llvm::errorCodeToError(EC);
+  // If everything went well, we already moved the file to another name. So
+  // don't delete it, as it might be taken by another file.
+  RemoveOnFail.release();
+  return llvm::ErrorSuccess();
+}
+
 // Uses disk as a storage for index shards. Creates a directory called
 // ".clangd-index/" under the path provided during construction.
 class DiskBackedIndexStorage : public BackgroundIndexStorage {
@@ -70,14 +99,9 @@
 
   llvm::Error storeShard(llvm::StringRef ShardIdentifier,
  IndexFileOut Shard) const override {
-auto ShardPath = getShardPathFromFilePath(DiskShardRoot, ShardIdentifier);
-std::error_code EC;
-llvm::raw_fd_ostream OS(ShardPath, EC);
-if (EC)
-  return llvm::errorCodeToError(EC);
-OS << Shard;
-OS.close();
-return llvm::errorCodeToError(OS.error());
+return writeAtomically(
+getShardPathFromFilePath(DiskShardRoot, ShardIdentifier),
+[&Shard](llvm::raw_ostream &OS) { OS << Shard; });
   }
 };
 


Index: clangd/index/BackgroundIndexStorage.cpp
===
--- clangd/index/BackgroundIndexStorage.cpp
+++ clangd/index/BackgroundIndexStorage.cpp
@@ -9,6 +9,8 @@
 
 #include "Logger.h"
 #include "index/Background.h"
+#include "llvm/ADT/ScopeExit.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
@@ -33,6 +35,33 @@
   return ShardRootSS.str();
 }
 
+template 
+llvm::Error writeAtomically(llvm::StringRef OutPath, WriterFunction &&Writer) {
+  // Write to a temporary file first.
+  llvm::SmallString<128> TempPath;
+  int FD;
+  auto EC = llvm::sys::fs::createUniqueFile(
+  llvm::Twine(OutPath, ".tmp."), FD, TempPath);
+  if (EC)
+return llvm::errorCodeToError(EC);
+  // Make sure temp file is destroyed on failure.
+  auto RemoveOnFail =
+  llvm::make_scope_exit([TempPath] { llvm::sys::fs::remove(TempPath); });
+  llvm::raw_fd_ostream OS(FD, /*shouldClose=*/true);
+  Writer(OS);
+  OS.close();
+  if (OS.has_error())
+return llvm::errorCodeToError(OS.error());
+  // Then move to real location.
+  EC = llvm::sys::fs::rename(TempPath, OutPath);
+  if (EC)
+return llvm::errorCodeToError(EC);
+  // If everything went well, we already moved the file to another name. So
+  // don't delete it, as it might be taken by another file.
+  RemoveOnFail.release();
+  return llvm::ErrorSuccess();
+}
+
 // Uses disk as a storage for index shards. Creates a directory called
 // ".clangd-index/" under the path provided during construction.
 class DiskBackedIndexStorage : public BackgroundIndexStorage {
@@ -70,14 +99,9 @@
 
   llvm::Error storeShard(llvm::StringRef ShardIdentifier,
  IndexFileOut Shard) const override {
-auto ShardPath = getShardPathFromFilePath(DiskShardRoot, ShardIdentifier);
-std::error_code EC;
-llvm::raw_fd_ostream OS(ShardPath, EC);
-if (EC)
-  return llvm::errorCodeToError(EC);
-OS << Shard;
-OS.close();
-return llvm::errorCodeToError(OS.error());
+return writeAtomically(
+getShardPathFromFilePath(DiskShardRoot, ShardIdentifier),
+[&Shard](llvm::raw_ostream &OS) { OS << Shard; });
   }
 };
 
___
cfe-commits mailing list

[PATCH] D53699: [ASTImporter] Fix inequality of functions with different attributes

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 178060.
martong marked 2 inline comments as done.
martong added a comment.

- Add more tests and simplify comment


Repository:
  rC Clang

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

https://reviews.llvm.org/D53699

Files:
  lib/AST/ASTStructuralEquivalence.cpp
  unittests/AST/StructuralEquivalenceTest.cpp


Index: unittests/AST/StructuralEquivalenceTest.cpp
===
--- unittests/AST/StructuralEquivalenceTest.cpp
+++ unittests/AST/StructuralEquivalenceTest.cpp
@@ -370,6 +370,31 @@
   EXPECT_FALSE(testStructuralMatch(t));
 }
 
+TEST_F(StructuralEquivalenceFunctionTest, FunctionsWithDifferentNoreturnAttr) {
+  auto t = makeNamedDecls(
+  "__attribute__((noreturn)) void foo();",
+  "  void foo();",
+  Lang_C);
+  EXPECT_TRUE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceFunctionTest,
+FunctionsWithDifferentCallingConventions) {
+  auto t = makeNamedDecls(
+  "__attribute__((fastcall)) void foo();",
+  "__attribute__((ms_abi))   void foo();",
+  Lang_C);
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceFunctionTest, FunctionsWithDifferentSavedRegsAttr) 
{
+  auto t = makeNamedDecls(
+  "__attribute__((no_caller_saved_registers)) void foo();",
+  "   void foo();",
+  Lang_C);
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
 struct StructuralEquivalenceCXXMethodTest : StructuralEquivalenceTest {
 };
 
Index: lib/AST/ASTStructuralEquivalence.cpp
===
--- lib/AST/ASTStructuralEquivalence.cpp
+++ lib/AST/ASTStructuralEquivalence.cpp
@@ -297,6 +297,32 @@
   return true;
 }
 
+/// Determine structural equivalence based on the ExtInfo of functions. This
+/// is inspired by ASTContext::mergeFunctionTypes(), we compare calling
+/// conventions bits but must not compare some other bits.
+static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
+ FunctionType::ExtInfo EI1,
+ FunctionType::ExtInfo EI2) {
+  // Compatible functions must have compatible calling conventions.
+  if (EI1.getCC() != EI2.getCC())
+return false;
+
+  // Regparm is part of the calling convention.
+  if (EI1.getHasRegParm() != EI2.getHasRegParm())
+return false;
+  if (EI1.getRegParm() != EI2.getRegParm())
+return false;
+
+  if (EI1.getProducesResult() != EI2.getProducesResult())
+return false;
+  if (EI1.getNoCallerSavedRegs() != EI2.getNoCallerSavedRegs())
+return false;
+  if (EI1.getNoCfCheck() != EI2.getNoCfCheck())
+return false;
+
+  return true;
+}
+
 /// Determine structural equivalence of two types.
 static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
  QualType T1, QualType T2) {
@@ -540,7 +566,8 @@
 if (!IsStructurallyEquivalent(Context, Function1->getReturnType(),
   Function2->getReturnType()))
   return false;
-if (Function1->getExtInfo() != Function2->getExtInfo())
+if (!IsStructurallyEquivalent(Context, Function1->getExtInfo(),
+  Function2->getExtInfo()))
   return false;
 break;
   }


Index: unittests/AST/StructuralEquivalenceTest.cpp
===
--- unittests/AST/StructuralEquivalenceTest.cpp
+++ unittests/AST/StructuralEquivalenceTest.cpp
@@ -370,6 +370,31 @@
   EXPECT_FALSE(testStructuralMatch(t));
 }
 
+TEST_F(StructuralEquivalenceFunctionTest, FunctionsWithDifferentNoreturnAttr) {
+  auto t = makeNamedDecls(
+  "__attribute__((noreturn)) void foo();",
+  "  void foo();",
+  Lang_C);
+  EXPECT_TRUE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceFunctionTest,
+FunctionsWithDifferentCallingConventions) {
+  auto t = makeNamedDecls(
+  "__attribute__((fastcall)) void foo();",
+  "__attribute__((ms_abi))   void foo();",
+  Lang_C);
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceFunctionTest, FunctionsWithDifferentSavedRegsAttr) {
+  auto t = makeNamedDecls(
+  "__attribute__((no_caller_saved_registers)) void foo();",
+  "   void foo();",
+  Lang_C);
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
 struct StructuralEquivalenceCXXMethodTest : StructuralEquivalenceTest {
 };
 
Index: lib/AST/ASTStructuralEquivalence.cpp
===
--- lib/AST/ASTStructuralEquivalence.cpp
+++ lib/AST/ASTStructuralEquivalence.cpp
@@ -297,6 +297,32 @@
   return true;
 }
 
+/// Determine structural equivalence based on the ExtInfo of functions. This
+/// is inspired by ASTContext::mergeFunctionTypes(), we compare calling
+///

[PATCH] D53699: [ASTImporter] Fix inequality of functions with different attributes

[Gabor Marton via Phabricator via cfe-commits]

martong marked 4 inline comments as done.
martong added inline comments.



Comment at: lib/AST/ASTStructuralEquivalence.cpp:302
+/// is inspired by ASTContext::mergeFunctionTypes(), we compare calling
+/// conventions bits but must not compare some other bits, e.g. the noreturn
+/// bit.

shafik wrote:
> This comment is confusing b/c it looks like the noreturn bits are the only 
> one you are not checking.
Ok, I have removed that part of the comment.



Comment at: unittests/AST/StructuralEquivalenceTest.cpp:373
 
+TEST_F(StructuralEquivalenceFunctionTest,
+FunctionsWithDifferentAttributesButSameTypesShouldBeEqual) {

shafik wrote:
> Can we get some more tests to be a little more thorough and can we also get a 
> test where it is expected to to be false as well?
Ok, I have adder a few more tests and renamed the existing one.


Repository:
  rC Clang

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

https://reviews.llvm.org/D53699



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

[PATCH] D55649: [clangd] Enable cross-namespace completions by default in clangd

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clangd/tool/ClangdMain.cpp:144
 "can insert scope qualifiers."),
-cl::init(false), cl::Hidden);
+cl::init(true));
 

ioeric wrote:
> kadircet wrote:
> > why not keep it hidden ?
> We hid the flag because the feature was experimental. Now that it's the 
> default, I think it makes sense to expose the flag.
I wouldn't expect anyone to turn it off, but not that important.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55649



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

[PATCH] D55649: [clangd] Enable cross-namespace completions by default in clangd

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.

LGTM. When we know everyone is happy with the new behaivour, we can remove the 
flag altogether.




Comment at: clangd/tool/ClangdMain.cpp:144
 "can insert scope qualifiers."),
-cl::init(false), cl::Hidden);
+cl::init(true));
 

kadircet wrote:
> ioeric wrote:
> > kadircet wrote:
> > > why not keep it hidden ?
> > We hid the flag because the feature was experimental. Now that it's the 
> > default, I think it makes sense to expose the flag.
> I wouldn't expect anyone to turn it off, but not that important.
+1 to making it visible. Now that actually enable it by default, it would be 
nice if the users could find it and turn it off in case they need it.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55649



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

[PATCH] D55656: [OpenCL] Address space for default class members

[Mikael Nilsson via Phabricator via cfe-commits]

mikael created this revision.
mikael added reviewers: Anastasia, rjmccall.
Herald added subscribers: cfe-commits, yaxunl.

- Implicity and explicity defaulted class members
- Resolved the FIXMEs in addrspace-of-this.cl


Repository:
  rC Clang

https://reviews.llvm.org/D55656

Files:
  include/clang/Sema/Sema.h
  lib/AST/ASTContext.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaInit.cpp
  test/CodeGenOpenCLCXX/addrspace-of-this.cl
  test/CodeGenOpenCLCXX/explicitly-defaulted-methods.cl
  test/CodeGenOpenCLCXX/implicitly-defaulted-methods.cl

Index: test/CodeGenOpenCLCXX/implicitly-defaulted-methods.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/implicitly-defaulted-methods.cl
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -emit-llvm -pedantic -verify -O0
+// expected-no-diagnostics
+
+// Test implicitly defaulted special member functions and their invocation.
+// The 'this' address space is expected to be generic.
+
+// CHECK-LABEL:  _Z3foov
+// CHECK: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* %0)
+
+// CHECK-LABEL: _ZNU3AS41CC1Ev(%class.C addrspace(4)* %this)
+// CHECK:   %this.addr = alloca %class.C addrspace(4)*, align 4
+// CHECK:   store %class.C addrspace(4)* %this, %class.C addrspace(4)** %this.addr, align 4
+// CHECK:   %this1 = load %class.C addrspace(4)*, %class.C addrspace(4)** %this.addr, align 4
+// CHECK:   call void @_ZNU3AS41CC2Ev(%class.C addrspace(4)* %this1) #4
+// CHECK:   ret void
+
+class C {
+public:
+  int a = 0;
+};
+
+extern C&& bar();
+
+__kernel void foo() {
+  C c;
+  C c2(c);
+  C c3(bar());
+  C c5 = bar();
+  C c6 = c;
+  // To force constructors to be CodeGen:ed
+  int x = c6.a + c.a;
+}
+
Index: test/CodeGenOpenCLCXX/explicitly-defaulted-methods.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/explicitly-defaulted-methods.cl
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -emit-llvm -pedantic -verify -O0 -o - | FileCheck %s
+// expected-no-diagnostics
+
+// Test explicitly defaulted special member functions and their invocation.
+// The 'this' address space is expected to be generic.
+
+// FIXME: Other address spaces?
+
+// CHECK-LABEL:  _Z3foov
+// CHECK: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* %0)
+
+// CHECK-LABEL: @_ZNU3AS41CC2Ev(%class.C addrspace(4)* %this)
+// CHECK:   %this.addr = alloca %class.C addrspace(4)*, align 4
+// CHECK:   store %class.C addrspace(4)* %this, %class.C addrspace(4)** %this.addr, align 4
+// CHECK:   %this1 = load %class.C addrspace(4)*, %class.C addrspace(4)** %this.addr, align 4
+// CHECK:   ret void
+
+class C {
+public:
+  C() {}
+  C(const C &) = default;
+  C(C &&) = default;
+  C &operator=(const C &) = default;
+  C &operator=(C&&) & = default;
+};
+
+extern C&& bar();
+
+__kernel void foo() {
+  C c;
+  C c2(c);
+  C c3(bar());
+  C c5 = bar();
+  C c6 = c;
+}
Index: test/CodeGenOpenCLCXX/addrspace-of-this.cl
===
--- test/CodeGenOpenCLCXX/addrspace-of-this.cl
+++ test/CodeGenOpenCLCXX/addrspace-of-this.cl
@@ -9,18 +9,21 @@
 public:
   int v;
   C() { v = 2; }
-  // FIXME: Does not work yet.
-  // C(C &&c) { v = c.v; }
+  C(C &&c) { v = c.v; }
   C(const C &c) { v = c.v; }
   C &operator=(const C &c) {
 v = c.v;
 return *this;
   }
-  // FIXME: Does not work yet.
-  //C &operator=(C&& c) & {
-  //  v = c.v;
-  //  return *this;
-  //}
+  C &operator=(C &&c) & {
+v = c.v;
+return *this;
+  }
+
+  C operator+(const C& c) {
+v += c.v;
+return *this;
+  }
 
   int get() { return v; }
 
@@ -41,15 +44,13 @@
   C c1(c);
   C c2;
   c2 = c1;
-  // FIXME: Does not work yet.
-  // C c3 = c1 + c2;
-  // C c4(foo());
-  // C c5 = foo();
-
+  C c3 = c1 + c2;
+  C c4(foo());
+  C c5 = foo();
 }
 
 // CHECK-LABEL: @__cxx_global_var_init()
-// CHECK: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast (%class.C addrspace(1)* @c to %class.C addrspace(4)*)) #4
+// CHECK: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast (%class.C addrspace(1)* @c to %class.C addrspace(4)*))
 
 // Test that the address space is __generic for the constructor
 // CHECK-LABEL: @_ZNU3AS41CC1Ev(%class.C addrspace(4)* %this)
@@ -57,7 +58,7 @@
 // CHECK:   %this.addr = alloca %class.C addrspace(4)*, align 4
 // CHECK:   store %class.C addrspace(4)* %this, %class.C addrspace(4)** %this.addr, align 4
 // CHECK:   %this1 = load %class.C addrspace(4)*, %class.C addrspace(4)** %this.addr, align 4
-// CHECK:   call void @_ZNU3AS41CC2Ev(%class.C addrspace(4)* %this1) #4
+// CHECK:   call void @_ZNU3AS41CC2Ev(%class.C addrspace(4)* %this1)
 // CHECK:   ret void
 
 // CHECK-LABEL: @_Z12test__globalv()
@@ -74,13 +75,29 @@
 
 // Test the address space of 'this' when invoking a constructor.
 // CHECK:   %1 = addrspacecast %class.C* %c2 to %class.C addrspace(4)*
-// CHECK:   

[clang-tools-extra] r349049 - [clangd] Enable cross-namespace completions by default in clangd

[Eric Liu via cfe-commits]

Author: ioeric
Date: Thu Dec 13 07:35:43 2018
New Revision: 349049

URL: http://llvm.org/viewvc/llvm-project?rev=349049&view=rev
Log:
[clangd] Enable cross-namespace completions by default in clangd

Summary:
Code completion will suggest symbols from any scope (incl. inaccessible
scopes) when there's no qualifier explicitly specified. E.g.
{F7689815}

As we are assigning relatively low scores for cross-namespace completion items, 
the overall code completion quality doesn't regress. The feature has been tried 
out by a few folks, and the feedback is generally positive, so I think it 
should be ready to be enabled by default.

Reviewers: hokein, ilya-biryukov, kadircet

Reviewed By: hokein, ilya-biryukov

Subscribers: MaskRay, jkorous, arphaman, cfe-commits

Differential Revision: https://reviews.llvm.org/D55649

Modified:
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp

Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=349049&r1=349048&r2=349049&view=diff
==
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Thu Dec 13 07:35:43 2018
@@ -141,7 +141,7 @@ static cl::opt AllScopesCompletion
 "not defined in the scopes (e.g. "
 "namespaces) visible from the code completion point. Such completions "
 "can insert scope qualifiers."),
-cl::init(false), cl::Hidden);
+cl::init(true));
 
 static cl::opt
 ShowOrigins("debug-origin", cl::desc("Show origins of completion items"),


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

[PATCH] D55649: [clangd] Enable cross-namespace completions by default in clangd

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL349049: [clangd] Enable cross-namespace completions by 
default in clangd (authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55649

Files:
  clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
@@ -141,7 +141,7 @@
 "not defined in the scopes (e.g. "
 "namespaces) visible from the code completion point. Such completions "
 "can insert scope qualifiers."),
-cl::init(false), cl::Hidden);
+cl::init(true));
 
 static cl::opt
 ShowOrigins("debug-origin", cl::desc("Show origins of completion items"),


Index: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
@@ -141,7 +141,7 @@
 "not defined in the scopes (e.g. "
 "namespaces) visible from the code completion point. Such completions "
 "can insert scope qualifiers."),
-cl::init(false), cl::Hidden);
+cl::init(true));
 
 static cl::opt
 ShowOrigins("debug-origin", cl::desc("Show origins of completion items"),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r349050 - [CodeComplete] Set preferred type to bool on conditions

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Thu Dec 13 07:36:32 2018
New Revision: 349050

URL: http://llvm.org/viewvc/llvm-project?rev=349050&view=rev
Log:
[CodeComplete] Set preferred type to bool on conditions

Reviewers: kadircet

Reviewed By: kadircet

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D55431

Added:
cfe/trunk/test/CodeCompletion/preferred-type.cpp
Modified:
cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp
cfe/trunk/lib/Sema/SemaCodeComplete.cpp

Modified: cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp?rev=349050&r1=349049&r2=349050&view=diff
==
--- cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp (original)
+++ cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp Thu Dec 13 07:36:32 2018
@@ -539,9 +539,12 @@ void PrintingCodeCompleteConsumer::Proce
 unsigned NumResults) {
   std::stable_sort(Results, Results + NumResults);
 
-  StringRef Filter = SemaRef.getPreprocessor().getCodeCompletionFilter();
+  if (!Context.getPreferredType().isNull())
+OS << "PREFERRED-TYPE: " << Context.getPreferredType().getAsString()
+   << "\n";
 
-  // Print the results.
+  StringRef Filter = SemaRef.getPreprocessor().getCodeCompletionFilter();
+  // Print the completions.
   for (unsigned I = 0; I != NumResults; ++I) {
 if (!Filter.empty() && isResultFilteredOut(Filter, Results[I]))
   continue;

Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=349050&r1=349049&r2=349050&view=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Thu Dec 13 07:36:32 2018
@@ -3521,7 +3521,7 @@ static void HandleCodeCompleteResults(Se
 CodeCompleter->ProcessCodeCompleteResults(*S, Context, Results, 
NumResults);
 }
 
-static enum CodeCompletionContext::Kind
+static CodeCompletionContext
 mapCodeCompletionContext(Sema &S, Sema::ParserCompletionContext PCC) {
   switch (PCC) {
   case Sema::PCC_Namespace:
@@ -3558,8 +3558,10 @@ mapCodeCompletionContext(Sema &S, Sema::
   return CodeCompletionContext::CCC_Expression;
 
   case Sema::PCC_Expression:
-  case Sema::PCC_Condition:
 return CodeCompletionContext::CCC_Expression;
+  case Sema::PCC_Condition:
+return CodeCompletionContext(CodeCompletionContext::CCC_Expression,
+ S.getASTContext().BoolTy);
 
   case Sema::PCC_Statement:
 return CodeCompletionContext::CCC_Statement;

Added: cfe/trunk/test/CodeCompletion/preferred-type.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeCompletion/preferred-type.cpp?rev=349050&view=auto
==
--- cfe/trunk/test/CodeCompletion/preferred-type.cpp (added)
+++ cfe/trunk/test/CodeCompletion/preferred-type.cpp Thu Dec 13 07:36:32 2018
@@ -0,0 +1,15 @@
+void test(bool x) {
+  if (x) {}
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:2:7 %s | FileCheck %s
+  // CHECK: PREFERRED-TYPE: _Bool
+
+  while (x) {}
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:6:10 %s | FileCheck 
%s
+
+  for (; x;) {}
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:9:10 %s | FileCheck 
%s
+
+  // FIXME(ibiryukov): the condition in do-while is parsed as expression, so we
+  // fail to detect it should be converted to bool.
+  // do {} while (x);
+}


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

[PATCH] D55431: [CodeComplete] Set preferred type to bool on conditions

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL349050: [CodeComplete] Set preferred type to bool on 
conditions (authored by ibiryukov, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55431

Files:
  cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp
  cfe/trunk/lib/Sema/SemaCodeComplete.cpp
  cfe/trunk/test/CodeCompletion/preferred-type.cpp


Index: cfe/trunk/test/CodeCompletion/preferred-type.cpp
===
--- cfe/trunk/test/CodeCompletion/preferred-type.cpp
+++ cfe/trunk/test/CodeCompletion/preferred-type.cpp
@@ -0,0 +1,15 @@
+void test(bool x) {
+  if (x) {}
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:2:7 %s | FileCheck %s
+  // CHECK: PREFERRED-TYPE: _Bool
+
+  while (x) {}
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:6:10 %s | FileCheck 
%s
+
+  for (; x;) {}
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:9:10 %s | FileCheck 
%s
+
+  // FIXME(ibiryukov): the condition in do-while is parsed as expression, so we
+  // fail to detect it should be converted to bool.
+  // do {} while (x);
+}
Index: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
===
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp
@@ -3521,7 +3521,7 @@
 CodeCompleter->ProcessCodeCompleteResults(*S, Context, Results, 
NumResults);
 }
 
-static enum CodeCompletionContext::Kind
+static CodeCompletionContext
 mapCodeCompletionContext(Sema &S, Sema::ParserCompletionContext PCC) {
   switch (PCC) {
   case Sema::PCC_Namespace:
@@ -3558,8 +3558,10 @@
   return CodeCompletionContext::CCC_Expression;
 
   case Sema::PCC_Expression:
-  case Sema::PCC_Condition:
 return CodeCompletionContext::CCC_Expression;
+  case Sema::PCC_Condition:
+return CodeCompletionContext(CodeCompletionContext::CCC_Expression,
+ S.getASTContext().BoolTy);
 
   case Sema::PCC_Statement:
 return CodeCompletionContext::CCC_Statement;
Index: cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp
===
--- cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp
+++ cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp
@@ -539,9 +539,12 @@
 unsigned NumResults) {
   std::stable_sort(Results, Results + NumResults);
 
-  StringRef Filter = SemaRef.getPreprocessor().getCodeCompletionFilter();
+  if (!Context.getPreferredType().isNull())
+OS << "PREFERRED-TYPE: " << Context.getPreferredType().getAsString()
+   << "\n";
 
-  // Print the results.
+  StringRef Filter = SemaRef.getPreprocessor().getCodeCompletionFilter();
+  // Print the completions.
   for (unsigned I = 0; I != NumResults; ++I) {
 if (!Filter.empty() && isResultFilteredOut(Filter, Results[I]))
   continue;


Index: cfe/trunk/test/CodeCompletion/preferred-type.cpp
===
--- cfe/trunk/test/CodeCompletion/preferred-type.cpp
+++ cfe/trunk/test/CodeCompletion/preferred-type.cpp
@@ -0,0 +1,15 @@
+void test(bool x) {
+  if (x) {}
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:2:7 %s | FileCheck %s
+  // CHECK: PREFERRED-TYPE: _Bool
+
+  while (x) {}
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:6:10 %s | FileCheck %s
+
+  for (; x;) {}
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:9:10 %s | FileCheck %s
+
+  // FIXME(ibiryukov): the condition in do-while is parsed as expression, so we
+  // fail to detect it should be converted to bool.
+  // do {} while (x);
+}
Index: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
===
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp
@@ -3521,7 +3521,7 @@
 CodeCompleter->ProcessCodeCompleteResults(*S, Context, Results, NumResults);
 }
 
-static enum CodeCompletionContext::Kind
+static CodeCompletionContext
 mapCodeCompletionContext(Sema &S, Sema::ParserCompletionContext PCC) {
   switch (PCC) {
   case Sema::PCC_Namespace:
@@ -3558,8 +3558,10 @@
   return CodeCompletionContext::CCC_Expression;
 
   case Sema::PCC_Expression:
-  case Sema::PCC_Condition:
 return CodeCompletionContext::CCC_Expression;
+  case Sema::PCC_Condition:
+return CodeCompletionContext(CodeCompletionContext::CCC_Expression,
+ S.getASTContext().BoolTy);
 
   case Sema::PCC_Statement:
 return CodeCompletionContext::CCC_Statement;
Index: cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp
===
--- cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp
+++ cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp
@@ -539,9 +539,12 @@
 unsigned NumResults) {
   std::stable_sort(Results, Results +

[PATCH] D55656: [OpenCL] Address space for default class members

[Mikael Nilsson via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaInit.cpp:4539
+  if (InitCategory.isPRValue() || InitCategory.isXValue())
+T1Quals.removeAddressSpace();
+

This is probably not the correct way to solve this. But it fixes the issues 
that i get. 

The code that triggers this issue is found in 
test/CodeGenOpenCLCXX/addrspace-of-this.cl:47

```
C c3 = c1 + c2;
```

So the original (InitializedEntity) Entity.getType()  looks like:

```
RValueReferenceType 0x717470 '__generic class C &&'
`-QualType 0x717458 '__generic class C' __generic
  `-RecordType 0x717160 'class C'
`-CXXRecord 0x7170d0 'C'
```

It might be so that we actually want to qualify the RValueReferenceType rather 
than the RecordType here. 


Repository:
  rC Clang

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

https://reviews.llvm.org/D55656



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

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

[Nico Weber via Phabricator via cfe-commits]

thakis accepted this revision.
thakis added a comment.
This revision is now accepted and ready to land.

Firefox is good enough.

Thanks for finding 
http://comments.gmane.org/gmane.comp.compilers.clang.scm/47203 -- this was from 
before we did reviews on phab, so I was able to find the review in my inbox. I 
can't find the full discussion online (we also moved list servers :-/ Fragments 
are at http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20120220.txt ). 
Here's the discussion between Richard and me about the behavior you're changing:

Richard: """

> When building all of chromium and its many dependencies, the warning
>  did also find 3 false positives, but they all look alike: Ffmepg
>  contains three different functions that all look like this:

4 bugs and 3 false positives doesn't sound great to me. Have you considered
relaxing the warning in cases where the integral summand is a constant
expression and is in-bounds? Would this pattern have matched any of your
real bugs?
"""

Me: """

> 4 bugs and 3 false positives doesn't sound great to me.

True. All of the the 3 false positives are really the same snippet
though. (On the other hand, 3 of the 4 bugs are very similar as well.)
The bugs this did find are crash bugs, and they were in rarely run
error-logging code, so it does find interesting bugs. Maybe you guys
can run it on your codebase after it's in, and we can move it out of
-Wmost if you consider it too noisy in practice?

> Have you considered
>  relaxing the warning in cases where the integral summand is a constant
>  expression and is in-bounds? Would this pattern have matched any of your
>  real bugs?

That's a good idea (for my bugs, the int was always a declrefexpr,
never a literal). The ffmpeg example roughly looks like this:

  #define A "foo"
  #define B "bar"
  consume(A B + sizeof(A) - 1);

The RHS is just "sizeof(A)" without the "- 1", but since A B has a
length of 6, this still makes this warning go away. I added this to
the patch. With this change, it's 4 bugs / 0 false positives.

Note that this suppresses the warning for most enums which start at 0.
Or did you mean to do this only for non-enum constant expressions?
"""

Richard: """

> Note that this suppresses the warning for most enums which start at 0.
>  Or did you mean to do this only for non-enum constant expressions?

I could imagine code legitimately wanting to do something like this:

template
struct S {
  enum { Offset = /* ... */ };
  static const char *str = "some string" + Offset;
};

That said, I'm happy for us to warn on such code. Whatever you prefer seems 
fine to me; we can refine this later, if a need arises.
"""

Given that discussion, your results on Firefox, and that this is useful to you 
sound like it should be fine to land this. So let's do that and see what 
happens. If anyone out there has any feedback on this change, we're all ears :-)


Repository:
  rC Clang

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

https://reviews.llvm.org/D55382



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

[PATCH] D55648: [CodeComplete] Fill preferred type on binary expressions

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 178069.
ilya-biryukov marked 4 inline comments as done.
ilya-biryukov added a comment.

- Address review comments


Repository:
  rC Clang

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

https://reviews.llvm.org/D55648

Files:
  include/clang/Sema/Sema.h
  lib/Parse/ParseExpr.cpp
  lib/Sema/SemaCodeComplete.cpp
  unittests/Sema/CodeCompleteTest.cpp

Index: unittests/Sema/CodeCompleteTest.cpp
===
--- unittests/Sema/CodeCompleteTest.cpp
+++ unittests/Sema/CodeCompleteTest.cpp
@@ -14,31 +14,39 @@
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaDiagnostic.h"
 #include "clang/Tooling/Tooling.h"
-#include "gtest/gtest.h"
 #include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+#include 
 
 namespace {
 
 using namespace clang;
 using namespace clang::tooling;
+using ::testing::Each;
 using ::testing::UnorderedElementsAre;
 
 const char TestCCName[] = "test.cc";
-using VisitedContextResults = std::vector;
 
-class VisitedContextFinder: public CodeCompleteConsumer {
+struct CompletionContext {
+  std::vector VisitedNamespaces;
+  std::string PreferredType;
+};
+
+class VisitedContextFinder : public CodeCompleteConsumer {
 public:
-  VisitedContextFinder(VisitedContextResults &Results)
+  VisitedContextFinder(CompletionContext &ResultCtx)
   : CodeCompleteConsumer(/*CodeCompleteOpts=*/{},
  /*CodeCompleteConsumer*/ false),
-VCResults(Results),
+ResultCtx(ResultCtx),
 CCTUInfo(std::make_shared()) {}
 
   void ProcessCodeCompleteResults(Sema &S, CodeCompletionContext Context,
   CodeCompletionResult *Results,
   unsigned NumResults) override {
-VisitedContexts = Context.getVisitedContexts();
-VCResults = getVisitedNamespace();
+ResultCtx.VisitedNamespaces =
+getVisitedNamespace(Context.getVisitedContexts());
+ResultCtx.PreferredType = Context.getPreferredType().getAsString();
   }
 
   CodeCompletionAllocator &getAllocator() override {
@@ -47,7 +55,9 @@
 
   CodeCompletionTUInfo &getCodeCompletionTUInfo() override { return CCTUInfo; }
 
-  std::vector getVisitedNamespace() const {
+private:
+  std::vector getVisitedNamespace(
+  CodeCompletionContext::VisitedContextSet VisitedContexts) const {
 std::vector NSNames;
 for (const auto *Context : VisitedContexts)
   if (const auto *NS = llvm::dyn_cast(Context))
@@ -55,27 +65,25 @@
 return NSNames;
   }
 
-private:
-  VisitedContextResults& VCResults;
+  CompletionContext &ResultCtx;
   CodeCompletionTUInfo CCTUInfo;
-  CodeCompletionContext::VisitedContextSet VisitedContexts;
 };
 
 class CodeCompleteAction : public SyntaxOnlyAction {
 public:
-  CodeCompleteAction(ParsedSourceLocation P, VisitedContextResults &Results)
-  : CompletePosition(std::move(P)), VCResults(Results) {}
+  CodeCompleteAction(ParsedSourceLocation P, CompletionContext &ResultCtx)
+  : CompletePosition(std::move(P)), ResultCtx(ResultCtx) {}
 
   bool BeginInvocation(CompilerInstance &CI) override {
 CI.getFrontendOpts().CodeCompletionAt = CompletePosition;
-CI.setCodeCompletionConsumer(new VisitedContextFinder(VCResults));
+CI.setCodeCompletionConsumer(new VisitedContextFinder(ResultCtx));
 return true;
   }
 
 private:
   // 1-based code complete position ;
   ParsedSourceLocation CompletePosition;
-  VisitedContextResults& VCResults;
+  CompletionContext &ResultCtx;
 };
 
 ParsedSourceLocation offsetToPosition(llvm::StringRef Code, size_t Offset) {
@@ -88,21 +96,49 @@
   static_cast(Offset - StartOfLine + 1)};
 }
 
-VisitedContextResults runCodeCompleteOnCode(StringRef Code) {
-  VisitedContextResults Results;
-  auto TokenOffset = Code.find('^');
-  assert(TokenOffset != StringRef::npos &&
- "Completion token ^ wasn't found in Code.");
-  std::string WithoutToken = Code.take_front(TokenOffset);
-  WithoutToken += Code.drop_front(WithoutToken.size() + 1);
-  assert(StringRef(WithoutToken).find('^') == StringRef::npos &&
- "expected exactly one completion token ^ inside the code");
-
+CompletionContext runCompletion(StringRef Code, size_t Offset) {
+  CompletionContext ResultCtx;
   auto Action = llvm::make_unique(
-  offsetToPosition(WithoutToken, TokenOffset), Results);
+  offsetToPosition(Code, Offset), ResultCtx);
   clang::tooling::runToolOnCodeWithArgs(Action.release(), Code, {"-std=c++11"},
 TestCCName);
-  return Results;
+  return ResultCtx;
+}
+
+struct ParsedAnnotations {
+  std::vector Points;
+  std::string Code;
+};
+
+ParsedAnnotations parseAnnotations(StringRef AnnotatedCode) {
+  ParsedAnnotations R;
+  while (!AnnotatedCode.empty()) {
+size_t NextPoint = AnnotatedCode.find('^');
+if (NextPoint == StringRef::npos) {
+  R.Code += AnnotatedCode;
+  AnnotatedCode = "";
+

[PATCH] D55315: [clangd] Only reduce priority of a thread for indexing.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/index/Background.h:121
   bool ShouldStop = false;
-  std::deque Queue;
+  std::list> Tasks;
   std::vector ThreadPool; // FIXME: Abstract this away.

Why not `std::deque`?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55315



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

[PATCH] D55315: [clangd] Only reduce priority of a thread for indexing.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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



Comment at: clangd/index/Background.h:121
   bool ShouldStop = false;
-  std::deque Queue;
+  std::list> Tasks;
   std::vector ThreadPool; // FIXME: Abstract this away.

ilya-biryukov wrote:
> Why not `std::deque`?
because we can insert in the middle of the queue now, when we skip existing 
high priority tasks. 


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55315



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

[PATCH] D55648: [CodeComplete] Fill preferred type on binary expressions

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Thanks for the review!




Comment at: lib/Sema/SemaCodeComplete.cpp:4928
+if (LHSType->isIntegralOrEnumerationType())
+  return S.getASTContext().IntTy;
+return QualType();

kadircet wrote:
> why not LHSType ?
The shifts are non-symmetrical, so it didn't feel the link between lhs and rhs 
provided much value.
It's not true for other ops, e.g. `|`, where you would typically use the same 
operand type for both values.

It's not a very strong argument, though, happy to change this code later.



Repository:
  rC Clang

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

https://reviews.llvm.org/D55648



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

[PATCH] D55648: [CodeComplete] Fill preferred type on binary expressions

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: lib/Sema/SemaCodeComplete.cpp:4928
+if (LHSType->isIntegralOrEnumerationType())
+  return S.getASTContext().IntTy;
+return QualType();

ilya-biryukov wrote:
> kadircet wrote:
> > why not LHSType ?
> The shifts are non-symmetrical, so it didn't feel the link between lhs and 
> rhs provided much value.
> It's not true for other ops, e.g. `|`, where you would typically use the same 
> operand type for both values.
> 
> It's not a very strong argument, though, happy to change this code later.
> 
yeah you are right, it makes more sense this way.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55648



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

[PATCH] D55648: [CodeComplete] Fill preferred type on binary expressions

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 178071.
ilya-biryukov added a comment.
Herald added a subscriber: arphaman.

- Update the test after changes


Repository:
  rC Clang

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

https://reviews.llvm.org/D55648

Files:
  include/clang/Sema/Sema.h
  lib/Parse/ParseExpr.cpp
  lib/Sema/SemaCodeComplete.cpp
  test/Index/complete-exprs.c
  unittests/Sema/CodeCompleteTest.cpp

Index: unittests/Sema/CodeCompleteTest.cpp
===
--- unittests/Sema/CodeCompleteTest.cpp
+++ unittests/Sema/CodeCompleteTest.cpp
@@ -14,31 +14,39 @@
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaDiagnostic.h"
 #include "clang/Tooling/Tooling.h"
-#include "gtest/gtest.h"
 #include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+#include 
 
 namespace {
 
 using namespace clang;
 using namespace clang::tooling;
+using ::testing::Each;
 using ::testing::UnorderedElementsAre;
 
 const char TestCCName[] = "test.cc";
-using VisitedContextResults = std::vector;
 
-class VisitedContextFinder: public CodeCompleteConsumer {
+struct CompletionContext {
+  std::vector VisitedNamespaces;
+  std::string PreferredType;
+};
+
+class VisitedContextFinder : public CodeCompleteConsumer {
 public:
-  VisitedContextFinder(VisitedContextResults &Results)
+  VisitedContextFinder(CompletionContext &ResultCtx)
   : CodeCompleteConsumer(/*CodeCompleteOpts=*/{},
  /*CodeCompleteConsumer*/ false),
-VCResults(Results),
+ResultCtx(ResultCtx),
 CCTUInfo(std::make_shared()) {}
 
   void ProcessCodeCompleteResults(Sema &S, CodeCompletionContext Context,
   CodeCompletionResult *Results,
   unsigned NumResults) override {
-VisitedContexts = Context.getVisitedContexts();
-VCResults = getVisitedNamespace();
+ResultCtx.VisitedNamespaces =
+getVisitedNamespace(Context.getVisitedContexts());
+ResultCtx.PreferredType = Context.getPreferredType().getAsString();
   }
 
   CodeCompletionAllocator &getAllocator() override {
@@ -47,7 +55,9 @@
 
   CodeCompletionTUInfo &getCodeCompletionTUInfo() override { return CCTUInfo; }
 
-  std::vector getVisitedNamespace() const {
+private:
+  std::vector getVisitedNamespace(
+  CodeCompletionContext::VisitedContextSet VisitedContexts) const {
 std::vector NSNames;
 for (const auto *Context : VisitedContexts)
   if (const auto *NS = llvm::dyn_cast(Context))
@@ -55,27 +65,25 @@
 return NSNames;
   }
 
-private:
-  VisitedContextResults& VCResults;
+  CompletionContext &ResultCtx;
   CodeCompletionTUInfo CCTUInfo;
-  CodeCompletionContext::VisitedContextSet VisitedContexts;
 };
 
 class CodeCompleteAction : public SyntaxOnlyAction {
 public:
-  CodeCompleteAction(ParsedSourceLocation P, VisitedContextResults &Results)
-  : CompletePosition(std::move(P)), VCResults(Results) {}
+  CodeCompleteAction(ParsedSourceLocation P, CompletionContext &ResultCtx)
+  : CompletePosition(std::move(P)), ResultCtx(ResultCtx) {}
 
   bool BeginInvocation(CompilerInstance &CI) override {
 CI.getFrontendOpts().CodeCompletionAt = CompletePosition;
-CI.setCodeCompletionConsumer(new VisitedContextFinder(VCResults));
+CI.setCodeCompletionConsumer(new VisitedContextFinder(ResultCtx));
 return true;
   }
 
 private:
   // 1-based code complete position ;
   ParsedSourceLocation CompletePosition;
-  VisitedContextResults& VCResults;
+  CompletionContext &ResultCtx;
 };
 
 ParsedSourceLocation offsetToPosition(llvm::StringRef Code, size_t Offset) {
@@ -88,21 +96,49 @@
   static_cast(Offset - StartOfLine + 1)};
 }
 
-VisitedContextResults runCodeCompleteOnCode(StringRef Code) {
-  VisitedContextResults Results;
-  auto TokenOffset = Code.find('^');
-  assert(TokenOffset != StringRef::npos &&
- "Completion token ^ wasn't found in Code.");
-  std::string WithoutToken = Code.take_front(TokenOffset);
-  WithoutToken += Code.drop_front(WithoutToken.size() + 1);
-  assert(StringRef(WithoutToken).find('^') == StringRef::npos &&
- "expected exactly one completion token ^ inside the code");
-
+CompletionContext runCompletion(StringRef Code, size_t Offset) {
+  CompletionContext ResultCtx;
   auto Action = llvm::make_unique(
-  offsetToPosition(WithoutToken, TokenOffset), Results);
+  offsetToPosition(Code, Offset), ResultCtx);
   clang::tooling::runToolOnCodeWithArgs(Action.release(), Code, {"-std=c++11"},
 TestCCName);
-  return Results;
+  return ResultCtx;
+}
+
+struct ParsedAnnotations {
+  std::vector Points;
+  std::string Code;
+};
+
+ParsedAnnotations parseAnnotations(StringRef AnnotatedCode) {
+  ParsedAnnotations R;
+  while (!AnnotatedCode.empty()) {
+size_t NextPoint = AnnotatedCode.find('^');
+if (NextPoint == StringRef::npos) {
+  R.Code += AnnotatedCode;
+  

r349054 - Make -Wstring-plus-int warns even if when the result is not out of bounds

[Sylvestre Ledru via cfe-commits]

Author: sylvestre
Date: Thu Dec 13 08:06:23 2018
New Revision: 349054

URL: http://llvm.org/viewvc/llvm-project?rev=349054&view=rev
Log:
Make -Wstring-plus-int warns even if when the result is not out of bounds

Summary: Patch by Arnaud Bienner

Reviewers: sylvestre.ledru, thakis

Reviewed By: thakis

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D55382

Modified:
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/SemaCXX/string-plus-int.cpp

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=349054&r1=349053&r2=349054&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Dec 13 08:06:23 2018
@@ -9135,16 +9135,6 @@ static void diagnoseStringPlusInt(Sema &
   if (!IsStringPlusInt || IndexExpr->isValueDependent())
 return;
 
-  Expr::EvalResult Result;
-  if (IndexExpr->EvaluateAsInt(Result, Self.getASTContext())) {
-llvm::APSInt index = Result.Val.getInt();
-unsigned StrLenWithNull = StrExpr->getLength() + 1;
-if (index.isNonNegative() &&
-index <= llvm::APSInt(llvm::APInt(index.getBitWidth(), StrLenWithNull),
-  index.isUnsigned()))
-  return;
-  }
-
   SourceRange DiagRange(LHSExpr->getBeginLoc(), RHSExpr->getEndLoc());
   Self.Diag(OpLoc, diag::warn_string_plus_int)
   << DiagRange << IndexExpr->IgnoreImpCasts()->getType();

Modified: cfe/trunk/test/SemaCXX/string-plus-int.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/string-plus-int.cpp?rev=349054&r1=349053&r2=349054&view=diff
==
--- cfe/trunk/test/SemaCXX/string-plus-int.cpp (original)
+++ cfe/trunk/test/SemaCXX/string-plus-int.cpp Thu Dec 13 08:06:23 2018
@@ -31,37 +31,36 @@ void f(int index) {
   consume("foo" + 5);  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume("foo" + index);  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume("foo" + kMyEnum);  // expected-warning {{adding 'MyEnum' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
+  consume("foo" + kMySmallEnum); // expected-warning {{adding 'MyEnum' to a 
string does not append to the string}} expected-note {{use array indexing to 
silence this warning}}
 
   consume(5 + "foo");  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume(index + "foo");  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume(kMyEnum + "foo");  // expected-warning {{adding 'MyEnum' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
+  consume(kMySmallEnum + "foo"); // expected-warning {{adding 'MyEnum' to a 
string does not append to the string}} expected-note {{use array indexing to 
silence this warning}}
 
   // FIXME: suggest replacing with "foo"[5]
   consumeChar(*("foo" + 5));  // expected-warning {{adding 'int' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
   consumeChar(*(5 + "foo"));  // expected-warning {{adding 'int' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
 
   consume(L"foo" + 5);  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
+  consume(L"foo" + 2); // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
+
+  consume("foo" + 3);  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
+  consume("foo" + 4);  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
+  consume("\pfoo" + 4);  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
+
+  #define A "foo"
+  #define B "bar"
+  consume(A B + sizeof(A) - 1); // expected-warning {{to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
 
   // Should not warn.
   consume(&("foo"[3]));
   consume(&("foo"[index]));
   consume(&("foo"[kMyEnum]));
-  consume("foo" + kMySmallEnum);
-  consume(kMySmallEnum + "foo");
 
-  consume(L"foo" + 2);
-
-  consume("foo" + 3)

r349053 - [CodeComplete] Fill preferred type on binary expressions

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Thu Dec 13 08:06:11 2018
New Revision: 349053

URL: http://llvm.org/viewvc/llvm-project?rev=349053&view=rev
Log:
[CodeComplete] Fill preferred type on binary expressions

Reviewers: kadircet

Reviewed By: kadircet

Subscribers: arphaman, cfe-commits

Differential Revision: https://reviews.llvm.org/D55648

Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Parse/ParseExpr.cpp
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/test/Index/complete-exprs.c
cfe/trunk/unittests/Sema/CodeCompleteTest.cpp

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=349053&r1=349052&r2=349053&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Thu Dec 13 08:06:11 2018
@@ -10350,7 +10350,7 @@ public:
   void CodeCompleteInitializer(Scope *S, Decl *D);
   void CodeCompleteReturn(Scope *S);
   void CodeCompleteAfterIf(Scope *S);
-  void CodeCompleteAssignmentRHS(Scope *S, Expr *LHS);
+  void CodeCompleteBinaryRHS(Scope *S, Expr *LHS, tok::TokenKind Op);
 
   void CodeCompleteQualifiedId(Scope *S, CXXScopeSpec &SS,
bool EnteringContext, QualType BaseType);

Modified: cfe/trunk/lib/Parse/ParseExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExpr.cpp?rev=349053&r1=349052&r2=349053&view=diff
==
--- cfe/trunk/lib/Parse/ParseExpr.cpp (original)
+++ cfe/trunk/lib/Parse/ParseExpr.cpp Thu Dec 13 08:06:11 2018
@@ -393,10 +393,11 @@ Parser::ParseRHSOfBinaryExpression(ExprR
   }
 }
 
-// Code completion for the right-hand side of an assignment expression
-// goes through a special hook that takes the left-hand side into account.
-if (Tok.is(tok::code_completion) && NextTokPrec == prec::Assignment) {
-  Actions.CodeCompleteAssignmentRHS(getCurScope(), LHS.get());
+// Code completion for the right-hand side of a binary expression goes
+// through a special hook that takes the left-hand side into account.
+if (Tok.is(tok::code_completion)) {
+  Actions.CodeCompleteBinaryRHS(getCurScope(), LHS.get(),
+OpToken.getKind());
   cutOffParsing();
   return ExprError();
 }

Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=349053&r1=349052&r2=349053&view=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Thu Dec 13 08:06:11 2018
@@ -4878,9 +4878,86 @@ void Sema::CodeCompleteAfterIf(Scope *S)
 Results.data(), Results.size());
 }
 
-void Sema::CodeCompleteAssignmentRHS(Scope *S, Expr *LHS) {
-  if (LHS)
-CodeCompleteExpression(S, static_cast(LHS)->getType());
+static QualType getPreferredTypeOfBinaryRHS(Sema &S, Expr *LHS,
+tok::TokenKind Op) {
+  if (!LHS)
+return QualType();
+
+  QualType LHSType = LHS->getType();
+  if (LHSType->isPointerType()) {
+if (Op == tok::plus || Op == tok::plusequal || Op == tok::minusequal)
+  return S.getASTContext().getPointerDiffType();
+// Pointer difference is more common than subtracting an int from a 
pointer.
+if (Op == tok::minus)
+  return LHSType;
+  }
+
+  switch (Op) {
+  // No way to infer the type of RHS from LHS.
+  case tok::comma:
+return QualType();
+  // Prefer the type of the left operand for all of these.
+  // Arithmetic operations.
+  case tok::plus:
+  case tok::plusequal:
+  case tok::minus:
+  case tok::minusequal:
+  case tok::percent:
+  case tok::percentequal:
+  case tok::slash:
+  case tok::slashequal:
+  case tok::star:
+  case tok::starequal:
+  // Assignment.
+  case tok::equal:
+  // Comparison operators.
+  case tok::equalequal:
+  case tok::exclaimequal:
+  case tok::less:
+  case tok::lessequal:
+  case tok::greater:
+  case tok::greaterequal:
+  case tok::spaceship:
+return LHS->getType();
+  // Binary shifts are often overloaded, so don't try to guess those.
+  case tok::greatergreater:
+  case tok::greatergreaterequal:
+  case tok::lessless:
+  case tok::lesslessequal:
+if (LHSType->isIntegralOrEnumerationType())
+  return S.getASTContext().IntTy;
+return QualType();
+  // Logical operators, assume we want bool.
+  case tok::ampamp:
+  case tok::pipepipe:
+  case tok::caretcaret:
+return S.getASTContext().BoolTy;
+  // Operators often used for bit manipulation are typically used with the type
+  // of the left argument.
+  case tok::pipe:
+  case tok::pipeequal:
+  case tok::caret:
+  case tok::caretequal:
+  case tok::amp:
+  case tok::ampequal:
+if (LHSType->isIntegral

[PATCH] D55648: [CodeComplete] Fill preferred type on binary expressions

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC349053: [CodeComplete] Fill preferred type on binary 
expressions (authored by ibiryukov, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D55648?vs=178071&id=178072#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D55648

Files:
  include/clang/Sema/Sema.h
  lib/Parse/ParseExpr.cpp
  lib/Sema/SemaCodeComplete.cpp
  test/Index/complete-exprs.c
  unittests/Sema/CodeCompleteTest.cpp

Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -10350,7 +10350,7 @@
   void CodeCompleteInitializer(Scope *S, Decl *D);
   void CodeCompleteReturn(Scope *S);
   void CodeCompleteAfterIf(Scope *S);
-  void CodeCompleteAssignmentRHS(Scope *S, Expr *LHS);
+  void CodeCompleteBinaryRHS(Scope *S, Expr *LHS, tok::TokenKind Op);
 
   void CodeCompleteQualifiedId(Scope *S, CXXScopeSpec &SS,
bool EnteringContext, QualType BaseType);
Index: test/Index/complete-exprs.c
===
--- test/Index/complete-exprs.c
+++ test/Index/complete-exprs.c
@@ -33,16 +33,11 @@
 // CHECK-CC1: ParmDecl:{ResultType int}{TypedText j} (8)
 // CHECK-CC1: NotImplemented:{ResultType size_t}{TypedText sizeof}{LeftParen (}{Placeholder expression-or-type}{RightParen )} (40)
 // RUN: env CINDEXTEST_EDITING=1 CINDEXTEST_COMPLETION_CACHING=1 c-index-test -code-completion-at=%s:7:10 -Xclang -code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC1 %s
-// RUN: c-index-test -code-completion-at=%s:7:14 -Xclang -code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC3 %s
-// RUN: env CINDEXTEST_EDITING=1 CINDEXTEST_COMPLETION_CACHING=1 c-index-test -code-completion-at=%s:7:14 -Xclang -code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC3 %s
-// CHECK-CC3: macro definition:{TypedText __VERSION__} (70)
-// CHECK-CC3: FunctionDecl:{ResultType int}{TypedText f}{LeftParen (}{Placeholder int}{RightParen )} (50)
-// CHECK-CC3-NOT: NotImplemented:{TypedText float}
-// CHECK-CC3: ParmDecl:{ResultType int}{TypedText j} (34)
-// CHECK-CC3: NotImplemented:{ResultType size_t}{TypedText sizeof}{LeftParen (}{Placeholder expressio
+// RUN: c-index-test -code-completion-at=%s:7:14 -Xclang -code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC1 %s
+// RUN: env CINDEXTEST_EDITING=1 CINDEXTEST_COMPLETION_CACHING=1 c-index-test -code-completion-at=%s:7:14 -Xclang -code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC1 %s
 
-// RUN: c-index-test -code-completion-at=%s:7:18 -Xclang -code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC3 %s
-// RUN: c-index-test -code-completion-at=%s:7:22 -Xclang -code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC3 %s
+// RUN: c-index-test -code-completion-at=%s:7:18 -Xclang -code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC1 %s
+// RUN: c-index-test -code-completion-at=%s:7:22 -Xclang -code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC1 %s
 // RUN: c-index-test -code-completion-at=%s:7:2 -Xclang -code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC2 %s
 // CHECK-CC2: macro definition:{TypedText __VERSION__} (70)
 // CHECK-CC2: FunctionDecl:{ResultType int}{TypedText f}{LeftParen (}{Placeholder int}{RightParen )} (50)
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -4878,9 +4878,86 @@
 Results.data(), Results.size());
 }
 
-void Sema::CodeCompleteAssignmentRHS(Scope *S, Expr *LHS) {
-  if (LHS)
-CodeCompleteExpression(S, static_cast(LHS)->getType());
+static QualType getPreferredTypeOfBinaryRHS(Sema &S, Expr *LHS,
+tok::TokenKind Op) {
+  if (!LHS)
+return QualType();
+
+  QualType LHSType = LHS->getType();
+  if (LHSType->isPointerType()) {
+if (Op == tok::plus || Op == tok::plusequal || Op == tok::minusequal)
+  return S.getASTContext().getPointerDiffType();
+// Pointer difference is more common than subtracting an int from a pointer.
+if (Op == tok::minus)
+  return LHSType;
+  }
+
+  switch (Op) {
+  // No way to infer the type of RHS from LHS.
+  case tok::comma:
+return QualType();
+  // Prefer the type of the left operand for all of these.
+  // Arithmetic operations.
+  case tok::plus:
+  case tok::plusequal:
+  case tok::minus:
+  case tok::minusequal:
+  case tok::percent:
+  case tok::percentequal:
+  case tok::slash:
+  case tok::slashequal:
+  case tok::star:
+  case tok::starequal:
+  // Assignment.
+  case tok::equal:
+  // Comparison operators.
+  case tok::equalequal:
+  case tok::exclaimequal:
+  case tok::less:
+  case tok::lessequal:
+ 

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL349054: Make -Wstring-plus-int warns even if when the result 
is not out of bounds (authored by sylvestre, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55382

Files:
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/test/SemaCXX/string-plus-int.cpp


Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -9135,16 +9135,6 @@
   if (!IsStringPlusInt || IndexExpr->isValueDependent())
 return;
 
-  Expr::EvalResult Result;
-  if (IndexExpr->EvaluateAsInt(Result, Self.getASTContext())) {
-llvm::APSInt index = Result.Val.getInt();
-unsigned StrLenWithNull = StrExpr->getLength() + 1;
-if (index.isNonNegative() &&
-index <= llvm::APSInt(llvm::APInt(index.getBitWidth(), StrLenWithNull),
-  index.isUnsigned()))
-  return;
-  }
-
   SourceRange DiagRange(LHSExpr->getBeginLoc(), RHSExpr->getEndLoc());
   Self.Diag(OpLoc, diag::warn_string_plus_int)
   << DiagRange << IndexExpr->IgnoreImpCasts()->getType();
Index: cfe/trunk/test/SemaCXX/string-plus-int.cpp
===
--- cfe/trunk/test/SemaCXX/string-plus-int.cpp
+++ cfe/trunk/test/SemaCXX/string-plus-int.cpp
@@ -31,37 +31,36 @@
   consume("foo" + 5);  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume("foo" + index);  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume("foo" + kMyEnum);  // expected-warning {{adding 'MyEnum' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
+  consume("foo" + kMySmallEnum); // expected-warning {{adding 'MyEnum' to a 
string does not append to the string}} expected-note {{use array indexing to 
silence this warning}}
 
   consume(5 + "foo");  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume(index + "foo");  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume(kMyEnum + "foo");  // expected-warning {{adding 'MyEnum' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
+  consume(kMySmallEnum + "foo"); // expected-warning {{adding 'MyEnum' to a 
string does not append to the string}} expected-note {{use array indexing to 
silence this warning}}
 
   // FIXME: suggest replacing with "foo"[5]
   consumeChar(*("foo" + 5));  // expected-warning {{adding 'int' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
   consumeChar(*(5 + "foo"));  // expected-warning {{adding 'int' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
 
   consume(L"foo" + 5);  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
+  consume(L"foo" + 2); // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
+
+  consume("foo" + 3);  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
+  consume("foo" + 4);  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
+  consume("\pfoo" + 4);  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
+
+  #define A "foo"
+  #define B "bar"
+  consume(A B + sizeof(A) - 1); // expected-warning {{to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
 
   // Should not warn.
   consume(&("foo"[3]));
   consume(&("foo"[index]));
   consume(&("foo"[kMyEnum]));
-  consume("foo" + kMySmallEnum);
-  consume(kMySmallEnum + "foo");
 
-  consume(L"foo" + 2);
-
-  consume("foo" + 3);  // Points at the \0
-  consume("foo" + 4);  // Points 1 past the \0, which is legal too.
-  consume("\pfoo" + 4);  // Pascal strings don't have a trailing \0, but they
- // have a leading length byte, so this is fine too.
 
   consume("foo" + kMyOperatorOverloadedEnum);
   consume(kMyOperatorOverloadedEnum + "foo");
-
-  #define A "foo"
-  #define B "bar"
-  consume(A B + sizeof(A) - 

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner added a comment.

Thanks for the archeological work and finding the conversation related to the 
initial patch :)
Interesting that the last case you mentioned is exactly the bug I had in my 
project (though in your case this would have been intended).


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55382



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

[PATCH] D55455: Remove stat cache chaining as it's no longer needed after PTH support has been removed

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

It has been about a week since your cfe-dev post with no concerns.  I think you 
can commit.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55455



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

[PATCH] D55657: [libcxx] [regex] Use distinct __regex_word on NetBSD

[Michał Górny via Phabricator via cfe-commits]

mgorny created this revision.
mgorny added reviewers: krytarowski, howard.hinnant, dsanders, EricWF.
Herald added subscribers: libcxx-commits, ldionne.

NetBSD defines character classes up to 0x2000.  Use 0x8000 as a safe
__regex_word that hopefully will not collide with other values
in the foreseeable future.


Repository:
  rCXX libc++

https://reviews.llvm.org/D55657

Files:
  include/regex


Index: include/regex
===
--- include/regex
+++ include/regex
@@ -990,6 +990,9 @@
 
 #if defined(__mips__) && defined(__GLIBC__)
 static const char_class_type __regex_word = 
static_cast(_ISbit(15));
+#elif defined(__NetBSD__)
+// NetBSD defines classes up to 0x2000
+static const char_class_type __regex_word = 0x8000;
 #else
 static const char_class_type __regex_word = 0x80;
 #endif


Index: include/regex
===
--- include/regex
+++ include/regex
@@ -990,6 +990,9 @@
 
 #if defined(__mips__) && defined(__GLIBC__)
 static const char_class_type __regex_word = static_cast(_ISbit(15));
+#elif defined(__NetBSD__)
+// NetBSD defines classes up to 0x2000
+static const char_class_type __regex_word = 0x8000;
 #else
 static const char_class_type __regex_word = 0x80;
 #endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54489: Implement -frecord-command-line (-frecord-gcc-switches)

[Scott Linder via Phabricator via cfe-commits]

scott.linder updated this revision to Diff 178082.
scott.linder added a comment.

Update documentation to match section naming change, and to explicitly note 
that the section format differs from the equivalent GCC format.


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

https://reviews.llvm.org/D54489

Files:
  docs/ClangCommandLineReference.rst
  include/clang/Basic/CodeGenOptions.h
  include/clang/Driver/CC1Options.td
  include/clang/Driver/Options.td
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/Driver/clang_f_opts.c
  test/Driver/debug-options.c

Index: test/Driver/debug-options.c
===
--- test/Driver/debug-options.c
+++ test/Driver/debug-options.c
@@ -157,6 +157,17 @@
 // RUN: %clang -### -c -O3 -ffunction-sections -grecord-gcc-switches %s 2>&1 \
 // | FileCheck -check-prefix=GRECORD_OPT %s
 //
+// RUN: %clang -### -c -grecord-command-line %s 2>&1 \
+// | FileCheck -check-prefix=GRECORD %s
+// RUN: %clang -### -c -gno-record-command-line %s 2>&1 \
+// | FileCheck -check-prefix=GNO_RECORD %s
+// RUN: %clang -### -c -grecord-command-line -gno-record-command-line %s 2>&1 \
+// | FileCheck -check-prefix=GNO_RECORD %s/
+// RUN: %clang -### -c -grecord-command-line -o - %s 2>&1 \
+// | FileCheck -check-prefix=GRECORD_O %s
+// RUN: %clang -### -c -O3 -ffunction-sections -grecord-command-line %s 2>&1 \
+// | FileCheck -check-prefix=GRECORD_OPT %s
+//
 // RUN: %clang -### -c -gstrict-dwarf -gno-strict-dwarf %s 2>&1 \
 // RUN:| FileCheck -check-prefix=GIGNORE %s
 //
Index: test/Driver/clang_f_opts.c
===
--- test/Driver/clang_f_opts.c
+++ test/Driver/clang_f_opts.c
@@ -542,3 +542,18 @@
 // RUN: %clang -### -S -fomit-frame-pointer -fno-omit-frame-pointer -pg %s 2>&1 | FileCheck -check-prefix=CHECK-MIX-NO-OMIT-FP-PG %s
 // CHECK-NO-MIX-OMIT-FP-PG: '-fomit-frame-pointer' not allowed with '-pg'
 // CHECK-MIX-NO-OMIT-FP-PG-NOT: '-fomit-frame-pointer' not allowed with '-pg'
+
+// RUN: %clang -### -S -target x86_64-unknown-linux -frecord-gcc-switches %s 2>&1 | FileCheck -check-prefix=CHECK-RECORD-GCC-SWITCHES %s
+// RUN: %clang -### -S -target x86_64-unknown-linux -fno-record-gcc-switches %s 2>&1 | FileCheck -check-prefix=CHECK-NO-RECORD-GCC-SWITCHES %s
+// RUN: %clang -### -S -target x86_64-unknown-linux -fno-record-gcc-switches -frecord-gcc-switches %s 2>&1 | FileCheck -check-prefix=CHECK-RECORD-GCC-SWITCHES %s
+// RUN: %clang -### -S -target x86_64-unknown-linux -frecord-gcc-switches -fno-record-gcc-switches %s 2>&1 | FileCheck -check-prefix=CHECK-NO-RECORD-GCC-SWITCHES %s
+// RUN: %clang -### -S -target x86_64-unknown-linux -frecord-command-line %s 2>&1 | FileCheck -check-prefix=CHECK-RECORD-GCC-SWITCHES %s
+// RUN: %clang -### -S -target x86_64-unknown-linux -fno-record-command-line %s 2>&1 | FileCheck -check-prefix=CHECK-NO-RECORD-GCC-SWITCHES %s
+// RUN: %clang -### -S -target x86_64-unknown-linux -fno-record-command-line -frecord-command-line %s 2>&1 | FileCheck -check-prefix=CHECK-RECORD-GCC-SWITCHES %s
+// RUN: %clang -### -S -target x86_64-unknown-linux -frecord-command-line -fno-record-command-line %s 2>&1 | FileCheck -check-prefix=CHECK-NO-RECORD-GCC-SWITCHES %s
+// Test with a couple examples of non-ELF object file formats
+// RUN: %clang -### -S -target x86_64-unknown-macosx -frecord-command-line %s 2>&1 | FileCheck -check-prefix=CHECK-RECORD-GCC-SWITCHES-ERROR %s
+// RUN: %clang -### -S -target x86_64-unknown-windows -frecord-command-line %s 2>&1 | FileCheck -check-prefix=CHECK-RECORD-GCC-SWITCHES-ERROR %s
+// CHECK-RECORD-GCC-SWITCHES: "-record-command-line"
+// CHECK-NO-RECORD-GCC-SWITCHES-NOT: "-record-command-line"
+// CHECK-RECORD-GCC-SWITCHES-ERROR: error: unsupported option '-frecord-command-line' for target
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -755,6 +755,7 @@
   Args.hasFlag(OPT_ffine_grained_bitfield_accesses,
OPT_fno_fine_grained_bitfield_accesses, false);
   Opts.DwarfDebugFlags = Args.getLastArgValue(OPT_dwarf_debug_flags);
+  Opts.RecordCommandLine = Args.getLastArgValue(OPT_record_command_line);
   Opts.MergeAllConstants = Args.hasArg(OPT_fmerge_all_constants);
   Opts.NoCommon = Args.hasArg(OPT_fno_common);
   Opts.NoImplicitFloat = Args.hasArg(OPT_no_implicit_float);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -5063,14 +5063,22 @@
 
   const char *Exec = D.getClangProgramPath();
 
-  // Optionally embed the -cc1 level arguments into the debug info, for build

r349059 - Fix CodeCompleteTest.cpp for older gcc plus ccache builds

[David Green via cfe-commits]

Author: dmgreen
Date: Thu Dec 13 09:20:06 2018
New Revision: 349059

URL: http://llvm.org/viewvc/llvm-project?rev=349059&view=rev
Log:
Fix CodeCompleteTest.cpp for older gcc plus ccache builds

Some versions of gcc, especially when invoked through ccache (-E), can have
trouble with raw string literals inside macros. This moves the string out of
the macro.

Modified:
cfe/trunk/unittests/Sema/CodeCompleteTest.cpp

Modified: cfe/trunk/unittests/Sema/CodeCompleteTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Sema/CodeCompleteTest.cpp?rev=349059&r1=349058&r2=349059&view=diff
==
--- cfe/trunk/unittests/Sema/CodeCompleteTest.cpp (original)
+++ cfe/trunk/unittests/Sema/CodeCompleteTest.cpp Thu Dec 13 09:20:06 2018
@@ -183,113 +183,113 @@ TEST(SemaCodeCompleteTest, VisitedNSWith
 
 TEST(PreferredTypeTest, BinaryExpr) {
   // Check various operations for arithmetic types.
-  EXPECT_THAT(collectPreferredTypes(R"cpp(
+  StringRef code1 = R"cpp(
 void test(int x) {
   x = ^10;
   x += ^10; x -= ^10; x *= ^10; x /= ^10; x %= ^10;
   x + ^10; x - ^10; x * ^10; x / ^10; x % ^10;
-})cpp"),
-  Each("int"));
-  EXPECT_THAT(collectPreferredTypes(R"cpp(
+})cpp";
+  EXPECT_THAT(collectPreferredTypes(code1), Each("int"));
+  StringRef code2 = R"cpp(
 void test(float x) {
   x = ^10;
   x += ^10; x -= ^10; x *= ^10; x /= ^10; x %= ^10;
   x + ^10; x - ^10; x * ^10; x / ^10; x % ^10;
-})cpp"),
-  Each("float"));
+})cpp";
+  EXPECT_THAT(collectPreferredTypes(code2), Each("float"));
 
   // Pointer types.
-  EXPECT_THAT(collectPreferredTypes(R"cpp(
+  StringRef code3 = R"cpp(
 void test(int *ptr) {
   ptr - ^ptr;
   ptr = ^ptr;
-})cpp"),
-  Each("int *"));
+})cpp";
+  EXPECT_THAT(collectPreferredTypes(code3), Each("int *"));
 
-  EXPECT_THAT(collectPreferredTypes(R"cpp(
+  StringRef code4 = R"cpp(
 void test(int *ptr) {
   ptr + ^10;
   ptr += ^10;
   ptr -= ^10;
-})cpp"),
-  Each("long")); // long is normalized 'ptrdiff_t'.
+})cpp";
+  EXPECT_THAT(collectPreferredTypes(code4), Each("long")); // long is 
normalized 'ptrdiff_t'.
 
   // Comparison operators.
-  EXPECT_THAT(collectPreferredTypes(R"cpp(
+  StringRef code5 = R"cpp(
 void test(int i) {
   i <= ^1; i < ^1; i >= ^1; i > ^1; i == ^1; i != ^1;
 }
-  )cpp"),
-  Each("int"));
+  )cpp";
+  EXPECT_THAT(collectPreferredTypes(code5), Each("int"));
 
-  EXPECT_THAT(collectPreferredTypes(R"cpp(
+  StringRef code6 = R"cpp(
 void test(int *ptr) {
   ptr <= ^ptr; ptr < ^ptr; ptr >= ^ptr; ptr > ^ptr;
   ptr == ^ptr; ptr != ^ptr;
 }
-  )cpp"),
-  Each("int *"));
+  )cpp";
+  EXPECT_THAT(collectPreferredTypes(code6), Each("int *"));
 
   // Relational operations.
-  EXPECT_THAT(collectPreferredTypes(R"cpp(
+  StringRef code7 = R"cpp(
 void test(int i, int *ptr) {
   i && ^1; i || ^1;
   ptr && ^1; ptr || ^1;
 }
-  )cpp"),
-  Each("_Bool"));
+  )cpp";
+  EXPECT_THAT(collectPreferredTypes(code7), Each("_Bool"));
 
   // Bitwise operations.
-  EXPECT_THAT(collectPreferredTypes(R"cpp(
+  StringRef code8 = R"cpp(
 void test(long long ll) {
   ll | ^1; ll & ^1;
 }
-  )cpp"),
-  Each("long long"));
+  )cpp";
+  EXPECT_THAT(collectPreferredTypes(code8), Each("long long"));
 
-  EXPECT_THAT(collectPreferredTypes(R"cpp(
+  StringRef code9 = R"cpp(
 enum A {};
 void test(A a) {
   a | ^1; a & ^1;
 }
-  )cpp"),
-  Each("enum A"));
+  )cpp";
+  EXPECT_THAT(collectPreferredTypes(code9), Each("enum A"));
 
-  EXPECT_THAT(collectPreferredTypes(R"cpp(
+  StringRef code10 = R"cpp(
 enum class A {};
 void test(A a) {
   // This is technically illegal with the 'enum class' without overloaded
   // operators, but we pretend it's fine.
   a | ^a; a & ^a;
 }
-  )cpp"),
-  Each("enum A"));
+  )cpp";
+  EXPECT_THAT(collectPreferredTypes(code10), Each("enum A"));
 
   // Binary shifts.
-  EXPECT_THAT(collectPreferredTypes(R"cpp(
+  StringRef code11 = R"cpp(
 void test(int i, long long ll) {
   i << ^1; ll << ^1;
   i <<= ^1; i <<= ^1;
   i >> ^1; ll >> ^1;
   i >>= ^1; i >>= ^1;
 }
-  )cpp"),
-  Each("int"));
+  )cpp";
+  EXPECT_THAT(collectPreferredTypes(code11), Each("int"));
 
   // Comma does not provide any useful information.
-  EXPECT_THAT(collectPreferredTypes(R"cpp(
+  StringRef code12 = R"cpp(
 class Cls {};
 void test(int i, int* ptr, Cls x) {
   (i, ^i);
   (ptr, ^ptr);
   (x, ^x);
 }
-  )cpp"),
-  Each("NULL TYPE"));
+  )cpp";
+  EXPECT_THAT(collectPreferredTypes(code12), Each("NULL TYPE"));
 
   // User-defined types do not take operator overloading into account.
   // However, they provide heuristics for some common cases.
-

Re: r349053 - [CodeComplete] Fill preferred type on binary expressions

[David Green via cfe-commits]

Hello!

Certain version of gcc (along with ccache iirc, where they use -E) don't like 
these raw strings literals inside macros.

This happens: https://godbolt.org/g/fsXjB7


I've tried to fix it up in  
https://reviews.llvm.org/rL349059. My choice of variable names may not have 
been very inspired though. Let me know if anything looks off.


Cheers

Dave

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

r349061 - [CodeComplete] Temporarily disable failing assertion

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Thu Dec 13 09:23:48 2018
New Revision: 349061

URL: http://llvm.org/viewvc/llvm-project?rev=349061&view=rev
Log:
[CodeComplete] Temporarily disable failing assertion

Found the case in the clang codebase where the assertion fires.
To avoid crashing assertion-enabled builds before I re-add the missing
operation.
Will restore the assertion alongside the upcoming fix.

Modified:
cfe/trunk/lib/Sema/SemaCodeComplete.cpp

Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=349061&r1=349060&r2=349061&view=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Thu Dec 13 09:23:48 2018
@@ -4949,7 +4949,8 @@ static QualType getPreferredTypeOfBinary
   case tok::arrowstar:
 return QualType();
   default:
-assert(false && "unhandled binary op");
+// FIXME(ibiryukov): handle the missing op, re-add the assertion.
+// assert(false && "unhandled binary op");
 return QualType();
   }
 }


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

[PATCH] D55658: Avoid Decl::getASTContext in hot paths where possible, Part 1

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno created this revision.
riccibruno added a reviewer: aaron.ballman.
riccibruno added a project: clang.
Herald added subscribers: cfe-commits, arphaman.
Herald added a reviewer: shafik.

`Decl::getASTContext` and `DeclContext::getParentASTContext` are not that cheap
since they must walk back to the TUDecl, potentially after many cache misses 
along the way.

Instrumentation shows that most of the iterations in `getTranslationUnitDecl` 
could be
eliminated by passing a ref to the `ASTContext` as a function parameter.
The goal here is not to remove all the calls to `getASTContext`, but instead
eliminate a good fraction of the iterations in `getTranslationUnitDecl`.

F7691183: instrumentation_ref_d0fc178812 

This patch deals with:

VarDecl::isThisDeclarationADefinition
VarTemplateDecl::isThisDeclarationADefinition
Decl::canBeWeakImported
Decl::isWeakImported
VarDecl::getActingDefinition
ValueDecl::isWeak
VarDecl::checkInitIsICE
ComparisonCategoryInfo::ValueInfo::getIntValue
ComparisonCategoryInfo::ValueInfo::hasValidIntValue
VarDecl::isKnownToBeDefined
VarDecl::getDefinition
VarDecl::hasDefinition
VarTemplateDecl::getDefinition 
VarDecl::getTemplateInstantiationPattern


Repository:
  rC Clang

https://reviews.llvm.org/D55658

Files:
  include/clang/AST/ComparisonCategories.h
  include/clang/AST/Decl.h
  include/clang/AST/DeclBase.h
  include/clang/AST/DeclTemplate.h
  include/clang/ASTMatchers/ASTMatchers.h
  include/clang/Sema/SemaInternal.h
  lib/AST/ASTContext.cpp
  lib/AST/ASTImporter.cpp
  lib/AST/ComparisonCategories.cpp
  lib/AST/Decl.cpp
  lib/AST/DeclBase.cpp
  lib/AST/DeclTemplate.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprAgg.cpp
  lib/CodeGen/CGObjCGNU.cpp
  lib/CodeGen/CGObjCMac.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/Index/IndexingContext.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprCXX.cpp
  lib/Sema/SemaLookup.cpp
  lib/Sema/SemaOpenMP.cpp
  lib/Sema/SemaTemplateInstantiate.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Sema/SemaType.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  tools/libclang/CIndex.cpp
  tools/libclang/CXIndexDataConsumer.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -3336,18 +3336,20 @@
   FromTU, varDecl(hasName("a"))); // Decl with definition
   ASSERT_EQ(FromDWithInit, FromDWithDef->getPreviousDecl());
   ASSERT_TRUE(FromDWithInit->getInit());
-  ASSERT_FALSE(FromDWithInit->isThisDeclarationADefinition());
-  ASSERT_TRUE(FromDWithDef->isThisDeclarationADefinition());
+  ASSERT_FALSE(FromDWithInit->isThisDeclarationADefinition(
+  FromDWithInit->getASTContext()));
+  ASSERT_TRUE(FromDWithDef->isThisDeclarationADefinition(
+  FromDWithDef->getASTContext()));
   ASSERT_FALSE(FromDWithDef->getInit());
 
   auto *ToD = FirstDeclMatcher().match(
   ToTU, varDecl(hasName("a"))); // Decl with init
   ASSERT_TRUE(ToD->getInit());
-  ASSERT_FALSE(ToD->getDefinition());
+  ASSERT_FALSE(ToD->getDefinition(ToD->getASTContext()));
 
   auto *ImportedD = cast(Import(FromDWithDef, Lang_CXX11));
   EXPECT_TRUE(ImportedD->getAnyInitializer());
-  EXPECT_TRUE(ImportedD->getDefinition());
+  EXPECT_TRUE(ImportedD->getDefinition(ImportedD->getASTContext()));
 }
 
 TEST_P(ImportVariables, InitAndDefinitionAreInTheFromContext) {
@@ -3367,18 +3369,20 @@
   FromTU, varDecl(hasName("a"))); // Decl with definition and with init.
   ASSERT_EQ(FromDDeclarationOnly, FromDWithDef->getPreviousDecl());
   ASSERT_FALSE(FromDDeclarationOnly->getInit());
-  ASSERT_FALSE(FromDDeclarationOnly->isThisDeclarationADefinition());
-  ASSERT_TRUE(FromDWithDef->isThisDeclarationADefinition());
+  ASSERT_FALSE(FromDDeclarationOnly->isThisDeclarationADefinition(
+  FromDDeclarationOnly->getASTContext()));
+  ASSERT_TRUE(FromDWithDef->isThisDeclarationADefinition(
+  FromDWithDef->getASTContext()));
   ASSERT_TRUE(FromDWithDef->getInit());
 
   auto *ToD = FirstDeclMatcher().match(
   ToTU, varDecl(hasName("a")));
   ASSERT_FALSE(ToD->getInit());
-  ASSERT_FALSE(ToD->getDefinition());
+  ASSERT_FALSE(ToD->getDefinition(ToD->getASTContext()));
 
   auto *ImportedD = cast(Import(FromDWithDef, Lang_CXX11));
   EXPECT_TRUE(ImportedD->getAnyInitializer());
-  EXPECT_TRUE(ImportedD->getDefinition());
+  EXPECT_TRUE(ImportedD->getDefinition(ImportedD->getASTContext()));
 }
 
 struct DeclContextTest : ASTImporterTestBase {};
Index: tools/libclang/CXIndexDataConsumer.cpp
===
--- tools/libclang/CXIndexDataConsumer.cpp
+++ tools/libclang/CXIndexDataConsumer.cpp
@@ -6

[PATCH] D55315: [clangd] Only reduce priority of a thread for indexing.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/index/Background.h:121
   bool ShouldStop = false;
-  std::deque Queue;
+  std::list> Tasks;
   std::vector ThreadPool; // FIXME: Abstract this away.

kadircet wrote:
> ilya-biryukov wrote:
> > Why not `std::deque`?
> because we can insert in the middle of the queue now, when we skip existing 
> high priority tasks. 
`deque::insert` is `O(smaller of the distances to either begin or end)` and we 
spend the same time on the linear search anyway.
I believe we can keep it, even though I'm not strictly sure which is more 
efficient (we certainly don't care)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55315



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

Re: r349053 - [CodeComplete] Fill preferred type on binary expressions

[Ilya Biryukov via cfe-commits]

Hi David,

Thanks for the fix. And sorry for the inconvenience, I've been hit by this
so many times and yet I forget again and again...

On Thu, Dec 13, 2018 at 6:25 PM David Green  wrote:

> Hello!
>
> Certain version of gcc (along with ccache iirc, where they use -E) don't
> like these raw strings literals inside macros.
>
> This happens: https://godbolt.org/g/fsXjB7
>
> I've tried to fix it up in  
> https://reviews.llvm.org/rL349059. My choice of variable names may not
> have been very inspired though. Let me know if anything looks off.
>
>
> Cheers
>
> Dave
>
>

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

[PATCH] D55659: [Sema][ObjC] Disallow non-trivial C struct fields in unions

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak created this revision.
ahatanak added a reviewer: rjmccall.
ahatanak added a project: clang.
Herald added subscribers: dexonsmith, jkorous.

This patch fixes a bug where clang doesn’t reject union fields of non-trivial 
struct types:

  struct S0 {
id x;
  };
  
  struct S1 {
id y;
  };
  
  union U0 {
struct S0 s0;  // no diagnostics.
struct S1 s1;  // no diagnostics.
  };
  
  union U1 {
id x;  // clang rejects ObjC pointer fields in unions.
  };
  
  void test(union U0 a) {
// Both ‘S0::x’ and ‘S1::y' are destructed in the IR.
  }

rdar://problem/46677858


Repository:
  rC Clang

https://reviews.llvm.org/D55659

Files:
  include/clang/AST/Type.h
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDecl.cpp
  test/SemaObjC/arc-decls.m
  test/SemaObjCXX/objc-weak.mm

Index: test/SemaObjCXX/objc-weak.mm
===
--- test/SemaObjCXX/objc-weak.mm
+++ test/SemaObjCXX/objc-weak.mm
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -fobjc-runtime-has-weak -fobjc-weak -fblocks -Wno-objc-root-class -std=c++98 -Wno-c++0x-extensions -verify %s
+// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime-has-weak -fobjc-weak -fblocks -Wno-objc-root-class -std=c++11 -Wno-c++0x-extensions -verify %s
 
 @interface AnObject
 @property(weak) id value;
@@ -9,14 +10,19 @@
 @end
 
 struct S {
-  __weak id a; // expected-note {{because type 'S' has a member with __weak ownership}}
+  __weak id a;
 };
 
 union U {
   __weak id a; // expected-error {{ARC forbids Objective-C objects in union}}
-  S b; // expected-error {{union member 'b' has a non-trivial copy constructor}}
+  S b;
 };
 
+#if __cplusplus < 201103L
+// expected-note@-9 {{because type 'S' has a member with __weak ownership}}
+// expected-error@-5 {{union member 'b' has a non-trivial copy constructor}}
+#endif
+
 void testCast(AnObject *o) {
   __weak id a = reinterpret_cast<__weak NOWEAK *>(o); // expected-error {{class is incompatible with __weak references}} \
   // expected-error {{explicit ownership qualifier on cast result has no effect}} \
Index: test/SemaObjC/arc-decls.m
===
--- test/SemaObjC/arc-decls.m
+++ test/SemaObjC/arc-decls.m
@@ -10,6 +10,10 @@
 id u; // expected-error {{ARC forbids Objective-C objects in union}}
 };
 
+union u_nontrivial_c {
+struct A a; // expected-error {{ARC forbids non-trivial C types in union}}
+};
+
 @interface I {
struct A a; 
struct B {
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -15760,13 +15760,13 @@
   // Verify that all the fields are okay.
   SmallVector RecFields;
 
-  bool ObjCFieldLifetimeErrReported = false;
+  bool NonTrivialPrimitiveFieldErrReported = false;
   for (ArrayRef::iterator i = Fields.begin(), end = Fields.end();
i != end; ++i) {
 FieldDecl *FD = cast(*i);
 
 // Get the type for the field.
-const Type *FDTy = FD->getType().getTypePtr();
+QualType FDTy = FD->getType();
 
 if (!FD->isAnonymousStructOrUnion()) {
   // Remember all fields written by the user.
@@ -15868,6 +15868,40 @@
   FD->setInvalidDecl();
   EnclosingDecl->setInvalidDecl();
   continue;
+} else if (FDTy->isObjCObjectType()) {
+  /// A field cannot be an Objective-c object
+  Diag(FD->getLocation(), diag::err_statically_allocated_object)
+<< FixItHint::CreateInsertion(FD->getLocation(), "*");
+  QualType T = Context.getObjCObjectPointerType(FD->getType());
+  FD->setType(T);
+} else if (Record && !NonTrivialPrimitiveFieldErrReported &&
+   Record->isUnion() &&
+   (FDTy.hasNonTrivialObjCLifetime() ||
+(!getLangOpts().CPlusPlus &&
+ (FDTy.isNonTrivialToPrimitiveDefaultInitialize() ||
+  !FDTy.isTrivialOrTrivialVolatileToPrimitiveCopy() ||
+  !FDTy.isTrivialOrTrivialVolatileToPrimitiveMove() ||
+  FDTy.isDestructedType() {
+  // It's an error to have a field that has a non-trivial ObjC lifetime or
+  // a non-trivial C type in a union.
+  // We don't want to report this in a system header, though,
+  // so we just make the field unavailable.
+  // FIXME: that's really not sufficient; we need to make the type
+  // itself invalid to, say, initialize or copy.
+  SourceLocation loc = FD->getLocation();
+  if (getSourceManager().isInSystemHeader(loc)) {
+if (!FD->hasAttr()) {
+  FD->addAttr(UnavailableAttr::CreateImplicit(Context, "",
+UnavailableAttr::IR_ARCFieldWithOwnership, loc));
+}
+  } else if (FDTy.hasNonTrivialObjCLifetime()) {
+Diag(FD->getLocation(), diag::err_arc_objc_object_in_tag)
+  << FDTy->isBlockPointerType() << Recor

r349063 - [CodeComplete] Adhere to LLVM naming style in CodeCompletionTest. NFC

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Thu Dec 13 09:32:38 2018
New Revision: 349063

URL: http://llvm.org/viewvc/llvm-project?rev=349063&view=rev
Log:
[CodeComplete] Adhere to LLVM naming style in CodeCompletionTest. NFC

Also reuses the same var for multiple to reduce the chance of
accidentally referecing the previous test.

Modified:
cfe/trunk/unittests/Sema/CodeCompleteTest.cpp

Modified: cfe/trunk/unittests/Sema/CodeCompleteTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Sema/CodeCompleteTest.cpp?rev=349063&r1=349062&r2=349063&view=diff
==
--- cfe/trunk/unittests/Sema/CodeCompleteTest.cpp (original)
+++ cfe/trunk/unittests/Sema/CodeCompleteTest.cpp Thu Dec 13 09:32:38 2018
@@ -183,79 +183,80 @@ TEST(SemaCodeCompleteTest, VisitedNSWith
 
 TEST(PreferredTypeTest, BinaryExpr) {
   // Check various operations for arithmetic types.
-  StringRef code1 = R"cpp(
+  StringRef Code = R"cpp(
 void test(int x) {
   x = ^10;
   x += ^10; x -= ^10; x *= ^10; x /= ^10; x %= ^10;
   x + ^10; x - ^10; x * ^10; x / ^10; x % ^10;
 })cpp";
-  EXPECT_THAT(collectPreferredTypes(code1), Each("int"));
-  StringRef code2 = R"cpp(
+  EXPECT_THAT(collectPreferredTypes(Code), Each("int"));
+
+  Code = R"cpp(
 void test(float x) {
   x = ^10;
   x += ^10; x -= ^10; x *= ^10; x /= ^10; x %= ^10;
   x + ^10; x - ^10; x * ^10; x / ^10; x % ^10;
 })cpp";
-  EXPECT_THAT(collectPreferredTypes(code2), Each("float"));
+  EXPECT_THAT(collectPreferredTypes(Code), Each("float"));
 
   // Pointer types.
-  StringRef code3 = R"cpp(
+  Code = R"cpp(
 void test(int *ptr) {
   ptr - ^ptr;
   ptr = ^ptr;
 })cpp";
-  EXPECT_THAT(collectPreferredTypes(code3), Each("int *"));
+  EXPECT_THAT(collectPreferredTypes(Code), Each("int *"));
 
-  StringRef code4 = R"cpp(
+  Code = R"cpp(
 void test(int *ptr) {
   ptr + ^10;
   ptr += ^10;
   ptr -= ^10;
 })cpp";
-  EXPECT_THAT(collectPreferredTypes(code4), Each("long")); // long is 
normalized 'ptrdiff_t'.
+  EXPECT_THAT(collectPreferredTypes(Code), Each("long")); // long is 
normalized 'ptrdiff_t'.
 
   // Comparison operators.
-  StringRef code5 = R"cpp(
+  Code = R"cpp(
 void test(int i) {
   i <= ^1; i < ^1; i >= ^1; i > ^1; i == ^1; i != ^1;
 }
   )cpp";
-  EXPECT_THAT(collectPreferredTypes(code5), Each("int"));
+  EXPECT_THAT(collectPreferredTypes(Code), Each("int"));
 
-  StringRef code6 = R"cpp(
+  Code = R"cpp(
 void test(int *ptr) {
   ptr <= ^ptr; ptr < ^ptr; ptr >= ^ptr; ptr > ^ptr;
   ptr == ^ptr; ptr != ^ptr;
 }
   )cpp";
-  EXPECT_THAT(collectPreferredTypes(code6), Each("int *"));
+  EXPECT_THAT(collectPreferredTypes(Code), Each("int *"));
 
   // Relational operations.
-  StringRef code7 = R"cpp(
+  Code = R"cpp(
 void test(int i, int *ptr) {
   i && ^1; i || ^1;
   ptr && ^1; ptr || ^1;
 }
   )cpp";
-  EXPECT_THAT(collectPreferredTypes(code7), Each("_Bool"));
+  EXPECT_THAT(collectPreferredTypes(Code), Each("_Bool"));
 
   // Bitwise operations.
-  StringRef code8 = R"cpp(
+  Code = R"cpp(
 void test(long long ll) {
   ll | ^1; ll & ^1;
 }
   )cpp";
-  EXPECT_THAT(collectPreferredTypes(code8), Each("long long"));
+  EXPECT_THAT(collectPreferredTypes(Code), Each("long long"));
 
-  StringRef code9 = R"cpp(
+  Code = R"cpp(
 enum A {};
 void test(A a) {
   a | ^1; a & ^1;
 }
   )cpp";
-  EXPECT_THAT(collectPreferredTypes(code9), Each("enum A"));
+  EXPECT_THAT(collectPreferredTypes(Code), Each("enum A"));
 
-  StringRef code10 = R"cpp(
+  Code = R"cpp(
 enum class A {};
 void test(A a) {
   // This is technically illegal with the 'enum class' without overloaded
@@ -263,10 +264,10 @@ TEST(PreferredTypeTest, BinaryExpr) {
   a | ^a; a & ^a;
 }
   )cpp";
-  EXPECT_THAT(collectPreferredTypes(code10), Each("enum A"));
+  EXPECT_THAT(collectPreferredTypes(Code), Each("enum A"));
 
   // Binary shifts.
-  StringRef code11 = R"cpp(
+  Code = R"cpp(
 void test(int i, long long ll) {
   i << ^1; ll << ^1;
   i <<= ^1; i <<= ^1;
@@ -274,10 +275,10 @@ TEST(PreferredTypeTest, BinaryExpr) {
   i >>= ^1; i >>= ^1;
 }
   )cpp";
-  EXPECT_THAT(collectPreferredTypes(code11), Each("int"));
+  EXPECT_THAT(collectPreferredTypes(Code), Each("int"));
 
   // Comma does not provide any useful information.
-  StringRef code12 = R"cpp(
+  Code = R"cpp(
 class Cls {};
 void test(int i, int* ptr, Cls x) {
   (i, ^i);
@@ -285,11 +286,11 @@ TEST(PreferredTypeTest, BinaryExpr) {
   (x, ^x);
 }
   )cpp";
-  EXPECT_THAT(collectPreferredTypes(code12), Each("NULL TYPE"));
+  EXPECT_THAT(collectPreferredTypes(Code), Each("NULL TYPE"));
 
   // User-defined types do not take operator overloading into account.
   // However, they provide heuristics for some common cases.
-  StringRef code13 = R"cpp(
+  Code = R"cpp(
 class Cls {};
   

r349064 - Try to update the test to fix the breakage

[Sylvestre Ledru via cfe-commits]

Author: sylvestre
Date: Thu Dec 13 09:39:02 2018
New Revision: 349064

URL: http://llvm.org/viewvc/llvm-project?rev=349064&view=rev
Log:
Try to update the test to fix the breakage
With the new warning, we are showing one more output in the test.


Modified:
cfe/trunk/bindings/python/tests/cindex/test_diagnostics.py

Modified: cfe/trunk/bindings/python/tests/cindex/test_diagnostics.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/bindings/python/tests/cindex/test_diagnostics.py?rev=349064&r1=349063&r2=349064&view=diff
==
--- cfe/trunk/bindings/python/tests/cindex/test_diagnostics.py (original)
+++ cfe/trunk/bindings/python/tests/cindex/test_diagnostics.py Thu Dec 13 
09:39:02 2018
@@ -15,7 +15,7 @@ import unittest
 class TestDiagnostics(unittest.TestCase):
 def test_diagnostic_warning(self):
 tu = get_tu('int f0() {}\n')
-self.assertEqual(len(tu.diagnostics), 1)
+self.assertEqual(len(tu.diagnostics), 2)
 self.assertEqual(tu.diagnostics[0].severity, Diagnostic.Warning)
 self.assertEqual(tu.diagnostics[0].location.line, 1)
 self.assertEqual(tu.diagnostics[0].location.column, 11)


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

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay marked 6 inline comments as done.
MyDeveloperDay added a comment.



>> 
>> 
>>   unsigned BlockOrCode = 0;
>>   llvm::ErrorOr Res = skipUntilRecordOrBlock(Stream, BlockOrCode);
>>   if (!Res)
>> Res.getError();
>>
> 
> AFAIK `llvm::Error` must be consumed because if it goes out of scope 
> unhandled it will `assert`. Not sure how to handle that.

Actually in this case its the getError() that the offender




Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:73
+  // c++17 compilers.
+  if (!getLangOpts().CPlusPlus)
+return;

curdeius wrote:
> I'd move this if to the bottom of the function as it's the most general one 
> and fix the comment above: e.g. `// Ignore non-C++ code.`.
merged into one if as suggested by @JonasToth 



Comment at: test/clang-tidy/modernize-use-nodiscard.cpp:126
+template
+class Bar
+{

JonasToth wrote:
> MyDeveloperDay wrote:
> > curdeius wrote:
> > > JonasToth wrote:
> > > > I think the template tests should be improved.
> > > > What happens for `T empty()`, `typename T::value_type empty()` and so 
> > > > on. THe same for the parameters for functions/methods (including 
> > > > function templates).
> > > > 
> > > > Thinking of it, it might be a good idea to ignore functions where the 
> > > > heuristic depends on the template-paramters.
> > > It might be a good idea to add a note in the documentation about handling 
> > > of function templates and functions in class templates.
> > I think I need some help to determine if the parameter is a template 
> > parameter (specially a const T & or a const_reference)
> > 
> > I'm trying to remove functions which have any type of Template parameter 
> > (at least for now)
> > 
> > I've modified the hasNonConstReferenceOrPointerArguments matcher to use 
> > isTemplateTypeParamType()
> > 
> > but this doesn't seem to work though an Alias or even just with a const &
> > 
> > ```
> >   return llvm::any_of(Node.parameters(), [](const ParmVarDecl *Par) {
> > QualType ParType = Par->getType();
> > 
> > if (ParType->isTemplateTypeParmType())
> >   return true;
> > 
> > if (ParType->isPointerType() || isNonConstReferenceType(ParType))
> >   return true;
> > 
> > return false;
> >   });
> > ```
> > 
> > mostly the tests cases work for  T and T&  (see below)
> > 
> > but it does not seem to work for const T&, or const_reference, where it 
> > still wants to add the [[nodiscard]]
> > 
> > Could anyone give me any pointers, or somewhere I can look to learn?  I was 
> > thinking I needed to look at the getUnqualifiedDeSugared() but it didn't 
> > seem to work the way I expected.
> > 
> > ```
> > template
> > class Bar
> > {
> > public:
> > using value_type = T;
> > using reference = value_type&;
> > using const_reference = const value_type&;
> > 
> > bool empty() const;
> > // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'empty' should be 
> > marked NO_DISCARD [modernize-use-nodiscard]
> > // CHECK-FIXES: NO_DISCARD bool empty() const;
> > 
> > // we cannot assume that the template parameter isn't a pointer
> > bool f25(value_type) const;
> > // CHECK-MESSAGES-NOT: warning:
> > // CHECK-FIXES: bool f25(value_type) const;
> > 
> > bool f27(reference) const;
> > // CHECK-MESSAGES-NOT: warning:
> > // CHECK-FIXES: bool f27(reference) const
> > 
> > typename T::value_type f35() const;
> > // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f35' should be 
> > marked NO_DISCARD [modernize-use-nodiscard]
> > // CHECK-FIXES: NO_DISCARD typename T::value_type f35() const
> > 
> > T f34() const;
> > // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f34' should be 
> > marked NO_DISCARD [modernize-use-nodiscard]
> > // CHECK-FIXES: NO_DISCARD T f34() const
> > 
> > bool f31(T) const;
> > // CHECK-MESSAGES-NOT: warning:
> > // CHECK-FIXES: bool f31(T) const
> > 
> > bool f33(T&) const;
> > // CHECK-MESSAGES-NOT: warning:
> > // CHECK-FIXES: bool f33(T&) const
> > 
> > // -  FIXME TESTS BELOW FAIL - //
> > bool f26(const_reference) const;
> > // CHECK-MESSAGES-NOT: warning:
> > // CHECK-FIXES: bool f26(const_reference) const;
> > 
> > bool f32(const T&) const;
> > // CHECK-MESSAGES-NOT: warning:
> > // CHECK-FIXES: bool f32(const T&) const
> > };
> > 
> > ```
> > 
> > 
> > 
> > 
> > 
> > 
> Is this resolved, as you marked it done?
Still working on these last 2 cases, they don't seem to be excluded with the 
isTemplateTypeParmType() call

// -  FIXME TESTS BELOW FAIL - //
bool f26(const_reference) const;
// CHECK-MESSAGES-NOT: warning:
// CHECK-FIXES: bool f26(const_reference) const;

bool f32(const T&) const;
// CHECK-MESSAGES-NOT: warning:
// CHECK-FIXES: bool f32(const T&) const

Let me fix what I can and I'll send an updated rev

[PATCH] D55659: [Sema][ObjC] Disallow non-trivial C struct fields in unions

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

Just to clarify, this patch doesn't change clang's handling of C++ unions that 
have non-static data members with non-trivial special member functions.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55659



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

r349065 - Reinstate DW_AT_comp_dir support after D55519.

[Adrian Prantl via cfe-commits]

Author: adrian
Date: Thu Dec 13 09:53:29 2018
New Revision: 349065

URL: http://llvm.org/viewvc/llvm-project?rev=349065&view=rev
Log:
Reinstate DW_AT_comp_dir support after D55519.

The DIFile used by the CU is special and distinct from the main source
file. Its directory part specifies what becomes the DW_AT_comp_dir
(the compilation directory), even if the source file was specified
with an absolute path.

To support the .dwo workflow, a valid DW_AT_comp_dir is necessary even
if source files were specified with an absolute path.

Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/test/CodeGen/debug-info-abspath.c

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=349065&r1=349064&r2=349065&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Thu Dec 13 09:53:29 2018
@@ -416,7 +416,6 @@ llvm::DIFile *CGDebugInfo::getOrCreateFi
 
   // Cache the results.
   auto It = DIFileCache.find(FileName.data());
-
   if (It != DIFileCache.end()) {
 // Verify that the information still exists.
 if (llvm::Metadata *V = It->second)
@@ -595,22 +594,27 @@ void CGDebugInfo::CreateCompileUnit() {
 break;
   }
 
+  uint64_t DwoId = 0;
+  auto &CGOpts = CGM.getCodeGenOpts();
+  // The DIFile used by the CU is distinct from the main source
+  // file. Its directory part specifies what becomes the
+  // DW_AT_comp_dir (the compilation directory), even if the source
+  // file was specified with an absolute path.
   if (CSKind)
 CSInfo.emplace(*CSKind, Checksum);
+  llvm::DIFile *CUFile = DBuilder.createFile(
+  remapDIPath(MainFileName), remapDIPath(getCurrentDirname()), CSInfo,
+  getSource(SM, SM.getMainFileID()));
 
   // Create new compile unit.
-  // FIXME - Eliminate TheCU.
-  auto &CGOpts = CGM.getCodeGenOpts();
   TheCU = DBuilder.createCompileUnit(
-  LangTag,
-  createFile(MainFileName, CSInfo, getSource(SM, SM.getMainFileID())),
-  CGOpts.EmitVersionIdentMetadata ? Producer : "",
+  LangTag, CUFile, CGOpts.EmitVersionIdentMetadata ? Producer : "",
   LO.Optimize || CGOpts.PrepareForLTO || CGOpts.PrepareForThinLTO,
   CGOpts.DwarfDebugFlags, RuntimeVers,
   (CGOpts.getSplitDwarfMode() != CodeGenOptions::NoFission)
   ? ""
   : CGOpts.SplitDwarfFile,
-  EmissionKind, 0 /* DWOid */, CGOpts.SplitDwarfInlining,
+  EmissionKind, DwoId, CGOpts.SplitDwarfInlining,
   CGOpts.DebugInfoForProfiling,
   CGM.getTarget().getTriple().isNVPTX()
   ? llvm::DICompileUnit::DebugNameTableKind::None

Modified: cfe/trunk/test/CodeGen/debug-info-abspath.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/debug-info-abspath.c?rev=349065&r1=349064&r2=349065&view=diff
==
--- cfe/trunk/test/CodeGen/debug-info-abspath.c (original)
+++ cfe/trunk/test/CodeGen/debug-info-abspath.c Thu Dec 13 09:53:29 2018
@@ -8,12 +8,29 @@
 // RUN: cp %s %t.c
 // RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple \
 // RUN:   %t.c -emit-llvm -o - | FileCheck %s --check-prefix=INTREE
+
+// RUN: cd %t/UNIQUEISH_SENTINEL
+// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple \
+// RUN:   debug-info-abspath.c -emit-obj -o /tmp/t.o
+// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple \
+// RUN:   debug-info-abspath.c -emit-llvm -o - \
+// RUN:   | FileCheck %s --check-prefix=CURDIR
+// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple \
+// RUN:   %s -emit-llvm -o - | FileCheck %s --check-prefix=CURDIR
+
 void foo() {}
 
 // Since %s is an absolute path, directory should be the common
 // prefix, but the directory part should be part of the filename.
 
-// CHECK: DIFile(filename: "{{.*}}UNIQUEISH_SENTINEL{{.*}}debug-info-abspath.c"
-// CHECK-NOT:directory: "{{.*}}UNIQUEISH_SENTINEL
+// CHECK: = distinct !DISubprogram({{.*}}file: ![[SPFILE:[0-9]+]]
+// CHECK: ![[SPFILE]] = !DIFile(filename: "{{.*}}UNIQUEISH_SENTINEL
+// CHECK-SAME:  debug-info-abspath.c"
+// CHECK-NOT:   directory: "{{.*}}UNIQUEISH_SENTINEL
 
+// INTREE: = distinct !DISubprogram({{.*}}![[SPFILE:[0-9]+]]
 // INTREE: DIFile({{.*}}directory: "{{.+}}CodeGen{{.*}}")
+
+// CURDIR: = distinct !DICompileUnit({{.*}}file: ![[CUFILE:[0-9]+]]
+// CURDIR: ![[CUFILE]] = !DIFile({{.*}}directory: "{{.+}}UNIQUEISH_SENTINEL")
+


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

[PATCH] D55545: Allow IncludeSorter to use #import for Objective-C files

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Please add a unittest for it, you can add it in 
`unittests/clang-tidy/IncludeInserterTest.cpp`.




Comment at: clang-tidy/utils/IncludeSorter.cpp:116
   bool IsAngled) {
+  std::string IncludeDirective = LangOpts->ObjC ? "#import " : "#include ";
   std::string IncludeStmt =

What about the ObjC++? The current behavior is always using `#import`, google 
objc style guide says `#import Objective-C and Objective-C++ headers, and 
#include C/C++ headers.`, I don't think we have a way to do it smartly.

Maybe a conservative way is to use `#import ` for ObjC only?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55545



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

[PATCH] D55545: Allow IncludeSorter to use #import for Objective-C files

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added inline comments.



Comment at: clang-tidy/utils/IncludeSorter.cpp:116
   bool IsAngled) {
+  std::string IncludeDirective = LangOpts->ObjC ? "#import " : "#include ";
   std::string IncludeStmt =

hokein wrote:
> What about the ObjC++? The current behavior is always using `#import`, google 
> objc style guide says `#import Objective-C and Objective-C++ headers, and 
> #include C/C++ headers.`, I don't think we have a way to do it smartly.
> 
> Maybe a conservative way is to use `#import ` for ObjC only?
Yeah, came here to say this. Let's enable `#import` for ObjC but disable for 
C++ (which will include ObjC++).



Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55545



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

[PATCH] D55315: [clangd] Only reduce priority of a thread for indexing.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clangd/index/Background.h:121
   bool ShouldStop = false;
-  std::deque Queue;
+  std::list> Tasks;
   std::vector ThreadPool; // FIXME: Abstract this away.

ilya-biryukov wrote:
> kadircet wrote:
> > ilya-biryukov wrote:
> > > Why not `std::deque`?
> > because we can insert in the middle of the queue now, when we skip existing 
> > high priority tasks. 
> `deque::insert` is `O(smaller of the distances to either begin or end)` and 
> we spend the same time on the linear search anyway.
> I believe we can keep it, even though I'm not strictly sure which is more 
> efficient (we certainly don't care)
Cool, didn't know that. If that's the case we definitely don't care.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55315



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

[PATCH] D55315: [clangd] Only reduce priority of a thread for indexing.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 178094.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Use std::deque instead of std::list


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55315

Files:
  clangd/Threading.cpp
  clangd/Threading.h
  clangd/index/Background.cpp
  clangd/index/Background.h

Index: clangd/index/Background.h
===
--- clangd/index/Background.h
+++ clangd/index/Background.h
@@ -13,6 +13,7 @@
 #include "Context.h"
 #include "FSProvider.h"
 #include "GlobalCompilationDatabase.h"
+#include "Threading.h"
 #include "index/FileIndex.h"
 #include "index/Index.h"
 #include "index/Serialization.h"
@@ -110,14 +111,14 @@
   // queue management
   using Task = std::function;
   void run(); // Main loop executed by Thread. Runs tasks from Queue.
-  void enqueueTask(Task T);
+  void enqueueTask(Task T, ThreadPriority Prioirty);
   void enqueueLocked(tooling::CompileCommand Cmd,
  BackgroundIndexStorage *IndexStorage);
-  std::mutex QueueMu;
+  std::mutex TasksMu;
   unsigned NumActiveTasks = 0; // Only idle when queue is empty *and* no tasks.
-  std::condition_variable QueueCV;
+  std::condition_variable TasksCV;
   bool ShouldStop = false;
-  std::deque Queue;
+  std::deque> Tasks;
   std::vector ThreadPool; // FIXME: Abstract this away.
   GlobalCompilationDatabase::CommandChanged::Subscription CommandsChanged;
 };
Index: clangd/index/Background.cpp
===
--- clangd/index/Background.cpp
+++ clangd/index/Background.cpp
@@ -22,6 +22,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/SHA1.h"
@@ -102,14 +103,8 @@
   })) {
   assert(ThreadPoolSize > 0 && "Thread pool size can't be zero.");
   assert(this->IndexStorageFactory && "Storage factory can not be null!");
-  while (ThreadPoolSize--) {
+  while (ThreadPoolSize--)
 ThreadPool.emplace_back([this] { run(); });
-// Set priority to low, since background indexing is a long running task we
-// do not want to eat up cpu when there are any other high priority threads.
-// FIXME: In the future we might want a more general way of handling this to
-// support tasks with various priorities.
-setThreadPriority(ThreadPool.back(), ThreadPriority::Low);
-  }
 }
 
 BackgroundIndex::~BackgroundIndex() {
@@ -120,86 +115,109 @@
 
 void BackgroundIndex::stop() {
   {
-std::lock_guard Lock(QueueMu);
+std::lock_guard Lock(TasksMu);
 ShouldStop = true;
   }
-  QueueCV.notify_all();
+  TasksCV.notify_all();
 }
 
 void BackgroundIndex::run() {
   WithContext Background(BackgroundContext.clone());
   while (true) {
 Optional Task;
+ThreadPriority Priority;
 {
-  std::unique_lock Lock(QueueMu);
-  QueueCV.wait(Lock, [&] { return ShouldStop || !Queue.empty(); });
+  std::unique_lock Lock(TasksMu);
+  TasksCV.wait(Lock, [&] { return ShouldStop || !Tasks.empty(); });
   if (ShouldStop) {
-Queue.clear();
-QueueCV.notify_all();
+Tasks.clear();
+TasksCV.notify_all();
 return;
   }
   ++NumActiveTasks;
-  Task = std::move(Queue.front());
-  Queue.pop_front();
+  std::tie(Task, Priority) = std::move(Tasks.front());
+  Tasks.pop_front();
 }
+
+if (Priority != ThreadPriority::Normal)
+  setCurrentThreadPriority(Priority);
 (*Task)();
+if (Priority != ThreadPriority::Normal)
+  setCurrentThreadPriority(ThreadPriority::Normal);
+
 {
-  std::unique_lock Lock(QueueMu);
+  std::unique_lock Lock(TasksMu);
   assert(NumActiveTasks > 0 && "before decrementing");
   --NumActiveTasks;
 }
-QueueCV.notify_all();
+TasksCV.notify_all();
   }
 }
 
 bool BackgroundIndex::blockUntilIdleForTest(
 llvm::Optional TimeoutSeconds) {
-  std::unique_lock Lock(QueueMu);
-  return wait(Lock, QueueCV, timeoutSeconds(TimeoutSeconds),
-  [&] { return Queue.empty() && NumActiveTasks == 0; });
+  std::unique_lock Lock(TasksMu);
+  return wait(Lock, TasksCV, timeoutSeconds(TimeoutSeconds),
+  [&] { return Tasks.empty() && NumActiveTasks == 0; });
 }
 
 void BackgroundIndex::enqueue(const std::vector &ChangedFiles) {
-  enqueueTask([this, ChangedFiles] {
-trace::Span Tracer("BackgroundIndexEnqueue");
-// We're doing this asynchronously, because we'll read shards here too.
-// FIXME: read shards here too.
-
-log("Enqueueing {0} commands for indexing", ChangedFiles.size());
-SPAN_ATTACH(Tracer, "files", int64_t(ChangedFiles.size()));
-
-// We shuffle the files because processing them in a random order should
-// quickly give us good cover

[PATCH] D55640: [clang-tidy] Implement a check for large Objective-C type encodings 🔍

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/clang-tidy/checks/objc-type-encoding-size.rst:6
+
+Finds Objective-C type encodings that exceed a configured threshold.
+

Please synchronize with Release Notes.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55640



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

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Andy Zhang via Phabricator via cfe-commits]

axzhang added a comment.

In D55044#1329604 , @hokein wrote:

> In fact, the existing `modernize-make-unique` can be configured to support 
> `absl::make_unique`, you'd just need to configure the check option 
> `MakeSmartPtrFunction` to `absl::make_unique` (this is what we do inside 
> google).
>
> The biggest missing part of the `modernize-make-unique` is 
> `absl::WrapUnique`, I think we should extend `MakeSmartPtrCheck` class (maybe 
> add hooks) to support it.


What is the best way to extend `MakeSmartPtrCheck`? The behavior I want to 
achieve is that `absl::WrapUnique` is suggested when brace initialization is 
used, but `absl::make_unique` is used in all other cases.


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

https://reviews.llvm.org/D55044



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

[PATCH] D55662: [Sema][ObjC] Do not warn about repeated uses of weak variables when the variables are accessed in an unevaluated context.

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak created this revision.
ahatanak added reviewers: erik.pilkington, rjmccall, rsmith.
ahatanak added a project: clang.
Herald added subscribers: dexonsmith, jkorous.

Sema shouldn't record uses of weak variables when they are used in an 
unevaluated context.

For example:

  decltype([obj weakProp]) 
  decltype(objc.weakProp)
  __typeof__(objc.weakProp)

rdar://problem/45742525


Repository:
  rC Clang

https://reviews.llvm.org/D55662

Files:
  lib/Sema/SemaExprObjC.cpp
  lib/Sema/SemaType.cpp
  test/SemaObjC/arc-repeated-weak.mm


Index: test/SemaObjC/arc-repeated-weak.mm
===
--- test/SemaObjC/arc-repeated-weak.mm
+++ test/SemaObjC/arc-repeated-weak.mm
@@ -462,6 +462,9 @@
   NSString * t2 = NSBundle.foo2.prop;
   use(NSBundle.foo2.weakProp); // expected-warning{{weak property 'weakProp' 
may be accessed multiple times}}
   use(NSBundle2.foo2.weakProp); // expected-note{{also accessed here}}
+  decltype([NSBundle2.foo2 weakProp]) t3;
+  decltype(NSBundle2.foo2.weakProp) t4;
+  __typeof__(NSBundle2.foo2.weakProp) t5;
 }
 
 // This used to crash in the constructor of WeakObjectProfileTy when a
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -8040,6 +8040,8 @@
 }
 
 QualType Sema::BuildTypeofExprType(Expr *E, SourceLocation Loc) {
+  EnterExpressionEvaluationContext Unevaluated(
+  *this, ExpressionEvaluationContext::Unevaluated);
   ExprResult ER = CheckPlaceholderExpr(E);
   if (ER.isInvalid()) return QualType();
   E = ER.get();
@@ -8127,6 +8129,9 @@
 
 QualType Sema::BuildDecltypeType(Expr *E, SourceLocation Loc,
  bool AsUnevaluated) {
+  EnterExpressionEvaluationContext Unevaluated(
+  *this, ExpressionEvaluationContext::Unevaluated);
+
   ExprResult ER = CheckPlaceholderExpr(E);
   if (ER.isInvalid()) return QualType();
   E = ER.get();
Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -3140,7 +3140,7 @@
   Prop->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_weak;
 if (!IsWeak && Sel.isUnarySelector())
   IsWeak = ReturnType.getObjCLifetime() & Qualifiers::OCL_Weak;
-if (IsWeak &&
+if (IsWeak && !isUnevaluatedContext() &&
 !Diags.isIgnored(diag::warn_arc_repeated_use_of_weak, LBracLoc))
   getCurFunction()->recordUseOfWeak(Result, Prop);
   }


Index: test/SemaObjC/arc-repeated-weak.mm
===
--- test/SemaObjC/arc-repeated-weak.mm
+++ test/SemaObjC/arc-repeated-weak.mm
@@ -462,6 +462,9 @@
   NSString * t2 = NSBundle.foo2.prop;
   use(NSBundle.foo2.weakProp); // expected-warning{{weak property 'weakProp' may be accessed multiple times}}
   use(NSBundle2.foo2.weakProp); // expected-note{{also accessed here}}
+  decltype([NSBundle2.foo2 weakProp]) t3;
+  decltype(NSBundle2.foo2.weakProp) t4;
+  __typeof__(NSBundle2.foo2.weakProp) t5;
 }
 
 // This used to crash in the constructor of WeakObjectProfileTy when a
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -8040,6 +8040,8 @@
 }
 
 QualType Sema::BuildTypeofExprType(Expr *E, SourceLocation Loc) {
+  EnterExpressionEvaluationContext Unevaluated(
+  *this, ExpressionEvaluationContext::Unevaluated);
   ExprResult ER = CheckPlaceholderExpr(E);
   if (ER.isInvalid()) return QualType();
   E = ER.get();
@@ -8127,6 +8129,9 @@
 
 QualType Sema::BuildDecltypeType(Expr *E, SourceLocation Loc,
  bool AsUnevaluated) {
+  EnterExpressionEvaluationContext Unevaluated(
+  *this, ExpressionEvaluationContext::Unevaluated);
+
   ExprResult ER = CheckPlaceholderExpr(E);
   if (ER.isInvalid()) return QualType();
   E = ER.get();
Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -3140,7 +3140,7 @@
   Prop->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_weak;
 if (!IsWeak && Sel.isUnarySelector())
   IsWeak = ReturnType.getObjCLifetime() & Qualifiers::OCL_Weak;
-if (IsWeak &&
+if (IsWeak && !isUnevaluatedContext() &&
 !Diags.isIgnored(diag::warn_arc_repeated_use_of_weak, LBracLoc))
   getCurFunction()->recordUseOfWeak(Result, Prop);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r349073 - [clang-tidy] Add the abseil-duration-subtraction check

[Hyrum Wright via cfe-commits]

Author: hwright
Date: Thu Dec 13 11:23:52 2018
New Revision: 349073

URL: http://llvm.org/viewvc/llvm-project?rev=349073&view=rev
Log:
[clang-tidy] Add the abseil-duration-subtraction check

Summary:
This check uses the context of a subtraction expression as well as knowledge
about the Abseil Time types, to infer the type of the second operand of some
subtraction expressions in Duration conversions. For example:

   absl::ToDoubleSeconds(duration) - foo

can become
   absl::ToDoubleSeconds(duration - absl::Seconds(foo))

This ensures that time calculations are done in the proper domain, and also
makes it easier to further deduce the types of the second operands to these
expressions.

Reviewed By: JonasToth

Tags: #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D55245

Added:
clang-tools-extra/trunk/clang-tidy/abseil/DurationSubtractionCheck.cpp
clang-tools-extra/trunk/clang-tidy/abseil/DurationSubtractionCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-subtraction.rst
clang-tools-extra/trunk/test/clang-tidy/abseil-duration-subtraction.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/abseil/DurationComparisonCheck.cpp
clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp
clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.h
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp?rev=349073&r1=349072&r2=349073&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp Thu Dec 13 
11:23:52 2018
@@ -14,6 +14,7 @@
 #include "DurationDivisionCheck.h"
 #include "DurationFactoryFloatCheck.h"
 #include "DurationFactoryScaleCheck.h"
+#include "DurationSubtractionCheck.h"
 #include "FasterStrsplitDelimiterCheck.h"
 #include "NoInternalDependenciesCheck.h"
 #include "NoNamespaceCheck.h"
@@ -37,6 +38,8 @@ public:
 "abseil-duration-factory-float");
 CheckFactories.registerCheck(
 "abseil-duration-factory-scale");
+CheckFactories.registerCheck(
+"abseil-duration-subtraction");
 CheckFactories.registerCheck(
 "abseil-faster-strsplit-delimiter");
 CheckFactories.registerCheck(

Modified: clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt?rev=349073&r1=349072&r2=349073&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt Thu Dec 13 
11:23:52 2018
@@ -7,6 +7,7 @@ add_clang_library(clangTidyAbseilModule
   DurationFactoryFloatCheck.cpp
   DurationFactoryScaleCheck.cpp
   DurationRewriter.cpp
+  DurationSubtractionCheck.cpp
   FasterStrsplitDelimiterCheck.cpp
   NoInternalDependenciesCheck.cpp
   NoNamespaceCheck.cpp

Modified: clang-tools-extra/trunk/clang-tidy/abseil/DurationComparisonCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/DurationComparisonCheck.cpp?rev=349073&r1=349072&r2=349073&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/DurationComparisonCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/DurationComparisonCheck.cpp Thu 
Dec 13 11:23:52 2018
@@ -19,101 +19,6 @@ namespace clang {
 namespace tidy {
 namespace abseil {
 
-/// Given the name of an inverse Duration function (e.g., `ToDoubleSeconds`),
-/// return its `DurationScale`, or `None` if a match is not found.
-static llvm::Optional getScaleForInverse(llvm::StringRef Name) {
-  static const llvm::DenseMap ScaleMap(
-  {{"ToDoubleHours", DurationScale::Hours},
-   {"ToInt64Hours", DurationScale::Hours},
-   {"ToDoubleMinutes", DurationScale::Minutes},
-   {"ToInt64Minutes", DurationScale::Minutes},
-   {"ToDoubleSeconds", DurationScale::Seconds},
-   {"ToInt64Seconds", DurationScale::Seconds},
-   {"ToDoubleMilliseconds", DurationScale::Milliseconds},
-   {"ToInt64Milliseconds", DurationScale::Milliseconds},
-   {"ToDoubleMicroseconds", DurationScale::Microseconds},
-   {"ToInt64Microseconds", DurationScale::Microseconds},
-   {"ToDoubleNanoseconds", DurationScale::Nanoseconds},
-   {"ToInt64Nanoseconds", DurationScale::Nanoseconds}});
-
-  auto ScaleIter = ScaleMap.find(std::string(Name));
-  if (ScaleIter == ScaleMap.end())
-return

[PATCH] D55245: [clang-tidy] Add the abseil-duration-subtraction check

[Hyrum Wright via Phabricator via cfe-commits]

hwright added a comment.

Thanks for reviewing, I'll go ahead and commit.

I've also removed the hash specialization, since we moved to `llvm::IndexedMap` 
instead of `std::unordered_map` inside of `getInverseForScale`.


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

https://reviews.llvm.org/D55245



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

[PATCH] D55245: [clang-tidy] Add the abseil-duration-subtraction check

[Hyrum Wright via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL349073: [clang-tidy] Add the abseil-duration-subtraction 
check (authored by hwright, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55245?vs=178039&id=178101#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55245

Files:
  clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/abseil/DurationComparisonCheck.cpp
  clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp
  clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.h
  clang-tools-extra/trunk/clang-tidy/abseil/DurationSubtractionCheck.cpp
  clang-tools-extra/trunk/clang-tidy/abseil/DurationSubtractionCheck.h
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-subtraction.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
  clang-tools-extra/trunk/test/clang-tidy/abseil-duration-subtraction.cpp

Index: clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-subtraction.rst
===
--- clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-subtraction.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-subtraction.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - abseil-duration-subtraction
+
+abseil-duration-subtraction
+===
+
+Checks for cases where subtraction should be performed in the
+``absl::Duration`` domain. When subtracting two values, and the first one is
+known to be a conversion from ``absl::Duration``, we can infer that the second
+should also be interpreted as an ``absl::Duration``, and make that inference
+explicit.
+
+Examples:
+
+.. code-block:: c++
+
+  // Original - Subtraction in the double domain
+  double x;
+  absl::Duration d;
+  double result = absl::ToDoubleSeconds(d) - x;
+
+  // Suggestion - Subtraction in the absl::Duration domain instead
+  double result = absl::ToDoubleSeconds(d - absl::Seconds(x));
+
+
+  // Original - Subtraction of two Durations in the double domain
+  absl::Duration d1, d2;
+  double result = absl::ToDoubleSeconds(d1) - absl::ToDoubleSeconds(d2);
+
+  // Suggestion - Subtraction in the absl::Duration domain instead
+  double result = absl::ToDoubleSeconds(d1 - d2);
+
+Note: As with other ``clang-tidy`` checks, it is possible that multiple fixes
+may overlap (as in the case of nested expressions), so not all occurences can
+be transformed in one run. In particular, this may occur for nested subtraction
+expressions. Running ``clang-tidy`` multiple times will find and fix these
+overlaps.
Index: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
@@ -8,6 +8,7 @@
abseil-duration-division
abseil-duration-factory-float
abseil-duration-factory-scale
+   abseil-duration-subtraction
abseil-faster-strsplit-delimiter
abseil-no-internal-dependencies
abseil-no-namespace
Index: clang-tools-extra/trunk/docs/ReleaseNotes.rst
===
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst
@@ -93,6 +93,12 @@
   Checks for cases where arguments to ``absl::Duration`` factory functions are
   scaled internally and could be changed to a different factory function.
 
+- New :doc:`abseil-duration-subtraction
+  ` check.
+
+  Checks for cases where subtraction should be performed in the
+  ``absl::Duration`` domain.
+
 - New :doc:`abseil-faster-strsplit-delimiter
   ` check.
 
Index: clang-tools-extra/trunk/test/clang-tidy/abseil-duration-subtraction.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/abseil-duration-subtraction.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/abseil-duration-subtraction.cpp
@@ -0,0 +1,64 @@
+// RUN: %check_clang_tidy %s abseil-duration-subtraction %t -- -- -I %S/Inputs
+
+#include "absl/time/time.h"
+
+void f() {
+  double x;
+  absl::Duration d, d1, d2;
+
+  x = absl::ToDoubleSeconds(d) - 1.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleSeconds(d - absl::Seconds(1))
+  x = absl::ToDoubleSeconds(d) - absl::ToDoubleSeconds(d1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleSeconds(d - d1);
+  x = absl::ToDoubleSeconds(d) - 6.5 - 8.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: per