[PATCH] D58606: [clang-tidy] misc-string-integer-assignment: fix false positive

[Clement Courbet via Phabricator via cfe-commits]

courbet added a comment.

Thanks !

> is there a bug or similar? If yes please mention it somewhere in the summary 
> or so and close it :)

Yes, PR27723. Done.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58606



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

[PATCH] D58236: Make address space conversions a bit stricter.

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added a comment.

In D58236#1412267 , @Anastasia wrote:

> LGTM! Thanks a lot for fixing this old bug! Btw, do you plan to look at 
> generalizing this to C++ as well?


That does sound like a good idea and I will probably look into it when I have 
more time. I'd also like to look into making an RFC for the improved address 
space specification support. Again, when I have time :)

In D58236#1412324 , @efriedma wrote:

> Generally, with an explicit cast, C allows any pointer cast with a reasonable 
> interpretation, even if the underlying operation is suspicious.  For example, 
> you can cast an "long*" to a "int*" (as in "(int*)(long*)p") without any 
> complaint, even though dereferencing the result is likely undefined behavior. 
>  (C doesn't allow explicitly writing a reinterpret_cast, but that's basically 
> the operation in question.)


That is true... The current rules are very permissive about pointer casts.

> Along those lines, in general, the normal C rules should allow casting `foo*` 
> to `bar*` for any object types foo and bar, even if foo and bar are pointers 
> with address spaces, like `__local int *` and `__global int *`.  I don't see 
> anything in the OpenCL standard that would contradict this.  It looks like 
> this patch changes that?

I vaguely recall that when I was looking into fixing this in our downstream a 
while back I found that OpenCL did not have any explicit provisions for nested 
address spaces, only direct ones.

So should we drop the extra error on nested space mismatches, since the rules 
should be as permissive as possible unless the spec prohibits it? Technically 
the patch could be changed to only fix the incorrect warning on direct address 
space mismatches instead.


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

https://reviews.llvm.org/D58236



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

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added inline comments.



Comment at: lib/Sema/SemaCast.cpp:2309
+auto DestPointeeTypeWithoutAS = Self.Context.removeAddrSpaceQualType(
+DestPointeeType.getCanonicalType());
+return Self.Context.hasSameType(SrcPointeeTypeWithoutAS,

Anastasia wrote:
> rjmccall wrote:
> > ebevhan wrote:
> > > Anastasia wrote:
> > > > Anastasia wrote:
> > > > > ebevhan wrote:
> > > > > > Maybe I'm mistaken, but won't getting the canonical type here drop 
> > > > > > qualifiers (like cv) in nested pointers? If so, an addrspace_cast 
> > > > > > might strip qualifiers by mistake.
> > > > > Yes, indeed I will need to extend this to nested pointers when we are 
> > > > > ready. But for now I can try to change this bit... however I am not 
> > > > > sure it will work w/o canonical types when we have typedef. I will 
> > > > > try to create an example and see.
> > > > I checked the canonical type does preserve the qualifiers correctly.
> > > > 
> > > > Here is the AST dump of the following C type `mytype const __generic*` 
> > > > where  `typedef  __generic int* mytype;`.
> > > > 
> > > > 
> > > > ```
> > > > PointerType 0x204d3b0 'const __generic mytype *'
> > > > `-QualType 0x204d369 'const __generic mytype' const __generic
> > > >   `-TypedefType 0x204d320 'mytype' sugar
> > > > |-Typedef 0x204d1b0 'mytype'
> > > > `-PointerType 0x204d170 '__generic int *'
> > > >   `-QualType 0x204d158 '__generic int' __generic
> > > > `-BuiltinType 0x2009750 'int'
> > > > ```
> > > > 
> > > > and it's canonical representation in AST is:
> > > > 
> > > > ```
> > > > PointerType 0x204d380 '__generic int *const __generic *'
> > > > `-QualType 0x204d349 '__generic int *const __generic' const __generic
> > > >   `-PointerType 0x204d170 '__generic int *'
> > > > `-QualType 0x204d158 '__generic int' __generic
> > > >   `-BuiltinType 0x2009750 'int'
> > > > ```
> > > > 
> > > > So using canonical type will just simply handling of nested pointer 
> > > > chain by avoiding special casing typedefs. We won't loose any 
> > > > qualifiers.
> > > Okay, seems good then!
> > It seems to me that the rules in this function are the reasonable 
> > cross-language rules.  If you want to check for OpenCL at the top as a 
> > fast-path check (taking advantage of that fact that no other language modes 
> > currently have overlapping address spaces), that might be reasonable, but 
> > the comment and indentation should reflect that.
> Btw, I attempted to remove the OpenCL check and unfortunately found failure 
> in the following test: **test/CodeGenCXX/address-space-cast.cpp**
> 
> 
> ```
> error: C-style cast from 'char *' to '__attribute__((address_space(5))) char 
> *' converts between mismatching address spaces
>   __private__ char *priv_char_ptr = (__private__ char *)gen_char_ptr;
> ```
> 
> I am not sure what such conversion does but I feel if I want to update it I 
> might need to get wider agreement. I think sending an RFC might be a good way 
> forward.
> 
Seems like that would happen if the address spaces don't overlap, which none of 
the target address spaces do. That sort of cast works in C because the ASes are 
only tested for compatibility when you're building for OpenCL (in 
SemaCast:checkAddressSpaceCast). Without a guard on OpenCL, any target AS 
compatibility check will fail.

A possible fix could be to omit the `isAddressSpaceOverlapping` check when 
`!OpenCL`, but properly formalizing the AS conversion behavior would most 
likely be the best fix for this.




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

https://reviews.llvm.org/D58346



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

[PATCH] D58764: [clang-tidy] ignore predefined expressions in cppcoreguidelines-pro-bounds-array-to-pointer-decay check

[Lewis Clark via Phabricator via cfe-commits]

lewmpk created this revision.
lewmpk added a reviewer: clang-tools-extra.
lewmpk added projects: clang, clang-tools-extra.
Herald added subscribers: cfe-commits, kbarton, xazax.hun, nemanjai.

Bugzilla: 40852

   c++
  int main()
  {
const char* a = __FILE__; 
const char* b = __FUNCTION__; 
  }

variable `b` is now not marked as an error by 
`cppcoreguidelines-pro-bounds-array-to-pointer-decay` check


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D58764

Files:
  clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
  test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp


Index: test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
===
--- test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
+++ test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
@@ -44,6 +44,9 @@
 return ("clang"); // OK, ParenExpr hides the literal-pointer decay
 }
 
+const char *line = __FILE__; // OK
+const char *func = __FUNCTION__; // OK, predefined value to pointer
+
 void f2(void *const *);
 void bug25362() {
   void *a[2];
Index: clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
===
--- clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
+++ clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
@@ -56,12 +56,14 @@
   // 1) just before array subscription
   // 2) inside a range-for over an array
   // 3) if it converts a string literal to a pointer
+  // 4) if it converts a predefined value to a pointer
   Finder->addMatcher(
   implicitCastExpr(
   unless(hasParent(arraySubscriptExpr())),
   unless(hasParentIgnoringImpCasts(explicitCastExpr())),
   unless(isInsideOfRangeBeginEndStmt()),
-  unless(hasSourceExpression(ignoringParens(stringLiteral()
+  unless(hasSourceExpression(ignoringParens(stringLiteral(,
+  unless(hasSourceExpression(ignoringParens(predefinedExpr()
   .bind("cast"),
   this);
 }


Index: test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
===
--- test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
+++ test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
@@ -44,6 +44,9 @@
 return ("clang"); // OK, ParenExpr hides the literal-pointer decay
 }
 
+const char *line = __FILE__; // OK
+const char *func = __FUNCTION__; // OK, predefined value to pointer
+
 void f2(void *const *);
 void bug25362() {
   void *a[2];
Index: clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
===
--- clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
+++ clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
@@ -56,12 +56,14 @@
   // 1) just before array subscription
   // 2) inside a range-for over an array
   // 3) if it converts a string literal to a pointer
+  // 4) if it converts a predefined value to a pointer
   Finder->addMatcher(
   implicitCastExpr(
   unless(hasParent(arraySubscriptExpr())),
   unless(hasParentIgnoringImpCasts(explicitCastExpr())),
   unless(isInsideOfRangeBeginEndStmt()),
-  unless(hasSourceExpression(ignoringParens(stringLiteral()
+  unless(hasSourceExpression(ignoringParens(stringLiteral(,
+  unless(hasSourceExpression(ignoringParens(predefinedExpr()
   .bind("cast"),
   this);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58764: [clang-tidy] ignore predefined expressions in cppcoreguidelines-pro-bounds-array-to-pointer-decay check

[Roman Lebedev via Phabricator via cfe-commits]

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

See D22196 .


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58764



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

[clang-tools-extra] r355076 - [clang-tidy] misc-string-integer-assignment: fix false positive

[Clement Courbet via cfe-commits]

Author: courbet
Date: Thu Feb 28 02:33:32 2019
New Revision: 355076

URL: http://llvm.org/viewvc/llvm-project?rev=355076&view=rev
Log:
[clang-tidy] misc-string-integer-assignment: fix false positive

Summary:
using CodePoint = uint32_t;
CodePoint cp;
basic_string s;
s += cp;

See PR27723.

Reviewers: xazax.hun, alexfh

Subscribers: rnkovacs, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp

clang-tools-extra/trunk/test/clang-tidy/bugprone-string-integer-assignment.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp?rev=355076&r1=355075&r2=355076&view=diff
==
--- 
clang-tools-extra/trunk/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp 
(original)
+++ 
clang-tools-extra/trunk/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp 
Thu Feb 28 02:33:32 2019
@@ -26,7 +26,8 @@ void StringIntegerAssignmentCheck::regis
 hasOverloadedOperatorName("+=")),
   callee(cxxMethodDecl(ofClass(classTemplateSpecializationDecl(
   hasName("::std::basic_string"),
-  hasTemplateArgument(0, 
refersToType(qualType().bind("type"))),
+  hasTemplateArgument(0, refersToType(hasCanonicalType(
+ qualType().bind("type",
   hasArgument(
   1,
   ignoringImpCasts(
@@ -34,7 +35,11 @@ void StringIntegerAssignmentCheck::regis
// Ignore calls to tolower/toupper (see PR27723).
unless(callExpr(callee(functionDecl(
hasAnyName("tolower", "std::tolower", "toupper",
-  "std::toupper"))
+  "std::toupper"),
+   // Do not warn if assigning e.g. `CodePoint` to
+   // `basic_string`
+   unless(hasType(qualType(
+   hasCanonicalType(equalsBoundNode("type"))
   .bind("expr"))),
   unless(isInTemplateInstantiation())),
   this);

Modified: 
clang-tools-extra/trunk/test/clang-tidy/bugprone-string-integer-assignment.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/bugprone-string-integer-assignment.cpp?rev=355076&r1=355075&r2=355076&view=diff
==
--- 
clang-tools-extra/trunk/test/clang-tidy/bugprone-string-integer-assignment.cpp 
(original)
+++ 
clang-tools-extra/trunk/test/clang-tidy/bugprone-string-integer-assignment.cpp 
Thu Feb 28 02:33:32 2019
@@ -53,8 +53,8 @@ int main() {
 
   std::basic_string as;
   as = 6;
-// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: an integer is interpreted as a 
chara
-// CHECK-FIXES: {{^}}  as = 6;{{$}}
+  as = static_cast(6);
+  as = 'a';
 
   s += toupper(x);
   s += tolower(x);


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

[PATCH] D58606: [clang-tidy] misc-string-integer-assignment: fix false positive

[Clement Courbet via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL355076: [clang-tidy] misc-string-integer-assignment: fix 
false positive (authored by courbet, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58606

Files:
  clang-tools-extra/trunk/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
  clang-tools-extra/trunk/test/clang-tidy/bugprone-string-integer-assignment.cpp


Index: 
clang-tools-extra/trunk/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
@@ -26,7 +26,8 @@
 hasOverloadedOperatorName("+=")),
   callee(cxxMethodDecl(ofClass(classTemplateSpecializationDecl(
   hasName("::std::basic_string"),
-  hasTemplateArgument(0, 
refersToType(qualType().bind("type"))),
+  hasTemplateArgument(0, refersToType(hasCanonicalType(
+ qualType().bind("type",
   hasArgument(
   1,
   ignoringImpCasts(
@@ -34,7 +35,11 @@
// Ignore calls to tolower/toupper (see PR27723).
unless(callExpr(callee(functionDecl(
hasAnyName("tolower", "std::tolower", "toupper",
-  "std::toupper"))
+  "std::toupper"),
+   // Do not warn if assigning e.g. `CodePoint` to
+   // `basic_string`
+   unless(hasType(qualType(
+   hasCanonicalType(equalsBoundNode("type"))
   .bind("expr"))),
   unless(isInTemplateInstantiation())),
   this);
Index: 
clang-tools-extra/trunk/test/clang-tidy/bugprone-string-integer-assignment.cpp
===
--- 
clang-tools-extra/trunk/test/clang-tidy/bugprone-string-integer-assignment.cpp
+++ 
clang-tools-extra/trunk/test/clang-tidy/bugprone-string-integer-assignment.cpp
@@ -53,8 +53,8 @@
 
   std::basic_string as;
   as = 6;
-// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: an integer is interpreted as a 
chara
-// CHECK-FIXES: {{^}}  as = 6;{{$}}
+  as = static_cast(6);
+  as = 'a';
 
   s += toupper(x);
   s += tolower(x);


Index: clang-tools-extra/trunk/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
@@ -26,7 +26,8 @@
 hasOverloadedOperatorName("+=")),
   callee(cxxMethodDecl(ofClass(classTemplateSpecializationDecl(
   hasName("::std::basic_string"),
-  hasTemplateArgument(0, refersToType(qualType().bind("type"))),
+  hasTemplateArgument(0, refersToType(hasCanonicalType(
+ qualType().bind("type",
   hasArgument(
   1,
   ignoringImpCasts(
@@ -34,7 +35,11 @@
// Ignore calls to tolower/toupper (see PR27723).
unless(callExpr(callee(functionDecl(
hasAnyName("tolower", "std::tolower", "toupper",
-  "std::toupper"))
+  "std::toupper"),
+   // Do not warn if assigning e.g. `CodePoint` to
+   // `basic_string`
+   unless(hasType(qualType(
+   hasCanonicalType(equalsBoundNode("type"))
   .bind("expr"))),
   unless(isInTemplateInstantiation())),
   this);
Index: clang-tools-extra/trunk/test/clang-tidy/bugprone-string-integer-assignment.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-string-integer-assignment.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-string-integer-assignment.cpp
@@ -53,8 +53,8 @@
 
   std::basic_string as;
   as = 6;
-// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: an integer is interpreted as a chara
-// CHECK-FIXES: {{^}}  as = 6;{{$}}
+  as = static_cast(6);
+  as = 'a';
 
   s += toupper(x);
   s += tolower(x);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58609: [clang-tidy] bugprone-string-integer-assignment: Reduce false positives.

[Clement Courbet via Phabricator via cfe-commits]

courbet updated this revision to Diff 188690.
courbet marked an inline comment as done.
courbet added a comment.

- rebase
- -more cosmetics


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58609

Files:
  clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
  test/clang-tidy/bugprone-string-integer-assignment.cpp


Index: test/clang-tidy/bugprone-string-integer-assignment.cpp
===
--- test/clang-tidy/bugprone-string-integer-assignment.cpp
+++ test/clang-tidy/bugprone-string-integer-assignment.cpp
@@ -59,4 +59,11 @@
   s += toupper(x);
   s += tolower(x);
   s += std::tolower(x);
+
+  // Likely character expressions.
+  s += x & 0xff;
+  s += 0xff & x;
+
+  s += 'a' + (x % 26);
+  s += (x % 10) + 'b';
 }
Index: clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
===
--- clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
+++ clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
@@ -45,11 +45,40 @@
   this);
 }
 
+static bool isLikelyCharExpression(const Expr *Argument,
+   const ASTContext &Ctx) {
+  const auto *BinOp = dyn_cast(Argument);
+  if (!BinOp)
+return false;
+  const auto *LHS = BinOp->getLHS()->IgnoreParenImpCasts();
+  const auto *RHS = BinOp->getRHS()->IgnoreParenImpCasts();
+  //  & , mask is a compile time constant.
+  Expr::EvalResult RHSVal;
+  if (BinOp->getOpcode() == BO_And &&
+  (RHS->EvaluateAsInt(RHSVal, Ctx, Expr::SE_AllowSideEffects) ||
+   LHS->EvaluateAsInt(RHSVal, Ctx, Expr::SE_AllowSideEffects)))
+return true;
+  //  + ( % ), where  is a char literal.
+  const auto IsCharPlusModExpr = [](const Expr *L, const Expr *R) {
+const auto *ROp = dyn_cast(R);
+return ROp && ROp->getOpcode() == BO_Rem && isa(L);
+  };
+  if (BinOp->getOpcode() == BO_Add) {
+if (IsCharPlusModExpr(LHS, RHS) || IsCharPlusModExpr(RHS, LHS))
+  return true;
+  }
+  return false;
+}
+
 void StringIntegerAssignmentCheck::check(
 const MatchFinder::MatchResult &Result) {
   const auto *Argument = Result.Nodes.getNodeAs("expr");
   SourceLocation Loc = Argument->getBeginLoc();
 
+  // Try to detect a few common expressions to reduce false positives.
+  if (isLikelyCharExpression(Argument, *Result.Context))
+return;
+
   auto Diag =
   diag(Loc, "an integer is interpreted as a character code when assigning "
 "it to a string; if this is intended, cast the integer to the "


Index: test/clang-tidy/bugprone-string-integer-assignment.cpp
===
--- test/clang-tidy/bugprone-string-integer-assignment.cpp
+++ test/clang-tidy/bugprone-string-integer-assignment.cpp
@@ -59,4 +59,11 @@
   s += toupper(x);
   s += tolower(x);
   s += std::tolower(x);
+
+  // Likely character expressions.
+  s += x & 0xff;
+  s += 0xff & x;
+
+  s += 'a' + (x % 26);
+  s += (x % 10) + 'b';
 }
Index: clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
===
--- clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
+++ clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
@@ -45,11 +45,40 @@
   this);
 }
 
+static bool isLikelyCharExpression(const Expr *Argument,
+   const ASTContext &Ctx) {
+  const auto *BinOp = dyn_cast(Argument);
+  if (!BinOp)
+return false;
+  const auto *LHS = BinOp->getLHS()->IgnoreParenImpCasts();
+  const auto *RHS = BinOp->getRHS()->IgnoreParenImpCasts();
+  //  & , mask is a compile time constant.
+  Expr::EvalResult RHSVal;
+  if (BinOp->getOpcode() == BO_And &&
+  (RHS->EvaluateAsInt(RHSVal, Ctx, Expr::SE_AllowSideEffects) ||
+   LHS->EvaluateAsInt(RHSVal, Ctx, Expr::SE_AllowSideEffects)))
+return true;
+  //  + ( % ), where  is a char literal.
+  const auto IsCharPlusModExpr = [](const Expr *L, const Expr *R) {
+const auto *ROp = dyn_cast(R);
+return ROp && ROp->getOpcode() == BO_Rem && isa(L);
+  };
+  if (BinOp->getOpcode() == BO_Add) {
+if (IsCharPlusModExpr(LHS, RHS) || IsCharPlusModExpr(RHS, LHS))
+  return true;
+  }
+  return false;
+}
+
 void StringIntegerAssignmentCheck::check(
 const MatchFinder::MatchResult &Result) {
   const auto *Argument = Result.Nodes.getNodeAs("expr");
   SourceLocation Loc = Argument->getBeginLoc();
 
+  // Try to detect a few common expressions to reduce false positives.
+  if (isLikelyCharExpression(Argument, *Result.Context))
+return;
+
   auto Diag =
   diag(Loc, "an integer is interpreted as a character code when assigning "
 "it to a string; if this is intended, cast the integer to the "
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58666: [OpenCL] Undefine cl_intel_planar_yuv extension

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: test/SemaOpenCL/extension-begin.cl:26
+
 #ifndef IMPLICIT_INCLUDE
 #include "extension-begin.h"

sidorovd wrote:
> Anastasia wrote:
> > sidorovd wrote:
> > > Anastasia wrote:
> > > > Can we also test that macro `my_ext` is not defined here but defined 
> > > > above?
> > > > 
> > > > It seems we are not testing anything like this...
> > > #pragma OPENCL EXTENSION my_ext : begin doesn't define an appropriate 
> > > macro. And so cl-ext=+my_ext.
> > But don't you need to expose the definition of it?
> Certainly I need, but now the only proper way to do so is by adding an 
> extension via adding it in OpenCLExtensions.def. Previously we decided to 
> avoid adding an extension directly into clang, so with a new approach I'd 
> prefer not to add a definition of the macro in the header but define it 
> somewhere else, otherwise the macro becomes defined  where it's not supposed 
> to be (even for ARM and AMD =) ). 
However, my understanding is that you should define the macro when you define 
the extension itself.


```
#pragma OPENCL EXTENSION my_ext : begin
#define my_ext
...
#pragma OPENCL EXTENSION my_ext : end
```

does it not work for you?


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

https://reviews.llvm.org/D58666



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

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

[Anastasia Stulova via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaCast.cpp:2309
+auto DestPointeeTypeWithoutAS = Self.Context.removeAddrSpaceQualType(
+DestPointeeType.getCanonicalType());
+return Self.Context.hasSameType(SrcPointeeTypeWithoutAS,

ebevhan wrote:
> Anastasia wrote:
> > rjmccall wrote:
> > > ebevhan wrote:
> > > > Anastasia wrote:
> > > > > Anastasia wrote:
> > > > > > ebevhan wrote:
> > > > > > > Maybe I'm mistaken, but won't getting the canonical type here 
> > > > > > > drop qualifiers (like cv) in nested pointers? If so, an 
> > > > > > > addrspace_cast might strip qualifiers by mistake.
> > > > > > Yes, indeed I will need to extend this to nested pointers when we 
> > > > > > are ready. But for now I can try to change this bit... however I am 
> > > > > > not sure it will work w/o canonical types when we have typedef. I 
> > > > > > will try to create an example and see.
> > > > > I checked the canonical type does preserve the qualifiers correctly.
> > > > > 
> > > > > Here is the AST dump of the following C type `mytype const 
> > > > > __generic*` where  `typedef  __generic int* mytype;`.
> > > > > 
> > > > > 
> > > > > ```
> > > > > PointerType 0x204d3b0 'const __generic mytype *'
> > > > > `-QualType 0x204d369 'const __generic mytype' const __generic
> > > > >   `-TypedefType 0x204d320 'mytype' sugar
> > > > > |-Typedef 0x204d1b0 'mytype'
> > > > > `-PointerType 0x204d170 '__generic int *'
> > > > >   `-QualType 0x204d158 '__generic int' __generic
> > > > > `-BuiltinType 0x2009750 'int'
> > > > > ```
> > > > > 
> > > > > and it's canonical representation in AST is:
> > > > > 
> > > > > ```
> > > > > PointerType 0x204d380 '__generic int *const __generic *'
> > > > > `-QualType 0x204d349 '__generic int *const __generic' const __generic
> > > > >   `-PointerType 0x204d170 '__generic int *'
> > > > > `-QualType 0x204d158 '__generic int' __generic
> > > > >   `-BuiltinType 0x2009750 'int'
> > > > > ```
> > > > > 
> > > > > So using canonical type will just simply handling of nested pointer 
> > > > > chain by avoiding special casing typedefs. We won't loose any 
> > > > > qualifiers.
> > > > Okay, seems good then!
> > > It seems to me that the rules in this function are the reasonable 
> > > cross-language rules.  If you want to check for OpenCL at the top as a 
> > > fast-path check (taking advantage of that fact that no other language 
> > > modes currently have overlapping address spaces), that might be 
> > > reasonable, but the comment and indentation should reflect that.
> > Btw, I attempted to remove the OpenCL check and unfortunately found failure 
> > in the following test: **test/CodeGenCXX/address-space-cast.cpp**
> > 
> > 
> > ```
> > error: C-style cast from 'char *' to '__attribute__((address_space(5))) 
> > char *' converts between mismatching address spaces
> >   __private__ char *priv_char_ptr = (__private__ char *)gen_char_ptr;
> > ```
> > 
> > I am not sure what such conversion does but I feel if I want to update it I 
> > might need to get wider agreement. I think sending an RFC might be a good 
> > way forward.
> > 
> Seems like that would happen if the address spaces don't overlap, which none 
> of the target address spaces do. That sort of cast works in C because the 
> ASes are only tested for compatibility when you're building for OpenCL (in 
> SemaCast:checkAddressSpaceCast). Without a guard on OpenCL, any target AS 
> compatibility check will fail.
> 
> A possible fix could be to omit the `isAddressSpaceOverlapping` check when 
> `!OpenCL`, but properly formalizing the AS conversion behavior would most 
> likely be the best fix for this.
> 
> 
Ok yes, I would like to formalize this indeed, but I will definitely need some 
help. I think the best way to proceed is to gather all the ideas we have 
discussed so far and send an RFC. 


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

https://reviews.llvm.org/D58346



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

[PATCH] D58764: [clang-tidy] ignore predefined expressions in cppcoreguidelines-pro-bounds-array-to-pointer-decay check

[Lewis Clark via Phabricator via cfe-commits]

lewmpk added a comment.

