date:20200923

[PATCH] D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-09-23 Thread Jonas Toth via Phabricator via cfe-commits

JonasToth added a comment.

In D54943#2289037 , @0x8000- wrote:

> Master branch has too many false positives for tidy - would it be possible to 
> create a branch that contains this patch set on top of llvm-11.0-rc3? I would 
> then add this to our internal CI.

Yeah, it is not that simple to get this check into something really usefull :/
I can create a branch in my own fork, that would need a little work, which i 
can do in ~8 hours. I would then ping you.

> For the legacy code (see previous reports) we found quite a few false 
> positives. For a new code base that we're developing from scratch with C++20, 
> there have been no false positives - it catches missed 'const' much better 
> than human reviewers - we still keep a Clang10 around that I built from 
> source with the previous version of the patches :)

It would be great if you could keep testing your legacy code-base once this 
patch progresses even further.
It is a great message, that the modern codebase does not make big trouble :) 
Given I tested mostly on LLVM itself so far, this is not too surprising as its 
a modern codebase itself.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

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

[PATCH] D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines

2020-09-23 Thread Aleksandr Platonov via Phabricator via cfe-commits

ArcsinX updated this revision to Diff 293665.
ArcsinX added a comment.

std::pow => bitwise shift.  
Take care about integers overflow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87891

Files:
  clang-tools-extra/clangd/XRefs.cpp


Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -563,23 +563,49 @@
 assert(SM.getFileID(Loc) == File && "spelled token in wrong file?");
 unsigned Line = SM.getSpellingLineNumber(Loc);
 if (Line > WordLine)
-  return 1 + llvm::Log2_64(Line - WordLine);
+  return 1 + Line - WordLine;
 if (Line < WordLine)
-  return 2 + llvm::Log2_64(WordLine - Line);
+  return 2 + WordLine - Line;
 return 0;
   };
   const syntax::Token *BestTok = nullptr;
   // Search bounds are based on word length: 2^N lines forward.
-  unsigned BestCost = Word.Text.size() + 1;
+  unsigned BestCost = 1 << std::min(Word.Text.size(), sizeof(unsigned) * 8 - 
1);
+  auto Code = SM.getBuffer(File)->getBuffer();
+  unsigned FileLines = std::count(Code.begin(), Code.end(), '\n');
+  auto TruncUnsigned = [](unsigned U) {
+return std::min(U, std::numeric_limits::max());
+  };
+  // Min position:
+  // - Line: max(0, WordLine - 2^N)
+  // - Column: 0
+  unsigned LineMin =
+  TruncUnsigned(WordLine < BestCost ? 0 : WordLine - BestCost);
+  auto OffsetMin = positionToOffset(Code, {static_cast(LineMin), 0},
+/*AllowColumnBeyondLineLength=*/true);
+  assert(OffsetMin);
+  auto LocMin = SM.getLocForStartOfFile(File).getLocWithOffset(*OffsetMin);
+  // Max position:
+  // - Line: min(, WordLine + 2^N + 1)
+  // - Column: 0
+  unsigned LineMax = TruncUnsigned(
+  std::min(WordLine > std::numeric_limits::max() - BestCost - 1
+   ? std::numeric_limits::max()
+   : WordLine + BestCost + 1,
+   FileLines));
+  auto OffsetMax = positionToOffset(Code, {static_cast(LineMax), 0},
+/*AllowColumnBeyondLineLength=*/true);
+  assert(OffsetMax);
+  auto LocMax = SM.getLocForStartOfFile(File).getLocWithOffset(*OffsetMax);
 
   // Updates BestTok and BestCost if Tok is a good candidate.
   // May return true if the cost is too high for this token.
   auto Consider = [&](const syntax::Token &Tok) {
+if (Tok.location() < LocMin || Tok.location() > LocMax)
+  return true; // we are too far from the word, break the outer loop.
 if (!(Tok.kind() == tok::identifier && Tok.text(SM) == Word.Text))
   return false;
-// No point guessing the same location we started with.
-if (Tok.location() == Word.Location)
-  return false;
+
 // We've done cheap checks, compute cost so we can break the caller's loop.
 unsigned TokCost = Cost(Tok.location());
 if (TokCost >= BestCost)


Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -563,23 +563,49 @@
 assert(SM.getFileID(Loc) == File && "spelled token in wrong file?");
 unsigned Line = SM.getSpellingLineNumber(Loc);
 if (Line > WordLine)
-  return 1 + llvm::Log2_64(Line - WordLine);
+  return 1 + Line - WordLine;
 if (Line < WordLine)
-  return 2 + llvm::Log2_64(WordLine - Line);
+  return 2 + WordLine - Line;
 return 0;
   };
   const syntax::Token *BestTok = nullptr;
   // Search bounds are based on word length: 2^N lines forward.
-  unsigned BestCost = Word.Text.size() + 1;
+  unsigned BestCost = 1 << std::min(Word.Text.size(), sizeof(unsigned) * 8 - 1);
+  auto Code = SM.getBuffer(File)->getBuffer();
+  unsigned FileLines = std::count(Code.begin(), Code.end(), '\n');
+  auto TruncUnsigned = [](unsigned U) {
+return std::min(U, std::numeric_limits::max());
+  };
+  // Min position:
+  // - Line: max(0, WordLine - 2^N)
+  // - Column: 0
+  unsigned LineMin =
+  TruncUnsigned(WordLine < BestCost ? 0 : WordLine - BestCost);
+  auto OffsetMin = positionToOffset(Code, {static_cast(LineMin), 0},
+/*AllowColumnBeyondLineLength=*/true);
+  assert(OffsetMin);
+  auto LocMin = SM.getLocForStartOfFile(File).getLocWithOffset(*OffsetMin);
+  // Max position:
+  // - Line: min(, WordLine + 2^N + 1)
+  // - Column: 0
+  unsigned LineMax = TruncUnsigned(
+  std::min(WordLine > std::numeric_limits::max() - BestCost - 1
+   ? std::numeric_limits::max()
+   : WordLine + BestCost + 1,
+   FileLines));
+  auto OffsetMax = positionToOffset(Code, {static_cast(LineMax), 0},
+/*AllowColumnBeyondLineLength=*/true);
+  assert(OffsetMax);
+  auto LocMax = SM.getLocForStartOfFile(File).getLocWithOffset(*Offset

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

2020-09-23 Thread Russell Gallop via Phabricator via cfe-commits

russell.gallop added a comment.

In D86694#2288660 , @aganea wrote:

> I'm also in favor, I think this is good direction ahead. It'd be nice if 
> following issues were fixed -- in subsequent patches if you wish:
>
> - Stage1 `ninja check-scudo` fails many tests for me, see F13037612: 
> errors.txt .

I intend to fix these (or mark an unsupported) for this patch.

> - Stage2 `ninja check-llvm` fails after a while on high-core machines (4TB 
> issue mentionned in comments above). Lowering `AllocatorSize` to 256GB would 
> fix the issue on the short term.

I'll add that.

> - Fix & test the "exclusive" mode (or just skip to scudo-standalone if it's 
> too complicated).

I don't intend to do that for this patch, as shared mode works. I will 
re-evaluate after the above.

I also intend to remove the -fsanitize related changes. I believe that they're 
not required on Windows.

Thanks
Russ




Comment at: compiler-rt/lib/scudo/scudo_platform.h:67
 #if SANITIZER_CAN_USE_ALLOCATOR64
 # if defined(__aarch64__) && SANITIZER_ANDROID
 const uptr AllocatorSize = 0x40ULL;  // 256G.

aganea wrote:
> `&& SANITIZER_WINDOWS` ?
I think that should be:


```
# if (defined(__aarch64__) && SANITIZER_ANDROID) || SANITIZER_WINDOWS
```

I'll either do that or add a new elif.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86694

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

[PATCH] D88100: [analyzer][StdLibraryFunctionsChecker] Separate the signature from the summaries

2020-09-23 Thread Gabor Marton via Phabricator via cfe-commits

martong added a comment.

Thanks for the review! :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88100

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

[PATCH] D88092: [analyzer][StdLibraryFunctionsChecker] Fix getline/getdelim signatures

2020-09-23 Thread Gabor Marton via Phabricator via cfe-commits

martong added a comment.

In D88092#2289312 , @Szelethus wrote:

> A joy of reviewing C++ code is that you get to marvel in all the great things 
> the language has, without having to pull fistfuls of hair out to get get to 
> that point. These patches are always a treat. LGTM!

Thanks! And thanks for your time with the review! :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88092

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

[clang] d63a945 - [analyzer][StdLibraryFunctionsChecker] Fix getline/getdelim signatures

2020-09-23 Thread Gabor Marton via cfe-commits


Author: Gabor Marton
Date: 2020-09-23T10:48:14+02:00
New Revision: d63a945a13048b66f06e222d8b0810d7db9592f6

URL: 
https://github.com/llvm/llvm-project/commit/d63a945a13048b66f06e222d8b0810d7db9592f6
DIFF: 
https://github.com/llvm/llvm-project/commit/d63a945a13048b66f06e222d8b0810d7db9592f6.diff

LOG: [analyzer][StdLibraryFunctionsChecker] Fix getline/getdelim signatures

It is no longer needed to add summaries of 'getline' for different
possible underlying types of ssize_t. We can just simply lookup the
type.

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
clang/test/Analysis/std-c-library-functions.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 45711cad5633..270ee36646de 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -947,7 +947,6 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
   const QualType IntTy = ACtx.IntTy;
   const QualType UnsignedIntTy = ACtx.UnsignedIntTy;
   const QualType LongTy = ACtx.LongTy;
-  const QualType LongLongTy = ACtx.LongLongTy;
   const QualType SizeTy = ACtx.getSizeType();
 
   const QualType VoidPtrTy = getPointerTy(VoidTy); // void *
@@ -973,7 +972,6 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
   const RangeInt UnsignedIntMax =
   BVF.getMaxValue(UnsignedIntTy).getLimitedValue();
   const RangeInt LongMax = BVF.getMaxValue(LongTy).getLimitedValue();
-  const RangeInt LongLongMax = BVF.getMaxValue(LongLongTy).getLimitedValue();
   const RangeInt SizeMax = BVF.getMaxValue(SizeTy).getLimitedValue();
 
   // Set UCharRangeMax to min of int or uchar maximum value.
@@ -1076,6 +1074,12 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 return IntRangeVector{std::pair{b, *e}};
   return IntRangeVector{};
 }
