[libcxx] r328477 - Fix test case initialization issues in permissions test

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Mon Mar 26 00:06:25 2018
New Revision: 328477

URL: http://llvm.org/viewvc/llvm-project?rev=328477&view=rev
Log:
Fix test case initialization issues in permissions test

Modified:

libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.permissions/permissions.pass.cpp

Modified: 
libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.permissions/permissions.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.permissions/permissions.pass.cpp?rev=328477&r1=328476&r2=328477&view=diff
==
--- 
libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.permissions/permissions.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.permissions/permissions.pass.cpp
 Mon Mar 26 00:06:25 2018
@@ -95,7 +95,10 @@ TEST_CASE(basic_permissions_test)
   path p;
   perms set_perms;
   perms expected;
-  perm_options opts = perm_options::replace;
+  perm_options opts;
+  TestCase(path xp, perms xperms, perms xexpect,
+   perm_options xopts = perm_options::replace)
+  : p(xp), set_perms(xperms), expected(xexpect), opts(xopts) {}
 } cases[] = {
 // test file
 {file, perms::none, perms::none},
@@ -147,6 +150,9 @@ TEST_CASE(test_no_resolve_symlink_on_sym
 perms set_perms;
 perms expected; // only expected on platform that support symlink 
perms.
 perm_options opts = perm_options::replace;
+TestCase(perms xperms, perms xexpect,
+   perm_options xopts = perm_options::replace)
+  : set_perms(xperms), expected(xexpect), opts(xopts) {}
 } cases[] = {
 {perms::owner_all, perms::owner_all},
 {perms::group_all, perms::owner_all | perms::group_all, 
perm_options::add},


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

[PATCH] D44883: [Sema] Extend -Wself-assign with -Wself-assign-overloaded to warn on overloaded self-assignment (classes)

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I'm not sure you really need to put these in their own warning sub-group just 
because they're user-defined operators.  That's especially true because it 
appears we already have divisions in the warning group based on the form of the 
l-value; we don't want this to go combinatorial.




Comment at: lib/Sema/SemaExpr.cpp:12087
+  case BO_AndAssign:
+  case BO_OrAssign:
+DiagnoseSelfAssignment(S, LHS, RHS, OpLoc, /*IsBuiltin=*/false);

Quuxplusone wrote:
> I understand why `x &= x` and `x |= x` are mathematically special for the 
> built-in types, but surely `x -= x` and `x ^= x` and `x /= x` are just as 
> likely to indicate programmer error. I would be happy if Clang either took 
> the philosophical stance "We will diagnose `x = x` but uniformly //never// `x 
> op= x`," or else took the pragmatic stance "We will diagnose any `x op= x` or 
> `x op x` that seems likely to be a programming bug." This "middle way" of 
> warning only for `&=` and `|=` is bothersome to me.
I think "we want to diagnose anything that seems likely to be a programming 
bug" is already our policy here.  It's inevitable that we'll overlook examples 
of that.  I agree that we should apply this warning to at least -=, ^=, and /=.


Repository:
  rC Clang

https://reviews.llvm.org/D44883



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

[PATCH] D44826: Add -Wunused-using, a warning that finds unused using declarations.

[Carlos Alberto Enciso via Phabricator via cfe-commits]

CarlosAlbertoEnciso added a comment.

In https://reviews.llvm.org/D44826#1046910, @Eugene.Zelenko wrote:

> This duplicates Clang-tidy misc-unused-using-decls 
> . 
> If Clang will provide same or better functionality, it should be removed.


Thanks for bringing this up.


Repository:
  rC Clang

https://reviews.llvm.org/D44826



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

[PATCH] D44826: Add -Wunused-using, a warning that finds unused using declarations.

[Carlos Alberto Enciso via Phabricator via cfe-commits]

CarlosAlbertoEnciso added a comment.

In https://reviews.llvm.org/D44826#1046913, @Eugene.Zelenko wrote:

> Please also mention new warning in Release Notes and documentation.


I will do it.

> Will this warning be part of -Wall or -Wextra?

The warning is part of -Wall

Thanks for your feedback.


Repository:
  rC Clang

https://reviews.llvm.org/D44826



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

[PATCH] D43967: [ASTImporter] Add test helper Fixture

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Hi Aleksei, 
I don't have commit permissions, could you please help and commit?


Repository:
  rC Clang

https://reviews.llvm.org/D43967



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

[PATCH] D44831: [clang-format] Refine ObjC guesser to handle child lines of child lines

[Daniel Jasper via Phabricator via cfe-commits]

djasper added inline comments.



Comment at: lib/Format/Format.cpp:1542
 };
-for (auto Line : AnnotatedLines) {
-  if (LineContainsObjCCode(*Line))
+llvm::DenseSet LinesToCheckSet;
+LinesToCheckSet.reserve(AnnotatedLines.size());

Wouldn't it be much easier to call this function recursively for Children 
instead of using the lambda as well as this additional set? Lines and their 
children should form a tree, I think, so you should never see the same line 
again during recursion.


Repository:
  rC Clang

https://reviews.llvm.org/D44831



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

[PATCH] D44272: [clangd] Support incremental document syncing

[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.




Comment at: unittests/clangd/DraftStoreTests.cpp:1
+//===-- DraftStoreTests.cpp - Clangd unit tests -*- C++ 
-*-===//
+//

NIT: s/Clangd unit tests/Draft store tests
Or remove the comment title altogether, the filename seems to provide the same 
information.




Comment at: unittests/clangd/DraftStoreTests.cpp:20
+
+using namespace llvm;
+

It seems this `using namespace` can be removed (at least there are lots of 
names qualified with `llvm::` throughout the file)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44272



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

[PATCH] D44764: [clangd] Use a header for common inclusions for tests

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In https://reviews.llvm.org/D44764#1046766, @sammccall wrote:

>


You raise a good point. If we can get away with using `operator <<` everywhere, 
it is definitely the best approach.
Then our rule of thumb is: never define `PrintTo` overload, define `operator 
<<` for a type instead, right?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44764



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

[PATCH] D44882: [clangd] Implementation of workspace/symbol request

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added reviewers: sammccall, ilya-biryukov.
ilya-biryukov added a subscriber: sammccall.
ilya-biryukov added a comment.

Adding @sammccall, he will definitely want to take a look at index-related 
changes.
On a high level, the approach seems just right.
A few questions immedieately that came to mind:

- Unconditionally storing much more symbols in the index might have subtle 
performance implications, especially for our internal use (the codebase is 
huge).  I bet that internally we wouldn't want to store the symbols not needed 
for completion, so we'll probably need a switch to disable storing them in the 
indexing implementation. But let's wait for Sam to take a look, he certainly 
has a better perspective on the issues there.
- Current `fuzzyFind` implementation can only match qualifiers strictly (i.e. 
st::vector won't match std::vector). Should we look into allowing fuzzy matches 
for this feature?  (not in this patch, rather in the long term).
- Have you considered also allowing `'.'` and `' '` (space) as separators in 
the request? Having additional separators doesn't really hurt complexity of the 
implementation, but allows to switch between tools for different languages 
easier.

E.g., `std.vector.push_back` and `std vector push_back` could produce the same 
matches as `std::vector::push_back`.




Comment at: clangd/ClangdServer.h:163
+  StringRef Query, const clangd::WorkspaceSymbolOptions &Opts,
+  UniqueFunction>)>
+  Callback);

NIT: use `Callback` typedef.



Comment at: clangd/WorkspaceSymbols.cpp:165
+[](const SymbolInformation &A, const SymbolInformation &B) {
+  return A.name < B.name;
+});

We have `FuzzyMatcher`, it can produce decent match scores and is already used 
in completion.
Any reason not to use it here?



Comment at: clangd/index/Index.h:141
   unsigned References = 0;
 
+  // Whether or not the symbol should be considered for completion.

NIT: remove empty lines to be consistent with the rest of the struct.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44882



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

[PATCH] D44893: [ASTMatchers] Add isAssignmentOperator matcher

[Peter Szecsi via Phabricator via cfe-commits]

szepet created this revision.
szepet added reviewers: aaron.ballman, alexfh.
Herald added subscribers: dkrupp, rnkovacs, klimek.

Adding a matcher for BinaryOperator and cxxOperatorCallExpr to be able to 
decide whether it is any kind of assignment operator or not. This would be 
useful since allows us to easily detect assignments via matchers for static 
analysis (Tidy, SA) purposes.


Repository:
  rC Clang

https://reviews.llvm.org/D44893

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2139,5 +2139,20 @@
   functionDecl(hasTrailingReturn(;
 }
 
+TEST(IsAssignmentOperator, Basic) {
+  StatementMatcher BinAsgmtOperator = binaryOperator(isAssignmentOperator());
+  StatementMatcher CXXAsgmtOperator =
+  cxxOperatorCallExpr(isAssignmentOperator());
+
+  EXPECT_TRUE(matches("void x() { int a; a += 1; }", BinAsgmtOperator));
+  EXPECT_TRUE(matches("void x() { int a; a = 2; }", BinAsgmtOperator));
+  EXPECT_TRUE(matches("void x() { int a; a &= 3; }", BinAsgmtOperator));
+  EXPECT_TRUE(matches("struct S { S& operator=(const S&); };"
+  "void x() { S s1, s2; s1 = s2; }",
+  CXXAsgmtOperator));
+  EXPECT_TRUE(
+  notMatches("void x() { int a; if(a == 0) return; }", BinAsgmtOperator));
+}
+
 } // namespace ast_matchers
 } // namespace clang
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -322,6 +322,7 @@
   REGISTER_MATCHER(isAnyPointer);
   REGISTER_MATCHER(isArray);
   REGISTER_MATCHER(isArrow);
+  REGISTER_MATCHER(isAssignmentOperator);
   REGISTER_MATCHER(isBaseInitializer);
   REGISTER_MATCHER(isBitField);
   REGISTER_MATCHER(isCatchAll);
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -4003,6 +4003,26 @@
   return Name == Node.getOpcodeStr(Node.getOpcode());
 }
 
+/// \brief Matches on all kinds of assignment operators.
+///
+/// Example 1: matches a += b (matcher = binaryOperator(isAssignmentOperator()))
+/// \code
+///   if (a == b)
+/// a += b;
+/// \endcode
+///
+/// Example 2: matches s1 = s2
+///(matcher = cxxOperatorCallExpr(isAssignmentOperator()))
+/// \code
+///   struct S { S& operator=(const S&); };
+///   void x() { S s1, s2; s1 = s2; })
+/// \endcode
+AST_POLYMORPHIC_MATCHER(isAssignmentOperator,
+AST_POLYMORPHIC_SUPPORTED_TYPES(BinaryOperator,
+CXXOperatorCallExpr)) {
+  return Node.isAssignmentOp();
+}
+
 /// \brief Matches the left hand side of binary operator expressions.
 ///
 /// Example matches a (matcher = binaryOperator(hasLHS()))
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -1926,6 +1926,19 @@
 
 
 
+MatcherBinaryOperator>isAssignmentOperator
+Matches all kinds of assignment operators.
+
+Example 1: matches a += b (matcher = binaryOperator(isAssignmentOperator()))
+  if (a == b)
+a += b;
+
+Example 2: matches s1 = s2 (matcher = cxxOperatorCallExpr(isAssignmentOperator()))
+  struct S { S& operator=(const S&); };
+  void x() { S s1, s2; s1 = s2; })
+
+
+
 MatcherCXXBoolLiteralExpr>equalsbool Value
 
 
@@ -2306,6 +2319,19 @@
 
 
 
+MatcherCXXOperatorCallExpr>isAssignmentOperator
+Matches all kinds of assignment operators.
+
+Example 1: matches a += b (matcher = binaryOperator(isAssignmentOperator()))
+  if (a == b)
+a += b;
+
+Example 2: matches s1 = s2 (matcher = cxxOperatorCallExpr(isAssignmentOperator()))
+  struct S { S& operator=(const S&); };
+  void x() { S s1, s2; s1 = s2; })
+
+
+
 MatcherCXXRecordDecl>hasDefinition
 Matches a class declaration that is defined.
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38921: [analyzer] LoopUnrolling: update the matched assignment operators

[Peter Szecsi via Phabricator via cfe-commits]

szepet marked 2 inline comments as done.
szepet added a comment.

In https://reviews.llvm.org/D38921#1037180, @alexfh wrote:

> Do you want to submit the matcher part separately?


Yepp, but added as a different patch since implemented it for cxxOperatorCall 
as well. (I guess Aaron's test request implied it and I find it useful as well.)
See: https://reviews.llvm.org/D44893


https://reviews.llvm.org/D38921



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

[PATCH] D44883: [Sema] Extend -Wself-assign with -Wself-assign-overloaded to warn on overloaded self-assignment (classes)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D44883#1048010, @rjmccall wrote:

> I'm not sure you really need to put these in their own warning sub-group just 
> because they're user-defined operators.  That's especially true because it 
> appears we already have divisions in the warning group based on the form of 
> the l-value; we don't want this to go combinatorial.


Several reasons:

- The initial `-Wself-assign` was intentionally implemented not to warn on 
overloaded operators. 
https://github.com/llvm-mirror/clang/commit/9f7a6441bcbb1b17208cb3abd65a0017525a#diff-e0deb7b32f28507a3044a6bf9a63b515R31
 (https://reviews.llvm.org/rL122804)
- While it is an obvious bug when self-operation happens with builtin 
operators, i'm less certain of that with overloaded operators. If you happen to 
be routinely using self-assignment via oh-so-very-special overloaded operator=, 
and you don't like to have this diagnostic, you could just disable it, and not 
loose the coverage of the `-Wself-assign-builtin`. If it is all in one group, 
you can't do that...
- Based on previous expirience, separate diag groups are good, see e.g 
https://reviews.llvm.org/D37620, https://reviews.llvm.org/D37629
- I'm failing to find the original quote, but i **think** @rsmith said 
something along the "diag groups are cheap, use them". But i may as well be 
mis-remembering/having false memories here, sorry.

TLDR: if you insist, sure, i can just cram it into the already-existing 
`-Wself-assign`,
but i believe that is the opposite of what should be done, and is against the 
way it was done previously.




Comment at: lib/Sema/SemaExpr.cpp:12087
+  case BO_AndAssign:
+  case BO_OrAssign:
+DiagnoseSelfAssignment(S, LHS, RHS, OpLoc, /*IsBuiltin=*/false);

rjmccall wrote:
> Quuxplusone wrote:
> > I understand why `x &= x` and `x |= x` are mathematically special for the 
> > built-in types, but surely `x -= x` and `x ^= x` and `x /= x` are just as 
> > likely to indicate programmer error. I would be happy if Clang either took 
> > the philosophical stance "We will diagnose `x = x` but uniformly //never// 
> > `x op= x`," or else took the pragmatic stance "We will diagnose any `x op= 
> > x` or `x op x` that seems likely to be a programming bug." This "middle 
> > way" of warning only for `&=` and `|=` is bothersome to me.
> I think "we want to diagnose anything that seems likely to be a programming 
> bug" is already our policy here.  It's inevitable that we'll overlook 
> examples of that.  I agree that we should apply this warning to at least -=, 
> ^=, and /=.
Ok, will extend.


Repository:
  rC Clang

https://reviews.llvm.org/D44883



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

[PATCH] D44559: [Sema] Wrong width of result of mul operation

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

In https://reviews.llvm.org/D44559#1045758, @rjmccall wrote:

> No, I still oppose this patch.


OK, we have 2 possibilities here (fmpov):

1. Forget about the issue and don't do anything now - it is not a bug
2. Return the width based on real analyze of mul args:
  - signed vs. unsigned
  - chars, shorts, etc.

What do you suggest to do? Or maybe there are other variants?


https://reviews.llvm.org/D44559



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

[PATCH] D38921: [analyzer] LoopUnrolling: update the matched assignment operators

[Peter Szecsi via Phabricator via cfe-commits]

szepet updated this revision to Diff 139776.
szepet added a comment.

Added the assignment operator matcher patch as a dependency and rebased on top 
of that.


https://reviews.llvm.org/D38921

Files:
  lib/StaticAnalyzer/Core/LoopUnrolling.cpp
  test/Analysis/loop-unrolling.cpp

Index: test/Analysis/loop-unrolling.cpp
===
--- test/Analysis/loop-unrolling.cpp
+++ test/Analysis/loop-unrolling.cpp
@@ -99,13 +99,101 @@
   return 0;
 }
 
+int no_unroll_assignment() {
+  for (int i = 0; i < 9; i++) {
+i = i + 1;
+clang_analyzer_numTimesReached(); // expected-warning {{4}}
+  }
+  return 0;
+}
+
+int no_unroll_assignment2() {
+  for (int i = 0; i < 9; i++) {
+i *= 2;
+clang_analyzer_numTimesReached(); // expected-warning {{4}}
+  }
+  return 0;
+}
+
+int no_unroll_assignment3() {
+  for (int i = 128; i > 0; i--) {
+i /= 2;
+clang_analyzer_numTimesReached(); // expected-warning {{4}}
+  }
+  return 0;
+}
+
+int no_unroll_assignment4() {
+  for (int i = 0; i < 9; i++) {
+i -= 2;
+clang_analyzer_numTimesReached(); // expected-warning {{4}}
+  }
+  return 0;
+}
+
+int no_unroll_assignment5() {
+  for (int i = 0; i < 9; i++) {
+i += 1;
+clang_analyzer_numTimesReached(); // expected-warning {{4}}
+  }
+  return 0;
+}
+
+int no_unroll_assignment6() {
+  for (int i = 128; i > 0; i--) {
+i >>= 1;
+clang_analyzer_numTimesReached(); // expected-warning {{4}}
+  }
+  return 0;
+}
+
+int no_unroll_assignment7() {
+  for (int i = 0; i < 512; i++) {
+i <<= 1;
+clang_analyzer_numTimesReached(); // expected-warning {{4}}
+  }
+  return 0;
+}
+
+int no_unroll_assignment8() {
+  for (int i = 0; i < 9; i++) {
+i %= 8;
+clang_analyzer_numTimesReached(); // expected-warning {{4}}
+  }
+  return 0;
+}
+
+int no_unroll_assignment9() {
+  for (int i = 0; i < 9; i++) {
+i &= 31;
+clang_analyzer_numTimesReached(); // expected-warning {{4}}
+  }
+  return 0;
+}
+
+int no_unroll_assignment10() {
+  for (int i = 0; i < 9; i++) {
+i |= 2;
+clang_analyzer_numTimesReached(); // expected-warning {{4}}
+  }
+  return 0;
+}
+
+int no_unroll_assignment11() {
+  for (int i = 0; i < 9; i++) {
+i ^= 2;
+clang_analyzer_numTimesReached(); // expected-warning {{4}}
+  }
+  return 0;
+}
+
 int make_new_branches_loop_cached() {
   for (int i = 0; i < 8; i++) {
 clang_analyzer_numTimesReached(); // expected-warning {{4}}
-if(getNum()){
-(void) i; // Since this Stmt does not change the State the analyzer
-  // won't make a new execution path but reuse the earlier nodes.
-  }
+if (getNum()) {
+  (void)i; // Since this Stmt does not change the State the analyzer
+   // won't make a new execution path but reuse the earlier nodes.
+}
   }
   clang_analyzer_warnIfReached(); // no-warning
   return 0;
@@ -115,7 +203,7 @@
   int l = 2;
   for (int i = 0; i < 8; i++) {
 clang_analyzer_numTimesReached(); // expected-warning {{10}}
-if(getNum()){
+if (getNum()) {
   ++l;
 }
   }
@@ -127,7 +215,7 @@
   int l = 2;
   for (int i = 0; i < 8; i++) {
 clang_analyzer_numTimesReached(); // expected-warning {{10}}
-if(getNum()){
+if (getNum()) {
   ++l;
 }
 (void)&i; // This ensures that the loop won't be unrolled.
@@ -185,7 +273,7 @@
 for (j = 0; j < 9; ++j) {
   clang_analyzer_numTimesReached(); // expected-warning {{4}}
   a[j] = 22;
-  (void) &j; // ensures that the inner loop won't be unrolled
+  (void)&j; // ensures that the inner loop won't be unrolled
 }
 a[i] = 42;
   }
@@ -268,8 +356,8 @@
   int k = 2;
   for (int i = 0; i < 5; i++) {
 clang_analyzer_numTimesReached(); // expected-warning {{13}}
-if(i == 0 && b) // Splits the state in the first iteration but the recursion
-// call will be unrolled anyway since the condition is known there.
+if (i == 0 && b)  // Splits the state in the first iteration but the recursion
+  // call will be unrolled anyway since the condition is known there.
   recursion_unroll1(false);
 clang_analyzer_numTimesReached(); // expected-warning {{14}}
   }
@@ -281,7 +369,7 @@
   int k = 0;
   for (int i = 0; i < 5; i++) {
 clang_analyzer_numTimesReached(); // expected-warning {{9}}
-if(i == 0 && b)
+if (i == 0 && b)
   recursion_unroll2(false);
 clang_analyzer_numTimesReached(); // expected-warning {{9}}
   }
@@ -307,7 +395,7 @@
   int k = 2;
   for (int i = 0; i < 5; i++) {
 clang_analyzer_numTimesReached(); // expected-warning {{13}}
-if(i == 0 && b) {
+if (i == 0 && b) {
   recursion_unroll4(false);
   continue;
 }
Index: lib/StaticAnalyzer/Core/LoopUnrolling.cpp
===
--- lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -97

[PATCH] D43821: [SemaCXX] _Pragma("clang optimize off") not affecting lambda.

[Carlos Alberto Enciso via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL328494: [SemaCXX] _Pragma("clang optimize off") 
not affecting lambda. (authored by CarlosAlbertoEnciso, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D43821?vs=137550&id=139783#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D43821

Files:
  cfe/trunk/lib/Sema/SemaLambda.cpp
  cfe/trunk/test/CodeGenCXX/optnone-pragma-optimize-off.cpp


Index: cfe/trunk/test/CodeGenCXX/optnone-pragma-optimize-off.cpp
===
--- cfe/trunk/test/CodeGenCXX/optnone-pragma-optimize-off.cpp
+++ cfe/trunk/test/CodeGenCXX/optnone-pragma-optimize-off.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 %s -triple %itanium_abi_triple -O1 -disable-llvm-passes 
-emit-llvm -o - | FileCheck %s
+
+// Test the attributes for the lambda function contains 'optnone' as result of
+// the _Pragma("clang optimize off").
+
+_Pragma("clang optimize off")
+
+void foo(int p) {
+  auto lambda = [&p]() { ++p; };
+  lambda();
+  // CHECK: define {{.*}} @"_ZZ3fooiENK3$_0clEv"({{.*}}) #[[LAMBDA_ATR:[0-9]+]]
+}
+
+_Pragma("clang optimize on")
+
+// CHECK: attributes #[[LAMBDA_ATR]] = { {{.*}} optnone {{.*}} }
\ No newline at end of file
Index: cfe/trunk/lib/Sema/SemaLambda.cpp
===
--- cfe/trunk/lib/Sema/SemaLambda.cpp
+++ cfe/trunk/lib/Sema/SemaLambda.cpp
@@ -904,6 +904,10 @@
 ParamInfo.getDeclSpec().isConstexprSpecified());
   if (ExplicitParams)
 CheckCXXDefaultArguments(Method);
+
+  // This represents the function body for the lambda function, check if we
+  // have to apply optnone due to a pragma.
+  AddRangeBasedOptnone(Method);
   
   // Attributes on the lambda apply to the method.  
   ProcessDeclAttributes(CurScope, Method, ParamInfo);


Index: cfe/trunk/test/CodeGenCXX/optnone-pragma-optimize-off.cpp
===
--- cfe/trunk/test/CodeGenCXX/optnone-pragma-optimize-off.cpp
+++ cfe/trunk/test/CodeGenCXX/optnone-pragma-optimize-off.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 %s -triple %itanium_abi_triple -O1 -disable-llvm-passes -emit-llvm -o - | FileCheck %s
+
+// Test the attributes for the lambda function contains 'optnone' as result of
+// the _Pragma("clang optimize off").
+
+_Pragma("clang optimize off")
+
+void foo(int p) {
+  auto lambda = [&p]() { ++p; };
+  lambda();
+  // CHECK: define {{.*}} @"_ZZ3fooiENK3$_0clEv"({{.*}}) #[[LAMBDA_ATR:[0-9]+]]
+}
+
+_Pragma("clang optimize on")
+
+// CHECK: attributes #[[LAMBDA_ATR]] = { {{.*}} optnone {{.*}} }
\ No newline at end of file
Index: cfe/trunk/lib/Sema/SemaLambda.cpp
===
--- cfe/trunk/lib/Sema/SemaLambda.cpp
+++ cfe/trunk/lib/Sema/SemaLambda.cpp
@@ -904,6 +904,10 @@
 ParamInfo.getDeclSpec().isConstexprSpecified());
   if (ExplicitParams)
 CheckCXXDefaultArguments(Method);
+
+  // This represents the function body for the lambda function, check if we
+  // have to apply optnone due to a pragma.
+  AddRangeBasedOptnone(Method);
   
   // Attributes on the lambda apply to the method.  
   ProcessDeclAttributes(CurScope, Method, ParamInfo);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44787: Migrate dockerfiles to use multi-stage builds.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.



Comment at: docs/Docker.rst:64
 If you want to write your own docker image, start with an ``example/`` 
subfolder.
-It provides incomplete Dockerfiles with (very few) FIXMEs explaining the steps
+It provides incomplete Dockerfile with (very few) FIXMEs explaining the steps
 you need to take in order to make your Dockerfiles functional.

an incomplete dockerfile


Repository:
  rL LLVM

https://reviews.llvm.org/D44787



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

r328495 - [clang-format] Wildcard expansion on Windows.

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Mon Mar 26 06:54:17 2018
New Revision: 328495

URL: http://llvm.org/viewvc/llvm-project?rev=328495&view=rev
Log:
[clang-format] Wildcard expansion on Windows.

Summary:
Add support for wildcard expansion in command line arguments on Windows.
See https://docs.microsoft.com/en-us/cpp/c-language/expanding-wildcard-arguments

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

Reviewers: klimek, djasper, rnk

Reviewed By: rnk

Subscribers: rnk, smeenai, zturner, alexfh, mgorny, cfe-commits

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

Modified:
cfe/trunk/tools/clang-format/ClangFormat.cpp

Modified: cfe/trunk/tools/clang-format/ClangFormat.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/ClangFormat.cpp?rev=328495&r1=328494&r2=328495&view=diff
==
--- cfe/trunk/tools/clang-format/ClangFormat.cpp (original)
+++ cfe/trunk/tools/clang-format/ClangFormat.cpp Mon Mar 26 06:54:17 2018
@@ -22,6 +22,7 @@
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 
 using namespace llvm;
@@ -339,11 +340,19 @@ static void PrintVersion(raw_ostream &OS
 int main(int argc, const char **argv) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
 
+  SmallVector Args;
+  llvm::SpecificBumpPtrAllocator ArgAllocator;
+  std::error_code EC = llvm::sys::Process::GetArgumentVector(
+  Args, llvm::makeArrayRef(argv, argc), ArgAllocator);
+  if (EC) {
+llvm::errs() << "error: couldn't get arguments: " << EC.message() << '\n';
+  }
+
   cl::HideUnrelatedOptions(ClangFormatCategory);
 
   cl::SetVersionPrinter(PrintVersion);
   cl::ParseCommandLineOptions(
-  argc, argv,
+  Args.size(), &Args[0],
   "A tool to format C/C++/Java/JavaScript/Objective-C/Protobuf code.\n\n"
   "If no arguments are specified, it formats the code from standard 
input\n"
   "and writes the result to the standard output.\n"


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

[PATCH] D44778: [clang-format] Wildcard expansion on Windows.

[Alexander Kornienko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC328495: [clang-format] Wildcard expansion on Windows. 
(authored by alexfh, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D44778?vs=139593&id=139784#toc

Repository:
  rC Clang

https://reviews.llvm.org/D44778

Files:
  tools/clang-format/ClangFormat.cpp


Index: tools/clang-format/ClangFormat.cpp
===
--- tools/clang-format/ClangFormat.cpp
+++ tools/clang-format/ClangFormat.cpp
@@ -22,6 +22,7 @@
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 
 using namespace llvm;
@@ -339,11 +340,19 @@
 int main(int argc, const char **argv) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
 
+  SmallVector Args;
+  llvm::SpecificBumpPtrAllocator ArgAllocator;
+  std::error_code EC = llvm::sys::Process::GetArgumentVector(
+  Args, llvm::makeArrayRef(argv, argc), ArgAllocator);
+  if (EC) {
+llvm::errs() << "error: couldn't get arguments: " << EC.message() << '\n';
+  }
+
   cl::HideUnrelatedOptions(ClangFormatCategory);
 
   cl::SetVersionPrinter(PrintVersion);
   cl::ParseCommandLineOptions(
-  argc, argv,
+  Args.size(), &Args[0],
   "A tool to format C/C++/Java/JavaScript/Objective-C/Protobuf code.\n\n"
   "If no arguments are specified, it formats the code from standard 
input\n"
   "and writes the result to the standard output.\n"


Index: tools/clang-format/ClangFormat.cpp
===
--- tools/clang-format/ClangFormat.cpp
+++ tools/clang-format/ClangFormat.cpp
@@ -22,6 +22,7 @@
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 
 using namespace llvm;
@@ -339,11 +340,19 @@
 int main(int argc, const char **argv) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
 
+  SmallVector Args;
+  llvm::SpecificBumpPtrAllocator ArgAllocator;
+  std::error_code EC = llvm::sys::Process::GetArgumentVector(
+  Args, llvm::makeArrayRef(argv, argc), ArgAllocator);
+  if (EC) {
+llvm::errs() << "error: couldn't get arguments: " << EC.message() << '\n';
+  }
+
   cl::HideUnrelatedOptions(ClangFormatCategory);
 
   cl::SetVersionPrinter(PrintVersion);
   cl::ParseCommandLineOptions(
-  argc, argv,
+  Args.size(), &Args[0],
   "A tool to format C/C++/Java/JavaScript/Objective-C/Protobuf code.\n\n"
   "If no arguments are specified, it formats the code from standard input\n"
   "and writes the result to the standard output.\n"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44882: [clangd] Implementation of workspace/symbol request

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added a comment.

In https://reviews.llvm.org/D44882#1048043, @ilya-biryukov wrote:

> - Unconditionally storing much more symbols in the index might have subtle 
> performance implications, especially for our internal use (the codebase is 
> huge).  I bet that internally we wouldn't want to store the symbols not 
> needed for completion, so we'll probably need a switch to disable storing 
> them in the indexing implementation. But let's wait for Sam to take a look, 
> he certainly has a better perspective on the issues there.


I'm a bit surprised that internally you do not want symbols beyond the ones for 
completion. We have a lot more features in mind that will make the index much 
bigger, like adding all occurrences (maybe not in the current YAML though). But 
having options to control the amount of information indexed sounds good to me 
as there can be a lot of different project sizes and there can be different 
tradeoffs. I had in mind to an option for including/excluding the occurrences 
because without them, you lose workspace/references, call hierarchy, etc, but 
you still have code completion and workspace/symbol, document/symbol, etc while 
making the index much smaller. But more options sounds good.

> - Current `fuzzyFind` implementation can only match qualifiers strictly (i.e. 
> st::vector won't match std::vector). Should we look into allowing fuzzy 
> matches for this feature?  (not in this patch, rather in the long term).

I'm not sure, I'm thinking there should be a balance between how fuzzy it it 
and how much noise it generates. Right now I find it a bit too fuzzy since when 
I type "Draft" (to find DraftMgr), it picks up things like 
DocumentRangeFormattingParams. Adding fuzziness to the namespace would make 
this worse. Maybe with improved scoring it won't matter too much? I'll try 
FuzzyMatcher and see.

> - Have you considered also allowing `'.'` and `' '` (space) as separators in 
> the request? Having additional separators doesn't really hurt complexity of 
> the implementation, but allows to switch between tools for different 
> languages easier. E.g., `std.vector.push_back` and `std vector push_back` 
> could produce the same matches as `std::vector::push_back`.

I haven't thought of that and I think it's a good idea. Sounds like a good, 
isolated thing to do in a separate patch.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44882



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

[PATCH] D44882: [clangd] Implementation of workspace/symbol request

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added inline comments.



Comment at: clangd/WorkspaceSymbols.cpp:165
+[](const SymbolInformation &A, const SymbolInformation &B) {
+  return A.name < B.name;
+});

ilya-biryukov wrote:
> We have `FuzzyMatcher`, it can produce decent match scores and is already 
> used in completion.
> Any reason not to use it here?
I think it was giving odd ordering but let me try this again and at least 
document the reason.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44882



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

[PATCH] D44774: [Driver] Allow use of -fsyntax-only together with -MJ

[David Stenberg via Phabricator via cfe-commits]

dstenb added a comment.

Downstream we use -MJ in a bit of an idiosyncratic way, as we're in a 
transition period where we, for a subset of the code base, only use the clang 
frontend for diagnostics, and not for the code generation. However, if you 
don't think that using -fsyntax-only and -MJ makes sense in any upstream 
application, I'll drop from this change. I'm leaving the assertion as-is.


Repository:
  rC Clang

https://reviews.llvm.org/D44774



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

[PATCH] D44883: [Sema] Extend -Wself-assign with -Wself-assign-overloaded to warn on overloaded self-assignment (classes)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 139788.
lebedev.ri marked 3 inline comments as done.
lebedev.ri added a comment.

- Also warn on `BO_DivAssign`, `BO_SubAssign`, `BO_XorAssign`.
- Add releasenotes entry.


Repository:
  rC Clang

https://reviews.llvm.org/D44883

Files:
  docs/ReleaseNotes.rst
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  test/SemaCXX/warn-self-assign-builtin.cpp
  test/SemaCXX/warn-self-assign-overloaded.cpp
  test/SemaCXX/warn-self-assign.cpp

Index: test/SemaCXX/warn-self-assign-overloaded.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-self-assign-overloaded.cpp
@@ -0,0 +1,88 @@
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -Wno-self-assign-builtin -DDUMMY -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-overloaded -DDUMMY -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -Wno-self-assign-builtin -DV0 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-overloaded -DV0 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -Wno-self-assign-builtin -DV1 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-overloaded -DV1 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -Wno-self-assign-builtin -DV2 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-overloaded -DV2 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -Wno-self-assign-builtin -DV3 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-overloaded -DV3 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -Wno-self-assign-builtin -DV4 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-overloaded -DV4 -verify %s
+
+#ifdef DUMMY
+struct S {};
+#else
+struct S {
+#if defined(V0)
+  S() = default;
+#elif defined(V1)
+  S &operator=(const S &) = default;
+#elif defined(V2)
+  S &operator=(S &) = default;
+#elif defined(V3)
+  S &operator=(const S &);
+#elif defined(V4)
+  S &operator=(S &);
+#else
+#error Define something!
+#endif
+  S &operator&=(const S &);
+  S &operator|=(const S &);
+  S &operator^=(const S &);
+  S &operator-=(const S &);
+  S &operator/=(const S &);
+  S &operator=(const volatile S &) volatile;
+};
+#endif
+
+void f() {
+  S a, b;
+  a = a; // expected-warning{{explicitly assigning}}
+  b = b; // expected-warning{{explicitly assigning}}
+  a = b;
+  b = a = b;
+  a = a = a; // expected-warning{{explicitly assigning}}
+  a = b = b = a;
+#ifndef DUMMY
+  a &= a; // expected-warning{{explicitly assigning}}
+  a |= a; // expected-warning{{explicitly assigning}}
+  a ^= a; // expected-warning{{explicitly assigning}}
+  a -= a; // expected-warning{{explicitly assigning}}
+  a /= a; // expected-warning{{explicitly assigning}}
+#endif
+}
+
+void false_positives() {
+#define OP =
+#define LHS a
+#define RHS a
+  S a;
+  // These shouldn't warn due to the use of the preprocessor.
+  a OP a;
+  LHS = a;
+  a = RHS;
+  LHS OP RHS;
+#undef OP
+#undef LHS
+#undef RHS
+
+#ifndef DUMMY
+  // Volatile stores aren't side-effect free.
+  volatile S vol_a;
+  vol_a = vol_a;
+  volatile S &vol_a_ref = vol_a;
+  vol_a_ref = vol_a_ref;
+#endif
+}
+
+template 
+void g() {
+  T a;
+  a = a; // expected-warning{{explicitly assigning}}
+}
+void instantiate() {
+  g();
+  g();
+}
Index: test/SemaCXX/warn-self-assign-builtin.cpp
===
--- test/SemaCXX/warn-self-assign-builtin.cpp
+++ test/SemaCXX/warn-self-assign-builtin.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -Wself-assign -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -Wno-self-assign-overloaded -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-builtin -verify %s
 
 void f() {
   int a = 42, b = 42;
@@ -40,7 +41,8 @@
   vol_a_ref = vol_a_ref;
 }
 
-template  void g() {
+template 
+void g() {
   T a;
   a = a; // May or may not be a builtin assignment operator, no warning.
 }
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -10698,14 +10698,13 @@
 }
 
 static void CheckIdentityFieldAssignment(Expr *LHSExpr, Expr *RHSExpr,
- SourceLocation Loc,
- Sema &Sema) {
+ SourceLocation Loc, Sema &Sema) {
   // C / C++ fields
   MemberExpr *ML = dyn_cast(LHSExpr);
   MemberExpr *MR = dyn_cast(RHSExpr);
   if (ML && MR && ML->getMemberDecl() == MR->getMemberDecl()) {
 if (isa(ML->getBase()) && isa(MR->getBase()))
-  Sema.Diag(Loc, diag::warn_identity_field_assign) << 0;
+  Sema.Diag(Loc, diag::warn_identity_field_assign_builtin) << 0;
   }
 
   // Objective-C instance variables
@@ -10715,7 +10714,7 @@
 DeclRefExpr *RL = dyn_cast(OL->getBase()->IgnoreImpCasts());
 DeclRefExpr *RR = dyn_cast(OR->getBase()->IgnoreImpCasts());
 if (RL && RR && RL->getDe

[PATCH] D44774: [Driver] Allow use of -fsyntax-only together with -MJ

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Oh, we certainly should never be hitting an assertion on front-end flags. As 
such, there is a problem to fix here. I still maintain that the combination of 
flags is non-sense, so the question is:

(1) Should be silently ignore -MJ? That would be useful for your use case, but 
seems to violate POLA.

(2) Should be error out explicitly when no output is known? This is more harsh, 
but seems to provide more consistent behavior for -MJ.


Repository:
  rC Clang

https://reviews.llvm.org/D44774



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

[PATCH] D44272: [clangd] Support incremental document syncing

[Simon Marchi via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL328500: [clangd] Support incremental document syncing 
(authored by simark, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D44272?vs=139596&id=139789#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D44272

Files:
  clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
  clang-tools-extra/trunk/clangd/DraftStore.cpp
  clang-tools-extra/trunk/clangd/DraftStore.h
  clang-tools-extra/trunk/clangd/Protocol.cpp
  clang-tools-extra/trunk/clangd/Protocol.h
  clang-tools-extra/trunk/test/clangd/initialize-params-invalid.test
  clang-tools-extra/trunk/test/clangd/initialize-params.test
  clang-tools-extra/trunk/test/clangd/textdocument-didchange-fail.test
  clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
  clang-tools-extra/trunk/unittests/clangd/DraftStoreTests.cpp

Index: clang-tools-extra/trunk/clangd/Protocol.cpp
===
--- clang-tools-extra/trunk/clangd/Protocol.cpp
+++ clang-tools-extra/trunk/clangd/Protocol.cpp
@@ -248,7 +248,8 @@
 
 bool fromJSON(const json::Expr &Params, TextDocumentContentChangeEvent &R) {
   json::ObjectMapper O(Params);
-  return O && O.map("text", R.text);
+  return O && O.map("range", R.range) && O.map("rangeLength", R.rangeLength) &&
+ O.map("text", R.text);
 }
 
 bool fromJSON(const json::Expr &Params, FormattingOptions &R) {
Index: clang-tools-extra/trunk/clangd/DraftStore.cpp
===
--- clang-tools-extra/trunk/clangd/DraftStore.cpp
+++ clang-tools-extra/trunk/clangd/DraftStore.cpp
@@ -8,6 +8,8 @@
 //===--===//
 
 #include "DraftStore.h"
+#include "SourceCode.h"
+#include "llvm/Support/Errc.h"
 
 using namespace clang;
 using namespace clang::clangd;
@@ -32,12 +34,72 @@
   return ResultVector;
 }
 
-void DraftStore::updateDraft(PathRef File, StringRef Contents) {
+void DraftStore::addDraft(PathRef File, StringRef Contents) {
   std::lock_guard Lock(Mutex);
 
   Drafts[File] = Contents;
 }
 
+llvm::Expected DraftStore::updateDraft(
+PathRef File, llvm::ArrayRef Changes) {
+  std::lock_guard Lock(Mutex);
+
+  auto EntryIt = Drafts.find(File);
+  if (EntryIt == Drafts.end()) {
+return llvm::make_error(
+"Trying to do incremental update on non-added document: " + File,
+llvm::errc::invalid_argument);
+  }
+
+  std::string Contents = EntryIt->second;
+
+  for (const TextDocumentContentChangeEvent &Change : Changes) {
+if (!Change.range) {
+  Contents = Change.text;
+  continue;
+}
+
+const Position &Start = Change.range->start;
+llvm::Expected StartIndex =
+positionToOffset(Contents, Start, false);
+if (!StartIndex)
+  return StartIndex.takeError();
+
+const Position &End = Change.range->end;
+llvm::Expected EndIndex = positionToOffset(Contents, End, false);
+if (!EndIndex)
+  return EndIndex.takeError();
+
+if (*EndIndex < *StartIndex)
+  return llvm::make_error(
+  llvm::formatv(
+  "Range's end position ({0}) is before start position ({1})", End,
+  Start),
+  llvm::errc::invalid_argument);
+
+if (Change.rangeLength &&
+(ssize_t)(*EndIndex - *StartIndex) != *Change.rangeLength)
+  return llvm::make_error(
+  llvm::formatv("Change's rangeLength ({0}) doesn't match the "
+"computed range length ({1}).",
+*Change.rangeLength, *EndIndex - *StartIndex),
+  llvm::errc::invalid_argument);
+
+std::string NewContents;
+NewContents.reserve(*StartIndex + Change.text.length() +
+(Contents.length() - *EndIndex));
+
+NewContents = Contents.substr(0, *StartIndex);
+NewContents += Change.text;
+NewContents += Contents.substr(*EndIndex);
+
+Contents = std::move(NewContents);
+  }
+
+  EntryIt->second = Contents;
+  return Contents;
+}
+
 void DraftStore::removeDraft(PathRef File) {
   std::lock_guard Lock(Mutex);
 
Index: clang-tools-extra/trunk/clangd/Protocol.h
===
--- clang-tools-extra/trunk/clangd/Protocol.h
+++ clang-tools-extra/trunk/clangd/Protocol.h
@@ -197,6 +197,20 @@
 using ShutdownParams = NoParams;
 using ExitParams = NoParams;
 
+/// Defines how the host (editor) should sync document changes to the language
+/// server.
+enum class TextDocumentSyncKind {
+  /// Documents should not be synced at all.
+  None = 0,
+
+  /// Documents are synced by always sending the full content of the document.
+  Full = 1,
+
+  /// Documents are synced by sending the full content on open.  After that
+  /// only incremental updates to the document are send.
+  Incremental = 2,
+};
+
 struct CompletionItemClientCapabilities {
   /// Cl

[PATCH] D44231: [clang-tidy] Check for sizeof that call functions

[Paul Fultz II via Phabricator via cfe-commits]

pfultz2 added a comment.

So, I've added tests for class enums and bols.


https://reviews.llvm.org/D44231



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

[PATCH] D43967: [ASTImporter] Add test helper Fixture

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 139791.
martong added a comment.

- Fix gcc 5 warnings


Repository:
  rC Clang

https://reviews.llvm.org/D43967

Files:
  unittests/AST/ASTImporterTest.cpp
  unittests/AST/DeclMatcher.h

Index: unittests/AST/DeclMatcher.h
===
--- /dev/null
+++ unittests/AST/DeclMatcher.h
@@ -0,0 +1,68 @@
+//===- unittest/AST/DeclMatcher.h - AST unit test support ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_UNITTESTS_AST_DECLMATCHER_H
+#define LLVM_CLANG_UNITTESTS_AST_DECLMATCHER_H
+
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+namespace clang {
+namespace ast_matchers {
+
+enum class DeclMatcherKind { First, Last };
+
+// Matcher class to retrieve the first/last matched node under a given AST.
+template 
+class DeclMatcher : public MatchFinder::MatchCallback {
+  NodeType *Node = nullptr;
+  void run(const MatchFinder::MatchResult &Result) override {
+if ((MatcherKind == DeclMatcherKind::First && Node == nullptr) ||
+MatcherKind == DeclMatcherKind::Last) {
+  Node = const_cast(Result.Nodes.getNodeAs(""));
+}
+  }
+public:
+  // Returns the first/last matched node under the tree rooted in `D`.
+  template 
+  NodeType *match(const Decl *D, const MatcherType &AMatcher) {
+MatchFinder Finder;
+Finder.addMatcher(AMatcher.bind(""), this);
+Finder.matchAST(D->getASTContext());
+assert(Node);
+return Node;
+  }
+};
+template 
+using LastDeclMatcher = DeclMatcher;
+template 
+using FirstDeclMatcher = DeclMatcher;
+
+template 
+class DeclCounter : public MatchFinder::MatchCallback {
+  unsigned Count = 0;
+  void run(const MatchFinder::MatchResult &Result) override {
+  if(Result.Nodes.getNodeAs("")) {
+++Count;
+  }
+  }
+public:
+  // Returns the number of matched nodes under the tree rooted in `D`.
+  template 
+  unsigned match(const Decl *D, const MatcherType &AMatcher) {
+MatchFinder Finder;
+Finder.addMatcher(AMatcher.bind(""), this);
+Finder.matchAST(D->getASTContext());
+return Count;
+  }
+};
+
+} // end namespace ast_matchers
+} // end namespace clang
+
+#endif
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -17,6 +17,8 @@
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/Tooling.h"
+
+#include "DeclMatcher.h"
 #include "gtest/gtest.h"
 
 namespace clang {
@@ -29,7 +31,7 @@
   return Lang == Lang_CXX || Lang == Lang_CXX11;
 }
 
-static RunOptions getRunOptionsForLanguage(Language Lang) {
+static ArgVector getBasicRunOptionsForLanguage(Language Lang) {
   ArgVector BasicArgs;
   // Test with basic arguments.
   switch (Lang) {
@@ -49,6 +51,11 @@
   case Lang_OBJCXX:
 llvm_unreachable("Not implemented yet!");
   }
+  return BasicArgs;
+}
+
+static RunOptions getRunOptionsForLanguage(Language Lang) {
+  ArgVector BasicArgs = getBasicRunOptionsForLanguage(Lang);
 
   // For C++, test with "-fdelayed-template-parsing" enabled to handle MSVC
   // default behaviour.
@@ -61,6 +68,19 @@
   return {BasicArgs};
 }
 
+// Creates a virtual file and assigns that to the context of given AST. If the
+// file already exists then the file will not be created again as a duplicate.
+static void createVirtualFileIfNeeded(ASTUnit *ToAST, StringRef FileName,
+  const std::string &Code) {
+  assert(ToAST);
+  ASTContext &ToCtx = ToAST->getASTContext();
+  auto *OFS = static_cast(
+  ToCtx.getSourceManager().getFileManager().getVirtualFileSystem().get());
+  auto *MFS =
+  static_cast(OFS->overlays_begin()->get());
+  MFS->addFile(FileName, 0, llvm::MemoryBuffer::getMemBuffer(Code.c_str()));
+}
+
 template
 testing::AssertionResult
 testImport(const std::string &FromCode, const ArgVector &FromArgs,
@@ -79,11 +99,7 @@
 
   // Add input.cc to virtual file system so importer can 'find' it
   // while importing SourceLocations.
-  vfs::OverlayFileSystem *OFS = static_cast(
-ToCtx.getSourceManager().getFileManager().getVirtualFileSystem().get());
-  vfs::InMemoryFileSystem *MFS = static_cast(
-OFS->overlays_begin()->get());
-  MFS->addFile(InputFileName, 0, llvm::MemoryBuffer::getMemBuffer(FromCode));
+  createVirtualFileIfNeeded(ToAST.get(), InputFileName, FromCode);
 
   ASTImporter Importer(ToCtx, ToAST->getFileManager(),
FromCtx, FromAST->getFileManager(), false);
@@ -133,6 +149,165 @@
  Verifier, AMatcher));
 }
 
+const StringRef DeclToImportID = "declToImport";
+
+// This class provides gene

[PATCH] D44787: Migrate dockerfiles to use multi-stage builds.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL328503: Migrate dockerfiles to use multi-stage builds. 
(authored by ibiryukov, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D44787?vs=139457&id=139792#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D44787

Files:
  llvm/trunk/docs/Docker.rst
  llvm/trunk/utils/docker/build_docker_image.sh
  llvm/trunk/utils/docker/debian8/Dockerfile
  llvm/trunk/utils/docker/debian8/build/Dockerfile
  llvm/trunk/utils/docker/debian8/release/Dockerfile
  llvm/trunk/utils/docker/example/Dockerfile
  llvm/trunk/utils/docker/example/build/Dockerfile
  llvm/trunk/utils/docker/example/release/Dockerfile
  llvm/trunk/utils/docker/nvidia-cuda/Dockerfile
  llvm/trunk/utils/docker/nvidia-cuda/build/Dockerfile
  llvm/trunk/utils/docker/nvidia-cuda/release/Dockerfile
  llvm/trunk/utils/docker/scripts/build_install_llvm.sh

Index: llvm/trunk/docs/Docker.rst
===
--- llvm/trunk/docs/Docker.rst
+++ llvm/trunk/docs/Docker.rst
@@ -53,24 +53,15 @@
 LLVM components, compiled from sources. The sources are checked out from the
 upstream svn repository when building the image.
 
-Inside each subfolder we host Dockerfiles for two images:
+The resulting image contains only the requested LLVM components and a few extra
+packages to make the image minimally useful for C++ development, e.g. libstdc++
+and binutils.
 
-- ``build/`` image is used to compile LLVM, it installs a system compiler and all
-  build dependencies of LLVM. After the build process is finished, the build
-  image will have an archive with compiled components at ``/tmp/clang.tar.gz``.
-- ``release/`` image usually only contains LLVM components, compiled by the
-  ``build/`` image, and also libstdc++ and binutils to make image minimally
-  useful for C++ development. The assumption is that you usually want clang to
-  be one of the provided components.
-
-To build both of those images, use ``build_docker_image.sh`` script.
-It will checkout LLVM sources and build clang in the ``build`` container, copy results
-of the build to the local filesystem and then build the ``release`` container using
-those. The ``build_docker_image.sh`` accepts a list of LLVM repositories to
-checkout, and arguments for CMake invocation.
+The interface to run the build is ``build_docker_image.sh`` script. It accepts a
+list of LLVM repositories to checkout and arguments for CMake invocation.
 
 If you want to write your own docker image, start with an ``example/`` subfolder.
-It provides incomplete Dockerfiles with (very few) FIXMEs explaining the steps
+It provides an incomplete Dockerfile with (very few) FIXMEs explaining the steps
 you need to take in order to make your Dockerfiles functional.
 
 Usage
@@ -110,10 +101,10 @@
 	-DBOOTSTRAP_CMAKE_BUILD_TYPE=Release \
 	-DCLANG_ENABLE_BOOTSTRAP=ON -DCLANG_BOOTSTRAP_TARGETS="install-clang;install-clang-headers"
 	
-This will produce two images, a release image ``clang-debian8:staging`` and a
-build image ``clang-debian8-build:staging`` from the latest upstream revision.
-After the image is built you can run bash inside a container based on your
-image like this:
+This will produce a new image ``clang-debian8:staging`` from the latest
+upstream revision.
+After the image is built you can run bash inside a container based on your image
+like this:
 
 .. code-block:: bash
 
@@ -181,19 +172,14 @@
 
 Minimizing docker image size
 
-Due to Docker restrictions we use two images (i.e., build and release folders)
-for the release image to be as small as possible. It's much easier to achieve
-that using two images, because Docker would store a filesystem layer for each
-command in the  Dockerfile, i.e. if you install some packages in one command,
-then remove  those in a separate command, the size of the resulting image will
-still be proportinal to the size of an image with installed packages.
-Therefore, we strive to provide a very simple release image which only copies
-compiled clang and does not do anything else.
-
-Docker 1.13 added a ``--squash`` flag that allows to flatten the layers of the
-image, i.e. remove the parts that were actually deleted. That is an easier way
-to produce the smallest images possible by using just a single image. We do not
-use it because as of today the flag is in experimental stage and not everyone
-may have the latest docker version available. When the flag is out of
-experimental stage, we should investigate replacing two images approach with
-just a single image, built using ``--squash`` flag.
+Due to how Docker's filesystem works, all intermediate writes are persisted in
+the resulting image, even if they are removed in the following commands.
+To minimize the resulting image size we use `multi-stage Docker builds
+`_.
+Internally Docker builds two im

[PATCH] D41648: [clang-tidy] implement cppcoreguidelines macro rules

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 139793.
JonasToth added a comment.

- [Feature] implement CAPS_ONLY features, adjust docs


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41648

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
  clang-tidy/cppcoreguidelines/MacroUsageCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp
  test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
  test/clang-tidy/cppcoreguidelines-macro-usage.cpp

Index: test/clang-tidy/cppcoreguidelines-macro-usage.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-macro-usage.cpp
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage %t
+
+#ifndef INCLUDE_GUARD
+#define INCLUDE_GUARD
+
+#define PROBLEMATIC_CONSTANT 0
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro used to declare a constant; consider using a 'constexpr' constant
+
+#define PROBLEMATIC_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: function like macro used; consider a (constexpr) template function
+
+#define PROBLEMATIC_VARIADIC(...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a variadic template
+
+#endif
Index: test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: cppcoreguidelines-macro-usage.AllowedRegexp, value: "DEBUG_*|TEST_*"}]}' --
+
+#ifndef INCLUDE_GUARD
+#define INCLUDE_GUARD
+
+#define PROBLEMATIC_CONSTANT 0
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro used to declare a constant; consider using a 'constexpr' constant
+
+#define PROBLEMATIC_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: function like macro used; consider a (constexpr) template function
+
+#define PROBLEMATIC_VARIADIC(...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a variadic template
+
+#define DEBUG_CONSTANT 0
+#define DEBUG_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+#define DEBUG_VARIADIC(...) (__VA_ARGS__)
+#define TEST_CONSTANT 0
+#define TEST_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+#define TEST_VARIADIC(...) (__VA_ARGS__)
+
+#endif
Index: test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp
@@ -0,0 +1,17 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: cppcoreguidelines-macro-usage.CheckCapsOnly, value: 1}]}' --
+
+#ifndef INCLUDE_GUARD
+#define INCLUDE_GUARD
+
+#define problematic_constant 0
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: use CAPS_ONLY for macros
+
+#define problematic_function(x, y) ((a) > (b) ? (a) : (b))
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: use CAPS_ONLY for macros
+
+#define problematic_variadic(...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: use CAPS_ONLY for macros
+
+#endif
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -75,6 +75,7 @@
cppcoreguidelines-avoid-goto
cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) 
cppcoreguidelines-interfaces-global-init
+   cppcoreguidelines-macro-usage
cppcoreguidelines-no-malloc
cppcoreguidelines-owning-memory
cppcoreguidelines-pro-bounds-array-to-pointer-decay
Index: docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
@@ -0,0 +1,28 @@
+.. title:: clang-tidy - cppcoreguidelines-macro-usage
+
+cppcoreguidelines-macro-usage
+=
+
+Find macro usage that is considered problematic because better language
+constructs exist for the task.
+
+The relevant sections in the C++ Core Guidelines are 
+`Enum.1 `_,
+`ES.30 `_,
+`ES.31 `_ and
+`ES.33 

[PATCH] D44831: [clang-format] Refine ObjC guesser to handle child lines of child lines

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added inline comments.



Comment at: lib/Format/Format.cpp:1542
 };
-for (auto Line : AnnotatedLines) {
-  if (LineContainsObjCCode(*Line))
+llvm::DenseSet LinesToCheckSet;
+LinesToCheckSet.reserve(AnnotatedLines.size());

djasper wrote:
> Wouldn't it be much easier to call this function recursively for Children 
> instead of using the lambda as well as this additional set? Lines and their 
> children should form a tree, I think, so you should never see the same line 
> again during recursion.
I'm less worried about cycles and more worried about a maliciously deeply 
nested set of children blowing out the stack and causing a crash.

It seemed safer to use the heap here to avoid that scenario (I don't know of 
any other way to avoid it, since we don't know the stack size and we don't 
control it.)


Repository:
  rC Clang

https://reviews.llvm.org/D44831



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

[PATCH] D44765: PR36643 Make clang-format support more generic TMarcos

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.

Thank you! One more comment from me and I'll leave the rest of the review to 
Krasimir, who has a better idea of how the configuration options should be 
named etc.




Comment at: include/clang/Format/Format.h:1676-1677
 
+  /// \brief A vector of macros that should be interpreted as string wrapping
+  /// macros instead of as function calls.
+  ///

I'm not sure "string wrapping macros" is the right term or can be easily 
understood by the reader of the code. The relevant MSDN page is talking about 
"generic-text mapping". I'm not sure this is a commonly used term either, but 
at least there's a precedent. Another relevant term is "encoding prefix". With 
these in mind, I could suggest following descriptions: "generic-text mapping 
macros", "string literal encoding prefix macros", "macros expanding to a string 
literal encoding prefix".

I'd also expand this a bit explaining where these macros are coming from and 
how they are special. Something along the lines of:

"Some libraries provide macro(s) to change the encoding prefix of string 
literals depending on configuration, for example, `_T()` macro on Microsoft 
platforms. When splitting string literals, the macro should be applied to each 
fragment of the literal to apply the same encoding prefix to all of them, which 
requires special treatment from clang-format. This option lists the names of 
these special macros."


Repository:
  rC Clang

https://reviews.llvm.org/D44765



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

[libcxxabi] r328507 - [demangler] Fix a bug in r328464 found by oss-fuzz.

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Mon Mar 26 08:34:36 2018
New Revision: 328507

URL: http://llvm.org/viewvc/llvm-project?rev=328507&view=rev
Log:
[demangler] Fix a bug in r328464 found by oss-fuzz.

Modified:
libcxxabi/trunk/src/cxa_demangle.cpp

Modified: libcxxabi/trunk/src/cxa_demangle.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/src/cxa_demangle.cpp?rev=328507&r1=328506&r2=328507&view=diff
==
--- libcxxabi/trunk/src/cxa_demangle.cpp (original)
+++ libcxxabi/trunk/src/cxa_demangle.cpp Mon Mar 26 08:34:36 2018
@@ -1096,23 +1096,48 @@ struct ForwardTemplateReference : Node {
   size_t Index;
   Node *Ref = nullptr;
 
+  // If we're currently printing this node. It is possible (though invalid) for
+  // a forward template reference to refer to itself via a substitution. This
+  // creates a cyclic AST, which will stack overflow printing. To fix this, 
bail
+  // out if more than one print* function is active.
+  mutable bool Printing = false;
+
   ForwardTemplateReference(size_t Index_)
   : Node(KForwardTemplateReference, Cache::Unknown, Cache::Unknown,
  Cache::Unknown),
 Index(Index_) {}
 
   bool hasRHSComponentSlow(OutputStream &S) const override {
+if (Printing)
+  return false;
+SwapAndRestore SavePrinting(Printing, true);
 return Ref->hasRHSComponent(S);
   }
   bool hasArraySlow(OutputStream &S) const override {
+if (Printing)
+  return false;
+SwapAndRestore SavePrinting(Printing, true);
 return Ref->hasArray(S);
   }
   bool hasFunctionSlow(OutputStream &S) const override {
+if (Printing)
+  return false;
+SwapAndRestore SavePrinting(Printing, true);
 return Ref->hasFunction(S);
   }
 
-  void printLeft(OutputStream &S) const override { Ref->printLeft(S); }
-  void printRight(OutputStream &S) const override { Ref->printRight(S); }
+  void printLeft(OutputStream &S) const override {
+if (Printing)
+  return;
+SwapAndRestore SavePrinting(Printing, true);
+Ref->printLeft(S);
+  }
+  void printRight(OutputStream &S) const override {
+if (Printing)
+  return;
+SwapAndRestore SavePrinting(Printing, true);
+Ref->printRight(S);
+  }
 };
 
 class NameWithTemplateArgs final : public Node {


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

[PATCH] D44231: [clang-tidy] Check for sizeof that call functions

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh accepted this revision.
alexfh added a comment.

LG. Do you need someone to submit this for you?




Comment at: test/clang-tidy/misc-sizeof-expression.cpp:17
 
+enum E { E_VALUE = 0 };
+

aaron.ballman wrote:
> Can you add a C++11 test case using `enum class` -- I am worried that the 
> `isInteger()` matcher will return false for that type.
> 
> Also, I think you should have a test case for `bool`; I think the check will 
> diagnose bool types but I'm not certain there's value in diagnosing those. 
> What do you think?
> I think the check will diagnose bool types but I'm not certain there's value 
> in diagnosing those

It will be easier to decide once someone finds a few examples of those in real 
life code.


https://reviews.llvm.org/D44231



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

[PATCH] D38455: [clang-tidy] new cppcoreguidelines-narrowing-conversions check.

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.

> I agree that more data would be useful, so I'll do an analysis of flagging 
> all (non-ceil/floor float/double)->integral conversions.

Removing from my dashboard for now.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D38455



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

[PATCH] D44852: [CodeGen] Mark fma as const for Android

[Pirama Arumuga Nainar via Phabricator via cfe-commits]

pirama updated this revision to Diff 139798.
pirama added a comment.

Fix comment.


Repository:
  rC Clang

https://reviews.llvm.org/D44852

Files:
  lib/Sema/SemaDecl.cpp
  test/CodeGen/math-builtins.c


Index: test/CodeGen/math-builtins.c
===
--- test/CodeGen/math-builtins.c
+++ test/CodeGen/math-builtins.c
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -w -S -o - -emit-llvm
  %s | FileCheck %s -check-prefix=NO__ERRNO
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -w -S -o - -emit-llvm 
-fmath-errno %s | FileCheck %s -check-prefix=HAS_ERRNO
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown-gnu -w -S -o - -emit-llvm 
-fmath-errno %s | FileCheck %s --check-prefix=HAS_ERRNO_GNU
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown-android -w -S -o - 
-emit-llvm -fmath-errno %s | FileCheck %s --check-prefix=HAS_ERRNO_ANDROID
 // RUN: %clang_cc1 -triple x86_64-unknown-windows-msvc -w -S -o - -emit-llvm 
-fmath-errno %s | FileCheck %s --check-prefix=HAS_ERRNO_WIN
 
 // Test attributes and codegen of math builtins.
@@ -296,6 +297,10 @@
 // HAS_ERRNO_GNU: declare float @llvm.fma.f32(float, float, float) 
[[READNONE_INTRINSIC]]
 // HAS_ERRNO_GNU: declare x86_fp80 @llvm.fma.f80(x86_fp80, x86_fp80, x86_fp80) 
[[READNONE_INTRINSIC]]
 
+// HAS_ERRNO_ANDROID: declare double @llvm.fma.f64(double, double, double) 
[[READNONE_INTRINSIC:#[0-9]+]]
+// HAS_ERRNO_ANDROID: declare float @llvm.fma.f32(float, float, float) 
[[READNONE_INTRINSIC]]
+// HAS_ERRNO_ANDROID: declare x86_fp80 @llvm.fma.f80(x86_fp80, x86_fp80, 
x86_fp80) [[READNONE_INTRINSIC]]
+
 // HAS_ERRNO_WIN: declare double @llvm.fma.f64(double, double, double) 
[[READNONE_INTRINSIC:#[0-9]+]]
 // HAS_ERRNO_WIN: declare float @llvm.fma.f32(float, float, float) 
[[READNONE_INTRINSIC]]
 // Long double is just double on win, so no f80 use/declaration.
@@ -582,5 +587,6 @@
 // HAS_ERRNO: attributes [[READNONE]] = { {{.*}}readnone{{.*}} }
 
 // HAS_ERRNO_GNU: attributes [[READNONE_INTRINSIC]] = { {{.*}}readnone{{.*}} }
+// HAS_ERRNO_ANDROID: attributes [[READNONE_INTRINSIC]] = { 
{{.*}}readnone{{.*}} }
 // HAS_ERRNO_WIN: attributes [[READNONE_INTRINSIC]] = { {{.*}}readnone{{.*}} }
 
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -13105,11 +13105,11 @@
 Context.BuiltinInfo.isConstWithoutErrno(BuiltinID))
   FD->addAttr(ConstAttr::CreateImplicit(Context, FD->getLocation()));
 
-// We make "fma" on GNU or Windows const because we know it does not set
+// We make "fma" on some platforms const because we know it does not set
 // errno in those environments even though it could set errno based on the
 // C standard.
 const llvm::Triple &Trip = Context.getTargetInfo().getTriple();
-if ((Trip.isGNUEnvironment() || Trip.isOSMSVCRT()) &&
+if ((Trip.isGNUEnvironment() || Trip.isAndroid() || Trip.isOSMSVCRT()) &&
 !FD->hasAttr()) {
   switch (BuiltinID) {
   case Builtin::BI__builtin_fma:


Index: test/CodeGen/math-builtins.c
===
--- test/CodeGen/math-builtins.c
+++ test/CodeGen/math-builtins.c
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -w -S -o - -emit-llvm  %s | FileCheck %s -check-prefix=NO__ERRNO
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -w -S -o - -emit-llvm -fmath-errno %s | FileCheck %s -check-prefix=HAS_ERRNO
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown-gnu -w -S -o - -emit-llvm -fmath-errno %s | FileCheck %s --check-prefix=HAS_ERRNO_GNU
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown-android -w -S -o - -emit-llvm -fmath-errno %s | FileCheck %s --check-prefix=HAS_ERRNO_ANDROID
 // RUN: %clang_cc1 -triple x86_64-unknown-windows-msvc -w -S -o - -emit-llvm -fmath-errno %s | FileCheck %s --check-prefix=HAS_ERRNO_WIN
 
 // Test attributes and codegen of math builtins.
@@ -296,6 +297,10 @@
 // HAS_ERRNO_GNU: declare float @llvm.fma.f32(float, float, float) [[READNONE_INTRINSIC]]
 // HAS_ERRNO_GNU: declare x86_fp80 @llvm.fma.f80(x86_fp80, x86_fp80, x86_fp80) [[READNONE_INTRINSIC]]
 
+// HAS_ERRNO_ANDROID: declare double @llvm.fma.f64(double, double, double) [[READNONE_INTRINSIC:#[0-9]+]]
+// HAS_ERRNO_ANDROID: declare float @llvm.fma.f32(float, float, float) [[READNONE_INTRINSIC]]
+// HAS_ERRNO_ANDROID: declare x86_fp80 @llvm.fma.f80(x86_fp80, x86_fp80, x86_fp80) [[READNONE_INTRINSIC]]
+
 // HAS_ERRNO_WIN: declare double @llvm.fma.f64(double, double, double) [[READNONE_INTRINSIC:#[0-9]+]]
 // HAS_ERRNO_WIN: declare float @llvm.fma.f32(float, float, float) [[READNONE_INTRINSIC]]
 // Long double is just double on win, so no f80 use/declaration.
@@ -582,5 +587,6 @@
 // HAS_ERRNO: attributes [[READNONE]] = { {{.*}}readnone{{.*}} }
 
 // HAS_ERRNO_GNU: attributes [[READNONE_INTRINSIC]] = { {{.*}}readnone{{.*}} }
+//

[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.



Comment at: docs/clang-tidy/checks/bugprone-parent-virtual-call.rst:26
+  int foo() override {... A::foo()...}
+   warning: qualified name A::foo refers to a
+member which was overridden in 

alexfh wrote:
> The formatting looks weird. Could you move `warning: .*` to the start of the 
> next line and put the whole message on the same line?
Could you address the "and put the whole message on the same line" part as well?

Please also put `// ` before `warning:` and `...`s to make the example look 
more like syntactically valid code.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44295



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

[PATCH] D44831: [clang-format] Refine ObjC guesser to handle child lines of child lines

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added inline comments.



Comment at: lib/Format/Format.cpp:1542
 };
-for (auto Line : AnnotatedLines) {
-  if (LineContainsObjCCode(*Line))
+llvm::DenseSet LinesToCheckSet;
+LinesToCheckSet.reserve(AnnotatedLines.size());

benhamilton wrote:
> djasper wrote:
> > Wouldn't it be much easier to call this function recursively for Children 
> > instead of using the lambda as well as this additional set? Lines and their 
> > children should form a tree, I think, so you should never see the same line 
> > again during recursion.
> I'm less worried about cycles and more worried about a maliciously deeply 
> nested set of children blowing out the stack and causing a crash.
> 
> It seemed safer to use the heap here to avoid that scenario (I don't know of 
> any other way to avoid it, since we don't know the stack size and we don't 
> control it.)
djasper@ informed me that `clang-format` is already full of places which rely 
on the stack for recursion on user input, so that window has already been 
broken. :)

I'll happily go ahead and change this.


Repository:
  rC Clang

https://reviews.llvm.org/D44831



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

[PATCH] D44893: [ASTMatchers] Add isAssignmentOperator matcher

[Alexander Kornienko via Phabricator via cfe-commits]

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

LG!


Repository:
  rC Clang

https://reviews.llvm.org/D44893



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

[PATCH] D42776: [Sema] Fix an assertion failure in constant expression evaluation of calls to functions with default arguments

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 139797.
ahatanak added a comment.

Assign an uninitialized APValue to a temporary instead of removing it from the 
temporary map when the temporary's lifetime has ended.


https://reviews.llvm.org/D42776

Files:
  include/clang/AST/APValue.h
  lib/AST/APValue.cpp
  lib/AST/ExprConstant.cpp
  test/SemaCXX/constant-expression-cxx1y.cpp
  test/SemaCXX/constexpr-default-arg.cpp

Index: test/SemaCXX/constexpr-default-arg.cpp
===
--- /dev/null
+++ test/SemaCXX/constexpr-default-arg.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -std=c++1y -S -o - -emit-llvm -verify %s
+
+namespace default_arg_temporary {
+
+constexpr bool equals(const float& arg = 1.0f) {
+  return arg == 1.0f;
+}
+
+constexpr const int &x(const int &p = 0) {
+  return p;
+}
+
+struct S {
+  constexpr S(const int &a = 0) {}
+};
+
+void test_default_arg2() {
+  // This piece of code used to cause an assertion failure in
+  // CallStackFrame::createTemporary because the same MTE is used to initilize
+  // both elements of the array (see PR33140).
+  constexpr S s[2] = {};
+
+  // This piece of code used to cause an assertion failure in
+  // CallStackFrame::createTemporary because multiple CXXDefaultArgExpr share
+  // the same MTE (see PR33140).
+  static_assert(equals() && equals(), "");
+
+  // Test that constant expression evaluation produces distinct lvalues for
+  // each call.
+  static_assert(&x() != &x(), "");
+}
+
+// Check that multiple CXXDefaultInitExprs don't cause an assertion failure.
+struct A { int &&r = 0; }; // expected-warning {{binding reference member}} // expected-note {{reference member declared here}}
+struct B { A x, y; };
+B b = {};
+
+}
Index: test/SemaCXX/constant-expression-cxx1y.cpp
===
--- test/SemaCXX/constant-expression-cxx1y.cpp
+++ test/SemaCXX/constant-expression-cxx1y.cpp
@@ -852,7 +852,6 @@
   static_assert(h(2) == 0, ""); // expected-error {{constant expression}} expected-note {{in call}}
   static_assert(h(3) == 0, ""); // expected-error {{constant expression}} expected-note {{in call}}
 
-  // FIXME: This function should be treated as non-constant.
   constexpr void lifetime_versus_loops() {
 int *p = 0;
 for (int i = 0; i != 2; ++i) {
@@ -862,10 +861,10 @@
   if (i)
 // This modifies the 'n' from the previous iteration of the loop outside
 // its lifetime.
-++*q;
+++*q; // expected-note {{increment of object outside its lifetime}}
 }
   }
-  static_assert((lifetime_versus_loops(), true), "");
+  static_assert((lifetime_versus_loops(), true), ""); // expected-error {{constant expression}} expected-note {{in call}}
 }
 
 namespace Bitfields {
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -452,8 +452,8 @@
 
 // Note that we intentionally use std::map here so that references to
 // values are stable.
-typedef std::map MapTy;
-typedef MapTy::const_iterator temp_iterator;
+typedef std::map VersionToValueMap;
+typedef std::map MapTy;
 /// Temporaries - Temporary lvalues materialized within this stack frame.
 MapTy Temporaries;
 
@@ -463,6 +463,20 @@
 /// Index - The call index of this call.
 unsigned Index;
 
+/// The stack of integers for tracking version numbers for temporaries.
+SmallVector TempVersionStack = {1};
+unsigned CurTempVersion = TempVersionStack.back();
+
+unsigned getTempVersion() const { return TempVersionStack.back(); }
+
+void pushTempVersion() {
+  TempVersionStack.push_back(++CurTempVersion);
+}
+
+void popTempVersion() {
+  TempVersionStack.pop_back();
+}
+
 // FIXME: Adding this to every 'CallStackFrame' may have a nontrivial impact
 // on the overall stack usage of deeply-recursing constexpr evaluataions.
 // (We should cache this map rather than recomputing it repeatedly.)
@@ -479,10 +493,33 @@
APValue *Arguments);
 ~CallStackFrame();
 
-APValue *getTemporary(const void *Key) {
+// Return the temporary for Key whose version number is Version.
+APValue *getTemporary(const void *Key, unsigned Version) {
+  MapTy::iterator I = Temporaries.find(Key);
+  if (I == Temporaries.end())
+return nullptr;
+  VersionToValueMap::iterator J = I->second.find(Version);
+  assert(J != I->second.end() && "temporary not found");
+  return &J->second;
+}
+
+// Return the current temporary for Key in the map.
+APValue *getCurrentTemporary(const void *Key) {
   MapTy::iterator I = Temporaries.find(Key);
-  return I == Temporaries.end() ? nullptr : &I->second;
+  if (I == Temporaries.end())
+return nullptr;
+  assert(!I->second.empty() && "temporary not found");
+  return &I->second.rbegin()->second;

[PATCH] D41648: [clang-tidy] implement cppcoreguidelines macro rules

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst:26
+
+This boolean flag disables all warnings for macros and checks only that 
+macro names are CAPS_ONLY. This option is meant to ease introduction of 
this

Will be good idea to rephrase statement: //except those with CAPS_ONLY names//?



Comment at: docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst:27
+This boolean flag disables all warnings for macros and checks only that 
+macro names are CAPS_ONLY. This option is meant to ease introduction of 
this
+check into older code bases. Default value is `0`/`false`.

is //intended//?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41648



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

[PATCH] D41648: [clang-tidy] implement cppcoreguidelines macro rules

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst:24
+
+.. option:: CheckCapsOnly
+

Will be good idea to generalize this option with other check which deal with 
macros.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41648



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

[libcxx] r328520 - Creating release directory for release_502.

[Tom Stellard via cfe-commits]

Author: tstellar
Date: Mon Mar 26 09:06:06 2018
New Revision: 328520

URL: http://llvm.org/viewvc/llvm-project?rev=328520&view=rev
Log:
Creating release directory for release_502.

Added:
libcxx/tags/RELEASE_502/

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

[libcxx] r328521 - Creating release candidate rc1 from release_502 branch

[Tom Stellard via cfe-commits]

Author: tstellar
Date: Mon Mar 26 09:06:11 2018
New Revision: 328521

URL: http://llvm.org/viewvc/llvm-project?rev=328521&view=rev
Log:
Creating release candidate rc1 from release_502 branch

Added:
libcxx/tags/RELEASE_502/rc1/   (props changed)
  - copied from r328520, libcxx/branches/release_50/

Propchange: libcxx/tags/RELEASE_502/rc1/
--
--- svn:mergeinfo (added)
+++ svn:mergeinfo Mon Mar 26 09:06:11 2018
@@ -0,0 +1,2 @@
+/libcxx/branches/apple:136569-137939
+/libcxx/trunk:309296,309307,309474,309838,309851,309917,309920


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

[libcxxabi] r328522 - Creating release directory for release_502.

[Tom Stellard via cfe-commits]

Author: tstellar
Date: Mon Mar 26 09:06:12 2018
New Revision: 328522

URL: http://llvm.org/viewvc/llvm-project?rev=328522&view=rev
Log:
Creating release directory for release_502.

Added:
libcxxabi/tags/RELEASE_502/

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

[libcxxabi] r328523 - Creating release candidate rc1 from release_502 branch

[Tom Stellard via cfe-commits]

Author: tstellar
Date: Mon Mar 26 09:06:14 2018
New Revision: 328523

URL: http://llvm.org/viewvc/llvm-project?rev=328523&view=rev
Log:
Creating release candidate rc1 from release_502 branch

Added:
libcxxabi/tags/RELEASE_502/rc1/
  - copied from r328522, libcxxabi/branches/release_50/

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

[libunwind] r328534 - Creating release directory for release_502.

[Tom Stellard via cfe-commits]

Author: tstellar
Date: Mon Mar 26 09:06:38 2018
New Revision: 328534

URL: http://llvm.org/viewvc/llvm-project?rev=328534&view=rev
Log:
Creating release directory for release_502.

Added:
libunwind/tags/RELEASE_502/

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

[libunwind] r328535 - Creating release candidate rc1 from release_502 branch

[Tom Stellard via cfe-commits]

Author: tstellar
Date: Mon Mar 26 09:06:40 2018
New Revision: 328535

URL: http://llvm.org/viewvc/llvm-project?rev=328535&view=rev
Log:
Creating release candidate rc1 from release_502 branch

Added:
libunwind/tags/RELEASE_502/rc1/   (props changed)
  - copied from r328534, libunwind/branches/release_50/

Propchange: libunwind/tags/RELEASE_502/rc1/
--
svn:mergeinfo = /libunwind/trunk:308871,309147


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

[PATCH] D44831: [clang-format] Refine ObjC guesser to handle child lines of child lines

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton updated this revision to Diff 139802.
benhamilton added a comment.

- Use recursion. Split lambda out into its own static method since recursion on 
lambdas is quite ugly until C++14


Repository:
  rC Clang

https://reviews.llvm.org/D44831

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


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12171,6 +12171,12 @@
 guessLanguage("foo.h", "#define FOO ({ std::string s; })"));
   EXPECT_EQ(FormatStyle::LK_ObjC,
 guessLanguage("foo.h", "#define FOO ({ NSString *s; })"));
+  EXPECT_EQ(
+  FormatStyle::LK_Cpp,
+  guessLanguage("foo.h", "#define FOO ({ foo(); ({ std::string s; }) })"));
+  EXPECT_EQ(
+  FormatStyle::LK_ObjC,
+  guessLanguage("foo.h", "#define FOO ({ foo(); ({ NSString *s; }) })"));
 }
 
 } // end namespace
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1446,6 +1446,14 @@
 private:
   static bool guessIsObjC(const SmallVectorImpl 
&AnnotatedLines,
   const AdditionalKeywords &Keywords) {
+for (auto Line : AnnotatedLines)
+  if (LineContainsObjCCode(*Line, Keywords))
+return true;
+return false;
+  }
+
+  static bool LineContainsObjCCode(const AnnotatedLine &Line,
+   const AdditionalKeywords &Keywords) {
 // Keep this array sorted, since we are binary searching over it.
 static constexpr llvm::StringLiteral FoundationIdentifiers[] = {
 "CGFloat",
@@ -1514,40 +1522,32 @@
 "UIView",
 };
 
-auto LineContainsObjCCode = [&Keywords](const AnnotatedLine &Line) {
-  for (const FormatToken *FormatTok = Line.First; FormatTok;
-   FormatTok = FormatTok->Next) {
-if ((FormatTok->Previous && FormatTok->Previous->is(tok::at) &&
- (FormatTok->isObjCAtKeyword(tok::objc_interface) ||
-  FormatTok->isObjCAtKeyword(tok::objc_implementation) ||
-  FormatTok->isObjCAtKeyword(tok::objc_protocol) ||
-  FormatTok->isObjCAtKeyword(tok::objc_end) ||
-  FormatTok->isOneOf(tok::numeric_constant, tok::l_square,
- tok::l_brace))) ||
-(FormatTok->Tok.isAnyIdentifier() &&
- std::binary_search(std::begin(FoundationIdentifiers),
-std::end(FoundationIdentifiers),
-FormatTok->TokenText)) ||
-FormatTok->is(TT_ObjCStringLiteral) ||
-FormatTok->isOneOf(Keywords.kw_NS_ENUM, Keywords.kw_NS_OPTIONS,
-   TT_ObjCBlockLBrace, TT_ObjCBlockLParen,
-   TT_ObjCDecl, TT_ObjCForIn, TT_ObjCMethodExpr,
-   TT_ObjCMethodSpecifier, TT_ObjCProperty)) {
-  return true;
-}
-  }
-  return false;
-};
-for (auto Line : AnnotatedLines) {
-  if (LineContainsObjCCode(*Line))
+for (const FormatToken *FormatTok = Line.First; FormatTok;
+ FormatTok = FormatTok->Next) {
+  if ((FormatTok->Previous && FormatTok->Previous->is(tok::at) &&
+   (FormatTok->isObjCAtKeyword(tok::objc_interface) ||
+FormatTok->isObjCAtKeyword(tok::objc_implementation) ||
+FormatTok->isObjCAtKeyword(tok::objc_protocol) ||
+FormatTok->isObjCAtKeyword(tok::objc_end) ||
+FormatTok->isOneOf(tok::numeric_constant, tok::l_square,
+   tok::l_brace))) ||
+  (FormatTok->Tok.isAnyIdentifier() &&
+   std::binary_search(std::begin(FoundationIdentifiers),
+  std::end(FoundationIdentifiers),
+  FormatTok->TokenText)) ||
+  FormatTok->is(TT_ObjCStringLiteral) ||
+  FormatTok->isOneOf(Keywords.kw_NS_ENUM, Keywords.kw_NS_OPTIONS,
+ TT_ObjCBlockLBrace, TT_ObjCBlockLParen,
+ TT_ObjCDecl, TT_ObjCForIn, TT_ObjCMethodExpr,
+ TT_ObjCMethodSpecifier, TT_ObjCProperty)) {
 return true;
-  for (auto ChildLine : Line->Children) {
-if (LineContainsObjCCode(*ChildLine))
-  return true;
   }
+  for (auto ChildLine : Line.Children)
+if (LineContainsObjCCode(*ChildLine, Keywords))
+  return true;
 }
 return false;
-  }
+  };
 
   bool IsObjC;
 };


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12171,6 +12171,12 @@
 guessLanguage("foo.h", "#define FOO ({ std::string s; })"));
   EXPECT_EQ(FormatStyle::LK_ObjC,
 guessLanguage("foo.h", "#define FOO 

[PATCH] D41648: [clang-tidy] implement cppcoreguidelines macro rules

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Which checks do you have in mind?

Am 26.03.2018 um 18:07 schrieb Eugene Zelenko via Phabricator:

> Eugene.Zelenko added inline comments.
> 
> 
>  Comment at: docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst:24
>  +
>  +.. option:: CheckCapsOnly
>  +
> 
>  
> 
> Will be good idea to generalize this option with other check which deal with 
> macros.
> 
> Repository:
> 
>   rCTE Clang Tools Extra
> 
> https://reviews.llvm.org/D41648


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41648



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

[PATCH] D44831: [clang-format] Refine ObjC guesser to handle child lines of child lines

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton updated this revision to Diff 139804.
benhamilton added a comment.

- Remove stray semicolon.


Repository:
  rC Clang

https://reviews.llvm.org/D44831

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


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12171,6 +12171,12 @@
 guessLanguage("foo.h", "#define FOO ({ std::string s; })"));
   EXPECT_EQ(FormatStyle::LK_ObjC,
 guessLanguage("foo.h", "#define FOO ({ NSString *s; })"));
+  EXPECT_EQ(
+  FormatStyle::LK_Cpp,
+  guessLanguage("foo.h", "#define FOO ({ foo(); ({ std::string s; }) })"));
+  EXPECT_EQ(
+  FormatStyle::LK_ObjC,
+  guessLanguage("foo.h", "#define FOO ({ foo(); ({ NSString *s; }) })"));
 }
 
 } // end namespace
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1446,6 +1446,14 @@
 private:
   static bool guessIsObjC(const SmallVectorImpl 
&AnnotatedLines,
   const AdditionalKeywords &Keywords) {
+for (auto Line : AnnotatedLines)
+  if (LineContainsObjCCode(*Line, Keywords))
+return true;
+return false;
+  }
+
+  static bool LineContainsObjCCode(const AnnotatedLine &Line,
+   const AdditionalKeywords &Keywords) {
 // Keep this array sorted, since we are binary searching over it.
 static constexpr llvm::StringLiteral FoundationIdentifiers[] = {
 "CGFloat",
@@ -1514,37 +1522,29 @@
 "UIView",
 };
 
-auto LineContainsObjCCode = [&Keywords](const AnnotatedLine &Line) {
-  for (const FormatToken *FormatTok = Line.First; FormatTok;
-   FormatTok = FormatTok->Next) {
-if ((FormatTok->Previous && FormatTok->Previous->is(tok::at) &&
- (FormatTok->isObjCAtKeyword(tok::objc_interface) ||
-  FormatTok->isObjCAtKeyword(tok::objc_implementation) ||
-  FormatTok->isObjCAtKeyword(tok::objc_protocol) ||
-  FormatTok->isObjCAtKeyword(tok::objc_end) ||
-  FormatTok->isOneOf(tok::numeric_constant, tok::l_square,
- tok::l_brace))) ||
-(FormatTok->Tok.isAnyIdentifier() &&
- std::binary_search(std::begin(FoundationIdentifiers),
-std::end(FoundationIdentifiers),
-FormatTok->TokenText)) ||
-FormatTok->is(TT_ObjCStringLiteral) ||
-FormatTok->isOneOf(Keywords.kw_NS_ENUM, Keywords.kw_NS_OPTIONS,
-   TT_ObjCBlockLBrace, TT_ObjCBlockLParen,
-   TT_ObjCDecl, TT_ObjCForIn, TT_ObjCMethodExpr,
-   TT_ObjCMethodSpecifier, TT_ObjCProperty)) {
-  return true;
-}
-  }
-  return false;
-};
-for (auto Line : AnnotatedLines) {
-  if (LineContainsObjCCode(*Line))
+for (const FormatToken *FormatTok = Line.First; FormatTok;
+ FormatTok = FormatTok->Next) {
+  if ((FormatTok->Previous && FormatTok->Previous->is(tok::at) &&
+   (FormatTok->isObjCAtKeyword(tok::objc_interface) ||
+FormatTok->isObjCAtKeyword(tok::objc_implementation) ||
+FormatTok->isObjCAtKeyword(tok::objc_protocol) ||
+FormatTok->isObjCAtKeyword(tok::objc_end) ||
+FormatTok->isOneOf(tok::numeric_constant, tok::l_square,
+   tok::l_brace))) ||
+  (FormatTok->Tok.isAnyIdentifier() &&
+   std::binary_search(std::begin(FoundationIdentifiers),
+  std::end(FoundationIdentifiers),
+  FormatTok->TokenText)) ||
+  FormatTok->is(TT_ObjCStringLiteral) ||
+  FormatTok->isOneOf(Keywords.kw_NS_ENUM, Keywords.kw_NS_OPTIONS,
+ TT_ObjCBlockLBrace, TT_ObjCBlockLParen,
+ TT_ObjCDecl, TT_ObjCForIn, TT_ObjCMethodExpr,
+ TT_ObjCMethodSpecifier, TT_ObjCProperty)) {
 return true;
-  for (auto ChildLine : Line->Children) {
-if (LineContainsObjCCode(*ChildLine))
-  return true;
   }
+  for (auto ChildLine : Line.Children)
+if (LineContainsObjCCode(*ChildLine, Keywords))
+  return true;
 }
 return false;
   }


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12171,6 +12171,12 @@
 guessLanguage("foo.h", "#define FOO ({ std::string s; })"));
   EXPECT_EQ(FormatStyle::LK_ObjC,
 guessLanguage("foo.h", "#define FOO ({ NSString *s; })"));
+  EXPECT_EQ(
+  FormatStyle::LK_Cpp,
+  guessLanguage("foo.h", "#define FOO ({ foo();

[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis updated this revision to Diff 139805.
zinovy.nis retitled this revision from "[clang-tidy] Detect and fix calls to 
grand-...parent virtual methods instead of calls to parent's virtual methods" 
to "[clang-tidy] Detects and fixes calls to grand-...parent virtual methods 
instead of calls to parent's virtual methods".
zinovy.nis added a comment.

- Beautify bugprone-parent-virtual-call.rst


https://reviews.llvm.org/D44295

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/ParentVirtualCallCheck.cpp
  clang-tidy/bugprone/ParentVirtualCallCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-parent-virtual-call.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-parent-virtual-call.cpp

Index: test/clang-tidy/bugprone-parent-virtual-call.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-parent-virtual-call.cpp
@@ -0,0 +1,177 @@
+// RUN: %check_clang_tidy %s bugprone-parent-virtual-call %t
+
+extern int foo();
+
+class A {
+public:
+  A() = default;
+  virtual ~A() = default;
+
+  virtual int virt_1() { return foo() + 1; }
+  virtual int virt_2() { return foo() + 2; }
+
+  int non_virt() { return foo() + 3; }
+  static int stat() { return foo() + 4; }
+};
+
+class B : public A {
+public:
+  B() = default;
+
+  // Nothing to fix: calls to direct parent.
+  int virt_1() override { return A::virt_1() + 3; }
+  int virt_2() override { return A::virt_2() + 4; }
+};
+
+class C : public B {
+public:
+  int virt_1() override { return A::virt_1() + B::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' refers to a member which was overridden in subclass 'B' [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return B::virt_1() + B::virt_1(); }
+  int virt_2() override { return A::virt_1() + B::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' refers to a member which was overridden in subclass 'B' [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_2() override { return B::virt_1() + B::virt_1(); }
+
+  // Test that non-virtual and static methods are not affected by this cherker.
+  int method_c() { return A::stat() + A::non_virt(); }
+};
+
+// Check aliased type names
+using A1 = A;
+typedef A A2;
+
+class C2 : public B {
+public:
+  int virt_1() override { return A1::virt_1() + A2::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' refers to a member which was overridden in subclass 'B' [bugprone-parent-virtual-call]
+  // CHECK-MESSAGES: :[[@LINE-2]]:49: warning: qualified name 'A::virt_1' refers to a member which was overridden in subclass 'B' [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return B::virt_1() + B::virt_1(); }
+};
+
+// Test that the check affects grand-grand..-parent calls too.
+class D : public C {
+public:
+  int virt_1() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' refers to a member which was overridden in subclass 'C' [bugprone-parent-virtual-call]
+  // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: qualified name 'B::virt_1' refers to a member which was overridden in subclass 'C' [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+  int virt_2() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' refers to a member which was overridden in subclass 'C' [bugprone-parent-virtual-call]
+  // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: qualified name 'B::virt_1' refers to a member which was overridden in subclass 'C' [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_2() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+};
+
+// Test classes in namespaces.
+namespace {
+class BN : public A {
+public:
+  int virt_1() override { return A::virt_1() + 3; }
+  int virt_2() override { return A::virt_2() + 4; }
+};
+} // namespace
+
+namespace N1 {
+class A {
+public:
+  A() = default;
+  virtual int virt_1() { return foo() + 1; }
+  virtual int virt_2() { return foo() + 2; }
+};
+} // namespace N1
+
+namespace N2 {
+class BN : public N1::A {
+public:
+  int virt_1() override { return A::virt_1() + 3; }
+  int virt_2() override { return A::virt_2() + 4; }
+};
+} // namespace N2
+
+class CN : public BN {
+public:
+  int virt_1() override { return A::virt_1() + BN::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' refers to a member which was overridden in subclass 'BN' [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return BN::virt_1() + BN::virt_1(); }
+  int virt_2() override { return A::virt_1() + BN::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warnin

r328544 - [OPENMP] Codegen for declare target with link clause.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Mon Mar 26 09:40:55 2018
New Revision: 328544

URL: http://llvm.org/viewvc/llvm-project?rev=328544&view=rev
Log:
[OPENMP] Codegen for declare target with link clause.

If the link clause is used on the declare target directive, the object
should be linked on target or target data directives, not during the
codegen. Patch adds support for this clause.

Added:
cfe/trunk/test/OpenMP/declare_target_link_codegen.cpp
Modified:
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h
cfe/trunk/lib/Sema/SemaOpenMP.cpp

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=328544&r1=328543&r2=328544&view=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Mon Mar 26 09:40:55 2018
@@ -9494,10 +9494,13 @@ bool ASTContext::DeclMustBeEmitted(const
   return true;
 
   // If the decl is marked as `declare target`, it should be emitted.
-  for (const auto *Decl = D->getMostRecentDecl(); Decl;
-   Decl = Decl->getPreviousDecl())
-if (Decl->hasAttr())
-  return true;
+  for (const auto *Decl : D->redecls()) {
+if (!Decl->hasAttrs())
+  continue;
+if (const auto *Attr = Decl->getAttr())
+  if (Attr->getMapType() != OMPDeclareTargetDeclAttr::MT_Link)
+return true;
+  }
 
   return false;
 }

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=328544&r1=328543&r2=328544&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Mon Mar 26 09:40:55 2018
@@ -2210,6 +2210,22 @@ static LValue EmitThreadPrivateVarDeclLV
   return CGF.MakeAddrLValue(Addr, T, AlignmentSource::Decl);
 }
 
+static Address emitDeclTargetLinkVarDeclLValue(CodeGenFunction &CGF,
+   const VarDecl *VD, QualType T) {
+  for (const auto *D : VD->redecls()) {
+if (!VD->hasAttrs())
+  continue;
+if (const auto *Attr = D->getAttr())
+  if (Attr->getMapType() == OMPDeclareTargetDeclAttr::MT_Link) {
+QualType PtrTy = CGF.getContext().getPointerType(VD->getType());
+Address Addr =
+CGF.CGM.getOpenMPRuntime().getAddrOfDeclareTargetLink(CGF, VD);
+return CGF.EmitLoadOfPointer(Addr, PtrTy->castAs());
+  }
+  }
+  return Address::invalid();
+}
+
 Address
 CodeGenFunction::EmitLoadOfReference(LValue RefLVal,
  LValueBaseInfo *PointeeBaseInfo,
@@ -2259,6 +2275,13 @@ static LValue EmitGlobalVarDeclLValue(Co
   if (VD->getTLSKind() == VarDecl::TLS_Dynamic &&
   CGF.CGM.getCXXABI().usesThreadWrapperFunction())
 return CGF.CGM.getCXXABI().EmitThreadLocalVarDeclLValue(CGF, VD, T);
+  // Check if the variable is marked as declare target with link clause in
+  // device codegen.
+  if (CGF.getLangOpts().OpenMPIsDevice) {
+Address Addr = emitDeclTargetLinkVarDeclLValue(CGF, VD, T);
+if (Addr.isValid())
+  return CGF.MakeAddrLValue(Addr, T, AlignmentSource::Decl);
+  }
 
   llvm::Value *V = CGF.CGM.GetAddrOfGlobalVar(VD);
   llvm::Type *RealVarTy = CGF.getTypes().ConvertTypeForMem(VD->getType());

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=328544&r1=328543&r2=328544&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Mon Mar 26 09:40:55 2018
@@ -893,6 +893,17 @@ static void EmitOMPAggregateInit(CodeGen
   CGF.EmitBlock(DoneBB, /*IsFinished=*/true);
 }
 
+static llvm::Optional
+isDeclareTargetDeclaration(const ValueDecl *VD) {
+  for (const auto *D : VD->redecls()) {
+if (!D->hasAttrs())
+  continue;
+if (const auto *Attr = D->getAttr())
+  return Attr->getMapType();
+  }
+  return llvm::None;
+}
+
 LValue ReductionCodeGen::emitSharedLValue(CodeGenFunction &CGF, const Expr *E) 
{
   return CGF.EmitOMPSharedLValue(E);
 }
@@ -2326,6 +2337,28 @@ llvm::Constant *CGOpenMPRuntime::createD
   return CGM.CreateRuntimeFunction(FnTy, Name);
 }
 
+Address CGOpenMPRuntime::getAddrOfDeclareTargetLink(CodeGenFunction &CGF,
+const VarDecl *VD) {
+  llvm::Optional Res =
+  isDeclareTargetDeclaration(VD);
+  if (Res && *Res == OMPDeclareTargetDeclAttr::MT_Link) {
+SmallString<64> PtrName;
+{
+  llvm::raw_svector_ostream OS(PtrName);
+  OS << CGM.getMangledName(GlobalDecl(VD)) << "_decl_tgt_link_ptr";
+}
+llvm::Value *Ptr = CGM.getModule().getNamedValue(PtrName);
+if (!Ptr)

[PATCH] D44882: [clangd] Implementation of workspace/symbol request

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Very nice! I'd like to reduce the scope of the initial patch, which seems to be 
possible, so we can review the details but not get bogged down too much.

In https://reviews.llvm.org/D44882#1048179, @malaperle wrote:

> In https://reviews.llvm.org/D44882#1048043, @ilya-biryukov wrote:
>
> > - Unconditionally storing much more symbols in the index might have subtle 
> > performance implications, especially for our internal use (the codebase is 
> > huge).  I bet that internally we wouldn't want to store the symbols not 
> > needed for completion, so we'll probably need a switch to disable storing 
> > them in the indexing implementation. But let's wait for Sam to take a look, 
> > he certainly has a better perspective on the issues there.
>
>
> I'm a bit surprised that internally you do not want symbols beyond the ones 
> for completion.


FWIW, I think we do, it just needs to be done carefully (with the right 
semantic filters at query time, and metadata at indexing time).
Can we split this patch up (it's pretty large anyway) and land the 
`workspaceSymbols` implementation first, without changing the indexer behavior?

I think the indexer changes are in the right direction, but I think 
"ForCompletion" isn't quite enough metadata, and don't want to block everything 
on that.

> We have a lot more features in mind that will make the index much bigger, 
> like adding all occurrences (maybe not in the current YAML though).

(tangential)
FWIW, I think *that* might be a case where we want quite a different API - I 
think this is a file-major index operation (handful of results per file) rather 
than a symbol-major one (results per symbol is essentially unbounded, e.g. 
`std::string`). At the scale of google's internal codebase, this means we need 
a different backend, and this may be the case for large open-source codebases 
too (I know chromium hits scaling issues with indexers they try). 
There are also other operations where results are files rather than symbols: 
which files include file x, etc.

> But having options to control the amount of information indexed sounds good 
> to me as there can be a lot of different project sizes and there can be 
> different tradeoffs. I had in mind to an option for including/excluding the 
> occurrences because without them, you lose workspace/references, call 
> hierarchy, etc, but you still have code completion and workspace/symbol, 
> document/symbol, etc while making the index much smaller. But more options 
> sounds good.

If we add options, someone has to actually set them.
Anything that's too big for google's internal index or similar can be filtered 
out in the wrapping code (mapreduce) if the indexer exposes enough information 
to do so.
We shouldn't have problems with the in-memory dynamic index.
Features that are off in the default static indexer configuration won't end up 
getting used much.

So I can see a use for filters where the command-line indexer (YAML) would run 
too slow otherwise, but a) let's cross that bridge when we come to it and b) 
there's lots of low-hanging fruit there - the YAML format itself should be 
considered a placeholder.

>> - Current `fuzzyFind` implementation can only match qualifiers strictly 
>> (i.e. st::vector won't match std::vector). Should we look into allowing 
>> fuzzy matches for this feature?  (not in this patch, rather in the long 
>> term).
> 
> I'm not sure, I'm thinking there should be a balance between how fuzzy it it 
> and how much noise it generates. Right now I find it a bit too fuzzy since 
> when I type "Draft" (to find DraftMgr), it picks up things like 
> DocumentRangeFormattingParams. Adding fuzziness to the namespace would make 
> this worse. Maybe with improved scoring it won't matter too much? I'll try 
> FuzzyMatcher and see.

+1, I think experience with `workspaceSymbols` will help us answer this 
question.




Comment at: clangd/ClangdLSPServer.cpp:99
   
Params.capabilities.textDocument.completion.completionItem.snippetSupport;
+  if (Params.capabilities.workspace && Params.capabilities.workspace->symbol &&
+  Params.capabilities.workspace->symbol->symbolKind)

This is the analogue to the one on `completion` that we currently ignore, and 
one on `symbol` corresponding to the `documentSymbol` call which isn't 
implemented yet.

I think the value of the extended types is low and it might be worth leaving 
out of the first version to keep complexity down.
(If we really want it, the supporting code (mapping tables) should probably be 
in a place where it can be reused by `documentSymbol` and can live next to the 
equivalent for `completion`...)



Comment at: clangd/ClangdLSPServer.h:95
   bool IsDone = false;
+  /// Indicates whether or not the index is available.
+  bool SymbolIndexAvailable = false;

This comment is just the var name, delete?



Comment at: clangd/ClangdLSPServer.h:96
+ 

[PATCH] D44823: [libcxx] Improving std::vector and std::deque perfomance

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists requested changes to this revision.
mclow.lists added a comment.
This revision now requires changes to proceed.

Please don't commit this.




Comment at: libcxx/trunk/include/__split_buffer:201
 __alloc_rr& __a = this->__alloc();
+pointer __to_be_end = this->__end_;
 do

I have been asked specifically by the optimizer folks to NOT do things like 
this in libc++, but rather to file bugs against the optimizer.

And I have done so for this exact case:  
https://bugs.llvm.org/show_bug.cgi?id=35637


Repository:
  rCXX libc++

https://reviews.llvm.org/D44823



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

r328552 - [CodeGen] Mark fma as const for Android

[Pirama Arumuga Nainar via cfe-commits]

Author: pirama
Date: Mon Mar 26 10:03:34 2018
New Revision: 328552

URL: http://llvm.org/viewvc/llvm-project?rev=328552&view=rev
Log:
[CodeGen] Mark fma as const for Android

Summary:
r318093 sets fma, fmaf, fmal as const for Gnu and MSVC.  Android also
does not set errno for these functions.  So mark these const for
Android.

Reviewers: spatel, efriedma, srhines, chh, enh

Subscribers: cfe-commits, llvm-commits

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

Modified:
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/test/CodeGen/math-builtins.c

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=328552&r1=328551&r2=328552&view=diff
==
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Mon Mar 26 10:03:34 2018
@@ -13116,11 +13116,11 @@ void Sema::AddKnownFunctionAttributes(Fu
 Context.BuiltinInfo.isConstWithoutErrno(BuiltinID))
   FD->addAttr(ConstAttr::CreateImplicit(Context, FD->getLocation()));
 
-// We make "fma" on GNU or Windows const because we know it does not set
+// We make "fma" on some platforms const because we know it does not set
 // errno in those environments even though it could set errno based on the
 // C standard.
 const llvm::Triple &Trip = Context.getTargetInfo().getTriple();
-if ((Trip.isGNUEnvironment() || Trip.isOSMSVCRT()) &&
+if ((Trip.isGNUEnvironment() || Trip.isAndroid() || Trip.isOSMSVCRT()) &&
 !FD->hasAttr()) {
   switch (BuiltinID) {
   case Builtin::BI__builtin_fma:

Modified: cfe/trunk/test/CodeGen/math-builtins.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/math-builtins.c?rev=328552&r1=328551&r2=328552&view=diff
==
--- cfe/trunk/test/CodeGen/math-builtins.c (original)
+++ cfe/trunk/test/CodeGen/math-builtins.c Mon Mar 26 10:03:34 2018
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -w -S -o - -emit-llvm
  %s | FileCheck %s -check-prefix=NO__ERRNO
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -w -S -o - -emit-llvm 
-fmath-errno %s | FileCheck %s -check-prefix=HAS_ERRNO
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown-gnu -w -S -o - -emit-llvm 
-fmath-errno %s | FileCheck %s --check-prefix=HAS_ERRNO_GNU
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown-android -w -S -o - 
-emit-llvm -fmath-errno %s | FileCheck %s --check-prefix=HAS_ERRNO_ANDROID
 // RUN: %clang_cc1 -triple x86_64-unknown-windows-msvc -w -S -o - -emit-llvm 
-fmath-errno %s | FileCheck %s --check-prefix=HAS_ERRNO_WIN
 
 // Test attributes and codegen of math builtins.
@@ -296,6 +297,10 @@ void foo(double *d, float f, float *fp,
 // HAS_ERRNO_GNU: declare float @llvm.fma.f32(float, float, float) 
[[READNONE_INTRINSIC]]
 // HAS_ERRNO_GNU: declare x86_fp80 @llvm.fma.f80(x86_fp80, x86_fp80, x86_fp80) 
[[READNONE_INTRINSIC]]
 
+// HAS_ERRNO_ANDROID: declare double @llvm.fma.f64(double, double, double) 
[[READNONE_INTRINSIC:#[0-9]+]]
+// HAS_ERRNO_ANDROID: declare float @llvm.fma.f32(float, float, float) 
[[READNONE_INTRINSIC]]
+// HAS_ERRNO_ANDROID: declare x86_fp80 @llvm.fma.f80(x86_fp80, x86_fp80, 
x86_fp80) [[READNONE_INTRINSIC]]
+
 // HAS_ERRNO_WIN: declare double @llvm.fma.f64(double, double, double) 
[[READNONE_INTRINSIC:#[0-9]+]]
 // HAS_ERRNO_WIN: declare float @llvm.fma.f32(float, float, float) 
[[READNONE_INTRINSIC]]
 // Long double is just double on win, so no f80 use/declaration.
@@ -582,5 +587,6 @@ void foo(double *d, float f, float *fp,
 // HAS_ERRNO: attributes [[READNONE]] = { {{.*}}readnone{{.*}} }
 
 // HAS_ERRNO_GNU: attributes [[READNONE_INTRINSIC]] = { {{.*}}readnone{{.*}} }
+// HAS_ERRNO_ANDROID: attributes [[READNONE_INTRINSIC]] = { 
{{.*}}readnone{{.*}} }
 // HAS_ERRNO_WIN: attributes [[READNONE_INTRINSIC]] = { {{.*}}readnone{{.*}} }
 


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

[PATCH] D44852: [CodeGen] Mark fma as const for Android

[Pirama Arumuga Nainar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL328552: [CodeGen] Mark fma as const for Android (authored by 
pirama, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D44852

Files:
  cfe/trunk/lib/Sema/SemaDecl.cpp
  cfe/trunk/test/CodeGen/math-builtins.c


Index: cfe/trunk/test/CodeGen/math-builtins.c
===
--- cfe/trunk/test/CodeGen/math-builtins.c
+++ cfe/trunk/test/CodeGen/math-builtins.c
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -w -S -o - -emit-llvm
  %s | FileCheck %s -check-prefix=NO__ERRNO
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -w -S -o - -emit-llvm 
-fmath-errno %s | FileCheck %s -check-prefix=HAS_ERRNO
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown-gnu -w -S -o - -emit-llvm 
-fmath-errno %s | FileCheck %s --check-prefix=HAS_ERRNO_GNU
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown-android -w -S -o - 
-emit-llvm -fmath-errno %s | FileCheck %s --check-prefix=HAS_ERRNO_ANDROID
 // RUN: %clang_cc1 -triple x86_64-unknown-windows-msvc -w -S -o - -emit-llvm 
-fmath-errno %s | FileCheck %s --check-prefix=HAS_ERRNO_WIN
 
 // Test attributes and codegen of math builtins.
@@ -296,6 +297,10 @@
 // HAS_ERRNO_GNU: declare float @llvm.fma.f32(float, float, float) 
[[READNONE_INTRINSIC]]
 // HAS_ERRNO_GNU: declare x86_fp80 @llvm.fma.f80(x86_fp80, x86_fp80, x86_fp80) 
[[READNONE_INTRINSIC]]
 
+// HAS_ERRNO_ANDROID: declare double @llvm.fma.f64(double, double, double) 
[[READNONE_INTRINSIC:#[0-9]+]]
+// HAS_ERRNO_ANDROID: declare float @llvm.fma.f32(float, float, float) 
[[READNONE_INTRINSIC]]
+// HAS_ERRNO_ANDROID: declare x86_fp80 @llvm.fma.f80(x86_fp80, x86_fp80, 
x86_fp80) [[READNONE_INTRINSIC]]
+
 // HAS_ERRNO_WIN: declare double @llvm.fma.f64(double, double, double) 
[[READNONE_INTRINSIC:#[0-9]+]]
 // HAS_ERRNO_WIN: declare float @llvm.fma.f32(float, float, float) 
[[READNONE_INTRINSIC]]
 // Long double is just double on win, so no f80 use/declaration.
@@ -582,5 +587,6 @@
 // HAS_ERRNO: attributes [[READNONE]] = { {{.*}}readnone{{.*}} }
 
 // HAS_ERRNO_GNU: attributes [[READNONE_INTRINSIC]] = { {{.*}}readnone{{.*}} }
+// HAS_ERRNO_ANDROID: attributes [[READNONE_INTRINSIC]] = { 
{{.*}}readnone{{.*}} }
 // HAS_ERRNO_WIN: attributes [[READNONE_INTRINSIC]] = { {{.*}}readnone{{.*}} }
 
Index: cfe/trunk/lib/Sema/SemaDecl.cpp
===
--- cfe/trunk/lib/Sema/SemaDecl.cpp
+++ cfe/trunk/lib/Sema/SemaDecl.cpp
@@ -13116,11 +13116,11 @@
 Context.BuiltinInfo.isConstWithoutErrno(BuiltinID))
   FD->addAttr(ConstAttr::CreateImplicit(Context, FD->getLocation()));
 
-// We make "fma" on GNU or Windows const because we know it does not set
+// We make "fma" on some platforms const because we know it does not set
 // errno in those environments even though it could set errno based on the
 // C standard.
 const llvm::Triple &Trip = Context.getTargetInfo().getTriple();
-if ((Trip.isGNUEnvironment() || Trip.isOSMSVCRT()) &&
+if ((Trip.isGNUEnvironment() || Trip.isAndroid() || Trip.isOSMSVCRT()) &&
 !FD->hasAttr()) {
   switch (BuiltinID) {
   case Builtin::BI__builtin_fma:


Index: cfe/trunk/test/CodeGen/math-builtins.c
===
--- cfe/trunk/test/CodeGen/math-builtins.c
+++ cfe/trunk/test/CodeGen/math-builtins.c
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -w -S -o - -emit-llvm  %s | FileCheck %s -check-prefix=NO__ERRNO
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -w -S -o - -emit-llvm -fmath-errno %s | FileCheck %s -check-prefix=HAS_ERRNO
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown-gnu -w -S -o - -emit-llvm -fmath-errno %s | FileCheck %s --check-prefix=HAS_ERRNO_GNU
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown-android -w -S -o - -emit-llvm -fmath-errno %s | FileCheck %s --check-prefix=HAS_ERRNO_ANDROID
 // RUN: %clang_cc1 -triple x86_64-unknown-windows-msvc -w -S -o - -emit-llvm -fmath-errno %s | FileCheck %s --check-prefix=HAS_ERRNO_WIN
 
 // Test attributes and codegen of math builtins.
@@ -296,6 +297,10 @@
 // HAS_ERRNO_GNU: declare float @llvm.fma.f32(float, float, float) [[READNONE_INTRINSIC]]
 // HAS_ERRNO_GNU: declare x86_fp80 @llvm.fma.f80(x86_fp80, x86_fp80, x86_fp80) [[READNONE_INTRINSIC]]
 
+// HAS_ERRNO_ANDROID: declare double @llvm.fma.f64(double, double, double) [[READNONE_INTRINSIC:#[0-9]+]]
+// HAS_ERRNO_ANDROID: declare float @llvm.fma.f32(float, float, float) [[READNONE_INTRINSIC]]
+// HAS_ERRNO_ANDROID: declare x86_fp80 @llvm.fma.f80(x86_fp80, x86_fp80, x86_fp80) [[READNONE_INTRINSIC]]
+
 // HAS_ERRNO_WIN: declare double @llvm.fma.f64(double, double, double) [[READNONE_INTRINSIC:#[0-9]+]]
 // HAS_ERRNO_WIN: declare float @llvm.fma.f32(float, float, float) [[READNONE_INTRINSIC]]
 // Long double is just double on

[PATCH] D44852: [CodeGen] Mark fma as const for Android

[Pirama Arumuga Nainar via Phabricator via cfe-commits]

pirama added a comment.

Thanks for the reviews!


Repository:
  rL LLVM

https://reviews.llvm.org/D44852



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

[PATCH] D41648: [clang-tidy] implement cppcoreguidelines macro rules

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

In https://reviews.llvm.org/D41648#1048312, @JonasToth wrote:

> Which checks do you have in mind?


bugprone-macro-*, bugprone-multiple-statement-macro.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41648



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

[PATCH] D44559: [Sema] Wrong width of result of mul operation

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D44559#1048085, @avt77 wrote:

> In https://reviews.llvm.org/D44559#1045758, @rjmccall wrote:
>
> > No, I still oppose this patch.
>
>
> OK, we have 2 possibilities here (fmpov):
>
> 1. Forget about the issue and don't do anything now - it is not a bug
> 2. Return the width based on real analyze of mul args:
>   - signed vs. unsigned
>   - chars, shorts, etc. What do you suggest to do? Or maybe there are other 
> variants?


I think we just close this.  If you want to put time into trying to diagnose in 
Aaron's example of uint8 * uint -> signed short, I think the right approach is 
to enhance IntRange so that it can optionally carry a more aggressive range, 
and then make -Wsign-compare use that when available.  That's a significantly 
different patch from this.


https://reviews.llvm.org/D44559



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

[PATCH] D44559: [Sema] Wrong width of result of mul operation

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

I'm having a very hard time following this thread. In fact, i can't follow the 
logic at all.

In https://reviews.llvm.org/D44559#1041007, @rjmccall wrote:

> ...
>  This patch, however, is contrary to the design of -Wconversion, which does 
> attempt to avoid warning in cases where the user might reasonably see an 
> operation as "really" being performed in its operand type.  If you want to 
> argue that we should change the design of -Wconversion, that is a broader 
> conversation that you should take to cfe-dev.


Can you at least point out where the current design goals are documented?


https://reviews.llvm.org/D44559



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

[PATCH] D44883: [Sema] Extend -Wself-assign with -Wself-assign-overloaded to warn on overloaded self-assignment (classes)

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D44883#1048081, @lebedev.ri wrote:

> In https://reviews.llvm.org/D44883#1048010, @rjmccall wrote:
>
> > I'm not sure you really need to put these in their own warning sub-group 
> > just because they're user-defined operators.  That's especially true 
> > because it appears we already have divisions in the warning group based on 
> > the form of the l-value; we don't want this to go combinatorial.
>
>
> Several reasons:
>
> - The initial `-Wself-assign` was intentionally implemented not to warn on 
> overloaded operators. 
> https://github.com/llvm-mirror/clang/commit/9f7a6441bcbb1b17208cb3abd65a0017525a#diff-e0deb7b32f28507a3044a6bf9a63b515R31
>  (https://reviews.llvm.org/rL122804)


That's interesting.  Maybe Chandler remembers the rationale for that?

> - While it is an obvious bug when self-operation happens with builtin 
> operators, i'm less certain of that with overloaded operators.

It's certainly more plausible that the programmer has a good reason to 
self-assign with a user-defined operator, yeah.

>   If you happen to be routinely using self-assignment via oh-so-very-special 
> overloaded operator=, and you don't like to have this
>   diagnostic, you could just disable it, and not loose the coverage of the 
> `-Wself-assign-builtin`.
>   If it is all in one group, you can't do that...

True.

> - Based on previous expirience, separate diag groups are good, see e.g 
> https://reviews.llvm.org/D37620, https://reviews.llvm.org/D37629
> - I'm failing to find the original quote, but i **think** @rsmith said 
> something along the "diag groups are cheap, use them". But i may as well be 
> mis-remembering/having false memories here, sorry.

Yeah, like I said, I'm just worried about the usability of having multiple 
dimensions of warning group here.  Like, do we need both 
-Wself-assign-field-builtin and -Wself-assign-field-overloaded?  Because I do 
think we should warn on field self-assignments for user-defined operators.

I think the term ought to be "user-defined" rather than "overloaded", by the 
way.  In a sense, these operators aren't really any more overloaded than the 
builtin operators (at least, not necessarily so) — C++ even formalizes the 
builtin operators as being a large family of overloads.  And the reason we want 
to treat them specially is that they have user-provided definitions that might 
be doing special things, not because of anything to do with name resolution.

> TLDR: if you insist, sure, i can just cram it into the already-existing 
> `-Wself-assign`,
>  but i believe that is the opposite of what should be done, and is against 
> the way it was done previously.

I'm not going to insist.  But I would like to hear if Chandler remembers why 
his patch didn't warn about user-defined operators.


Repository:
  rC Clang

https://reviews.llvm.org/D44883



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

[PATCH] D44231: [clang-tidy] Check for sizeof that call functions

[Paul Fultz II via Phabricator via cfe-commits]

pfultz2 added a comment.

> Do you need someone to submit this for you?

Yes


https://reviews.llvm.org/D44231



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

[PATCH] D44883: [Sema] Extend -Wself-assign with -Wself-assign-overloaded to warn on overloaded self-assignment (classes)

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

Two more high-level comments from me, and then I'll try to butt out.

Q1. I don't understand what `field-` is doing in the name of some diagnostics. 
Is there a situation where `x = x;` is "less/more of an error" just because `x` 
is a {data member, lambda capture, structured binding} versus not? Why should 
the "plain old variable" case be separately toggleable? (This question 
pre-dates Roman's proposed patch AFAICT. I think the answer is that 
self-assignment specifically //of a data member// specifically //in a 
constructor// is considered "more of an error" because it's likely to be a very 
particular kind of typo-bug. I doubt this answer is quite strong enough to 
justify multiple warning options, though.)

Q2. I don't understand why `x &= x` (and now `x /= x`, thanks) should be 
treated as "more of an error" than `x & x` (and now `x / x`), or for that 
matter `x == x`. The sin here is not "self-assignment" per se; it's 
"over-complicating a tautological mathematical operation." I admit I haven't 
thought about it as much as you folks, but the more I think about it, the more 
I think the only consistent thing for Clang to do would be to back off from 
warning about anything beyond plain old `x = x`, and leave *all* the 
mathematical-tautology cases to checkers like clang-tidy and PVS-Studio.


Repository:
  rC Clang

https://reviews.llvm.org/D44883



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

Re: r328282 - [analyzer] Trust _Nonnull annotations for system framework

[Alexander Kornienko via cfe-commits]

This checker crashes on almost each file in our codebase. No test case yet,
but here's a stack trace:
clang::Type::getTypeClass
clang::ReferenceType::classof
llvm::isa_impl::doit
llvm::isa_impl_cl::doit
llvm::isa_impl_wrap::doit
llvm::isa_impl_wrap::doit
llvm::isa
clang::Type::getAs
clang::ASTContext::getLValueReferenceType
::TrustNonnullChecker::checkPostCall
clang::ento::check::PostCall::_checkCall
clang::ento::CheckerFn::operator()
::CheckCallContext::runChecker
expandGraphWithCheckers
clang::ento::CheckerManager::runCheckersForCallEvent
clang::ento::CheckerManager::runCheckersForPostCall
clang::ento::ExprEngine::VisitCXXDestructor
clang::ento::ExprEngine::ProcessTemporaryDtor
clang::ento::ExprEngine::ProcessImplicitDtor
clang::ento::ExprEngine::processCFGElement
clang::ento::CoreEngine::dispatchWorkItem
clang::ento::CoreEngine::ExecuteWorkList
::AnalysisConsumer::ActionExprEngine
::AnalysisConsumer::HandleCode
::AnalysisConsumer::HandleTranslationUnit
clang::MultiplexConsumer::HandleTranslationUnit
clang::ParseAST
clang::FrontendAction::Execute
clang::CompilerInstance::ExecuteAction
clang::tooling::FrontendActionFactory::runInvocation
clang::tooling::ToolInvocation::runInvocation
clang::tooling::ToolInvocation::run

Could you fix or revert the patch?


On Fri, Mar 23, 2018 at 1:18 AM George Karpenkov via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: george.karpenkov
> Date: Thu Mar 22 17:16:03 2018
> New Revision: 328282
>
> URL: http://llvm.org/viewvc/llvm-project?rev=328282&view=rev
> Log:
> [analyzer] Trust _Nonnull annotations for system framework
>
> Changes the analyzer to believe that methods annotated with _Nonnull
> from system frameworks indeed return non null objects.
> Local methods with such annotation are still distrusted.
> rdar://24291919
>
> Differential Revision: https://reviews.llvm.org/D44341
>
> Added:
> cfe/trunk/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp
> cfe/trunk/test/Analysis/trustnonnullchecker_test.m
> Modified:
> cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
>
> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
> cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
> cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
> cfe/trunk/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
>
> cfe/trunk/test/Analysis/Inputs/system-header-simulator-for-nullability.h
>
> Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td?rev=328282&r1=328281&r2=328282&view=diff
>
> ==
> --- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td (original)
> +++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td Thu Mar 22
> 17:16:03 2018
> @@ -218,6 +218,14 @@ def NullableReturnedFromNonnullChecker :
>
>  } // end "nullability"
>
> +let ParentPackage = APIModeling in {
> +
> +def TrustNonnullChecker : Checker<"TrustNonnull">,
> +  HelpText<"Trust that returns from framework methods annotated with
> _Nonnull are not null">,
> +  DescFile<"TrustNonnullChecker.cpp">;
> +
> +}
> +
>
>  
> //===--===//
>  // Evaluate "builtin" functions.
>
>  
> //===--===//
>
> Modified:
> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h?rev=328282&r1=328281&r2=328282&view=diff
>
> ==
> ---
> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
> (original)
> +++
> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
> Thu Mar 22 17:16:03 2018
> @@ -21,6 +21,8 @@ namespace clang {
>
>  class Expr;
>  class VarDecl;
> +class QualType;
> +class AttributedType;
>
>  namespace ento {
>
> @@ -42,6 +44,25 @@ template  bool containsStmt(con
>  std::pair
>  parseAssignment(const Stmt *S);
>
> +// Do not reorder! The getMostNullable method relies on the order.
> +// Optimization: Most pointers expected to be unspecified. When a symbol
> has an
> +// unspecified or nonnull type non of the rules would indicate any
> problem for
> +// that symbol. For this reason only nullable and contradicted
> nullability are
> +// stored for a symbol. When a symbol is already contradicted, it can not
> be
> +// casted back to nullable.
> +enum class Nullability : char {
> +  Contradicted, // Tracked nullability is contradicted by an explicit
> cast. Do
> +// not report any nullability related issue for this
> symbol.
> +// This nullability is propagated aggressively to avoid
> false
> +// positive results. See 

[PATCH] D44883: [Sema] Extend -Wself-assign with -Wself-assign-overloaded to warn on overloaded self-assignment (classes)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a subscriber: thakis.
lebedev.ri added a comment.

In https://reviews.llvm.org/D44883#1048421, @Quuxplusone wrote:

> Two more high-level comments from me, and then I'll try to butt out.
>
> Q1. I don't understand what `field-` is doing in the name of some 
> diagnostics. Is there a situation where `x = x;` is "less/more of an error" 
> just because `x` is a {data member, lambda capture, structured binding} 
> versus not? Why should the "plain old variable" case be separately 
> toggleable? (This question pre-dates Roman's proposed patch AFAICT. I think 
> the answer is that self-assignment specifically //of a data member// 
> specifically //in a constructor// is considered "more of an error" because 
> it's likely to be a very particular kind of typo-bug. I doubt this answer is 
> quite strong enough to justify multiple warning options, though.)


I think we should ask @thakis that - https://reviews.llvm.org/rL159394

> Q2. I don't understand why `x &= x` (and now `x /= x`, thanks) should be 
> treated as "more of an error" than `x & x` (and now `x / x`), or for that 
> matter `x == x`. The sin here is not "self-assignment" per se; it's 
> "over-complicating a tautological mathematical operation." I admit I haven't 
> thought about it as much as you folks, but the more I think about it, the 
> more I think the only consistent thing for Clang to do would be to back off 
> from warning about anything beyond plain old `x = x`, and leave *all* the 
> mathematical-tautology cases to checkers like clang-tidy and PVS-Studio.

(oh please no pvs-studio stuff here too, it's getting a 'bit' spammy lately)

In https://reviews.llvm.org/D44883#1048400, @rjmccall wrote:

> Yeah, like I said, I'm just worried about the usability of having multiple 
> dimensions of warning group here.  Like, do we need both 
> -Wself-assign-field-builtin and -Wself-assign-field-overloaded?


I'm not saying that is not a valid concern. I'm simply following the 
pre-existing practice, which is, as far i'm aware, to split the diag groups if 
it makes sense.

> Because I do think we should warn on field self-assignments for user-defined 
> operators.

I agree, as i said, i did not want to put everything into one bit differential.

> I think the term ought to be "user-defined" rather than "overloaded", by the 
> way.  In a sense, these operators aren't really any more overloaded than the 
> builtin operators (at least, not necessarily so) — C++ even formalizes the 
> builtin operators as being a large family of overloads.  And the reason we 
> want to treat them specially is that they have user-provided definitions that 
> might be doing special things, not because of anything to do with name 
> resolution.

Can you elaborate a bit more, given this struct, which of these variants should 
be diagnosed as user-provided, and which as builtin?

  struct S {}; // certainly builtin?

  struct S {
S() = default; // builtin?
S &operator=(const S &) = default; // builtin?
S &operator=(S &) = default;  // builtin?
  
S &operator=(const S &); // certainly user-provided?
S &operator=(S &); // certainly user-provided?
  };



>> TLDR: if you insist, sure, i can just cram it into the already-existing 
>> `-Wself-assign`,
>>  but i believe that is the opposite of what should be done, and is against 
>> the way it was done previously.
> 
> I'm not going to insist.  But I would like to hear if Chandler remembers why 
> his patch didn't warn about user-defined operators.

+1


Repository:
  rC Clang

https://reviews.llvm.org/D44883



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

Re: r328282 - [analyzer] Trust _Nonnull annotations for system framework

[George Karpenkov via cfe-commits]

Yeah, I’m pretty sure this was fixed on Friday with 
https://reviews.llvm.org/rC328406

> On Mar 26, 2018, at 10:54 AM, Alexander Kornienko  wrote:
> 
> This checker crashes on almost each file in our codebase. No test case yet, 
> but here's a stack trace:
> clang::Type::getTypeClass
> clang::ReferenceType::classof
> llvm::isa_impl::doit
> llvm::isa_impl_cl::doit
> llvm::isa_impl_wrap::doit
> llvm::isa_impl_wrap::doit
> llvm::isa
> clang::Type::getAs
> clang::ASTContext::getLValueReferenceType
> ::TrustNonnullChecker::checkPostCall
> clang::ento::check::PostCall::_checkCall
> clang::ento::CheckerFn::operator()
> ::CheckCallContext::runChecker
> expandGraphWithCheckers
> clang::ento::CheckerManager::runCheckersForCallEvent
> clang::ento::CheckerManager::runCheckersForPostCall
> clang::ento::ExprEngine::VisitCXXDestructor
> clang::ento::ExprEngine::ProcessTemporaryDtor
> clang::ento::ExprEngine::ProcessImplicitDtor
> clang::ento::ExprEngine::processCFGElement
> clang::ento::CoreEngine::dispatchWorkItem
> clang::ento::CoreEngine::ExecuteWorkList
> ::AnalysisConsumer::ActionExprEngine
> ::AnalysisConsumer::HandleCode
> ::AnalysisConsumer::HandleTranslationUnit
> clang::MultiplexConsumer::HandleTranslationUnit
> clang::ParseAST
> clang::FrontendAction::Execute
> clang::CompilerInstance::ExecuteAction
> clang::tooling::FrontendActionFactory::runInvocation
> clang::tooling::ToolInvocation::runInvocation
> clang::tooling::ToolInvocation::run
> 
> Could you fix or revert the patch?
> 
> 
> On Fri, Mar 23, 2018 at 1:18 AM George Karpenkov via cfe-commits 
> mailto:cfe-commits@lists.llvm.org>> wrote:
> Author: george.karpenkov
> Date: Thu Mar 22 17:16:03 2018
> New Revision: 328282
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=328282&view=rev 
> 
> Log:
> [analyzer] Trust _Nonnull annotations for system framework
> 
> Changes the analyzer to believe that methods annotated with _Nonnull
> from system frameworks indeed return non null objects.
> Local methods with such annotation are still distrusted.
> rdar://24291919
> 
> Differential Revision: https://reviews.llvm.org/D44341 
> 
> 
> Added:
> cfe/trunk/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp
> cfe/trunk/test/Analysis/trustnonnullchecker_test.m
> Modified:
> cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
> cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
> cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
> cfe/trunk/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
> cfe/trunk/test/Analysis/Inputs/system-header-simulator-for-nullability.h
> 
> Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td?rev=328282&r1=328281&r2=328282&view=diff
>  
> 
> ==
> --- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td (original)
> +++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td Thu Mar 22 
> 17:16:03 2018
> @@ -218,6 +218,14 @@ def NullableReturnedFromNonnullChecker :
> 
>  } // end "nullability"
> 
> +let ParentPackage = APIModeling in {
> +
> +def TrustNonnullChecker : Checker<"TrustNonnull">,
> +  HelpText<"Trust that returns from framework methods annotated with 
> _Nonnull are not null">,
> +  DescFile<"TrustNonnullChecker.cpp">;
> +
> +}
> +
>  
> //===--===//
>  // Evaluate "builtin" functions.
>  
> //===--===//
> 
> Modified: 
> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h?rev=328282&r1=328281&r2=328282&view=diff
>  
> 
> ==
> --- 
> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h 
> (original)
> +++ 
> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h 
> Thu Mar 22 17:16:03 2018
> @@ -21,6 +21,8 @@ namespace clang {
> 
>  class Expr;
>  class VarDecl;
> +class QualType;
> +class AttributedType;
> 
>  namespace ento {
> 
> @@ -42,6 +44,25 @@ template  bool containsStmt(con
>  std::pair
>  parseAssignment(const Stmt *S);
> 
> +// Do not reorder! The getMostNullable method relies on the order.
> +// Optimization: Most pointers ex

[PATCH] D44901: [Diag] Avoid emitting a redefinition note if no location is available.

[Matt Davis via Phabricator via cfe-commits]

mattd created this revision.
mattd added reviewers: bruno, rsmith.

The "previous definition is here" note is not helpful if there is no location 
information. The note will reference nothing in such a case. This patch first 
checks to see if there is location data, and if so the note diagnostic is 
emitted.

This fixes PR15409.  The issue in the first comment seems to already be 
resolved. This patch addresses the second example.


https://reviews.llvm.org/D44901

Files:
  lib/Sema/SemaDecl.cpp
  test/Sema/redefine_extname.c


Index: test/Sema/redefine_extname.c
===
--- test/Sema/redefine_extname.c
+++ test/Sema/redefine_extname.c
@@ -4,3 +4,4 @@
 #pragma redefine_extname foo_static bar_static
 static int foo_static() { return 1; } // expected-warning {{#pragma 
redefine_extname is applicable to external C declarations only; not applied to 
function 'foo_static'}}
 
+unsigned __int128_t; // expected-error {{redefinition of '__int128_t' as 
different kind of symbol}}
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -4057,7 +4057,8 @@
   }
 
   // Redefinition coming from different files or couldn't do better above.
-  Diag(Old->getLocation(), diag::note_previous_definition);
+  if (Old->getLocation().isValid())
+Diag(Old->getLocation(), diag::note_previous_definition);
 }
 
 /// We've just determined that \p Old and \p New both appear to be definitions


Index: test/Sema/redefine_extname.c
===
--- test/Sema/redefine_extname.c
+++ test/Sema/redefine_extname.c
@@ -4,3 +4,4 @@
 #pragma redefine_extname foo_static bar_static
 static int foo_static() { return 1; } // expected-warning {{#pragma redefine_extname is applicable to external C declarations only; not applied to function 'foo_static'}}
 
+unsigned __int128_t; // expected-error {{redefinition of '__int128_t' as different kind of symbol}}
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -4057,7 +4057,8 @@
   }
 
   // Redefinition coming from different files or couldn't do better above.
-  Diag(Old->getLocation(), diag::note_previous_definition);
+  if (Old->getLocation().isValid())
+Diag(Old->getLocation(), diag::note_previous_definition);
 }
 
 /// We've just determined that \p Old and \p New both appear to be definitions
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44559: [Sema] Wrong width of result of mul operation

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D44559#1048399, @lebedev.ri wrote:

> I'm having a very hard time following this thread. In fact, i can't follow 
> the logic at all.
>
> In https://reviews.llvm.org/D44559#1041007, @rjmccall wrote:
>
> > ...
> >  This patch, however, is contrary to the design of -Wconversion, which does 
> > attempt to avoid warning in cases where the user might reasonably see an 
> > operation as "really" being performed in its operand type.  If you want to 
> > argue that we should change the design of -Wconversion, that is a broader 
> > conversation that you should take to cfe-dev.
>
>
> Can you at least point out where the current design goals are documented?


They probably aren't, other than in mailing-list discussions like this.

I remember basing the design off the vision of a white paper that someone 
working on GCC wrote up.  Maybe it was just this wiki page:

  https://gcc.gnu.org/wiki/NewWconversion

But our design was always more sophisticated than GCC's.  Note the comment at 
the bottom of that page about how you should write INT_MIN as (-INT_MAX - 1), 
because otherwise GCC was warning about it!  I don't know how that was ever 
considered acceptable.

Looking for that paper, I did find this GCC bug:

  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=40752

where they had a very similar debate to the one we just had.

The basic tenet of Clang's -Wconversion design, as far as this problem goes, is 
that users generally ought to be able to ignore most of the technical details 
of what C calls for with integer promotions and the types of literals; instead, 
they should generally be able to think of arithmetic as taking place in the 
narrowest promoted type of the opaque operands used in the expression.  If the 
actual computation type doesn't include the full range of that theoretical 
promoted type, that's worth a warning; this can happen when mixing the 
signedness of operands.  But otherwise they can generally think of the 
operation as computing a value of that promoted type.  If that operation 
overflows, so be it — we're not going to warn about the potential for overflow 
every time the user adds two ints, and by the same token, it doesn't make any 
sense to warn about every time the user adds two shorts just because the 
language made this otherwise-unimportant technical decision to do the 
arithmetic in a wider type.


https://reviews.llvm.org/D44559



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

r328567 - [MS] Fix late-parsed template infinite loop in eager instantiation

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Mon Mar 26 11:22:47 2018
New Revision: 328567

URL: http://llvm.org/viewvc/llvm-project?rev=328567&view=rev
Log:
[MS] Fix late-parsed template infinite loop in eager instantiation

Summary:
This fixes PR33561 and PR34185.

Don't store pending template instantiations for late-parsed templates in
the normal PendingInstantiations queue. Instead, use a separate list
that will only be parsed and instantiated at end of TU when late
template parsing actually works and doesn't infinite loop.

Reviewers: rsmith, thakis, hans

Subscribers: cfe-commits

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

Added:
cfe/trunk/test/PCH/late-parsed-instantiations.cpp
cfe/trunk/test/SemaTemplate/late-parsing-eager-instantiation.cpp
Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/Sema.cpp
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=328567&r1=328566&r2=328567&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Mon Mar 26 11:22:47 2018
@@ -7550,6 +7550,10 @@ public:
   /// but have not yet been performed.
   std::deque PendingInstantiations;
 
+  /// Queue of implicit template instantiations that cannot be performed
+  /// eagerly.
+  SmallVector LateParsedInstantiations;
+
   class GlobalEagerInstantiationScope {
   public:
 GlobalEagerInstantiationScope(Sema &S, bool Enabled)

Modified: cfe/trunk/lib/Sema/Sema.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=328567&r1=328566&r2=328567&view=diff
==
--- cfe/trunk/lib/Sema/Sema.cpp (original)
+++ cfe/trunk/lib/Sema/Sema.cpp Mon Mar 26 11:22:47 2018
@@ -850,6 +850,20 @@ void Sema::ActOnEndOfTranslationUnit() {
   if (PP.isCodeCompletionEnabled())
 return;
 
+  // Transfer late parsed template instantiations over to the pending template
+  // instantiation list. During normal compliation, the late template parser
+  // will be installed and instantiating these templates will succeed.
+  //
+  // If we are building a TU prefix for serialization, it is also safe to
+  // transfer these over, even though they are not parsed. The end of the TU
+  // should be outside of any eager template instantiation scope, so when this
+  // AST is deserialized, these templates will not be parsed until the end of
+  // the combined TU.
+  PendingInstantiations.insert(PendingInstantiations.end(),
+   LateParsedInstantiations.begin(),
+   LateParsedInstantiations.end());
+  LateParsedInstantiations.clear();
+
   // Complete translation units and modules define vtables and perform implicit
   // instantiations. PCH files do not.
   if (TUKind != TU_Prefix) {
@@ -879,8 +893,13 @@ void Sema::ActOnEndOfTranslationUnit() {
   PendingInstantiations.insert(PendingInstantiations.begin(),
Pending.begin(), Pending.end());
 }
+
 PerformPendingInstantiations();
 
+assert(LateParsedInstantiations.empty() &&
+   "end of TU template instantiation should not create more "
+   "late-parsed templates");
+
 if (LateTemplateParserCleanup)
   LateTemplateParserCleanup(OpaqueParser);
 

Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=328567&r1=328566&r2=328567&view=diff
==
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Mon Mar 26 11:22:47 2018
@@ -3837,8 +3837,8 @@ void Sema::InstantiateFunctionDefinition
   if (PatternDecl->isLateTemplateParsed() &&
   !LateTemplateParser) {
 Function->setInstantiationIsPending(true);
-PendingInstantiations.push_back(
-  std::make_pair(Function, PointOfInstantiation));
+LateParsedInstantiations.push_back(
+std::make_pair(Function, PointOfInstantiation));
 return;
   }
 

Added: cfe/trunk/test/PCH/late-parsed-instantiations.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/late-parsed-instantiations.cpp?rev=328567&view=auto
==
--- cfe/trunk/test/PCH/late-parsed-instantiations.cpp (added)
+++ cfe/trunk/test/PCH/late-parsed-instantiations.cpp Mon Mar 26 11:22:47 2018
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fdelayed-template-parsing -std=c++14 -emit-pch %s -o 
%t.pch -verify
+// RUN: %clang_cc1 -fdelayed-template-parsing -std=c++14 -include-pch %t.pch 
%s -verify
+
+#ifndef HEADER_INCLUDED
+
+#define HEADER_INCLUDED
+
+// pr33561
+class ArrayBuffer;
+template  

[PATCH] D44846: [MS] Fix late-parsed template infinite loop in eager instantiation

[Reid Kleckner via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL328567: [MS] Fix late-parsed template infinite loop in eager 
instantiation (authored by rnk, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D44846?vs=139663&id=139828#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D44846

Files:
  cfe/trunk/include/clang/Sema/Sema.h
  cfe/trunk/lib/Sema/Sema.cpp
  cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
  cfe/trunk/test/PCH/late-parsed-instantiations.cpp
  cfe/trunk/test/SemaTemplate/late-parsing-eager-instantiation.cpp

Index: cfe/trunk/include/clang/Sema/Sema.h
===
--- cfe/trunk/include/clang/Sema/Sema.h
+++ cfe/trunk/include/clang/Sema/Sema.h
@@ -7550,6 +7550,10 @@
   /// but have not yet been performed.
   std::deque PendingInstantiations;
 
+  /// Queue of implicit template instantiations that cannot be performed
+  /// eagerly.
+  SmallVector LateParsedInstantiations;
+
   class GlobalEagerInstantiationScope {
   public:
 GlobalEagerInstantiationScope(Sema &S, bool Enabled)
Index: cfe/trunk/test/SemaTemplate/late-parsing-eager-instantiation.cpp
===
--- cfe/trunk/test/SemaTemplate/late-parsing-eager-instantiation.cpp
+++ cfe/trunk/test/SemaTemplate/late-parsing-eager-instantiation.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -std=c++14 -verify %s
+
+// pr33561
+class ArrayBuffer;
+template  class Trans_NS_WTF_RefPtr {
+public:
+  ArrayBuffer *operator->() { return nullptr; }
+};
+Trans_NS_WTF_RefPtr get();
+template 
+constexpr void visit(_Visitor __visitor) {
+  __visitor(get()); // expected-note {{in instantiation}}
+}
+class ArrayBuffer {
+  char data() {
+visit([](auto buffer) -> char { // expected-note {{in instantiation}}
+  buffer->data();
+}); // expected-warning {{control reaches end of non-void lambda}}
+  } // expected-warning {{control reaches end of non-void function}}
+};
+
+// pr34185
+template  struct coroutine_handle {
+  Promise &promise() const { return
+*static_cast(nullptr); // expected-warning {{binding dereferenced null}}
+  }
+};
+
+template  auto GetCurrenPromise() {
+  struct Awaiter { // expected-note {{in instantiation}}
+void await_suspend(coroutine_handle h) {
+  h.promise(); // expected-note {{in instantiation}}
+}
+  };
+  return Awaiter{};
+}
+
+void foo() {
+  auto &&p = GetCurrenPromise(); // expected-note {{in instantiation}}
+}
Index: cfe/trunk/test/PCH/late-parsed-instantiations.cpp
===
--- cfe/trunk/test/PCH/late-parsed-instantiations.cpp
+++ cfe/trunk/test/PCH/late-parsed-instantiations.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fdelayed-template-parsing -std=c++14 -emit-pch %s -o %t.pch -verify
+// RUN: %clang_cc1 -fdelayed-template-parsing -std=c++14 -include-pch %t.pch %s -verify
+
+#ifndef HEADER_INCLUDED
+
+#define HEADER_INCLUDED
+
+// pr33561
+class ArrayBuffer;
+template  class Trans_NS_WTF_RefPtr {
+public:
+  ArrayBuffer *operator->() { return nullptr; }
+};
+Trans_NS_WTF_RefPtr get();
+template 
+constexpr void visit(_Visitor __visitor) {
+  __visitor(get()); // expected-note {{in instantiation}}
+}
+class ArrayBuffer {
+  char data() {
+visit([](auto buffer) -> char { // expected-note {{in instantiation}}
+  buffer->data();
+}); // expected-warning {{control reaches end of non-void lambda}}
+  } // expected-warning {{control reaches end of non-void function}}
+};
+
+#else
+
+// expected-no-diagnostics
+
+#endif
Index: cfe/trunk/lib/Sema/Sema.cpp
===
--- cfe/trunk/lib/Sema/Sema.cpp
+++ cfe/trunk/lib/Sema/Sema.cpp
@@ -850,6 +850,20 @@
   if (PP.isCodeCompletionEnabled())
 return;
 
+  // Transfer late parsed template instantiations over to the pending template
+  // instantiation list. During normal compliation, the late template parser
+  // will be installed and instantiating these templates will succeed.
+  //
+  // If we are building a TU prefix for serialization, it is also safe to
+  // transfer these over, even though they are not parsed. The end of the TU
+  // should be outside of any eager template instantiation scope, so when this
+  // AST is deserialized, these templates will not be parsed until the end of
+  // the combined TU.
+  PendingInstantiations.insert(PendingInstantiations.end(),
+   LateParsedInstantiations.begin(),
+   LateParsedInstantiations.end());
+  LateParsedInstantiations.clear();
+
   // Complete translation units and modules define vtables and perform implicit
   // instantiations. PCH files do not.
   if (TUKind != TU_Prefix) {
@@ -879,8 +893,13 @@
   PendingInstantiations.insert(PendingInstantiations.begin(),
Pending.begin(), 

[PATCH] D44846: [MS] Fix late-parsed template infinite loop in eager instantiation

[Reid Kleckner via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC328567: [MS] Fix late-parsed template infinite loop in eager 
instantiation (authored by rnk, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D44846?vs=139663&id=139827#toc

Repository:
  rC Clang

https://reviews.llvm.org/D44846

Files:
  include/clang/Sema/Sema.h
  lib/Sema/Sema.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  test/PCH/late-parsed-instantiations.cpp
  test/SemaTemplate/late-parsing-eager-instantiation.cpp

Index: lib/Sema/Sema.cpp
===
--- lib/Sema/Sema.cpp
+++ lib/Sema/Sema.cpp
@@ -850,6 +850,20 @@
   if (PP.isCodeCompletionEnabled())
 return;
 
+  // Transfer late parsed template instantiations over to the pending template
+  // instantiation list. During normal compliation, the late template parser
+  // will be installed and instantiating these templates will succeed.
+  //
+  // If we are building a TU prefix for serialization, it is also safe to
+  // transfer these over, even though they are not parsed. The end of the TU
+  // should be outside of any eager template instantiation scope, so when this
+  // AST is deserialized, these templates will not be parsed until the end of
+  // the combined TU.
+  PendingInstantiations.insert(PendingInstantiations.end(),
+   LateParsedInstantiations.begin(),
+   LateParsedInstantiations.end());
+  LateParsedInstantiations.clear();
+
   // Complete translation units and modules define vtables and perform implicit
   // instantiations. PCH files do not.
   if (TUKind != TU_Prefix) {
@@ -879,8 +893,13 @@
   PendingInstantiations.insert(PendingInstantiations.begin(),
Pending.begin(), Pending.end());
 }
+
 PerformPendingInstantiations();
 
+assert(LateParsedInstantiations.empty() &&
+   "end of TU template instantiation should not create more "
+   "late-parsed templates");
+
 if (LateTemplateParserCleanup)
   LateTemplateParserCleanup(OpaqueParser);
 
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -3837,8 +3837,8 @@
   if (PatternDecl->isLateTemplateParsed() &&
   !LateTemplateParser) {
 Function->setInstantiationIsPending(true);
-PendingInstantiations.push_back(
-  std::make_pair(Function, PointOfInstantiation));
+LateParsedInstantiations.push_back(
+std::make_pair(Function, PointOfInstantiation));
 return;
   }
 
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -7550,6 +7550,10 @@
   /// but have not yet been performed.
   std::deque PendingInstantiations;
 
+  /// Queue of implicit template instantiations that cannot be performed
+  /// eagerly.
+  SmallVector LateParsedInstantiations;
+
   class GlobalEagerInstantiationScope {
   public:
 GlobalEagerInstantiationScope(Sema &S, bool Enabled)
Index: test/SemaTemplate/late-parsing-eager-instantiation.cpp
===
--- test/SemaTemplate/late-parsing-eager-instantiation.cpp
+++ test/SemaTemplate/late-parsing-eager-instantiation.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -std=c++14 -verify %s
+
+// pr33561
+class ArrayBuffer;
+template  class Trans_NS_WTF_RefPtr {
+public:
+  ArrayBuffer *operator->() { return nullptr; }
+};
+Trans_NS_WTF_RefPtr get();
+template 
+constexpr void visit(_Visitor __visitor) {
+  __visitor(get()); // expected-note {{in instantiation}}
+}
+class ArrayBuffer {
+  char data() {
+visit([](auto buffer) -> char { // expected-note {{in instantiation}}
+  buffer->data();
+}); // expected-warning {{control reaches end of non-void lambda}}
+  } // expected-warning {{control reaches end of non-void function}}
+};
+
+// pr34185
+template  struct coroutine_handle {
+  Promise &promise() const { return
+*static_cast(nullptr); // expected-warning {{binding dereferenced null}}
+  }
+};
+
+template  auto GetCurrenPromise() {
+  struct Awaiter { // expected-note {{in instantiation}}
+void await_suspend(coroutine_handle h) {
+  h.promise(); // expected-note {{in instantiation}}
+}
+  };
+  return Awaiter{};
+}
+
+void foo() {
+  auto &&p = GetCurrenPromise(); // expected-note {{in instantiation}}
+}
Index: test/PCH/late-parsed-instantiations.cpp
===
--- test/PCH/late-parsed-instantiations.cpp
+++ test/PCH/late-parsed-instantiations.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fdelayed-template-parsing -std=c++14 -emit-pch %s -o %t.pch -verify
+// RUN: %clang_cc1 -fdelayed-template-parsing -std=c++14 -include-pch %t.pch %s -verify
+

[PATCH] D44883: [Sema] Extend -Wself-assign with -Wself-assign-overloaded to warn on overloaded self-assignment (classes)

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D44883#1048439, @lebedev.ri wrote:

> In https://reviews.llvm.org/D44883#1048421, @Quuxplusone wrote:
>
> > Two more high-level comments from me, and then I'll try to butt out.
> >
> > Q1. I don't understand what `field-` is doing in the name of some 
> > diagnostics. Is there a situation where `x = x;` is "less/more of an error" 
> > just because `x` is a {data member, lambda capture, structured binding} 
> > versus not? Why should the "plain old variable" case be separately 
> > toggleable? (This question pre-dates Roman's proposed patch AFAICT. I think 
> > the answer is that self-assignment specifically //of a data member// 
> > specifically //in a constructor// is considered "more of an error" because 
> > it's likely to be a very particular kind of typo-bug. I doubt this answer 
> > is quite strong enough to justify multiple warning options, though.)
>
>
> I think we should ask @thakis that - https://reviews.llvm.org/rL159394
>
> > Q2. I don't understand why `x &= x` (and now `x /= x`, thanks) should be 
> > treated as "more of an error" than `x & x` (and now `x / x`), or for that 
> > matter `x == x`. The sin here is not "self-assignment" per se; it's 
> > "over-complicating a tautological mathematical operation." I admit I 
> > haven't thought about it as much as you folks, but the more I think about 
> > it, the more I think the only consistent thing for Clang to do would be to 
> > back off from warning about anything beyond plain old `x = x`, and leave 
> > *all* the mathematical-tautology cases to checkers like clang-tidy and 
> > PVS-Studio.


It's fine to warn about really obvious tautologies in the compiler.  We don't 
need to go much further than this, though.

> In https://reviews.llvm.org/D44883#1048400, @rjmccall wrote:
> 
>> Yeah, like I said, I'm just worried about the usability of having multiple 
>> dimensions of warning group here.  Like, do we need both 
>> -Wself-assign-field-builtin and -Wself-assign-field-overloaded?
> 
> 
> I'm not saying that is not a valid concern. I'm simply following the 
> pre-existing practice, which is, as far i'm aware, to split the diag groups 
> if it makes sense.

I agree.  In general, I think this would be fine; my only concern is because we 
do already have some splitting along a different dimension, so we do need to 
stop and think about it.  Maybe the answer is that

>> Because I do think we should warn on field self-assignments for user-defined 
>> operators.
> 
> I agree, as i said, i did not want to put everything into one bit 
> differential.

I don't think it would over-complicate this patch to handle all the cases at 
once.

>> I think the term ought to be "user-defined" rather than "overloaded", by the 
>> way.  In a sense, these operators aren't really any more overloaded than the 
>> builtin operators (at least, not necessarily so) — C++ even formalizes the 
>> builtin operators as being a large family of overloads.  And the reason we 
>> want to treat them specially is that they have user-provided definitions 
>> that might be doing special things, not because of anything to do with name 
>> resolution.
> 
> Can you elaborate a bit more, given this struct, which of these variants 
> should be diagnosed as user-provided, and which as builtin?

That's an interesting question!  You're absolutely right, I do think we should 
warn about trivial C++ assignments as part of the builtin group.  If a C 
program includes a self-assignment that happens to be of struct type, we'll 
warn about that, and it seems to be that we shouldn't lose the warning just 
because that code is compiled as C++ instead.

As for your example, I think it should be based on whether the assignment 
operator selected is trivial.  If it's non-trivial, even if it's defaulted, we 
should warn about it as a "user-defined" operator, since ultimately it does 
involve calling such an operator.


Repository:
  rC Clang

https://reviews.llvm.org/D44883



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

[PATCH] D44883: [Sema] Extend -Wself-assign with -Wself-assign-overloaded to warn on overloaded self-assignment (classes)

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D44883#1048485, @rjmccall wrote:

> In https://reviews.llvm.org/D44883#1048439, @lebedev.ri wrote:
>
> > In https://reviews.llvm.org/D44883#1048400, @rjmccall wrote:
> >
> > > Yeah, like I said, I'm just worried about the usability of having 
> > > multiple dimensions of warning group here.  Like, do we need both 
> > > -Wself-assign-field-builtin and -Wself-assign-field-overloaded?
> >
> >
> > I'm not saying that is not a valid concern. I'm simply following the 
> > pre-existing practice, which is, as far i'm aware, to split the diag groups 
> > if it makes sense.
>
>
> I agree.  In general, I think this would be fine; my only concern is because 
> we do already have some splitting along a different dimension, so we do need 
> to stop and think about it.  Maybe the answer is that


Sorry, I didn't finish this thought.  Maybe we should just treat 
-Wself-assign-field as an accident of history; we can roll it generally into 
-Wself-assign and just allow that specific warning to be separately enabled if 
they want.  Or if it isn't a very old flag, maybe we can just remove it 
entirely.


Repository:
  rC Clang

https://reviews.llvm.org/D44883



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

[PATCH] D44815: [AArch64]: Add support for parsing rN registers.

[Peter Smith via Phabricator via cfe-commits]

peter.smith added a comment.

I think that we may have a bit of a conceptual difference with GCC here. As far 
as I can tell in GCC it doesn't matter if "rn", "wn", "xn" or even "bn" is 
used, this refers to register 0. The Operand constraint such as %w0 is used to 
control the substitution so the r registers aren't strictly aliases like the 
other registers. The comment

> // The S/D/Q and W/X registers overlap, but aren't really aliases; we 
>  //don't want to substitute one of these for a different-sized one.

suggests that this may have been the intent that Clang behave differently to 
GCC in this respect. For example in Clang there appears to be a bit more 
significance placed on the "wn" or "xn", for example clang will warn in the 
following example, whereas gcc will not:

  long f2(int i) {
  register int x asm("w6");
  register int y asm("w7");
  register int z asm("w8");
  asm("add %0, %1, %2\n": "=r"(x) : "r"(y), "r"(z));
  return y;
  }
  
  t2.c:5:34: warning: value size does not match register size specified by the
constraint and modifier [-Wasm-operand-widths]
  asm("add %0, %1, %2\n": "=r"(x) : "r"(y), "r"(z));
   ^~
   %w0
  // and so on for %1 and %2.   

Having said all that, clang will not warn when the operand modifier is 
explicit, for example:

  long f2(int i) {
  register long x asm("w6");
  register long y asm("w7");
  register long z asm("w8");
  asm("add %w0, %w1, %w2\n": "=r"(x) : "r"(y), "r"(z));
  return y;
  }

At the moment I'm not too sure what to make of this. Strictly speaking I think 
"rn" is an unsized register that Clang shouldn't warn about size mismatch in 
the operand modifier. With the implementation above it won't because we default 
to xn and clang doesn't seem to warn when there is an explicit modifier for wn, 
although this could change in the future.

I'd be very interested to hear other opinions on this? If register rn is to be 
supported perhaps a test that clang doesn't warn when asm("rn") is used with 
the operand modifier %wn is used.


Repository:
  rC Clang

https://reviews.llvm.org/D44815



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

[PATCH] D42938: [Sema] Emit -Winteger-overflow for arguments in function calls, ObjC messages.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Ping for reviewers.


https://reviews.llvm.org/D42938



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

[PATCH] D44883: [Sema] Extend -Wself-assign with -Wself-assign-overloaded to warn on overloaded self-assignment (classes)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Ok, once we understand why "field" diag is separate, and why since initial 
implementation it only warned for "builtin" operators, i'll work on the code 
further.

In https://reviews.llvm.org/D44883#1048485, @rjmccall wrote:

> That's an interesting question!  You're absolutely right, I do think we 
> should warn about trivial C++ assignments as part of the builtin group.  If a 
> C program includes a self-assignment that happens to be of struct type, we'll 
> warn about that, and it seems to be that we shouldn't lose the warning just 
> because that code is compiled as C++ instead.
>
> As for your example, I think it should be based on whether the assignment 
> operator selected is trivial.  If it's non-trivial, even if it's defaulted, 
> we should warn about it as a "user-defined" operator, since ultimately it 
> does involve calling such an operator.


To summarize:
https://godbolt.org/g/gwDASe - `A`, `B` and `C` case should be treated as 
built-in, and `D`, `E` and `F` as user-defined.


Repository:
  rC Clang

https://reviews.llvm.org/D44883



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

[PATCH] D44449: [Parser] Fix assertion-on-invalid for unexpected typename.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Ping.


https://reviews.llvm.org/D9



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

[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis updated this revision to Diff 139834.
zinovy.nis added a comment.

- Updated warning message to



> warning: qualified name 'A::foo' refers to a member overridden in subclass; 
> did you mean 'B'? [bugprone-parent-virtual-call]




https://reviews.llvm.org/D44295

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/ParentVirtualCallCheck.cpp
  clang-tidy/bugprone/ParentVirtualCallCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-parent-virtual-call.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-parent-virtual-call.cpp

Index: test/clang-tidy/bugprone-parent-virtual-call.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-parent-virtual-call.cpp
@@ -0,0 +1,177 @@
+// RUN: %check_clang_tidy %s bugprone-parent-virtual-call %t
+
+extern int foo();
+
+class A {
+public:
+  A() = default;
+  virtual ~A() = default;
+
+  virtual int virt_1() { return foo() + 1; }
+  virtual int virt_2() { return foo() + 2; }
+
+  int non_virt() { return foo() + 3; }
+  static int stat() { return foo() + 4; }
+};
+
+class B : public A {
+public:
+  B() = default;
+
+  // Nothing to fix: calls to direct parent.
+  int virt_1() override { return A::virt_1() + 3; }
+  int virt_2() override { return A::virt_2() + 4; }
+};
+
+class C : public B {
+public:
+  int virt_1() override { return A::virt_1() + B::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' refers to a member overridden in subclass; did you mean 'B'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return B::virt_1() + B::virt_1(); }
+  int virt_2() override { return A::virt_1() + B::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' refers to a member overridden in subclass; did you mean 'B'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_2() override { return B::virt_1() + B::virt_1(); }
+
+  // Test that non-virtual and static methods are not affected by this cherker.
+  int method_c() { return A::stat() + A::non_virt(); }
+};
+
+// Check aliased type names
+using A1 = A;
+typedef A A2;
+
+class C2 : public B {
+public:
+  int virt_1() override { return A1::virt_1() + A2::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' refers to a member overridden in subclass; did you mean 'B'? [bugprone-parent-virtual-call]
+  // CHECK-MESSAGES: :[[@LINE-2]]:49: warning: qualified name 'A::virt_1' refers to a member overridden in subclass; did you mean 'B'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return B::virt_1() + B::virt_1(); }
+};
+
+// Test that the check affects grand-grand..-parent calls too.
+class D : public C {
+public:
+  int virt_1() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' refers to a member overridden in subclass; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: qualified name 'B::virt_1' refers to a member overridden in subclass; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+  int virt_2() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' refers to a member overridden in subclass; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: qualified name 'B::virt_1' refers to a member overridden in subclass; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_2() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+};
+
+// Test classes in namespaces.
+namespace {
+class BN : public A {
+public:
+  int virt_1() override { return A::virt_1() + 3; }
+  int virt_2() override { return A::virt_2() + 4; }
+};
+} // namespace
+
+namespace N1 {
+class A {
+public:
+  A() = default;
+  virtual int virt_1() { return foo() + 1; }
+  virtual int virt_2() { return foo() + 2; }
+};
+} // namespace N1
+
+namespace N2 {
+class BN : public N1::A {
+public:
+  int virt_1() override { return A::virt_1() + 3; }
+  int virt_2() override { return A::virt_2() + 4; }
+};
+} // namespace N2
+
+class CN : public BN {
+public:
+  int virt_1() override { return A::virt_1() + BN::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' refers to a member overridden in subclass; did you mean 'BN'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return BN::virt_1() + BN::virt_1(); }
+  int virt_2() override { return A::virt_1() + BN::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' refers to a member overridden in subclass; did you mean 'BN'? [bugprone-parent-virtual

[PATCH] D44883: [Sema] Extend -Wself-assign with -Wself-assign-overloaded to warn on overloaded self-assignment (classes)

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Is D not considered trivial there?  Ugh, C++.

Well, we certainly could justify treating D as a builtin operator as well, 
since it doesn't involve running any user code, but I don't think you're 
obligated to write a whole bunch of analysis just to make that work if the 
information isn't already easily accessible.


Repository:
  rC Clang

https://reviews.llvm.org/D44883



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

[PATCH] D44823: [libcxx] Improving std::vector and std::deque perfomance

[Zhihao Yuan via Phabricator via cfe-commits]

lichray added inline comments.



Comment at: libcxx/trunk/include/__split_buffer:201
 __alloc_rr& __a = this->__alloc();
+pointer __to_be_end = this->__end_;
 do

mclow.lists wrote:
> I have been asked specifically by the optimizer folks to NOT do things like 
> this in libc++, but rather to file bugs against the optimizer.
> 
> And I have done so for this exact case:  
> https://bugs.llvm.org/show_bug.cgi?id=35637
From the thread I didn't see that the compiler side asked you not to do so.

And I disagree with the view.  libc++ shouldn't *wait* for compilers, because 
we don't dictate users' compiler choices.  This change doesn't make libc++ 
worse to coming compilers, and makes libc++ better on existing compilers, so 
what benefit we get by not approving this?


Repository:
  rCXX libc++

https://reviews.llvm.org/D44823



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

[PATCH] D44906: [clang-tidy] Define __clang_analyzer__ macro for clang-tidy for compatibility with clang static analyzer

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis created this revision.
zinovy.nis added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, a.sidorin, xazax.hun.
Herald added a reviewer: george.karpenkov.

This macro is widely used in many well-known projects, ex. Chromium 
.

But it's not set for clang-tidy, so for ex. DCHECK in Chromium is not 
considered as [[no-return]], and a lot of false-positive warnings about nullptr 
dereferenced are emitted.

This patch fixes the issue by implicitly added macro definition.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44906

Files:
  clang-tidy/ClangTidy.cpp
  test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp


Index: test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp
===
--- /dev/null
+++ test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp
@@ -0,0 +1,8 @@
+// RUN: %check_clang_tidy %s * %t
+
+#if defined(__clang_analyzer__)
+#warning __clang_analyzer__ is defined
+#endif
+// CHECK-MESSAGES: :[[@LINE-2]]:2: warning: __clang_analyzer__ is defined 
[clang-diagnostic-#warnings]
+
+
Index: clang-tidy/ClangTidy.cpp
===
--- clang-tidy/ClangTidy.cpp
+++ clang-tidy/ClangTidy.cpp
@@ -481,6 +481,16 @@
   ClangTool Tool(Compilations, InputFiles,
  std::make_shared(), BaseFS);
 
+  // Add __clang_analyzer__ macro definition for compatibility with the clang
+  // static analyzer.
+  ArgumentsAdjuster ClangTidyMacroDefinitionInserter =
+  [&Context](const CommandLineArguments &Args, StringRef Filename) {
+ClangTidyOptions Opts = Context.getOptionsForFile(Filename);
+CommandLineArguments AdjustedArgs = Args;
+AdjustedArgs.emplace_back("-D__clang_analyzer__");
+return AdjustedArgs;
+  };
+
   // Add extra arguments passed by the clang-tidy command-line.
   ArgumentsAdjuster PerFileExtraArgumentsInserter =
   [&Context](const CommandLineArguments &Args, StringRef Filename) {
@@ -515,6 +525,7 @@
 return AdjustedArgs;
   };
 
+  Tool.appendArgumentsAdjuster(ClangTidyMacroDefinitionInserter);
   Tool.appendArgumentsAdjuster(PerFileExtraArgumentsInserter);
   Tool.appendArgumentsAdjuster(PluginArgumentsRemover);
   if (Profile)


Index: test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp
===
--- /dev/null
+++ test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp
@@ -0,0 +1,8 @@
+// RUN: %check_clang_tidy %s * %t
+
+#if defined(__clang_analyzer__)
+#warning __clang_analyzer__ is defined
+#endif
+// CHECK-MESSAGES: :[[@LINE-2]]:2: warning: __clang_analyzer__ is defined [clang-diagnostic-#warnings]
+
+
Index: clang-tidy/ClangTidy.cpp
===
--- clang-tidy/ClangTidy.cpp
+++ clang-tidy/ClangTidy.cpp
@@ -481,6 +481,16 @@
   ClangTool Tool(Compilations, InputFiles,
  std::make_shared(), BaseFS);
 
+  // Add __clang_analyzer__ macro definition for compatibility with the clang
+  // static analyzer.
+  ArgumentsAdjuster ClangTidyMacroDefinitionInserter =
+  [&Context](const CommandLineArguments &Args, StringRef Filename) {
+ClangTidyOptions Opts = Context.getOptionsForFile(Filename);
+CommandLineArguments AdjustedArgs = Args;
+AdjustedArgs.emplace_back("-D__clang_analyzer__");
+return AdjustedArgs;
+  };
+
   // Add extra arguments passed by the clang-tidy command-line.
   ArgumentsAdjuster PerFileExtraArgumentsInserter =
   [&Context](const CommandLineArguments &Args, StringRef Filename) {
@@ -515,6 +525,7 @@
 return AdjustedArgs;
   };
 
+  Tool.appendArgumentsAdjuster(ClangTidyMacroDefinitionInserter);
   Tool.appendArgumentsAdjuster(PerFileExtraArgumentsInserter);
   Tool.appendArgumentsAdjuster(PluginArgumentsRemover);
   if (Profile)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43871: [modules] No longer include stdlib.h from mm_malloc.h.

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc resigned from this revision.
chandlerc added a comment.

I think this is more a question for Richard... Add me back to the reviewers if 
there is a specific need for my input here.


Repository:
  rC Clang

https://reviews.llvm.org/D43871



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

[PATCH] D44330: CMake option to allow enabling experimental new pass manager by default

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc requested changes to this revision.
chandlerc added a comment.
This revision now requires changes to proceed.

Just a question really.




Comment at: clang/CMakeLists.txt:215
 
+set(ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER FALSE CACHE BOOL
+  "Enable the experimental new pass manager by default.")

Does it make sense to put the word `DEFAULT` into the variable name? I don't 
feel strongly either way.


Repository:
  rC Clang

https://reviews.llvm.org/D44330



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

[PATCH] D42787: clang-format: do not add extra indent when wrapping last parameter

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc resigned from this revision.
chandlerc added a comment.

Since this seems not going anywhere, removing it from my review dashboard.


Repository:
  rC Clang

https://reviews.llvm.org/D42787



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

[PATCH] D44908: [ObjC++] Make parameter passing and function return compatible with ObjC

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak created this revision.
ahatanak added reviewers: rjmccall, doug.gregor, rsmith.

r326307 and r327870 made changes that enable __strong and __weak fields to be 
declared in C structs. This patch changes the ObjC++ ABI so that structs with 
__strong or __weak pointers can be passed to or returned from functions 
compiled in C mode.

ObjC and ObjC++ are currently incompatible because of the differences in the 
way structs are passed and destructed.

For example:

  typedef struct {
id f0;
__weak id f1;
  } S;
  
  // this code is compiled in c++.
  extern “C” {
void foo(S s);
  }
  
  void caller(S a) {
foo(a); // clang currently passes 'a' indirectly and the caller destructs 
'a'.
  }
  
  // this function is compiled in c.
  void foo(S a) {  // 'a' is passed directly and is destructed in the callee.
  }

This patch fixes the incompatibility by passing and returning structs with 
__strong or __weak fields using the C ABI in C++ mode: __strong and __weak 
fields in a struct do not cause the struct to be destructed in the caller and 
__strong fields do not make the struct to be passed indirectly.

Also, this patch fixes the microsoft ABI bug mentioned here:

https://reviews.llvm.org/D41039?id=128767#inline-364710


Repository:
  rC Clang

https://reviews.llvm.org/D44908

Files:
  include/clang/AST/Decl.h
  include/clang/AST/DeclCXX.h
  include/clang/AST/Type.h
  lib/AST/ASTContext.cpp
  lib/AST/Decl.cpp
  lib/AST/DeclCXX.cpp
  lib/AST/Type.cpp
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/CodeGen/MicrosoftCXXABI.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriter.cpp
  lib/Serialization/ASTWriterDecl.cpp
  test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
  test/CodeGenObjCXX/arc-special-member-functions.mm
  test/CodeGenObjCXX/objc-struct-cxx-abi.mm
  test/CodeGenObjCXX/property-dot-copy-elision.mm
  test/CodeGenObjCXX/trivial_abi.mm

Index: test/CodeGenObjCXX/property-dot-copy-elision.mm
===
--- test/CodeGenObjCXX/property-dot-copy-elision.mm
+++ test/CodeGenObjCXX/property-dot-copy-elision.mm
@@ -1,11 +1,13 @@
 // RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -std=c++1z -fobjc-arc -o - %s | FileCheck %s
 
 struct S0 {
+  ~S0();
   id f;
 };
 
 struct S1 {
   S1();
+  ~S1();
   S1(S0);
   id f;
 };
Index: test/CodeGenObjCXX/objc-struct-cxx-abi.mm
===
--- test/CodeGenObjCXX/objc-struct-cxx-abi.mm
+++ test/CodeGenObjCXX/objc-struct-cxx-abi.mm
@@ -1,27 +1,60 @@
 // RUN: %clang_cc1 -triple arm64-apple-ios11 -std=c++11 -fobjc-arc  -fobjc-weak -fobjc-runtime-has-weak -emit-llvm -o - %s | FileCheck %s
 // RUN: %clang_cc1 -triple arm64-apple-ios11 -std=c++11 -fobjc-arc  -fobjc-weak -fobjc-runtime-has-weak -fclang-abi-compat=4.0 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple arm64-apple-ios11 -std=c++11 -fobjc-arc  -fobjc-weak -fobjc-runtime-has-weak -emit-llvm -o - -DTRIVIALABI %s | FileCheck %s
+// RUN: %clang_cc1 -triple arm64-apple-ios11 -std=c++11 -fobjc-arc  -fobjc-weak -fobjc-runtime-has-weak -fclang-abi-compat=4.0 -emit-llvm -o - -DTRIVIALABI %s | FileCheck %s
+
+// Check that structs consisting solely of __strong or __weak pointer fields are
+// destructed in the callee function and structs consisting solely of __strong
+// pointer fields are passed directly.
 
 // CHECK: %[[STRUCT_STRONGWEAK:.*]] = type { i8*, i8* }
 // CHECK: %[[STRUCT_STRONG:.*]] = type { i8* }
 // CHECK: %[[STRUCT_S:.*]] = type { i8* }
+// CHECK: %[[STRUCT_CONTAINSNONTRIVIAL:.*]] = type { %{{.*}}, i8* }
 
+#ifdef TRIVIALABI
 struct __attribute__((trivial_abi)) StrongWeak {
+#else
+struct StrongWeak {
+#endif
   id fstrong;
   __weak id fweak;
 };
 
+#ifdef TRIVIALABI
 struct __attribute__((trivial_abi)) Strong {
+#else
+struct Strong {
+#endif
   id fstrong;
 };
 
 template
+#ifdef TRIVIALABI
 struct __attribute__((trivial_abi)) S {
+#else
+struct S {
+#endif
   T a;
 };
 
+struct NonTrivial {
+  NonTrivial();
+  NonTrivial(const NonTrivial &);
+  ~NonTrivial();
+  int *a;
+};
+
+// This struct is not passed directly nor destructed in the callee because f0
+// has type NonTrivial.
+struct ContainsNonTrivial {
+  NonTrivial f0;
+  id f1;
+};
+
 // CHECK: define void @_Z19testParamStrongWeak10StrongWeak(%[[STRUCT_STRONGWEAK]]* %{{.*}})
-// CHECK-NOT: call
-// CHECK: ret void
+// CHECK: call %struct.StrongWeak* @_ZN10StrongWeakD1Ev(
+// CHECK-NEXT: ret void
 
 void testParamStrongWeak(StrongWeak a) {
 }
@@ -33,7 +66,7 @@
 // CHECK: %[[V0:.*]] = load %[[STRUCT_STRONGWEAK]]*, %[[STRUCT_STRONGWEAK]]** %[[A_ADDR]], align 8
 // CHECK: %[[CALL:.*]] = call %[[STRUCT_STRONGWEAK]]* @_ZN10StrongWeakC1ERKS_(%[[STRUCT_STRONGWEAK]]* %[[AGG_TMP]], %[[STRUCT_STRONGWEAK]]* dereferenceable(16) %[[V0]])
 // CHECK: call void @_Z19testParamStrongWeak10StrongWeak(%[[STRUCT_STRONGWEAK]]* %[[AGG_TMP]])

[PATCH] D44908: [ObjC++] Make parameter passing and function return compatible with ObjC

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

I tried to move the code in MicrosoftCXXABI::getRecordArgABI and 
ItaniumCXXABI::passClassIndirect that determine whether a struct is passed 
indirectly to Sema::CheckCompletedCXXClass, but couldn't do so since 
getClangABICompat() is a method of CodeGenOpts, which Sema doesn't have access 
to. This would allow removing ForcePassIndirectly.


Repository:
  rC Clang

https://reviews.llvm.org/D44908



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

[PATCH] D44362: [clang] Change std::sort to llvm::sort in response to r327219

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm


Repository:
  rC Clang

https://reviews.llvm.org/D44362



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

r328584 - [Frontend] Fix some Clang-tidy modernize and Include What You Use warnings; other minor fixes (NFC).

[Eugene Zelenko via cfe-commits]

Author: eugenezelenko
Date: Mon Mar 26 14:45:04 2018
New Revision: 328584

URL: http://llvm.org/viewvc/llvm-project?rev=328584&view=rev
Log:
[Frontend] Fix some Clang-tidy modernize and Include What You Use warnings; 
other minor fixes (NFC).

Modified:
cfe/trunk/include/clang/Frontend/CompilerInvocation.h
cfe/trunk/include/clang/Frontend/FrontendOptions.h
cfe/trunk/include/clang/Frontend/SerializedDiagnosticReader.h
cfe/trunk/include/clang/Frontend/Utils.h
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Frontend/FrontendOptions.cpp
cfe/trunk/lib/Frontend/SerializedDiagnosticReader.cpp

Modified: cfe/trunk/include/clang/Frontend/CompilerInvocation.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CompilerInvocation.h?rev=328584&r1=328583&r2=328584&view=diff
==
--- cfe/trunk/include/clang/Frontend/CompilerInvocation.h (original)
+++ cfe/trunk/include/clang/Frontend/CompilerInvocation.h Mon Mar 26 14:45:04 
2018
@@ -1,4 +1,4 @@
-//===-- CompilerInvocation.h - Compiler Invocation Helper Data --*- C++ 
-*-===//
+//===- CompilerInvocation.h - Compiler Invocation Helper Data ---*- C++ 
-*-===//
 //
 // The LLVM Compiler Infrastructure
 //
@@ -7,11 +7,12 @@
 //
 
//===--===//
 
-#ifndef LLVM_CLANG_FRONTEND_COMPILERINVOCATION_H_
-#define LLVM_CLANG_FRONTEND_COMPILERINVOCATION_H_
+#ifndef LLVM_CLANG_FRONTEND_COMPILERINVOCATION_H
+#define LLVM_CLANG_FRONTEND_COMPILERINVOCATION_H
 
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/LLVM.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Frontend/CodeGenOptions.h"
 #include "clang/Frontend/DependencyOutputOptions.h"
@@ -21,23 +22,27 @@
 #include "clang/Frontend/PreprocessorOutputOptions.h"
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include 
 #include 
 
 namespace llvm {
+
 class Triple;
 
 namespace opt {
+
 class ArgList;
-}
-}
+
+} // namespace opt
+
+} // namespace llvm
 
 namespace clang {
-class PreprocessorOptions;
+
+class DiagnosticsEngine;
 class HeaderSearchOptions;
+class PreprocessorOptions;
 class TargetOptions;
-class LangOptions;
-class CompilerInvocation;
-class DiagnosticsEngine;
 
 /// \brief Fill out Opts based on the options given in Args.
 ///
@@ -52,8 +57,6 @@ bool ParseDiagnosticArgs(DiagnosticOptio
  bool DefaultShowOpt = true);
 
 class CompilerInvocationBase {
-  void operator=(const CompilerInvocationBase &) = delete;
-
 public:
   /// Options controlling the language variant.
   std::shared_ptr LangOpts;
@@ -71,24 +74,24 @@ public:
   std::shared_ptr PreprocessorOpts;
 
   CompilerInvocationBase();
-  ~CompilerInvocationBase();
-
   CompilerInvocationBase(const CompilerInvocationBase &X);
+  CompilerInvocationBase &operator=(const CompilerInvocationBase &) = delete;
+  ~CompilerInvocationBase();
 
   LangOptions *getLangOpts() { return LangOpts.get(); }
   const LangOptions *getLangOpts() const { return LangOpts.get(); }
 
   TargetOptions &getTargetOpts() { return *TargetOpts.get(); }
-  const TargetOptions &getTargetOpts() const {
-return *TargetOpts.get();
-  }
+  const TargetOptions &getTargetOpts() const { return *TargetOpts.get(); }
 
   DiagnosticOptions &getDiagnosticOpts() const { return *DiagnosticOpts; }
 
   HeaderSearchOptions &getHeaderSearchOpts() { return *HeaderSearchOpts; }
+
   const HeaderSearchOptions &getHeaderSearchOpts() const {
 return *HeaderSearchOpts;
   }
+
   std::shared_ptr getHeaderSearchOptsPtr() const {
 return HeaderSearchOpts;
   }
@@ -96,7 +99,9 @@ public:
   std::shared_ptr getPreprocessorOptsPtr() {
 return PreprocessorOpts;
   }
+
   PreprocessorOptions &getPreprocessorOpts() { return *PreprocessorOpts; }
+
   const PreprocessorOptions &getPreprocessorOpts() const {
 return *PreprocessorOpts;
   }
@@ -176,40 +181,35 @@ public:
   /// @name Option Subgroups
   /// @{
 
-  AnalyzerOptionsRef getAnalyzerOpts() const {
-return AnalyzerOpts;
-  }
+  AnalyzerOptionsRef getAnalyzerOpts() const { return AnalyzerOpts; }
 
   MigratorOptions &getMigratorOpts() { return MigratorOpts; }
-  const MigratorOptions &getMigratorOpts() const {
-return MigratorOpts;
-  }
+  const MigratorOptions &getMigratorOpts() const { return MigratorOpts; }
   
   CodeGenOptions &getCodeGenOpts() { return CodeGenOpts; }
-  const CodeGenOptions &getCodeGenOpts() const {
-return CodeGenOpts;
-  }
+  const CodeGenOptions &getCodeGenOpts() const { return CodeGenOpts; }
 
   DependencyOutputOptions &getDependencyOutputOpts() {
 return DependencyOutputOpts;
   }
+
   const DependencyOutputOptions &getDependencyOutputOpts() const {
 return DependencyOutputOpts;
   }
 
   FileSystemOptions &getFileSystemOpts() { return FileSyste

[PATCH] D44883: [Sema] Extend -Wself-assign with -Wself-assign-overloaded to warn on overloaded self-assignment (classes)

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I tracked Chandler down, and he doesn't remember why user-defined operators are 
excluded.


Repository:
  rC Clang

https://reviews.llvm.org/D44883



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

[PATCH] D42938: [Sema] Emit -Winteger-overflow for arguments in function calls, ObjC messages.

[Jan Korous via Phabricator via cfe-commits]

jkorous-apple accepted this revision.
jkorous-apple added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D42938



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

[PATCH] D44883: [Sema] Extend -Wself-assign with -Wself-assign-overloaded to warn on overloaded self-assignment (classes)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D44883#1048689, @rjmccall wrote:

> I tracked Chandler down, and he doesn't remember why user-defined operators 
> are excluded.


Ok then. Unless reviewers think it would be best to have separate diag groups 
for "builtin" and "user-provided" binary operators,
i'll just extend the `-Wself-assign` / `-Wself-assign-field` / `-Wself-move` (i 
wonder, does it work both the fields and usual variables, or not).

(That means, stage2 self-hosting testing will be needed...)


Repository:
  rC Clang

https://reviews.llvm.org/D44883



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

[PATCH] D44912: [clang-doc] Removing -Wunused-variable warning

[Julie Hockett via Phabricator via cfe-commits]

juliehockett created this revision.
juliehockett added reviewers: jakehehrlich, lebedev.ri, sammccall.
juliehockett added a project: clang-tools-extra.
Herald added subscribers: JDevlieghere, aprantl.

Warning was appearing in release with debug info build, this removes it.


https://reviews.llvm.org/D44912

Files:
  clang-doc/BitcodeWriter.cpp


Index: clang-doc/BitcodeWriter.cpp
===
--- clang-doc/BitcodeWriter.cpp
+++ clang-doc/BitcodeWriter.cpp
@@ -264,8 +264,8 @@
   Record.push_back(BID);
   Stream.EmitRecord(llvm::bitc::BLOCKINFO_CODE_SETBID, Record);
   Stream.EmitRecord(llvm::bitc::BLOCKINFO_CODE_BLOCKNAME,
-ArrayRef(BlockIdNameMap[BID].bytes_begin(),
-BlockIdNameMap[BID].bytes_end()));
+ArrayRef(BlockIdName.bytes_begin(),
+BlockIdName.bytes_end()));
 }
 
 /// \brief Emits a record name to the BLOCKINFO block.


Index: clang-doc/BitcodeWriter.cpp
===
--- clang-doc/BitcodeWriter.cpp
+++ clang-doc/BitcodeWriter.cpp
@@ -264,8 +264,8 @@
   Record.push_back(BID);
   Stream.EmitRecord(llvm::bitc::BLOCKINFO_CODE_SETBID, Record);
   Stream.EmitRecord(llvm::bitc::BLOCKINFO_CODE_BLOCKNAME,
-ArrayRef(BlockIdNameMap[BID].bytes_begin(),
-BlockIdNameMap[BID].bytes_end()));
+ArrayRef(BlockIdName.bytes_begin(),
+BlockIdName.bytes_end()));
 }
 
 /// \brief Emits a record name to the BLOCKINFO block.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44912: [clang-doc] Removing -Wunused-variable warning

[Jake Ehrlich via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D44912



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

[PATCH] D44883: [Sema] Extend -Wself-assign with -Wself-assign-overloaded to warn on overloaded self-assignment (classes)

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a subscriber: lebedev.ri.
dblaikie added a comment.

Historically Clang's policy on warnings was, I think, much more
conservative than it seems to be today. There was a strong desire not to
implement off-by-default warnings, and to have warnings with an
exceptionally low false-positive rate - maybe the user-defined operator
detection was either assumed to, or demonstrated to, have a sufficiently
high false positive rate to not meet that high bar.

(as for the flag splitting - that was sometimes done if the new variant of
a flag had enough bug-finding power that an existing codebase using the
existing flag behavior would need significant cleanup - by having the new
functionality under another flag name, existing codebases upgrading to a
newer compiler wouldn't be forced to either do all that cleanup up-front or
disable the flag & risk regressions... )


Repository:
  rC Clang

https://reviews.llvm.org/D44883



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

Re: [PATCH] D44883: [Sema] Extend -Wself-assign with -Wself-assign-overloaded to warn on overloaded self-assignment (classes)

[David Blaikie via cfe-commits]

Historically Clang's policy on warnings was, I think, much more
conservative than it seems to be today. There was a strong desire not to
implement off-by-default warnings, and to have warnings with an
exceptionally low false-positive rate - maybe the user-defined operator
detection was either assumed to, or demonstrated to, have a sufficiently
high false positive rate to not meet that high bar.

(as for the flag splitting - that was sometimes done if the new variant of
a flag had enough bug-finding power that an existing codebase using the
existing flag behavior would need significant cleanup - by having the new
functionality under another flag name, existing codebases upgrading to a
newer compiler wouldn't be forced to either do all that cleanup up-front or
disable the flag & risk regressions... )

On Mon, Mar 26, 2018 at 3:08 PM Roman Lebedev via Phabricator <
revi...@reviews.llvm.org> wrote:

> lebedev.ri added a comment.
>
> In https://reviews.llvm.org/D44883#1048689, @rjmccall wrote:
>
> > I tracked Chandler down, and he doesn't remember why user-defined
> operators are excluded.
>
>
> Ok then. Unless reviewers think it would be best to have separate diag
> groups for "builtin" and "user-provided" binary operators,
> i'll just extend the `-Wself-assign` / `-Wself-assign-field` /
> `-Wself-move` (i wonder, does it work both the fields and usual variables,
> or not).
>
> (That means, stage2 self-hosting testing will be needed...)
>
>
> Repository:
>   rC Clang
>
> https://reviews.llvm.org/D44883
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44912: [clang-doc] Removing -Wunused-variable warning

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri accepted this revision.
lebedev.ri added a comment.

LGTM.


https://reviews.llvm.org/D44912



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

[PATCH] D36918: [Sema] Take into account the current context when checking the accessibility of a member function pointer

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

ping


https://reviews.llvm.org/D36918



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

[clang-tools-extra] r328588 - [clang-doc] Removing -Wunused-variable warning

[Julie Hockett via cfe-commits]

Author: juliehockett
Date: Mon Mar 26 15:37:31 2018
New Revision: 328588

URL: http://llvm.org/viewvc/llvm-project?rev=328588&view=rev
Log:
[clang-doc] Removing -Wunused-variable warning

Warning was appearing in release with debug info build, this removes it.

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

Modified:
clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp

Modified: clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp?rev=328588&r1=328587&r2=328588&view=diff
==
--- clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp (original)
+++ clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp Mon Mar 26 15:37:31 2018
@@ -264,8 +264,8 @@ void ClangDocBitcodeWriter::emitBlockID(
   Record.push_back(BID);
   Stream.EmitRecord(llvm::bitc::BLOCKINFO_CODE_SETBID, Record);
   Stream.EmitRecord(llvm::bitc::BLOCKINFO_CODE_BLOCKNAME,
-ArrayRef(BlockIdNameMap[BID].bytes_begin(),
-BlockIdNameMap[BID].bytes_end()));
+ArrayRef(BlockIdName.bytes_begin(),
+BlockIdName.bytes_end()));
 }
 
 /// \brief Emits a record name to the BLOCKINFO block.


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

[PATCH] D44912: [clang-doc] Removing -Wunused-variable warning

[Julie Hockett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL328588: [clang-doc] Removing -Wunused-variable warning 
(authored by juliehockett, committed by ).
Herald added subscribers: llvm-commits, klimek.

Changed prior to commit:
  https://reviews.llvm.org/D44912?vs=139858&id=139860#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D44912

Files:
  clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp


Index: clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp
===
--- clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp
+++ clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp
@@ -264,8 +264,8 @@
   Record.push_back(BID);
   Stream.EmitRecord(llvm::bitc::BLOCKINFO_CODE_SETBID, Record);
   Stream.EmitRecord(llvm::bitc::BLOCKINFO_CODE_BLOCKNAME,
-ArrayRef(BlockIdNameMap[BID].bytes_begin(),
-BlockIdNameMap[BID].bytes_end()));
+ArrayRef(BlockIdName.bytes_begin(),
+BlockIdName.bytes_end()));
 }
 
 /// \brief Emits a record name to the BLOCKINFO block.


Index: clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp
===
--- clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp
+++ clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp
@@ -264,8 +264,8 @@
   Record.push_back(BID);
   Stream.EmitRecord(llvm::bitc::BLOCKINFO_CODE_SETBID, Record);
   Stream.EmitRecord(llvm::bitc::BLOCKINFO_CODE_BLOCKNAME,
-ArrayRef(BlockIdNameMap[BID].bytes_begin(),
-BlockIdNameMap[BID].bytes_end()));
+ArrayRef(BlockIdName.bytes_begin(),
+BlockIdName.bytes_end()));
 }
 
 /// \brief Emits a record name to the BLOCKINFO block.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44912: [clang-doc] Removing -Wunused-variable warning

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

I think it's fine to commit trivial fixes without going via review.


Repository:
  rL LLVM

https://reviews.llvm.org/D44912



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

[PATCH] D44913: [ObjC] Enable using C++ triviality type traits for non-trivial C structs

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak created this revision.
ahatanak added reviewers: rjmccall, doug.gregor, rsmith.

r326307 and r327870 made changes that allowed C structs to have __strong and 
__weak fields in ARC and be non-trivial. This patch make changes that allow 
using the following type traits for non-trivial C structs:

__has_trivial_assign
__has_trivial_move_assign
__has_trivial_copy
__has_trivial_move_constructor
__has_trivial_constructor
__has_trivial_destructor


Repository:
  rC Clang

https://reviews.llvm.org/D44913

Files:
  include/clang/Basic/TokenKinds.def
  lib/Sema/SemaExprCXX.cpp
  test/SemaObjC/non-trivial-struct-traits.m

Index: test/SemaObjC/non-trivial-struct-traits.m
===
--- /dev/null
+++ test/SemaObjC/non-trivial-struct-traits.m
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -fsyntax-only -fobjc-arc -verify %s
+
+// expected-no-diagnostics
+
+struct Trivial {
+  int x;
+};
+
+struct NonTrivial {
+  id x;
+};
+
+int trivial_assign[__has_trivial_assign(struct Trivial) ? 1 : -1];
+int trivial_move_assign[__has_trivial_move_assign(struct Trivial) ? 1 : -1];
+int trivial_copy_constructor[__has_trivial_copy(struct Trivial) ? 1 : -1];
+int trivial_move_constructor[__has_trivial_move_constructor(struct Trivial) ? 1 : -1];
+int trivial_constructor[__has_trivial_constructor(struct Trivial) ? 1 : -1];
+int trivial_destructor[__has_trivial_destructor(struct Trivial) ? 1 : -1];
+
+int non_trivial_assign[__has_trivial_assign(struct NonTrivial) ? -1 : 1];
+int non_trivial_move_assign[__has_trivial_move_assign(struct NonTrivial) ? -1 : 1];
+int non_trivial_copy_constructor[__has_trivial_copy(struct NonTrivial) ? -1 : 1];
+int non_trivial_move_constructor[__has_trivial_move_constructor(struct NonTrivial) ? -1 : 1];
+int non_trivial_constructor[__has_trivial_constructor(struct NonTrivial) ? -1 : 1];
+int non_trivial_destructor[__has_trivial_destructor(struct NonTrivial) ? -1 : 1];
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -4513,6 +4513,8 @@
 // does not correctly compute triviality in the presence of multiple special
 // members of the same kind. Revisit this once the g++ bug is fixed.
   case UTT_HasTrivialDefaultConstructor:
+if (T.isNonTrivialToPrimitiveDefaultInitialize())
+  return false;
 // http://gcc.gnu.org/onlinedocs/gcc/Type-Traits.html:
 //   If __is_pod (type) is true then the trait is true, else if type is
 //   a cv class or union type (or array thereof) with a trivial default
@@ -4524,6 +4526,8 @@
  !RD->hasNonTrivialDefaultConstructor();
 return false;
   case UTT_HasTrivialMoveConstructor:
+if (T.isNonTrivialToPrimitiveDestructiveMove())
+  return false;
 //  This trait is implemented by MSVC 2012 and needed to parse the
 //  standard library headers. Specifically this is used as the logic
 //  behind std::is_trivially_move_constructible (20.9.4.3).
@@ -4533,6 +4537,8 @@
   return RD->hasTrivialMoveConstructor() && !RD->hasNonTrivialMoveConstructor();
 return false;
   case UTT_HasTrivialCopy:
+if (T.isNonTrivialToPrimitiveCopy())
+  return false;
 // http://gcc.gnu.org/onlinedocs/gcc/Type-Traits.html:
 //   If __is_pod (type) is true or type is a reference type then
 //   the trait is true, else if type is a cv class or union type
@@ -4545,6 +4551,8 @@
  !RD->hasNonTrivialCopyConstructor();
 return false;
   case UTT_HasTrivialMoveAssign:
+if (T.isNonTrivialToPrimitiveDestructiveMove())
+  return false;
 //  This trait is implemented by MSVC 2012 and needed to parse the
 //  standard library headers. Specifically it is used as the logic
 //  behind std::is_trivially_move_assignable (20.9.4.3)
@@ -4554,6 +4562,8 @@
   return RD->hasTrivialMoveAssignment() && !RD->hasNonTrivialMoveAssignment();
 return false;
   case UTT_HasTrivialAssign:
+if (T.isNonTrivialToPrimitiveCopy())
+  return false;
 // http://gcc.gnu.org/onlinedocs/gcc/Type-Traits.html:
 //   If type is const qualified or is a reference type then the
 //   trait is false. Otherwise if __is_pod (type) is true then the
@@ -4624,6 +4634,8 @@
 return true;
 
   case UTT_HasTrivialDestructor:
+if (T.isDestructedType() == QualType::DK_nontrivial_c_struct)
+  return false;
 // http://gcc.gnu.org/onlinedocs/gcc/Type-Traits.html
 //   If __is_pod (type) is true or type is a reference type
 //   then the trait is true, else if type is a cv class or union
Index: include/clang/Basic/TokenKinds.def
===
--- include/clang/Basic/TokenKinds.def
+++ include/clang/Basic/TokenKinds.def
@@ -433,12 +433,12 @@
 TYPE_TRAIT_1(__has_nothrow_move_assign, HasNothrowMoveAssign, KEYCXX)
 TYPE_TRAIT_1(__has_nothrow_copy, HasNothrowCopy, KEYCXX)
 TYPE_TRAIT_1(__has_n