Okk, it seems that the consensus is that `__FUNCTION__` should not be cast to 
`char*`. ( I'll research a bit before I pick up a task in the future :) )
I'm happy to abandon this diff - any objections?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58764



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

[PATCH] D57898: CodeGen: Fix PR40605 by splitting constant struct initializers

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 188694.
glider marked 3 inline comments as done.
glider retitled this revision from "CodeGen: Fix PR40605" to "CodeGen: Fix 
PR40605 by splitting constant struct initializers".

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

https://reviews.llvm.org/D57898

Files:
  clang/lib/CodeGen/CGDecl.cpp
  clang/test/CodeGenCXX/auto-var-init.cpp

Index: clang/test/CodeGenCXX/auto-var-init.cpp
===
--- clang/test/CodeGenCXX/auto-var-init.cpp
+++ clang/test/CodeGenCXX/auto-var-init.cpp
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks %s -emit-llvm -o - | FileCheck %s
-// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=pattern %s -emit-llvm -o - | FileCheck %s -check-prefix=PATTERN
-// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=zero %s -emit-llvm -o - | FileCheck %s -check-prefix=ZERO
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks %s -emit-llvm -o - | FileCheck %s -check-prefixes=CHECK,CHECK-O0
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=pattern %s -emit-llvm -o - | FileCheck %s -check-prefixes=CHECK-O0,PATTERN,PATTERN-O0
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=pattern %s -O1 -emit-llvm -o - | FileCheck %s -check-prefixes=CHECK-O1,PATTERN,PATTERN-O1
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=zero %s -emit-llvm -o - | FileCheck %s -check-prefixes=CHECK-O0,ZERO,ZERO-O0
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=zero %s -O1 -emit-llvm -o - | FileCheck %s -check-prefixes=CHECK-O1,ZERO,ZERO-O1
 
 template void used(T &) noexcept;
 
@@ -30,120 +32,192 @@
 // PATTERN-NOT: undef
 // ZERO-NOT: undef
 
-// PATTERN: @__const.test_empty_uninit.uninit = private unnamed_addr constant %struct.empty { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_empty_uninit.uninit = private unnamed_addr constant %struct.empty { i8 -86 }, align 1
+// PATTERN-O1-NOT: @__const.test_empty_uninit.uninit
 struct empty {};
-// PATTERN: @__const.test_small_uninit.uninit = private unnamed_addr constant %struct.small { i8 -86 }, align 1
-// PATTERN: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
-// ZERO: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
+// PATTERN-O0: @__const.test_small_uninit.uninit = private unnamed_addr constant %struct.small { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
+// ZERO-O0: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
+// PATTERN-O1-NOT: @__const.test_small_uninit.uninit
+// PATTERN-O1-NOT: @__const.test_small_custom.custom
+// ZERO-O1-NOT: @__const.test_small_custom.custom
 struct small { char c; };
-// PATTERN: @__const.test_smallinit_uninit.uninit = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
-// PATTERN: @__const.test_smallinit_braces.braces = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
-// PATTERN: @__const.test_smallinit_custom.custom = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallinit_uninit.uninit = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallinit_braces.braces = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallinit_custom.custom = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
+// PATTERN-O1-NOT: @__const.test_smallinit_uninit.uninit
+// PATTERN-O1-NOT: @__const.test_smallinit_braces.braces
+// PATTERN-O1-NOT: @__const.test_smallinit_custom.custom
 struct smallinit { char c = 42; };
-// PATTERN: @__const.test_smallpartinit_uninit.uninit = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
-// PATTERN: @__const.test_smallpartinit_braces.braces = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
-// PATTERN: @__const.test_smallpartinit_custom.custom = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallpartinit_uninit.uninit = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallpartinit_braces.braces = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallpartinit_custom.custom = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
+// PATTERN-O1-NOT: @__const.test_smallpartinit_uninit.uninit
+// PATTER

[PATCH] D57898: CodeGen: Fix PR40605 by splitting constant struct initializers

[Alexander Potapenko via Phabricator via cfe-commits]

glider added inline comments.



Comment at: clang/test/CodeGenCXX/auto-var-init.cpp:130
+// PATTERN-NOT-O1: @__const.test_bool4_custom.custom
+// ZERO-NOT-O1: @__const.test_bool4_custom.custom
+

jfb wrote:
> `-NOT` is in the wrong place above.
Hm, I wonder if lit could automatically check that all the patterns in the file 
are used in RUN lines.
(Probably no, it's hard to tell if an all-caps word is a pattern or just a 
comment)


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

https://reviews.llvm.org/D57898



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

[PATCH] D58768: Moved SymbolLocation into its own header and implementation file

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr created this revision.
gribozavr added a reviewer: ioeric.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, mgorny.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58768

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/index/Index.cpp
  clang-tools-extra/clangd/index/Index.h
  clang-tools-extra/clangd/index/Merge.cpp
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/index/Serialization.h
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/SymbolLocation.cpp
  clang-tools-extra/clangd/index/SymbolLocation.h
  clang-tools-extra/clangd/index/YAMLSerialization.cpp

Index: clang-tools-extra/clangd/index/YAMLSerialization.cpp
===
--- clang-tools-extra/clangd/index/YAMLSerialization.cpp
+++ clang-tools-extra/clangd/index/YAMLSerialization.cpp
@@ -14,6 +14,7 @@
 
 #include "Index.h"
 #include "Serialization.h"
+#include "SymbolLocation.h"
 #include "Trace.h"
 #include "dex/Dex.h"
 #include "llvm/ADT/Optional.h"
Index: clang-tools-extra/clangd/index/SymbolLocation.h
===
--- /dev/null
+++ clang-tools-extra/clangd/index/SymbolLocation.h
@@ -0,0 +1,88 @@
+//===--- SymbolLocation.h *- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOL_LOCATION_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOL_LOCATION_H
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+
+namespace clang {
+namespace clangd {
+
+struct SymbolLocation {
+  // Specify a position (Line, Column) of symbol. Using Line/Column allows us to
+  // build LSP responses without reading the file content.
+  //
+  // Position is encoded into 32 bits to save space.
+  // If Line/Column overflow, the value will be their maximum value.
+  struct Position {
+Position() : Line(0), Column(0) {}
+void setLine(uint32_t Line);
+uint32_t line() const { return Line; }
+void setColumn(uint32_t Column);
+uint32_t column() const { return Column; }
+
+bool hasOverflow() const {
+  return Line >= MaxLine || Column >= MaxColumn;
+}
+
+static constexpr uint32_t MaxLine = (1 << 20) - 1;
+static constexpr uint32_t MaxColumn = (1 << 12) - 1;
+
+  private:
+uint32_t Line : 20; // 0-based
+// Using UTF-16 code units.
+uint32_t Column : 12; // 0-based
+  };
+
+  /// The symbol range, using half-open range [Start, End).
+  Position Start;
+  Position End;
+
+  explicit operator bool() const { return !llvm::StringRef(FileURI).empty(); }
+
+  // The URI of the source file where a symbol occurs.
+  // The string must be null-terminated.
+  //
+  // We avoid using llvm::StringRef here to save memory.
+  // WARNING: unless you know what you are doing, it is recommended to use it
+  // via llvm::StringRef.
+  const char *FileURI = "";
+};
+
+inline bool operator==(const SymbolLocation::Position &L,
+   const SymbolLocation::Position &R) {
+  return std::make_tuple(L.line(), L.column()) ==
+ std::make_tuple(R.line(), R.column());
+}
+inline bool operator<(const SymbolLocation::Position &L,
+  const SymbolLocation::Position &R) {
+  return std::make_tuple(L.line(), L.column()) <
+ std::make_tuple(R.line(), R.column());
+}
+inline bool operator==(const SymbolLocation &L, const SymbolLocation &R) {
+  assert(L.FileURI && R.FileURI);
+  return !std::strcmp(L.FileURI, R.FileURI) &&
+ std::tie(L.Start, L.End) == std::tie(R.Start, R.End);
+}
+inline bool operator<(const SymbolLocation &L, const SymbolLocation &R) {
+  assert(L.FileURI && R.FileURI);
+  int Cmp = std::strcmp(L.FileURI, R.FileURI);
+  if (Cmp != 0)
+return Cmp < 0;
+  return std::tie(L.Start, L.End) < std::tie(R.Start, R.End);
+}
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, const SymbolLocation &);
+
+} // namespace clangd
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOL_LOCATION_H
Index: clang-tools-extra/clangd/index/SymbolLocation.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/index/SymbolLocation.cpp
@@ -0,0 +1,40 @@
+//===--- SymbolLocation.cpp --*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+

[PATCH] D58769: Moved DenseMap support for SymbolID into SymbolID.h

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr created this revision.
gribozavr added a reviewer: ioeric.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58769

Files:
  clang-tools-extra/clangd/index/Index.h
  clang-tools-extra/clangd/index/SymbolID.h


Index: clang-tools-extra/clangd/index/SymbolID.h
===
--- clang-tools-extra/clangd/index/SymbolID.h
+++ clang-tools-extra/clangd/index/SymbolID.h
@@ -9,6 +9,7 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOLID_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOLID_H
 
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
@@ -61,4 +62,25 @@
 } // namespace clangd
 } // namespace clang
 
+namespace llvm {
+// Support SymbolIDs as DenseMap keys.
+template <> struct DenseMapInfo {
+  static inline clang::clangd::SymbolID getEmptyKey() {
+static clang::clangd::SymbolID EmptyKey("EMPTYKEY");
+return EmptyKey;
+  }
+  static inline clang::clangd::SymbolID getTombstoneKey() {
+static clang::clangd::SymbolID TombstoneKey("TOMBSTONEKEY");
+return TombstoneKey;
+  }
+  static unsigned getHashValue(const clang::clangd::SymbolID &Sym) {
+return hash_value(Sym);
+  }
+  static bool isEqual(const clang::clangd::SymbolID &LHS,
+  const clang::clangd::SymbolID &RHS) {
+return LHS == RHS;
+  }
+};
+} // namespace llvm
+
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOLID_H
Index: clang-tools-extra/clangd/index/Index.h
===
--- clang-tools-extra/clangd/index/Index.h
+++ clang-tools-extra/clangd/index/Index.h
@@ -94,31 +94,6 @@
 }
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const SymbolLocation &);
 
-} // namespace clangd
-} // namespace clang
-namespace llvm {
-// Support SymbolIDs as DenseMap keys.
-template <> struct DenseMapInfo {
-  static inline clang::clangd::SymbolID getEmptyKey() {
-static clang::clangd::SymbolID EmptyKey("EMPTYKEY");
-return EmptyKey;
-  }
-  static inline clang::clangd::SymbolID getTombstoneKey() {
-static clang::clangd::SymbolID TombstoneKey("TOMBSTONEKEY");
-return TombstoneKey;
-  }
-  static unsigned getHashValue(const clang::clangd::SymbolID &Sym) {
-return hash_value(Sym);
-  }
-  static bool isEqual(const clang::clangd::SymbolID &LHS,
-  const clang::clangd::SymbolID &RHS) {
-return LHS == RHS;
-  }
-};
-} // namespace llvm
-namespace clang {
-namespace clangd {
-
 // Describes the source of information about a symbol.
 // Mainly useful for debugging, e.g. understanding code completion reuslts.
 // This is a bitfield as information can be combined from several sources.


Index: clang-tools-extra/clangd/index/SymbolID.h
===
--- clang-tools-extra/clangd/index/SymbolID.h
+++ clang-tools-extra/clangd/index/SymbolID.h
@@ -9,6 +9,7 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOLID_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOLID_H
 
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
@@ -61,4 +62,25 @@
 } // namespace clangd
 } // namespace clang
 
+namespace llvm {
+// Support SymbolIDs as DenseMap keys.
+template <> struct DenseMapInfo {
+  static inline clang::clangd::SymbolID getEmptyKey() {
+static clang::clangd::SymbolID EmptyKey("EMPTYKEY");
+return EmptyKey;
+  }
+  static inline clang::clangd::SymbolID getTombstoneKey() {
+static clang::clangd::SymbolID TombstoneKey("TOMBSTONEKEY");
+return TombstoneKey;
+  }
+  static unsigned getHashValue(const clang::clangd::SymbolID &Sym) {
+return hash_value(Sym);
+  }
+  static bool isEqual(const clang::clangd::SymbolID &LHS,
+  const clang::clangd::SymbolID &RHS) {
+return LHS == RHS;
+  }
+};
+} // namespace llvm
+
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOLID_H
Index: clang-tools-extra/clangd/index/Index.h
===
--- clang-tools-extra/clangd/index/Index.h
+++ clang-tools-extra/clangd/index/Index.h
@@ -94,31 +94,6 @@
 }
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const SymbolLocation &);
 