+auto operator()(std::pair i0,
+std::pair> i1) {
+  if (i1.second)
+return IntRangeVector{i0, {i1.first, *(i1.second)}};
+  return IntRangeVector{i0};
+}
   } Range;
   auto SingleValue = [](RangeInt v) {
 return IntRangeVector{std::pair{v, v}};
@@ -1089,19 +1093,6 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
   Optional FilePtrTy = getPointerTy(FileTy);
   Optional FilePtrRestrictTy = getRestrictTy(FilePtrTy);
 
-  // Templates for summaries that are reused by many functions.
-  auto Read = [&](RetType R, RangeInt Max) {
-return Summary(ArgTypes{Irrelevant, Irrelevant, SizeTy}, RetType{R},
-   NoEvalCall)
-.Case({ReturnValueCondition(LessThanOrEq, ArgNo(2)),
-   ReturnValueCondition(WithinRange, Range(-1, Max))});
-  };
-  auto Getline = [&](RetType R, RangeInt Max) {
-return Summary(ArgTypes{Irrelevant, Irrelevant, Irrelevant}, RetType{R},
-   NoEvalCall)
-.Case({ReturnValueCondition(WithinRange, {{-1, -1}, {1, Max}})});
-  };
-
   // We are finally ready to define specifications for all supported functions.
   //
   // Argument ranges should always cover all variants. If return value
@@ -1296,27 +1287,52 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 RetType{SizeTy}),
   FreadSummary);
 
-  // We are not sure how ssize_t is defined on every platform, so we
-  // provide three variants that should cover common cases.
-  // FIXME Use lookupTy("ssize_t") instead of the `Read` lambda.
+  Optional Ssize_tTy = lookupTy("ssize_t");
+  Optional Ssize_tMax = getMaxValue(Ssize_tTy);
+
+  auto ReadSummary =
+  Summary(NoEvalCall)
+  .Case({ReturnValueCondition(LessThanOrEq, ArgNo(2)),
+ ReturnValueCondition(WithinRange, Range(-1, Ssize_tMax))});
+
   // FIXME these are actually defined by POSIX and not by the C standard, we
   // should handle them together with the rest of the POSIX functions.
-  addToFunctionSummaryMap("read", {Read(IntTy, IntMax), Read(LongTy, LongMax),
-   Read(LongLongTy, LongLongMax)});
-  addToFunctionSummaryMap("write", {Read(IntTy, IntMax), Read(LongTy, LongMax),
-Read(LongLongTy, LongLongMax)});
+  // ssize_t read(int fildes, void *buf, size_t nbyte);
+  addToFunctionSummaryMap(
+  "read", Signature(ArgTypes{IntTy, VoidPtrTy, SizeTy}, 
RetType{Ssize_tTy}),
+  ReadSummary);
+  // ssize_t write(int fildes, const void *buf, size_t nbyte);
+  addToFunctionSummaryMap(
+  "write",
+  Signature(ArgTypes{IntTy, ConstVoidPtrTy, SizeTy}, RetType{Ssize_tTy}),
+  ReadSummary);
+
+  auto GetLineSummary =
+  Summary(NoEvalCall)
+  .Case({ReturnValueCondition(WithinRange,
+

[PATCH] D88092: [analyzer][StdLibraryFunctionsChecker] Fix getline/getdelim signatures

2020-09-23 Thread Gabor Marton via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd63a945a1304: [analyzer][StdLibraryFunctionsChecker] Fix 
getline/getdelim signatures (authored by martong).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88092

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions.c

Index: clang/test/Analysis/std-c-library-functions.c
===
--- clang/test/Analysis/std-c-library-functions.c
+++ clang/test/Analysis/std-c-library-functions.c
@@ -57,7 +57,9 @@
 // CHECK-NEXT: Loaded summary for: unsigned int fwrite(const void *restrict, size_t, size_t, FILE *restrict)
 // CHECK-NEXT: Loaded summary for: ssize_t read(int, void *, size_t)
 // CHECK-NEXT: Loaded summary for: ssize_t write(int, const void *, size_t)
-// CHECK-NEXT: Loaded summary for: ssize_t getline(char **, size_t *, FILE *)
+// CHECK-NEXT: Loaded summary for: ssize_t getline(char **restrict, size_t *restrict, FILE *restrict)
+// CHECK-NEXT: Loaded summary for: ssize_t getdelim(char **restrict, size_t *restrict, int, FILE *restrict)
+
 
 void clang_analyzer_eval(int);
 
@@ -126,7 +128,8 @@
   (void)fread(ptr, sz, nmem, fp); // expected-warning {{1st function call argument is an uninitialized value}}
 }
 
-ssize_t getline(char **, size_t *, FILE *);
+ssize_t getline(char **restrict, size_t *restrict, FILE *restrict);
+ssize_t getdelim(char **restrict, size_t *restrict, int, FILE *restrict);
 void test_getline(FILE *fp) {
   char *line = 0;
   size_t n = 0;
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -947,7 +947,6 @@
   const QualType IntTy = ACtx.IntTy;
   const QualType UnsignedIntTy = ACtx.UnsignedIntTy;
   const QualType LongTy = ACtx.LongTy;
-  const QualType LongLongTy = ACtx.LongLongTy;
   const QualType SizeTy = ACtx.getSizeType();
 
   const QualType VoidPtrTy = getPointerTy(VoidTy); // void *
@@ -973,7 +972,6 @@
   const RangeInt UnsignedIntMax =
   BVF.getMaxValue(UnsignedIntTy).getLimitedValue();
   const RangeInt LongMax = BVF.getMaxValue(LongTy).getLimitedValue();
-  const RangeInt LongLongMax = BVF.getMaxValue(LongLongTy).getLimitedValue();
   const RangeInt SizeMax = BVF.getMaxValue(SizeTy).getLimitedValue();
 
   // Set UCharRangeMax to min of int or uchar maximum value.
@@ -1076,6 +1074,12 @@
 return IntRangeVector{std::pair{b, *e}};
   return IntRangeVector{};
 }
+auto operator()(std::pair i0,
+std::pair> i1) {
+  if (i1.second)
+return IntRangeVector{i0, {i1.first, *(i1.second)}};
+  return IntRangeVector{i0};
+}
   } Range;
   auto SingleValue = [](RangeInt v) {
 return IntRangeVector{std::pair{v, v}};
@@ -1089,19 +1093,6 @@
   Optional FilePtrTy = getPointerTy(FileTy);
   Optional FilePtrRestrictTy = getRestrictTy(FilePtrTy);
 
-  // Templates for summaries that are reused by many functions.
-  auto Read = [&](RetType R, RangeInt Max) {
-return Summary(ArgTypes{Irrelevant, Irrelevant, SizeTy}, RetType{R},
-   NoEvalCall)
-.Case({ReturnValueCondition(LessThanOrEq, ArgNo(2)),
-   ReturnValueCondition(WithinRange, Range(-1, Max))});
-  };
-  auto Getline = [&](RetType R, RangeInt Max) {
-return Summary(ArgTypes{Irrelevant, Irrelevant, Irrelevant}, RetType{R},
-   NoEvalCall)
-.Case({ReturnValueCondition(WithinRange, {{-1, -1}, {1, Max}})});
-  };
-
   // We are finally ready to define specifications for all supported functions.
   //
   // Argument ranges should always cover all variants. If return value
@@ -1296,27 +1287,52 @@
 RetType{SizeTy}),
   FreadSummary);
 
-  // We are not sure how ssize_t is defined on every platform, so we
-  // provide three variants that should cover common cases.
-  // FIXME Use lookupTy("ssize_t") instead of the `Read` lambda.
+  Optional Ssize_tTy = lookupTy("ssize_t");
+  Optional Ssize_tMax = getMaxValue(Ssize_tTy);
+
+  auto ReadSummary =
+  Summary(NoEvalCall)
+  .Case({ReturnValueCondition(LessThanOrEq, ArgNo(2)),
+ ReturnValueCondition(WithinRange, Range(-1, Ssize_tMax))});
+
   // FIXME these are actually defined by POSIX and not by the C standard, we
   // should handle them together with the rest of the POSIX functions.
-  addToFunctionSummaryMap("read", {Read(IntTy, IntMax), Read(LongTy, LongMax),
-   Read(LongLongTy, LongLongMax)});
-  addToFunctionSummaryMap("write", {Read(IntTy, IntMax), Read(LongTy, LongMax),
-Read(LongLongTy

[PATCH] D88100: [analyzer][StdLibraryFunctionsChecker] Separate the signature from the summaries

2020-09-23 Thread Gabor Marton via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG11d2e63ab006: [analyzer][StdLibraryFunctionsChecker] 
Separate the signature from the summaries (authored by martong).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88100

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -442,10 +442,6 @@
   ///   rules for the given parameter's type, those rules are checked once the
   ///   signature is matched.
   class Summary {
-// FIXME Probably the Signature should not be part of the Summary,
-// We can remove once all overload of addToFunctionSummaryMap requires the
-// Signature explicitly given.
-Optional Sign;
 const InvalidationKind InvalidationKd;
 Cases CaseConstraints;
 ConstraintSet ArgConstraints;
@@ -455,18 +451,8 @@
 const FunctionDecl *FD = nullptr;
 
   public:
-Summary(ArgTypes ArgTys, RetType RetTy, InvalidationKind InvalidationKd)
-: Sign(Signature(ArgTys, RetTy)), InvalidationKd(InvalidationKd) {}
-
 Summary(InvalidationKind InvalidationKd) : InvalidationKd(InvalidationKd) {}
 
-// FIXME Remove, once all overload of addToFunctionSummaryMap requires the
-// Signature explicitly given.
-Summary &setSignature(const Signature &S) {
-  Sign = S;
-  return *this;
-}
-
 Summary &Case(ConstraintSet &&CS) {
   CaseConstraints.push_back(std::move(CS));
   return *this;
@@ -488,10 +474,8 @@
 
 // Returns true if the summary should be applied to the given function.
 // And if yes then store the function declaration.
-bool matchesAndSet(const FunctionDecl *FD) {
-  assert(Sign &&
- "Signature must be set before comparing to a FunctionDecl");
-  bool Result = Sign->matches(FD) && validateByConstraints(FD);
+bool matchesAndSet(const Signature &Sign, const FunctionDecl *FD) {
+  bool Result = Sign.matches(FD) && validateByConstraints(FD);
   if (Result) {
 assert(!this->FD && "FD must not be set more than once");
 this->FD = FD;
@@ -499,13 +483,6 @@
   return Result;
 }
 
-// FIXME Remove, once all overload of addToFunctionSummaryMap requires the
-// Signature explicitly given.
-bool hasInvalidSignature() {
-  assert(Sign && "Signature must be set before this query");
-  return Sign->isInvalid();
-}
-
   private:
 // Once we know the exact type of the function then do sanity check on all
 // the given constraints.
@@ -1007,9 +984,8 @@
 // to the found FunctionDecl only if the signatures match.
 //
 // Returns true if the summary has been added, false otherwise.
-// FIXME remove all overloads without the explicit Signature parameter.
-bool operator()(StringRef Name, Summary S) {
-  if (S.hasInvalidSignature())
+bool operator()(StringRef Name, Signature Sign, Summary Sum) {
+  if (Sign.isInvalid())
 return false;
   IdentifierInfo &II = ACtx.Idents.get(Name);
   auto LookupRes = ACtx.getTranslationUnitDecl()->lookup(&II);
@@ -1017,8 +993,8 @@
 return false;
   for (Decl *D : LookupRes) {
 if (auto *FD = dyn_cast(D)) {
-  if (S.matchesAndSet(FD)) {
-auto Res = Map.insert({FD->getCanonicalDecl(), S});
+  if (Sum.matchesAndSet(Sign, FD)) {
+auto Res = Map.insert({FD->getCanonicalDecl(), Sum});
 assert(Res.second && "Function already has a summary set!");
 (void)Res;
 if (DisplayLoadedSummaries) {
@@ -1032,15 +1008,6 @@
   }
   return false;
 }
-// Add the summary with the Signature explicitly given.
-bool operator()(StringRef Name, Signature Sign, Summary Sum) {
-  return operator()(Name, Sum.setSignature(Sign));
-}
-// Add several summaries for the given name.
-void operator()(StringRef Name, const std::vector &Summaries) {
-  for (const Summary &S : Summaries)
-operator()(Name, S);
-}
 // Add the same summary for different names with the Signature explicitly
 // given.
 void operator()(std::vector Names, Signature Sign, Summary Sum) {
@@ -1109,8 +1076,8 @@
   // representable as unsigned char or is not equal to EOF. See e.g. C99
   // 7.4.1.2 The isalpha function (p: 181-182).
   addToFunctionSummaryMap(
-  "isalnum",
-  Summary(ArgTypes{IntTy}, RetType{IntTy}, EvalCallAsPure)
+  "isalnum", Signature(ArgTypes{IntTy}, RetType{IntTy}),
+  Summary(EvalCallAsPure)
   // Boils down to isupper() or islower() or isdigit().

[clang] 11d2e63 - [analyzer][StdLibraryFunctionsChecker] Separate the signature from the summaries

2020-09-23 Thread Gabor Marton via cfe-commits


Author: Gabor Marton
Date: 2020-09-23T10:59:34+02:00
New Revision: 11d2e63ab0060c656398afd8ea26760031a9fb96

URL: 
https://github.com/llvm/llvm-project/commit/11d2e63ab0060c656398afd8ea26760031a9fb96
DIFF: 
https://github.com/llvm/llvm-project/commit/11d2e63ab0060c656398afd8ea26760031a9fb96.diff

LOG: [analyzer][StdLibraryFunctionsChecker] Separate the signature from the 
summaries

The signature should not be part of the summaries as many FIXME comments
suggests. By separating the signature, we open up the way to a generic
matching implementation which could be used later under the hoods of
CallDescriptionMap.

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 270ee36646de..10011effe039 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -442,10 +442,6 @@ class StdLibraryFunctionsChecker
   ///   rules for the given parameter's type, those rules are checked once the
   ///   signature is matched.
   class Summary {
-// FIXME Probably the Signature should not be part of the Summary,
-// We can remove once all overload of addToFunctionSummaryMap requires the
-// Signature explicitly given.
-Optional Sign;
 const InvalidationKind InvalidationKd;
 Cases CaseConstraints;
 ConstraintSet ArgConstraints;
@@ -455,18 +451,8 @@ class StdLibraryFunctionsChecker
 const FunctionDecl *FD = nullptr;
 
   public:
-Summary(ArgTypes ArgTys, RetType RetTy, InvalidationKind InvalidationKd)
-: Sign(Signature(ArgTys, RetTy)), InvalidationKd(InvalidationKd) {}
-
 Summary(InvalidationKind InvalidationKd) : InvalidationKd(InvalidationKd) 
{}
 
-// FIXME Remove, once all overload of addToFunctionSummaryMap requires the
-// Signature explicitly given.
-Summary &setSignature(const Signature &S) {
-  Sign = S;
-  return *this;
-}
-
 Summary &Case(ConstraintSet &&CS) {
   CaseConstraints.push_back(std::move(CS));
   return *this;
@@ -488,10 +474,8 @@ class StdLibraryFunctionsChecker
 
 // Returns true if the summary should be applied to the given function.
 // And if yes then store the function declaration.
-bool matchesAndSet(const FunctionDecl *FD) {
-  assert(Sign &&
- "Signature must be set before comparing to a FunctionDecl");
-  bool Result = Sign->matches(FD) && validateByConstraints(FD);
+bool matchesAndSet(const Signature &Sign, const FunctionDecl *FD) {
+  bool Result = Sign.matches(FD) && validateByConstraints(FD);
   if (Result) {
 assert(!this->FD && "FD must not be set more than once");
 this->FD = FD;
@@ -499,13 +483,6 @@ class StdLibraryFunctionsChecker
   return Result;
 }
 
-// FIXME Remove, once all overload of addToFunctionSummaryMap requires the
-// Signature explicitly given.
-bool hasInvalidSignature() {
-  assert(Sign && "Signature must be set before this query");
-  return Sign->isInvalid();
-}
-
   private:
 // Once we know the exact type of the function then do sanity check on all
 // the given constraints.
@@ -1007,9 +984,8 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 // to the found FunctionDecl only if the signatures match.
 //
 // Returns true if the summary has been added, false otherwise.
-// FIXME remove all overloads without the explicit Signature parameter.
-bool operator()(StringRef Name, Summary S) {
-  if (S.hasInvalidSignature())
+bool operator()(StringRef Name, Signature Sign, Summary Sum) {
+  if (Sign.isInvalid())
 return false;
   IdentifierInfo &II = ACtx.Idents.get(Name);
   auto LookupRes = ACtx.getTranslationUnitDecl()->lookup(&II);
@@ -1017,8 +993,8 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
 return false;
   for (Decl *D : LookupRes) {
 if (auto *FD = dyn_cast(D)) {
-  if (S.matchesAndSet(FD)) {
-auto Res = Map.insert({FD->getCanonicalDecl(), S});
+  if (Sum.matchesAndSet(Sign, FD)) {
+auto Res = Map.insert({FD->getCanonicalDecl(), Sum});
 assert(Res.second && "Function already has a summary set!");
 (void)Res;
 if (DisplayLoadedSummaries) {
@@ -1032,15 +1008,6 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
   }
   return false;
 }
-// Add the summary with the Signature explicitly given.
-bool operator()(StringRef Name, Signature Sign, Summary Sum) {
-  return operator()(Name, Sum.setSignature(Sign));
-}
-// Add several summaries for the given nam

[PATCH] D52676: [clang-format] tweaked another case of lambda formatting

2020-09-23 Thread Marek Kurdej via Phabricator via cfe-commits

curdeius added subscribers: MyDeveloperDay, curdeius.
curdeius added a comment.

Hi,
I know it's an old revision, but I confirm that it provoked the bug 
https://bugs.llvm.org/show_bug.cgi?id=45141.
The problem is in the `TokenAnnotator::mustBreakBefore` as @jaafar pointed out:

  if (!Next->isOneOf(TT_LambdaLSquare, tok::l_brace, tok::caret))
return true; // << HERE

It should return false if everything fits on a single line.
I'm not sure how to check this though.
@MyDeveloperDay, would you have an idea?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D52676

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

[PATCH] D87394: [PowerPC][Power10] Implementation of 128-bit Binary Vector Mod and Sign Extend builtins

2020-09-23 Thread Albion Fung via Phabricator via cfe-commits

Conanap closed this revision.
Conanap added a comment.

Committed with Nemanja's comments addressed in the commit. Hash 
d7eb917a7cb793f49e16841fc24826b988dd5c8f 



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

https://reviews.llvm.org/D87394

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

[PATCH] D88140: [clang-tidy] Check for sigaction in cert-sig30-c.

2020-09-23 Thread Balázs Kéri via Phabricator via cfe-commits

balazske created this revision.
Herald added subscribers: cfe-commits, martong, gamesh411, Szelethus, dkrupp, 
xazax.hun, whisperity.
Herald added a project: clang.
balazske requested review of this revision.

The checker recognizes handlers assigned to members of a
"struct sigaction" variable. If a function is assigned to
these members it is assumable that the function is used as signal handler
(function 'sigaction' is called with it).
Without this simplification the check is possible only in path-sensitive way.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88140

Files:
  clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/signal.h
  clang-tools-extra/test/clang-tidy/checkers/cert-sig30-c.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cert-sig30-c.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cert-sig30-c.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cert-sig30-c.cpp
@@ -70,3 +70,83 @@
   // Do not find problems here.
   signal(SIGINT, handler_bad, 1);
 }
+
+void handler_sigaction1(int) {
+  printf("1234");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' is considered as non asynchronous-safe and should not be called from a signal handler [cert-sig30-c]
+}
+
+void handler_sigaction2(int) {
+  printf("1234");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' is considered as non asynchronous-safe and should not be called from a signal handler [cert-sig30-c]
+}
+
+void handler_sigaction3(int) {
+  printf("1234");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' is considered as non asynchronous-safe and should not be called from a signal handler [cert-sig30-c]
+}
+
+void handler_sigaction4(int) {
+  printf("1234");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' is considered as non asynchronous-safe and should not be called from a signal handler [cert-sig30-c]
+}
+
+void sigaction_handler1(int, siginfo_t *, void *) {
+  printf("1234");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' is considered as non asynchronous-safe and should not be called from a signal handler [cert-sig30-c]
+}
+
+void sigaction_handler2(int, siginfo_t *, void *) {
+  printf("1234");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' is considered as non asynchronous-safe and should not be called from a signal handler [cert-sig30-c]
+}
+
+void sigaction_handler3(int, siginfo_t *, void *) {
+  printf("1234");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' is considered as non asynchronous-safe and should not be called from a signal handler [cert-sig30-c]
+}
+
+void sigaction_handler4(int, siginfo_t *, void *) {
+  printf("1234");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' is considered as non asynchronous-safe and should not be called from a signal handler [cert-sig30-c]
+}
+
+void test_sigaction1() {
+  struct sigaction SA;
+  // The checker can find only assignment of the handler function into the struct sigaction.
+  // It is assumed that the assigned function is (or at least may be) used as signal handler.
+  SA.sa_handler = handler_sigaction1;
+  SA.sa_handler = SIG_IGN;
+
+  struct sigaction SA1 = {handler_sigaction2, 0, 0, 0, 0};
+}
+
+void test_sigaction2() {
+  struct sigaction SA;
+  SA.sa_sigaction = sigaction_handler1;
+
+  struct sigaction SA1 = {0, sigaction_handler2, 0, 0, 0};
+}
+
+void test_sigaction3() {
+  struct sigaction SA;
+  SA.sa_handler = handler_sigaction3;
+  SA.sa_sigaction = sigaction_handler3;
+
+  struct sigaction SA1 = {handler_sigaction4, sigaction_handler4, 0, 0};
+}
+
+void handler_sigaction5(int) {
+  printf("1234");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' is considered as non asynchronous-safe and should not be called from a signal handler [cert-sig30-c]
+}
+
+void sigaction_handler5(int, siginfo_t *, void *) {
+  printf("1234");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' is considered as non asynchronous-safe and should not be called from a signal handler [cert-sig30-c]
+}
+
+void test_pointer(struct sigaction *SA) {
+  SA->sa_handler = handler_sigaction5;
+  SA->sa_sigaction = sigaction_handler5;
+}
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/signal.h
===
--- clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/signal.h
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/signal.h
@@ -19,4 +19,18 @@
 typedef void (*sighandler_t)(int);
 sighandler_t signal(int signum, sighandler_t handler);
 
+typedef int siginfo_t;
+typedef int sigset_t;
+
+struct sigaction {
+  void (*sa_handler)(int);
+  void (*sa_sigaction)(int, siginfo_t *, void *);
+  sigset_t sa_mask;
+  int sa_flags;
+  void (*sa_restorer)(void);
+};
+
+int sigaction(int signum, const struct sigaction *act,
+  struct sigaction *oldact);
+
 #endif // _SIGNAL_H_
Index:

[PATCH] D88121: [X86] Add a memory clobber to the bittest intrinsic inline asm. Get default clobbers from the target

2020-09-23 Thread Simon Pilgrim via Phabricator via cfe-commits

RKSimon added inline comments.



Comment at: clang/test/CodeGen/bittest-intrin.c:50
 // X64: call i8 asm sideeffect "lock btrq $2, ($1)\0A\09setc ${0:b}", 
"=r,r,r,~{{.*}}"(i64* %{{.*}}, i64 {{.*}})
 // X64: call i8 asm sideeffect "lock btsq $2, ($1)\0A\09setc ${0:b}", 
"=r,r,r,~{{.*}}"(i64* %{{.*}}, i64 {{.*}})
 

Can you replace the regex with explicit checks for the clobbers here as well? 


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

https://reviews.llvm.org/D88121

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

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C.

2020-09-23 Thread Balázs Kéri via Phabricator via cfe-commits

balazske added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87449

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

[PATCH] D36836: [clang-tidy] Implement sonarsource-function-cognitive-complexity check

2020-09-23 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri updated this revision to Diff 293678.
lebedev.ri edited the summary of this revision.
lebedev.ri added a comment.

Rebased.

There is a number of official open-source LGPL-3 implementations already:

- https://github.com/SonarSource/SonarTS/pull/378
- https://github.com/SonarSource/sonar-java/pull/1385
- https://github.com/SonarSource/SonarJS/pull/449
- https://github.com/SonarSource/sonar-php/pull/173

There are other open-source LGPL-3 implementations already:

- https://pypi.org/project/cognitive-complexity/ (MIT)
- https://github.com/rossmacarthur/complexity (APACHE/MIT)

There are other 3rd party implementations:

- https://docs.codeclimate.com/docs/cognitive-complexity

Quite honestly, i do not understand how did the license question arose.
Would have it been fine if i based this on the open-source-licensed code?
Would have it not been? Would same license question be raised?
Somehow i don't think it would have been.

Is this really just about `Copyright SonarSource S.A., 2018, Switzerland. All 
content is copyright protected.` in 
https://www.sonarsource.com/docs/CognitiveComplexity.pdf ?
But that is only about the document, not the algorithm.
But even if we enternain the idea that all of the implementations must bow to 
that license,
then surely this is not the first code in LLVM that is implicitly/explicitly 
based on copyrighted doc.

This is rather frustrating.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D36836

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
  clang-tools-extra/clang-tidy/sonarsource/CMakeLists.txt
  clang-tools-extra/clang-tidy/sonarsource/FunctionCognitiveComplexityCheck.cpp
  clang-tools-extra/clang-tidy/sonarsource/FunctionCognitiveComplexityCheck.h
  clang-tools-extra/clang-tidy/sonarsource/LICENSE.TXT
  clang-tools-extra/clang-tidy/sonarsource/SONARSOURCETidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/sonarsource-function-cognitive-complexity.rst
  
clang-tools-extra/test/clang-tidy/checkers/sonarsource-function-cognitive-complexity.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/sonarsource-function-cognitive-complexity.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/sonarsource-function-cognitive-complexity.cpp
@@ -0,0 +1,956 @@
+// RUN: %check_clang_tidy %s sonarsource-function-cognitive-complexity %t -- -config='{CheckOptions: [{key: sonarsource-function-cognitive-complexity.Threshold, value: 0}]}' -- -std=c++11 -w
+
+// any function should be checked.
+
+extern int ext_func(int x = 0);
+
+int some_func(int x = 0);
+
+static int some_other_func(int x = 0) {}
+
+template void some_templ_func(T x = 0) {}
+
+class SomeClass {
+public:
+  int *begin(int x = 0);
+  int *end(int x = 0);
+  static int func(int x = 0);
+  template void some_templ_func(T x = 0) {}
+  SomeClass() = default;
+  SomeClass(SomeClass&) = delete;
+};
+
+// nothing ever decreases cognitive complexity, so we can check all the things
+// in one go. none of the following should increase cognitive complexity:
+void unittest_false() {
+  {};
+  ext_func();
+  some_func();
+  some_other_func();
+  some_templ_func();
+  some_templ_func();
+  SomeClass::func();
+  SomeClass C;
+  C.some_templ_func();
+  C.some_templ_func();
+  C.func();
+  C.end();
+  int i = some_func();
+  i = i;
+  i++;
+  --i;
+  i < 0;
+  int j = 0 ?: 1;
+  auto k = new int;
+  delete k;
+  throw i;
+  {
+throw i;
+  }
+end:
+  return;
+}
+
+#if 1
+#define CC100
+#else
+// this macro has cognitive complexity of 100.
+// it is needed to be able to compare the testcases with the
+// reference Sonar implementation. please place it right after the first
+// CHECK-NOTES in each function
+#define CC100 if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){}if(1){}
+#endif
+
+////
+//-- B1. Increments --//
+////
+// Check that every thing listed in B1 of the specification does indeed   //
+// recieve the base increment, and that not-body does not increase nesting//
+////
+
+// break does not increase cognitive complexity.
+// only  break LABEL  does, but it is unavaliable in C or C++
+
+// continue does not increase cognitive complexity.
+// only  continue LABEL  does, but it is unavaliable in C or C++
+
+void unittest_b1_00() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b1_00' has cognitive complexity of 33 (threshold 0) [sonarsource-function-cognitive

[PATCH] D83472: [SystemZ/ZOS] Add header file to encapsulate use of

2020-09-23 Thread Michał Górny via Phabricator via cfe-commits

mgorny added inline comments.



Comment at: llvm/include/llvm/Support/ExitCodes.h:19
+
+#include "llvm/Config/config.h"
+

This is a private LLVM header and it's not installed. Therefore, this installed 
header references uninstalled headers and breaks standalone builds of clang. 
Please use llvm-config.h instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83472

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

[PATCH] D88103: [JSON] Add error reporting facility, used in fromJSON and ObjectMapper.

2020-09-23 Thread Kadir Cetinkaya via Phabricator via cfe-commits

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

Thanks! This looks great. I've mostly did the full review anyway but feel free 
to land in small patches just in case some compiler becomes upset and you need 
to revert.

Mostly nits, and style related comments. LGTM!




Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:250
 ReplyOnce Reply) {
-  Param P;
-  if (fromJSON(RawParams, P)) {
-(Server.*Handler)(P, std::move(Reply));
-  } else {
-elog("Failed to decode {0} request.", Method);
-Reply(llvm::make_error("failed to decode request",
- ErrorCode::InvalidRequest));
-  }
+  if (auto P = parse(RawParams, Method, "request"))
+(Server.*Handler)(*P, std::move(Reply));

nit:
```
auto P = parse ...;
if(!P)
  return Reply(P.takeError());
(Server.*Handler)(*P, std::move(Reply));
```



Comment at: clang-tools-extra/clangd/ClangdLSPServer.h:195
+  Root.printErrorContext(Raw, OS);
+  log("{0}", OS.str());
+  // Report the error (e.g. to the client).

nit: i would merge with the previous elog, i.e. ` elog("Failed to decode {0} 
{1}:\n{2}", ... OS.str());`



Comment at: clang-tools-extra/clangd/ClangdLSPServer.h:198
+  return llvm::make_error(
+  llvm::formatv("failed to decode {0} {1}", PayloadName, PayloadKind),
+  ErrorCode::InvalidParams);

why not include `Root.err()` in the failure?



Comment at: llvm/include/llvm/Support/JSON.h:586
+return Path(this, Segment{reinterpret_cast(Field.data()),
+  unsigned(Field.size())});
+  }

static_cast



Comment at: llvm/include/llvm/Support/JSON.h:590
+private:
+  /// One element in a JSON path: an object field (.foo) or array index [27].
+  struct Segment {

`.. or a pointer to path::root in case of the "head".`

the technique is really need, but do we really need all of this compression ? 
can't we just have

```
struct Segment {
  union {
   llvm::StringRef Field;
   unsigned Index;
   Root &R;
  };
  enum { Object, Array, Head } Type;
};
```

This would be 24 bytes instead of 16, but hopefulyl will get rid of casts and 
have some extra type checking ?



Comment at: llvm/include/llvm/Support/JSON.h:605
+/// It also stores the latest reported error and the path where it occurred.
+class Path::Root {
+  llvm::StringRef Name;

i would could this `Path::Start` or `Path::Head` to emphasize the "begining of 
the linked list" bit, up to you.



Comment at: llvm/include/llvm/Support/JSON.h:610
+
+  friend Path; // for report().
+

nit: I would rather have a `public: void setError(const Path &P, 
llvm::StringLiteral Message)` but up to you



Comment at: llvm/include/llvm/Support/JSON.h:624
+
+  void printErrorContext(const Value &, llvm::raw_ostream &) const;
+};

would be nice to have some comments with example output



Comment at: llvm/lib/Support/JSON.cpp:245
+
+static std::vector sortedElements(const Object &O) 
{
+  std::vector Elements;

nit: drop static, already in anon namespace



Comment at: llvm/lib/Support/JSON.cpp:319
+  // 'Recurse' is the lambda itself, to allow recursive calls.
+  auto PrintValue = [&](const Value &V, ArrayRef Path, auto &Recurse) 
{
+auto HighlightCurrent = [&] {

nit: I believe this is big enough to be its own function, not sure what we gain 
by keeping it as a lambda.



Comment at: llvm/lib/Support/JSON.cpp:325
+  abbreviateChildren(V, JOS, OS);
+  return;
+};

nit: either drop return or `return abbreviateChildren..`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88103

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

[clang] 301e233 - [CUDA][HIP] Fix static device var used by host code only

2020-09-23 Thread Yaxun Liu via cfe-commits


Author: Yaxun (Sam) Liu
Date: 2020-09-23T08:18:19-04:00
New Revision: 301e23305d03cfb4004f845a1d9dfdc5e5931fd8

URL: 
https://github.com/llvm/llvm-project/commit/301e23305d03cfb4004f845a1d9dfdc5e5931fd8
DIFF: 
https://github.com/llvm/llvm-project/commit/301e23305d03cfb4004f845a1d9dfdc5e5931fd8.diff

LOG: [CUDA][HIP] Fix static device var used by host code only

A static device variable may be accessed in host code through
cudaMemCpyFromSymbol etc. Currently clang does not
emit the static device variable if it is only referenced by
host code, which causes host code to fail at run time.

This patch fixes that.

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

Added: 


Modified: 
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/CodeGenCUDA/static-device-var-no-rdc.cu

Removed: 




diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 3ecc8743265c..6a77f6b040a1 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -2195,6 +2195,11 @@ void CodeGenModule::EmitDeferred() {
 assert(DeferredVTables.empty());
   }
 
+  // Emit CUDA/HIP static device variables referenced by host code only.
+  if (getLangOpts().CUDA)
+for (auto V : getContext().CUDAStaticDeviceVarReferencedByHost)
+  DeferredDeclsToEmit.push_back(V);
+
   // Stop if we're out of both deferred vtables and deferred declarations.
   if (DeferredDeclsToEmit.empty())
 return;

diff  --git a/clang/test/CodeGenCUDA/static-device-var-no-rdc.cu 
b/clang/test/CodeGenCUDA/static-device-var-no-rdc.cu
index c7beb4c7e1ac..9cb1c6804a48 100644
--- a/clang/test/CodeGenCUDA/static-device-var-no-rdc.cu
+++ b/clang/test/CodeGenCUDA/static-device-var-no-rdc.cu
@@ -63,6 +63,13 @@ static constexpr int z2 = 456;
 // externalized nor registered.
 // DEV-DAG: @_ZZ6devfunPPKiE1p = linkonce_odr addrspace(4) constant i32 2, 
comdat
 
+// Check a static device variable referenced by host function only is 
externalized.
+// DEV-DAG: @_ZL1w = addrspace(1) externally_initialized global i32 0
+// HOST-DAG: @_ZL1w = internal global i32 undef
+// HOST-DAG: @[[DEVNAMEW:[0-9]+]] = {{.*}}c"_ZL1w\00"
+
+static __device__ int w;
+
 inline __device__ void devfun(const int ** b) {
   const static int p = 2;
   b[0] = &p;
@@ -92,11 +99,13 @@ void foo(const int **a) {
   getDeviceSymbol(&x);
   getDeviceSymbol(&x5);
   getDeviceSymbol(&y);
+  getDeviceSymbol(&w);
   z = 123;
   a[0] = &z2;
 }
 
 // HOST: __hipRegisterVar({{.*}}@_ZL1x {{.*}}@[[DEVNAMEX]]
 // HOST: __hipRegisterVar({{.*}}@_ZL1y {{.*}}@[[DEVNAMEY]]
+// HOST: __hipRegisterVar({{.*}}@_ZL1w {{.*}}@[[DEVNAMEW]]
 // HOST-NOT: __hipRegisterVar({{.*}}@_ZZ6kernelPiPPKiE1w
 // HOST-NOT: __hipRegisterVar({{.*}}@_ZZ6devfunPPKiE1p



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

[PATCH] D88115: [CUDA][HIP] Fix static device var used by host code only

2020-09-23 Thread Yaxun Liu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG301e23305d03: [CUDA][HIP] Fix static device var used by host 
code only (authored by yaxunl).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88115

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenCUDA/static-device-var-no-rdc.cu


Index: clang/test/CodeGenCUDA/static-device-var-no-rdc.cu
===
--- clang/test/CodeGenCUDA/static-device-var-no-rdc.cu
+++ clang/test/CodeGenCUDA/static-device-var-no-rdc.cu
@@ -63,6 +63,13 @@
 // externalized nor registered.
 // DEV-DAG: @_ZZ6devfunPPKiE1p = linkonce_odr addrspace(4) constant i32 2, 
comdat
 
+// Check a static device variable referenced by host function only is 
externalized.
+// DEV-DAG: @_ZL1w = addrspace(1) externally_initialized global i32 0
+// HOST-DAG: @_ZL1w = internal global i32 undef
+// HOST-DAG: @[[DEVNAMEW:[0-9]+]] = {{.*}}c"_ZL1w\00"
+
+static __device__ int w;
+
 inline __device__ void devfun(const int ** b) {
   const static int p = 2;
   b[0] = &p;
@@ -92,11 +99,13 @@
   getDeviceSymbol(&x);
   getDeviceSymbol(&x5);
   getDeviceSymbol(&y);
+  getDeviceSymbol(&w);
   z = 123;
   a[0] = &z2;
 }
 
 // HOST: __hipRegisterVar({{.*}}@_ZL1x {{.*}}@[[DEVNAMEX]]
 // HOST: __hipRegisterVar({{.*}}@_ZL1y {{.*}}@[[DEVNAMEY]]
+// HOST: __hipRegisterVar({{.*}}@_ZL1w {{.*}}@[[DEVNAMEW]]
 // HOST-NOT: __hipRegisterVar({{.*}}@_ZZ6kernelPiPPKiE1w
 // HOST-NOT: __hipRegisterVar({{.*}}@_ZZ6devfunPPKiE1p
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -2195,6 +2195,11 @@
 assert(DeferredVTables.empty());
   }
 
+  // Emit CUDA/HIP static device variables referenced by host code only.
+  if (getLangOpts().CUDA)
+for (auto V : getContext().CUDAStaticDeviceVarReferencedByHost)
+  DeferredDeclsToEmit.push_back(V);
+
   // Stop if we're out of both deferred vtables and deferred declarations.
   if (DeferredDeclsToEmit.empty())
 return;


Index: clang/test/CodeGenCUDA/static-device-var-no-rdc.cu
===
--- clang/test/CodeGenCUDA/static-device-var-no-rdc.cu
+++ clang/test/CodeGenCUDA/static-device-var-no-rdc.cu
@@ -63,6 +63,13 @@
 // externalized nor registered.
 // DEV-DAG: @_ZZ6devfunPPKiE1p = linkonce_odr addrspace(4) constant i32 2, comdat
 
+// Check a static device variable referenced by host function only is externalized.
+// DEV-DAG: @_ZL1w = addrspace(1) externally_initialized global i32 0
+// HOST-DAG: @_ZL1w = internal global i32 undef
+// HOST-DAG: @[[DEVNAMEW:[0-9]+]] = {{.*}}c"_ZL1w\00"
+
+static __device__ int w;
+
 inline __device__ void devfun(const int ** b) {
   const static int p = 2;
   b[0] = &p;
@@ -92,11 +99,13 @@
   getDeviceSymbol(&x);
   getDeviceSymbol(&x5);
   getDeviceSymbol(&y);
+  getDeviceSymbol(&w);
   z = 123;
   a[0] = &z2;
 }
 
 // HOST: __hipRegisterVar({{.*}}@_ZL1x {{.*}}@[[DEVNAMEX]]
 // HOST: __hipRegisterVar({{.*}}@_ZL1y {{.*}}@[[DEVNAMEY]]
+// HOST: __hipRegisterVar({{.*}}@_ZL1w {{.*}}@[[DEVNAMEW]]
 // HOST-NOT: __hipRegisterVar({{.*}}@_ZZ6kernelPiPPKiE1w
 // HOST-NOT: __hipRegisterVar({{.*}}@_ZZ6devfunPPKiE1p
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -2195,6 +2195,11 @@
 assert(DeferredVTables.empty());
   }
 
+  // Emit CUDA/HIP static device variables referenced by host code only.
+  if (getLangOpts().CUDA)
+for (auto V : getContext().CUDAStaticDeviceVarReferencedByHost)
+  DeferredDeclsToEmit.push_back(V);
+
   // Stop if we're out of both deferred vtables and deferred declarations.
   if (DeferredDeclsToEmit.empty())
 return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88144: [clangd] Disable suffix matching fallback for C during include insertion

2020-09-23 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, arphaman.
Herald added a project: clang.
kadircet requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

Clangd currently doesn't respect language and breaks the builds with
include insertion for C. This patch aims to stop the bleeding by not mapping
back to CPP standard library headers.

Improves https://github.com/clangd/clangd/issues/376.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88144

Files:
  clang-tools-extra/clangd/index/CanonicalIncludes.cpp
  clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp


Index: clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
===
--- clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
+++ clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
@@ -21,6 +21,10 @@
   CI.addSystemHeadersMapping(Language);
   // Usual standard library symbols are mapped correctly.
   EXPECT_EQ("", CI.mapHeader("path/stdio.h", "printf"));
+  // Suffix mapping isn't available for C, instead of mapping to ` we
+  // just leave the header as-is.
+  EXPECT_EQ("include/stdio.h",
+CI.mapHeader("include/stdio.h", "unknown_symbol"));
 }
 
 TEST(CanonicalIncludesTest, CXXStandardLibrary) {
Index: clang-tools-extra/clangd/index/CanonicalIncludes.cpp
===
--- clang-tools-extra/clangd/index/CanonicalIncludes.cpp
+++ clang-tools-extra/clangd/index/CanonicalIncludes.cpp
@@ -772,7 +772,10 @@
   MaxSuffixComponents;
  }) != SystemHeaderMap->keys().end());
 
-  StdSuffixHeaderMapping = SystemHeaderMap;
+  // FIXME: Suffix mapping contains invalid entries for C, so only enable it 
for
+  // CPP.
+  if (Language.CPlusPlus)
+StdSuffixHeaderMapping = SystemHeaderMap;
 }
 
 } // namespace clangd


Index: clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
===
--- clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
+++ clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
@@ -21,6 +21,10 @@
   CI.addSystemHeadersMapping(Language);
   // Usual standard library symbols are mapped correctly.
   EXPECT_EQ("", CI.mapHeader("path/stdio.h", "printf"));
+  // Suffix mapping isn't available for C, instead of mapping to ` we
+  // just leave the header as-is.
+  EXPECT_EQ("include/stdio.h",
+CI.mapHeader("include/stdio.h", "unknown_symbol"));
 }
 
 TEST(CanonicalIncludesTest, CXXStandardLibrary) {
Index: clang-tools-extra/clangd/index/CanonicalIncludes.cpp
===
--- clang-tools-extra/clangd/index/CanonicalIncludes.cpp
+++ clang-tools-extra/clangd/index/CanonicalIncludes.cpp
@@ -772,7 +772,10 @@
   MaxSuffixComponents;
  }) != SystemHeaderMap->keys().end());
 
-  StdSuffixHeaderMapping = SystemHeaderMap;
+  // FIXME: Suffix mapping contains invalid entries for C, so only enable it for
+  // CPP.
+  if (Language.CPlusPlus)
+StdSuffixHeaderMapping = SystemHeaderMap;
 }
 
 } // namespace clangd
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88146: Dummy git message. Update from phabricator.

2020-09-23 Thread Utkarsh Saxena via Phabricator via cfe-commits

usaxena95 created this revision.
Herald added subscribers: cfe-commits, kadircet, arphaman.
Herald added a project: clang.
usaxena95 requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88146

Files:
  clang-tools-extra/clangd/Quality.cpp
  clang-tools-extra/clangd/Quality.h

Index: clang-tools-extra/clangd/Quality.h
===
--- clang-tools-extra/clangd/Quality.h
+++ clang-tools-extra/clangd/Quality.h
@@ -136,6 +136,19 @@
   // Whether the item matches the type expected in the completion context.
   bool TypeMatchesPreferred = false;
 
+  /// Set of derived signals computed by calculateDerivedSignals(). Must not be
+  /// set explicitly.
+  struct DerivedSignals {
+/// Whether Name contains some word from context.
+bool NameMatchesContext = false;
+/// Min distance between SymbolURI and all the headers included by the TU.
+unsigned FileProximityDistance = FileDistance::Unreachable;
+/// Min distance between SymbolScope and all the available scopes.
+unsigned ScopeProximityDistance = FileDistance::Unreachable;
+  };
+
+  DerivedSignals calculateDerivedSignals() const;
+
   void merge(const CodeCompletionResult &SemaResult);
   void merge(const Symbol &IndexResult);
 
Index: clang-tools-extra/clangd/Quality.cpp
===
--- clang-tools-extra/clangd/Quality.cpp
+++ clang-tools-extra/clangd/Quality.cpp
@@ -320,35 +320,51 @@
   NeedsFixIts = !SemaCCResult.FixIts.empty();
 }
 
-static std::pair uriProximity(llvm::StringRef SymbolURI,
-   URIDistance *D) {
-  if (!D || SymbolURI.empty())
-return {0.f, 0u};
-  unsigned Distance = D->distance(SymbolURI);
+static float fileProximityScore(unsigned FileDistance) {
+  // Range: [0, 1]
+  // FileDistance = [0, 1, 2, 3, 4, .., FileDistance::Unreachable]
+  // Score = [1, 0.82, 0.67, 0.55, 0.45, .., 0]
+  if (FileDistance == FileDistance::Unreachable)
+return 0;
   // Assume approximately default options are used for sensible scoring.
-  return {std::exp(Distance * -0.4f / FileDistanceOptions().UpCost), Distance};
+  return std::exp(FileDistance * -0.4f / FileDistanceOptions().UpCost);
 }
 
-static float scopeBoost(ScopeDistance &Distance,
-llvm::Optional SymbolScope) {
-  if (!SymbolScope)
-return 1;
-  auto D = Distance.distance(*SymbolScope);
-  if (D == FileDistance::Unreachable)
+static float scopeProximityScore(unsigned ScopeDistance) {
+  // Range: [0.6, 2].
+  // ScopeDistance = [0, 1, 2, 3, 4, 5, 6, 7, .., FileDistance::Unreachable]
+  // Score = [2.0, 1.55, 1.2, 0.93, 0.72, 0.65, 0.65, 0.65, .., 0.6]
+  if (ScopeDistance == FileDistance::Unreachable)
 return 0.6f;
-  return std::max(0.65, 2.0 * std::pow(0.6, D / 2.0));
+  return std::max(0.65, 2.0 * std::pow(0.6, ScopeDistance / 2.0));
 }
 
 static llvm::Optional
 wordMatching(llvm::StringRef Name, const llvm::StringSet<> *ContextWords) {
   if (ContextWords)
-for (const auto& Word : ContextWords->keys())
+for (const auto &Word : ContextWords->keys())
   if (Name.contains_lower(Word))
 return Word;
   return llvm::None;
 }
 
+SymbolRelevanceSignals::DerivedSignals
+SymbolRelevanceSignals::calculateDerivedSignals() const {
+  DerivedSignals Derived;
+  Derived.NameMatchesContext = wordMatching(Name, ContextWords).hasValue();
+  Derived.FileProximityDistance = !FileProximityMatch || SymbolURI.empty()
+  ? FileDistance::Unreachable
+  : FileProximityMatch->distance(SymbolURI);
+  if (ScopeProximityMatch) {
+// For global symbol, the distance is 0.
+Derived.ScopeProximityDistance =
+SymbolScope ? ScopeProximityMatch->distance(*SymbolScope) : 0;
+  }
+  return Derived;
+}
+
 float SymbolRelevanceSignals::evaluate() const {
+  DerivedSignals Derived = calculateDerivedSignals();
   float Score = 1;
 
   if (Forbidden)
@@ -358,7 +374,7 @@
 
   // File proximity scores are [0,1] and we translate them into a multiplier in
   // the range from 1 to 3.
-  Score *= 1 + 2 * std::max(uriProximity(SymbolURI, FileProximityMatch).first,
+  Score *= 1 + 2 * std::max(fileProximityScore(Derived.FileProximityDistance),
 SemaFileProximityScore);
 
   if (ScopeProximityMatch)
@@ -366,10 +382,11 @@
 // can be tricky (e.g. class/function scope). Set to the max boost as we
 // don't load top-level symbols from the preamble and sema results are
 // always in the accessible scope.
-Score *=
-SemaSaysInScope ? 2.0 : scopeBoost(*ScopeProximityMatch, SymbolScope);
+Score *= SemaSaysInScope
+ ? 2.0
+ : scopeProximityScore(Derived.ScopeProximityDistance);
 
-  if (wordMatching(Name, ContextWords))
+  if (Derived.NameMatchesContext)
 Score *= 1.5;
 
   // Symbols like local variab

[PATCH] D79500: [clangd] Refactor code completion signal's utility properties.

2020-09-23 Thread Utkarsh Saxena via Phabricator via cfe-commits

usaxena95 updated this revision to Diff 293706.
usaxena95 marked 4 inline comments as done.
usaxena95 added a comment.

Addressed comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79500

Files:
  clang-tools-extra/clangd/Quality.cpp
  clang-tools-extra/clangd/Quality.h

Index: clang-tools-extra/clangd/Quality.h
===
--- clang-tools-extra/clangd/Quality.h
+++ clang-tools-extra/clangd/Quality.h
@@ -136,6 +136,19 @@
   // Whether the item matches the type expected in the completion context.
   bool TypeMatchesPreferred = false;
 
+  /// Set of derived signals computed by calculateDerivedSignals(). Must not be
+  /// set explicitly.
+  struct DerivedSignals {
+/// Whether Name contains some word from context.
+bool NameMatchesContext = false;
+/// Min distance between SymbolURI and all the headers included by the TU.
+unsigned FileProximityDistance = FileDistance::Unreachable;
+/// Min distance between SymbolScope and all the available scopes.
+unsigned ScopeProximityDistance = FileDistance::Unreachable;
+  };
+
+  DerivedSignals calculateDerivedSignals() const;
+
   void merge(const CodeCompletionResult &SemaResult);
   void merge(const Symbol &IndexResult);
 
Index: clang-tools-extra/clangd/Quality.cpp
===
--- clang-tools-extra/clangd/Quality.cpp
+++ clang-tools-extra/clangd/Quality.cpp
@@ -320,35 +320,51 @@
   NeedsFixIts = !SemaCCResult.FixIts.empty();
 }
 
-static std::pair uriProximity(llvm::StringRef SymbolURI,
-   URIDistance *D) {
-  if (!D || SymbolURI.empty())
-return {0.f, 0u};
-  unsigned Distance = D->distance(SymbolURI);
+static float fileProximityScore(unsigned FileDistance) {
+  // Range: [0, 1]
+  // FileDistance = [0, 1, 2, 3, 4, .., FileDistance::Unreachable]
+  // Score = [1, 0.82, 0.67, 0.55, 0.45, .., 0]
+  if (FileDistance == FileDistance::Unreachable)
+return 0;
   // Assume approximately default options are used for sensible scoring.
-  return {std::exp(Distance * -0.4f / FileDistanceOptions().UpCost), Distance};
+  return std::exp(FileDistance * -0.4f / FileDistanceOptions().UpCost);
 }
 
-static float scopeBoost(ScopeDistance &Distance,
-llvm::Optional SymbolScope) {
-  if (!SymbolScope)
-return 1;
-  auto D = Distance.distance(*SymbolScope);
-  if (D == FileDistance::Unreachable)
+static float scopeProximityScore(unsigned ScopeDistance) {
+  // Range: [0.6, 2].
+  // ScopeDistance = [0, 1, 2, 3, 4, 5, 6, 7, .., FileDistance::Unreachable]
+  // Score = [2.0, 1.55, 1.2, 0.93, 0.72, 0.65, 0.65, 0.65, .., 0.6]
+  if (ScopeDistance == FileDistance::Unreachable)
 return 0.6f;
-  return std::max(0.65, 2.0 * std::pow(0.6, D / 2.0));
+  return std::max(0.65, 2.0 * std::pow(0.6, ScopeDistance / 2.0));
 }
 
 static llvm::Optional
 wordMatching(llvm::StringRef Name, const llvm::StringSet<> *ContextWords) {
   if (ContextWords)
-for (const auto& Word : ContextWords->keys())
+for (const auto &Word : ContextWords->keys())
   if (Name.contains_lower(Word))
 return Word;
   return llvm::None;
 }
 
+SymbolRelevanceSignals::DerivedSignals
+SymbolRelevanceSignals::calculateDerivedSignals() const {
+  DerivedSignals Derived;
+  Derived.NameMatchesContext = wordMatching(Name, ContextWords).hasValue();
+  Derived.FileProximityDistance = !FileProximityMatch || SymbolURI.empty()
+  ? FileDistance::Unreachable
+  : FileProximityMatch->distance(SymbolURI);
+  if (ScopeProximityMatch) {
+// For global symbol, the distance is 0.
+Derived.ScopeProximityDistance =
+SymbolScope ? ScopeProximityMatch->distance(*SymbolScope) : 0;
+  }
+  return Derived;
+}
+
 float SymbolRelevanceSignals::evaluate() const {
+  DerivedSignals Derived = calculateDerivedSignals();
   float Score = 1;
 
   if (Forbidden)
@@ -358,7 +374,7 @@
 
   // File proximity scores are [0,1] and we translate them into a multiplier in
   // the range from 1 to 3.
-  Score *= 1 + 2 * std::max(uriProximity(SymbolURI, FileProximityMatch).first,
+  Score *= 1 + 2 * std::max(fileProximityScore(Derived.FileProximityDistance),
 SemaFileProximityScore);
 
   if (ScopeProximityMatch)
@@ -366,10 +382,11 @@
 // can be tricky (e.g. class/function scope). Set to the max boost as we
 // don't load top-level symbols from the preamble and sema results are
 // always in the accessible scope.
-Score *=
-SemaSaysInScope ? 2.0 : scopeBoost(*ScopeProximityMatch, SymbolScope);
+Score *= SemaSaysInScope
+ ? 2.0
+ : scopeProximityScore(Derived.ScopeProximityDistance);
 
-  if (wordMatching(Name, ContextWords))
+  if (Derived.NameMatchesContext)
 Score *=

[PATCH] D79500: [clangd] Refactor code completion signal's utility properties.

2020-09-23 Thread Utkarsh Saxena via Phabricator via cfe-commits

usaxena95 added inline comments.



Comment at: clang-tools-extra/clangd/Quality.h:129
 
+  // Properties and utilites used to compute derived signals. These are ignored
+  // by a scoring function. Must be explicitly assigned.

sammccall wrote:
> Why is it better to group the fields acconding to how they're used in the 
> scoring function, rather than by what they mean?
> (I find the new grouping harder to follow)
I intended to separate out the concrete signals from properties/utilities used 
to calculate other derived signals.

I agree the previous grouping made it makes it easier to follow the meaning of 
these. So reverted it.



Comment at: clang-tools-extra/clangd/Quality.h:155
+
+  void calculateDerivedSignals();
+

sammccall wrote:
> why must this be called explicitly rather than being computed by Evaluate?
Now evaluate() calls this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79500

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

[PATCH] D88088: WIP [clang] improve accuracy of ExprMutAnalyzer

2020-09-23 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/lib/Analysis/ExprMutationAnalyzer.cpp:44
+  InnerMatcher) {
+  // Unless the value is a derived class and is assigned to a
+  // reference to the base class. Other implicit casts should not

Unless the value -> Matches unless the value



Comment at: clang/lib/Analysis/ExprMutationAnalyzer.cpp:65
+// not have the 'arguments()' method.
+AST_MATCHER_P(InitListExpr, hasAnyArgumentExpr,
+  ast_matchers::internal::Matcher, InnerMatcher) {

I think I'd prefer this to be named `hasAnyInit()` to complement `hasInit()` -- 
these aren't really arguments.



Comment at: clang/lib/Analysis/ExprMutationAnalyzer.cpp:262
+  hasArgument(0, ignoringImpCasts(canResolveToExpr(equalsNode(Exp),
+  // operator call expression might be unresolved as well. If that is
+  // the case and the operator is called on the 'Exp' itself, this is

operator call expression -> The call operator expression



Comment at: clang/lib/Analysis/ExprMutationAnalyzer.cpp:264
+  // the case and the operator is called on the 'Exp' itself, this is
+  // considered a moditication.
+  cxxOperatorCallExpr(

moditication -> modification



Comment at: clang/lib/Analysis/ExprMutationAnalyzer.cpp:326
+  // If the initializer is for a reference type, there is no cast for
+  // the variable. Values are casted to RValue first.
+  initListExpr(

casted -> cast



Comment at: clang/lib/Analysis/ExprMutationAnalyzer.cpp:449
+
+  // It is possible, that containers do not provide a const-overload for their
+  // iterator accessors. If this is the case, the variable is used non-const

It is possible, that -> It is possible that



Comment at: clang/lib/Analysis/ExprMutationAnalyzer.cpp:452
+  // no matter what happens in the loop. This requires special detection as it
+  // is faster to find then all mutations of the loop variable.
+  // It aims at a different modification as well.

is faster to find then all -> is then faster to find all



Comment at: clang/lib/Analysis/ExprMutationAnalyzer.cpp:455
+  const auto HasAnyNonConstIterator =
+  anyOf(allOf(hasMethod(allOf(hasName("begin"), unless(isConst(,
+  unless(hasMethod(allOf(hasName("begin"), isConst(),

Do we want to look for methods that end with `_?[Bb]egin` or `_?[Ee]nd` so that 
this would catch patterns like `foo_begin()`/`foo_end()`, 
`FooBegin()`/`FooEnd()`, or `Foo_Begin()`/`Foo_End()`?



Comment at: clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp:65
+
+  std::string buffer;
   for (const auto *E = selectFirst("expr", Results); E != nullptr;) {

Was there a reason you hoisted this out of the `for` loop?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88088

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

[PATCH] D87652: Sema: add support for `attribute((__swift_newtype__))`

2020-09-23 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:2172
+def SwiftNewType : InheritableAttr {
+  // `swift_wrapper` is a "deprecated" alias and kept for compatibility with
+  // shipped toolchains.  New users should prefer the `swift_newtype` spelling.

Should we make these docs part of the public docs in AttrDocs.td? That's sort 
of what I had in mind with my comment (so that if someone runs into the 
attribute in the wild and wonders what it is, they have a hint).



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5964
+  if (!isa(D)) {
+S.Diag(AL.getLoc(), diag::warn_attribute_wrong_decl_type)
+<< AL << /* typedefs */13;

Rather than adding a new enumeration to `warn_attribute_wrong_decl_type`, I 
would use `warn_attribute_wrong_decl_type_str` and pass in the string as part 
of the diagnostic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87652

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

[clang] 1d1c382 - Fix typos in ASTMatchers.h; NFC

2020-09-23 Thread Aaron Ballman via cfe-commits


Author: YangZhihui
Date: 2020-09-23T09:09:11-04:00
New Revision: 1d1c382ed221f378fc866a524c7c673c239e94bc

URL: 
https://github.com/llvm/llvm-project/commit/1d1c382ed221f378fc866a524c7c673c239e94bc
DIFF: 
https://github.com/llvm/llvm-project/commit/1d1c382ed221f378fc866a524c7c673c239e94bc.diff

LOG: Fix typos in ASTMatchers.h; NFC

Added: 


Modified: 
clang/docs/LibASTMatchersReference.html
clang/include/clang/ASTMatchers/ASTMatchers.h

Removed: 




diff  --git a/clang/docs/LibASTMatchersReference.html 
b/clang/docs/LibASTMatchersReference.html
index c4c6de117c1c..fd8b217b7bc8 100644
--- a/clang/docs/LibASTMatchersReference.html
+++ b/clang/docs/LibASTMatchersReference.html
@@ -516,6 +516,16 @@ Node Matchers
 
 
 
+MatcherDecl>templateTemplateParmDeclMatcherTemplateTemplateParmDecl>...
+Matches 
template template parameter declarations.
+
+Given
+  template

1 2 >

1 - 100 of 144 matches

Mail list logo