-} // namespace clangd
-} // namespace clang
-namespace llvm {
-// Support SymbolIDs as DenseMap keys.
-template <> struct DenseMapInfo {
-  static inline clang::clangd::SymbolID getEmptyKey() {
-static clang::clangd::SymbolID EmptyKey("EMPTYKEY");
-return EmptyKey;
-  }
-  static inline clang::clangd::SymbolID getTombstoneKey() {
-static clang::clangd::SymbolID TombstoneKey("TOMBSTONEKEY");
-return TombstoneKey;
-  }
-  static unsigned getHashValue(const clang::clangd::SymbolID &Sym) {
-return hash_value(Sym);
-  }
-  static bool isEqual

[clang-tools-extra] r355081 - Moved DenseMap support for SymbolID into SymbolID.h

[Dmitri Gribenko via cfe-commits]

Author: gribozavr
Date: Thu Feb 28 03:00:44 2019
New Revision: 355081

URL: http://llvm.org/viewvc/llvm-project?rev=355081&view=rev
Log:
Moved DenseMap support for SymbolID into SymbolID.h

Modified:
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/SymbolID.h

Modified: clang-tools-extra/trunk/clangd/index/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=355081&r1=355080&r2=355081&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.h (original)
+++ clang-tools-extra/trunk/clangd/index/Index.h Thu Feb 28 03:00:44 2019
@@ -94,31 +94,6 @@ inline bool operator<(const SymbolLocati
 }
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const SymbolLocation &);
 
-} // namespace clangd
-} // namespace clang
-namespace llvm {
-// Support SymbolIDs as DenseMap keys.
-template <> struct DenseMapInfo {
-  static inline clang::clangd::SymbolID getEmptyKey() {
-static clang::clangd::SymbolID EmptyKey("EMPTYKEY");
-return EmptyKey;
-  }
-  static inline clang::clangd::SymbolID getTombstoneKey() {
-static clang::clangd::SymbolID TombstoneKey("TOMBSTONEKEY");
-return TombstoneKey;
-  }
-  static unsigned getHashValue(const clang::clangd::SymbolID &Sym) {
-return hash_value(Sym);
-  }
-  static bool isEqual(const clang::clangd::SymbolID &LHS,
-  const clang::clangd::SymbolID &RHS) {
-return LHS == RHS;
-  }
-};
-} // namespace llvm
-namespace clang {
-namespace clangd {
-
 // Describes the source of information about a symbol.
 // Mainly useful for debugging, e.g. understanding code completion reuslts.
 // This is a bitfield as information can be combined from several sources.

Modified: clang-tools-extra/trunk/clangd/index/SymbolID.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolID.h?rev=355081&r1=355080&r2=355081&view=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolID.h (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolID.h Thu Feb 28 03:00:44 2019
@@ -9,6 +9,7 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOLID_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOLID_H
 
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
@@ -61,4 +62,25 @@ llvm::raw_ostream &operator<<(llvm::raw_
 } // namespace clangd
 } // namespace clang
 
+namespace llvm {
+// Support SymbolIDs as DenseMap keys.
+template <> struct DenseMapInfo {
+  static inline clang::clangd::SymbolID getEmptyKey() {
+static clang::clangd::SymbolID EmptyKey("EMPTYKEY");
+return EmptyKey;
+  }
+  static inline clang::clangd::SymbolID getTombstoneKey() {
+static clang::clangd::SymbolID TombstoneKey("TOMBSTONEKEY");
+return TombstoneKey;
+  }
+  static unsigned getHashValue(const clang::clangd::SymbolID &Sym) {
+return hash_value(Sym);
+  }
+  static bool isEqual(const clang::clangd::SymbolID &LHS,
+  const clang::clangd::SymbolID &RHS) {
+return LHS == RHS;
+  }
+};
+} // namespace llvm
+
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOLID_H


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

[PATCH] D58236: Make address space conversions a bit stricter.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

> Along those lines, in general, the normal C rules should allow casting `foo*` 
> to `bar*` for any object types foo and bar, even if foo and bar are pointers 
> with address spaces, like `__local int *` and `__global int *`.  I don't see 
> anything in the OpenCL standard that would contradict this.  It looks like 
> this patch changes that?

s6.5 of OpenCL spec is very explicit about this case: "A pointer  to  address  
space  A  can  only  be  assigned  to  a  pointer  to  the  same  address  
space  A  or  a pointer  to  the  generic  address  space.  Casting  a  pointer 
 to  address  space  A  to  a  pointer  to  address space B is illegal if A and 
B are named address spaces and A is not the same as B". Although there is a 
typo with missing first occurrence of B.

The behavior with generic is explained in s6.5.5.

Btw, this patch doesn't change anything in the address space conversions of 
pointers in OpenCL in general but just changes the behavior of nested pointers 
for which spec doesn't say anything. I think trying to reject code that is 
doing something dangerous is a good thing!


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

https://reviews.llvm.org/D58236



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

[PATCH] D58768: Moved SymbolLocation into its own header and implementation file

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

I think another option here is to move Symbol+SymbolLocations to a Symbol.h 
library. SymbolLocation is often used along with Symbol.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58768



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

[clang-tools-extra] r355082 - Moved SymbolLocation into its own header and implementation file

[Dmitri Gribenko via cfe-commits]

Author: gribozavr
Date: Thu Feb 28 03:02:01 2019
New Revision: 355082

URL: http://llvm.org/viewvc/llvm-project?rev=355082&view=rev
Log:
Moved SymbolLocation into its own header and implementation file

Reviewers: ioeric

Subscribers: mgorny, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

Added:
clang-tools-extra/trunk/clangd/index/SymbolLocation.cpp
clang-tools-extra/trunk/clangd/index/SymbolLocation.h
Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/clangd/index/Index.cpp
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/Merge.cpp
clang-tools-extra/trunk/clangd/index/Serialization.cpp
clang-tools-extra/trunk/clangd/index/Serialization.h
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/clangd/index/YAMLSerialization.cpp

Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=355082&r1=355081&r2=355082&view=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Thu Feb 28 03:02:01 2019
@@ -63,6 +63,7 @@ add_clang_library(clangDaemon
   index/MemIndex.cpp
   index/Merge.cpp
   index/SymbolID.cpp
+  index/SymbolLocation.cpp
   index/Serialization.cpp
   index/SymbolCollector.cpp
   index/YAMLSerialization.cpp

Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=355082&r1=355081&r2=355082&view=diff
==
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Thu Feb 28 03:02:01 2019
@@ -10,8 +10,8 @@
 #include "Logger.h"
 #include "SourceCode.h"
 #include "URI.h"
-#include "index/Index.h"
 #include "index/Merge.h"
+#include "index/SymbolLocation.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Index/IndexDataConsumer.h"

Modified: clang-tools-extra/trunk/clangd/index/Index.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.cpp?rev=355082&r1=355081&r2=355082&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Index.cpp Thu Feb 28 03:02:01 2019
@@ -16,30 +16,6 @@
 namespace clang {
 namespace clangd {
 
-constexpr uint32_t SymbolLocation::Position::MaxLine;
-constexpr uint32_t SymbolLocation::Position::MaxColumn;
-void SymbolLocation::Position::setLine(uint32_t L) {
-  if (L > MaxLine) {
-Line = MaxLine;
-return;
-  }
-  Line = L;
-}
-void SymbolLocation::Position::setColumn(uint32_t Col) {
-  if (Col > MaxColumn) {
-Column = MaxColumn;
-return;
-  }
-  Column = Col;
-}
-
-llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const SymbolLocation &L) {
-  if (!L)
-return OS << "(none)";
-  return OS << L.FileURI << "[" << L.Start.line() << ":" << L.Start.column()
-<< "-" << L.End.line() << ":" << L.End.column() << ")";
-}
-
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, SymbolOrigin O) {
   if (O == SymbolOrigin::Unknown)
 return OS << "unknown";

Modified: clang-tools-extra/trunk/clangd/index/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=355082&r1=355081&r2=355082&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.h (original)
+++ clang-tools-extra/trunk/clangd/index/Index.h Thu Feb 28 03:02:01 2019
@@ -11,6 +11,7 @@
 
 #include "ExpectedTypes.h"
 #include "SymbolID.h"
+#include "SymbolLocation.h"
 #include "clang/Index/IndexSymbol.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/DenseMap.h"
@@ -30,70 +31,6 @@
 namespace clang {
 namespace clangd {
 
-struct SymbolLocation {
-  // Specify a position (Line, Column) of symbol. Using Line/Column allows us 
to
-  // build LSP responses without reading the file content.
-  //
-  // Position is encoded into 32 bits to save space.
-  // If Line/Column overflow, the value will be their maximum value.
-  struct Position {
-Position() : Line(0), Column(0) {}
-void setLine(uint32_t Line);
-uint32_t line() const { return Line; }
-void setColumn(uint32_t Column);
-uint32_t column() const { return Column; }
-
-bool hasOverflow() const {
-  return Line >= MaxLine || Column >= MaxColumn;
-}
-
-static constexpr uint32_t MaxLine = (1 << 20) - 1;
-static constexpr uint32_t MaxColumn = (1 << 12) - 1;
-
-  private:
-uint32_t Line : 20; // 0-based
-// Using U

[PATCH] D58768: Moved SymbolLocation into its own header and implementation file

[Dmitri Gribenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL355082: Moved SymbolLocation into its own header and 
implementation file (authored by gribozavr, committed by ).
Herald added subscribers: llvm-commits, ilya-biryukov.
Herald added a project: LLVM.

Changed prior to commit:
  https://reviews.llvm.org/D58768?vs=188695&id=188699#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58768

Files:
  clang-tools-extra/trunk/clangd/CMakeLists.txt
  clang-tools-extra/trunk/clangd/XRefs.cpp
  clang-tools-extra/trunk/clangd/index/Index.cpp
  clang-tools-extra/trunk/clangd/index/Index.h
  clang-tools-extra/trunk/clangd/index/Merge.cpp
  clang-tools-extra/trunk/clangd/index/Serialization.cpp
  clang-tools-extra/trunk/clangd/index/Serialization.h
  clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
  clang-tools-extra/trunk/clangd/index/SymbolLocation.cpp
  clang-tools-extra/trunk/clangd/index/SymbolLocation.h
  clang-tools-extra/trunk/clangd/index/YAMLSerialization.cpp

Index: clang-tools-extra/trunk/clangd/index/Index.cpp
===
--- clang-tools-extra/trunk/clangd/index/Index.cpp
+++ clang-tools-extra/trunk/clangd/index/Index.cpp
@@ -16,30 +16,6 @@
 namespace clang {
 namespace clangd {
 
-constexpr uint32_t SymbolLocation::Position::MaxLine;
-constexpr uint32_t SymbolLocation::Position::MaxColumn;
-void SymbolLocation::Position::setLine(uint32_t L) {
-  if (L > MaxLine) {
-Line = MaxLine;
-return;
-  }
-  Line = L;
-}
-void SymbolLocation::Position::setColumn(uint32_t Col) {
-  if (Col > MaxColumn) {
-Column = MaxColumn;
-return;
-  }
-  Column = Col;
-}
-
-llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const SymbolLocation &L) {
-  if (!L)
-return OS << "(none)";
-  return OS << L.FileURI << "[" << L.Start.line() << ":" << L.Start.column()
-<< "-" << L.End.line() << ":" << L.End.column() << ")";
-}
-
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, SymbolOrigin O) {
   if (O == SymbolOrigin::Unknown)
 return OS << "unknown";
Index: clang-tools-extra/trunk/clangd/index/SymbolLocation.h
===
--- clang-tools-extra/trunk/clangd/index/SymbolLocation.h
+++ clang-tools-extra/trunk/clangd/index/SymbolLocation.h
@@ -0,0 +1,88 @@
+//===--- SymbolLocation.h *- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOL_LOCATION_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOL_LOCATION_H
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+
+namespace clang {
+namespace clangd {
+
+struct SymbolLocation {
+  // Specify a position (Line, Column) of symbol. Using Line/Column allows us to
+  // build LSP responses without reading the file content.
+  //
+  // Position is encoded into 32 bits to save space.
+  // If Line/Column overflow, the value will be their maximum value.
+  struct Position {
+Position() : Line(0), Column(0) {}
+void setLine(uint32_t Line);
+uint32_t line() const { return Line; }
+void setColumn(uint32_t Column);
+uint32_t column() const { return Column; }
+
+bool hasOverflow() const {
+  return Line >= MaxLine || Column >= MaxColumn;
+}
+
+static constexpr uint32_t MaxLine = (1 << 20) - 1;
+static constexpr uint32_t MaxColumn = (1 << 12) - 1;
+
+  private:
+uint32_t Line : 20; // 0-based
+// Using UTF-16 code units.
+uint32_t Column : 12; // 0-based
+  };
+
+  /// The symbol range, using half-open range [Start, End).
+  Position Start;
+  Position End;
+
+  explicit operator bool() const { return !llvm::StringRef(FileURI).empty(); }
+
+  // The URI of the source file where a symbol occurs.
+  // The string must be null-terminated.
+  //
+  // We avoid using llvm::StringRef here to save memory.
+  // WARNING: unless you know what you are doing, it is recommended to use it
+  // via llvm::StringRef.
+  const char *FileURI = "";
+};
+
+inline bool operator==(const SymbolLocation::Position &L,
+   const SymbolLocation::Position &R) {
+  return std::make_tuple(L.line(), L.column()) ==
+ std::make_tuple(R.line(), R.column());
+}
+inline bool operator<(const SymbolLocation::Position &L,
+  const SymbolLocation::Position &R) {
+  return std::make_tuple(L.line(), L.column()) <
+ std::make_tuple(R.line(), R.column());
+}
+inline bool operator==(const SymbolLocation &L, const SymbolLocation &R) {
+  assert(L.FileURI && R.FileURI);
+  return !std::strcmp(L.FileURI

[PATCH] D56830: Prototype for include-fixer workflow in clangd. [NOT FOR REVIEW]

[Eric Liu via Phabricator via cfe-commits]

ioeric abandoned this revision.
ioeric added a comment.
Herald added a subscriber: jdoerfert.
Herald added a project: clang.

Include-fixer has been landed in clangd.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56830



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

[PATCH] D58769: Moved DenseMap support for SymbolID into SymbolID.h

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr closed this revision.
gribozavr added a comment.

Committed as https://reviews.llvm.org/rL355081.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58769



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

[PATCH] D58772: [clangd] Enable SuggestMissingIncludes by default.

[Eric Liu via Phabricator via cfe-commits]

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

This seems to work stably now. Turn on by default.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D58772

Files:
  clangd/tool/ClangdMain.cpp


Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -217,7 +217,7 @@
 "suggest-missing-includes",
 llvm::cl::desc("Attempts to fix diagnostic errors caused by missing "
"includes using index."),
-llvm::cl::init(false));
+llvm::cl::init(true));
 
 namespace {
 


Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -217,7 +217,7 @@
 "suggest-missing-includes",
 llvm::cl::desc("Attempts to fix diagnostic errors caused by missing "
"includes using index."),
-llvm::cl::init(false));
+llvm::cl::init(true));
 
 namespace {
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58773: Moved SymbolOrigin into its own header and implementation file

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr created this revision.
gribozavr added a reviewer: ioeric.
Herald added subscribers: cfe-commits, jdoerfert, kadircet, arphaman, jkorous, 
mgorny.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58773

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/CodeComplete.h
  clang-tools-extra/clangd/index/FileIndex.cpp
  clang-tools-extra/clangd/index/Index.cpp
  clang-tools-extra/clangd/index/Index.h
  clang-tools-extra/clangd/index/IndexAction.cpp
  clang-tools-extra/clangd/index/Merge.cpp
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/index/SymbolCollector.h
  clang-tools-extra/clangd/index/SymbolOrigin.cpp
  clang-tools-extra/clangd/index/SymbolOrigin.h
  clang-tools-extra/clangd/index/YAMLSerialization.cpp

Index: clang-tools-extra/clangd/index/YAMLSerialization.cpp
===
--- clang-tools-extra/clangd/index/YAMLSerialization.cpp
+++ clang-tools-extra/clangd/index/YAMLSerialization.cpp
@@ -15,6 +15,7 @@
 #include "Index.h"
 #include "Serialization.h"
 #include "SymbolLocation.h"
+#include "SymbolOrigin.h"
 #include "Trace.h"
 #include "dex/Dex.h"
 #include "llvm/ADT/Optional.h"
Index: clang-tools-extra/clangd/index/SymbolOrigin.h
===
--- /dev/null
+++ clang-tools-extra/clangd/index/SymbolOrigin.h
@@ -0,0 +1,47 @@
+//===--- SymbolOrigin.h --*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOL_ORIGIN_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOL_ORIGIN_H
+
+#include "llvm/Support/raw_ostream.h"
+#include 
+
+namespace clang {
+namespace clangd {
+
+// Describes the source of information about a symbol.
+// Mainly useful for debugging, e.g. understanding code completion reuslts.
+// This is a bitfield as information can be combined from several sources.
+enum class SymbolOrigin : uint8_t {
+  Unknown = 0,
+  AST = 1 << 0, // Directly from the AST (indexes should not set this).
+  Dynamic = 1 << 1, // From the dynamic index of opened files.
+  Static = 1 << 2,  // From the static, externally-built index.
+  Merge = 1 << 3,   // A non-trivial index merge was performed.
+  // Remaining bits reserved for index implementations.
+};
+
+inline SymbolOrigin operator|(SymbolOrigin A, SymbolOrigin B) {
+  return static_cast(static_cast(A) |
+   static_cast(B));
+}
+inline SymbolOrigin &operator|=(SymbolOrigin &A, SymbolOrigin B) {
+  return A = A | B;
+}
+inline SymbolOrigin operator&(SymbolOrigin A, SymbolOrigin B) {
+  return static_cast(static_cast(A) &
+   static_cast(B));
+}
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, SymbolOrigin);
+
+} // namespace clangd
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOL_ORIGIN_H
Index: clang-tools-extra/clangd/index/SymbolOrigin.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/index/SymbolOrigin.cpp
@@ -0,0 +1,25 @@
+//===--- SymbolOrigin.cpp *- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "SymbolOrigin.h"
+
+namespace clang {
+namespace clangd {
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, SymbolOrigin O) {
+  if (O == SymbolOrigin::Unknown)
+return OS << "unknown";
+  constexpr static char Sigils[] = "ADSM4567";
+  for (unsigned I = 0; I < sizeof(Sigils); ++I)
+if (static_cast(O) & 1u << I)
+  OS << Sigils[I];
+  return OS;
+}
+
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/index/SymbolCollector.h
===
--- clang-tools-extra/clangd/index/SymbolCollector.h
+++ clang-tools-extra/clangd/index/SymbolCollector.h
@@ -10,6 +10,7 @@
 
 #include "CanonicalIncludes.h"
 #include "Index.h"
+#include "SymbolOrigin.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/Basic/SourceLocation.h"
@@ -143,4 +144,5 @@
 
 } // namespace clangd
 } // namespace clang
+
 #endif
Index: clang-tools-extra/clangd/index/Serialization.cpp
===
--- clang-tools-extra/clangd/index/Serialization.cpp
+++ clang-tools-extra/clangd/index/Serializati

[PATCH] D58773: Moved SymbolOrigin into its own header and implementation file

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

I'm not sure if SymbolOrigin is interesting enough to be its own library. It's 
usually only used along with Symbol. Maybe we could pull a library Symbol.h 
instead and put SymbolOrigin by Symbol?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58773



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

[PATCH] D58060: Fix diagnostic for addr spaces in static_cast

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

Btw, I have changed the diagnostic wording... does this change make sense now?


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

https://reviews.llvm.org/D58060



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

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D58731#1412930 , @lewmpk wrote:

> cleaned up documentation


Are you planning on landing this anytime soon given that it was accepted? I 
would like to land D57087: [clang-tidy] add OverrideMacro to 
modernize-use-override check  but I suspect 
either you are I will need to rebase, I'm just trying to be nice and ask if 
you'd like to land first?


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

https://reviews.llvm.org/D58731



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

[PATCH] D57087: [clang-tidy] add OverrideMacro to modernize-use-override check

[Lewis Clark via Phabricator via cfe-commits]

lewmpk added a comment.

I'm happy to land this ASAP but I don't have commit rights


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

https://reviews.llvm.org/D57087



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

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

[Lewis Clark via Phabricator via cfe-commits]

lewmpk added a comment.

I'm happy to land this ASAP but I don't have commit rights


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

https://reviews.llvm.org/D58731



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

[PATCH] D58774: Moved Symbol into its own header and implementation file

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr created this revision.
gribozavr added a reviewer: ioeric.
Herald added subscribers: cfe-commits, jdoerfert, kadircet, arphaman, jkorous, 
mgorny.
Herald added a project: clang.
gribozavr added a parent revision: D58773: Moved SymbolOrigin into its own 
header and implementation file.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58774

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Headers.h
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/index/FileIndex.cpp
  clang-tools-extra/clangd/index/Index.cpp
  clang-tools-extra/clangd/index/Index.h
  clang-tools-extra/clangd/index/Symbol.cpp
  clang-tools-extra/clangd/index/Symbol.h

Index: clang-tools-extra/clangd/index/Symbol.h
===
--- /dev/null
+++ clang-tools-extra/clangd/index/Symbol.h
@@ -0,0 +1,172 @@
+//===--- Symbol.h *- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOL_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOL_H
+
+#include "SymbolID.h"
+#include "SymbolLocation.h"
+#include "SymbolOrigin.h"
+#include "clang/Index/IndexSymbol.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace clang {
+namespace clangd {
+
+/// The class presents a C++ symbol, e.g. class, function.
+///
+/// WARNING: Symbols do not own much of their underlying data - typically
+/// strings are owned by a SymbolSlab. They should be treated as non-owning
+/// references. Copies are shallow.
+///
+/// When adding new unowned data fields to Symbol, remember to update:
+///   - SymbolSlab::Builder in Index.cpp, to copy them to the slab's storage.
+///   - mergeSymbol in Merge.cpp, to properly combine two Symbols.
+///
+/// A fully documented symbol can be split as:
+/// size_type std::map::count(const K& key) const
+/// | Return  | Scope |Name|Signature |
+/// We split up these components to allow display flexibility later.
+struct Symbol {
+  /// The ID of the symbol.
+  SymbolID ID;
+  /// The symbol information, like symbol kind.
+  index::SymbolInfo SymInfo;
+  /// The unqualified name of the symbol, e.g. "bar" (for ns::bar).
+  llvm::StringRef Name;
+  /// The containing namespace. e.g. "" (global), "ns::" (top-level namespace).
+  llvm::StringRef Scope;
+  /// The location of the symbol's definition, if one was found.
+  /// This just covers the symbol name (e.g. without class/function body).
+  SymbolLocation Definition;
+  /// The location of the preferred declaration of the symbol.
+  /// This just covers the symbol name.
+  /// This may be the same as Definition.
+  ///
+  /// A C++ symbol may have multiple declarations, and we pick one to prefer.
+  ///   * For classes, the canonical declaration should be the definition.
+  ///   * For non-inline functions, the canonical declaration typically appears
+  /// in the ".h" file corresponding to the definition.
+  SymbolLocation CanonicalDeclaration;
+  /// The number of translation units that reference this symbol from their main
+  /// file. This number is only meaningful if aggregated in an index.
+  unsigned References = 0;
+  /// Where this symbol came from. Usually an index provides a constant value.
+  SymbolOrigin Origin = SymbolOrigin::Unknown;
+  /// A brief description of the symbol that can be appended in the completion
+  /// candidate list. For example, "(X x, Y y) const" is a function signature.
+  /// Only set when the symbol is indexed for completion.
+  llvm::StringRef Signature;
+  /// What to insert when completing this symbol, after the symbol name.
+  /// This is in LSP snippet syntax (e.g. "({$0})" for a no-args function).
+  /// (When snippets are disabled, the symbol name alone is used).
+  /// Only set when the symbol is indexed for completion.
+  llvm::StringRef CompletionSnippetSuffix;
+  /// Documentation including comment for the symbol declaration.
+  llvm::StringRef Documentation;
+  /// Type when this symbol is used in an expression. (Short display form).
+  /// e.g. return type of a function, or type of a variable.
+  /// Only set when the symbol is indexed for completion.
+  llvm::StringRef ReturnType;
+
+  /// Raw representation of the OpaqueType of the symbol, used for scoring
+  /// purposes.
+  /// Only set when the symbol is indexed for completion.
+  llvm::StringRef Type;
+
+  struct IncludeHeaderWithReferences {
+IncludeHeaderWithReferences() = default;
+
+IncludeHeaderWithReferences(llvm::StringRef IncludeHeader,
+unsigned References)
+ 

[PATCH] D58773: Moved SymbolOrigin into its own header and implementation file

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added a comment.

SymbolOrigin is used by itself, for example, in CodeComplete.h to define 
`struct CodeCompletion`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58773



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

[PATCH] D58773: Moved SymbolOrigin into its own header and implementation file

[Eric Liu via Phabricator via cfe-commits]

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

In D58773#1413486 , @gribozavr wrote:

> SymbolOrigin is used by itself, for example, in CodeComplete.h to define 
> `struct CodeCompletion`.


Fair enough. lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58773



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

[PATCH] D58666: [OpenCL] Undefine cl_intel_planar_yuv extension

[Dmitry Sidorov via Phabricator via cfe-commits]

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



Comment at: test/SemaOpenCL/extension-begin.cl:26
+
 #ifndef IMPLICIT_INCLUDE
 #include "extension-begin.h"

Anastasia wrote:
> sidorovd wrote:
> > Anastasia wrote:
> > > sidorovd wrote:
> > > > Anastasia wrote:
> > > > > Can we also test that macro `my_ext` is not defined here but defined 
> > > > > above?
> > > > > 
> > > > > It seems we are not testing anything like this...
> > > > #pragma OPENCL EXTENSION my_ext : begin doesn't define an appropriate 
> > > > macro. And so cl-ext=+my_ext.
> > > But don't you need to expose the definition of it?
> > Certainly I need, but now the only proper way to do so is by adding an 
> > extension via adding it in OpenCLExtensions.def. Previously we decided to 
> > avoid adding an extension directly into clang, so with a new approach I'd 
> > prefer not to add a definition of the macro in the header but define it 
> > somewhere else, otherwise the macro becomes defined  where it's not 
> > supposed to be (even for ARM and AMD =) ). 
> However, my understanding is that you should define the macro when you define 
> the extension itself.
> 
> 
> ```
> #pragma OPENCL EXTENSION my_ext : begin
> #define my_ext
> ...
> #pragma OPENCL EXTENSION my_ext : end
> ```
> 
> does it not work for you?
```
#pragma OPENCL EXTENSION my_ext : begin
#define my_ext
...
#pragma OPENCL EXTENSION my_ext : end
```
 ^
 I
 It defines the macro regardless begin : end range, so one can consider that 
it's the same code as:

```
#define my_ext
#pragma OPENCL EXTENSION my_ext : begin
...
#pragma OPENCL EXTENSION my_ext : end
```

I agree, that it's a better way to define a macro with defining an appropriate 
extension itself, but it makes it be defined where it's not supposed to be (at 
least from my perspective).

To sum up:
1. #pragma OPENCL EXTENSION my_ext : begin ... end  - makes an extension known 
for clang if the header included and if the extension enabled; doesn't affect a 
macro definition anyhow;
2.  OPENCLEXT_INTERNAL(my_ext, *version*, ~0U) -  makes an extension known for 
clang and defines an appropriate macro if the extension enabled.

I believe we are okay not to define cl_intel_planar_yuv macro in the header - 
just make the extension known for clang. 


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

https://reviews.llvm.org/D58666



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

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D58731#1413476 , @lewmpk wrote:

> I'm happy to land this ASAP but I don't have commit rights


So one of us could land it for you.. (I've not personally done that before as 
I'm a bit green too! but I do have commit rights)

Its pretty easy to get commit rights, and given your looking at multiple issues 
I'd recommend it..


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

https://reviews.llvm.org/D58731



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

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

[David Zarzycki via Phabricator via cfe-commits]

davezarzycki added a comment.

Can we please revert this code? One cannot use dynamic_cast on a char *. Did 
you mean reinterpret_cast?


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

https://reviews.llvm.org/D58418



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

[clang-tools-extra] r355086 - Moved SymbolOrigin into its own header and implementation file

[Dmitri Gribenko via cfe-commits]

Author: gribozavr
Date: Thu Feb 28 04:31:49 2019
New Revision: 355086

URL: http://llvm.org/viewvc/llvm-project?rev=355086&view=rev
Log:
Moved SymbolOrigin into its own header and implementation file

Reviewers: ioeric

Subscribers: mgorny, jkorous, arphaman, kadircet, jdoerfert, cfe-commits

Tags: #clang

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

Added:
clang-tools-extra/trunk/clangd/index/SymbolOrigin.cpp
clang-tools-extra/trunk/clangd/index/SymbolOrigin.h
Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/clangd/CodeComplete.h
clang-tools-extra/trunk/clangd/index/FileIndex.cpp
clang-tools-extra/trunk/clangd/index/Index.cpp
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/IndexAction.cpp
clang-tools-extra/trunk/clangd/index/Merge.cpp
clang-tools-extra/trunk/clangd/index/Serialization.cpp
clang-tools-extra/trunk/clangd/index/SymbolCollector.h
clang-tools-extra/trunk/clangd/index/YAMLSerialization.cpp

Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=355086&r1=355085&r2=355086&view=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Thu Feb 28 04:31:49 2019
@@ -62,10 +62,11 @@ add_clang_library(clangDaemon
   index/IndexAction.cpp
   index/MemIndex.cpp
   index/Merge.cpp
-  index/SymbolID.cpp
-  index/SymbolLocation.cpp
   index/Serialization.cpp
   index/SymbolCollector.cpp
+  index/SymbolID.cpp
+  index/SymbolLocation.cpp
+  index/SymbolOrigin.cpp
   index/YAMLSerialization.cpp
 
   index/dex/Dex.cpp

Modified: clang-tools-extra/trunk/clangd/CodeComplete.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.h?rev=355086&r1=355085&r2=355086&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.h (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.h Thu Feb 28 04:31:49 2019
@@ -21,6 +21,7 @@
 #include "Path.h"
 #include "Protocol.h"
 #include "index/Index.h"
+#include "index/SymbolOrigin.h"
 #include "clang/Frontend/PrecompiledPreamble.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "clang/Sema/CodeCompleteOptions.h"

Modified: clang-tools-extra/trunk/clangd/index/FileIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/FileIndex.cpp?rev=355086&r1=355085&r2=355086&view=diff
==
--- clang-tools-extra/trunk/clangd/index/FileIndex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/FileIndex.cpp Thu Feb 28 04:31:49 2019
@@ -14,6 +14,7 @@
 #include "index/Index.h"
 #include "index/MemIndex.h"
 #include "index/Merge.h"
+#include "index/SymbolOrigin.h"
 #include "index/dex/Dex.h"
 #include "clang/Index/IndexingAction.h"
 #include "clang/Lex/MacroInfo.h"

Modified: clang-tools-extra/trunk/clangd/index/Index.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.cpp?rev=355086&r1=355085&r2=355086&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Index.cpp Thu Feb 28 04:31:49 2019
@@ -16,16 +16,6 @@
 namespace clang {
 namespace clangd {
 
-llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, SymbolOrigin O) {
-  if (O == SymbolOrigin::Unknown)
-return OS << "unknown";
-  constexpr static char Sigils[] = "ADSM4567";
-  for (unsigned I = 0; I < sizeof(Sigils); ++I)
-if (static_cast(O) & 1u << I)
-  OS << Sigils[I];
-  return OS;
-}
-
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, Symbol::SymbolFlag F) {
   if (F == Symbol::None)
 return OS << "None";

Modified: clang-tools-extra/trunk/clangd/index/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=355086&r1=355085&r2=355086&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.h (original)
+++ clang-tools-extra/trunk/clangd/index/Index.h Thu Feb 28 04:31:49 2019
@@ -12,6 +12,7 @@
 #include "ExpectedTypes.h"
 #include "SymbolID.h"
 #include "SymbolLocation.h"
+#include "SymbolOrigin.h"
 #include "clang/Index/IndexSymbol.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/DenseMap.h"
@@ -31,30 +32,6 @@
 namespace clang {
 namespace clangd {
 
-// Describes the source of information about a symbol.
-// Mainly useful for debugging, e.g. understanding code completion reuslts.
-// This is a bitfield as information can be combined from several sources.
-enum class SymbolOrigin : uint8_t {
-  Unknown = 0,
-  AST = 1 << 0, // Directly from the

[PATCH] D58773: Moved SymbolOrigin into its own header and implementation file

[Dmitri Gribenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE355086: Moved SymbolOrigin into its own header and 
implementation file (authored by gribozavr, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58773?vs=188703&id=188709#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58773

Files:
  clangd/CMakeLists.txt
  clangd/CodeComplete.h
  clangd/index/FileIndex.cpp
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/IndexAction.cpp
  clangd/index/Merge.cpp
  clangd/index/Serialization.cpp
  clangd/index/SymbolCollector.h
  clangd/index/SymbolOrigin.cpp
  clangd/index/SymbolOrigin.h
  clangd/index/YAMLSerialization.cpp

Index: clangd/CodeComplete.h
===
--- clangd/CodeComplete.h
+++ clangd/CodeComplete.h
@@ -21,6 +21,7 @@
 #include "Path.h"
 #include "Protocol.h"
 #include "index/Index.h"
+#include "index/SymbolOrigin.h"
 #include "clang/Frontend/PrecompiledPreamble.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "clang/Sema/CodeCompleteOptions.h"
Index: clangd/index/SymbolOrigin.h
===
--- clangd/index/SymbolOrigin.h
+++ clangd/index/SymbolOrigin.h
@@ -0,0 +1,47 @@
+//===--- SymbolOrigin.h --*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOL_ORIGIN_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOL_ORIGIN_H
+
+#include "llvm/Support/raw_ostream.h"
+#include 
+
+namespace clang {
+namespace clangd {
+
+// Describes the source of information about a symbol.
+// Mainly useful for debugging, e.g. understanding code completion reuslts.
+// This is a bitfield as information can be combined from several sources.
+enum class SymbolOrigin : uint8_t {
+  Unknown = 0,
+  AST = 1 << 0, // Directly from the AST (indexes should not set this).
+  Dynamic = 1 << 1, // From the dynamic index of opened files.
+  Static = 1 << 2,  // From the static, externally-built index.
+  Merge = 1 << 3,   // A non-trivial index merge was performed.
+  // Remaining bits reserved for index implementations.
+};
+
+inline SymbolOrigin operator|(SymbolOrigin A, SymbolOrigin B) {
+  return static_cast(static_cast(A) |
+   static_cast(B));
+}
+inline SymbolOrigin &operator|=(SymbolOrigin &A, SymbolOrigin B) {
+  return A = A | B;
+}
+inline SymbolOrigin operator&(SymbolOrigin A, SymbolOrigin B) {
+  return static_cast(static_cast(A) &
+   static_cast(B));
+}
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, SymbolOrigin);
+
+} // namespace clangd
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOL_ORIGIN_H
Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -10,6 +10,7 @@
 
 #include "CanonicalIncludes.h"
 #include "Index.h"
+#include "SymbolOrigin.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/Basic/SourceLocation.h"
@@ -143,4 +144,5 @@
 
 } // namespace clangd
 } // namespace clang
+
 #endif
Index: clangd/index/SymbolOrigin.cpp
===
--- clangd/index/SymbolOrigin.cpp
+++ clangd/index/SymbolOrigin.cpp
@@ -0,0 +1,25 @@
+//===--- SymbolOrigin.cpp *- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "SymbolOrigin.h"
+
+namespace clang {
+namespace clangd {
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, SymbolOrigin O) {
+  if (O == SymbolOrigin::Unknown)
+return OS << "unknown";
+  constexpr static char Sigils[] = "ADSM4567";
+  for (unsigned I = 0; I < sizeof(Sigils); ++I)
+if (static_cast(O) & 1u << I)
+  OS << Sigils[I];
+  return OS;
+}
+
+} // namespace clangd
+} // namespace clang
Index: clangd/index/Serialization.cpp
===
--- clangd/index/Serialization.cpp
+++ clangd/index/Serialization.cpp
@@ -10,6 +10,7 @@
 #include "Logger.h"
 #include "RIFF.h"
 #include "SymbolLocation.h"
+#include "SymbolOrigin.h"
 #include "Trace.h"
 #include "dex/Dex.h"
 #include "llvm/Support/Compression.h"
Index: clangd/inde

[PATCH] D58774: Moved Symbol into its own header and implementation file

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

The design of Symbol and SymbolSlab are closely related. I would suggest 
putting them close together in the same library.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58774



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

[PATCH] D58774: Moved Symbol into its own header and implementation file

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr updated this revision to Diff 188712.
gribozavr added a comment.

Moving SymbolSlab as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58774

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/CodeComplete.h
  clang-tools-extra/clangd/Headers.h
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/IncludeFixer.h
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/index/FileIndex.h
  clang-tools-extra/clangd/index/Index.cpp
  clang-tools-extra/clangd/index/Index.h
  clang-tools-extra/clangd/index/Merge.cpp
  clang-tools-extra/clangd/index/Serialization.h
  clang-tools-extra/clangd/index/Symbol.cpp
  clang-tools-extra/clangd/index/Symbol.h

Index: clang-tools-extra/clangd/index/Symbol.h
===
--- /dev/null
+++ clang-tools-extra/clangd/index/Symbol.h
@@ -0,0 +1,231 @@
+//===--- Symbol.h *- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOL_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOL_H
+
+#include "SymbolID.h"
+#include "SymbolLocation.h"
+#include "SymbolOrigin.h"
+#include "clang/Index/IndexSymbol.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/StringSaver.h"
+
+namespace clang {
+namespace clangd {
+
+/// The class presents a C++ symbol, e.g. class, function.
+///
+/// WARNING: Symbols do not own much of their underlying data - typically
+/// strings are owned by a SymbolSlab. They should be treated as non-owning
+/// references. Copies are shallow.
+///
+/// When adding new unowned data fields to Symbol, remember to update:
+///   - SymbolSlab::Builder in Index.cpp, to copy them to the slab's storage.
+///   - mergeSymbol in Merge.cpp, to properly combine two Symbols.
+///
+/// A fully documented symbol can be split as:
+/// size_type std::map::count(const K& key) const
+/// | Return  | Scope |Name|Signature |
+/// We split up these components to allow display flexibility later.
+struct Symbol {
+  /// The ID of the symbol.
+  SymbolID ID;
+  /// The symbol information, like symbol kind.
+  index::SymbolInfo SymInfo;
+  /// The unqualified name of the symbol, e.g. "bar" (for ns::bar).
+  llvm::StringRef Name;
+  /// The containing namespace. e.g. "" (global), "ns::" (top-level namespace).
+  llvm::StringRef Scope;
+  /// The location of the symbol's definition, if one was found.
+  /// This just covers the symbol name (e.g. without class/function body).
+  SymbolLocation Definition;
+  /// The location of the preferred declaration of the symbol.
+  /// This just covers the symbol name.
+  /// This may be the same as Definition.
+  ///
+  /// A C++ symbol may have multiple declarations, and we pick one to prefer.
+  ///   * For classes, the canonical declaration should be the definition.
+  ///   * For non-inline functions, the canonical declaration typically appears
+  /// in the ".h" file corresponding to the definition.
+  SymbolLocation CanonicalDeclaration;
+  /// The number of translation units that reference this symbol from their main
+  /// file. This number is only meaningful if aggregated in an index.
+  unsigned References = 0;
+  /// Where this symbol came from. Usually an index provides a constant value.
+  SymbolOrigin Origin = SymbolOrigin::Unknown;
+  /// A brief description of the symbol that can be appended in the completion
+  /// candidate list. For example, "(X x, Y y) const" is a function signature.
+  /// Only set when the symbol is indexed for completion.
+  llvm::StringRef Signature;
+  /// What to insert when completing this symbol, after the symbol name.
+  /// This is in LSP snippet syntax (e.g. "({$0})" for a no-args function).
+  /// (When snippets are disabled, the symbol name alone is used).
+  /// Only set when the symbol is indexed for completion.
+  llvm::StringRef CompletionSnippetSuffix;
+  /// Documentation including comment for the symbol declaration.
+  llvm::StringRef Documentation;
+  /// Type when this symbol is used in an expression. (Short display form).
+  /// e.g. return type of a function, or type of a variable.
+  /// Only set when the symbol is indexed for completion.
+  llvm::StringRef ReturnType;
+
+  /// Raw representation of the OpaqueType of the symbol, used for scoring
+  /// purposes.
+  /// Only set when the symbol is indexed for completion.
+  llvm::StringRef Type;
+
+  struct IncludeHeaderWithReferences {
+IncludeHeaderWithReferences() = default;
+
+IncludeHeaderWithReferences(llvm

[PATCH] D58774: Moved Symbol into its own header and implementation file

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added a comment.

Moved SymbolSlab as well, PTAL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58774



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

[PATCH] D58774: Moved Symbol into its own header and implementation file

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clang-tools-extra/clangd/index/Index.cpp:19
 
-llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, Symbol::SymbolFlag F) {
-  if (F == Symbol::None)
-return OS << "None";
-  std::string S;
-  if (F & Symbol::Deprecated)
-S += "deprecated|";
-  if (F & Symbol::IndexedForCodeCompletion)
-S += "completion|";
-  return OS << llvm::StringRef(S).rtrim('|');
-}
-
-llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Symbol &S) {
-  return OS << S.Scope << S.Name;
-}
-
-float quality(const Symbol &S) {
-  // This avoids a sharp gradient for tail symbols, and also neatly avoids the
-  // question of whether 0 references means a bad symbol or missing data.
-  if (S.References < 3)
-return 1;
-  return std::log(S.References);
-}
-
 SymbolSlab::const_iterator SymbolSlab::find(const SymbolID &ID) const {
   auto It = std::lower_bound(

should also move the implementation?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58774



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

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In D58731#1413498 , @MyDeveloperDay 
wrote:

> In D58731#1413476 , @lewmpk wrote:
>
> > I'm happy to land this ASAP but I don't have commit rights
>
>
> So one of us could land it for you.. (I've not personally done that before as 
> I'm a bit green too! but I do have commit rights)
>
> Its pretty easy to get commit rights, and given your looking at multiple 
> issues I'd recommend it.. 
> (https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access)


@MyDeveloperDay if you want to try the procedure out, go ahead ;) Otherwise I 
can land this patch now.


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

https://reviews.llvm.org/D58731



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

[PATCH] D58777: [analyzer] Fix an assertation failurure for invalid sourcelocation, add a new debug checker

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

Szelethus created this revision.
Szelethus added reviewers: NoQ, xazax.hun, rnkovacs, baloghadamsoftware, 
Charusso.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, szepet, whisperity.

For a rather short code snippet, if `debug.ReportStmts` (added in this patch) 
was enabled, a bug reporter visitor crashed:

  struct h {
operator int();
  };
  
  int k() {
return h();
  }

  include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h:472: 
clang::ento::PathDiagnosticSpotPiece::PathDiagnosticSpotPiece(const 
clang::ento::PathDiagnosticLocation &, llvm::StringRef, 
PathDiagnosticPiece::Kind, bool): Assertion `Pos.isValid() && 
Pos.asLocation().isValid() && "PathDiagnosticSpotPiece's must have a valid 
location."' failed.

Ultimately, this originated from `PathDiagnosticLocation::createMemberLoc`, as 
it didn't handle the case where it's `MemberExpr` typed parameter returned and 
invalid `SourceLocation` for `MemberExpr::getMemberLoc`. The solution was to 
find any related valid `SourceLocaion`, and `Stmt::getBeginLoc` happens to be 
just that.


Repository:
  rC Clang

https://reviews.llvm.org/D58777

Files:
  docs/analyzer/developer-docs/DebugChecks.rst
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
  lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  test/Analysis/invalid-pos-fix.cpp
  test/Analysis/plist-html-macros.c

Index: test/Analysis/plist-html-macros.c
===
--- test/Analysis/plist-html-macros.c
+++ test/Analysis/plist-html-macros.c
@@ -3,7 +3,10 @@
 
 // RUN: rm -rf %t.dir
 // RUN: mkdir -p %t.dir
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-output=plist-html -o %t.dir/index.plist %s
+//
+// RUN: %clang_analyze_cc1 -o %t.dir/index.plist %s \
+// RUN:   -analyzer-checker=core -analyzer-output=plist-html
+//
 // RUN: ls %t.dir | grep '\.html' | count 1
 // RUN: grep '\.html' %t.dir/index.plist | count 1
 
Index: test/Analysis/invalid-pos-fix.cpp
===
--- /dev/null
+++ test/Analysis/invalid-pos-fix.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-output=plist -o %t.plist \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ReportStmts
+
+struct h {
+  operator int();
+};
+
+int k() {
+  return h(); // expected-warning{{Statement}}
+  // expected-warning@-1{{Statement}}
+  // expected-warning@-2{{Statement}}
+}
Index: lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===
--- lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -571,6 +571,8 @@
 } while (!L.isValid());
   }
 
+  // FIXME: Ironically, this assert actually fails in some cases.
+  //assert(L.isValid());
   return L;
 }
 
@@ -671,7 +673,13 @@
 PathDiagnosticLocation
 PathDiagnosticLocation::createMemberLoc(const MemberExpr *ME,
 const SourceManager &SM) {
-  return PathDiagnosticLocation(ME->getMemberLoc(), SM, SingleLocK);
+
+  assert(ME->getMemberLoc().isValid() || ME->getBeginLoc().isValid());
+
+  if (ME->getMemberLoc().isValid())
+return PathDiagnosticLocation(ME->getMemberLoc(), SM, SingleLocK);
+
+  return PathDiagnosticLocation(ME->getBeginLoc(), SM, SingleLocK);
 }
 
 PathDiagnosticLocation
Index: lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
===
--- lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
+++ lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
@@ -266,3 +266,36 @@
 bool ento::shouldRegisterExplodedGraphViewer(const LangOptions &LO) {
   return true;
 }
+
+//===--===//
+// Emits a report for each and every Stmt.
+//===--===//
+
+namespace {
+
+class ReportStmts : public Checker> {
+  std::unique_ptr BT_stmtLoc;
+
+public:
+  ReportStmts() : BT_stmtLoc(new BuiltinBug(this, "Statement")) {}
+
+  void checkPreStmt(const Stmt *S, CheckerContext &C) const {
+ExplodedNode *Node = C.generateNonFatalErrorNode();
+if (!Node)
+  return;
+
+auto Report = llvm::make_unique(*BT_stmtLoc, "Statement", Node);
+
+C.emitReport(std::move(Report));
+  }
+};
+
+} // end of anonymous namespace
+
+void ento::registerReportStmts(CheckerManager &mgr) {
+  mgr.registerChecker();
+}
+
+bool ento::shouldRegisterReportStmts(const LangOptions &LO) {
+  return true;
+}
Index: include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1017,6 +1017,10 @@
   HelpText<"View Exploded 

[PATCH] D58774: Moved Symbol into its own header and implementation file

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr updated this revision to Diff 188714.
gribozavr added a comment.
Herald added a subscriber: mgrang.

Also moved the SymbolSlab implementation into Symbol.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58774

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/CodeComplete.h
  clang-tools-extra/clangd/Headers.h
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/IncludeFixer.h
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/index/FileIndex.h
  clang-tools-extra/clangd/index/Index.cpp
  clang-tools-extra/clangd/index/Index.h
  clang-tools-extra/clangd/index/Merge.cpp
  clang-tools-extra/clangd/index/Serialization.h
  clang-tools-extra/clangd/index/Symbol.cpp
  clang-tools-extra/clangd/index/Symbol.h

Index: clang-tools-extra/clangd/index/Symbol.h
===
--- /dev/null
+++ clang-tools-extra/clangd/index/Symbol.h
@@ -0,0 +1,231 @@
+//===--- Symbol.h *- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOL_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOL_H
+
+#include "SymbolID.h"
+#include "SymbolLocation.h"
+#include "SymbolOrigin.h"
+#include "clang/Index/IndexSymbol.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/StringSaver.h"
+
+namespace clang {
+namespace clangd {
+
+/// The class presents a C++ symbol, e.g. class, function.
+///
+/// WARNING: Symbols do not own much of their underlying data - typically
+/// strings are owned by a SymbolSlab. They should be treated as non-owning
+/// references. Copies are shallow.
+///
+/// When adding new unowned data fields to Symbol, remember to update:
+///   - SymbolSlab::Builder in Index.cpp, to copy them to the slab's storage.
+///   - mergeSymbol in Merge.cpp, to properly combine two Symbols.
+///
+/// A fully documented symbol can be split as:
+/// size_type std::map::count(const K& key) const
+/// | Return  | Scope |Name|Signature |
+/// We split up these components to allow display flexibility later.
+struct Symbol {
+  /// The ID of the symbol.
+  SymbolID ID;
+  /// The symbol information, like symbol kind.
+  index::SymbolInfo SymInfo;
+  /// The unqualified name of the symbol, e.g. "bar" (for ns::bar).
+  llvm::StringRef Name;
+  /// The containing namespace. e.g. "" (global), "ns::" (top-level namespace).
+  llvm::StringRef Scope;
+  /// The location of the symbol's definition, if one was found.
+  /// This just covers the symbol name (e.g. without class/function body).
+  SymbolLocation Definition;
+  /// The location of the preferred declaration of the symbol.
+  /// This just covers the symbol name.
+  /// This may be the same as Definition.
+  ///
+  /// A C++ symbol may have multiple declarations, and we pick one to prefer.
+  ///   * For classes, the canonical declaration should be the definition.
+  ///   * For non-inline functions, the canonical declaration typically appears
+  /// in the ".h" file corresponding to the definition.
+  SymbolLocation CanonicalDeclaration;
+  /// The number of translation units that reference this symbol from their main
+  /// file. This number is only meaningful if aggregated in an index.
+  unsigned References = 0;
+  /// Where this symbol came from. Usually an index provides a constant value.
+  SymbolOrigin Origin = SymbolOrigin::Unknown;
+  /// A brief description of the symbol that can be appended in the completion
+  /// candidate list. For example, "(X x, Y y) const" is a function signature.
+  /// Only set when the symbol is indexed for completion.
+  llvm::StringRef Signature;
+  /// What to insert when completing this symbol, after the symbol name.
+  /// This is in LSP snippet syntax (e.g. "({$0})" for a no-args function).
+  /// (When snippets are disabled, the symbol name alone is used).
+  /// Only set when the symbol is indexed for completion.
+  llvm::StringRef CompletionSnippetSuffix;
+  /// Documentation including comment for the symbol declaration.
+  llvm::StringRef Documentation;
+  /// Type when this symbol is used in an expression. (Short display form).
+  /// e.g. return type of a function, or type of a variable.
+  /// Only set when the symbol is indexed for completion.
+  llvm::StringRef ReturnType;
+
+  /// Raw representation of the OpaqueType of the symbol, used for scoring
+  /// purposes.
+  /// Only set when the symbol is indexed for completion.
+  llvm::StringRef Type;
+
+  struct IncludeHeaderWithReferences {
+IncludeHeaderWi

[PATCH] D58777: [analyzer] Fix an assertation failurure for invalid sourcelocation, add a new debug checker

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

Szelethus updated this revision to Diff 188713.
Szelethus added a comment.

Added context.


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

https://reviews.llvm.org/D58777

Files:
  docs/analyzer/developer-docs/DebugChecks.rst
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
  lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  test/Analysis/invalid-pos-fix.cpp
  test/Analysis/plist-html-macros.c

Index: test/Analysis/plist-html-macros.c
===
--- test/Analysis/plist-html-macros.c
+++ test/Analysis/plist-html-macros.c
@@ -3,7 +3,10 @@
 
 // RUN: rm -rf %t.dir
 // RUN: mkdir -p %t.dir
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-output=plist-html -o %t.dir/index.plist %s
+//
+// RUN: %clang_analyze_cc1 -o %t.dir/index.plist %s \
+// RUN:   -analyzer-checker=core -analyzer-output=plist-html
+//
 // RUN: ls %t.dir | grep '\.html' | count 1
 // RUN: grep '\.html' %t.dir/index.plist | count 1
 
Index: test/Analysis/invalid-pos-fix.cpp
===
--- /dev/null
+++ test/Analysis/invalid-pos-fix.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-output=plist -o %t.plist \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ReportStmts
+
+struct h {
+  operator int();
+};
+
+int k() {
+  return h(); // expected-warning{{Statement}}
+  // expected-warning@-1{{Statement}}
+  // expected-warning@-2{{Statement}}
+}
Index: lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===
--- lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -571,6 +571,8 @@
 } while (!L.isValid());
   }
 
+  // FIXME: Ironically, this assert actually fails in some cases.
+  //assert(L.isValid());
   return L;
 }
 
@@ -671,7 +673,13 @@
 PathDiagnosticLocation
 PathDiagnosticLocation::createMemberLoc(const MemberExpr *ME,
 const SourceManager &SM) {
-  return PathDiagnosticLocation(ME->getMemberLoc(), SM, SingleLocK);
+
+  assert(ME->getMemberLoc().isValid() || ME->getBeginLoc().isValid());
+
+  if (ME->getMemberLoc().isValid())
+return PathDiagnosticLocation(ME->getMemberLoc(), SM, SingleLocK);
+
+  return PathDiagnosticLocation(ME->getBeginLoc(), SM, SingleLocK);
 }
 
 PathDiagnosticLocation
Index: lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
===
--- lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
+++ lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
@@ -266,3 +266,36 @@
 bool ento::shouldRegisterExplodedGraphViewer(const LangOptions &LO) {
   return true;
 }
+
+//===--===//
+// Emits a report for each and every Stmt.
+//===--===//
+
+namespace {
+
+class ReportStmts : public Checker> {
+  std::unique_ptr BT_stmtLoc;
+
+public:
+  ReportStmts() : BT_stmtLoc(new BuiltinBug(this, "Statement")) {}
+
+  void checkPreStmt(const Stmt *S, CheckerContext &C) const {
+ExplodedNode *Node = C.generateNonFatalErrorNode();
+if (!Node)
+  return;
+
+auto Report = llvm::make_unique(*BT_stmtLoc, "Statement", Node);
+
+C.emitReport(std::move(Report));
+  }
+};
+
+} // end of anonymous namespace
+
+void ento::registerReportStmts(CheckerManager &mgr) {
+  mgr.registerChecker();
+}
+
+bool ento::shouldRegisterReportStmts(const LangOptions &LO) {
+  return true;
+}
Index: include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1017,6 +1017,10 @@
   HelpText<"View Exploded Graphs using GraphViz">,
   Documentation;
 
+def ReportStmts : Checker<"ReportStmts">,
+  HelpText<"Emits a warning for every statement.">,
+  Documentation;
+
 } // end "debug"
 
 
Index: docs/analyzer/developer-docs/DebugChecks.rst
===
--- docs/analyzer/developer-docs/DebugChecks.rst
+++ docs/analyzer/developer-docs/DebugChecks.rst
@@ -285,3 +285,10 @@
 statistics within the analyzer engine. Note the Stats checker (which produces at
 least one bug report per function) may actually change the values reported by
 -analyzer-stats.
+
+Output testing checkers
+===
+
+- debug.ReportStmts reports a warning at **every** statement, making it a very
+  useful tool for testing thoroughly bug report construction and output
+  emission.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56160: [clang-tidy] modernize-use-trailing-return-type check

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

From my side only the nits are left.




Comment at: clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:94
+  bool VisitDeclRefExpr(DeclRefExpr *S) {
+const DeclarationName Name = S->getNameInfo().getName();
+if (!S->getQualifierLoc() && Name.isIdentifier() &&

Nit: `const` is only used for references or pointers.



Comment at: clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:154
+}
+
+break;

Nit: I think this and the next empty line could be ellided. They do not add a 
lot of value and we try to keep the code dense.



Comment at: clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:241
+!ExtendedLeft) {
+  for (int J = static_cast(I) - 1; J >= 0 && IsCV(Tokens[J]); J--)
+ReturnTypeRange.setBegin(Tokens[J].getLocation());

integer bug for big `Tokens.size()`. Please add an `assert(I <= 
size_t(std::numeric_limits::max()) && "Integer overflow detected")` or the 
like.
Optimally `gsl::narrow<>()` could be used, which we do not have in LLVM.


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

https://reviews.llvm.org/D56160



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

[clang-tools-extra] r355088 - Moved Symbol into its own header and implementation file

[Dmitri Gribenko via cfe-commits]

Author: gribozavr
Date: Thu Feb 28 05:23:03 2019
New Revision: 355088

URL: http://llvm.org/viewvc/llvm-project?rev=355088&view=rev
Log:
Moved Symbol into its own header and implementation file

Reviewers: ioeric

Subscribers: mgorny, jkorous, arphaman, kadircet, jdoerfert, cfe-commits

Tags: #clang

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

Added:
clang-tools-extra/trunk/clangd/index/Symbol.cpp
clang-tools-extra/trunk/clangd/index/Symbol.h
Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/CodeComplete.h
clang-tools-extra/trunk/clangd/Headers.h
clang-tools-extra/trunk/clangd/IncludeFixer.cpp
clang-tools-extra/trunk/clangd/IncludeFixer.h
clang-tools-extra/trunk/clangd/index/Background.cpp
clang-tools-extra/trunk/clangd/index/FileIndex.h
clang-tools-extra/trunk/clangd/index/Index.cpp
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/Merge.cpp
clang-tools-extra/trunk/clangd/index/Serialization.h

Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=355088&r1=355087&r2=355088&view=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Thu Feb 28 05:23:03 2019
@@ -63,6 +63,7 @@ add_clang_library(clangDaemon
   index/MemIndex.cpp
   index/Merge.cpp
   index/Serialization.cpp
+  index/Symbol.cpp
   index/SymbolCollector.cpp
   index/SymbolID.cpp
   index/SymbolLocation.cpp

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=355088&r1=355087&r2=355088&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Thu Feb 28 05:23:03 2019
@@ -34,6 +34,7 @@
 #include "Trace.h"
 #include "URI.h"
 #include "index/Index.h"
+#include "index/Symbol.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/Basic/LangOptions.h"

Modified: clang-tools-extra/trunk/clangd/CodeComplete.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.h?rev=355088&r1=355087&r2=355088&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.h (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.h Thu Feb 28 05:23:03 2019
@@ -21,6 +21,7 @@
 #include "Path.h"
 #include "Protocol.h"
 #include "index/Index.h"
+#include "index/Symbol.h"
 #include "index/SymbolOrigin.h"
 #include "clang/Frontend/PrecompiledPreamble.h"
 #include "clang/Sema/CodeCompleteConsumer.h"

Modified: clang-tools-extra/trunk/clangd/Headers.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Headers.h?rev=355088&r1=355087&r2=355088&view=diff
==
--- clang-tools-extra/trunk/clangd/Headers.h (original)
+++ clang-tools-extra/trunk/clangd/Headers.h Thu Feb 28 05:23:03 2019
@@ -12,7 +12,7 @@
 #include "Path.h"
 #include "Protocol.h"
 #include "SourceCode.h"
-#include "index/Index.h"
+#include "index/Symbol.h"
 #include "clang/Format/Format.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/PPCallbacks.h"

Modified: clang-tools-extra/trunk/clangd/IncludeFixer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/IncludeFixer.cpp?rev=355088&r1=355087&r2=355088&view=diff
==
--- clang-tools-extra/trunk/clangd/IncludeFixer.cpp (original)
+++ clang-tools-extra/trunk/clangd/IncludeFixer.cpp Thu Feb 28 05:23:03 2019
@@ -13,6 +13,7 @@
 #include "SourceCode.h"
 #include "Trace.h"
 #include "index/Index.h"
+#include "index/Symbol.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/NestedNameSpecifier.h"

Modified: clang-tools-extra/trunk/clangd/IncludeFixer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/IncludeFixer.h?rev=355088&r1=355087&r2=355088&view=diff
==
--- clang-tools-extra/trunk/clangd/IncludeFixer.h (original)
+++ clang-tools-extra/trunk/clangd/IncludeFixer.h Thu Feb 28 05:23:03 2019
@@ -12,6 +12,7 @@
 #include "Diagnostics.h"
 #include "Headers.h"
 #include "index/Index.h"
+#include "index/Symbol.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/SourceLocation.h"

Modified: clang-tools-extra/trunk/clangd/index/Background.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Back

[PATCH] D58649: Fix inline assembler constraint validation

[Joerg Sonnenberger via Phabricator via cfe-commits]

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



Comment at: include/clang/Basic/TargetInfo.h:860
+  if (!ImmSet.empty())
+return ImmSet.count(Value.getZExtValue()) != 0;
+  return !ImmRange.isConstrained || (Value.sge(ImmRange.Min) && 
Value.sle(ImmRange.Max));

efriedma wrote:
> Isn't the "ImmSet.count" call still wrong?  It looks like it crashes on an 
> 128-bit immediate, and implicitly truncates a 64-bit immediate.  I guess you 
> could check `Value.isSignedIntN(32)`?
I've added that and a test case for truncation as a follow-up.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58649



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

[PATCH] D58774: Moved Symbol into its own header and implementation file

[Dmitri Gribenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE355088: Moved Symbol into its own header and 
implementation file (authored by gribozavr, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58774?vs=188714&id=188715#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58774

Files:
  clangd/CMakeLists.txt
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  clangd/Headers.h
  clangd/IncludeFixer.cpp
  clangd/IncludeFixer.h
  clangd/index/Background.cpp
  clangd/index/FileIndex.h
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/Merge.cpp
  clangd/index/Serialization.h
  clangd/index/Symbol.cpp
  clangd/index/Symbol.h

Index: clangd/CMakeLists.txt
===
--- clangd/CMakeLists.txt
+++ clangd/CMakeLists.txt
@@ -63,6 +63,7 @@
   index/MemIndex.cpp
   index/Merge.cpp
   index/Serialization.cpp
+  index/Symbol.cpp
   index/SymbolCollector.cpp
   index/SymbolID.cpp
   index/SymbolLocation.cpp
Index: clangd/IncludeFixer.h
===
--- clangd/IncludeFixer.h
+++ clangd/IncludeFixer.h
@@ -12,6 +12,7 @@
 #include "Diagnostics.h"
 #include "Headers.h"
 #include "index/Index.h"
+#include "index/Symbol.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/SourceLocation.h"
Index: clangd/index/Index.cpp
===
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -16,67 +16,6 @@
 namespace clang {
 namespace clangd {
 
-llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, Symbol::SymbolFlag F) {
-  if (F == Symbol::None)
-return OS << "None";
-  std::string S;
-  if (F & Symbol::Deprecated)
-S += "deprecated|";
-  if (F & Symbol::IndexedForCodeCompletion)
-S += "completion|";
-  return OS << llvm::StringRef(S).rtrim('|');
-}
-
-llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Symbol &S) {
-  return OS << S.Scope << S.Name;
-}
-
-float quality(const Symbol &S) {
-  // This avoids a sharp gradient for tail symbols, and also neatly avoids the
-  // question of whether 0 references means a bad symbol or missing data.
-  if (S.References < 3)
-return 1;
-  return std::log(S.References);
-}
-
-SymbolSlab::const_iterator SymbolSlab::find(const SymbolID &ID) const {
-  auto It = std::lower_bound(
-  Symbols.begin(), Symbols.end(), ID,
-  [](const Symbol &S, const SymbolID &I) { return S.ID < I; });
-  if (It != Symbols.end() && It->ID == ID)
-return It;
-  return Symbols.end();
-}
-
-// Copy the underlying data of the symbol into the owned arena.
-static void own(Symbol &S, llvm::UniqueStringSaver &Strings) {
-  visitStrings(S, [&](llvm::StringRef &V) { V = Strings.save(V); });
-}
-
-void SymbolSlab::Builder::insert(const Symbol &S) {
-  auto R = SymbolIndex.try_emplace(S.ID, Symbols.size());
-  if (R.second) {
-Symbols.push_back(S);
-own(Symbols.back(), UniqueStrings);
-  } else {
-auto &Copy = Symbols[R.first->second] = S;
-own(Copy, UniqueStrings);
-  }
-}
-
-SymbolSlab SymbolSlab::Builder::build() && {
-  Symbols = {Symbols.begin(), Symbols.end()}; // Force shrink-to-fit.
-  // Sort symbols so the slab can binary search over them.
-  llvm::sort(Symbols,
- [](const Symbol &L, const Symbol &R) { return L.ID < R.ID; });
-  // We may have unused strings from overwritten symbols. Build a new arena.
-  llvm::BumpPtrAllocator NewArena;
-  llvm::UniqueStringSaver Strings(NewArena);
-  for (auto &S : Symbols)
-own(S, Strings);
-  return SymbolSlab(std::move(NewArena), std::move(Symbols));
-}
-
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, RefKind K) {
   if (K == RefKind::Unknown)
 return OS << "Unknown";
Index: clangd/index/Symbol.cpp
===
--- clangd/index/Symbol.cpp
+++ clangd/index/Symbol.cpp
@@ -0,0 +1,76 @@
+//===--- Symbol.cpp --*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Symbol.h"
+
+namespace clang {
+namespace clangd {
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, Symbol::SymbolFlag F) {
+  if (F == Symbol::None)
+return OS << "None";
+  std::string S;
+  if (F & Symbol::Deprecated)
+S += "deprecated|";
+  if (F & Symbol::IndexedForCodeCompletion)
+S += "completion|";
+  return OS << llvm::StringRef(S).rtrim('|');
+}
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Symbol &S) {
+  return OS << S.Scope << S.Name;
+}
+
+float quality(const Symbol &S) {
+  // Thi

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 188716.
JonasToth marked 3 inline comments as done.
JonasToth added a comment.

- address review comments
- rebase to master


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D54395

Files:
  clang-tidy/performance/ForRangeCopyCheck.cpp
  clang-tidy/performance/UnnecessaryCopyInitialization.cpp
  clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tidy/utils/FixItHintUtils.cpp
  clang-tidy/utils/FixItHintUtils.h
  clang-tidy/utils/LexerUtils.cpp
  clang-tidy/utils/LexerUtils.h
  unittests/clang-tidy/AddConstTest.cpp
  unittests/clang-tidy/CMakeLists.txt

Index: unittests/clang-tidy/CMakeLists.txt
===
--- unittests/clang-tidy/CMakeLists.txt
+++ unittests/clang-tidy/CMakeLists.txt
@@ -7,6 +7,7 @@
 include_directories(${CLANG_LINT_SOURCE_DIR})
 
 add_extra_unittest(ClangTidyTests
+  AddConstTest.cpp
   ClangTidyDiagnosticConsumerTest.cpp
   ClangTidyOptionsTest.cpp
   IncludeInserterTest.cpp
Index: unittests/clang-tidy/AddConstTest.cpp
===
--- /dev/null
+++ unittests/clang-tidy/AddConstTest.cpp
@@ -0,0 +1,905 @@
+#include "../clang-tidy/utils/FixItHintUtils.h"
+#include "ClangTidyTest.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace tidy {
+
+namespace {
+using namespace clang::ast_matchers;
+using namespace utils::fixit;
+
+template 
+class ConstTransform : public ClangTidyCheck {
+public:
+  ConstTransform(StringRef CheckName, ClangTidyContext *Context)
+  : ClangTidyCheck(CheckName, Context) {}
+
+  void registerMatchers(MatchFinder *Finder) override {
+Finder->addMatcher(varDecl(hasName("target")).bind("var"), this);
+  }
+
+  void check(const MatchFinder::MatchResult &Result) override {
+const auto *D = Result.Nodes.getNodeAs("var");
+using utils::fixit::addQualifierToVarDecl;
+Optional Fix = addQualifierToVarDecl(*D, DeclSpec::TQ::TQ_const,
+CT, CP, Result.Context);
+auto Diag = diag(D->getBeginLoc(), "doing const transformation");
+if (Fix)
+  Diag << *Fix;
+  }
+};
+} // namespace
+
+namespace test {
+using PointeeLTransform =
+ConstTransform;
+using PointeeRTransform =
+ConstTransform;
+
+using ValueLTransform =
+ConstTransform;
+using ValueRTransform =
+ConstTransform;
+
+// 
+// Test Value-like types. Everything with indirection is done later.
+// 
+
+TEST(Values, Builtin) {
+  StringRef Snippet = "int target = 0;";
+
+  EXPECT_EQ("const int target = 0;", runCheckOnCode(Snippet));
+  EXPECT_EQ("const int target = 0;",
+runCheckOnCode(Snippet));
+
+  EXPECT_EQ("int const target = 0;", runCheckOnCode(Snippet));
+  EXPECT_EQ("int const target = 0;",
+runCheckOnCode(Snippet));
+}
+TEST(Values, TypedefBuiltin) {
+  StringRef T = "typedef int MyInt;";
+  StringRef S = "MyInt target = 0;";
+  auto Cat = [&T](StringRef S) { return (T + S).str(); };
+
+  EXPECT_EQ(Cat("const MyInt target = 0;"),
+runCheckOnCode(Cat(S)));
+  EXPECT_EQ(Cat("const MyInt target = 0;"),
+runCheckOnCode(Cat(S)));
+
+  EXPECT_EQ(Cat("MyInt const target = 0;"),
+runCheckOnCode(Cat(S)));
+  EXPECT_EQ(Cat("MyInt const target = 0;"),
+runCheckOnCode(Cat(S)));
+}
+TEST(Values, TypedefBuiltinPointer) {
+  StringRef T = "typedef int* MyInt;";
+  StringRef S = "MyInt target = nullptr;";
+  auto Cat = [&T](StringRef S) { return (T + S).str(); };
+
+  EXPECT_EQ(Cat("const MyInt target = nullptr;"),
+runCheckOnCode(Cat(S)));
+  EXPECT_EQ(Cat("const MyInt target = nullptr;"),
+runCheckOnCode(Cat(S)));
+
+  EXPECT_EQ(Cat("MyInt const target = nullptr;"),
+runCheckOnCode(Cat(S)));
+  EXPECT_EQ(Cat("MyInt const target = nullptr;"),
+runCheckOnCode(Cat(S)));
+}
+TEST(Values, AutoValue) {
+  StringRef T = "int f() { return 42; }\n";
+  StringRef S = "auto target = f();";
+  auto Cat = [&T](StringRef S) { return (T + S).str(); };
+
+  EXPECT_EQ(Cat("const auto target = f();"),
+runCheckOnCode(Cat(S)));
+  EXPECT_EQ(Cat("const auto target = f();"),
+runCheckOnCode(Cat(S)));
+
+  EXPECT_EQ(Cat("auto const target = f();"),
+runCheckOnCode(Cat(S)));
+  EXPECT_EQ(Cat("auto const target = f();"),
+runCheckOnCode(Cat(S)));
+}
+TEST(Values, AutoPointer) {
+  StringRef T = "int* f() { return nullptr; }\n";
+  StringRef S = "auto target = f();";
+  auto Cat = [&T](StringRef S) { return (T + S).str(); };
+
+  EXPECT_EQ(Cat("const auto target = f();"),
+

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

[Jonas Toth via Phabricator via cfe-commits]

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



Comment at: unittests/clang-tidy/AddConstTest.cpp:15
+
+template 

alexfh wrote:
> What's the point of default values of template arguments here?
Wups, relict of older version.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D54395



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

[PATCH] D58778: Moved Ref into its own header and implementation file

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr created this revision.
gribozavr added a reviewer: ioeric.
Herald added subscribers: cfe-commits, kadircet, arphaman, mgrang, jkorous, 
mgorny.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58778

Files:
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Quality.cpp
  clang-tools-extra/clangd/index/Index.cpp
  clang-tools-extra/clangd/index/Index.h
  clang-tools-extra/clangd/index/Ref.cpp
  clang-tools-extra/clangd/index/Ref.h
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/indexer/IndexerMain.cpp

Index: clang-tools-extra/clangd/indexer/IndexerMain.cpp
===
--- clang-tools-extra/clangd/indexer/IndexerMain.cpp
+++ clang-tools-extra/clangd/indexer/IndexerMain.cpp
@@ -10,10 +10,10 @@
 //
 //===--===//
 
-#include "index/Index.h"
 #include "index/IndexAction.h"
 #include "index/Merge.h"
 #include "index/Serialization.h"
+#include "index/Symbol.h"
 #include "index/SymbolCollector.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Execution.h"
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -11,6 +11,7 @@
 #include "CanonicalIncludes.h"
 #include "CodeComplete.h"
 #include "CodeCompletionStrings.h"
+#include "ExpectedTypes.h"
 #include "Logger.h"
 #include "SourceCode.h"
 #include "SymbolLocation.h"
Index: clang-tools-extra/clangd/index/Ref.h
===
--- /dev/null
+++ clang-tools-extra/clangd/index/Ref.h
@@ -0,0 +1,119 @@
+//===--- Ref.h ---*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_REF_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_REF_H
+
+#include "SymbolID.h"
+#include "SymbolLocation.h"
+#include "clang/Index/IndexSymbol.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/Support/StringSaver.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+
+namespace clang {
+namespace clangd {
+
+/// Describes the kind of a cross-reference.
+///
+/// This is a bitfield which can be combined from different kinds.
+enum class RefKind : uint8_t {
+  Unknown = 0,
+  Declaration = static_cast(index::SymbolRole::Declaration),
+  Definition = static_cast(index::SymbolRole::Definition),
+  Reference = static_cast(index::SymbolRole::Reference),
+  All = Declaration | Definition | Reference,
+};
+
+inline RefKind operator|(RefKind L, RefKind R) {
+  return static_cast(static_cast(L) |
+  static_cast(R));
+}
+inline RefKind &operator|=(RefKind &L, RefKind R) { return L = L | R; }
+inline RefKind operator&(RefKind A, RefKind B) {
+  return static_cast(static_cast(A) &
+  static_cast(B));
+}
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, RefKind);
+
+/// Represents a symbol occurrence in the source file.
+/// Despite the name, it could be a declaration/definition/reference.
+///
+/// WARNING: Location does not own the underlying data - Copies are shallow.
+struct Ref {
+  /// The source location where the symbol is named.
+  SymbolLocation Location;
+  RefKind Kind = RefKind::Unknown;
+};
+
+inline bool operator<(const Ref &L, const Ref &R) {
+  return std::tie(L.Location, L.Kind) < std::tie(R.Location, R.Kind);
+}
+inline bool operator==(const Ref &L, const Ref &R) {
+  return std::tie(L.Location, L.Kind) == std::tie(R.Location, R.Kind);
+}
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Ref &);
+
+/// An efficient structure of storing large set of symbol references in memory.
+/// Filenames are deduplicated.
+class RefSlab {
+public:
+  using value_type = std::pair>;
+  using const_iterator = std::vector::const_iterator;
+  using iterator = const_iterator;
+
+  RefSlab() = default;
+  RefSlab(RefSlab &&Slab) = default;
+  RefSlab &operator=(RefSlab &&RHS) = default;
+
+  const_iterator begin() const { return Refs.begin(); }
+  const_iterator end() const { return Refs.end(); }
+  /// Gets the number of symbols.
+  size_t size() const { return Refs.size(); }
+  size_t numRefs() const { return NumRefs; }
+  bool empty() const { return Refs.empty(); }
+
+  size_t bytes() const {
+return sizeof(*this) + Arena.getTotalMemory() +
+   sizeof(value_type) * Refs.si

[PATCH] D58609: [clang-tidy] bugprone-string-integer-assignment: Reduce false positives.

[Jonas Toth via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58609



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

[PATCH] D58743: Handle built-in when importing SourceLocation and FileID

[Gabor Marton via Phabricator via cfe-commits]

martong accepted this revision.
martong added a reviewer: balazske.
martong added a subscriber: balazske.
martong added a comment.

Shafik, this looks good to me, once teemperor's comments are addressed.
Note, I added @balazske as a reviewer, he recently worked with importing the 
FileIDs, he may have some comments.


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

https://reviews.llvm.org/D58743



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

[PATCH] D58778: Moved Ref into its own header and implementation file

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clang-tools-extra/clangd/index/Ref.cpp:1
+#include "Ref.h"
+

License?



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:14
 #include "CodeCompletionStrings.h"
+#include "ExpectedTypes.h"
 #include "Logger.h"

Is this change relevant in this patch?



Comment at: clang-tools-extra/clangd/indexer/IndexerMain.cpp:16
 #include "index/Serialization.h"
+#include "index/Symbol.h"
 #include "index/SymbolCollector.h"

Maybe also include Ref.h?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58778



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

[PATCH] D58764: [clang-tidy] ignore predefined expressions in cppcoreguidelines-pro-bounds-array-to-pointer-decay check

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

You can abondon this. A short justification (with reference to the other 
revision) on the bug report would be great!


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58764



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

[clang-tools-extra] r355089 - [clang-tidy] bugprone-string-integer-assignment: Reduce false positives.

[Clement Courbet via cfe-commits]

Author: courbet
Date: Thu Feb 28 05:39:01 2019
New Revision: 355089

URL: http://llvm.org/viewvc/llvm-project?rev=355089&view=rev
Log:
[clang-tidy] bugprone-string-integer-assignment: Reduce false positives.

Summary: Detect a few expressions as likely character expressions, see PR27723.

Reviewers: xazax.hun, alexfh

Subscribers: rnkovacs, jdoerfert, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp

clang-tools-extra/trunk/test/clang-tidy/bugprone-string-integer-assignment.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp?rev=355089&r1=355088&r2=355089&view=diff
==
--- 
clang-tools-extra/trunk/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp 
(original)
+++ 
clang-tools-extra/trunk/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp 
Thu Feb 28 05:39:01 2019
@@ -45,11 +45,40 @@ void StringIntegerAssignmentCheck::regis
   this);
 }
 
+static bool isLikelyCharExpression(const Expr *Argument,
+   const ASTContext &Ctx) {
+  const auto *BinOp = dyn_cast(Argument);
+  if (!BinOp)
+return false;
+  const auto *LHS = BinOp->getLHS()->IgnoreParenImpCasts();
+  const auto *RHS = BinOp->getRHS()->IgnoreParenImpCasts();
+  //  & , mask is a compile time constant.
+  Expr::EvalResult RHSVal;
+  if (BinOp->getOpcode() == BO_And &&
+  (RHS->EvaluateAsInt(RHSVal, Ctx, Expr::SE_AllowSideEffects) ||
+   LHS->EvaluateAsInt(RHSVal, Ctx, Expr::SE_AllowSideEffects)))
+return true;
+  //  + ( % ), where  is a char literal.
+  const auto IsCharPlusModExpr = [](const Expr *L, const Expr *R) {
+const auto *ROp = dyn_cast(R);
+return ROp && ROp->getOpcode() == BO_Rem && isa(L);
+  };
+  if (BinOp->getOpcode() == BO_Add) {
+if (IsCharPlusModExpr(LHS, RHS) || IsCharPlusModExpr(RHS, LHS))
+  return true;
+  }
+  return false;
+}
+
 void StringIntegerAssignmentCheck::check(
 const MatchFinder::MatchResult &Result) {
   const auto *Argument = Result.Nodes.getNodeAs("expr");
   SourceLocation Loc = Argument->getBeginLoc();
 
+  // Try to detect a few common expressions to reduce false positives.
+  if (isLikelyCharExpression(Argument, *Result.Context))
+return;
+
   auto Diag =
   diag(Loc, "an integer is interpreted as a character code when assigning "
 "it to a string; if this is intended, cast the integer to the "

Modified: 
clang-tools-extra/trunk/test/clang-tidy/bugprone-string-integer-assignment.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/bugprone-string-integer-assignment.cpp?rev=355089&r1=355088&r2=355089&view=diff
==
--- 
clang-tools-extra/trunk/test/clang-tidy/bugprone-string-integer-assignment.cpp 
(original)
+++ 
clang-tools-extra/trunk/test/clang-tidy/bugprone-string-integer-assignment.cpp 
Thu Feb 28 05:39:01 2019
@@ -59,4 +59,11 @@ int main() {
   s += toupper(x);
   s += tolower(x);
   s += std::tolower(x);
+
+  // Likely character expressions.
+  s += x & 0xff;
+  s += 0xff & x;
+
+  s += 'a' + (x % 26);
+  s += (x % 10) + 'b';
 }


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

[PATCH] D58778: Moved Ref into its own header and implementation file

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr updated this revision to Diff 188723.
gribozavr marked an inline comment as done.
gribozavr added a comment.

Added a license header.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58778

Files:
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Quality.cpp
  clang-tools-extra/clangd/index/Index.cpp
  clang-tools-extra/clangd/index/Index.h
  clang-tools-extra/clangd/index/Ref.cpp
  clang-tools-extra/clangd/index/Ref.h
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/indexer/IndexerMain.cpp

Index: clang-tools-extra/clangd/indexer/IndexerMain.cpp
===
--- clang-tools-extra/clangd/indexer/IndexerMain.cpp
+++ clang-tools-extra/clangd/indexer/IndexerMain.cpp
@@ -10,10 +10,10 @@
 //
 //===--===//
 
-#include "index/Index.h"
 #include "index/IndexAction.h"
 #include "index/Merge.h"
 #include "index/Serialization.h"
+#include "index/Symbol.h"
 #include "index/SymbolCollector.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Execution.h"
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -11,6 +11,7 @@
 #include "CanonicalIncludes.h"
 #include "CodeComplete.h"
 #include "CodeCompletionStrings.h"
+#include "ExpectedTypes.h"
 #include "Logger.h"
 #include "SourceCode.h"
 #include "SymbolLocation.h"
Index: clang-tools-extra/clangd/index/Ref.h
===
--- /dev/null
+++ clang-tools-extra/clangd/index/Ref.h
@@ -0,0 +1,119 @@
+//===--- Ref.h ---*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_REF_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_REF_H
+
+#include "SymbolID.h"
+#include "SymbolLocation.h"
+#include "clang/Index/IndexSymbol.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/Support/StringSaver.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+
+namespace clang {
+namespace clangd {
+
+/// Describes the kind of a cross-reference.
+///
+/// This is a bitfield which can be combined from different kinds.
+enum class RefKind : uint8_t {
+  Unknown = 0,
+  Declaration = static_cast(index::SymbolRole::Declaration),
+  Definition = static_cast(index::SymbolRole::Definition),
+  Reference = static_cast(index::SymbolRole::Reference),
+  All = Declaration | Definition | Reference,
+};
+
+inline RefKind operator|(RefKind L, RefKind R) {
+  return static_cast(static_cast(L) |
+  static_cast(R));
+}
+inline RefKind &operator|=(RefKind &L, RefKind R) { return L = L | R; }
+inline RefKind operator&(RefKind A, RefKind B) {
+  return static_cast(static_cast(A) &
+  static_cast(B));
+}
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, RefKind);
+
+/// Represents a symbol occurrence in the source file.
+/// Despite the name, it could be a declaration/definition/reference.
+///
+/// WARNING: Location does not own the underlying data - Copies are shallow.
+struct Ref {
+  /// The source location where the symbol is named.
+  SymbolLocation Location;
+  RefKind Kind = RefKind::Unknown;
+};
+
+inline bool operator<(const Ref &L, const Ref &R) {
+  return std::tie(L.Location, L.Kind) < std::tie(R.Location, R.Kind);
+}
+inline bool operator==(const Ref &L, const Ref &R) {
+  return std::tie(L.Location, L.Kind) == std::tie(R.Location, R.Kind);
+}
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Ref &);
+
+/// An efficient structure of storing large set of symbol references in memory.
+/// Filenames are deduplicated.
+class RefSlab {
+public:
+  using value_type = std::pair>;
+  using const_iterator = std::vector::const_iterator;
+  using iterator = const_iterator;
+
+  RefSlab() = default;
+  RefSlab(RefSlab &&Slab) = default;
+  RefSlab &operator=(RefSlab &&RHS) = default;
+
+  const_iterator begin() const { return Refs.begin(); }
+  const_iterator end() const { return Refs.end(); }
+  /// Gets the number of symbols.
+  size_t size() const { return Refs.size(); }
+  size_t numRefs() const { return NumRefs; }
+  bool empty() const { return Refs.empty(); }
+
+  size_t bytes() const {
+return sizeof(*this) + Arena.getTotalMemory() +
+   si

[PATCH] D58609: [clang-tidy] bugprone-string-integer-assignment: Reduce false positives.

[Clement Courbet via Phabricator via cfe-commits]

courbet added a comment.

Thanks.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58609



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

[PATCH] D58609: [clang-tidy] bugprone-string-integer-assignment: Reduce false positives.

[Clement Courbet via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE355089: [clang-tidy] bugprone-string-integer-assignment: 
Reduce false positives. (authored by courbet, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58609?vs=188690&id=188722#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58609

Files:
  clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
  test/clang-tidy/bugprone-string-integer-assignment.cpp


Index: clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
===
--- clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
+++ clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
@@ -45,11 +45,40 @@
   this);
 }
 
+static bool isLikelyCharExpression(const Expr *Argument,
+   const ASTContext &Ctx) {
+  const auto *BinOp = dyn_cast(Argument);
+  if (!BinOp)
+return false;
+  const auto *LHS = BinOp->getLHS()->IgnoreParenImpCasts();
+  const auto *RHS = BinOp->getRHS()->IgnoreParenImpCasts();
+  //  & , mask is a compile time constant.
+  Expr::EvalResult RHSVal;
+  if (BinOp->getOpcode() == BO_And &&
+  (RHS->EvaluateAsInt(RHSVal, Ctx, Expr::SE_AllowSideEffects) ||
+   LHS->EvaluateAsInt(RHSVal, Ctx, Expr::SE_AllowSideEffects)))
+return true;
+  //  + ( % ), where  is a char literal.
+  const auto IsCharPlusModExpr = [](const Expr *L, const Expr *R) {
+const auto *ROp = dyn_cast(R);
+return ROp && ROp->getOpcode() == BO_Rem && isa(L);
+  };
+  if (BinOp->getOpcode() == BO_Add) {
+if (IsCharPlusModExpr(LHS, RHS) || IsCharPlusModExpr(RHS, LHS))
+  return true;
+  }
+  return false;
+}
+
 void StringIntegerAssignmentCheck::check(
 const MatchFinder::MatchResult &Result) {
   const auto *Argument = Result.Nodes.getNodeAs("expr");
   SourceLocation Loc = Argument->getBeginLoc();
 
+  // Try to detect a few common expressions to reduce false positives.
+  if (isLikelyCharExpression(Argument, *Result.Context))
+return;
+
   auto Diag =
   diag(Loc, "an integer is interpreted as a character code when assigning "
 "it to a string; if this is intended, cast the integer to the "
Index: test/clang-tidy/bugprone-string-integer-assignment.cpp
===
--- test/clang-tidy/bugprone-string-integer-assignment.cpp
+++ test/clang-tidy/bugprone-string-integer-assignment.cpp
@@ -59,4 +59,11 @@
   s += toupper(x);
   s += tolower(x);
   s += std::tolower(x);
+
+  // Likely character expressions.
+  s += x & 0xff;
+  s += 0xff & x;
+
+  s += 'a' + (x % 26);
+  s += (x % 10) + 'b';
 }


Index: clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
===
--- clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
+++ clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
@@ -45,11 +45,40 @@
   this);
 }
 
+static bool isLikelyCharExpression(const Expr *Argument,
+   const ASTContext &Ctx) {
+  const auto *BinOp = dyn_cast(Argument);
+  if (!BinOp)
+return false;
+  const auto *LHS = BinOp->getLHS()->IgnoreParenImpCasts();
+  const auto *RHS = BinOp->getRHS()->IgnoreParenImpCasts();
+  //  & , mask is a compile time constant.
+  Expr::EvalResult RHSVal;
+  if (BinOp->getOpcode() == BO_And &&
+  (RHS->EvaluateAsInt(RHSVal, Ctx, Expr::SE_AllowSideEffects) ||
+   LHS->EvaluateAsInt(RHSVal, Ctx, Expr::SE_AllowSideEffects)))
+return true;
+  //  + ( % ), where  is a char literal.
+  const auto IsCharPlusModExpr = [](const Expr *L, const Expr *R) {
+const auto *ROp = dyn_cast(R);
+return ROp && ROp->getOpcode() == BO_Rem && isa(L);
+  };
+  if (BinOp->getOpcode() == BO_Add) {
+if (IsCharPlusModExpr(LHS, RHS) || IsCharPlusModExpr(RHS, LHS))
+  return true;
+  }
+  return false;
+}
+
 void StringIntegerAssignmentCheck::check(
 const MatchFinder::MatchResult &Result) {
   const auto *Argument = Result.Nodes.getNodeAs("expr");
   SourceLocation Loc = Argument->getBeginLoc();
 
+  // Try to detect a few common expressions to reduce false positives.
+  if (isLikelyCharExpression(Argument, *Result.Context))
+return;
+
   auto Diag =
   diag(Loc, "an integer is interpreted as a character code when assigning "
 "it to a string; if this is intended, cast the integer to the "
Index: test/clang-tidy/bugprone-string-integer-assignment.cpp
===
--- test/clang-tidy/bugprone-string-integer-assignment.cpp
+++ test/clang-tidy/bugprone-string-integer-assignment.cpp
@@ -59,4 +59,11 @@
   s += toupper(x);
   s += tolower(x);
   s += std::tolower(x);
+
+  // Likely character expressions.
+  s += x & 0xff;
+  s += 0xff & x;
+
+  s += 'a' + (x % 26);
+  s += (x % 10) + 'b';
 }
___

[PATCH] D58778: Moved Ref into its own header and implementation file

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr updated this revision to Diff 188724.
gribozavr marked 2 inline comments as done.
gribozavr added a comment.

Added a missing include.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58778

Files:
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Quality.cpp
  clang-tools-extra/clangd/index/Index.cpp
  clang-tools-extra/clangd/index/Index.h
  clang-tools-extra/clangd/index/Ref.cpp
  clang-tools-extra/clangd/index/Ref.h
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/indexer/IndexerMain.cpp

Index: clang-tools-extra/clangd/indexer/IndexerMain.cpp
===
--- clang-tools-extra/clangd/indexer/IndexerMain.cpp
+++ clang-tools-extra/clangd/indexer/IndexerMain.cpp
@@ -10,10 +10,11 @@
 //
 //===--===//
 
-#include "index/Index.h"
 #include "index/IndexAction.h"
 #include "index/Merge.h"
+#include "index/Ref.h"
 #include "index/Serialization.h"
+#include "index/Symbol.h"
 #include "index/SymbolCollector.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Execution.h"
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -11,6 +11,7 @@
 #include "CanonicalIncludes.h"
 #include "CodeComplete.h"
 #include "CodeCompletionStrings.h"
+#include "ExpectedTypes.h"
 #include "Logger.h"
 #include "SourceCode.h"
 #include "SymbolLocation.h"
Index: clang-tools-extra/clangd/index/Ref.h
===
--- /dev/null
+++ clang-tools-extra/clangd/index/Ref.h
@@ -0,0 +1,119 @@
+//===--- Ref.h ---*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_REF_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_REF_H
+
+#include "SymbolID.h"
+#include "SymbolLocation.h"
+#include "clang/Index/IndexSymbol.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/Support/StringSaver.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+
+namespace clang {
+namespace clangd {
+
+/// Describes the kind of a cross-reference.
+///
+/// This is a bitfield which can be combined from different kinds.
+enum class RefKind : uint8_t {
+  Unknown = 0,
+  Declaration = static_cast(index::SymbolRole::Declaration),
+  Definition = static_cast(index::SymbolRole::Definition),
+  Reference = static_cast(index::SymbolRole::Reference),
+  All = Declaration | Definition | Reference,
+};
+
+inline RefKind operator|(RefKind L, RefKind R) {
+  return static_cast(static_cast(L) |
+  static_cast(R));
+}
+inline RefKind &operator|=(RefKind &L, RefKind R) { return L = L | R; }
+inline RefKind operator&(RefKind A, RefKind B) {
+  return static_cast(static_cast(A) &
+  static_cast(B));
+}
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, RefKind);
+
+/// Represents a symbol occurrence in the source file.
+/// Despite the name, it could be a declaration/definition/reference.
+///
+/// WARNING: Location does not own the underlying data - Copies are shallow.
+struct Ref {
+  /// The source location where the symbol is named.
+  SymbolLocation Location;
+  RefKind Kind = RefKind::Unknown;
+};
+
+inline bool operator<(const Ref &L, const Ref &R) {
+  return std::tie(L.Location, L.Kind) < std::tie(R.Location, R.Kind);
+}
+inline bool operator==(const Ref &L, const Ref &R) {
+  return std::tie(L.Location, L.Kind) == std::tie(R.Location, R.Kind);
+}
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Ref &);
+
+/// An efficient structure of storing large set of symbol references in memory.
+/// Filenames are deduplicated.
+class RefSlab {
+public:
+  using value_type = std::pair>;
+  using const_iterator = std::vector::const_iterator;
+  using iterator = const_iterator;
+
+  RefSlab() = default;
+  RefSlab(RefSlab &&Slab) = default;
+  RefSlab &operator=(RefSlab &&RHS) = default;
+
+  const_iterator begin() const { return Refs.begin(); }
+  const_iterator end() const { return Refs.end(); }
+  /// Gets the number of symbols.
+  size_t size() const { return Refs.size(); }
+  size_t numRefs() const { return NumRefs; }
+  bool empty() const { return Refs.empty(); }
+
+  size_t bytes() const {
+return sizeof(*this) + Arena.getTotal

[PATCH] D58778: Moved Ref into its own header and implementation file

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/index/Ref.cpp:1
+#include "Ref.h"
+

ioeric wrote:
> License?
Added.



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:14
 #include "CodeCompletionStrings.h"
+#include "ExpectedTypes.h"
 #include "Logger.h"

ioeric wrote:
> Is this change relevant in this patch?
Yes, once I replaced inclusion of Index.h with a more specific file somewhere 
else, SymbolCollector.cpp stopped building because it relied on transitive 
inclusion of ExpectedTypes.h.



Comment at: clang-tools-extra/clangd/indexer/IndexerMain.cpp:16
 #include "index/Serialization.h"
+#include "index/Symbol.h"
 #include "index/SymbolCollector.h"

ioeric wrote:
> Maybe also include Ref.h?
Added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58778



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

[clang-tools-extra] r355090 - Moved Ref into its own header and implementation file

[Dmitri Gribenko via cfe-commits]

Author: gribozavr
Date: Thu Feb 28 05:49:25 2019
New Revision: 355090

URL: http://llvm.org/viewvc/llvm-project?rev=355090&view=rev
Log:
Moved Ref into its own header and implementation file

Reviewers: ioeric

Subscribers: mgorny, jkorous, mgrang, arphaman, kadircet, cfe-commits

Tags: #clang

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

Added:
clang-tools-extra/trunk/clangd/index/Ref.cpp
clang-tools-extra/trunk/clangd/index/Ref.h
Modified:
clang-tools-extra/trunk/clangd/AST.h
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/clangd/IncludeFixer.cpp
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/Quality.cpp
clang-tools-extra/trunk/clangd/index/Index.cpp
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp

Modified: clang-tools-extra/trunk/clangd/AST.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/AST.h?rev=355090&r1=355089&r2=355090&view=diff
==
--- clang-tools-extra/trunk/clangd/AST.h (original)
+++ clang-tools-extra/trunk/clangd/AST.h Thu Feb 28 05:49:25 2019
@@ -13,9 +13,10 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H_
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H_
 
-#include "index/Index.h"
+#include "index/SymbolID.h"
 #include "clang/AST/Decl.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Lex/MacroInfo.h"
 
 namespace clang {
 class SourceManager;

Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=355090&r1=355089&r2=355090&view=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Thu Feb 28 05:49:25 2019
@@ -62,6 +62,7 @@ add_clang_library(clangDaemon
   index/IndexAction.cpp
   index/MemIndex.cpp
   index/Merge.cpp
+  index/Ref.cpp
   index/Serialization.cpp
   index/Symbol.cpp
   index/SymbolCollector.cpp

Modified: clang-tools-extra/trunk/clangd/IncludeFixer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/IncludeFixer.cpp?rev=355090&r1=355089&r2=355090&view=diff
==
--- clang-tools-extra/trunk/clangd/IncludeFixer.cpp (original)
+++ clang-tools-extra/trunk/clangd/IncludeFixer.cpp Thu Feb 28 05:49:25 2019
@@ -24,6 +24,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TokenKinds.h"
+#include "clang/Lex/Lexer.h"
 #include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Scope.h"

Modified: clang-tools-extra/trunk/clangd/Protocol.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.cpp?rev=355090&r1=355089&r2=355090&view=diff
==
--- clang-tools-extra/trunk/clangd/Protocol.cpp (original)
+++ clang-tools-extra/trunk/clangd/Protocol.cpp Thu Feb 28 05:49:25 2019
@@ -13,7 +13,6 @@
 #include "Protocol.h"
 #include "Logger.h"
 #include "URI.h"
-#include "index/Index.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/SmallString.h"

Modified: clang-tools-extra/trunk/clangd/Quality.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Quality.cpp?rev=355090&r1=355089&r2=355090&view=diff
==
--- clang-tools-extra/trunk/clangd/Quality.cpp (original)
+++ clang-tools-extra/trunk/clangd/Quality.cpp Thu Feb 28 05:49:25 2019
@@ -5,11 +5,12 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 
//===--===//
+
 #include "Quality.h"
 #include "AST.h"
 #include "FileDistance.h"
 #include "URI.h"
-#include "index/Index.h"
+#include "index/Symbol.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"

Modified: clang-tools-extra/trunk/clangd/index/Index.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.cpp?rev=355090&r1=355089&r2=355090&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Index.cpp Thu Feb 28 05:49:25 2019
@@ -12,57 +12,11 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 
 namespace clang {
 namespace clangd {
 
-llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, RefKind K) {
-  if (K == RefKind::Unknown)
-return OS << "Unknown";
-  static const std:

[PATCH] D58781: Added missing license headers

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr created this revision.
gribozavr added a reviewer: ioeric.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58781

Files:
  clang-tools-extra/clangd/ExpectedTypes.cpp
  clang-tools-extra/clangd/index/IndexAction.cpp
  clang-tools-extra/clangd/xpc/framework/ClangdXPC.cpp
  clang-tools-extra/clangd/xpc/test-client/ClangdXPCTestClient.cpp


Index: clang-tools-extra/clangd/xpc/test-client/ClangdXPCTestClient.cpp
===
--- clang-tools-extra/clangd/xpc/test-client/ClangdXPCTestClient.cpp
+++ clang-tools-extra/clangd/xpc/test-client/ClangdXPCTestClient.cpp
@@ -1,3 +1,11 @@
+//===--- ClangXPCTestClient.cpp --*- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
 #include "xpc/Conversion.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/SmallString.h"
Index: clang-tools-extra/clangd/xpc/framework/ClangdXPC.cpp
===
--- clang-tools-extra/clangd/xpc/framework/ClangdXPC.cpp
+++ clang-tools-extra/clangd/xpc/framework/ClangdXPC.cpp
@@ -1,3 +1,10 @@
+//===--- ClangXPC.cpp *- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
 
 /// Returns the bundle identifier of the Clangd XPC service.
 extern "C" const char *clangd_xpc_get_bundle_identifier() {
Index: clang-tools-extra/clangd/index/IndexAction.cpp
===
--- clang-tools-extra/clangd/index/IndexAction.cpp
+++ clang-tools-extra/clangd/index/IndexAction.cpp
@@ -1,5 +1,12 @@
-#include "IndexAction.h"
+//===--- IndexAction.cpp -*- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
 
+#include "IndexAction.h"
 #include "index/SymbolOrigin.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Index/IndexDataConsumer.h"
Index: clang-tools-extra/clangd/ExpectedTypes.cpp
===
--- clang-tools-extra/clangd/ExpectedTypes.cpp
+++ clang-tools-extra/clangd/ExpectedTypes.cpp
@@ -1,3 +1,11 @@
+//===--- ExpectedTypes.cpp ---*- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
 #include "ExpectedTypes.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Type.h"


Index: clang-tools-extra/clangd/xpc/test-client/ClangdXPCTestClient.cpp
===
--- clang-tools-extra/clangd/xpc/test-client/ClangdXPCTestClient.cpp
+++ clang-tools-extra/clangd/xpc/test-client/ClangdXPCTestClient.cpp
@@ -1,3 +1,11 @@
+//===--- ClangXPCTestClient.cpp --*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
 #include "xpc/Conversion.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/SmallString.h"
Index: clang-tools-extra/clangd/xpc/framework/ClangdXPC.cpp
===
--- clang-tools-extra/clangd/xpc/framework/ClangdXPC.cpp
+++ clang-tools-extra/clangd/xpc/framework/ClangdXPC.cpp
@@ -1,3 +1,10 @@
+//===--- ClangXPC.cpp *- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
 
 /// Returns the bundle identifier of the Clangd XPC service.
 extern "C" const char *clangd_xpc_

[PATCH] D58778: Moved Ref into its own header and implementation file

[Dmitri Gribenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
gribozavr marked an inline comment as done.
Closed by commit rL355090: Moved Ref into its own header and implementation 
file (authored by gribozavr, committed by ).
Herald added subscribers: llvm-commits, ilya-biryukov.
Herald added a project: LLVM.

Changed prior to commit:
  https://reviews.llvm.org/D58778?vs=188724&id=188727#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58778

Files:
  clang-tools-extra/trunk/clangd/AST.h
  clang-tools-extra/trunk/clangd/CMakeLists.txt
  clang-tools-extra/trunk/clangd/IncludeFixer.cpp
  clang-tools-extra/trunk/clangd/Protocol.cpp
  clang-tools-extra/trunk/clangd/Quality.cpp
  clang-tools-extra/trunk/clangd/index/Index.cpp
  clang-tools-extra/trunk/clangd/index/Index.h
  clang-tools-extra/trunk/clangd/index/Ref.cpp
  clang-tools-extra/trunk/clangd/index/Ref.h
  clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
  clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp

Index: clang-tools-extra/trunk/clangd/CMakeLists.txt
===
--- clang-tools-extra/trunk/clangd/CMakeLists.txt
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt
@@ -62,6 +62,7 @@
   index/IndexAction.cpp
   index/MemIndex.cpp
   index/Merge.cpp
+  index/Ref.cpp
   index/Serialization.cpp
   index/Symbol.cpp
   index/SymbolCollector.cpp
Index: clang-tools-extra/trunk/clangd/index/Index.cpp
===
--- clang-tools-extra/trunk/clangd/index/Index.cpp
+++ clang-tools-extra/trunk/clangd/index/Index.cpp
@@ -12,57 +12,11 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 
 namespace clang {
 namespace clangd {
 
-llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, RefKind K) {
-  if (K == RefKind::Unknown)
-return OS << "Unknown";
-  static const std::vector Messages = {"Decl", "Def", "Ref"};
-  bool VisitedOnce = false;
-  for (unsigned I = 0; I < Messages.size(); ++I) {
-if (static_cast(K) & 1u << I) {
-  if (VisitedOnce)
-OS << ", ";
-  OS << Messages[I];
-  VisitedOnce = true;
-}
-  }
-  return OS;
-}
-
-llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Ref &R) {
-  return OS << R.Location << ":" << R.Kind;
-}
-
-void RefSlab::Builder::insert(const SymbolID &ID, const Ref &S) {
-  auto &M = Refs[ID];
-  M.push_back(S);
-  M.back().Location.FileURI =
-  UniqueStrings.save(M.back().Location.FileURI).data();
-}
-
-RefSlab RefSlab::Builder::build() && {
-  // We can reuse the arena, as it only has unique strings and we need them all.
-  // Reallocate refs on the arena to reduce waste and indirections when reading.
-  std::vector>> Result;
-  Result.reserve(Refs.size());
-  size_t NumRefs = 0;
-  for (auto &Sym : Refs) {
-auto &SymRefs = Sym.second;
-llvm::sort(SymRefs);
-// FIXME: do we really need to dedup?
-SymRefs.erase(std::unique(SymRefs.begin(), SymRefs.end()), SymRefs.end());
-
-NumRefs += SymRefs.size();
-auto *Array = Arena.Allocate(SymRefs.size());
-std::uninitialized_copy(SymRefs.begin(), SymRefs.end(), Array);
-Result.emplace_back(Sym.first, llvm::ArrayRef(Array, SymRefs.size()));
-  }
-  return RefSlab(std::move(Result), std::move(Arena), NumRefs);
-}
-
 void SwapIndex::reset(std::unique_ptr Index) {
   // Keep the old index alive, so we don't destroy it under lock (may be slow).
   std::shared_ptr Pin;
Index: clang-tools-extra/trunk/clangd/index/Ref.h
===
--- clang-tools-extra/trunk/clangd/index/Ref.h
+++ clang-tools-extra/trunk/clangd/index/Ref.h
@@ -0,0 +1,119 @@
+//===--- Ref.h ---*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_REF_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_REF_H
+
+#include "SymbolID.h"
+#include "SymbolLocation.h"
+#include "clang/Index/IndexSymbol.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/Support/StringSaver.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+
+namespace clang {
+namespace clangd {
+
+/// Describes the kind of a cross-reference.
+///
+/// This is a bitfield which can be combined from different kinds.
+enum class RefKind : uint8_t {
+  Unknown = 0,
+  Declaration = static_cast(index::SymbolRole::Declaration),
+  Definition = static_cast(index::SymbolRole::Definition),
+  Reference = static_cast(index::SymbolRole::Reference),
+  All = Declaration | Definition | Reference,
+};
+
+inline RefKind operator|(RefKind L

[PATCH] D58782: Use ArrayRef::copy, instead of copying data manually

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr created this revision.
gribozavr added a reviewer: ioeric.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58782

Files:
  clang-tools-extra/clangd/index/Ref.cpp


Index: clang-tools-extra/clangd/index/Ref.cpp
===
--- clang-tools-extra/clangd/index/Ref.cpp
+++ clang-tools-extra/clangd/index/Ref.cpp
@@ -51,9 +51,7 @@
 SymRefs.erase(std::unique(SymRefs.begin(), SymRefs.end()), SymRefs.end());
 
 NumRefs += SymRefs.size();
-auto *Array = Arena.Allocate(SymRefs.size());
-std::uninitialized_copy(SymRefs.begin(), SymRefs.end(), Array);
-Result.emplace_back(Sym.first, llvm::ArrayRef(Array, SymRefs.size()));
+Result.emplace_back(Sym.first, llvm::ArrayRef(SymRefs).copy(Arena));
   }
   return RefSlab(std::move(Result), std::move(Arena), NumRefs);
 }


Index: clang-tools-extra/clangd/index/Ref.cpp
===
--- clang-tools-extra/clangd/index/Ref.cpp
+++ clang-tools-extra/clangd/index/Ref.cpp
@@ -51,9 +51,7 @@
 SymRefs.erase(std::unique(SymRefs.begin(), SymRefs.end()), SymRefs.end());
 
 NumRefs += SymRefs.size();
-auto *Array = Arena.Allocate(SymRefs.size());
-std::uninitialized_copy(SymRefs.begin(), SymRefs.end(), Array);
-Result.emplace_back(Sym.first, llvm::ArrayRef(Array, SymRefs.size()));
+Result.emplace_back(Sym.first, llvm::ArrayRef(SymRefs).copy(Arena));
   }
   return RefSlab(std::move(Result), std::move(Arena), NumRefs);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58781: Added missing license headers

[Eric Liu via Phabricator via cfe-commits]

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

lg. thanks!!!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58781



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

[PATCH] D58782: Use ArrayRef::copy, instead of copying data manually

[Eric Liu via Phabricator via cfe-commits]

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

Nice!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58782



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

[PATCH] D58782: Use ArrayRef::copy, instead of copying data manually

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Would it please be possible to write actual commit / DR titles, 
i.e. include appropriate `[tag]`s into them,
and ideally proper commit messages, too?
It really clutters -commit list listing otherwise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58782



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

[clang-tools-extra] r355091 - Use ArrayRef::copy, instead of copying data manually

[Dmitri Gribenko via cfe-commits]

Author: gribozavr
Date: Thu Feb 28 06:00:26 2019
New Revision: 355091

URL: http://llvm.org/viewvc/llvm-project?rev=355091&view=rev
Log:
Use ArrayRef::copy, instead of copying data manually

Reviewers: ioeric

Subscribers: jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

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

Modified: clang-tools-extra/trunk/clangd/index/Ref.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Ref.cpp?rev=355091&r1=355090&r2=355091&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Ref.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Ref.cpp Thu Feb 28 06:00:26 2019
@@ -51,9 +51,7 @@ RefSlab RefSlab::Builder::build() && {
 SymRefs.erase(std::unique(SymRefs.begin(), SymRefs.end()), SymRefs.end());
 
 NumRefs += SymRefs.size();
-auto *Array = Arena.Allocate(SymRefs.size());
-std::uninitialized_copy(SymRefs.begin(), SymRefs.end(), Array);
-Result.emplace_back(Sym.first, llvm::ArrayRef(Array, SymRefs.size()));
+Result.emplace_back(Sym.first, llvm::ArrayRef(SymRefs).copy(Arena));
   }
   return RefSlab(std::move(Result), std::move(Arena), NumRefs);
 }


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

[PATCH] D58782: Use ArrayRef::copy, instead of copying data manually

[Dmitri Gribenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE355091: Use ArrayRef::copy, instead of copying data 
manually (authored by gribozavr, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58782?vs=188729&id=188730#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58782

Files:
  clangd/index/Ref.cpp


Index: clangd/index/Ref.cpp
===
--- clangd/index/Ref.cpp
+++ clangd/index/Ref.cpp
@@ -51,9 +51,7 @@
 SymRefs.erase(std::unique(SymRefs.begin(), SymRefs.end()), SymRefs.end());
 
 NumRefs += SymRefs.size();
-auto *Array = Arena.Allocate(SymRefs.size());
-std::uninitialized_copy(SymRefs.begin(), SymRefs.end(), Array);
-Result.emplace_back(Sym.first, llvm::ArrayRef(Array, SymRefs.size()));
+Result.emplace_back(Sym.first, llvm::ArrayRef(SymRefs).copy(Arena));
   }
   return RefSlab(std::move(Result), std::move(Arena), NumRefs);
 }


Index: clangd/index/Ref.cpp
===
--- clangd/index/Ref.cpp
+++ clangd/index/Ref.cpp
@@ -51,9 +51,7 @@
 SymRefs.erase(std::unique(SymRefs.begin(), SymRefs.end()), SymRefs.end());
 
 NumRefs += SymRefs.size();
-auto *Array = Arena.Allocate(SymRefs.size());
-std::uninitialized_copy(SymRefs.begin(), SymRefs.end(), Array);
-Result.emplace_back(Sym.first, llvm::ArrayRef(Array, SymRefs.size()));
+Result.emplace_back(Sym.first, llvm::ArrayRef(SymRefs).copy(Arena));
   }
   return RefSlab(std::move(Result), std::move(Arena), NumRefs);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r355092 - Added missing license headers

[Dmitri Gribenko via cfe-commits]

Author: gribozavr
Date: Thu Feb 28 06:01:11 2019
New Revision: 355092

URL: http://llvm.org/viewvc/llvm-project?rev=355092&view=rev
Log:
Added missing license headers

Reviewers: ioeric

Subscribers: jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/ExpectedTypes.cpp
clang-tools-extra/trunk/clangd/index/IndexAction.cpp
clang-tools-extra/trunk/clangd/xpc/framework/ClangdXPC.cpp
clang-tools-extra/trunk/clangd/xpc/test-client/ClangdXPCTestClient.cpp

Modified: clang-tools-extra/trunk/clangd/ExpectedTypes.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ExpectedTypes.cpp?rev=355092&r1=355091&r2=355092&view=diff
==
--- clang-tools-extra/trunk/clangd/ExpectedTypes.cpp (original)
+++ clang-tools-extra/trunk/clangd/ExpectedTypes.cpp Thu Feb 28 06:01:11 2019
@@ -1,3 +1,11 @@
+//===--- ExpectedTypes.cpp ---*- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
 #include "ExpectedTypes.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Type.h"

Modified: clang-tools-extra/trunk/clangd/index/IndexAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/IndexAction.cpp?rev=355092&r1=355091&r2=355092&view=diff
==
--- clang-tools-extra/trunk/clangd/index/IndexAction.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/IndexAction.cpp Thu Feb 28 06:01:11 
2019
@@ -1,5 +1,12 @@
-#include "IndexAction.h"
+//===--- IndexAction.cpp -*- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
 
+#include "IndexAction.h"
 #include "index/SymbolOrigin.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Index/IndexDataConsumer.h"

Modified: clang-tools-extra/trunk/clangd/xpc/framework/ClangdXPC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/xpc/framework/ClangdXPC.cpp?rev=355092&r1=355091&r2=355092&view=diff
==
--- clang-tools-extra/trunk/clangd/xpc/framework/ClangdXPC.cpp (original)
+++ clang-tools-extra/trunk/clangd/xpc/framework/ClangdXPC.cpp Thu Feb 28 
06:01:11 2019
@@ -1,3 +1,10 @@
+//===--- ClangXPC.cpp *- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
 
 /// Returns the bundle identifier of the Clangd XPC service.
 extern "C" const char *clangd_xpc_get_bundle_identifier() {

Modified: clang-tools-extra/trunk/clangd/xpc/test-client/ClangdXPCTestClient.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/xpc/test-client/ClangdXPCTestClient.cpp?rev=355092&r1=355091&r2=355092&view=diff
==
--- clang-tools-extra/trunk/clangd/xpc/test-client/ClangdXPCTestClient.cpp 
(original)
+++ clang-tools-extra/trunk/clangd/xpc/test-client/ClangdXPCTestClient.cpp Thu 
Feb 28 06:01:11 2019
@@ -1,3 +1,11 @@
+//===--- ClangXPCTestClient.cpp --*- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
 #include "xpc/Conversion.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/SmallString.h"


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

[PATCH] D58781: Added missing license headers

[Dmitri Gribenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE355092: Added missing license headers (authored by 
gribozavr, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58781?vs=188726&id=188731#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58781

Files:
  clangd/ExpectedTypes.cpp
  clangd/index/IndexAction.cpp
  clangd/xpc/framework/ClangdXPC.cpp
  clangd/xpc/test-client/ClangdXPCTestClient.cpp


Index: clangd/ExpectedTypes.cpp
===
--- clangd/ExpectedTypes.cpp
+++ clangd/ExpectedTypes.cpp
@@ -1,3 +1,11 @@
+//===--- ExpectedTypes.cpp ---*- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
 #include "ExpectedTypes.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Type.h"
Index: clangd/index/IndexAction.cpp
===
--- clangd/index/IndexAction.cpp
+++ clangd/index/IndexAction.cpp
@@ -1,5 +1,12 @@
-#include "IndexAction.h"
+//===--- IndexAction.cpp -*- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
 
+#include "IndexAction.h"
 #include "index/SymbolOrigin.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Index/IndexDataConsumer.h"
Index: clangd/xpc/framework/ClangdXPC.cpp
===
--- clangd/xpc/framework/ClangdXPC.cpp
+++ clangd/xpc/framework/ClangdXPC.cpp
@@ -1,3 +1,10 @@
+//===--- ClangXPC.cpp *- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
 
 /// Returns the bundle identifier of the Clangd XPC service.
 extern "C" const char *clangd_xpc_get_bundle_identifier() {
Index: clangd/xpc/test-client/ClangdXPCTestClient.cpp
===
--- clangd/xpc/test-client/ClangdXPCTestClient.cpp
+++ clangd/xpc/test-client/ClangdXPCTestClient.cpp
@@ -1,3 +1,11 @@
+//===--- ClangXPCTestClient.cpp --*- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
 #include "xpc/Conversion.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/SmallString.h"


Index: clangd/ExpectedTypes.cpp
===
--- clangd/ExpectedTypes.cpp
+++ clangd/ExpectedTypes.cpp
@@ -1,3 +1,11 @@
+//===--- ExpectedTypes.cpp ---*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
 #include "ExpectedTypes.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Type.h"
Index: clangd/index/IndexAction.cpp
===
--- clangd/index/IndexAction.cpp
+++ clangd/index/IndexAction.cpp
@@ -1,5 +1,12 @@
-#include "IndexAction.h"
+//===--- IndexAction.cpp -*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
 
+#include "IndexAction.h"
 #include "index/SymbolOrigin.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Index/IndexDataConsumer.h"
Index: clangd/xpc/framework/ClangdXPC.cpp
===
--- clangd/xpc/framework/ClangdXPC.cpp
+++ clangd/xpc/framework/ClangdXPC.cpp
@@ -1,3 +1,10 @@
+//===--- ClangXPC.cpp --

[PATCH] D58743: Handle built-in when importing SourceLocation and FileID

[Balázs Kéri via Phabricator via cfe-commits]

balazske added inline comments.



Comment at: include/clang/AST/ASTImporter.h:342
 // FIXME: Remove this version.
-FileID Import(FileID);
+FileID Import(FileID, bool isBuiltin=false);
 

teemperor wrote:
> `IsBuiltin`, not `isBuiltin`
`IsBuiltin = false` should be the correct formatting.



Comment at: lib/AST/ASTImporter.cpp:8250
 
 Expected ASTImporter::Import_New(FileID FromID) {
   FileID ToID = Import(FromID);

The new parameter should be added here too.



Comment at: lib/AST/ASTImporter.cpp:8284
+
+if (!isBuiltin) {
+   // Include location of this file.

The `Cache` can be moved into this block and the block to `else if`.



Comment at: lib/AST/ASTImporter.cpp:8301
+ToID = ToSM.createFileID(Entry, ToIncludeLoc,
+ 
FromSLoc.getFile().getFileCharacteristic());
+}

Is it possible that in the `isBuiltin` true case this part of the code does run 
(always) without assertion or other error and the result is always an invalid 
`ToID`? (If yes the whole change is not needed, so I want to know what was the 
reason for this change, was there any crash or bug.)


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

https://reviews.llvm.org/D58743



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

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In D58731#1413552 , @JonasToth wrote:

> In D58731#1413498 , @MyDeveloperDay 
> wrote:
>
> > In D58731#1413476 , @lewmpk wrote:
> >
> > > I'm happy to land this ASAP but I don't have commit rights
> >
> >
> > So one of us could land it for you.. (I've not personally done that before 
> > as I'm a bit green too! but I do have commit rights)
> >
> > Its pretty easy to get commit rights, and given your looking at multiple 
> > issues I'd recommend it.. 
> > (https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access)
>
>
> @MyDeveloperDay if you want to try the procedure out, go ahead ;) Otherwise I 
> can land this patch now.


I'll let you folks do this. But please note that the patch wasn't generated 
with arcanist, so it doesn't apply with `arc patch --nobranch`. Copy-pasting 
the raw diff to `patch -p0` works though (as would downloading the diff and 
feeding it to patch as a file). Make sure to put "Differential Revision: 
https://reviews.llvm.org/D58731"; at the end of the message for Phabricator to 
close this revision automatically.


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

https://reviews.llvm.org/D58731



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

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D58731#1413695 , @alexfh wrote:

> In D58731#1413552 , @JonasToth wrote:
>
> > In D58731#1413498 , 
> > @MyDeveloperDay wrote:
> >
> > > In D58731#1413476 , @lewmpk 
> > > wrote:
> > >
> > > > I'm happy to land this ASAP but I don't have commit rights
> > >
> > >
> > > So one of us could land it for you.. (I've not personally done that 
> > > before as I'm a bit green too! but I do have commit rights)
> > >
> > > Its pretty easy to get commit rights, and given your looking at multiple 
> > > issues I'd recommend it.. 
> > > (https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access)
> >
> >
> > @MyDeveloperDay if you want to try the procedure out, go ahead ;) Otherwise 
> > I can land this patch now.
>
>
> I'll let you folks do this. But please note that the patch wasn't generated 
> with arcanist, so it doesn't apply with `arc patch --nobranch`. Copy-pasting 
> the raw diff to `patch -p0` works though (as would downloading the diff and 
> feeding it to patch as a file). Make sure to put "Differential Revision: 
> https://reviews.llvm.org/D58731"; at the end of the message for Phabricator to 
> close this revision automatically.


@JonasToth  you are more qualified than me please go ahead...


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

https://reviews.llvm.org/D58731



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

[PATCH] D57914: [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

[Bruno Ricci via Phabricator via cfe-commits]

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



Comment at: include/clang/Basic/Sanitizers.h:29
+class hash_code;
+}
 

What ? You are forward-declaring `hash_code` here and using it as a value a few 
lines later. Just include `llvm/ADT/Hashing.h`.


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

https://reviews.llvm.org/D57914



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

[clang-tools-extra] r355093 - [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

[Jonas Toth via cfe-commits]

Author: jonastoth
Date: Thu Feb 28 06:55:12 2019
New Revision: 355093

URL: http://llvm.org/viewvc/llvm-project?rev=355093&view=rev
Log:
[clang-tidy] added cppcoreguidelines-explicit-virtual-functions

Addresses the bugzilla bug #30397. (https://bugs.llvm.org/show_bug.cgi?id=30397)
modernize-use-override suggests that destructors require the override specifier
and the CPP core guidelines do not recommend this.

Patch by lewmpk.

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

Added:

clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-explicit-virtual-functions.rst

clang-tools-extra/trunk/test/clang-tidy/modernize-use-override-no-destructors.cpp
Modified:

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.cpp
clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.h
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-override.rst

Modified: 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp?rev=355093&r1=355092&r2=355093&view=diff
==
--- 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
 (original)
+++ 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
 Thu Feb 28 06:55:12 2019
@@ -12,6 +12,7 @@
 #include "../misc/NonPrivateMemberVariablesInClassesCheck.h"
 #include "../misc/UnconventionalAssignOperatorCheck.h"
 #include "../modernize/AvoidCArraysCheck.h"
+#include "../modernize/UseOverrideCheck.h"
 #include "../readability/MagicNumbersCheck.h"
 #include "AvoidGotoCheck.h"
 #include "InterfacesGlobalInitCheck.h"
@@ -46,6 +47,8 @@ public:
 "cppcoreguidelines-avoid-goto");
 CheckFactories.registerCheck(
 "cppcoreguidelines-avoid-magic-numbers");
+CheckFactories.registerCheck(
+"cppcoreguidelines-explicit-virtual-functions");
 CheckFactories.registerCheck(
 "cppcoreguidelines-interfaces-global-init");
 CheckFactories.registerCheck(
@@ -91,6 +94,9 @@ public:
 Opts["cppcoreguidelines-non-private-member-variables-in-classes."
  "IgnoreClassesWithAllMemberVariablesBeingPublic"] = "1";
 
+Opts["cppcoreguidelines-explicit-virtual-functions."
+ "IgnoreDestructors"] = "1";
+
 return Options;
   }
 };

Modified: clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.cpp?rev=355093&r1=355092&r2=355093&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.cpp Thu Feb 
28 06:55:12 2019
@@ -17,9 +17,20 @@ namespace clang {
 namespace tidy {
 namespace modernize {
 
+void UseOverrideCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnoreDestructors", IgnoreDestructors);
+}
+
 void UseOverrideCheck::registerMatchers(MatchFinder *Finder) {
   // Only register the matcher for C++11.
-  if (getLangOpts().CPlusPlus11)
+  if (!getLangOpts().CPlusPlus11)
+return;
+
+  if (IgnoreDestructors)
+Finder->addMatcher(
+cxxMethodDecl(isOverride(), 
unless(cxxDestructorDecl())).bind("method"),
+this);
+  else
 Finder->addMatcher(cxxMethodDecl(isOverride()).bind("method"), this);
 }
 

Modified: clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.h?rev=355093&r1=355092&r2=355093&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.h (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.h Thu Feb 28 
06:55:12 2019
@@ -19,9 +19,14 @@ namespace modernize {
 class UseOverrideCheck : public ClangTidyCheck {
 public:
   UseOverrideCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+IgnoreDestructors(Options.get("IgnoreDestructors", false)) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+  const bool IgnoreDestructors;
 };
 
 } // namespace modernize

Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=355093&r1=355092&

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

[Jonas Toth via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL355093: [clang-tidy] added 
cppcoreguidelines-explicit-virtual-functions (authored by JonasToth, committed 
by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58731?vs=188642&id=188739#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58731

Files:
  
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.cpp
  clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.h
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-explicit-virtual-functions.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-override.rst
  
clang-tools-extra/trunk/test/clang-tidy/modernize-use-override-no-destructors.cpp

Index: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -12,6 +12,7 @@
 #include "../misc/NonPrivateMemberVariablesInClassesCheck.h"
 #include "../misc/UnconventionalAssignOperatorCheck.h"
 #include "../modernize/AvoidCArraysCheck.h"
+#include "../modernize/UseOverrideCheck.h"
 #include "../readability/MagicNumbersCheck.h"
 #include "AvoidGotoCheck.h"
 #include "InterfacesGlobalInitCheck.h"
@@ -46,6 +47,8 @@
 "cppcoreguidelines-avoid-goto");
 CheckFactories.registerCheck(
 "cppcoreguidelines-avoid-magic-numbers");
+CheckFactories.registerCheck(
+"cppcoreguidelines-explicit-virtual-functions");
 CheckFactories.registerCheck(
 "cppcoreguidelines-interfaces-global-init");
 CheckFactories.registerCheck(
@@ -91,6 +94,9 @@
 Opts["cppcoreguidelines-non-private-member-variables-in-classes."
  "IgnoreClassesWithAllMemberVariablesBeingPublic"] = "1";
 
+Opts["cppcoreguidelines-explicit-virtual-functions."
+ "IgnoreDestructors"] = "1";
+
 return Options;
   }
 };
Index: clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.h
+++ clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.h
@@ -19,9 +19,14 @@
 class UseOverrideCheck : public ClangTidyCheck {
 public:
   UseOverrideCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+IgnoreDestructors(Options.get("IgnoreDestructors", false)) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+  const bool IgnoreDestructors;
 };
 
 } // namespace modernize
Index: clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.cpp
@@ -17,9 +17,20 @@
 namespace tidy {
 namespace modernize {
 
+void UseOverrideCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnoreDestructors", IgnoreDestructors);
+}
+
 void UseOverrideCheck::registerMatchers(MatchFinder *Finder) {
   // Only register the matcher for C++11.
-  if (getLangOpts().CPlusPlus11)
+  if (!getLangOpts().CPlusPlus11)
+return;
+
+  if (IgnoreDestructors)
+Finder->addMatcher(
+cxxMethodDecl(isOverride(), unless(cxxDestructorDecl())).bind("method"),
+this);
+  else
 Finder->addMatcher(cxxMethodDecl(isOverride()).bind("method"), this);
 }
 
Index: clang-tools-extra/trunk/docs/ReleaseNotes.rst
===
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst
@@ -98,6 +98,11 @@
   Checks whether there are underscores in googletest test and test case names in
   test macros, which is prohibited by the Googletest FAQ.
 
+- New alias :doc:`cppcoreguidelines-explicit-virtual-functions
+  ` to
+  :doc:`modernize-use-override
+  ` was added.
+
 - The :doc:`bugprone-argument-comment
   ` now supports
   `CommentBoolLiterals`, `CommentIntegerLiterals`,  `CommentFloatLiterals`,
Index: clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-override.rst
===
--- clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-override.rst
+++ clan

[clang-tools-extra] r355094 - [clang-tidy] attempt to fix documentation build-error

[Jonas Toth via cfe-commits]

Author: jonastoth
Date: Thu Feb 28 07:01:17 2019
New Revision: 355094

URL: http://llvm.org/viewvc/llvm-project?rev=355094&view=rev
Log:
[clang-tidy] attempt to fix documentation build-error

Modified:
clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-time-subtraction.rst

Modified: 
clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-time-subtraction.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-time-subtraction.rst?rev=355094&r1=355093&r2=355094&view=diff
==
--- clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-time-subtraction.rst 
(original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-time-subtraction.rst 
Thu Feb 28 07:01:17 2019
@@ -8,9 +8,10 @@ in the Time domain instead of the numeri
 
 There are two cases of Time subtraction in which deduce additional type
 information:
- - When the result is an ``absl::Duration`` and the first argument is an
-   ``absl::Time``.
- - When the second argument is a ``absl::Time``.
+
+- When the result is an ``absl::Duration`` and the first argument is an
+  ``absl::Time``.
+- When the second argument is a ``absl::Time``.
 
 In the first case, we must know the result of the operation, since without that
 the second operand could be either an ``absl::Time`` or an ``absl::Duration``.


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

[PATCH] D57906: [CTU] Do not allow different CPP dialects in CTU

[Gábor Horváth via Phabricator via cfe-commits]

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

LGTM! I think we should commit this as is for now but maybe adding a TODO 
comment to summarize the problem would be nice. Maybe we could have an 
isSameDialect or similar method within LangOpts, so it is harder to break this 
code.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57906



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

[clang-tools-extra] r355095 - [clang-tidy] another issue in documentation, double empty line seemed to confuse code-block

[Jonas Toth via cfe-commits]

Author: jonastoth
Date: Thu Feb 28 07:18:54 2019
New Revision: 355095

URL: http://llvm.org/viewvc/llvm-project?rev=355095&view=rev
Log:
[clang-tidy] another issue in documentation, double empty line seemed to 
confuse code-block

Modified:

clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-subtraction.rst

Modified: 
clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-subtraction.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-subtraction.rst?rev=355095&r1=355094&r2=355095&view=diff
==
--- 
clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-subtraction.rst 
(original)
+++ 
clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-subtraction.rst 
Thu Feb 28 07:18:54 2019
@@ -21,7 +21,6 @@ Examples:
   // Suggestion - Subtraction in the absl::Duration domain instead
   double result = absl::ToDoubleSeconds(d - absl::Seconds(x));
 
-
   // Original - Subtraction of two Durations in the double domain
   absl::Duration d1, d2;
   double result = absl::ToDoubleSeconds(d1) - absl::ToDoubleSeconds(d2);


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

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Thank you for the patch!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58731



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

r355096 - [CTU] Do not allow different CPP dialects in CTU

[Gabor Marton via cfe-commits]

Author: martong
Date: Thu Feb 28 07:24:59 2019
New Revision: 355096

URL: http://llvm.org/viewvc/llvm-project?rev=355096&view=rev
Log:
[CTU] Do not allow different CPP dialects in CTU

Summary:
If CPP dialects are different then return with error.

Consider this STL code:
  template
struct __alloc_traits
  #if __cplusplus >= 201103L
: std::allocator_traits<_Alloc>
  #endif
{ // ...
};
This class template would create ODR errors during merging the two units,
since in one translation unit the class template has a base class, however
in the other unit it has none.

Reviewers: xazax.hun, a_sidorin, r.stahl

Subscribers: rnkovacs, dkrupp, Szelethus, gamesh411, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h
cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp

Modified: cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h?rev=355096&r1=355095&r2=355096&view=diff
==
--- cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h (original)
+++ cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h Thu Feb 28 07:24:59 
2019
@@ -43,7 +43,8 @@ enum class index_error_code {
   failed_to_get_external_ast,
   failed_to_generate_usr,
   triple_mismatch,
-  lang_mismatch
+  lang_mismatch,
+  lang_dialect_mismatch
 };
 
 class IndexError : public llvm::ErrorInfo {

Modified: cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp?rev=355096&r1=355095&r2=355096&view=diff
==
--- cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp (original)
+++ cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp Thu Feb 28 07:24:59 2019
@@ -42,6 +42,7 @@ STATISTIC(NumGetCTUSuccess,
   "requested function's body");
 STATISTIC(NumTripleMismatch, "The # of triple mismatches");
 STATISTIC(NumLangMismatch, "The # of language mismatches");
+STATISTIC(NumLangDialectMismatch, "The # of language dialect mismatches");
 
 // Same as Triple's equality operator, but we check a field only if that is
 // known in both instances.
@@ -99,6 +100,8 @@ public:
   return "Triple mismatch";
 case index_error_code::lang_mismatch:
   return "Language mismatch";
+case index_error_code::lang_dialect_mismatch:
+  return "Language dialect mismatch";
 }
 llvm_unreachable("Unrecognized index_error_code.");
   }
@@ -228,6 +231,7 @@ CrossTranslationUnitContext::getCrossTUD
 
   const auto &LangTo = Context.getLangOpts();
   const auto &LangFrom = Unit->getASTContext().getLangOpts();
+
   // FIXME: Currenty we do not support CTU across C++ and C and across
   // different dialects of C++.
   if (LangTo.CPlusPlus != LangFrom.CPlusPlus) {
@@ -235,6 +239,28 @@ CrossTranslationUnitContext::getCrossTUD
 return llvm::make_error(index_error_code::lang_mismatch);
   }
 
+  // If CPP dialects are different then return with error.
+  //
+  // Consider this STL code:
+  //   template
+  // struct __alloc_traits
+  //   #if __cplusplus >= 201103L
+  // : std::allocator_traits<_Alloc>
+  //   #endif
+  // { // ...
+  // };
+  // This class template would create ODR errors during merging the two units,
+  // since in one translation unit the class template has a base class, however
+  // in the other unit it has none.
+  if (LangTo.CPlusPlus11 != LangFrom.CPlusPlus11 ||
+  LangTo.CPlusPlus14 != LangFrom.CPlusPlus14 ||
+  LangTo.CPlusPlus17 != LangFrom.CPlusPlus17 ||
+  LangTo.CPlusPlus2a != LangFrom.CPlusPlus2a) {
+++NumLangDialectMismatch;
+return llvm::make_error(
+index_error_code::lang_dialect_mismatch);
+  }
+
   TranslationUnitDecl *TU = Unit->getASTContext().getTranslationUnitDecl();
   if (const FunctionDecl *ResultDecl =
   findFunctionInDeclContext(TU, LookupFnName))


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

[PATCH] D57906: [CTU] Do not allow different CPP dialects in CTU

[Gabor Marton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
martong marked an inline comment as done.
Closed by commit rC355096: [CTU] Do not allow different CPP dialects in CTU 
(authored by martong, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57906?vs=185926&id=188740#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D57906

Files:
  include/clang/CrossTU/CrossTranslationUnit.h
  lib/CrossTU/CrossTranslationUnit.cpp


Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -42,6 +42,7 @@
   "requested function's body");
 STATISTIC(NumTripleMismatch, "The # of triple mismatches");
 STATISTIC(NumLangMismatch, "The # of language mismatches");
+STATISTIC(NumLangDialectMismatch, "The # of language dialect mismatches");
 
 // Same as Triple's equality operator, but we check a field only if that is
 // known in both instances.
@@ -99,6 +100,8 @@
   return "Triple mismatch";
 case index_error_code::lang_mismatch:
   return "Language mismatch";
+case index_error_code::lang_dialect_mismatch:
+  return "Language dialect mismatch";
 }
 llvm_unreachable("Unrecognized index_error_code.");
   }
@@ -228,6 +231,7 @@
 
   const auto &LangTo = Context.getLangOpts();
   const auto &LangFrom = Unit->getASTContext().getLangOpts();
+
   // FIXME: Currenty we do not support CTU across C++ and C and across
   // different dialects of C++.
   if (LangTo.CPlusPlus != LangFrom.CPlusPlus) {
@@ -235,6 +239,28 @@
 return llvm::make_error(index_error_code::lang_mismatch);
   }
 
+  // If CPP dialects are different then return with error.
+  //
+  // Consider this STL code:
+  //   template
+  // struct __alloc_traits
+  //   #if __cplusplus >= 201103L
+  // : std::allocator_traits<_Alloc>
+  //   #endif
+  // { // ...
+  // };
+  // This class template would create ODR errors during merging the two units,
+  // since in one translation unit the class template has a base class, however
+  // in the other unit it has none.
+  if (LangTo.CPlusPlus11 != LangFrom.CPlusPlus11 ||
+  LangTo.CPlusPlus14 != LangFrom.CPlusPlus14 ||
+  LangTo.CPlusPlus17 != LangFrom.CPlusPlus17 ||
+  LangTo.CPlusPlus2a != LangFrom.CPlusPlus2a) {
+++NumLangDialectMismatch;
+return llvm::make_error(
+index_error_code::lang_dialect_mismatch);
+  }
+
   TranslationUnitDecl *TU = Unit->getASTContext().getTranslationUnitDecl();
   if (const FunctionDecl *ResultDecl =
   findFunctionInDeclContext(TU, LookupFnName))
Index: include/clang/CrossTU/CrossTranslationUnit.h
===
--- include/clang/CrossTU/CrossTranslationUnit.h
+++ include/clang/CrossTU/CrossTranslationUnit.h
@@ -43,7 +43,8 @@
   failed_to_get_external_ast,
   failed_to_generate_usr,
   triple_mismatch,
-  lang_mismatch
+  lang_mismatch,
+  lang_dialect_mismatch
 };
 
 class IndexError : public llvm::ErrorInfo {


Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -42,6 +42,7 @@
   "requested function's body");
 STATISTIC(NumTripleMismatch, "The # of triple mismatches");
 STATISTIC(NumLangMismatch, "The # of language mismatches");
+STATISTIC(NumLangDialectMismatch, "The # of language dialect mismatches");
 
 // Same as Triple's equality operator, but we check a field only if that is
 // known in both instances.
@@ -99,6 +100,8 @@
   return "Triple mismatch";
 case index_error_code::lang_mismatch:
   return "Language mismatch";
+case index_error_code::lang_dialect_mismatch:
+  return "Language dialect mismatch";
 }
 llvm_unreachable("Unrecognized index_error_code.");
   }
@@ -228,6 +231,7 @@
 
   const auto &LangTo = Context.getLangOpts();
   const auto &LangFrom = Unit->getASTContext().getLangOpts();
+
   // FIXME: Currenty we do not support CTU across C++ and C and across
   // different dialects of C++.
   if (LangTo.CPlusPlus != LangFrom.CPlusPlus) {
@@ -235,6 +239,28 @@
 return llvm::make_error(index_error_code::lang_mismatch);
   }
 
+  // If CPP dialects are different then return with error.
+  //
+  // Consider this STL code:
+  //   template
+  // struct __alloc_traits
+  //   #if __cplusplus >= 201103L
+  // : std::allocator_traits<_Alloc>
+  //   #endif
+  // { // ...
+  // };
+  // This class template would create ODR errors during merging the two units,
+  // since in one translation unit the class template has a base class, however
+  // in the other unit it has none.
+  if (LangTo.CPlusPlus11 != LangFrom.CPlusPlus11 ||
+  LangTo.CPlusPlus14 != LangFrom.CPlusPlus14 ||
+  LangTo.CPlusPlus17 != Lan

[clang-tools-extra] r355097 - [clang-tidy] tryfix documenation continued

[Jonas Toth via cfe-commits]

Author: jonastoth
Date: Thu Feb 28 07:28:36 2019
New Revision: 355097

URL: http://llvm.org/viewvc/llvm-project?rev=355097&view=rev
Log:
[clang-tidy] tryfix documenation continued

Modified:

clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-subtraction.rst

Modified: 
clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-subtraction.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-subtraction.rst?rev=355097&r1=355096&r2=355097&view=diff
==
--- 
clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-subtraction.rst 
(original)
+++ 
clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-subtraction.rst 
Thu Feb 28 07:28:36 2019
@@ -28,6 +28,7 @@ Examples:
   // Suggestion - Subtraction in the absl::Duration domain instead
   double result = absl::ToDoubleSeconds(d1 - d2);
 
+
 Note: As with other ``clang-tidy`` checks, it is possible that multiple fixes
 may overlap (as in the case of nested expressions), so not all occurences can
 be transformed in one run. In particular, this may occur for nested subtraction


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

[clang-tools-extra] r355098 - [clang-tidy] documentation fixing the actual correct file

[Jonas Toth via cfe-commits]

Author: jonastoth
Date: Thu Feb 28 07:39:47 2019
New Revision: 355098

URL: http://llvm.org/viewvc/llvm-project?rev=355098&view=rev
Log:
[clang-tidy] documentation fixing the actual correct file

Modified:
clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-time-subtraction.rst

Modified: 
clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-time-subtraction.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-time-subtraction.rst?rev=355098&r1=355097&r2=355098&view=diff
==
--- clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-time-subtraction.rst 
(original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-time-subtraction.rst 
Thu Feb 28 07:39:47 2019
@@ -21,6 +21,7 @@ subtracting an ``absl::Time`` from an ``
 Examples:
 
 .. code-block:: c++
+
   int x;
   absl::Time t;
 


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

[clang-tools-extra] r355100 - [clang-tidy] include cppcoreguidelines-explicit-virtual-functions in list of checks and fix redirection

[Jonas Toth via cfe-commits]

Author: jonastoth
Date: Thu Feb 28 07:47:10 2019
New Revision: 355100

URL: http://llvm.org/viewvc/llvm-project?rev=355100&view=rev
Log:
[clang-tidy] include cppcoreguidelines-explicit-virtual-functions in list of 
checks and fix redirection

Modified:

clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-explicit-virtual-functions.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: 
clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-explicit-virtual-functions.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-explicit-virtual-functions.rst?rev=355100&r1=355099&r2=355100&view=diff
==
--- 
clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-explicit-virtual-functions.rst
 (original)
+++ 
clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-explicit-virtual-functions.rst
 Thu Feb 28 07:47:10 2019
@@ -1,6 +1,6 @@
 .. title:: clang-tidy - cppcoreguidelines-explicit-virtual-functions
 .. meta::
-   :http-equiv=refresh: 5;URL=cppcoreguidelines-explicit-virtual-functions.html
+   :http-equiv=refresh: 5;URL=modernize-use-override.html
 
 cppcoreguidelines-explicit-virtual-functions
 

Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=355100&r1=355099&r2=355100&view=diff
==
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Thu Feb 28 07:47:10 
2019
@@ -99,6 +99,7 @@ Clang-Tidy Checks
cppcoreguidelines-avoid-goto
cppcoreguidelines-avoid-magic-numbers (redirects to 
readability-magic-numbers) 
cppcoreguidelines-c-copy-assignment-signature (redirects to 
misc-unconventional-assign-operator) 

+   cppcoreguidelines-explicit-virtual-functions (redirects to 
modernize-use-override) 
cppcoreguidelines-interfaces-global-init
cppcoreguidelines-macro-usage
cppcoreguidelines-narrowing-conversions


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

[PATCH] D54881: [clang-format] Prevent Clang-Format from editing leading whitespace on lines outside of the format range

[Russell McClellan via Phabricator via cfe-commits]

russellmcc added a comment.
Herald added a subscriber: jdoerfert.

Bump!


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

https://reviews.llvm.org/D54881



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

[PATCH] D40988: Clang-format: add finer-grained options for putting all arguments on one line

[Russell McClellan via Phabricator via cfe-commits]

russellmcc added a comment.

Bump!  Would really appreciate any feedback on this!


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

https://reviews.llvm.org/D40988



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

[PATCH] D57914: [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

[pierre gousseau via Phabricator via cfe-commits]

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



Comment at: include/clang/Basic/Sanitizers.h:29
+class hash_code;
+}
 

riccibruno wrote:
> What ? You are forward-declaring `hash_code` here and using it as a value a 
> few lines later. Just include `llvm/ADT/Hashing.h`.
Thanks for reviewing!
I am happy to include `Hashing.h` but I thought it was generally frown upon to 
add includes if avoidable?
`hash_code` is only used in the return type of the `hash_value()` declaration 
here no?
I saw StringRef.h or APInt.h also forward declare `hash_code`.
Thanks again for reviewing and sorry for the many iterations.


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

https://reviews.llvm.org/D57914



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

[clang-tools-extra] r355102 - [clang-tidy] redirection in list of checks adjusted

[Jonas Toth via cfe-commits]

Author: jonastoth
Date: Thu Feb 28 07:56:40 2019
New Revision: 355102

URL: http://llvm.org/viewvc/llvm-project?rev=355102&view=rev
Log:
[clang-tidy] redirection in list of checks adjusted

Modified:
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=355102&r1=355101&r2=355102&view=diff
==
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Thu Feb 28 07:56:40 
2019
@@ -99,7 +99,7 @@ Clang-Tidy Checks
cppcoreguidelines-avoid-goto
cppcoreguidelines-avoid-magic-numbers (redirects to 
readability-magic-numbers) 
cppcoreguidelines-c-copy-assignment-signature (redirects to 
misc-unconventional-assign-operator) 

-   cppcoreguidelines-explicit-virtual-functions (redirects to 
modernize-use-override) 
+   cppcoreguidelines-explicit-virtual-functions (redirects to 
modernize-use-override) 
cppcoreguidelines-interfaces-global-init
cppcoreguidelines-macro-usage
cppcoreguidelines-narrowing-conversions


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

[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

[David Chisnall via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CGObjCGNU.cpp:188
 
+  StringRef SymbolPrefix() {
+return CGM.getTriple().isOSBinFormatCOFF() ? "_" : "._";

DHowett-MSFT wrote:
> Should this be `SymbolPrefix()` or something more like 
> `MangleSymbol(StringRef sym)`? I know that right now we only need to prepend 
> `_` or `._`, but is it more future-proof to make it generic?
I have refactored this, and then tried adding a $ instead of the . for 
mangling.  On further testing, the latest link.exe from VS 2017 no longer 
complains about symbols starting with a dot, so I'm inclined to revert that 
part of it entirely (lld-link.exe was always happy).  I'd prefer to minimise 
differences between COFF and ELF and this means that we have different section 
names, but aside from needing the extra global initialisation on COFF, 
everything else is the same.  



Comment at: clang/lib/CodeGen/CGObjCGNU.cpp:1528
 InitStructBuilder.addInt(Int64Ty, 0);
-for (auto *s : SectionsBaseNames) {
-  auto bounds = GetSectionBounds(s);
-  InitStructBuilder.add(bounds.first);
-  InitStructBuilder.add(bounds.second);
-};
+if (CGM.getTriple().isOSBinFormatCOFF()) {
+  for (auto *s : PECOFFSectionsBaseNames) {

DHowett-MSFT wrote:
> We may be able to reduce the duplication here (aside: it is very cool that 
> Phabricator shows duplicated lines) by capturing `auto sectionVec = 
> &SectionsBaseNames;` and switching which is in use based on bin format.
I thought about that and didn't do it because if the two arrays are different 
lengths, they're different types.  But, of course, they're the same length and 
if they're ever different lengths in the future then that's probably a bug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58724



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

[PATCH] D57914: [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added inline comments.



Comment at: include/clang/Basic/Sanitizers.h:29
+class hash_code;
+}
 

pgousseau wrote:
> riccibruno wrote:
> > What ? You are forward-declaring `hash_code` here and using it as a value a 
> > few lines later. Just include `llvm/ADT/Hashing.h`.
> Thanks for reviewing!
> I am happy to include `Hashing.h` but I thought it was generally frown upon 
> to add includes if avoidable?
> `hash_code` is only used in the return type of the `hash_value()` declaration 
> here no?
> I saw StringRef.h or APInt.h also forward declare `hash_code`.
> Thanks again for reviewing and sorry for the many iterations.
Never mind, I overlooked that it is only the declaration of `llvm::hash_code 
hash_value(const clang::SanitizerMask &Arg);` and not the definition. Sorry 
about that.

I believe that is it correct to have the overload of `hash_value` in the 
namespace `clang`, so that it can be found by ADL since `SanitizerMask` is in 
`clang`. However you can probably move the declaration of `llvm::hash_code 
hash_value(const clang::SanitizerMask &Arg);` just after `SanitizerMask` so 
that you don't have to forward-declare `SanitizerMask` (but keep the 
forward-declaration of `hash_code` in `llvm`).


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

https://reviews.llvm.org/D57914



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

[PATCH] D57914: [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

[pierre gousseau via Phabricator via cfe-commits]

pgousseau updated this revision to Diff 188748.
pgousseau added a comment.

Change `hash_value()` declaration's location as suggested.


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

https://reviews.llvm.org/D57914

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/Sanitizers.def
  include/clang/Basic/Sanitizers.h
  include/clang/Driver/ToolChain.h
  lib/Basic/SanitizerSpecialCaseList.cpp
  lib/Basic/Sanitizers.cpp
  lib/CodeGen/CGExpr.cpp
  lib/Driver/SanitizerArgs.cpp
  lib/Driver/ToolChain.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaDeclAttr.cpp

Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -6211,7 +6211,8 @@
 if (!S.checkStringLiteralArgumentAttr(AL, I, SanitizerName, &LiteralLoc))
   return;
 
-if (parseSanitizerValue(SanitizerName, /*AllowGroups=*/true) == 0)
+if (parseSanitizerValue(SanitizerName, /*AllowGroups=*/true) ==
+SanitizerMask())
   S.Diag(LiteralLoc, diag::warn_unknown_sanitizer_ignored) << SanitizerName;
 else if (isGlobalVar(D) && SanitizerName != "address")
   S.Diag(D->getLocation(), diag::err_attribute_wrong_decl_type)
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -551,7 +551,7 @@
 DiagnosticsEngine &Diags, SanitizerSet &S) {
   for (const auto &Sanitizer : Sanitizers) {
 SanitizerMask K = parseSanitizerValue(Sanitizer, /*AllowGroups=*/false);
-if (K == 0)
+if (K == SanitizerMask())
   Diags.Report(diag::err_drv_invalid_value) << FlagName << Sanitizer;
 else
   S.set(K, true);
Index: lib/Driver/ToolChain.cpp
===
--- lib/Driver/ToolChain.cpp
+++ lib/Driver/ToolChain.cpp
@@ -818,21 +818,23 @@
   // Return sanitizers which don't require runtime support and are not
   // platform dependent.
 
-  using namespace SanitizerKind;
-
-  SanitizerMask Res = (Undefined & ~Vptr & ~Function) | (CFI & ~CFIICall) |
-  CFICastStrict | UnsignedIntegerOverflow |
-  ImplicitConversion | Nullability | LocalBounds;
+  SanitizerMask Res = (SanitizerKind::Undefined & ~SanitizerKind::Vptr &
+   ~SanitizerKind::Function) |
+  (SanitizerKind::CFI & ~SanitizerKind::CFIICall) |
+  SanitizerKind::CFICastStrict |
+  SanitizerKind::UnsignedIntegerOverflow |
+  SanitizerKind::ImplicitConversion |
+  SanitizerKind::Nullability | SanitizerKind::LocalBounds;
   if (getTriple().getArch() == llvm::Triple::x86 ||
   getTriple().getArch() == llvm::Triple::x86_64 ||
   getTriple().getArch() == llvm::Triple::arm ||
   getTriple().getArch() == llvm::Triple::aarch64 ||
   getTriple().getArch() == llvm::Triple::wasm32 ||
   getTriple().getArch() == llvm::Triple::wasm64)
-Res |= CFIICall;
+Res |= SanitizerKind::CFIICall;
   if (getTriple().getArch() == llvm::Triple::x86_64 ||
   getTriple().getArch() == llvm::Triple::aarch64)
-Res |= ShadowCallStack;
+Res |= SanitizerKind::ShadowCallStack;
   return Res;
 }
 
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -21,33 +21,52 @@
 #include 
 
 using namespace clang;
-using namespace clang::SanitizerKind;
 using namespace clang::driver;
 using namespace llvm::opt;
 
-enum : SanitizerMask {
-  NeedsUbsanRt = Undefined | Integer | ImplicitConversion | Nullability | CFI,
-  NeedsUbsanCxxRt = Vptr | CFI,
-  NotAllowedWithTrap = Vptr,
-  NotAllowedWithMinimalRuntime = Vptr,
-  RequiresPIE = DataFlow | HWAddress | Scudo,
-  NeedsUnwindTables = Address | HWAddress | Thread | Memory | DataFlow,
-  SupportsCoverage = Address | HWAddress | KernelAddress | KernelHWAddress |
- Memory | KernelMemory | Leak | Undefined | Integer |
- ImplicitConversion | Nullability | DataFlow | Fuzzer |
- FuzzerNoLink,
-  RecoverableByDefault = Undefined | Integer | ImplicitConversion | Nullability,
-  Unrecoverable = Unreachable | Return,
-  AlwaysRecoverable = KernelAddress | KernelHWAddress,
-  LegacyFsanitizeRecoverMask = Undefined | Integer,
-  NeedsLTO = CFI,
-  TrappingSupported = (Undefined & ~Vptr) | UnsignedIntegerOverflow |
-  ImplicitConversion | Nullability | LocalBounds | CFI,
-  TrappingDefault = CFI,
-  CFIClasses =
-  CFIVCall | CFINVCall | CFIMFCall | CFIDerivedCast | CFIUnrelatedCast,
-  CompatibleWithMinimalRuntime = TrappingSupported | Scudo | ShadowCallStack,
-};
+static const SanitizerMask NeedsUbsanRt =
+SanitizerKind::Undefined | S

[PATCH] D57914: [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

[pierre gousseau via Phabricator via cfe-commits]

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



Comment at: include/clang/Basic/Sanitizers.h:29
+class hash_code;
+}
 

riccibruno wrote:
> pgousseau wrote:
> > riccibruno wrote:
> > > What ? You are forward-declaring `hash_code` here and using it as a value 
> > > a few lines later. Just include `llvm/ADT/Hashing.h`.
> > Thanks for reviewing!
> > I am happy to include `Hashing.h` but I thought it was generally frown upon 
> > to add includes if avoidable?
> > `hash_code` is only used in the return type of the `hash_value()` 
> > declaration here no?
> > I saw StringRef.h or APInt.h also forward declare `hash_code`.
> > Thanks again for reviewing and sorry for the many iterations.
> Never mind, I overlooked that it is only the declaration of `llvm::hash_code 
> hash_value(const clang::SanitizerMask &Arg);` and not the definition. Sorry 
> about that.
> 
> I believe that is it correct to have the overload of `hash_value` in the 
> namespace `clang`, so that it can be found by ADL since `SanitizerMask` is in 
> `clang`. However you can probably move the declaration of `llvm::hash_code 
> hash_value(const clang::SanitizerMask &Arg);` just after `SanitizerMask` so 
> that you don't have to forward-declare `SanitizerMask` (but keep the 
> forward-declaration of `hash_code` in `llvm`).
Sounds good, done!


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

https://reviews.llvm.org/D57914



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

[PATCH] D58537: lib/Header: Simplify CMakeLists.txt

[Tom Stellard via Phabricator via cfe-commits]

tstellar updated this revision to Diff 188749.
tstellar added a comment.

Fix an issue with the generated arm headers that I discovered after
doing some more testing.  Also, remove comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58537

Files:
  clang/lib/Headers/CMakeLists.txt


Index: clang/lib/Headers/CMakeLists.txt
===
--- clang/lib/Headers/CMakeLists.txt
+++ clang/lib/Headers/CMakeLists.txt
@@ -123,60 +123,51 @@
 )
 
 set(output_dir ${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}/include)
-
-# Generate arm_neon.h
-clang_tablegen(arm_neon.h -gen-arm-neon
-  -I ${CLANG_SOURCE_DIR}/include/clang/Basic/
-  SOURCE ${CLANG_SOURCE_DIR}/include/clang/Basic/arm_neon.td)
-# Generate arm_fp16.h
-clang_tablegen(arm_fp16.h -gen-arm-fp16
-  -I ${CLANG_SOURCE_DIR}/include/clang/Basic/
-  SOURCE ${CLANG_SOURCE_DIR}/include/clang/Basic/arm_fp16.td)
-
 set(out_files)
-foreach( f ${files} ${cuda_wrapper_files} )
-  set( src ${CMAKE_CURRENT_SOURCE_DIR}/${f} )
-  set( dst ${output_dir}/${f} )
+
+function(copy_header_to_output_dir src_dir file)
+  set(src ${src_dir}/${file})
+  set(dst ${output_dir}/${file})
   add_custom_command(OUTPUT ${dst}
 DEPENDS ${src}
 COMMAND ${CMAKE_COMMAND} -E copy_if_different ${src} ${dst}
-COMMENT "Copying clang's ${f}...")
+COMMENT "Copying clang's ${file}...")
   list(APPEND out_files ${dst})
+  set(out_files ${out_files} PARENT_SCOPE)
+endfunction(copy_header_to_output_dir)
+
+function(clang_generate_header td_option td_file out_file)
+  clang_tablegen(${out_file} ${td_option}
+  -I ${CLANG_SOURCE_DIR}/include/clang/Basic/
+  SOURCE ${CLANG_SOURCE_DIR}/include/clang/Basic/${td_file})
+
+  copy_header_to_output_dir(${CMAKE_CURRENT_BINARY_DIR} ${out_file})
+  set(out_files ${out_files} PARENT_SCOPE)
+endfunction(clang_generate_header)
+
+
+# Copy header files from the source directory to the build directory
+foreach( f ${files} ${cuda_wrapper_files} )
+  copy_header_to_output_dir(${CMAKE_CURRENT_SOURCE_DIR} ${f})
 endforeach( f )
 
-add_custom_command(OUTPUT ${output_dir}/arm_neon.h
-  DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/arm_neon.h
-  COMMAND ${CMAKE_COMMAND} -E copy_if_different 
${CMAKE_CURRENT_BINARY_DIR}/arm_neon.h ${output_dir}/arm_neon.h
-  COMMENT "Copying clang's arm_neon.h...")
-list(APPEND out_files ${output_dir}/arm_neon.h)
-add_custom_command(OUTPUT ${output_dir}/arm_fp16.h
-  DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/arm_fp16.h
-  COMMAND ${CMAKE_COMMAND} -E copy_if_different 
${CMAKE_CURRENT_BINARY_DIR}/arm_fp16.h ${output_dir}/arm_fp16.h
-  COMMENT "Copying clang's arm_fp16.h...")
-list(APPEND out_files ${output_dir}/arm_fp16.h)
+# Generate header files and copy them to the build directory
+# Generate arm_neon.h
+clang_generate_header(-gen-arm-neon arm_neon.td arm_neon.h)
+# Generate arm_fp16.h
+clang_generate_header(-gen-arm-fp16 arm_fp16.td arm_fp16.h)
 
 add_custom_target(clang-headers ALL DEPENDS ${out_files})
 set_target_properties(clang-headers PROPERTIES
   FOLDER "Misc"
   RUNTIME_OUTPUT_DIRECTORY "${output_dir}")
 
-install(
-  FILES ${files} ${CMAKE_CURRENT_BINARY_DIR}/arm_neon.h
-  COMPONENT clang-headers
-  PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ
-  DESTINATION lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/include)
-
-install(
-  FILES ${files} ${CMAKE_CURRENT_BINARY_DIR}/arm_fp16.h
-  COMPONENT clang-headers
-  PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ
-  DESTINATION lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/include)
+set(header_install_dir lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION})
 
 install(
-  FILES ${cuda_wrapper_files}
-  COMPONENT clang-headers
-  PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ
-  DESTINATION 
lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/include/cuda_wrappers)
+  DIRECTORY ${output_dir}
+  DESTINATION ${header_install_dir}
+  COMPONENT clang-headers)
 
 if (NOT LLVM_ENABLE_IDE)
   add_llvm_install_targets(install-clang-headers


Index: clang/lib/Headers/CMakeLists.txt
===
--- clang/lib/Headers/CMakeLists.txt
+++ clang/lib/Headers/CMakeLists.txt
@@ -123,60 +123,51 @@
 )
 
 set(output_dir ${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}/include)
-
-# Generate arm_neon.h
-clang_tablegen(arm_neon.h -gen-arm-neon
-  -I ${CLANG_SOURCE_DIR}/include/clang/Basic/
-  SOURCE ${CLANG_SOURCE_DIR}/include/clang/Basic/arm_neon.td)
-# Generate arm_fp16.h
-clang_tablegen(arm_fp16.h -gen-arm-fp16
-  -I ${CLANG_SOURCE_DIR}/include/clang/Basic/
-  SOURCE ${CLANG_SOURCE_DIR}/include/clang/Basic/arm_fp16.td)
-
 set(out_files)
-foreach( f ${files} ${cuda_wrapper_files} )
-  set( src ${CMAKE_CURRENT_SOURCE_DIR}/${f} )
-  set( dst ${output_dir}/${f} )
+
+function(copy_header_to_output_dir src_dir file)
+  set(src ${src_dir}/${file})
+  set(dst ${output_dir}/${file})
   add

r355106 - Partial revert of r353952: [HIP] Handle compile -m options and propagate into LLC

[Yaxun Liu via cfe-commits]

Author: yaxunl
Date: Thu Feb 28 09:08:26 2019
New Revision: 355106

URL: http://llvm.org/viewvc/llvm-project?rev=355106&view=rev
Log:
Partial revert of r353952: [HIP] Handle compile -m options and propagate into 
LLC

Remove comments and tests about passing -mcode-object-v3 to driver since it does
not work. Other -m options are OK.

Also put back -mattr=-code-object-v3 since HIP is still not ready for code 
object
v3.

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

Modified:
cfe/trunk/lib/Driver/ToolChains/HIP.cpp
cfe/trunk/test/Driver/hip-toolchain-features.hip

Modified: cfe/trunk/lib/Driver/ToolChains/HIP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/HIP.cpp?rev=355106&r1=355105&r2=355106&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/HIP.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/HIP.cpp Thu Feb 28 09:08:26 2019
@@ -159,7 +159,7 @@ const char *AMDGCN::Linker::constructLlc
 llvm::StringRef OutputFilePrefix, const char *InputFileName) const {
   // Construct llc command.
   ArgStringList LlcArgs{InputFileName, "-mtriple=amdgcn-amd-amdhsa",
-"-filetype=obj",
+"-filetype=obj", "-mattr=-code-object-v3",
 Args.MakeArgString("-mcpu=" + SubArchName)};
 
   // Extract all the -m options
@@ -167,7 +167,7 @@ const char *AMDGCN::Linker::constructLlc
   handleTargetFeaturesGroup(
 Args, Features, options::OPT_m_amdgpu_Features_Group);
 
-  // Add features to mattr such as code-object-v3 and xnack
+  // Add features to mattr such as xnack
   std::string MAttrString = "-mattr=";
   for(auto OneFeature : Features) {
 MAttrString.append(Args.MakeArgString(OneFeature));

Modified: cfe/trunk/test/Driver/hip-toolchain-features.hip
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/hip-toolchain-features.hip?rev=355106&r1=355105&r2=355106&view=diff
==
--- cfe/trunk/test/Driver/hip-toolchain-features.hip (original)
+++ cfe/trunk/test/Driver/hip-toolchain-features.hip Thu Feb 28 09:08:26 2019
@@ -4,17 +4,6 @@
 
 // RUN: %clang -### -c -target x86_64-linux-gnu -fgpu-rdc \
 // RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
-// RUN:   -mcode-object-v3 2>&1 | FileCheck %s -check-prefix=COV3
-// RUN: %clang -### -c -target x86_64-linux-gnu -fgpu-rdc \
-// RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
-// RUN:   -mno-code-object-v3 2>&1 | FileCheck %s -check-prefix=NOCOV3
-
-// COV3: {{.*}}clang{{.*}}"-target-feature" "+code-object-v3"
-// NOCOV3: {{.*}}clang{{.*}}"-target-feature" "-code-object-v3"
-
-
-// RUN: %clang -### -c -target x86_64-linux-gnu -fgpu-rdc \
-// RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
 // RUN:   -mxnack 2>&1 | FileCheck %s -check-prefix=XNACK
 // RUN: %clang -### -c -target x86_64-linux-gnu -fgpu-rdc \
 // RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
@@ -37,12 +26,12 @@
 
 // RUN: %clang -### -c -target x86_64-linux-gnu -fgpu-rdc \
 // RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
-// RUN:   -mcode-object-v3 -mxnack -msram-ecc \
+// RUN:   -mxnack -msram-ecc \
 // RUN:   2>&1 | FileCheck %s -check-prefix=ALL3
 // RUN: %clang -### -c -target x86_64-linux-gnu -fgpu-rdc \
 // RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s \
-// RUN:   -mno-code-object-v3 -mno-xnack -mno-sram-ecc \
+// RUN:   -mno-xnack -mno-sram-ecc \
 // RUN:   2>&1 | FileCheck %s -check-prefix=NOALL3
 
-// ALL3: {{.*}}clang{{.*}}"-target-feature" "+code-object-v3" 
"-target-feature" "+xnack" "-target-feature" "+sram-ecc"
-// NOALL3: {{.*}}clang{{.*}}"-target-feature" "-code-object-v3" 
"-target-feature" "-xnack" "-target-feature" "-sram-ecc"
+// ALL3: {{.*}}clang{{.*}}"-target-feature" "+xnack" "-target-feature" 
"+sram-ecc"
+// NOALL3: {{.*}}clang{{.*}}"-target-feature" "-xnack" "-target-feature" 
"-sram-ecc"


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

[PATCH] D58502: [ASTImporter] Fix redecl failures of Class and ClassTemplate

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

@shafik, Could you please take a look?
I have run the LLDB tests on our macOS and I could not discover any regression.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58502



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

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Hmm.  Yeah, Embedded C allows these casts, so contra my previous comment, I 
think we can't make them ill-formed outside of OpenCL mode.


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

https://reviews.llvm.org/D58346



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