[PATCH] D84494: [Analyzer] Use of BugType in DereferenceChecker (NFC).

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

balazske created this revision.
Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso, 
gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, 
baloghadamsoftware, xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a project: clang.

Use of BuiltinBug is replaced by BugType.
Class BuiltinBug seems to have no benefits and is confusing.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84494

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


Index: clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
@@ -30,8 +30,9 @@
 : public Checker< check::Location,
   check::Bind,
   EventDispatcher > {
-  mutable std::unique_ptr BT_null;
-  mutable std::unique_ptr BT_undef;
+  BugType BT_Null{this, "Dereference of null pointer", 
categories::MemoryError};
+  BugType BT_Undef{this, "Dereference of undefined pointer value",
+   categories::MemoryError};
 
   void reportBug(ProgramStateRef State, const Stmt *S, CheckerContext &C) 
const;
 
@@ -123,11 +124,6 @@
   if (!N)
 return;
 
-  // We know that 'location' cannot be non-null.  This is what
-  // we call an "explicit" null dereference.
-  if (!BT_null)
-BT_null.reset(new BuiltinBug(this, "Dereference of null pointer"));
-
   SmallString<100> buf;
   llvm::raw_svector_ostream os(buf);
 
@@ -180,7 +176,7 @@
   }
 
   auto report = std::make_unique(
-  *BT_null, buf.empty() ? BT_null->getDescription() : StringRef(buf), N);
+  BT_Null, buf.empty() ? BT_Null.getDescription() : StringRef(buf), N);
 
   bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), *report);
 
@@ -196,12 +192,8 @@
   // Check for dereference of an undefined value.
   if (l.isUndef()) {
 if (ExplodedNode *N = C.generateErrorNode()) {
-  if (!BT_undef)
-BT_undef.reset(
-new BuiltinBug(this, "Dereference of undefined pointer value"));
-
   auto report = std::make_unique(
-  *BT_undef, BT_undef->getDescription(), N);
+  BT_Undef, BT_Undef.getDescription(), N);
   bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), 
*report);
   C.emitReport(std::move(report));
 }
@@ -220,6 +212,8 @@
   std::tie(notNullState, nullState) = state->assume(location);
 
   // The explicit NULL case.
+  // We know that 'location' cannot be non-null.  This is what
+  // we call an "explicit" null dereference.
   if (nullState) {
 if (!notNullState) {
   const Expr *expr = getDereferenceExpr(S);


Index: clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
@@ -30,8 +30,9 @@
 : public Checker< check::Location,
   check::Bind,
   EventDispatcher > {
-  mutable std::unique_ptr BT_null;
-  mutable std::unique_ptr BT_undef;
+  BugType BT_Null{this, "Dereference of null pointer", categories::MemoryError};
+  BugType BT_Undef{this, "Dereference of undefined pointer value",
+   categories::MemoryError};
 
   void reportBug(ProgramStateRef State, const Stmt *S, CheckerContext &C) const;
 
@@ -123,11 +124,6 @@
   if (!N)
 return;
 
-  // We know that 'location' cannot be non-null.  This is what
-  // we call an "explicit" null dereference.
-  if (!BT_null)
-BT_null.reset(new BuiltinBug(this, "Dereference of null pointer"));
-
   SmallString<100> buf;
   llvm::raw_svector_ostream os(buf);
 
@@ -180,7 +176,7 @@
   }
 
   auto report = std::make_unique(
-  *BT_null, buf.empty() ? BT_null->getDescription() : StringRef(buf), N);
+  BT_Null, buf.empty() ? BT_Null.getDescription() : StringRef(buf), N);
 
   bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), *report);
 
@@ -196,12 +192,8 @@
   // Check for dereference of an undefined value.
   if (l.isUndef()) {
 if (ExplodedNode *N = C.generateErrorNode()) {
-  if (!BT_undef)
-BT_undef.reset(
-new BuiltinBug(this, "Dereference of undefined pointer value"));
-
   auto report = std::make_unique(
-  *BT_undef, BT_undef->getDescription(), N);
+  BT_Undef, BT_Undef.getDescription(), N);
   bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), *report);
   C.emitReport(std::move(report));
 }
@@ -220,6 +212,8 @@
   std::tie(notNullState, nullState) = state->assume(location);
 
   // The explicit NULL case.
+  // We know that 'location' cannot be non-null.  This is what
+  // we call an "explicit" null dereference.
   if (nullState) {
 if (!notNullState) {
   const Expr *expr = getDereferenceExpr(S);
_

[PATCH] D83972: Modify ImportDefiniton for ObjCInterfaceDecl so that we always the ImportDeclContext one we start the definition

[Gabor Marton via Phabricator via cfe-commits]

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

> Unlike RecordDecl case which uses DefinitionCompleter to force 
> completeDefinition we don't seem to have a similar mechanism for 
> ObjCInterfaceDecl.

It is unfortunate that the AST does not have something similar for ObjC 
interfaces. 
So, this seems to be the right way unless we want to modify DeclObjC.h.
LGTM!


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

https://reviews.llvm.org/D83972



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

[PATCH] D84232: [clangd] Set minimum gRPC version to 1.27

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev planned changes to this revision.
kbobyrev added a comment.

Maybe wait for Debian packages to get updated and then whitelist 1.26.0-4 but 
fail with anything lower than that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84232



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

[clang] 7f00395 - [SystemZ] Implement __builtin_eh_return_data_regno

[Ulrich Weigand via cfe-commits]

Author: Ulrich Weigand
Date: 2020-07-24T10:28:06+02:00
New Revision: 7f003957bfcd1ed29ded176f04e3cdb55a0c0112

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

LOG: [SystemZ] Implement __builtin_eh_return_data_regno

Implement __builtin_eh_return_data_regno for SystemZ.
Match behavior of GCC.

Author: slavek-kucera

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

Added: 


Modified: 
clang/lib/Basic/Targets/SystemZ.h
clang/test/CodeGen/builtins-systemz.c

Removed: 




diff  --git a/clang/lib/Basic/Targets/SystemZ.h 
b/clang/lib/Basic/Targets/SystemZ.h
index d7869e3754a8..39fdcf90d0c8 100644
--- a/clang/lib/Basic/Targets/SystemZ.h
+++ b/clang/lib/Basic/Targets/SystemZ.h
@@ -157,6 +157,10 @@ class LLVM_LIBRARY_VISIBILITY SystemZTargetInfo : public 
TargetInfo {
   const char *getLongDoubleMangling() const override { return "g"; }
 
   bool hasExtIntType() const override { return true; }
+
+  int getEHDataRegisterNumber(unsigned RegNo) const override {
+return RegNo < 4 ? 6 + RegNo : -1;
+  }
 };
 } // namespace targets
 } // namespace clang

diff  --git a/clang/test/CodeGen/builtins-systemz.c 
b/clang/test/CodeGen/builtins-systemz.c
index f5de7009acba..bf8a995383a2 100644
--- a/clang/test/CodeGen/builtins-systemz.c
+++ b/clang/test/CodeGen/builtins-systemz.c
@@ -142,3 +142,10 @@ void test_htmxl1(void) {
   result = __TM_failure_code (tdb);
 }
 
+void test_eh_return_data_regno() {
+  volatile int res;
+  res = __builtin_eh_return_data_regno(0); // CHECK: store volatile i32 6
+  res = __builtin_eh_return_data_regno(1); // CHECK: store volatile i32 7
+  res = __builtin_eh_return_data_regno(2); // CHECK: store volatile i32 8
+  res = __builtin_eh_return_data_regno(3); // CHECK: store volatile i32 9
+}



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

[PATCH] D84341: Implement __builtin_eh_return_data_regno for SystemZ

[Ulrich Weigand via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7f003957bfcd: [SystemZ] Implement 
__builtin_eh_return_data_regno (authored by uweigand).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84341

Files:
  clang/lib/Basic/Targets/SystemZ.h
  clang/test/CodeGen/builtins-systemz.c


Index: clang/test/CodeGen/builtins-systemz.c
===
--- clang/test/CodeGen/builtins-systemz.c
+++ clang/test/CodeGen/builtins-systemz.c
@@ -142,3 +142,10 @@
   result = __TM_failure_code (tdb);
 }
 
+void test_eh_return_data_regno() {
+  volatile int res;
+  res = __builtin_eh_return_data_regno(0); // CHECK: store volatile i32 6
+  res = __builtin_eh_return_data_regno(1); // CHECK: store volatile i32 7
+  res = __builtin_eh_return_data_regno(2); // CHECK: store volatile i32 8
+  res = __builtin_eh_return_data_regno(3); // CHECK: store volatile i32 9
+}
Index: clang/lib/Basic/Targets/SystemZ.h
===
--- clang/lib/Basic/Targets/SystemZ.h
+++ clang/lib/Basic/Targets/SystemZ.h
@@ -157,6 +157,10 @@
   const char *getLongDoubleMangling() const override { return "g"; }
 
   bool hasExtIntType() const override { return true; }
+
+  int getEHDataRegisterNumber(unsigned RegNo) const override {
+return RegNo < 4 ? 6 + RegNo : -1;
+  }
 };
 } // namespace targets
 } // namespace clang


Index: clang/test/CodeGen/builtins-systemz.c
===
--- clang/test/CodeGen/builtins-systemz.c
+++ clang/test/CodeGen/builtins-systemz.c
@@ -142,3 +142,10 @@
   result = __TM_failure_code (tdb);
 }
 
+void test_eh_return_data_regno() {
+  volatile int res;
+  res = __builtin_eh_return_data_regno(0); // CHECK: store volatile i32 6
+  res = __builtin_eh_return_data_regno(1); // CHECK: store volatile i32 7
+  res = __builtin_eh_return_data_regno(2); // CHECK: store volatile i32 8
+  res = __builtin_eh_return_data_regno(3); // CHECK: store volatile i32 9
+}
Index: clang/lib/Basic/Targets/SystemZ.h
===
--- clang/lib/Basic/Targets/SystemZ.h
+++ clang/lib/Basic/Targets/SystemZ.h
@@ -157,6 +157,10 @@
   const char *getLongDoubleMangling() const override { return "g"; }
 
   bool hasExtIntType() const override { return true; }
+
+  int getEHDataRegisterNumber(unsigned RegNo) const override {
+return RegNo < 4 ? 6 + RegNo : -1;
+  }
 };
 } // namespace targets
 } // namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84453: [clang-tidy] Suppress one unittest under ASan.

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

I was having issues with this test case under macos in D82188 
.
It would fail for seemingly no apparent reason until I disable a test in a 
different translation unit.
This made me think there is a subtle bug in the linker used on macos. That 
could also explain why asan is having a hard time with this as well.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D84453



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

[PATCH] D83831: [clangd] Add more logs and attach tracers to remote index server routines

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 280365.
kbobyrev added a comment.

Rebase on top of master


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

https://reviews.llvm.org/D83831

Files:
  clang-tools-extra/clangd/index/remote/Client.cpp
  clang-tools-extra/clangd/index/remote/server/Server.cpp

Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -9,6 +9,8 @@
 #include "index/Index.h"
 #include "index/Serialization.h"
 #include "index/remote/marshalling/Marshalling.h"
+#include "support/Logger.h"
+#include "support/Trace.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Path.h"
@@ -35,6 +37,16 @@
 llvm::cl::opt IndexRoot(llvm::cl::desc(""),
  llvm::cl::Positional, llvm::cl::Required);

+llvm::cl::opt TracerFile(
+"tracer-file",
+llvm::cl::desc("Path to the file where tracer logs will be stored"));
+
+llvm::cl::opt PrettyPrint{
+"pretty",
+llvm::cl::desc("Pretty-print JSON output"),
+llvm::cl::init(true),
+};
+
 llvm::cl::opt ServerAddress(
 "server-address", llvm::cl::init("0.0.0.0:50051"),
 llvm::cl::desc("Address of the invoked server. Defaults to 0.0.0.0:50051"));
@@ -59,65 +71,96 @@
   grpc::Status Lookup(grpc::ServerContext *Context,
   const LookupRequest *Request,
   grpc::ServerWriter *Reply) override {
+trace::Span Tracer("LookupRequest");
+SPAN_ATTACH(Tracer, "Request", Request->DebugString());
 clangd::LookupRequest Req;
 for (const auto &ID : Request->ids()) {
   auto SID = SymbolID::fromStr(StringRef(ID));
-  if (!SID)
+  if (!SID) {
+elog("Lookup request cancelled: invalid SymbolID {1}", SID.takeError());
 return grpc::Status::CANCELLED;
+  }
   Req.IDs.insert(*SID);
 }
+unsigned Sent = 0;
+unsigned FailedToSend = 0;
 Index->lookup(Req, [&](const clangd::Symbol &Sym) {
   auto SerializedSymbol = ProtobufMarshaller->toProtobuf(Sym);
-  if (!SerializedSymbol)
+  if (!SerializedSymbol) {
+++FailedToSend;
 return;
+  }
   LookupReply NextMessage;
   *NextMessage.mutable_stream_result() = *SerializedSymbol;
   Reply->Write(NextMessage);
+  ++Sent;
 });
 LookupReply LastMessage;
 LastMessage.set_final_result(true);
 Reply->Write(LastMessage);
+SPAN_ATTACH(Tracer, "Sent", Sent);
+SPAN_ATTACH(Tracer, "Failed to send", FailedToSend);
 return grpc::Status::OK;
   }

   grpc::Status FuzzyFind(grpc::ServerContext *Context,
  const FuzzyFindRequest *Request,
  grpc::ServerWriter *Reply) override {
+trace::Span Tracer("FuzzyFind");
+SPAN_ATTACH(Tracer, "Request", Request->DebugString());
 const auto Req = ProtobufMarshaller->fromProtobuf(Request);
+unsigned Sent = 0;
+unsigned FailedToSend = 0;
 bool HasMore = Index->fuzzyFind(Req, [&](const clangd::Symbol &Sym) {
   auto SerializedSymbol = ProtobufMarshaller->toProtobuf(Sym);
-  if (!SerializedSymbol)
+  if (!SerializedSymbol) {
+++FailedToSend;
 return;
+  }
   FuzzyFindReply NextMessage;
   *NextMessage.mutable_stream_result() = *SerializedSymbol;
   Reply->Write(NextMessage);
+  ++Sent;
 });
 FuzzyFindReply LastMessage;
 LastMessage.set_final_result(HasMore);
 Reply->Write(LastMessage);
+SPAN_ATTACH(Tracer, "Sent", Sent);
+SPAN_ATTACH(Tracer, "Failed to send", FailedToSend);
 return grpc::Status::OK;
   }

   grpc::Status Refs(grpc::ServerContext *Context, const RefsRequest *Request,
 grpc::ServerWriter *Reply) override {
+trace::Span Tracer("Refs");
+SPAN_ATTACH(Tracer, "Request", Request->DebugString());
 clangd::RefsRequest Req;
 for (const auto &ID : Request->ids()) {
   auto SID = SymbolID::fromStr(StringRef(ID));
-  if (!SID)
+  if (!SID) {
+elog("Refs request cancelled: invalid SymbolID {1}", SID.takeError());
 return grpc::Status::CANCELLED;
+  }
   Req.IDs.insert(*SID);
 }
+unsigned Sent = 0;
+unsigned FailedToSend = 0;
 bool HasMore = Index->refs(Req, [&](const clangd::Ref &Reference) {
   auto SerializedRef = ProtobufMarshaller->toProtobuf(Reference);
-  if (!SerializedRef)
+  if (!SerializedRef) {
+++FailedToSend;
 return;
+  }
   RefsReply NextMessage;
   *NextMessage.mutable_stream_result() = *SerializedRef;
   Reply->Write(NextMessage);
+  ++Sent;
 });
 RefsReply LastMessage;
 LastMessage.set_final_result(HasMore);
 Reply->Write(LastMessage);
+SPAN_ATTACH(Tracer, "Sent", Sent);
+SPAN_ATTACH(Tracer, "Failed to send", FailedToSen

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

[Gabor Marton via Phabricator via cfe-commits]

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

- Rebase to master


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79431

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
@@ -126,6 +126,8 @@
 }
 ArgNo getArgNo() const { return ArgN; }
 
+virtual StringRef getName() const = 0;
+
   protected:
 ArgNo ArgN; // Argument to which we apply the constraint.
 
@@ -144,6 +146,7 @@
 IntRangeVector Args; // Polymorphic arguments.
 
   public:
+StringRef getName() const override { return "Range"; }
 RangeConstraint(ArgNo ArgN, RangeKind Kind, const IntRangeVector &Args)
 : ValueConstraint(ArgN), Kind(Kind), Args(Args) {}
 
@@ -198,6 +201,7 @@
 ArgNo OtherArgN;
 
   public:
+virtual StringRef getName() const override { return "Comparison"; };
 ComparisonConstraint(ArgNo ArgN, BinaryOperator::Opcode Opcode,
  ArgNo OtherArgN)
 : ValueConstraint(ArgN), Opcode(Opcode), OtherArgN(OtherArgN) {}
@@ -214,6 +218,7 @@
 bool CannotBeNull = true;
 
   public:
+StringRef getName() const override { return "NonNull"; }
 ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
   const Summary &Summary,
   CheckerContext &C) const override {
@@ -259,6 +264,7 @@
 BinaryOperator::Opcode Op = BO_LE;
 
   public:
+StringRef getName() const override { return "BufferSize"; }
 BufferSizeConstraint(ArgNo Buffer, ArgNo BufSize)
 : ValueConstraint(Buffer), SizeArgN(BufSize) {}
 
@@ -413,6 +419,8 @@
   return *this;
 }
 Summary &ArgConstraint(ValueConstraintPtr VC) {
+  assert(VC->getArgNo() != Ret &&
+ "Arg constraint should not refer to the return value");
   ArgConstraints.push_back(VC);
   return *this;
 }
@@ -489,17 +497,23 @@
   void initFunctionSummaries(CheckerContext &C) const;
 
   void reportBug(const CallEvent &Call, ExplodedNode *N,
- CheckerContext &C) const {
+ const ValueConstraint *VC, CheckerContext &C) const {
 if (!ChecksEnabled[CK_StdCLibraryFunctionArgsChecker])
   return;
-// TODO Add detailed diagnostic.
-StringRef Msg = "Function argument constraint is not satisfied";
+// TODO Add more detailed diagnostic.
+std::string Msg =
+std::string("Function argument constraint is not satisfied, ") +
+VC->getName().data() + ", ArgN: " + std::to_string(VC->getArgNo());
 if (!BT_InvalidArg)
   BT_InvalidArg = std::make_unique(
   CheckNames[CK_StdCLibraryFunctionArgsChecker],
   "Unsatisfied argument constraints", categories::LogicError);
 auto R = std::make_unique(*BT_InvalidArg, Msg, N);
-bugreporter::trackExpressionValue(N, Call.getArgExpr(0), *R);
+bugreporter::trackExpressionValue(N, Call.getArgExpr(VC->getArgNo()), *R);
+
+// Highlight the range of the argument that was violated.
+R->addRange(Call.getArgSourceRange(VC->getArgNo()));
+
 C.emitReport(std::move(R));
   }
 };
@@ -632,7 +646,7 @@
 // The argument constraint is not satisfied.
 if (FailureSt && !SuccessSt) {
   if (ExplodedNode *N = C.generateErrorNode(NewState))
-reportBug(Call, N, C);
+reportBug(Call, N, Constraint.get(), C);
   break;
 } else {
   // We will apply the constraint even if we cannot reason about the


Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -126,6 +126,8 @@
 }
 ArgNo getArgNo() const { return ArgN; }
 
+virtual StringRef getName() const = 0;
+
   protected:
 ArgNo ArgN; // Argument to which we apply the constraint.
 
@@ -144,6 +146,7 @@
 IntRangeVector Args; // Polymorphic arguments.
 
   public:
+StringRef getName() const override { return "Range"; }
 RangeConstraint(ArgNo ArgN, RangeKind Kind, const IntRangeVector &Args)
 : ValueConstraint(ArgN), Kind(Kind), Args(Args) {}
 
@@ -198,6 +201,7 @@
 ArgNo OtherArgN;
 
   public:
+virtual StringRef getName() const override { return "Comparison"; };
 ComparisonConstraint(ArgNo ArgN, BinaryOperator::Opcode Opcode,
  ArgNo OtherArgN)
 : ValueConstraint(ArgN), Opcode(Opcode), OtherArgN(OtherArgN) {}
@@ -214,6 +218,7 @@
 bool CannotBeNull = true;
 
   public:
+StringRef getName() const override { return "NonNull"; }
 

[PATCH] D84499: [clangd] Add more logs and attach tracers to remote index server routines

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84499

Files:
  clang-tools-extra/clangd/index/remote/Client.cpp
  clang-tools-extra/clangd/index/remote/server/Server.cpp

Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -9,6 +9,8 @@
 #include "index/Index.h"
 #include "index/Serialization.h"
 #include "index/remote/marshalling/Marshalling.h"
+#include "support/Logger.h"
+#include "support/Trace.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Path.h"
@@ -35,6 +37,16 @@
 llvm::cl::opt IndexRoot(llvm::cl::desc(""),
  llvm::cl::Positional, llvm::cl::Required);
 
+llvm::cl::opt TracerFile(
+"tracer-file",
+llvm::cl::desc("Path to the file where tracer logs will be stored"));
+
+llvm::cl::opt PrettyPrint{
+"pretty",
+llvm::cl::desc("Pretty-print JSON output"),
+llvm::cl::init(true),
+};
+
 llvm::cl::opt ServerAddress(
 "server-address", llvm::cl::init("0.0.0.0:50051"),
 llvm::cl::desc("Address of the invoked server. Defaults to 0.0.0.0:50051"));
@@ -59,65 +71,96 @@
   grpc::Status Lookup(grpc::ServerContext *Context,
   const LookupRequest *Request,
   grpc::ServerWriter *Reply) override {
+trace::Span Tracer("LookupRequest");
+SPAN_ATTACH(Tracer, "Request", Request->DebugString());
 clangd::LookupRequest Req;
 for (const auto &ID : Request->ids()) {
   auto SID = SymbolID::fromStr(StringRef(ID));
-  if (!SID)
+  if (!SID) {
+elog("Lookup request cancelled: invalid SymbolID {1}", SID.takeError());
 return grpc::Status::CANCELLED;
+  }
   Req.IDs.insert(*SID);
 }
+unsigned Sent = 0;
+unsigned FailedToSend = 0;
 Index->lookup(Req, [&](const clangd::Symbol &Sym) {
   auto SerializedSymbol = ProtobufMarshaller->toProtobuf(Sym);
-  if (!SerializedSymbol)
+  if (!SerializedSymbol) {
+++FailedToSend;
 return;
+  }
   LookupReply NextMessage;
   *NextMessage.mutable_stream_result() = *SerializedSymbol;
   Reply->Write(NextMessage);
+  ++Sent;
 });
 LookupReply LastMessage;
 LastMessage.set_final_result(true);
 Reply->Write(LastMessage);
+SPAN_ATTACH(Tracer, "Sent", Sent);
+SPAN_ATTACH(Tracer, "Failed to send", FailedToSend);
 return grpc::Status::OK;
   }
 
   grpc::Status FuzzyFind(grpc::ServerContext *Context,
  const FuzzyFindRequest *Request,
  grpc::ServerWriter *Reply) override {
+trace::Span Tracer("FuzzyFind");
+SPAN_ATTACH(Tracer, "Request", Request->DebugString());
 const auto Req = ProtobufMarshaller->fromProtobuf(Request);
+unsigned Sent = 0;
+unsigned FailedToSend = 0;
 bool HasMore = Index->fuzzyFind(Req, [&](const clangd::Symbol &Sym) {
   auto SerializedSymbol = ProtobufMarshaller->toProtobuf(Sym);
-  if (!SerializedSymbol)
+  if (!SerializedSymbol) {
+++FailedToSend;
 return;
+  }
   FuzzyFindReply NextMessage;
   *NextMessage.mutable_stream_result() = *SerializedSymbol;
   Reply->Write(NextMessage);
+  ++Sent;
 });
 FuzzyFindReply LastMessage;
 LastMessage.set_final_result(HasMore);
 Reply->Write(LastMessage);
+SPAN_ATTACH(Tracer, "Sent", Sent);
+SPAN_ATTACH(Tracer, "Failed to send", FailedToSend);
 return grpc::Status::OK;
   }
 
   grpc::Status Refs(grpc::ServerContext *Context, const RefsRequest *Request,
 grpc::ServerWriter *Reply) override {
+trace::Span Tracer("Refs");
+SPAN_ATTACH(Tracer, "Request", Request->DebugString());
 clangd::RefsRequest Req;
 for (const auto &ID : Request->ids()) {
   auto SID = SymbolID::fromStr(StringRef(ID));
-  if (!SID)
+  if (!SID) {
+elog("Refs request cancelled: invalid SymbolID {1}", SID.takeError());
 return grpc::Status::CANCELLED;
+  }
   Req.IDs.insert(*SID);
 }
+unsigned Sent = 0;
+unsigned FailedToSend = 0;
 bool HasMore = Index->refs(Req, [&](const clangd::Ref &Reference) {
   auto SerializedRef = ProtobufMarshaller->toProtobuf(Reference);
-  if (!SerializedRef)
+  if (!SerializedRef) {
+++FailedToSend;
 return;
+  }
   RefsReply NextMessage;
   *NextMessage.mutable_stream_result() = *SerializedRef;
   Reply->Write(NextMessage);
+  ++Sent;
 });
 RefsReply LastMessage;
 LastMessage.set_final_result(HasMore);
 Reply->Write(LastMessage);
+SPAN_ATTACH(Tracer

[PATCH] D83831: [clangd] Add more logs and attach tracers to remote index server routines

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev abandoned this revision.
kbobyrev added a comment.

In favor of D84499  with clean diffs.


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

https://reviews.llvm.org/D83831



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

[PATCH] D84499: [clangd] Add more logs and attach tracers to remote index server routines

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

The first iteration is D83831  but the diffs 
are clobbered and I can't change the history there :( So, creating a new one is 
maybe the best solution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84499



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

[PATCH] D83592: [Coverage] Add comment to skipped regions

[Hans Wennborg via Phabricator via cfe-commits]

hans added inline comments.



Comment at: clang/lib/Lex/Preprocessor.cpp:973
+  if ((LexLevel == 0 || PreprocessToken) &&
+  !Result.getFlag(Token::IsReinjected)) {
+if (LexLevel == 0)

zequanwu wrote:
> @hans , can you take a look on this part? I saw `TokenCount` was introduced 
> for a warning `-Wmax-tokens`.
Can you explain why things are changing here?

The idea of TokenCount is to simply count the preprocessor tokens. At this 
point I think you have more knowledge here than I did when I added it :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83592



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

[PATCH] D82582: [SVE] Remove calls to VectorType::getNumElements from clang

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added a comment.

there's a few places the `getNumElements` calls can be fixed by getting the 
initial cast right




Comment at: clang/lib/CodeGen/CGBuiltin.cpp:5986
   case NEON::BI__builtin_neon_vqrdmulh_lane_v: {
 auto *RTy = cast(Ty);
 if (BuiltinID == NEON::BI__builtin_neon_vqdmulhq_lane_v ||

cast here



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:9872
   case NEON::BI__builtin_neon_vfma_laneq_v: {
 llvm::VectorType *VTy = cast(Ty);
 // v1f64 fma should be mapped to Neon scalar f64 fma

cast here



Comment at: clang/lib/CodeGen/CGExpr.cpp:1765
   }
   auto *VectorTy = dyn_cast(
   cast(Addr.getPointer()->getType())->getElementType());

cast here



Comment at: clang/lib/CodeGen/CGExpr.cpp:1799
   llvm::Type *SrcTy = Value->getType();
   auto *VecTy = dyn_cast(SrcTy);
   // Handle vec3 special.

cast here



Comment at: clang/lib/CodeGen/CGExpr.cpp:2214
   if (const VectorType *VTy = Dst.getType()->getAs()) {
 unsigned NumSrcElts = VTy->getNumElements();
 unsigned NumDstElts =

missed one here



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1638
 
 llvm::VectorType *MTy = cast(Mask->getType());
 

cast here



Comment at: clang/lib/CodeGen/SwiftCallingConv.cpp:321
   // If we have a vector type, split it.
   if (auto vecTy = dyn_cast_or_null(type)) {
 auto eltTy = vecTy->getElementType();

cast here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82582



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

[PATCH] D72932: [ARM] Follow AACPS standard for volatile bit-fields access width

[Diogo N. Sampaio via Phabricator via cfe-commits]

dnsampaio added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72932



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

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

[Gabor Marton via Phabricator via cfe-commits]

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

In D79431#2020951 , @Szelethus wrote:

> Sure, this is an improvement because we display more information, but I'd 
> argue that it isn't really a more readable warning message :) How about
>
> th argument to the call to 
>
> - cannot be represented with a character
> - is a null pointer
> - ... , which violates the function's preconditions.
>
>   WDYT?


I'd rather prefer @balazske's approach (see below) as that is similarly 
expressive but sparser.




Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:88
   /// obviously uint32_t should be enough for all practical purposes.
   typedef uint32_t ArgNo;
   static const ArgNo Ret;

steakhal wrote:
> Shouldn't we use `using` instead?
Yes, we should. But it's legacy code from the times long before.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:323
+std::string("Function argument constraint is not satisfied, ") +
+VC->getName().data() + ", ArgN: " + std::to_string(VC->getArgNo());
 if (!BT_InvalidArg)

steakhal wrote:
> balazske wrote:
> > baloghadamsoftware wrote:
> > > Instead of `std::string` we usually use `llvm::SmallString` and an 
> > > `llvm::raw_svector_ostream` to print into it.
> > This would look better?
> > "Function argument constraint is not satisfied, constraint: , ArgN: 
> > "
> Maybe `llvm::Twine` would be a better choice to `llvm::raw_svector_ostream` 
> for string concatenations.
> docs:
> > Twine - A lightweight data structure for efficiently representing the 
> > concatenation of temporary values as strings.
We can use a Twine to create the owning std::string without creating interim 
objects. But then we must pass an owning object to the ctor of 
PathSensitiveBugReport, otherwise we might end up with dangling pointers. 
https://llvm.org/docs/ProgrammersManual.html#dss-twine


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79431



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

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:323
+std::string("Function argument constraint is not satisfied, ") +
+VC->getName().data() + ", ArgN: " + std::to_string(VC->getArgNo());
 if (!BT_InvalidArg)

martong wrote:
> steakhal wrote:
> > balazske wrote:
> > > baloghadamsoftware wrote:
> > > > Instead of `std::string` we usually use `llvm::SmallString` and an 
> > > > `llvm::raw_svector_ostream` to print into it.
> > > This would look better?
> > > "Function argument constraint is not satisfied, constraint: , ArgN: 
> > > "
> > Maybe `llvm::Twine` would be a better choice to `llvm::raw_svector_ostream` 
> > for string concatenations.
> > docs:
> > > Twine - A lightweight data structure for efficiently representing the 
> > > concatenation of temporary values as strings.
> We can use a Twine to create the owning std::string without creating interim 
> objects. But then we must pass an owning object to the ctor of 
> PathSensitiveBugReport, otherwise we might end up with dangling pointers. 
> https://llvm.org/docs/ProgrammersManual.html#dss-twine
Yes, I like this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79431



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

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 280376.
martong marked 3 inline comments as done.
martong added a comment.

- Use Twine


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79431

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
@@ -126,6 +126,8 @@
 }
 ArgNo getArgNo() const { return ArgN; }
 
+virtual StringRef getName() const = 0;
+
   protected:
 ArgNo ArgN; // Argument to which we apply the constraint.
 
@@ -144,6 +146,7 @@
 IntRangeVector Args; // Polymorphic arguments.
 
   public:
+StringRef getName() const override { return "Range"; }
 RangeConstraint(ArgNo ArgN, RangeKind Kind, const IntRangeVector &Args)
 : ValueConstraint(ArgN), Kind(Kind), Args(Args) {}
 
@@ -198,6 +201,7 @@
 ArgNo OtherArgN;
 
   public:
+virtual StringRef getName() const override { return "Comparison"; };
 ComparisonConstraint(ArgNo ArgN, BinaryOperator::Opcode Opcode,
  ArgNo OtherArgN)
 : ValueConstraint(ArgN), Opcode(Opcode), OtherArgN(OtherArgN) {}
@@ -214,6 +218,7 @@
 bool CannotBeNull = true;
 
   public:
+StringRef getName() const override { return "NonNull"; }
 ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
   const Summary &Summary,
   CheckerContext &C) const override {
@@ -259,6 +264,7 @@
 BinaryOperator::Opcode Op = BO_LE;
 
   public:
+StringRef getName() const override { return "BufferSize"; }
 BufferSizeConstraint(ArgNo Buffer, ArgNo BufSize)
 : ValueConstraint(Buffer), SizeArgN(BufSize) {}
 
@@ -413,6 +419,8 @@
   return *this;
 }
 Summary &ArgConstraint(ValueConstraintPtr VC) {
+  assert(VC->getArgNo() != Ret &&
+ "Arg constraint should not refer to the return value");
   ArgConstraints.push_back(VC);
   return *this;
 }
@@ -489,17 +497,24 @@
   void initFunctionSummaries(CheckerContext &C) const;
 
   void reportBug(const CallEvent &Call, ExplodedNode *N,
- CheckerContext &C) const {
+ const ValueConstraint *VC, CheckerContext &C) const {
 if (!ChecksEnabled[CK_StdCLibraryFunctionArgsChecker])
   return;
-// TODO Add detailed diagnostic.
-StringRef Msg = "Function argument constraint is not satisfied";
+// TODO Add more detailed diagnostic.
+std::string Msg =
+(Twine("Function argument constraint is not satisfied, constraint: ") +
+ VC->getName().data() + ", ArgN: " + Twine(VC->getArgNo()))
+.str();
 if (!BT_InvalidArg)
   BT_InvalidArg = std::make_unique(
   CheckNames[CK_StdCLibraryFunctionArgsChecker],
   "Unsatisfied argument constraints", categories::LogicError);
 auto R = std::make_unique(*BT_InvalidArg, Msg, N);
-bugreporter::trackExpressionValue(N, Call.getArgExpr(0), *R);
+bugreporter::trackExpressionValue(N, Call.getArgExpr(VC->getArgNo()), *R);
+
+// Highlight the range of the argument that was violated.
+R->addRange(Call.getArgSourceRange(VC->getArgNo()));
+
 C.emitReport(std::move(R));
   }
 };
@@ -632,7 +647,7 @@
 // The argument constraint is not satisfied.
 if (FailureSt && !SuccessSt) {
   if (ExplodedNode *N = C.generateErrorNode(NewState))
-reportBug(Call, N, C);
+reportBug(Call, N, Constraint.get(), C);
   break;
 } else {
   // We will apply the constraint even if we cannot reason about the


Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -126,6 +126,8 @@
 }
 ArgNo getArgNo() const { return ArgN; }
 
+virtual StringRef getName() const = 0;
+
   protected:
 ArgNo ArgN; // Argument to which we apply the constraint.
 
@@ -144,6 +146,7 @@
 IntRangeVector Args; // Polymorphic arguments.
 
   public:
+StringRef getName() const override { return "Range"; }
 RangeConstraint(ArgNo ArgN, RangeKind Kind, const IntRangeVector &Args)
 : ValueConstraint(ArgN), Kind(Kind), Args(Args) {}
 
@@ -198,6 +201,7 @@
 ArgNo OtherArgN;
 
   public:
+virtual StringRef getName() const override { return "Comparison"; };
 ComparisonConstraint(ArgNo ArgN, BinaryOperator::Opcode Opcode,
  ArgNo OtherArgN)
 : ValueConstraint(ArgN), Opcode(Opcode), OtherArgN(OtherArgN) {}
@@ -214,6 +218,7 @@
 bool CannotBeNull = true;
 
   public:
+Str

[PATCH] D81385: Fix libdl linking for libclang in standalone mode

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Pushed to 11.x as 833f8c958601bb640ba6a25d627c1dc58dad14d2 
. Please 
let me know if there are any follow-ups.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81385



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

[PATCH] D84136: [clangd] Fix visitation of ConceptSpecializationExpr in constrained-parameter

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

as you mentioned in the description, there are two problems, I'd prefer address 
them in two different patches.




Comment at: clang/include/clang/AST/ExprConcepts.h:132
+// there may not be a template argument list.
+return ArgsAsWritten->RAngleLoc.isValid() ? ArgsAsWritten->RAngleLoc
+  : ConceptName.getEndLoc();

kadircet wrote:
> i think we should have some tests in clang, at least an ast-dump test in 
> `clang/test/AST/` (for both cases) and possibly also in 
> `clang/unittests/AST/SourceLocationTest.cpp`
+1, ast-dump should be enough to test the invalid end loc, I have a D84461 to 
slightly improve the dump.



Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:1843
+  if (const auto *TC = D->getTypeConstraint()) {
+TRY_TO(TraverseStmt(TC->getImmediatelyDeclaredConstraint()));
 TRY_TO(TraverseConceptReference(*TC));

Looks like we may visit some nodes in `ConceptReference` twice:
-  getImmediatelyDeclaredConstraint returns a `ConceptSpecializationExpr` (most 
cases?) which is a subclass of `ConceptReference`;
- `TraverseStmt(ConceptSpecializationExpr*)` will dispatch to 
`TraverseConceptSpecializationExpr` which invokes `TraverseConceptReference` 
(see Line 2719);


It is sad that we don't have enough test coverage, could you write some tests 
in `clang/unittests/Tooling/RecursiveASTVisitorTests/`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84136



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

[PATCH] D84499: [clangd] Add more logs and attach tracers to remote index server routines

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 280381.
kbobyrev added a comment.

Abstract out the client request into a single function call.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84499

Files:
  clang-tools-extra/clangd/index/remote/Client.cpp
  clang-tools-extra/clangd/index/remote/server/Server.cpp

Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -8,7 +8,10 @@
 
 #include "index/Index.h"
 #include "index/Serialization.h"
+#include "index/Symbol.h"
 #include "index/remote/marshalling/Marshalling.h"
+#include "support/Logger.h"
+#include "support/Trace.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Path.h"
@@ -16,6 +19,7 @@
 
 #include 
 #include 
+#include 
 
 #include "Index.grpc.pb.h"
 
@@ -35,6 +39,16 @@
 llvm::cl::opt IndexRoot(llvm::cl::desc(""),
  llvm::cl::Positional, llvm::cl::Required);
 
+llvm::cl::opt TracerFile(
+"tracer-file",
+llvm::cl::desc("Path to the file where tracer logs will be stored"));
+
+llvm::cl::opt PrettyPrint{
+"pretty",
+llvm::cl::desc("Pretty-print JSON output"),
+llvm::cl::init(true),
+};
+
 llvm::cl::opt ServerAddress(
 "server-address", llvm::cl::init("0.0.0.0:50051"),
 llvm::cl::desc("Address of the invoked server. Defaults to 0.0.0.0:50051"));
@@ -56,24 +70,52 @@
   }
 
 private:
+  template 
+  typename std::result_of::type
+  streamRPC(const RequestT *Request, grpc::ServerWriter *Reply,
+ClangdRequestT ClangdRequest, IndexCall &&Call) {
+trace::Span Tracer(RequestT::descriptor()->name());
+SPAN_ATTACH(Tracer, "Request", Request->DebugString());
+unsigned Sent = 0;
+unsigned FailedToSend = 0;
+SPAN_ATTACH(Tracer, "Sent", Sent);
+SPAN_ATTACH(Tracer, "Failed to send", FailedToSend);
+return std::forward(Call)(
+*Index, ClangdRequest, [&](const StreamType &Item) {
+  auto SerializedItem = ProtobufMarshaller->toProtobuf(Item);
+  if (!SerializedItem) {
+++FailedToSend;
+return;
+  }
+  ReplyT NextMessage;
+  *NextMessage.mutable_stream_result() = *SerializedItem;
+  Reply->Write(NextMessage);
+  ++Sent;
+});
+  }
+
   grpc::Status Lookup(grpc::ServerContext *Context,
   const LookupRequest *Request,
   grpc::ServerWriter *Reply) override {
 clangd::LookupRequest Req;
 for (const auto &ID : Request->ids()) {
   auto SID = SymbolID::fromStr(StringRef(ID));
-  if (!SID)
+  if (!SID) {
+elog("Lookup request cancelled: invalid SymbolID {1}", SID.takeError());
 return grpc::Status::CANCELLED;
+  }
   Req.IDs.insert(*SID);
 }
-Index->lookup(Req, [&](const clangd::Symbol &Sym) {
-  auto SerializedSymbol = ProtobufMarshaller->toProtobuf(Sym);
-  if (!SerializedSymbol)
-return;
-  LookupReply NextMessage;
-  *NextMessage.mutable_stream_result() = *SerializedSymbol;
-  Reply->Write(NextMessage);
-});
+std::function)>
+IndexCall = &clangd::SymbolIndex::lookup;
+streamRPC>(
+Request, Reply, Req, std::move(IndexCall));
 LookupReply LastMessage;
 LastMessage.set_final_result(true);
 Reply->Write(LastMessage);
@@ -83,15 +125,18 @@
   grpc::Status FuzzyFind(grpc::ServerContext *Context,
  const FuzzyFindRequest *Request,
  grpc::ServerWriter *Reply) override {
+trace::Span Tracer("FuzzyFind");
+SPAN_ATTACH(Tracer, "Request", Request->DebugString());
 const auto Req = ProtobufMarshaller->fromProtobuf(Request);
-bool HasMore = Index->fuzzyFind(Req, [&](const clangd::Symbol &Sym) {
-  auto SerializedSymbol = ProtobufMarshaller->toProtobuf(Sym);
-  if (!SerializedSymbol)
-return;
-  FuzzyFindReply NextMessage;
-  *NextMessage.mutable_stream_result() = *SerializedSymbol;
-  Reply->Write(NextMessage);
-});
+std::function)>
+IndexCall = &clangd::SymbolIndex::fuzzyFind;
+bool HasMore =
+streamRPC>(
+Request, Reply, Req, std::move(IndexCall));
 FuzzyFindReply LastMessage;
 LastMessage.set_final_result(HasMore);
 Reply->Write(LastMessage);
@@ -103,18 +148,20 @@
 clangd::RefsRequest Req;
 for (const auto &ID : Request->ids()) {
   auto SID = SymbolID::fromStr(StringRef(ID));
-  if (!SID)
+  if (!SID) {
+elog("Refs request cancelled: invalid SymbolID {1}", SID.takeError());
 return grpc::Status::CANCELLED;
+  }
   Req.IDs.insert(*SID);
 }
-bool HasMore = Index->refs(Req, [&](const clangd::Ref &Reference) {
-  au

[PATCH] D83831: [clangd] Add more logs and attach tracers to remote index server routines

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 280383.
kbobyrev added a comment.

Reset D83831  to master.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83831

Files:
  clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
  clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp


Index: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
===
--- clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
+++ clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
@@ -282,10 +282,17 @@
 
 TEST(RemoteMarshallingTest, FuzzyFindRequestSerialization) {
   clangd::FuzzyFindRequest Request;
+<<< HEAD
   Request.ProximityPaths = {testPath("local/Header.h"),
 testPath("local/subdir/OtherHeader.h"),
 testPath("remote/File.h"), "Not a Path."};
   Marshaller ProtobufMarshaller(testPath("remote/"), testPath("local/"));
+===
+  Request.ProximityPaths = {testPath("remote/Header.h"),
+testPath("remote/subdir/OtherHeader.h"),
+testPath("notremote/File.h"), "Not a Path."};
+  Marshaller ProtobufMarshaller(testPath("remote/"), testPath("home/"));
+>>> 529b83e6cf3... [clangd] Don't send invalid messages from remote index
   auto Serialized = ProtobufMarshaller.toProtobuf(Request);
   EXPECT_EQ(Serialized.proximity_paths_size(), 2);
   auto Deserialized = ProtobufMarshaller.fromProtobuf(&Serialized);
Index: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
===
--- clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -51,7 +51,11 @@
 
 clangd::FuzzyFindRequest
 Marshaller::fromProtobuf(const FuzzyFindRequest *Request) {
+<<< HEAD
   assert(RemoteIndexRoot);
+===
+  assert(LocalIndexRoot);
+>>> 529b83e6cf3... [clangd] Don't send invalid messages from remote index
   clangd::FuzzyFindRequest Result;
   Result.Query = Request->query();
   for (const auto &Scope : Request->scopes())
@@ -61,7 +65,11 @@
 Result.Limit = Request->limit();
   Result.RestrictForCodeCompletion = Request->restricted_for_code_completion();
   for (const auto &Path : Request->proximity_paths()) {
+<<< HEAD
 llvm::SmallString<256> LocalPath = llvm::StringRef(*RemoteIndexRoot);
+===
+llvm::SmallString<256> LocalPath = llvm::StringRef(*LocalIndexRoot);
+>>> 529b83e6cf3... [clangd] Don't send invalid messages from remote index
 llvm::sys::path::append(LocalPath, Path);
 Result.ProximityPaths.push_back(std::string(LocalPath));
   }
@@ -146,7 +154,11 @@
 }
 
 FuzzyFindRequest Marshaller::toProtobuf(const clangd::FuzzyFindRequest &From) {
+<<< HEAD
   assert(LocalIndexRoot);
+===
+  assert(RemoteIndexRoot);
+>>> 529b83e6cf3... [clangd] Don't send invalid messages from remote index
   FuzzyFindRequest RPCRequest;
   RPCRequest.set_query(From.Query);
   for (const auto &Scope : From.Scopes)
@@ -157,7 +169,11 @@
   
RPCRequest.set_restricted_for_code_completion(From.RestrictForCodeCompletion);
   for (const auto &Path : From.ProximityPaths) {
 llvm::SmallString<256> RelativePath = llvm::StringRef(Path);
+<<< HEAD
 if (llvm::sys::path::replace_path_prefix(RelativePath, *LocalIndexRoot,
+===
+if (llvm::sys::path::replace_path_prefix(RelativePath, *RemoteIndexRoot,
+>>> 529b83e6cf3... [clangd] Don't send invalid messages from remote index
  ""))
   RPCRequest.add_proximity_paths(llvm::sys::path::convert_to_slash(
   RelativePath, llvm::sys::path::Style::posix));


Index: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
===
--- clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
+++ clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
@@ -282,10 +282,17 @@
 
 TEST(RemoteMarshallingTest, FuzzyFindRequestSerialization) {
   clangd::FuzzyFindRequest Request;
+<<< HEAD
   Request.ProximityPaths = {testPath("local/Header.h"),
 testPath("local/subdir/OtherHeader.h"),
 testPath("remote/File.h"), "Not a Path."};
   Marshaller ProtobufMarshaller(testPath("remote/"), testPath("local/"));
+===
+  Request.ProximityPaths = {testPath("remote/Header.h"),
+testPath("remote/subdir/OtherHeader.h"),
+testPath("notremote/File.h"), "Not a Path."};
+  Marshaller ProtobufMarshaller(testPath("remote/"), testPath("home/"));
+>>> 529b83e6cf3... [clangd] Don't send invalid messages from remote index
   auto Serialized = ProtobufMarshaller.toProtobu

[PATCH] D84494: [Analyzer] Use of BugType in DereferenceChecker (NFC).

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

balazske updated this revision to Diff 280385.
balazske added a comment.

Fixed failed tests because change of bug category from "Logic error" to "Memory 
error".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84494

Files:
  clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
  clang/test/Analysis/Inputs/expected-plists/conditional-path-notes.c.plist
  clang/test/Analysis/Inputs/expected-plists/cxx-for-range.cpp.plist
  clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
  clang/test/Analysis/Inputs/expected-plists/inline-plist.c.plist
  clang/test/Analysis/Inputs/expected-plists/inline-unique-reports.c.plist
  clang/test/Analysis/Inputs/expected-plists/null-deref-path-notes.m.plist
  clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
  
clang/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
  clang/test/Analysis/Inputs/expected-plists/plist-macros.cpp.plist
  clang/test/Analysis/Inputs/expected-plists/plist-output-alternate.m.plist
  clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist
  clang/test/Analysis/Inputs/expected-plists/retain-release.m.objc.plist
  clang/test/Analysis/Inputs/expected-plists/retain-release.m.objcpp.plist
  clang/test/Analysis/Inputs/expected-plists/unix-fns.c.plist
  
clang/test/Analysis/diagnostics/Inputs/expected-plists/deref-track-symbolic-region.c.plist
  
clang/test/Analysis/diagnostics/Inputs/expected-plists/plist-multi-file.c.plist
  
clang/test/Analysis/diagnostics/Inputs/expected-plists/undef-value-param.c.plist
  
clang/test/Analysis/inlining/Inputs/expected-plists/eager-reclamation-path-notes.c.plist
  clang/test/Analysis/inlining/Inputs/expected-plists/path-notes.c.plist
  clang/test/Analysis/inlining/Inputs/expected-plists/path-notes.cpp.plist
  clang/test/Analysis/inlining/Inputs/expected-plists/path-notes.m.plist

Index: clang/test/Analysis/inlining/Inputs/expected-plists/path-notes.m.plist
===
--- clang/test/Analysis/inlining/Inputs/expected-plists/path-notes.m.plist
+++ clang/test/Analysis/inlining/Inputs/expected-plists/path-notes.m.plist
@@ -334,7 +334,7 @@
 

descriptionDereference of null pointer
-   categoryLogic error
+   categoryMemory error
typeDereference of null pointer
check_namecore.NullDereference

@@ -1506,7 +1506,7 @@
 

descriptionDereference of null pointer (loaded from variable 'x')
-   categoryLogic error
+   categoryMemory error
typeDereference of null pointer
check_namecore.NullDereference

Index: clang/test/Analysis/inlining/Inputs/expected-plists/path-notes.cpp.plist
===
--- clang/test/Analysis/inlining/Inputs/expected-plists/path-notes.cpp.plist
+++ clang/test/Analysis/inlining/Inputs/expected-plists/path-notes.cpp.plist
@@ -478,7 +478,7 @@
 

descriptionDereference of null pointer (loaded from variable 'p')
-   categoryLogic error
+   categoryMemory error
typeDereference of null pointer
check_namecore.NullDereference

@@ -718,7 +718,7 @@
 

descriptionDereference of null pointer (loaded from variable 'p')
-   categoryLogic error
+   categoryMemory error
typeDereference of null pointer
check_namecore.NullDereference

@@ -1050,7 +1050,7 @@
 

descriptionDereference of null pointer (loaded from variable 'globalPtr')
-   categoryLogic error
+   categoryMemory error
typeDereference of null pointer
check_namecore.NullDereference

@@ -1380,7 +1380,7 @@
 

descriptionDereference of null pointer (loaded from variable 'globalPtr')
-   categoryLogic error
+   categoryMemory error
typeDereference of null pointer
check_namecore.NullDereference

@@ -1710,7 +1710,7 @@
 

descriptionDereference of null pointer (loaded from variable 'globalPtr')
-   categoryLogic error
+   categoryMemory error
typeDereference of null pointer
check_namecore.NullDereference

@@ -2042,7 +2042,7 @@
 

descriptionDereference of null pointer (loaded from variable 'globalPtr')
-   categoryLogic error
+   categoryMemory error
typeDereference of null pointer
check_namecore.NullDereference

@@ -2377,7 +2377,7 @@
 

descriptionDereference of null pointer (loaded from variable 'globalPtr')
-   categoryLogic error
+   categoryMemory error
typeDereference of null pointer
check_namecore.NullDereference

@@ -2685,7 +2685,7 @@
 

descriptionDereference of null pointer (loaded from variable 'globalPtr')
-   categoryLogic error
+   categoryMemory error
typeDereference of null pointer
check_namecore.NullDereference

@@ -3810,7 +3810,7 @@
 

descriptionDereference of null pointer (loaded from field 'ptr')
-   categoryLogic error
+   categoryMemory error
typeDereference 

[PATCH] D84499: [clangd] Add more logs and attach tracers to remote index server routines

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev planned changes to this revision.
kbobyrev added a comment.

Tracer messages for sent and failed to parse items are not correct. Have to 
figure out how to print them _after_ the callback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84499



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

[PATCH] D84499: [clangd] Add more logs and attach tracers to remote index server routines

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 280391.
kbobyrev added a comment.

Fix (un)successful message numbers reporting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84499

Files:
  clang-tools-extra/clangd/index/remote/Client.cpp
  clang-tools-extra/clangd/index/remote/server/Server.cpp

Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -8,7 +8,10 @@
 
 #include "index/Index.h"
 #include "index/Serialization.h"
+#include "index/Symbol.h"
 #include "index/remote/marshalling/Marshalling.h"
+#include "support/Logger.h"
+#include "support/Trace.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Path.h"
@@ -16,6 +19,7 @@
 
 #include 
 #include 
+#include 
 
 #include "Index.grpc.pb.h"
 
@@ -35,6 +39,16 @@
 llvm::cl::opt IndexRoot(llvm::cl::desc(""),
  llvm::cl::Positional, llvm::cl::Required);
 
+llvm::cl::opt TracerFile(
+"tracer-file",
+llvm::cl::desc("Path to the file where tracer logs will be stored"));
+
+llvm::cl::opt PrettyPrint{
+"pretty",
+llvm::cl::desc("Pretty-print JSON output"),
+llvm::cl::init(true),
+};
+
 llvm::cl::opt ServerAddress(
 "server-address", llvm::cl::init("0.0.0.0:50051"),
 llvm::cl::desc("Address of the invoked server. Defaults to 0.0.0.0:50051"));
@@ -56,24 +70,63 @@
   }
 
 private:
+  template 
+  typename std::result_of::type
+  streamRPC(const RequestT *Request, grpc::ServerWriter *Reply,
+ClangdRequestT ClangdRequest, IndexCall &&Call) {
+// Adding Sent and FailedToSend messages to the tracer has to happen after
+// Callback but the temporary value can not be stored. Use a dummy struct
+// to hold data for tracer and then add data to the trace on destruction
+// happening after the callback.
+struct TracerCounter {
+  trace::Span Tracer;
+  unsigned Sent = 0;
+  unsigned FailedToSend = 0;
+  TracerCounter(llvm::StringRef RequestInfo)
+  : Tracer(RequestT::descriptor()->name()) {
+SPAN_ATTACH(Tracer, "Request", RequestInfo);
+  }
+  ~TracerCounter() {
+SPAN_ATTACH(Tracer, "Sent", Sent);
+SPAN_ATTACH(Tracer, "Failed to send", FailedToSend);
+  }
+} Counter(Request->DebugString());
+return std::forward(Call)(
+*Index, ClangdRequest, [&](const StreamType &Item) {
+  auto SerializedItem = ProtobufMarshaller->toProtobuf(Item);
+  if (!SerializedItem) {
+++Counter.FailedToSend;
+return;
+  }
+  ReplyT NextMessage;
+  *NextMessage.mutable_stream_result() = *SerializedItem;
+  Reply->Write(NextMessage);
+  ++Counter.Sent;
+});
+  }
+
   grpc::Status Lookup(grpc::ServerContext *Context,
   const LookupRequest *Request,
   grpc::ServerWriter *Reply) override {
 clangd::LookupRequest Req;
 for (const auto &ID : Request->ids()) {
   auto SID = SymbolID::fromStr(StringRef(ID));
-  if (!SID)
+  if (!SID) {
+elog("Lookup request cancelled: invalid SymbolID {1}", SID.takeError());
 return grpc::Status::CANCELLED;
+  }
   Req.IDs.insert(*SID);
 }
-Index->lookup(Req, [&](const clangd::Symbol &Sym) {
-  auto SerializedSymbol = ProtobufMarshaller->toProtobuf(Sym);
-  if (!SerializedSymbol)
-return;
-  LookupReply NextMessage;
-  *NextMessage.mutable_stream_result() = *SerializedSymbol;
-  Reply->Write(NextMessage);
-});
+std::function)>
+IndexCall = &clangd::SymbolIndex::lookup;
+streamRPC>(
+Request, Reply, Req, std::move(IndexCall));
 LookupReply LastMessage;
 LastMessage.set_final_result(true);
 Reply->Write(LastMessage);
@@ -83,15 +136,18 @@
   grpc::Status FuzzyFind(grpc::ServerContext *Context,
  const FuzzyFindRequest *Request,
  grpc::ServerWriter *Reply) override {
+trace::Span Tracer("FuzzyFind");
+SPAN_ATTACH(Tracer, "Request", Request->DebugString());
 const auto Req = ProtobufMarshaller->fromProtobuf(Request);
-bool HasMore = Index->fuzzyFind(Req, [&](const clangd::Symbol &Sym) {
-  auto SerializedSymbol = ProtobufMarshaller->toProtobuf(Sym);
-  if (!SerializedSymbol)
-return;
-  FuzzyFindReply NextMessage;
-  *NextMessage.mutable_stream_result() = *SerializedSymbol;
-  Reply->Write(NextMessage);
-});
+std::function)>
+IndexCall = &clangd::SymbolIndex::fuzzyFind;
+bool HasMore =
+streamRPC>(
+Request, Reply, Req, std::move(IndexCall));
 FuzzyFindReply LastMessage;
 LastMessage.set_final_result(HasMore)

[PATCH] D84508: [clang-tools-extra] Change the default path to find-all-symbols

[Gaël Écorchard via Phabricator via cfe-commits]

galou created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Change the default parameter `binary` in run-find-all-symbols.py to 
`find-all-symbols`, so that the script works out-of-the box when installed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84508

Files:
  
clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py


Index: 
clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
===
--- 
clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
+++ 
clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
@@ -66,7 +66,7 @@
   parser = argparse.ArgumentParser(description='Runs find-all-symbols over all'
'files in a compilation database.')
   parser.add_argument('-binary', metavar='PATH',
-  default='./bin/find-all-symbols',
+  default='find-all-symbols',
   help='path to find-all-symbols binary')
   parser.add_argument('-j', type=int, default=0,
   help='number of instances to be run in parallel.')


Index: clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
===
--- clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
+++ clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
@@ -66,7 +66,7 @@
   parser = argparse.ArgumentParser(description='Runs find-all-symbols over all'
'files in a compilation database.')
   parser.add_argument('-binary', metavar='PATH',
-  default='./bin/find-all-symbols',
+  default='find-all-symbols',
   help='path to find-all-symbols binary')
   parser.add_argument('-j', type=int, default=0,
   help='number of instances to be run in parallel.')
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84508: [clang-tools-extra] Change the default path to find-all-symbols

[Gaël Écorchard via Phabricator via cfe-commits]

galou added a comment.

This is necessary at least on Ubuntu 18.04.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84508



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

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

[Petr Hosek via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG10b1b4a231a4: [CMake] Simplify CMake handling for zlib 
(authored by phosek).
Herald added subscribers: msifontes, jurahul, Kayjukh, grosul1, Joonsoo, 
stephenneuendorffer, liufengdb, aartbik, lucyrfox, mgester, arpith-jacob, 
nicolasvasilache, antiagainst, shauheen, jpienaar, rriddle, mehdi_amini.
Herald added a project: MLIR.

Changed prior to commit:
  https://reviews.llvm.org/D79219?vs=280305&id=280331#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

Files:
  clang/CMakeLists.txt
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/test/lit.common.configured.in
  lld/CMakeLists.txt
  lld/test/CMakeLists.txt
  lld/test/lit.site.cfg.py.in
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/cmake/config-ix.cmake
  llvm/include/llvm/Config/config.h.cmake
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CRC.cpp
  llvm/lib/Support/Compression.cpp
  llvm/test/CMakeLists.txt
  llvm/test/lit.site.cfg.py.in
  llvm/unittests/Support/CompressionTest.cpp
  mlir/examples/standalone/CMakeLists.txt

Index: mlir/examples/standalone/CMakeLists.txt
===
--- mlir/examples/standalone/CMakeLists.txt
+++ mlir/examples/standalone/CMakeLists.txt
@@ -28,6 +28,11 @@
 
 list(APPEND CMAKE_MODULE_PATH "${MLIR_CMAKE_DIR}")
 list(APPEND CMAKE_MODULE_PATH "${LLVM_CMAKE_DIR}")
+
+if(LLVM_ENABLE_ZLIB)
+  find_package(ZLIB)
+endif()
+
 include(TableGen)
 include(AddLLVM)
 include(AddMLIR)
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 
 void TestZlibCompression(StringRef Input, int Level) {
   SmallString<32> Compressed;
Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -33,7 +33,7 @@
 config.host_ldflags = '@HOST_LDFLAGS@'
 config.llvm_use_intel_jitevents = @LLVM_USE_INTEL_JITEVENTS@
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @HAVE_LIBZ@
+config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.have_libxar = @HAVE_LIBXAR@
 config.have_dia_sdk = @LLVM_ENABLE_DIA_SDK@
 config.enable_ffi = @LLVM_ENABLE_FFI@
Index: llvm/test/CMakeLists.txt
===
--- llvm/test/CMakeLists.txt
+++ llvm/test/CMakeLists.txt
@@ -1,12 +1,12 @@
 llvm_canonicalize_cmake_booleans(
   BUILD_SHARED_LIBS
   HAVE_LIBXAR
-  HAVE_LIBZ
   HAVE_OCAMLOPT
   HAVE_OCAML_OUNIT
   LLVM_ENABLE_DIA_SDK
   LLVM_ENABLE_FFI
   LLVM_ENABLE_THREADS
+  LLVM_ENABLE_ZLIB
   LLVM_INCLUDE_GO_TESTS
   LLVM_LIBXML2_ENABLED
   LLVM_LINK_LLVM_DYLIB
Index: llvm/lib/Support/Compression.cpp
===
--- llvm/lib/Support/Compression.cpp
+++ llvm/lib/Support/Compression.cpp
@@ -17,13 +17,13 @@
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_ZLIB_H
+#if LLVM_ENABLE_ZLIB
 #include 
 #endif
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 static Error createError(StringRef Err) {
   return make_error(Err, inconvertibleErrorCode());
 }
Index: llvm/lib/Support/CRC.cpp
===
--- llvm/lib/Support/CRC.cpp
+++ llvm/lib/Support/CRC.cpp
@@ -25,7 +25,7 @@
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 0 || !HAVE_ZLIB_H
+#if !LLVM_ENABLE_ZLIB
 
 static const uint32_t CRCTable[256] = {
 0x, 0x77073096, 0xee0e612c, 0x990951ba, 0x076dc419, 0x706af48f,
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -1,7 +1,7 @@
-set(system_libs)
-if ( LLVM_ENABLE_ZLIB AND HAVE_LIBZ )
-  set(system_libs ${system_libs} ${ZLIB_LIBRARIES})
+if(LLVM_ENABLE_ZLIB)
+  set(imported_libs ZLIB::ZLIB)
 endif()
+
 if( MSVC OR MINGW )
   # libuuid required for FOLDERID_Profile usage in lib/Support/Windows/Path.inc.
   # advapi32 required for CryptAcquireContextW in lib/Support/Windows/Path.inc.
@@ -194,10 +194,32 @@
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/ADT
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/Support
   ${Backtrace_INCLUDE_DIRS}
-  LINK_LIBS ${system_libs} ${delayload_flags} ${Z3_LINK_FILES}
+  LINK_LIBS ${system_libs} ${imported_libs} ${delayload_flags} ${Z3_LINK_FILES}
   )
 
-set_property(TARGET LLVMSupport PROPERTY LLVM_SYSTEM

[PATCH] D84506: [tools] Add paranthesese for print in Python

[Gaël Écorchard via Phabricator via cfe-commits]

galou created this revision.
Herald added a reviewer: bollu.
Herald added subscribers: llvm-commits, openmp-commits, lldb-commits, 
Sanitizers, cfe-commits, usaxena95, kadircet, arphaman, jkorous.
Herald added projects: clang, Sanitizers, LLDB, OpenMP, LLVM.

Add paranthesese for print in Python


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84506

Files:
  
clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
  clang-tools-extra/clangd/include-mapping/gen_std.py
  compiler-rt/lib/dfsan/scripts/build-libc-list.py
  compiler-rt/test/sanitizer_common/android_commands/android_common.py
  compiler-rt/test/sanitizer_common/android_commands/android_compile.py
  compiler-rt/test/sanitizer_common/ios_commands/iossim_compile.py
  debuginfo-tests/llgdb-tests/llgdb.py
  lldb/utils/lui/lldbutil.py
  llvm/bindings/python/llvm/object.py
  openmp/runtime/tools/summarizeStats.py
  polly/utils/jscop2cloog.py
  polly/utils/pyscop/jscop2iscc.py

Index: polly/utils/pyscop/jscop2iscc.py
===
--- polly/utils/pyscop/jscop2iscc.py
+++ polly/utils/pyscop/jscop2iscc.py
@@ -1,4 +1,7 @@
 #!/usr/bin/env python
+
+from __future__ import print_function
+
 import argparse, isl, os
 import json
 
@@ -9,8 +12,8 @@
   for statement in scop['statements']:
 domain = domain.union(isl.USet(statement['domain']))
 
-  print "D :=",
-  print str(domain) + ";"
+  print('D :=', end='')
+  print(str(domain) + ';')
 
 def printAccesses(scop):
 
@@ -21,8 +24,8 @@
   if access['kind'] == 'read':
 read = read.union(isl.UMap(access['relation']))
 
-  print "R :=",
-  print str(read) + ";"
+  print('R :=', end='')
+  print(str(read) + ';')
 
   write = isl.UMap('{}')
 
@@ -31,8 +34,8 @@
   if access['kind'] == 'write':
 write = write.union(isl.UMap(access['relation']))
 
-  print "W :=",
-  print str(write) + ";"
+  print('W :=', end='')
+  print(str(write) + ';')
 
 def printSchedule(scop):
 
@@ -41,8 +44,8 @@
   for statement in scop['statements']:
 schedule = schedule.union(isl.UMap(statement['schedule']))
 
-  print "S :=",
-  print str(schedule) + ";"
+  print('S :=', end='')
+  print(str(schedule) + ';')
 
 def __main__():
   description = 'Translate JSCoP into iscc input'
@@ -58,10 +61,10 @@
   printAccesses(scop)
   printSchedule(scop)
 
-  print 'R := R * D;'
-  print 'W := W * D;'
-  print 'Dep := (last W before R under S)[0];'
-  print 'schedule D respecting Dep minimizing Dep;'
+  print('R := R * D;')
+  print('W := W * D;')
+  print('Dep := (last W before R under S)[0];')
+  print('schedule D respecting Dep minimizing Dep;')
 
 
 __main__()
Index: polly/utils/jscop2cloog.py
===
--- polly/utils/jscop2cloog.py
+++ polly/utils/jscop2cloog.py
@@ -50,7 +50,7 @@
   context = scop['context']
   domains = getDomains(scop)
   schedules = getSchedules(scop)
-  print template % (context, domains, schedules)
+  print(template % (context, domains, schedules))
 
 def __main__():
   description = 'Translate JSCoP into iscc input'
Index: openmp/runtime/tools/summarizeStats.py
===
--- openmp/runtime/tools/summarizeStats.py
+++ openmp/runtime/tools/summarizeStats.py
@@ -143,7 +143,7 @@
 res["counters"] = readCounters(f)
 return res
 except (OSError, IOError):
-print "Cannot open " + fname
+print("Cannot open " + fname)
 return None
 
 def usefulValues(l):
@@ -235,7 +235,7 @@
 compKeys[key] = data[key]
 else:
 nonCompKeys[key] = data[key]
-print "comp keys:", compKeys, "\n\n non comp keys:", nonCompKeys
+print("comp keys:", compKeys, "\n\n non comp keys:", nonCompKeys)
 return [compKeys, nonCompKeys]
 
 def drawMainPie(data, filebase, colors):
@@ -301,7 +301,7 @@
 """radar Charts finish here"""
 plt.savefig(filebase+"_"+s+"_"+chartType, bbox_inches='tight')
 elif s == 'timers':
-print "overheads in "+filebase
+print("overheads in " + filebase)
 numThreads = tmp[s]['SampleCount']['Total_OMP_parallel']
 for key in data.keys():
 if key[0:5] == 'Total':
Index: llvm/bindings/python/llvm/object.py
===
--- llvm/bindings/python/llvm/object.py
+++ llvm/bindings/python/llvm/object.py
@@ -57,22 +57,22 @@
 # This is NOT OK. You perform a lookup after the object has expired.
 symbols = list(obj.get_symbols())
 for symbol in symbols:
-print symbol.name # This raises because the object has expired.
+print(symbol.name) # This raises because the object has expired.
 
 # In this example, we mix a working and failing scenario.
 symbols = []
 for symbol in obj.get_symbols():
 symbols

[PATCH] D84506: [tools] Add paranthesese for print in Python

[Gaël Écorchard via Phabricator via cfe-commits]

galou added a comment.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.

Add parentheses for print in Python (all Python files where the print statement 
was used).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84506



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

[PATCH] D84511: Fix update_cc_test_checks.py --llvm-bin after D78478

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added reviewers: vitalybuka, jdoerfert, MaskRay.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Not passing --clang would result in a python exception after this change:
(TypeError: expected str, bytes or os.PathLike object, not NoneType)
because the --clang argument default was only being populated in the
initial argument parsing pass but not later on.
Fix this by adding an argparse callback to set the default values.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84511

Files:
  clang/test/utils/update_cc_test_checks/basic-cplusplus.test
  clang/test/utils/update_cc_test_checks/lit.local.cfg
  llvm/utils/UpdateTestChecks/common.py
  llvm/utils/update_cc_test_checks.py

Index: llvm/utils/update_cc_test_checks.py
===
--- llvm/utils/update_cc_test_checks.py
+++ llvm/utils/update_cc_test_checks.py
@@ -112,6 +112,20 @@
 return []
   return shlex.split(value)
 
+
+def infer_dependent_args(args):
+  if args.clang is None:
+if args.llvm_bin is None:
+  args.clang = 'clang'
+else:
+  args.clang = os.path.join(args.llvm_bin, 'clang')
+  if args.opt is None:
+if args.llvm_bin is None:
+  args.opt = 'opt'
+else:
+  args.opt = os.path.join(args.llvm_bin, 'opt')
+
+
 def config():
   parser = argparse.ArgumentParser(
   description=__doc__,
@@ -135,12 +149,8 @@
   help='Check "Function Attributes" for functions')
   parser.add_argument('tests', nargs='+')
   args = common.parse_commandline_args(parser)
+  infer_dependent_args(args)
 
-  if args.clang is None:
-if args.llvm_bin is None:
-  args.clang = 'clang'
-else:
-  args.clang = os.path.join(args.llvm_bin, 'clang')
   if not distutils.spawn.find_executable(args.clang):
 print('Please specify --llvm-bin or --clang', file=sys.stderr)
 sys.exit(1)
@@ -157,11 +167,6 @@
 common.warn('Could not determine clang builtins directory, some tests '
 'might not update correctly.')
 
-  if args.opt is None:
-if args.llvm_bin is None:
-  args.opt = 'opt'
-else:
-  args.opt = os.path.join(args.llvm_bin, 'opt')
   if not distutils.spawn.find_executable(args.opt):
 # Many uses of this tool will not need an opt binary, because it's only
 # needed for updating a test that runs clang | opt | FileCheck. So we
@@ -203,7 +208,7 @@
   script_name = os.path.basename(__file__)
 
   for ti in common.itertests(initial_args.tests, parser, 'utils/' + script_name,
- comment_prefix='//'):
+ comment_prefix='//', argparse_callback=infer_dependent_args):
 # Build a list of clang command lines and check prefixes from RUN lines.
 run_list = []
 line2spell_and_mangled_list = collections.defaultdict(list)
Index: llvm/utils/UpdateTestChecks/common.py
===
--- llvm/utils/UpdateTestChecks/common.py
+++ llvm/utils/UpdateTestChecks/common.py
@@ -44,8 +44,9 @@
 
 class TestInfo(object):
   def __init__(self, test, parser, script_name, input_lines, args, argv,
-   comment_prefix):
+   comment_prefix, argparse_callback):
 self.parser = parser
+self.argparse_callback = argparse_callback
 self.path = test
 self.args = args
 self.argv = argv
@@ -68,14 +69,14 @@
   if input_line.startswith(self.autogenerated_note_prefix):
 continue
   self.args, self.argv = check_for_command(input_line, self.parser,
-   self.args, self.argv)
+   self.args, self.argv, self.argparse_callback)
   if not self.args.enabled:
 output_lines.append(input_line)
 continue
   yield InputLineInfo(input_line, line_num, self.args, self.argv)
 
 
-def itertests(test_patterns, parser, script_name, comment_prefix=None):
+def itertests(test_patterns, parser, script_name, comment_prefix=None, argparse_callback=None):
   for pattern in test_patterns:
 # On Windows we must expand the patterns ourselves.
 tests_list = glob.glob(pattern)
@@ -86,19 +87,21 @@
   with open(test) as f:
 input_lines = [l.rstrip() for l in f]
   args = parser.parse_args()
+  if argparse_callback is not None:
+argparse_callback(args)
   argv = sys.argv[:]
   first_line = input_lines[0] if input_lines else ""
   if UTC_ADVERT in first_line:
 if script_name not in first_line and not args.force_update:
   warn("Skipping test which wasn't autogenerated by " + script_name, test)
   continue
-args, argv = check_for_command(first_line, parser, args, argv)
+args, argv = check_for_command(first_line, parser, args, argv, argparse_callback)
   elif args.update_only:
 assert UTC_ADVERT not in first_l

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

[Balázs Benics via Phabricator via cfe-commits]

steakhal updated this revision to Diff 280398.
steakhal added a comment.

- Created the `CStringChecker` subdirectory holding the related files.
- `CStringChecker` split up to 4 files:
  - `CStringChecker.h`: Declaration of the checker class.
  - `CStringChecker.cpp`: Definitions ONLY of the cstirng modeling functions.
  - `CStringLength.h`: The interface declaration of the cstring 
query/modification API (aka. //API header//).
  - `CStringLengthModeling.cpp`: Definitions of the bookkeeping part of the 
`CStringChecker` class AND the definitions of the //API header//.
- Added the `CStringChecker` folder as an include directory for the analyzer 
library, to make it easier to include the //API header//.


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

https://reviews.llvm.org/D84316

Files:
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringChecker.h
  clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringLength.h
  clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringLengthModeling.cpp

Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringLengthModeling.cpp
===
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringLengthModeling.cpp
@@ -0,0 +1,313 @@
+//=== CStringLengthModeling.cpp  Implementation of CStringLength API 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
+//
+//===--===//
+//
+// Implements the CStringLength API and the CStringChecker bookkeeping parts.
+// Updates the associated cstring lengths of memory regions:
+//  - Infers the cstring length of string literals.
+//  - Removes cstring length associations of dead symbols.
+//  - Handles region invalidation.
+//
+//===--===//
+
+#include "CStringChecker.h"
+#include "CStringLength.h"
+
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace clang;
+using namespace ento;
+using namespace cstring;
+
+/// Associates an strlen to a memory region.
+REGISTER_MAP_WITH_PROGRAMSTATE(CStringLengthMap, const MemRegion *, SVal)
+
+//===--===//
+// Implementation of the public CStringLength API.
+//===--===//
+
+ProgramStateRef cstring::setCStringLength(ProgramStateRef State,
+  const MemRegion *MR, SVal StrLength) {
+  assert(!StrLength.isUndef() && "Attempt to set an undefined string length");
+
+  MR = MR->StripCasts();
+
+  switch (MR->getKind()) {
+  case MemRegion::StringRegionKind:
+// FIXME: This can happen if we strcpy() into a string region. This is
+// undefined [C99 6.4.5p6], but we should still warn about it.
+return State;
+
+  case MemRegion::SymbolicRegionKind:
+  case MemRegion::AllocaRegionKind:
+  case MemRegion::NonParamVarRegionKind:
+  case MemRegion::ParamVarRegionKind:
+  case MemRegion::FieldRegionKind:
+  case MemRegion::ObjCIvarRegionKind:
+// These are the types we can currently track string lengths for.
+break;
+
+  case MemRegion::ElementRegionKind:
+// FIXME: Handle element regions by upper-bounding the parent region's
+// string length.
+return State;
+
+  default:
+// Other regions (mostly non-data) can't have a reliable C string length.
+// For now, just ignore the change.
+// FIXME: These are rare but not impossible. We should output some kind of
+// warning for things like strcpy((char[]){'a', 0}, "b");
+return State;
+  }
+
+  if (StrLength.isUnknown())
+return removeCStringLength(State, MR);
+  return State->set(MR, StrLength);
+}
+
+ProgramStateRef cstring::removeCStringLength(ProgramStateRef State,
+ const MemRegion *MR) {
+  return State->remove(MR);
+}
+
+static SVal getCStringLengthForRegion(CheckerContext &Ctx,
+  ProgramStateRef &State, const Expr *Ex,
+  const MemRegion *MR, bool Hypothetical) {
+  if (!Hypothetical) {
+// If there's a recorded length, go ahead and return it.
+if (const SVal *Recorded = State->get(MR))
+  return *Recorded;
+  }
+
+  // Otherwise, get a new symbol and update the state.
+ 

[PATCH] D84513: [clangd] Collect references for externally visible main-file symbols

[Aleksandr Platonov via Phabricator via cfe-commits]

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

Without this patch clangd does not collect references for main-file symbols if 
there is no public declaration in preamble.
Example:
`test1.c`

  void f1() {}

`test2.c`

  extern void f1();
  void f2() {
f^1();
  }

`Find all references` does not show definition of f1() in the result, but GTD 
works OK.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84513

Files:
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
  clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp


Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -624,11 +624,13 @@
   EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(Symbols, "NS").ID, _;
   EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "MACRO").ID,
   HaveRanges(Main.ranges("macro");
-  // Symbols *only* in the main file (a, b, c, FUNC) had no refs collected.
+  // Symbols *only* in the main file:
+  // - (a, b) externally visible and should have refs.
+  // - (c, FUNC) externally invisible and had no refs collected.
   auto MainSymbols =
   TestTU::withHeaderCode(SymbolsOnlyInMainCode.code()).headerSymbols();
-  EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "a").ID, _;
-  EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "b").ID, _;
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(MainSymbols, "a").ID, _)));
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(MainSymbols, "b").ID, _)));
   EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "c").ID, _;
   EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "FUNC").ID, 
_;
 }
@@ -816,11 +818,15 @@
 $Foo[[Foo]] fo;
   }
   )");
-  // The main file is normal .cpp file, we shouldn't collect any refs of 
symbols
-  // which are not declared in the preamble.
+  // The main file is normal .cpp file, we should collect the refs
+  // for externally visible symbols.
   TestFileName = testPath("foo.cpp");
   runSymbolCollector("", Header.code());
-  EXPECT_THAT(Refs, UnorderedElementsAre());
+  EXPECT_THAT(Refs,
+  UnorderedElementsAre(Pair(findSymbol(Symbols, "Foo").ID,
+HaveRanges(Header.ranges("Foo"))),
+   Pair(findSymbol(Symbols, "Func").ID,
+HaveRanges(Header.ranges("Func");
 
   // Run the .h file as main file, we should collect the refs.
   TestFileName = testPath("foo.h");
Index: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -171,7 +171,7 @@
   #endif
   )cpp";
   FS.Files[testPath("root/A.cc")] =
-  "#include \"A.h\"\nvoid g() { (void)common; }";
+  "#include \"A.h\"\nstatic void g() { (void)common; }";
   FS.Files[testPath("root/B.cc")] =
   R"cpp(
   #define A 0
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -314,7 +314,8 @@
   // file locations for references (as it aligns the behavior of clangd's
   // AST-based xref).
   // FIXME: we should try to use the file locations for other fields.
-  if (CollectRef && !IsMainFileOnly && !isa(ND) &&
+  if (CollectRef && (!IsMainFileOnly || ND->isExternallyVisible()) &&
+  !isa(ND) &&
   (Opts.RefsInHeaders ||
SM.getFileID(SM.getFileLoc(Loc)) == SM.getMainFileID()))
 DeclRefs[ND].emplace_back(SM.getFileLoc(Loc), Roles);
Index: clang-tools-extra/clangd/index/Serialization.cpp
===
--- clang-tools-extra/clangd/index/Serialization.cpp
+++ clang-tools-extra/clangd/index/Serialization.cpp
@@ -419,7 +419,7 @@
 // The current versioning scheme is simple - non-current versions are rejected.
 // If you make a breaking change, bump this version number to invalidate stored
 // data. Later we may want to support some backward compatibility.
-constexpr static uint32_t Version = 13;
+constexpr static uint32_t Version = 14;
 
 llvm::Expected readRIFF(llvm::StringRef Data) {
   auto RIFF = riff::readFile(Data);


Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-e

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

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

Szelethus added a comment.

I personally preferred the previous diff, the reporting part of the checker and 
the string length modeling was separated far more cleanly.

In D84316#2171267 , @NoQ wrote:

> Mmm, none of these benefits sound like they outweigh confusing the cost of 
> users with a new checker flag that can't even be used in any sensible way.


I think the new checker would be an ideal candidate for a hidden checker, not 
to mention that D78126 +D81761 
 enforces it anyways with asserts. With that 
in mind, I don't see what we're giving up here.

> If you want separate files, just put the checker into a header and include it 
> from multiple cpp files. A few checkers already do that - RetainCountChecker, 
> MPIChecker, UninitializedObjectChecker. There's nothing fundamental about 
> keeping checkers in an anonymous namespace. With that said, I don't want to 
> make an example out of RetainCountChecker -- the way it is structured is not 
> in line, at least the way I see it, with the modern way to develop large 
> checkers.

I agree that there is no magical gain from keeping them there. 
UninitializedObjectChecker, btw, doesn't peek out -- only some of the related 
infrastructure.


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

https://reviews.llvm.org/D84316



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

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

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

Szelethus added a comment.

In D84316#2171270 , @NoQ wrote:

> Imagine something like re-using the state trait implementation between 
> `MallocChecker` and `StreamChecker` because they both model "resources that 
> can be deallocated twice or leaked" - regardless of the specific nature of 
> these resources. These checkers can implement their own API modeling maps, 
> escape rules, warning messages, maybe model additional aspects of their 
> problems, but fundamentally they're solving the same problem: finding leaks 
> and overreleases of resources. This problem should ideally be solved once. 
> This is why i advocate for abstract, generalized, "half-baked" state trait 
> boilerplate implementations that can be re-used across checkers.


Big +1! I think there is a lot of smart in MallocChecker, and its far less 
confusing now then it used to be. It'd be worth exploring the merger of those 
checkers, but we should probably reverse this discussion for another time.


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

https://reviews.llvm.org/D84316



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

[PATCH] D83814: [clangd] Add Random Forest runtime for code completion.

[UTKARSH SAXENA via Phabricator via cfe-commits]

usaxena95 updated this revision to Diff 280406.
usaxena95 added a comment.

Addressed offline comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83814

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/CompletionModel.cmake
  clang-tools-extra/clangd/CompletionModelCodegen.py
  clang-tools-extra/clangd/for-review-only/CompletionModel.cpp
  clang-tools-extra/clangd/for-review-only/CompletionModel.h
  clang-tools-extra/clangd/for-review-only/DecisionForestRuntimeTest.cpp
  clang-tools-extra/clangd/for-review-only/DecisionForestRuntimeTest.h
  clang-tools-extra/clangd/model/features.json
  clang-tools-extra/clangd/model/forest.json
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/DecisionForestTests.cpp
  clang-tools-extra/clangd/unittests/model/CategoricalFeature.h
  clang-tools-extra/clangd/unittests/model/features.json
  clang-tools-extra/clangd/unittests/model/forest.json

Index: clang-tools-extra/clangd/unittests/model/forest.json
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/model/forest.json
@@ -0,0 +1,52 @@
+[
+{
+"operation": "if_greater",
+"feature": "ANumber",
+"threshold": 200.0,
+"then": {
+"operation": "if_greater",
+"feature": "AFloat",
+"threshold": -1,
+"then": {
+"operation": "boost",
+"score": 10.0
+},
+"else": {
+"operation": "boost",
+"score": -20.0
+}
+},
+"else": {
+"operation": "if_member",
+"feature": "ACategorical",
+"set": [
+"A",
+"C"
+],
+"then": {
+"operation": "boost",
+"score": 3.0
+},
+"else": {
+"operation": "boost",
+"score": -4.0
+}
+}
+},
+{
+"operation": "if_member",
+"feature": "ACategorical",
+"set": [
+"A",
+"B"
+],
+"then": {
+"operation": "boost",
+"score": 5.0
+},
+"else": {
+"operation": "boost",
+"score": -6.0
+}
+}
+]
\ No newline at end of file
Index: clang-tools-extra/clangd/unittests/model/features.json
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/model/features.json
@@ -0,0 +1,16 @@
+[
+{
+"name": "ANumber",
+"type": "NUMERICAL"
+},
+{
+"name": "AFloat",
+"type": "NUMERICAL"
+},
+{
+"name": "ACategorical",
+"type": "CATEGORICAL",
+"enum": "ns1::ns2::TestEnum",
+"header": "model/CategoricalFeature.h"
+}
+]
\ No newline at end of file
Index: clang-tools-extra/clangd/unittests/model/CategoricalFeature.h
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/model/CategoricalFeature.h
@@ -0,0 +1,5 @@
+namespace ns1 {
+namespace ns2 {
+enum TestEnum { A, B, C, D };
+} // namespace ns2
+} // namespace ns1
\ No newline at end of file
Index: clang-tools-extra/clangd/unittests/DecisionForestTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/DecisionForestTests.cpp
@@ -0,0 +1,30 @@
+
+#include "DecisionForestRuntimeTest.h"
+#include "model/CategoricalFeature.h"
+#include "gtest/gtest.h"
+
+namespace clangd {
+namespace clangd {
+
+TEST(DecisionForestRuntime, Evaluate) {
+  using Example = ::ns1::ns2::test::Example;
+  using Cat = ::ns1::ns2::TestEnum;
+  using ::ns1::ns2::test::Evaluate;
+
+  Example E;
+  E.SetANumber(200); // True
+  E.SetAFloat(0);// True: +10.0
+  E.SetACategorical(Cat::A); // True: +5.0
+  EXPECT_EQ(Evaluate(E), 15.0);
+
+  E.SetANumber(200); // True
+  E.SetAFloat(-2.5); // False: -20.0
+  E.SetACategorical(Cat::B); // True: +5.0
+  EXPECT_EQ(Evaluate(E), -15.0);
+
+  E.SetANumber(100); // False
+  E.SetACategorical(Cat::C); // True: +3.0, False: -6.0
+  EXPECT_EQ(Evaluate(E), -3.0);
+}
+} // namespace clangd
+} // namespace clangd
\ No newline at end of file
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -10,6 +10,7 @@
 #include "ClangdServer.h"
 #include "CodeComplete.h"
 #include "Compiler.h"
+#include "CompletionModel.h"
 #include "Matchers.h"
 #include "Protocol.h"

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

[Jeroen Dobbelaere via Phabricator via cfe-commits]

jeroen.dobbelaere added a comment.
Herald added a subscriber: dang.

Found some issue when looking at this code:  -ftrapping_math and 
-fno_trapping_math will never have effect




Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3155
+  if (Args.hasArg(OPT_ftrapping_math)) {
+Opts.setFPExceptionMode(LangOptions::FPE_Strict);
+  }

Calling 'Opts.setFPExceptionMode(xx)' here has no effect, as it will be 
overruled later on, on line 3174 !
Same is true on line 3159



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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

[PATCH] D84520: [Analyzer] Improve invalid dereference bug reporting in DereferenceChecker.

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

balazske created this revision.
Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso, 
gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, 
baloghadamsoftware, xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a project: clang.

Report undefined pointer dereference in similar way as null pointer dereference.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84520

Files:
  clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
  clang/test/Analysis/Inputs/expected-plists/null-deref-path-notes.m.plist
  clang/test/Analysis/Inputs/expected-plists/retain-release.m.objc.plist
  clang/test/Analysis/Inputs/expected-plists/retain-release.m.objcpp.plist
  clang/test/Analysis/misc-ps-region-store.m
  clang/test/Analysis/null-deref-offsets.c
  clang/test/Analysis/null-deref-path-notes.c
  clang/test/Analysis/null-deref-path-notes.cpp
  clang/test/Analysis/null-deref-path-notes.m
  clang/test/Analysis/null-deref-ps.c
  clang/test/Analysis/silence-checkers-and-packages-core-div-by-zero.cpp

Index: clang/test/Analysis/silence-checkers-and-packages-core-div-by-zero.cpp
===
--- clang/test/Analysis/silence-checkers-and-packages-core-div-by-zero.cpp
+++ clang/test/Analysis/silence-checkers-and-packages-core-div-by-zero.cpp
@@ -14,5 +14,5 @@
 return;
 
   int x = p[0];
-  // expected-warning@-1 {{Array access (from variable 'p') results in a null pointer dereference}}
+  // expected-warning@-1 {{Array access (from variable 'p') results in a dereference of a null pointer}}
 }
Index: clang/test/Analysis/null-deref-ps.c
===
--- clang/test/Analysis/null-deref-ps.c
+++ clang/test/Analysis/null-deref-ps.c
@@ -34,7 +34,7 @@
   if (x)
 return x[i - 1];
   
-  return x[i+1]; // expected-warning{{Array access (from variable 'x') results in a null pointer dereference}}
+  return x[i+1]; // expected-warning{{Array access (from variable 'x') results in a dereference of a null pointer}}
 }
 
 int f3_b(char* x) {
@@ -44,7 +44,7 @@
   if (x)
 return x[i - 1];
   
-  return x[i+1]++; // expected-warning{{Array access (from variable 'x') results in a null pointer dereference}}
+  return x[i+1]++; // expected-warning{{Array access (from variable 'x') results in a dereference of a null pointer}}
 }
 
 int f4(int *p) {
Index: clang/test/Analysis/null-deref-path-notes.m
===
--- clang/test/Analysis/null-deref-path-notes.m
+++ clang/test/Analysis/null-deref-path-notes.m
@@ -58,8 +58,8 @@
 @public int *p;
 }
 - (void)useArray {
-  p[1] = 2; // expected-warning{{Array access (via ivar 'p') results in a null pointer dereference}}
-// expected-note@-1{{Array access (via ivar 'p') results in a null pointer dereference}}
+  p[1] = 2; // expected-warning{{Array access (via ivar 'p') results in a dereference of a null pointer}}
+// expected-note@-1{{Array access (via ivar 'p') results in a dereference of a null pointer}}
 }
 @end
 
Index: clang/test/Analysis/null-deref-path-notes.cpp
===
--- clang/test/Analysis/null-deref-path-notes.cpp
+++ clang/test/Analysis/null-deref-path-notes.cpp
@@ -15,8 +15,8 @@
 // Properly track the null pointer in the array field back to the default
 // constructor of 'h'.
 void c::f(B &g, int &i) {
-  e(g.d[9], i); // expected-warning{{Array access (via field 'd') results in a null pointer dereference}}
-// expected-note@-1{{Array access (via field 'd') results in a null pointer dereference}}
+  e(g.d[9], i); // expected-warning{{Array access (via field 'd') results in a dereference of a null pointer}}
+// expected-note@-1{{Array access (via field 'd') results in a dereference of a null pointer}}
   B h, a; // expected-note{{Value assigned to 'h.d'}}
   a.d == __null; // expected-note{{Assuming the condition is true}}
   a.d != h.d; // expected-note{{Assuming 'a.d' is equal to 'h.d'}}
Index: clang/test/Analysis/null-deref-path-notes.c
===
--- clang/test/Analysis/null-deref-path-notes.c
+++ clang/test/Analysis/null-deref-path-notes.c
@@ -4,8 +4,8 @@
 // of the null pointer for path notes.
 void pr34373() {
   int *a = 0; // expected-note{{'a' initialized to a null pointer value}}
-  (a + 0)[0]; // expected-warning{{Array access results in a null pointer dereference}}
-  // expected-note@-1{{Array access results in a null pointer dereference}}
+  (a + 0)[0]; // expected-warning{{Array access results in a dereference of a null pointer}}
+  // expected-note@-1{{Array access results in a dereference of a null pointer}}
 }
 
 typedef __typeof(sizeof(int)) size_t;
Index: clang/test/Analysis/null-deref-offsets.c
==

[PATCH] D84005: Introduce ns_error_domain attribute.

[Michael Forster via Phabricator via cfe-commits]

MForster updated this revision to Diff 280415.
MForster marked 15 inline comments as done.
MForster added a comment.
Herald added a reviewer: jdoerfert.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84005

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/ns_error_enum.c

Index: clang/test/Sema/ns_error_enum.c
===
--- /dev/null
+++ clang/test/Sema/ns_error_enum.c
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -verify %s -x c -Wno-elaborated-enum-base
+// RUN: %clang_cc1 -verify %s -x c++ -Wno-elaborated-enum-base
+// RUN: %clang_cc1 -verify %s -x objective-c
+// RUN: %clang_cc1 -verify %s -x objective-c++
+
+
+#define CF_ENUM(_type, _name) enum _name : _type _name; enum _name : _type
+#define NS_ENUM(_type, _name) CF_ENUM(_type, _name)
+
+#define NS_ERROR_ENUM(_type, _name, _domain)  \
+  enum _name : _type _name; enum __attribute__((ns_error_domain(_domain))) _name : _type
+
+typedef NS_ENUM(unsigned, MyEnum) {
+  MyFirst,
+  MySecond,
+};
+
+typedef NS_ENUM(invalidType, MyInvalidEnum) {
+// expected-error@-1{{unknown type name 'invalidType'}}
+// expected-error@-2{{unknown type name 'invalidType'}}
+  MyFirstInvalid,
+  MySecondInvalid,
+};
+
+const char *MyErrorDomain;
+typedef NS_ERROR_ENUM(unsigned char, MyErrorEnum, MyErrorDomain) {
+	MyErrFirst,
+	MyErrSecond,
+};
+struct __attribute__((ns_error_domain(MyErrorDomain))) MyStructErrorDomain {};
+  // expected-error@-1{{'ns_error_domain' attribute only applies to enums}}
+
+int __attribute__((ns_error_domain(MyErrorDomain))) NotTagDecl;
+  // expected-error@-1{{'ns_error_domain' attribute only applies to enums}}
+
+typedef NS_ERROR_ENUM(unsigned char, MyErrorEnumInvalid, InvalidDomain) {
+	// expected-error@-1{{domain argument 'InvalidDomain' does not refer to global constant}}
+	MyErrFirstInvalid,
+	MyErrSecondInvalid,
+};
+
+typedef NS_ERROR_ENUM(unsigned char, MyErrorEnumInvalid, "domain-string");
+  // expected-error@-1{{'ns_error_domain' attribute requires parameter 1 to be an identifier}}
+
+void foo() {}
+typedef NS_ERROR_ENUM(unsigned char, MyErrorEnumInvalidFunction, foo);
+  // expected-error@-1{{domain argument 'foo' does not refer to global constant}}
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -80,6 +80,7 @@
 // CHECK-NEXT: MipsShortCall (SubjectMatchRule_function)
 // CHECK-NEXT: NSConsumed (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: NSConsumesSelf (SubjectMatchRule_objc_method)
+// CHECK-NEXT: NSErrorDomain (SubjectMatchRule_enum)
 // CHECK-NEXT: Naked (SubjectMatchRule_function)
 // CHECK-NEXT: NoBuiltin (SubjectMatchRule_function)
 // CHECK-NEXT: NoCommon (SubjectMatchRule_variable)
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -5322,6 +5322,35 @@
   D->addAttr(::new (S.Context) ObjCRequiresSuperAttr(S.Context, Attrs));
 }
 
+static void handleNSErrorDomain(Sema &S, Decl *D, const ParsedAttr &AL) {
+  IdentifierLoc *IdentLoc = AL.isArgIdent(0) ? AL.getArgAsIdent(0) : nullptr;
+  if (!IdentLoc || !IdentLoc->Ident) {
+// Try to locate the argument directly
+SourceLocation loc = AL.getLoc();
+if (const Expr *E = AL.getArgAsExpr(0))
+  loc = E->getBeginLoc();
+
+S.Diag(loc, diag::err_attribute_argument_n_type)
+<< AL << 1 << AANT_ArgumentIdentifier;
+
+return;
+  }
+
+  // Verify that the identifier is a valid decl in the C decl namespace
+  LookupResult lookupResult(S, DeclarationName(IdentLoc->Ident),
+SourceLocation(),
+Sema::LookupNameKind::LookupOrdinaryName);
+  if (!S.LookupName(lookupResult, S.TUScope) ||
+  !lookupResult.getAsSingle()) {
+S.Diag(IdentLoc->Loc, diag::err_nserrordomain_invalid_decl)
+<< IdentLoc->Ident;
+return;
+  }
+
+  D->addAttr(::new (S.Context)
+ NSErrorDomainAttr(S.Context, AL, IdentLoc->Ident));
+}
+
 static void handleObjCBridgeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   IdentifierLoc *Parm = AL.isArgIdent(0) ? AL.getArgAsIdent(0) : nullptr;
 
@@ -7093,6 +7122,9 @@
   case ParsedAttr::AT_ObjCBoxable:
 handleObjCBoxable(S, D, AL);
 break;
+  case ParsedAttr::AT_NSErrorDomain:
+handleNSErrorDomain(S, D, AL);
+break;
   case ParsedAttr::AT_CFAuditedTransfer:
 handleSimpleAttributeWithExclusions(S, D, AL);
Index: clang/include/

[PATCH] D84005: Introduce ns_error_domain attribute.

[Michael Forster via Phabricator via cfe-commits]

MForster added a comment.

In D84005#2162433 , @aaron.ballman 
wrote:

> It's a bit odd that this attribute has an AST node created for it but nothing 
> is using that AST node elsewhere in the project. Are there other patches 
> expected for making use of this attribute?


It gets used from Swift.




Comment at: clang/include/clang/Basic/AttrDocs.td:3330
+
+For example:
+

aaron.ballman wrote:
> I still feel like I'm lacking information about the domain. In the example, 
> the domain is an uninitialized `const char *`, so how does this information 
> get used by the attribute? Can I use any type of global I want? What about a 
> literal?
I won't claim that I understand this well. Maybe @gribozavr2 or @milseman can 
explain this better. Literals are not allowed, however. The test explicitly 
forbids that.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9454
+  "domain argument %0 does not refer to global constant">;
+def err_nserrordomain_requires_identifier : Error<
+  "domain argument must be an identifier">;

aaron.ballman wrote:
> I don't think this requires a custom diagnostic -- we can use 
> `err_attribute_argument_n_type` instead.
> I don't think this requires a custom diagnostic -- we can use 
> `err_attribute_argument_n_type` instead.


Done. Can you please confirm that `n` is 1-based?



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5355
+S.Diag(identLoc->Loc, diag::err_nserrordomain_invalid_decl)
+<< identLoc->Ident;
+return;

aaron.ballman wrote:
> We're doing this lookup in the context of where the attribute is being used, 
> but then creating the attribute with only the identifier and not the result 
> of that lookup. This makes me a bit worried that when something goes to 
> resolve that identifier to a variable later, it may get a different result 
> because the lookup context will be different. Do you need to store the 
> VarDecl on the semantic attribute, rather than the identifier?
>We're doing this lookup in the context of where the attribute is being used, 
>but then creating the attribute with only the identifier and not the result of 
>that lookup. This makes me a bit worried that when something goes to resolve 
>that identifier to a variable later, it may get a different result because the 
>lookup context will be different. Do you need to store the VarDecl on the 
>semantic attribute, rather than the identifier?


When this gets imported into Swift, only the name of the identifier gets used. 
I'm not quite sure why this was defined like this. This is another thing where 
I would hope that @gribozavr2 or @milseman know more...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84005



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

[PATCH] D84520: [Analyzer] Improve invalid dereference bug reporting in DereferenceChecker.

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

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

Not sure if this is a good solution. There are few tests that test the 
undefined reference case. But the added code should work the same way as in the 
null pointer case?




Comment at: clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp:135
+DerefKindStr = "dereference of an undefined pointer value";
+break;
+  };

Add "results in a " here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84520



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

[PATCH] D84005: Introduce ns_error_domain attribute.

[Michael Forster via Phabricator via cfe-commits]

MForster added a subscriber: doug.gregor.
MForster added a comment.

@milseman, @doug.gregor, could you please help with the open questions on this 
review?

Specifically:

- Is `ns_error_domain` ever needed for something other than an enum?
- Why is this designed in the way it is (requiring an identifier for the 
domain, not allowing literals and then only using the name of the identifier 
from Swift)?
- Is it ok to make this attribute inheritable?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84005



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

[PATCH] D84005: Introduce ns_error_domain attribute.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5344
+  if (!S.LookupName(lookupResult, S.TUScope) ||
+  !lookupResult.getAsSingle()) {
+S.Diag(IdentLoc->Loc, diag::err_nserrordomain_invalid_decl)

Just a note that `LookupResult::getAsSingle` has tricky semantics (returns null 
if the result is not exactly `LookupResultKind::Found`) and has been (and still 
is) the source of many bugs in clang.

(Example: my favourite one is still the silly:
```
struct S {
  void not_overloaded();
  enum { not_overloaded }; // error; redefinition of 'not_overloaded'

  void overloaded();
  void overloaded(int);
  enum { overloaded }; // clang is fine with this!
};
```
)

I don't know if it is a problem here or not though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84005



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

[PATCH] D83665: [OpenCL] Fixed missing address space for templated copy constructor

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Thanks


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

https://reviews.llvm.org/D83665



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

[PATCH] D84310: [libTooling] Add assorted `EditGenerator` combinators.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 280424.
ymandel added a comment.

fixed lint


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84310

Files:
  clang/include/clang/Tooling/Transformer/MatchConsumer.h
  clang/include/clang/Tooling/Transformer/RewriteRule.h
  clang/lib/Tooling/Transformer/RewriteRule.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -10,6 +10,7 @@
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/Tooling.h"
 #include "clang/Tooling/Transformer/RangeSelector.h"
+#include "clang/Tooling/Transformer/RewriteRule.h"
 #include "clang/Tooling/Transformer/Stencil.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
@@ -378,6 +379,41 @@
Input, Expected);
 }
 
+TEST_F(TransformerTest, NoEdits) {
+  using transformer::noEdits;
+  std::string Input = "int f(int x) { return x; }";
+  testRule(makeRule(returnStmt().bind("return"), noEdits()), Input, Input);
+}
+
+TEST_F(TransformerTest, IfBound2Args) {
+  using transformer::ifBound;
+  std::string Input = "int f(int x) { return x; }";
+  std::string Expected = "int f(int x) { CHANGE; }";
+  testRule(makeRule(returnStmt().bind("return"),
+ifBound("return", changeTo(cat("CHANGE;",
+   Input, Expected);
+}
+
+TEST_F(TransformerTest, IfBound3Args) {
+  using transformer::ifBound;
+  std::string Input = "int f(int x) { return x; }";
+  std::string Expected = "int f(int x) { CHANGE; }";
+  testRule(makeRule(returnStmt().bind("return"),
+ifBound("nothing", changeTo(cat("ERROR")),
+changeTo(cat("CHANGE;",
+   Input, Expected);
+}
+
+TEST_F(TransformerTest, ShrinkTo) {
+  using transformer::shrinkTo;
+  std::string Input = "int f(int x) { return x; }";
+  std::string Expected = "return x;";
+  testRule(makeRule(functionDecl(hasDescendant(returnStmt().bind("return")))
+.bind("function"),
+shrinkTo(node("function"), node("return"))),
+   Input, Expected);
+}
+
 TEST_F(TransformerTest, InsertBeforeEdit) {
   std::string Input = R"cc(
 int f() {
@@ -497,6 +533,90 @@
   Input, Expected);
 }
 
+TEST_F(TransformerTest, EditList) {
+  using clang::transformer::editList;
+  std::string Input = R"cc(
+void foo() {
+  if (10 > 1.0)
+log(1) << "oh no!";
+  else
+log(0) << "ok";
+}
+  )cc";
+  std::string Expected = R"(
+void foo() {
+  if (true) { /* then */ }
+  else { /* else */ }
+}
+  )";
+
+  StringRef C = "C", T = "T", E = "E";
+  testRule(makeRule(ifStmt(hasCondition(expr().bind(C)),
+   hasThen(stmt().bind(T)), hasElse(stmt().bind(E))),
+editList({changeTo(node(std::string(C)), cat("true")),
+  changeTo(statement(std::string(T)),
+   cat("{ /* then */ }")),
+  changeTo(statement(std::string(E)),
+   cat("{ /* else */ }"))})),
+   Input, Expected);
+}
+
+TEST_F(TransformerTest, Flatten) {
+  using clang::transformer::editList;
+  std::string Input = R"cc(
+void foo() {
+  if (10 > 1.0)
+log(1) << "oh no!";
+  else
+log(0) << "ok";
+}
+  )cc";
+  std::string Expected = R"(
+void foo() {
+  if (true) { /* then */ }
+  else { /* else */ }
+}
+  )";
+
+  StringRef C = "C", T = "T", E = "E";
+  testRule(
+  makeRule(
+  ifStmt(hasCondition(expr().bind(C)), hasThen(stmt().bind(T)),
+ hasElse(stmt().bind(E))),
+  flatten(changeTo(node(std::string(C)), cat("true")),
+  changeTo(statement(std::string(T)), cat("{ /* then */ }")),
+  changeTo(statement(std::string(E)), cat("{ /* else */ }",
+  Input, Expected);
+}
+
+TEST_F(TransformerTest, FlattenWithMixedArgs) {
+  using clang::transformer::editList;
+  std::string Input = R"cc(
+void foo() {
+  if (10 > 1.0)
+log(1) << "oh no!";
+  else
+log(0) << "ok";
+}
+  )cc";
+  std::string Expected = R"(
+void foo() {
+  if (true) { /* then */ }
+  else { /* else */ }
+}
+  )";
+
+  StringRef C = "C", T = "T", E = "E";
+  testRule(makeRule(ifStmt(hasCondition(expr().bind(C)),
+   hasThen(stmt().bind(T)), hasElse(stmt().bind(E))),
+flatten(changeTo(node(std::string(C)), cat("true")),
+edit(changeTo(statement(std::string(T)),
+  cat("{ /* then */ }"))),
+editList({changeTo(statement(std::string(E)),
+ 

[PATCH] D84513: [clangd] Collect references for externally visible main-file symbols

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks, this looks good in general, but it would be nice to make sure we are 
not blowing the index memory and disk usage.

could you grab some numbers for these two before/after this patch?




Comment at: clang-tools-extra/clangd/index/Serialization.cpp:422
 // data. Later we may want to support some backward compatibility.
-constexpr static uint32_t Version = 13;
+constexpr static uint32_t Version = 14;
 

this isn't really necessary as we are not changing the serialization format. 
you would need to delete the existing clangd caches from your machine instead.



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:317
   // FIXME: we should try to use the file locations for other fields.
-  if (CollectRef && !IsMainFileOnly && !isa(ND) &&
+  if (CollectRef && (!IsMainFileOnly || ND->isExternallyVisible()) &&
+  !isa(ND) &&

this will result in calculation and caching of linkage. it should be OK to do 
so though, as we run indexing after parsing finishes. (no action needed, just 
some notes for future travellers).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84513



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

[PATCH] D84506: [tools] Add paranthesese for print in Python

[Johannes Doerfert via Phabricator via cfe-commits]

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

LGTM. Seems NFC and towards the right syntax, thx :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84506



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

[PATCH] D84310: [libTooling] Add assorted `EditGenerator` combinators.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcf428778128f: [libTooling] Add assorted `EditGenerator` 
combinators. (authored by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84310

Files:
  clang/include/clang/Tooling/Transformer/MatchConsumer.h
  clang/include/clang/Tooling/Transformer/RewriteRule.h
  clang/lib/Tooling/Transformer/RewriteRule.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -10,6 +10,7 @@
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/Tooling.h"
 #include "clang/Tooling/Transformer/RangeSelector.h"
+#include "clang/Tooling/Transformer/RewriteRule.h"
 #include "clang/Tooling/Transformer/Stencil.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
@@ -378,6 +379,41 @@
Input, Expected);
 }
 
+TEST_F(TransformerTest, NoEdits) {
+  using transformer::noEdits;
+  std::string Input = "int f(int x) { return x; }";
+  testRule(makeRule(returnStmt().bind("return"), noEdits()), Input, Input);
+}
+
+TEST_F(TransformerTest, IfBound2Args) {
+  using transformer::ifBound;
+  std::string Input = "int f(int x) { return x; }";
+  std::string Expected = "int f(int x) { CHANGE; }";
+  testRule(makeRule(returnStmt().bind("return"),
+ifBound("return", changeTo(cat("CHANGE;",
+   Input, Expected);
+}
+
+TEST_F(TransformerTest, IfBound3Args) {
+  using transformer::ifBound;
+  std::string Input = "int f(int x) { return x; }";
+  std::string Expected = "int f(int x) { CHANGE; }";
+  testRule(makeRule(returnStmt().bind("return"),
+ifBound("nothing", changeTo(cat("ERROR")),
+changeTo(cat("CHANGE;",
+   Input, Expected);
+}
+
+TEST_F(TransformerTest, ShrinkTo) {
+  using transformer::shrinkTo;
+  std::string Input = "int f(int x) { return x; }";
+  std::string Expected = "return x;";
+  testRule(makeRule(functionDecl(hasDescendant(returnStmt().bind("return")))
+.bind("function"),
+shrinkTo(node("function"), node("return"))),
+   Input, Expected);
+}
+
 TEST_F(TransformerTest, InsertBeforeEdit) {
   std::string Input = R"cc(
 int f() {
@@ -497,6 +533,90 @@
   Input, Expected);
 }
 
+TEST_F(TransformerTest, EditList) {
+  using clang::transformer::editList;
+  std::string Input = R"cc(
+void foo() {
+  if (10 > 1.0)
+log(1) << "oh no!";
+  else
+log(0) << "ok";
+}
+  )cc";
+  std::string Expected = R"(
+void foo() {
+  if (true) { /* then */ }
+  else { /* else */ }
+}
+  )";
+
+  StringRef C = "C", T = "T", E = "E";
+  testRule(makeRule(ifStmt(hasCondition(expr().bind(C)),
+   hasThen(stmt().bind(T)), hasElse(stmt().bind(E))),
+editList({changeTo(node(std::string(C)), cat("true")),
+  changeTo(statement(std::string(T)),
+   cat("{ /* then */ }")),
+  changeTo(statement(std::string(E)),
+   cat("{ /* else */ }"))})),
+   Input, Expected);
+}
+
+TEST_F(TransformerTest, Flatten) {
+  using clang::transformer::editList;
+  std::string Input = R"cc(
+void foo() {
+  if (10 > 1.0)
+log(1) << "oh no!";
+  else
+log(0) << "ok";
+}
+  )cc";
+  std::string Expected = R"(
+void foo() {
+  if (true) { /* then */ }
+  else { /* else */ }
+}
+  )";
+
+  StringRef C = "C", T = "T", E = "E";
+  testRule(
+  makeRule(
+  ifStmt(hasCondition(expr().bind(C)), hasThen(stmt().bind(T)),
+ hasElse(stmt().bind(E))),
+  flatten(changeTo(node(std::string(C)), cat("true")),
+  changeTo(statement(std::string(T)), cat("{ /* then */ }")),
+  changeTo(statement(std::string(E)), cat("{ /* else */ }",
+  Input, Expected);
+}
+
+TEST_F(TransformerTest, FlattenWithMixedArgs) {
+  using clang::transformer::editList;
+  std::string Input = R"cc(
+void foo() {
+  if (10 > 1.0)
+log(1) << "oh no!";
+  else
+log(0) << "ok";
+}
+  )cc";
+  std::string Expected = R"(
+void foo() {
+  if (true) { /* then */ }
+  else { /* else */ }
+}
+  )";
+
+  StringRef C = "C", T = "T", E = "E";
+  testRule(makeRule(ifStmt(hasCondition(expr().bind(C)),
+   hasThen(stmt().bind(T)), hasElse(stmt().bind(E))),
+flatten(changeTo(node(std::string(C)), cat("true")),
+edit(changeTo(statement(std::string(T)),
+  cat("{ /* th

[clang] cf42877 - [libTooling] Add assorted `EditGenerator` combinators.

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2020-07-24T12:51:54Z
New Revision: cf428778128fed5eacee884964af53bf4a9f74f2

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

LOG: [libTooling] Add assorted `EditGenerator` combinators.

Summary:
This patch adds various combinators that help in constructing `EditGenerator`s:
   * `noEdits`
   * `ifBound`, specialized to `ASTEdit`
   * `flatten` and `flattenVector` which allow for easy construction from a set
 of sub edits.
   * `shrinkTo`, which generates edits to shrink a given range to another that
 it encloses.

Reviewers: asoffer, gribozavr2

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/include/clang/Tooling/Transformer/MatchConsumer.h
clang/include/clang/Tooling/Transformer/RewriteRule.h
clang/lib/Tooling/Transformer/RewriteRule.cpp
clang/unittests/Tooling/TransformerTest.cpp

Removed: 




diff  --git a/clang/include/clang/Tooling/Transformer/MatchConsumer.h 
b/clang/include/clang/Tooling/Transformer/MatchConsumer.h
index f407ffce3d25..cb0a5f684b7d 100644
--- a/clang/include/clang/Tooling/Transformer/MatchConsumer.h
+++ b/clang/include/clang/Tooling/Transformer/MatchConsumer.h
@@ -99,11 +99,5 @@ llvm::Expected MatchComputation::eval(
   return Output;
 }
 } // namespace transformer
-
-namespace tooling {
-// DEPRECATED: Temporary alias supporting client migration to the `transformer`
-// namespace.
-using transformer::ifBound;
-} // namespace tooling
 } // namespace clang
 #endif // LLVM_CLANG_TOOLING_TRANSFORMER_MATCH_CONSUMER_H_

diff  --git a/clang/include/clang/Tooling/Transformer/RewriteRule.h 
b/clang/include/clang/Tooling/Transformer/RewriteRule.h
index 1be572736460..c22a2da81fe6 100644
--- a/clang/include/clang/Tooling/Transformer/RewriteRule.h
+++ b/clang/include/clang/Tooling/Transformer/RewriteRule.h
@@ -107,9 +107,42 @@ struct ASTEdit {
 /// clients.  We recommend use of the \c AtomicChange or \c Replacements 
classes
 /// for assistance in detecting such conflicts.
 EditGenerator editList(llvm::SmallVector Edits);
-// Convenience form of `editList` for a single edit.
+/// Convenience form of `editList` for a single edit.
 EditGenerator edit(ASTEdit);
 
+/// Convenience generator for a no-op edit generator.
+inline EditGenerator noEdits() { return editList({}); }
+
+/// Convenience version of `ifBound` specialized to `ASTEdit`.
+inline EditGenerator ifBound(std::string ID, ASTEdit TrueEdit,
+ ASTEdit FalseEdit) {
+  return ifBound(std::move(ID), edit(std::move(TrueEdit)),
+ edit(std::move(FalseEdit)));
+}
+
+/// Convenience version of `ifBound` that has no "False" branch. If the node is
+/// not bound, then no edits are produced.
+inline EditGenerator ifBound(std::string ID, ASTEdit TrueEdit) {
+  return ifBound(std::move(ID), edit(std::move(TrueEdit)), noEdits());
+}
+
+/// Flattens a list of generators into a single generator whose elements are 
the
+/// concatenation of the results of the argument generators.
+EditGenerator flattenVector(SmallVector Generators);
+
+namespace detail {
+/// Convenience function to construct an \c EditGenerator. Overloaded for 
common
+/// cases so that user doesn't need to specify which factory function to
+/// use. This pattern gives benefits similar to implicit constructors, while
+/// maintaing a higher degree of explicitness.
+inline EditGenerator injectEdits(ASTEdit E) { return edit(std::move(E)); }
+inline EditGenerator injectEdits(EditGenerator G) { return G; }
+} // namespace detail
+
+template  EditGenerator flatten(Ts &&...Edits) {
+  return flattenVector({detail::injectEdits(std::forward(Edits))...});
+}
+
 /// Format of the path in an include directive -- angle brackets or quotes.
 enum class IncludeFormat {
   Quoted,
@@ -291,6 +324,14 @@ inline ASTEdit withMetadata(ASTEdit Edit, Callable 
Metadata) {
   return Edit;
 }
 
+/// Assuming that the inner range is enclosed by the outer range, creates
+/// precision edits to remove the parts of the outer range that are not 
included
+/// in the inner range.
+inline EditGenerator shrinkTo(RangeSelector outer, RangeSelector inner) {
+  return editList({remove(enclose(before(outer), before(inner))),
+   remove(enclose(after(inner), after(outer)))});
+}
+
 /// The following three functions are a low-level part of the RewriteRule
 /// API. We expose them for use in implementing the fixtures that interpret
 /// RewriteRule, like Transformer and TransfomerTidy, or for more advanced

diff  --git a/clang/lib/Tooling/Transformer/RewriteRule.cpp 
b/clang/lib/Tooling/Transformer/RewriteRule.cpp
index a212a868c81d..c145895af7ab 100644
--- a/clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ b/cl

[PATCH] D84476: Make hip math headers easier to use from C

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

I'm fine with this @yaxunl should accept though.




Comment at: clang/lib/Headers/__clang_hip_math.h:561
 inline double abs(double __x) { return __ocml_fabs_f64(__x); }
+#endif
 __DEVICE__

Nit: You mix the C and C++ math declarations in this file, while possible, I 
somehow thing the cuda_{cmath/math} split is nicer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84476



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

[PATCH] D84525: [clangd] Add marshalling code for all request types

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Only FuzzyFindRequest is implemented via Marshaller even though other requests
also follow a similar pattern. Unify them under the marshalling umbrella and
make the server requests even more uniform to complement D84499 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84525

Files:
  clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
  clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
  clang-tools-extra/clangd/index/remote/server/Server.cpp
  clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp

Index: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
===
--- clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
+++ clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
@@ -289,7 +289,8 @@
   auto Serialized = ProtobufMarshaller.toProtobuf(Request);
   EXPECT_EQ(Serialized.proximity_paths_size(), 2);
   auto Deserialized = ProtobufMarshaller.fromProtobuf(&Serialized);
-  EXPECT_THAT(Deserialized.ProximityPaths,
+  EXPECT_TRUE(Deserialized);
+  EXPECT_THAT(Deserialized->ProximityPaths,
   testing::ElementsAre(testPath("remote/Header.h"),
testPath("remote/subdir/OtherHeader.h")));
 }
Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -59,14 +59,10 @@
   grpc::Status Lookup(grpc::ServerContext *Context,
   const LookupRequest *Request,
   grpc::ServerWriter *Reply) override {
-clangd::LookupRequest Req;
-for (const auto &ID : Request->ids()) {
-  auto SID = SymbolID::fromStr(StringRef(ID));
-  if (!SID)
-return grpc::Status::CANCELLED;
-  Req.IDs.insert(*SID);
-}
-Index->lookup(Req, [&](const clangd::Symbol &Sym) {
+const auto Req = ProtobufMarshaller->fromProtobuf(Request);
+if (!Req)
+  return grpc::Status::CANCELLED;
+Index->lookup(*Req, [&](const clangd::Symbol &Sym) {
   auto SerializedSymbol = ProtobufMarshaller->toProtobuf(Sym);
   if (!SerializedSymbol)
 return;
@@ -84,7 +80,9 @@
  const FuzzyFindRequest *Request,
  grpc::ServerWriter *Reply) override {
 const auto Req = ProtobufMarshaller->fromProtobuf(Request);
-bool HasMore = Index->fuzzyFind(Req, [&](const clangd::Symbol &Sym) {
+if (!Req)
+  return grpc::Status::CANCELLED;
+bool HasMore = Index->fuzzyFind(*Req, [&](const clangd::Symbol &Sym) {
   auto SerializedSymbol = ProtobufMarshaller->toProtobuf(Sym);
   if (!SerializedSymbol)
 return;
@@ -100,14 +98,10 @@
 
   grpc::Status Refs(grpc::ServerContext *Context, const RefsRequest *Request,
 grpc::ServerWriter *Reply) override {
-clangd::RefsRequest Req;
-for (const auto &ID : Request->ids()) {
-  auto SID = SymbolID::fromStr(StringRef(ID));
-  if (!SID)
-return grpc::Status::CANCELLED;
-  Req.IDs.insert(*SID);
-}
-bool HasMore = Index->refs(Req, [&](const clangd::Ref &Reference) {
+const auto Req = ProtobufMarshaller->fromProtobuf(Request);
+if (!Req)
+  return grpc::Status::CANCELLED;
+bool HasMore = Index->refs(*Req, [&](const clangd::Ref &Reference) {
   auto SerializedRef = ProtobufMarshaller->toProtobuf(Reference);
   if (!SerializedRef)
 return;
Index: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
===
--- clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
+++ clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
@@ -38,10 +38,15 @@
   Marshaller() = delete;
   Marshaller(llvm::StringRef RemoteIndexRoot, llvm::StringRef LocalIndexRoot);
 
-  clangd::FuzzyFindRequest fromProtobuf(const FuzzyFindRequest *Request);
   llvm::Optional fromProtobuf(const Symbol &Message);
   llvm::Optional fromProtobuf(const Ref &Message);
 
+  llvm::Optional
+  fromProtobuf(const LookupRequest *Message);
+  llvm::Optional
+  fromProtobuf(const FuzzyFindRequest *Message);
+  llvm::Optional fromProtobuf(const RefsRequest *Message);
+
   /// toProtobuf() functions serialize native clangd types and strip IndexRoot
   /// from the file paths specific to indexing machine. fromProtobuf() functions
   /// deserialize clangd types and translate relative paths into machine-native
Index: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
===
--- 

[PATCH] D83909: [OPENMP] Fix PR46730: Fix compiler crash on taskloop over constructible loop counters.

[Johannes Doerfert via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83909



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

[PATCH] D83814: [clangd] Add Random Forest runtime for code completion.

[UTKARSH SAXENA via Phabricator via cfe-commits]

usaxena95 updated this revision to Diff 280455.
usaxena95 edited the summary of this revision.
usaxena95 added a comment.

Better formatting in generated files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83814

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/CompletionModel.cmake
  clang-tools-extra/clangd/CompletionModelCodegen.py
  clang-tools-extra/clangd/for-review-only/CompletionModel.cpp
  clang-tools-extra/clangd/for-review-only/CompletionModel.h
  clang-tools-extra/clangd/for-review-only/DecisionForestRuntimeTest.cpp
  clang-tools-extra/clangd/for-review-only/DecisionForestRuntimeTest.h
  clang-tools-extra/clangd/model/features.json
  clang-tools-extra/clangd/model/forest.json
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/DecisionForestTests.cpp
  clang-tools-extra/clangd/unittests/model/CategoricalFeature.h
  clang-tools-extra/clangd/unittests/model/features.json
  clang-tools-extra/clangd/unittests/model/forest.json

Index: clang-tools-extra/clangd/unittests/model/forest.json
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/model/forest.json
@@ -0,0 +1,52 @@
+[
+{
+"operation": "if_greater",
+"feature": "ANumber",
+"threshold": 200.0,
+"then": {
+"operation": "if_greater",
+"feature": "AFloat",
+"threshold": -1,
+"then": {
+"operation": "boost",
+"score": 10.0
+},
+"else": {
+"operation": "boost",
+"score": -20.0
+}
+},
+"else": {
+"operation": "if_member",
+"feature": "ACategorical",
+"set": [
+"A",
+"C"
+],
+"then": {
+"operation": "boost",
+"score": 3.0
+},
+"else": {
+"operation": "boost",
+"score": -4.0
+}
+}
+},
+{
+"operation": "if_member",
+"feature": "ACategorical",
+"set": [
+"A",
+"B"
+],
+"then": {
+"operation": "boost",
+"score": 5.0
+},
+"else": {
+"operation": "boost",
+"score": -6.0
+}
+}
+]
\ No newline at end of file
Index: clang-tools-extra/clangd/unittests/model/features.json
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/model/features.json
@@ -0,0 +1,16 @@
+[
+{
+"name": "ANumber",
+"type": "NUMERICAL"
+},
+{
+"name": "AFloat",
+"type": "NUMERICAL"
+},
+{
+"name": "ACategorical",
+"type": "CATEGORICAL",
+"enum": "ns1::ns2::TestEnum",
+"header": "model/CategoricalFeature.h"
+}
+]
\ No newline at end of file
Index: clang-tools-extra/clangd/unittests/model/CategoricalFeature.h
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/model/CategoricalFeature.h
@@ -0,0 +1,5 @@
+namespace ns1 {
+namespace ns2 {
+enum TestEnum { A, B, C, D };
+} // namespace ns2
+} // namespace ns1
Index: clang-tools-extra/clangd/unittests/DecisionForestTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/DecisionForestTests.cpp
@@ -0,0 +1,29 @@
+#include "DecisionForestRuntimeTest.h"
+#include "model/CategoricalFeature.h"
+#include "gtest/gtest.h"
+
+namespace clangd {
+namespace clangd {
+
+TEST(DecisionForestRuntime, Evaluate) {
+  using Example = ::ns1::ns2::test::Example;
+  using Cat = ::ns1::ns2::TestEnum;
+  using ::ns1::ns2::test::Evaluate;
+
+  Example E;
+  E.SetANumber(200); // True
+  E.SetAFloat(0);// True: +10.0
+  E.SetACategorical(Cat::A); // True: +5.0
+  EXPECT_EQ(Evaluate(E), 15.0);
+
+  E.SetANumber(200); // True
+  E.SetAFloat(-2.5); // False: -20.0
+  E.SetACategorical(Cat::B); // True: +5.0
+  EXPECT_EQ(Evaluate(E), -15.0);
+
+  E.SetANumber(100); // False
+  E.SetACategorical(Cat::C); // True: +3.0, False: -6.0
+  EXPECT_EQ(Evaluate(E), -3.0);
+}
+} // namespace clangd
+} // namespace clangd
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -10,6 +10,7 @@
 #include "ClangdServer.h"
 #include "CodeComplete.h"
 #include "Compiler.h"
+#include "CompletionModel.h"
 #include "Matchers.h"
 #include "Protocol.h"

[PATCH] D84409: [libTooling] Add an `EditGenerator` that applies a rule throughout a bound node.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 280458.
ymandel added a comment.

addressed comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84409

Files:
  clang/include/clang/Tooling/Transformer/RewriteRule.h
  clang/lib/Tooling/Transformer/RewriteRule.cpp
  clang/lib/Tooling/Transformer/Transformer.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -114,7 +114,9 @@
   if (C) {
 Changes.push_back(std::move(*C));
   } else {
-consumeError(C.takeError());
+// FIXME: stash this error rather then printing.
+llvm::errs() << "Error generating changes: "
+ << llvm::toString(C.takeError()) << "\n";
 ++ErrorCount;
   }
 };
@@ -414,6 +416,105 @@
Input, Expected);
 }
 
+// Rewrite various Stmts inside a Decl.
+TEST_F(TransformerTest, RewriteDescendantsDeclChangeStmt) {
+  std::string Input =
+  "int f(int x) { int y = x; { int z = x * x; } return x; }";
+  std::string Expected =
+  "int f(int x) { int y = 3; { int z = 3 * 3; } return 3; }";
+  auto InlineX =
+  makeRule(declRefExpr(to(varDecl(hasName("x", changeTo(cat("3")));
+  testRule(makeRule(functionDecl(hasName("f")).bind("fun"),
+rewriteDescendants("fun", InlineX)),
+   Input, Expected);
+}
+
+// Rewrite various TypeLocs inside a Decl.
+TEST_F(TransformerTest, RewriteDescendantsDeclChangeTypeLoc) {
+  std::string Input = "int f(int *x) { return *x; }";
+  std::string Expected = "char f(char *x) { return *x; }";
+  auto IntToChar = makeRule(typeLoc(loc(qualType(isInteger(), builtinType(,
+changeTo(cat("char")));
+  testRule(makeRule(functionDecl(hasName("f")).bind("fun"),
+rewriteDescendants("fun", IntToChar)),
+   Input, Expected);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsStmt) {
+  // Add an unrelated definition to the header that also has a variable named
+  // "x", to test that the rewrite is limited to the scope we intend.
+  appendToHeader(R"cc(int g(int x) { return x; })cc");
+  std::string Input =
+  "int f(int x) { int y = x; { int z = x * x; } return x; }";
+  std::string Expected =
+  "int f(int x) { int y = 3; { int z = 3 * 3; } return 3; }";
+  auto InlineX =
+  makeRule(declRefExpr(to(varDecl(hasName("x", changeTo(cat("3")));
+  testRule(makeRule(functionDecl(hasName("f"), hasBody(stmt().bind("body"))),
+rewriteDescendants("body", InlineX)),
+   Input, Expected);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsTypeLoc) {
+  std::string Input = "int f(int *x) { return *x; }";
+  std::string Expected = "int f(char *x) { return *x; }";
+  auto IntToChar =
+  makeRule(typeLoc(loc(qualType(isInteger(), builtinType(.bind("loc"),
+   changeTo(cat("char")));
+  testRule(
+  makeRule(functionDecl(hasName("f"),
+hasParameter(0, varDecl(hasTypeLoc(
+typeLoc().bind("parmType"),
+   rewriteDescendants("parmType", IntToChar)),
+  Input, Expected);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsReferToParentBinding) {
+  std::string Input =
+  "int f(int p) { int y = p; { int z = p * p; } return p; }";
+  std::string Expected =
+  "int f(int p) { int y = 3; { int z = 3 * 3; } return 3; }";
+  std::string VarId = "var";
+  auto InlineVar = makeRule(declRefExpr(to(varDecl(equalsBoundNode(VarId,
+changeTo(cat("3")));
+  testRule(makeRule(functionDecl(hasName("f"),
+ hasParameter(0, varDecl().bind(VarId)))
+.bind("fun"),
+rewriteDescendants("fun", InlineVar)),
+   Input, Expected);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsUnboundNode) {
+  std::string Input =
+  "int f(int x) { int y = x; { int z = x * x; } return x; }";
+  auto InlineX =
+  makeRule(declRefExpr(to(varDecl(hasName("x", changeTo(cat("3")));
+  Transformer T(makeRule(functionDecl(hasName("f")),
+ rewriteDescendants("UNBOUND", InlineX)),
+consumer());
+  T.registerMatchers(&MatchFinder);
+  EXPECT_FALSE(rewrite(Input));
+  EXPECT_THAT(Changes, IsEmpty());
+  EXPECT_EQ(ErrorCount, 1);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsInvalidNodeType) {
+  std::string Input =
+  "int f(int x) { int y = x; { int z = x * x; } return x; }";
+  auto IntToChar =
+  makeRule(qualType(isInteger(), builtinType()), changeTo(cat("char")));
+  Transformer T(
+  makeRule(functionDecl(
+   hasName("f"),
+   hasParameter(0, varDecl(hasType(qu

[PATCH] D84409: [libTooling] Add an `EditGenerator` that applies a rule throughout a bound node.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel marked 7 inline comments as done.
ymandel added inline comments.



Comment at: clang/include/clang/Tooling/Transformer/RewriteRule.h:337
+// refer to nodes bound by the calling rule. `Rule` is not applied to the node
+// itself.
+EditGenerator rewriteDescendants(std::string NodeId, RewriteRule Rule);

gribozavr2 wrote:
> Please use three slashes for doc comments.
Done, thanks for catching! Also, added example.



Comment at: clang/lib/Tooling/Transformer/RewriteRule.cpp:194
+for (auto &Matcher : transformer::detail::buildMatchers(Rule))
+  MF->addMatcher(forEachDescendantDynamically(std::move(Nodes), 
Matcher),
+ this);

gribozavr2 wrote:
> Wouldn't this line move the same value multiple times?
good catch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84409



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

[PATCH] D84409: [libTooling] Add an `EditGenerator` that applies a rule throughout a bound node.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
ymandel marked 2 inline comments as done.
Closed by commit rGc332a984aefc: [libTooling] Add an `EditGenerator` that 
applies a rule throughout a bound node. (authored by ymandel).

Changed prior to commit:
  https://reviews.llvm.org/D84409?vs=280458&id=280463#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84409

Files:
  clang/include/clang/Tooling/Transformer/RewriteRule.h
  clang/lib/Tooling/Transformer/RewriteRule.cpp
  clang/lib/Tooling/Transformer/Transformer.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -114,7 +114,9 @@
   if (C) {
 Changes.push_back(std::move(*C));
   } else {
-consumeError(C.takeError());
+// FIXME: stash this error rather then printing.
+llvm::errs() << "Error generating changes: "
+ << llvm::toString(C.takeError()) << "\n";
 ++ErrorCount;
   }
 };
@@ -414,6 +416,120 @@
Input, Expected);
 }
 
+// Rewrite various Stmts inside a Decl.
+TEST_F(TransformerTest, RewriteDescendantsDeclChangeStmt) {
+  std::string Input =
+  "int f(int x) { int y = x; { int z = x * x; } return x; }";
+  std::string Expected =
+  "int f(int x) { int y = 3; { int z = 3 * 3; } return 3; }";
+  auto InlineX =
+  makeRule(declRefExpr(to(varDecl(hasName("x", changeTo(cat("3")));
+  testRule(makeRule(functionDecl(hasName("f")).bind("fun"),
+rewriteDescendants("fun", InlineX)),
+   Input, Expected);
+}
+
+// Rewrite various TypeLocs inside a Decl.
+TEST_F(TransformerTest, RewriteDescendantsDeclChangeTypeLoc) {
+  std::string Input = "int f(int *x) { return *x; }";
+  std::string Expected = "char f(char *x) { return *x; }";
+  auto IntToChar = makeRule(typeLoc(loc(qualType(isInteger(), builtinType(,
+changeTo(cat("char")));
+  testRule(makeRule(functionDecl(hasName("f")).bind("fun"),
+rewriteDescendants("fun", IntToChar)),
+   Input, Expected);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsStmt) {
+  // Add an unrelated definition to the header that also has a variable named
+  // "x", to test that the rewrite is limited to the scope we intend.
+  appendToHeader(R"cc(int g(int x) { return x; })cc");
+  std::string Input =
+  "int f(int x) { int y = x; { int z = x * x; } return x; }";
+  std::string Expected =
+  "int f(int x) { int y = 3; { int z = 3 * 3; } return 3; }";
+  auto InlineX =
+  makeRule(declRefExpr(to(varDecl(hasName("x", changeTo(cat("3")));
+  testRule(makeRule(functionDecl(hasName("f"), hasBody(stmt().bind("body"))),
+rewriteDescendants("body", InlineX)),
+   Input, Expected);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsStmtWithAdditionalChange) {
+  std::string Input =
+  "int f(int x) { int y = x; { int z = x * x; } return x; }";
+  std::string Expected =
+  "int newName(int x) { int y = 3; { int z = 3 * 3; } return 3; }";
+  auto InlineX =
+  makeRule(declRefExpr(to(varDecl(hasName("x", changeTo(cat("3")));
+  testRule(
+  makeRule(
+  functionDecl(hasName("f"), hasBody(stmt().bind("body"))).bind("f"),
+  flatten(changeTo(name("f"), cat("newName")),
+  rewriteDescendants("body", InlineX))),
+  Input, Expected);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsTypeLoc) {
+  std::string Input = "int f(int *x) { return *x; }";
+  std::string Expected = "int f(char *x) { return *x; }";
+  auto IntToChar =
+  makeRule(typeLoc(loc(qualType(isInteger(), builtinType(.bind("loc"),
+   changeTo(cat("char")));
+  testRule(
+  makeRule(functionDecl(hasName("f"),
+hasParameter(0, varDecl(hasTypeLoc(
+typeLoc().bind("parmType"),
+   rewriteDescendants("parmType", IntToChar)),
+  Input, Expected);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsReferToParentBinding) {
+  std::string Input =
+  "int f(int p) { int y = p; { int z = p * p; } return p; }";
+  std::string Expected =
+  "int f(int p) { int y = 3; { int z = 3 * 3; } return 3; }";
+  std::string VarId = "var";
+  auto InlineVar = makeRule(declRefExpr(to(varDecl(equalsBoundNode(VarId,
+changeTo(cat("3")));
+  testRule(makeRule(functionDecl(hasName("f"),
+ hasParameter(0, varDecl().bind(VarId)))
+.bind("fun"),
+rewriteDescendants("fun", InlineVar)),
+   Input, Expected);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsUnboundNode) {
+  std::s

[clang] c332a98 - [libTooling] Add an `EditGenerator` that applies a rule throughout a bound node.

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2020-07-24T14:38:17Z
New Revision: c332a984aefc6f8a6b44fd3687a5bbce3f8f035c

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

LOG: [libTooling] Add an `EditGenerator` that applies a rule throughout a bound 
node.

The new combinator, `rewriteDescendants`, applies a rewrite rule to all
descendants of a specified bound node.  That rewrite rule can refer to nodes
bound by the parent, both in the matcher and in the edits.

Reviewed By: gribozavr2

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

Added: 


Modified: 
clang/include/clang/Tooling/Transformer/RewriteRule.h
clang/lib/Tooling/Transformer/RewriteRule.cpp
clang/lib/Tooling/Transformer/Transformer.cpp
clang/unittests/Tooling/TransformerTest.cpp

Removed: 




diff  --git a/clang/include/clang/Tooling/Transformer/RewriteRule.h 
b/clang/include/clang/Tooling/Transformer/RewriteRule.h
index c22a2da81fe6..2a26d32817dd 100644
--- a/clang/include/clang/Tooling/Transformer/RewriteRule.h
+++ b/clang/include/clang/Tooling/Transformer/RewriteRule.h
@@ -332,6 +332,23 @@ inline EditGenerator shrinkTo(RangeSelector outer, 
RangeSelector inner) {
remove(enclose(after(inner), after(outer)))});
 }
 
+/// Applies `Rule` to all descendants of the node bound to `NodeId`. `Rule` can
+/// refer to nodes bound by the calling rule. `Rule` is not applied to the node
+/// itself.
+///
+/// For example,
+/// ```
+/// auto InlineX =
+/// makeRule(declRefExpr(to(varDecl(hasName("x", changeTo(cat("3")));
+/// makeRule(functionDecl(hasName("f"), 
hasBody(stmt().bind("body"))).bind("f"),
+///  flatten(
+///changeTo(name("f"), cat("newName")),
+///rewriteDescendants("body", InlineX)));
+/// ```
+/// Here, we find the function `f`, change its name to `newName` and change all
+/// appearances of `x` in its body to `3`.
+EditGenerator rewriteDescendants(std::string NodeId, RewriteRule Rule);
+
 /// The following three functions are a low-level part of the RewriteRule
 /// API. We expose them for use in implementing the fixtures that interpret
 /// RewriteRule, like Transformer and TransfomerTidy, or for more advanced

diff  --git a/clang/lib/Tooling/Transformer/RewriteRule.cpp 
b/clang/lib/Tooling/Transformer/RewriteRule.cpp
index c145895af7ab..ce773b59a7e7 100644
--- a/clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ b/clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -7,6 +7,8 @@
 
//===--===//
 
 #include "clang/Tooling/Transformer/RewriteRule.h"
+#include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Stmt.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Basic/SourceLocation.h"
@@ -115,15 +117,144 @@ ASTEdit transformer::remove(RangeSelector S) {
   return change(std::move(S), std::make_shared(""));
 }
 
-RewriteRule transformer::makeRule(ast_matchers::internal::DynTypedMatcher M,
-  EditGenerator Edits,
+RewriteRule transformer::makeRule(DynTypedMatcher M, EditGenerator Edits,
   TextGenerator Explanation) {
   return RewriteRule{{RewriteRule::Case{
   std::move(M), std::move(Edits), std::move(Explanation), {;
 }
 
+namespace {
+
+/// Unconditionally binds the given node set before trying `InnerMatcher` and
+/// keeps the bound nodes on a successful match.
+template 
+class BindingsMatcher : public ast_matchers::internal::MatcherInterface {
+  ast_matchers::BoundNodes Nodes;
+  const ast_matchers::internal::Matcher InnerMatcher;
+
+public:
+  explicit BindingsMatcher(ast_matchers::BoundNodes Nodes,
+   ast_matchers::internal::Matcher InnerMatcher)
+  : Nodes(std::move(Nodes)), InnerMatcher(std::move(InnerMatcher)) {}
+
+  bool matches(
+  const T &Node, ast_matchers::internal::ASTMatchFinder *Finder,
+  ast_matchers::internal::BoundNodesTreeBuilder *Builder) const override {
+ast_matchers::internal::BoundNodesTreeBuilder Result(*Builder);
+for (const auto &N : Nodes.getMap())
+  Result.setBinding(N.first, N.second);
+if (InnerMatcher.matches(Node, Finder, &Result)) {
+  *Builder = std::move(Result);
+  return true;
+}
+return false;
+  }
+};
+
+/// Matches nodes of type T that have at least one descendant node for which 
the
+/// given inner matcher matches.  Will match for each descendant node that
+/// matches.  Based on ForEachDescendantMatcher, but takes a dynamic matcher,
+/// instead of a static one, because it is used by RewriteRule, which carries
+/// (only top-level) dynamic matchers.
+template 
+class DynamicForEachDescendantMatcher
+: public ast_matchers::intern

[PATCH] D83817: [clangd] Add option to use remote index as static index

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:457
+"remote-index-address",
+cat(Remote),
+desc("Address of the remote index server"),

not sure whether we need a new category, probably can live in Features?



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:459
+desc("Address of the remote index server"),
+Hidden,
+};

since we hide this in the `CLANGD_ENABLE_REMOTE` flag, maybe remove the 
`Hidden`?



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:463
+// FIXME(kirillbobyrev): Should this be the location of compile_commands.json?
+opt ProjectPath{
+"project-path",

maybe `ProjectRoot`?



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:702
   }
+  if (RemoteIndexAddress.empty() != ProjectPath.empty()) {
+llvm::errs() << "remote-index-address and project-path have to be "

the new code section here should be guarded under `#ifdef CLANGD_ENABLE_REMOTE`



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:707
+  }
+  if (!RemoteIndexAddress.empty()) {
+if (!IndexFile.empty()) {

if remote-index is on, should we turn off the background index? I think this is 
our plan, right?



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:709
+if (!IndexFile.empty()) {
+  llvm::errs() << "When remote index is enabled, IndexFile should not be "
+  "specified. Only one can be used at time.";

nit: use elog.



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:711
+  "specified. Only one can be used at time.";
+  return 1;
+}

instead of exiting clangd, maybe just continue (since this is not a fatal 
error), and adjust the error message stating that index-file option is ignored.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83817



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

[PATCH] D83909: [OPENMP] Fix PR46730: Fix compiler crash on taskloop over constructible loop counters.

[Alexey Bataev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9840208db698: [OPENMP] Fix PR46730: Fix compiler crash on 
taskloop over constructible loop… (authored by ABataev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83909

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/taskloop_codegen.cpp


Index: clang/test/OpenMP/taskloop_codegen.cpp
===
--- clang/test/OpenMP/taskloop_codegen.cpp
+++ clang/test/OpenMP/taskloop_codegen.cpp
@@ -229,4 +229,20 @@
 // CHECK: br label %
 // CHECK: ret i32 0
 
+class St {
+public:
+  operator int();
+  St &operator+=(int);
+};
+
+// CHECK-LABEL: taskloop_with_class
+void taskloop_with_class() {
+  St s1;
+  // CHECK: [[TD:%.+]] = call i8* @__kmpc_omp_task_alloc(%struct.ident_t* 
@{{.+}}, i32 [[GTID:%.+]], i32 1, i64 88, i64 8, i32 (i32, i8*)* bitcast (i32 
(i32, [[TD_TYPE:%.+]]*)* @{{.+}} to i32 (i32, i8*)*))
+  // CHECK: call void @__kmpc_taskloop(%struct.ident_t* @{{.+}}, i32 [[GTID]], 
i8* [[TD]], i32 1, i64* %{{.+}}, i64* %{{.+}}, i64 %{{.+}}, i32 1, i32 0, i64 
0, i8* bitcast (void ([[TD_TYPE]]*, [[TD_TYPE]]*, i32)* @{{.+}} to i8*))
+#pragma omp taskloop
+  for (St s = St(); s < s1; s += 1) {
+  }
+}
+
 #endif
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -2244,7 +2244,11 @@
   [](OpenMPDirectiveKind K) { return isOpenMPTaskingDirective(K); },
   Level)) {
 bool IsTriviallyCopyable =
-D->getType().getNonReferenceType().isTriviallyCopyableType(Context);
+D->getType().getNonReferenceType().isTriviallyCopyableType(Context) &&
+!D->getType()
+ .getNonReferenceType()
+ .getCanonicalType()
+ ->getAsCXXRecordDecl();
 OpenMPDirectiveKind DKind = DSAStack->getDirective(Level);
 SmallVector CaptureRegions;
 getOpenMPCaptureRegions(CaptureRegions, DKind);


Index: clang/test/OpenMP/taskloop_codegen.cpp
===
--- clang/test/OpenMP/taskloop_codegen.cpp
+++ clang/test/OpenMP/taskloop_codegen.cpp
@@ -229,4 +229,20 @@
 // CHECK: br label %
 // CHECK: ret i32 0
 
+class St {
+public:
+  operator int();
+  St &operator+=(int);
+};
+
+// CHECK-LABEL: taskloop_with_class
+void taskloop_with_class() {
+  St s1;
+  // CHECK: [[TD:%.+]] = call i8* @__kmpc_omp_task_alloc(%struct.ident_t* @{{.+}}, i32 [[GTID:%.+]], i32 1, i64 88, i64 8, i32 (i32, i8*)* bitcast (i32 (i32, [[TD_TYPE:%.+]]*)* @{{.+}} to i32 (i32, i8*)*))
+  // CHECK: call void @__kmpc_taskloop(%struct.ident_t* @{{.+}}, i32 [[GTID]], i8* [[TD]], i32 1, i64* %{{.+}}, i64* %{{.+}}, i64 %{{.+}}, i32 1, i32 0, i64 0, i8* bitcast (void ([[TD_TYPE]]*, [[TD_TYPE]]*, i32)* @{{.+}} to i8*))
+#pragma omp taskloop
+  for (St s = St(); s < s1; s += 1) {
+  }
+}
+
 #endif
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -2244,7 +2244,11 @@
   [](OpenMPDirectiveKind K) { return isOpenMPTaskingDirective(K); },
   Level)) {
 bool IsTriviallyCopyable =
-D->getType().getNonReferenceType().isTriviallyCopyableType(Context);
+D->getType().getNonReferenceType().isTriviallyCopyableType(Context) &&
+!D->getType()
+ .getNonReferenceType()
+ .getCanonicalType()
+ ->getAsCXXRecordDecl();
 OpenMPDirectiveKind DKind = DSAStack->getDirective(Level);
 SmallVector CaptureRegions;
 getOpenMPCaptureRegions(CaptureRegions, DKind);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 9840208 - [OPENMP] Fix PR46730: Fix compiler crash on taskloop over constructible loop counters.

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2020-07-24T10:48:20-04:00
New Revision: 9840208db6980f690d09b209e6ad6d57133ec5e5

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

LOG: [OPENMP] Fix PR46730: Fix compiler crash on taskloop over constructible 
loop counters.

Summary:
If the variable is constrcutible, its copy is created by calling a
constructor. Such variables are duplicated and thus, must be captured.

Reviewers: jdoerfert

Subscribers: yaxunl, guansong, cfe-commits, sstefan1, caomhin

Tags: #clang

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

Added: 


Modified: 
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/taskloop_codegen.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 51609e37e20c..0192df3bd170 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -2244,7 +2244,11 @@ OpenMPClauseKind Sema::isOpenMPPrivateDecl(ValueDecl *D, 
unsigned Level,
   [](OpenMPDirectiveKind K) { return isOpenMPTaskingDirective(K); },
   Level)) {
 bool IsTriviallyCopyable =
-D->getType().getNonReferenceType().isTriviallyCopyableType(Context);
+D->getType().getNonReferenceType().isTriviallyCopyableType(Context) &&
+!D->getType()
+ .getNonReferenceType()
+ .getCanonicalType()
+ ->getAsCXXRecordDecl();
 OpenMPDirectiveKind DKind = DSAStack->getDirective(Level);
 SmallVector CaptureRegions;
 getOpenMPCaptureRegions(CaptureRegions, DKind);

diff  --git a/clang/test/OpenMP/taskloop_codegen.cpp 
b/clang/test/OpenMP/taskloop_codegen.cpp
index 55e43ff3a115..7402c2ad65eb 100644
--- a/clang/test/OpenMP/taskloop_codegen.cpp
+++ b/clang/test/OpenMP/taskloop_codegen.cpp
@@ -229,4 +229,20 @@ struct S {
 // CHECK: br label %
 // CHECK: ret i32 0
 
+class St {
+public:
+  operator int();
+  St &operator+=(int);
+};
+
+// CHECK-LABEL: taskloop_with_class
+void taskloop_with_class() {
+  St s1;
+  // CHECK: [[TD:%.+]] = call i8* @__kmpc_omp_task_alloc(%struct.ident_t* 
@{{.+}}, i32 [[GTID:%.+]], i32 1, i64 88, i64 8, i32 (i32, i8*)* bitcast (i32 
(i32, [[TD_TYPE:%.+]]*)* @{{.+}} to i32 (i32, i8*)*))
+  // CHECK: call void @__kmpc_taskloop(%struct.ident_t* @{{.+}}, i32 [[GTID]], 
i8* [[TD]], i32 1, i64* %{{.+}}, i64* %{{.+}}, i64 %{{.+}}, i32 1, i32 0, i64 
0, i8* bitcast (void ([[TD_TYPE]]*, [[TD_TYPE]]*, i32)* @{{.+}} to i8*))
+#pragma omp taskloop
+  for (St s = St(); s < s1; s += 1) {
+  }
+}
+
 #endif



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

[PATCH] D84005: Introduce ns_error_domain attribute.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D84005#2171962 , @MForster wrote:

> In D84005#2162433 , @aaron.ballman 
> wrote:
>
> > It's a bit odd that this attribute has an AST node created for it but 
> > nothing is using that AST node elsewhere in the project. Are there other 
> > patches expected for making use of this attribute?
>
>
> It gets used from Swift.


Are there plans to use this attribute in Clang itself? If not, is there a way 
Swift can use attribute plugins instead of modifying Clang?




Comment at: clang/include/clang/Basic/AttrDocs.td:3330
+
+For example:
+

MForster wrote:
> aaron.ballman wrote:
> > I still feel like I'm lacking information about the domain. In the example, 
> > the domain is an uninitialized `const char *`, so how does this information 
> > get used by the attribute? Can I use any type of global I want? What about 
> > a literal?
> I won't claim that I understand this well. Maybe @gribozavr2 or @milseman can 
> explain this better. Literals are not allowed, however. The test explicitly 
> forbids that.
> Literals are not allowed, however. The test explicitly forbids that.

And I'm wondering why. For instance, attributes with enumeration arguments 
accept the enumerator as either an identifier or a string literal. I was sort 
of assuming that these domains are notionally like enumerators in that there is 
a list of domains, which is why I was asking about literals.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9454
+  "domain argument %0 does not refer to global constant">;
+def err_nserrordomain_requires_identifier : Error<
+  "domain argument must be an identifier">;

MForster wrote:
> aaron.ballman wrote:
> > I don't think this requires a custom diagnostic -- we can use 
> > `err_attribute_argument_n_type` instead.
> > I don't think this requires a custom diagnostic -- we can use 
> > `err_attribute_argument_n_type` instead.
> 
> 
> Done. Can you please confirm that `n` is 1-based?
Yes, the argument positions are 1-based.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5336
+  }
+  IdentifierLoc *identLoc =
+  Attr.isArgIdent(0) ? Attr.getArgAsIdent(0) : nullptr;

aaron.ballman wrote:
> Variables should match the usual coding style conventions.
It looks like `loc` was missed and still need to be updated to `Loc`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84005



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

[PATCH] D84499: [clangd] Add more logs and attach tracers to remote index server routines

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/index/remote/Client.cpp:61
   Callback(*Response);
+  ++Received;
 }

shouldn't we increment this before the `continue` above ? (or rename this to 
`successful` rather than `received` ?)



Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:42
 
+llvm::cl::opt TracerFile(
+"tracer-file",

i think `TraceFile` is more appropriate here.



Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:48
+"pretty",
+llvm::cl::desc("Pretty-print JSON output"),
+llvm::cl::init(true),

is it only for pretty printing trace files? do we expect to have any other 
components this will interact in future? (if not maybe state that in the 
comments)



Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:49
+llvm::cl::desc("Pretty-print JSON output"),
+llvm::cl::init(true),
+};

this sounds like a debug feature, do we really want it to be true by default?



Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:122
 }
-Index->lookup(Req, [&](const clangd::Symbol &Sym) {
-  auto SerializedSymbol = ProtobufMarshaller->toProtobuf(Sym);
-  if (!SerializedSymbol)
-return;
-  LookupReply NextMessage;
-  *NextMessage.mutable_stream_result() = *SerializedSymbol;
-  Reply->Write(NextMessage);
-});
+std::function
void IndexCallback(Marshaller &M, StreamType &Item) {
  trace::Span Tracer;
  auto SerializedSymbol = M.toProtobuf(Item);
  // log the events and much more ...
}

```

then call it in the callbacks. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84499



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

[PATCH] D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist

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

Szelethus added a comment.

Thank you so much, you really went out of your way to dig that out! I'll try to 
patch this somehow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598



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

[PATCH] D82122: [analyzer][Liveness][RFC][NFC] Propose to turn statement liveness into expression liveness

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

Szelethus added a comment.

This was accidentally committed as a part of 
rGb6cbe6cb0399d4671e5384dcc326af56bc6bd122 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82122



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

[PATCH] D84525: [clangd] Add marshalling code for all request types

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp:53
 
-clangd::FuzzyFindRequest
-Marshaller::fromProtobuf(const FuzzyFindRequest *Request) {
+namespace {
+template 

nit: move anon namespace to the top



Comment at: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp:55
+template 
+llvm::Optional> getIDs(MessageT *Message) {
+  llvm::DenseSet Result;

I would make this return an expected instead of an optional and print the error 
in the callers, instead of logging twice.

(I also think a similar problem is present with the existing `fromProtobuf` 
APIs too, they return None in case of failure while logging the error, and then 
callers again log the error. I am not sure if we plan to implement some error 
handling strategies but even if we did, it feels like Marshalling is the lowest 
layer that wouldn't know any other strategy to handle such errors, so it seems 
like always returning an error from the marshalling and letting the caller 
handle the error sounds like a safe bet. But no action needed in this patch 
just stating some ideas in case you find them useful :D)



Comment at: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp:292
   auto Deserialized = ProtobufMarshaller.fromProtobuf(&Serialized);
-  EXPECT_THAT(Deserialized.ProximityPaths,
+  EXPECT_TRUE(Deserialized);
+  EXPECT_THAT(Deserialized->ProximityPaths,

`ASSERT_TRUE` to ensure we don't try to dereference in case of None.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84525



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

[PATCH] D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist

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

Szelethus added a comment.

Wow, I never realized I accidentally landed that assert (D82122#2172360 
), but I guess its great to have that 
covered. Would you prefer to have that reverted as I'm looking to fix this for 
good?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598



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

[PATCH] D84511: Fix update_cc_test_checks.py --llvm-bin after D78478

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/utils/update_cc_test_checks/lit.local.cfg:24
+# works as expected
+config.substitutions.append(
+('%update_cc_test_checks_llvm_bin', "%s %s %s" % (

The substitution is here just to make a test. Can it be tested with existing 
substitutions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84511



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

[PATCH] D84356: [AIX] remove -u from the clang when invoke aix as assembler

[Digger via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG77b61177d7d4: [AIX] remove -u from the clang when invoke aix 
as assembler (authored by DiggerLin).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84356

Files:
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/test/Driver/aix-as.c


Index: clang/test/Driver/aix-as.c
===
--- clang/test/Driver/aix-as.c
+++ clang/test/Driver/aix-as.c
@@ -9,7 +9,6 @@
 // CHECK-AS32: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"powerpc-ibm-aix7.1.0.0"
 // CHECK-AS32: "{{.*}}as{{(.exe)?}}" 
 // CHECK-AS32: "-a32" 
-// CHECK-AS32: "-u" 
 // CHECK-AS32: "-many" 
 
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit.
@@ -20,7 +19,6 @@
 // CHECK-AS64: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"powerpc64-ibm-aix7.1.0.0"
 // CHECK-AS64: "{{.*}}as{{(.exe)?}}" 
 // CHECK-AS64: "-a64" 
-// CHECK-AS64: "-u" 
 // CHECK-AS64: "-many"
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit. -Xassembler  option. 
@@ -32,7 +30,6 @@
 // CHECK-AS32-Xassembler: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"powerpc-ibm-aix7.1.0.0"
 // CHECK-AS32-Xassembler: "{{.*}}as{{(.exe)?}}" 
 // CHECK-AS32-Xassembler: "-a32" 
-// CHECK-AS32-Xassembler: "-u" 
 // CHECK-AS32-Xassembler: "-many"
 // CHECK-AS32-Xassembler: "-w"
 
@@ -45,7 +42,6 @@
 // CHECK-AS64-Wa: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"powerpc64-ibm-aix7.1.0.0"
 // CHECK-AS64-Wa: "{{.*}}as{{(.exe)?}}" 
 // CHECK-AS64-Wa: "-a64" 
-// CHECK-AS64-Wa: "-u" 
 // CHECK-AS64-Wa: "-many"
 // CHECK-AS64-Wa: "-v"
 // CHECK-AS64-Wa: "-w"
@@ -60,13 +56,10 @@
 // CHECK-AS32-MultiInput-NOT: warning:
 // CHECK-AS32-MultiInput: "{{.*}}as{{(.exe)?}}"
 // CHECK-AS32-MultiInput: "-a32"
-// CHECK-AS32-MultiInput: "-u"
 // CHECK-AS32-MultiInput: "-many"
 // CHECK-AS32-MultiInput: "{{.*}}as{{(.exe)?}}"
 // CHECK-AS32-MultiInput: "-a32"
-// CHECK-AS32-MultiInput: "-u"
 // CHECK-AS32-MultiInput: "-many"
 // CHECK-AS32-MultiInput: "{{.*}}as{{(.exe)?}}"
 // CHECK-AS32-MultiInput: "-a32"
-// CHECK-AS32-MultiInput: "-u"
 // CHECK-AS32-MultiInput: "-many"
Index: clang/lib/Driver/ToolChains/AIX.cpp
===
--- clang/lib/Driver/ToolChains/AIX.cpp
+++ clang/lib/Driver/ToolChains/AIX.cpp
@@ -44,12 +44,6 @@
 CmdArgs.push_back("-a64");
   }
 
-  // Accept an undefined symbol as an extern so that an error message is not
-  // displayed. Otherwise, undefined symbols are flagged with error messages.
-  // FIXME: This should be removed when the assembly generation from the
-  // compiler is able to write externs properly.
-  CmdArgs.push_back("-u");
-
   // Accept any mixture of instructions.
   // On Power for AIX and Linux, this behaviour matches that of GCC for both 
the
   // user-provided assembler source case and the compiler-produced assembler


Index: clang/test/Driver/aix-as.c
===
--- clang/test/Driver/aix-as.c
+++ clang/test/Driver/aix-as.c
@@ -9,7 +9,6 @@
 // CHECK-AS32: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
 // CHECK-AS32: "{{.*}}as{{(.exe)?}}" 
 // CHECK-AS32: "-a32" 
-// CHECK-AS32: "-u" 
 // CHECK-AS32: "-many" 
 
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit.
@@ -20,7 +19,6 @@
 // CHECK-AS64: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc64-ibm-aix7.1.0.0"
 // CHECK-AS64: "{{.*}}as{{(.exe)?}}" 
 // CHECK-AS64: "-a64" 
-// CHECK-AS64: "-u" 
 // CHECK-AS64: "-many"
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit. -Xassembler  option. 
@@ -32,7 +30,6 @@
 // CHECK-AS32-Xassembler: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
 // CHECK-AS32-Xassembler: "{{.*}}as{{(.exe)?}}" 
 // CHECK-AS32-Xassembler: "-a32" 
-// CHECK-AS32-Xassembler: "-u" 
 // CHECK-AS32-Xassembler: "-many"
 // CHECK-AS32-Xassembler: "-w"
 
@@ -45,7 +42,6 @@
 // CHECK-AS64-Wa: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc64-ibm-aix7.1.0.0"
 // CHECK-AS64-Wa: "{{.*}}as{{(.exe)?}}" 
 // CHECK-AS64-Wa: "-a64" 
-// CHECK-AS64-Wa: "-u" 
 // CHECK-AS64-Wa: "-many"
 // CHECK-AS64-Wa: "-v"
 // CHECK-AS64-Wa: "-w"
@@ -60,13 +56,10 @@
 // CHECK-AS32-MultiInput-NOT: warning:
 // CHECK-AS32-MultiInput: "{{.*}}as{{(.exe)?}}"
 // CHECK-AS32-MultiInput: "-a32"
-// CHECK-AS32-MultiInput: "-u"
 // CHECK-AS32-MultiInput: "-many"
 // CHECK-AS32-MultiInput: "{{.*}}as{{(.exe)?}}"
 // CHECK-AS32-MultiInput: "-a32"
-// CHECK-AS32-MultiInput: "-u"
 // CHECK-AS32-MultiInput: "-many"
 // CHECK-AS32-MultiInput: "{{.*}}as{{(.exe)?}}"
 // CHECK-AS32-MultiInput: "-a32"
-// CHECK-AS32-MultiInput: "-u"
 // CHECK-AS32-MultiInput: "-many"
Index: clang/lib/Driver/ToolChains/AIX.cpp
===
--- clang/lib/Driver/ToolChains/AIX.cpp
+++ clang/lib/Driver/ToolChains/AIX.cpp
@@ -44,12 +44,6 @@
 CmdArgs.push_back("-a64");
   }
 
-  // Accept an und

[clang] 77b6117 - [AIX] remove -u from the clang when invoke aix as assembler

[via cfe-commits]

Author: diggerlin
Date: 2020-07-24T11:28:17-04:00
New Revision: 77b61177d7d4c4fe8714f8828123f626f4549be1

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

LOG: [AIX] remove -u from the clang when invoke aix as assembler

SUMMARY:

since we add .extern directive for external symbol, the -u option for aix as do 
not need any more.

Reviewers:  Jason liu

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/AIX.cpp
clang/test/Driver/aix-as.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/AIX.cpp 
b/clang/lib/Driver/ToolChains/AIX.cpp
index ac5544eedb00..f9d8e18d6fd0 100644
--- a/clang/lib/Driver/ToolChains/AIX.cpp
+++ b/clang/lib/Driver/ToolChains/AIX.cpp
@@ -44,12 +44,6 @@ void aix::Assembler::ConstructJob(Compilation &C, const 
JobAction &JA,
 CmdArgs.push_back("-a64");
   }
 
-  // Accept an undefined symbol as an extern so that an error message is not
-  // displayed. Otherwise, undefined symbols are flagged with error messages.
-  // FIXME: This should be removed when the assembly generation from the
-  // compiler is able to write externs properly.
-  CmdArgs.push_back("-u");
-
   // Accept any mixture of instructions.
   // On Power for AIX and Linux, this behaviour matches that of GCC for both 
the
   // user-provided assembler source case and the compiler-produced assembler

diff  --git a/clang/test/Driver/aix-as.c b/clang/test/Driver/aix-as.c
index cb3053f5acd3..aa8c61035903 100644
--- a/clang/test/Driver/aix-as.c
+++ b/clang/test/Driver/aix-as.c
@@ -9,7 +9,6 @@
 // CHECK-AS32: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"powerpc-ibm-aix7.1.0.0"
 // CHECK-AS32: "{{.*}}as{{(.exe)?}}" 
 // CHECK-AS32: "-a32" 
-// CHECK-AS32: "-u" 
 // CHECK-AS32: "-many" 
 
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit.
@@ -20,7 +19,6 @@
 // CHECK-AS64: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"powerpc64-ibm-aix7.1.0.0"
 // CHECK-AS64: "{{.*}}as{{(.exe)?}}" 
 // CHECK-AS64: "-a64" 
-// CHECK-AS64: "-u" 
 // CHECK-AS64: "-many"
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit. -Xassembler  option. 
@@ -32,7 +30,6 @@
 // CHECK-AS32-Xassembler: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"powerpc-ibm-aix7.1.0.0"
 // CHECK-AS32-Xassembler: "{{.*}}as{{(.exe)?}}" 
 // CHECK-AS32-Xassembler: "-a32" 
-// CHECK-AS32-Xassembler: "-u" 
 // CHECK-AS32-Xassembler: "-many"
 // CHECK-AS32-Xassembler: "-w"
 
@@ -45,7 +42,6 @@
 // CHECK-AS64-Wa: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"powerpc64-ibm-aix7.1.0.0"
 // CHECK-AS64-Wa: "{{.*}}as{{(.exe)?}}" 
 // CHECK-AS64-Wa: "-a64" 
-// CHECK-AS64-Wa: "-u" 
 // CHECK-AS64-Wa: "-many"
 // CHECK-AS64-Wa: "-v"
 // CHECK-AS64-Wa: "-w"
@@ -60,13 +56,10 @@
 // CHECK-AS32-MultiInput-NOT: warning:
 // CHECK-AS32-MultiInput: "{{.*}}as{{(.exe)?}}"
 // CHECK-AS32-MultiInput: "-a32"
-// CHECK-AS32-MultiInput: "-u"
 // CHECK-AS32-MultiInput: "-many"
 // CHECK-AS32-MultiInput: "{{.*}}as{{(.exe)?}}"
 // CHECK-AS32-MultiInput: "-a32"
-// CHECK-AS32-MultiInput: "-u"
 // CHECK-AS32-MultiInput: "-many"
 // CHECK-AS32-MultiInput: "{{.*}}as{{(.exe)?}}"
 // CHECK-AS32-MultiInput: "-a32"
-// CHECK-AS32-MultiInput: "-u"
 // CHECK-AS32-MultiInput: "-many"



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

[PATCH] D84534: [AIX] Static init frontend recovery and backend support

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L created this revision.
Xiangling_L added reviewers: jasonliu, hubert.reinterpretcast, yusra.syeda, 
zarko, xingxue.
Xiangling_L added a project: LLVM.
Herald added subscribers: llvm-commits, cfe-commits, jfb, kbarton, hiraditya, 
nemanjai.
Herald added a project: clang.

1. Frontend side
2. Recovered AIX static init frontend to use the linkage type and function 
names Clang chooses for sinit related function;
3. Removed the `GlobalUniqueModuleId` calculation and usage;
4. Adjusted the FE testcases accordingly;
5. Added one frontend testcase to demonstrate and validate separate 
initialization on AIX;

2. Backend side on the assembly path only
3. Set correct linkage and function names for sinit/sterm functions
4. Added testcases


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84534

Files:
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
  clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp
  clang/test/CodeGenCXX/aix-static-init.cpp
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
  llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
  llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
  llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll

Index: llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll
@@ -0,0 +1,10 @@
+; RUN: not llc -mtriple powerpc-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+; RUN: not llc -mtriple powerpc64-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 655, void ()* @foo, i8* null }]
+
+define void @foo() {
+  ret void
+}
+
+// CHECK: LLVM ERROR: prioritized sinit and sterm functions are not yet supported
Index: llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
@@ -0,0 +1,7 @@
+; RUN: not llc -mtriple powerpc-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+; RUN: not llc -mtriple powerpc64-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @foo, i8* null }]
+@llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @bar, i8* null }]
+
+// CHECK: LLVM ERROR: cannot produce a unique identifier for this module based on strong external symbols
Index: llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
@@ -0,0 +1,31 @@
+; RUN: llc -mtriple powerpc-ibm-aix-xcoff < %s | FileCheck %s
+; RUN: llc -mtriple powerpc64-ibm-aix-xcoff < %s | FileCheck %s
+
+@llvm.global_ctors = appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @init1, i8* null }, { i32, void ()*, i8* } { i32 65535, void ()* @init2, i8* null }]
+@llvm.global_dtors = appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @destruct1, i8* null }, { i32, void ()*, i8* } { i32 65535, void ()* @destruct2, i8* null }]
+
+define i32 @extFunc() {
+entry:
+  ret i32 3
+}
+
+define internal void @init1() {
+  ret void
+}
+
+define internal void @destruct1() {
+  ret void
+}
+
+define internal void @init2() {
+  ret void
+}
+
+define internal void @destruct2() {
+  ret void
+}
+
+; CHECK: .globl	.__sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_0
+; CHECK: .globl	.__sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_0
+; CHECK: .globl	.__sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_1
+; CHECK: .globl	.__sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_1
Index: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
===
--- llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
+++ llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
@@ -67,6 +67,7 @@
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetMachine.h"
+#include "llvm/Transforms/Utils/ModuleUtils.h"
 #include 
 #include 
 #include 
@@ -153,6 +154,9 @@
   /// linkage for them in AIX.
   SmallPtrSet ExtSymSDNodeSymbols;
 
+  /// A unique trailing identifier as a part of sinit/sterm functions.
+  std::string GlobalUniqueModuleId;
+
   static void ValidateGV(const GlobalVariable *GV);
   // Record a list of GlobalAlia

[PATCH] D84192: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

@ABataev, is there any other concern for this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84192



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

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

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



Comment at: llvm/include/llvm/CodeGen/MachineCfgTraits.h:44
+// use on a 32-bit architecture.
+assert(wrapped != (uintptr_t)-1 && wrapped != (uintptr_t)-2);
+

arsenm wrote:
> I feel like there should be a better way to do this; we should probably have 
> an assert where virtual registers are created
The reason for doing it here is that this is the place where the reinterpret 
happens. If the check is elsewhere, it's easy to miss by a user of this.



Comment at: llvm/include/llvm/CodeGen/MachineCfgTraits.h:101
+  return nullptr;
+return m_regInfo->getUniqueVRegDef(value)->getParent();
+  }

arsenm wrote:
> I think regular getVRegDef is preferable for SSA MIR
Fixed locally.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088



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

[PATCH] D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I still wonder what made this statement live in my example. There must have 
been some non-trivial liveness analysis going on that caused a statement to be 
live; probably something to do with the C++ destructor elements.

In D82598#2172371 , @Szelethus wrote:

> Wow, I never realized I accidentally landed that assert (D82122#2172360 
> ), but I guess its great to have 
> that covered. Would you prefer to have that reverted as I'm looking to fix 
> this for good?


It doesn't add any actual functionality so i think it makes sense to revert 
unless you have a quick fix.

Also if the assert is in fact true then i'd rather make a much stronger 
statement by banning statements from the Environment entirely, as in, like, at 
compile time by making it accept `Expr *` instead of `Stmt *`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598



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

[PATCH] D84192: [OpenMP5.0] map item can be non-contiguous for target update

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8914
+  ASTContext &C = CGF.getContext();
+  QualType Int64Ty = C.getIntTypeForBitwidth(/*DestWidth=*/64, 
/*Signed=*/true);
+  RecordDecl *RD;

1. The second argument must be of integer type, not boolean.
2. Why it is signed?



Comment at: clang/lib/Sema/SemaOpenMP.cpp:16915
+  continue;
+else if (OASE && OASE->getLength())
+  break;

No need for `else` here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84192



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

[PATCH] D83955: [PowerPC][Power10] Implementation of 128-bit Binary Vector Multiply builtins

[Amy Kwan via Phabricator via cfe-commits]

amyk accepted this revision as: amyk.
amyk added a comment.

I think this LGTM now. The file is already upstream, so your tests will need to 
be added to that file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83955



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

[PATCH] D84197: [PowerPC][Power10] 128-bit Vector String Isolate instruction definitions and MC Tests

[Amy Kwan via Phabricator via cfe-commits]

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

Could you please add back the MC tests for the rightmost load/store 
instructions, and then add the vector string isolate tests to the end of the 
file?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84197



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

[PATCH] D83338: [PowerPC][Power10] Implemented Vector Shift Builtins

[Amy Kwan via Phabricator via cfe-commits]

amyk added a comment.

I realize it may be possible to open code these, as these functions already 
exist in altivec.h. Could you look into if this is the case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83338



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

[PATCH] D84192: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 280474.
cchen added a comment.

Fix coding style and argument on getIntTypeForBitwidth


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84192

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/target_update_ast_print.cpp
  clang/test/OpenMP/target_update_codegen.cpp
  clang/test/OpenMP/target_update_messages.cpp
  clang/test/OpenMP/target_update_to_messages.cpp

Index: clang/test/OpenMP/target_update_to_messages.cpp
===
--- clang/test/OpenMP/target_update_to_messages.cpp
+++ clang/test/OpenMP/target_update_to_messages.cpp
@@ -79,6 +79,10 @@
 #pragma omp target update to(*(*(this->ptr)+a+this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
 #pragma omp target update to(*(this+this)) // expected-error {{invalid operands to binary expression ('S8 *' and 'S8 *')}}
 {}
+
+double marr[10][5][10];
+#pragma omp target update to(marr [0:1][2:4][1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
   }
 };
 
Index: clang/test/OpenMP/target_update_messages.cpp
===
--- clang/test/OpenMP/target_update_messages.cpp
+++ clang/test/OpenMP/target_update_messages.cpp
@@ -1,8 +1,8 @@
-// RUN: %clang_cc1 -verify=expected,omp45 -fopenmp -fopenmp-version=45 -ferror-limit 100 -o - -std=c++11 %s -Wuninitialized
-// RUN: %clang_cc1 -verify=expected,omp50 -fopenmp -fopenmp-version=50 -ferror-limit 100 -o - -std=c++11 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -fopenmp-version=45 -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
-// RUN: %clang_cc1 -verify=expected,omp45 -fopenmp-simd -fopenmp-version=45 -ferror-limit 100 -o - -std=c++11 %s -Wuninitialized
-// RUN: %clang_cc1 -verify=expected,omp50 -fopenmp-simd -fopenmp-version=50 -ferror-limit 100 -o - -std=c++11 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -fopenmp-version=45 -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp-simd -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
 void xxx(int argc) {
   int x; // expected-note {{initialize the variable 'x' to silence this warning}}
@@ -39,10 +39,33 @@
 foo();
   }
 
+  double marr[10][5][10];
+#pragma omp target update to(marr[0:2][2:4][1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(marr[0:2][2:4][1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+#pragma omp target update to(marr[0:][1:2:2][1:2]) // le50-error {{array section does not specify length for outermost dimension}} le45-error {{expected ']'}} le45-note {{to match this '['}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(marr[0:][1:2:2][1:2]) // le50-error {{array section does not specify length for outermost dimension}} le45-error {{expected ']'}} le45-note {{to match this '['}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  int arr[4][3][2][1];
+#pragma omp target update to(arr[0:2][2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(arr[0:2][2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  double ***dptr;
+#pragma omp target update to(dptr[0:2][2:4][1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target up

[PATCH] D84525: [clangd] Add marshalling code for all request types

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 280476.
kbobyrev marked 4 inline comments as done.
kbobyrev added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84525

Files:
  clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
  clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
  clang-tools-extra/clangd/index/remote/server/Server.cpp
  clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp

Index: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
===
--- clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
+++ clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
@@ -289,7 +289,8 @@
   auto Serialized = ProtobufMarshaller.toProtobuf(Request);
   EXPECT_EQ(Serialized.proximity_paths_size(), 2);
   auto Deserialized = ProtobufMarshaller.fromProtobuf(&Serialized);
-  EXPECT_THAT(Deserialized.ProximityPaths,
+  ASSERT_TRUE(Deserialized);
+  EXPECT_THAT(Deserialized->ProximityPaths,
   testing::ElementsAre(testPath("remote/Header.h"),
testPath("remote/subdir/OtherHeader.h")));
 }
Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -59,14 +59,10 @@
   grpc::Status Lookup(grpc::ServerContext *Context,
   const LookupRequest *Request,
   grpc::ServerWriter *Reply) override {
-clangd::LookupRequest Req;
-for (const auto &ID : Request->ids()) {
-  auto SID = SymbolID::fromStr(StringRef(ID));
-  if (!SID)
-return grpc::Status::CANCELLED;
-  Req.IDs.insert(*SID);
-}
-Index->lookup(Req, [&](const clangd::Symbol &Sym) {
+const auto Req = ProtobufMarshaller->fromProtobuf(Request);
+if (!Req)
+  return grpc::Status::CANCELLED;
+Index->lookup(*Req, [&](const clangd::Symbol &Sym) {
   auto SerializedSymbol = ProtobufMarshaller->toProtobuf(Sym);
   if (!SerializedSymbol)
 return;
@@ -84,7 +80,9 @@
  const FuzzyFindRequest *Request,
  grpc::ServerWriter *Reply) override {
 const auto Req = ProtobufMarshaller->fromProtobuf(Request);
-bool HasMore = Index->fuzzyFind(Req, [&](const clangd::Symbol &Sym) {
+if (!Req)
+  return grpc::Status::CANCELLED;
+bool HasMore = Index->fuzzyFind(*Req, [&](const clangd::Symbol &Sym) {
   auto SerializedSymbol = ProtobufMarshaller->toProtobuf(Sym);
   if (!SerializedSymbol)
 return;
@@ -100,14 +98,10 @@
 
   grpc::Status Refs(grpc::ServerContext *Context, const RefsRequest *Request,
 grpc::ServerWriter *Reply) override {
-clangd::RefsRequest Req;
-for (const auto &ID : Request->ids()) {
-  auto SID = SymbolID::fromStr(StringRef(ID));
-  if (!SID)
-return grpc::Status::CANCELLED;
-  Req.IDs.insert(*SID);
-}
-bool HasMore = Index->refs(Req, [&](const clangd::Ref &Reference) {
+const auto Req = ProtobufMarshaller->fromProtobuf(Request);
+if (!Req)
+  return grpc::Status::CANCELLED;
+bool HasMore = Index->refs(*Req, [&](const clangd::Ref &Reference) {
   auto SerializedRef = ProtobufMarshaller->toProtobuf(Reference);
   if (!SerializedRef)
 return;
Index: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
===
--- clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
+++ clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
@@ -38,10 +38,15 @@
   Marshaller() = delete;
   Marshaller(llvm::StringRef RemoteIndexRoot, llvm::StringRef LocalIndexRoot);
 
-  clangd::FuzzyFindRequest fromProtobuf(const FuzzyFindRequest *Request);
   llvm::Optional fromProtobuf(const Symbol &Message);
   llvm::Optional fromProtobuf(const Ref &Message);
 
+  llvm::Optional
+  fromProtobuf(const LookupRequest *Message);
+  llvm::Optional
+  fromProtobuf(const FuzzyFindRequest *Message);
+  llvm::Optional fromProtobuf(const RefsRequest *Message);
+
   /// toProtobuf() functions serialize native clangd types and strip IndexRoot
   /// from the file paths specific to indexing machine. fromProtobuf() functions
   /// deserialize clangd types and translate relative paths into machine-native
Index: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
===
--- clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -17,6 +17,7 @@
 #include "index/SymbolOrigin.h"
 #include "support/Logger.h"
 #include "clang/Index/Inde

[PATCH] D84348: WIP: Add complete id-expression support to syntax trees

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

What is this diff based on? On the left I see, for example, 
NamespaceNameSpecifier, which is not in the repository yet.




Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:777
   // FIXME: Support Microsoft's __super
-  return new (allocator()) syntax::UnknownNameSpecifier;
+  assert(false && "We don't yet treat the __super specifier");
 }

s/treat/support/

Also, use llvm::report_fatal_error instead? assert is not supposed to ever 
trigger.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84348



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

[PATCH] D84525: [clangd] Add marshalling code for all request types

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.



Comment at: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp:55
+template 
+llvm::Optional> getIDs(MessageT *Message) {
+  llvm::DenseSet Result;

kadircet wrote:
> I would make this return an expected instead of an optional and print the 
> error in the callers, instead of logging twice.
> 
> (I also think a similar problem is present with the existing `fromProtobuf` 
> APIs too, they return None in case of failure while logging the error, and 
> then callers again log the error. I am not sure if we plan to implement some 
> error handling strategies but even if we did, it feels like Marshalling is 
> the lowest layer that wouldn't know any other strategy to handle such errors, 
> so it seems like always returning an error from the marshalling and letting 
> the caller handle the error sounds like a safe bet. But no action needed in 
> this patch just stating some ideas in case you find them useful :D)
Good point: we thought about error handling and discussed it with Sam on 
multiple occasions, we tested several strategies and I think converged to this. 
I don't think it's optimal, you are right, but this code is consistent with the 
rest of the file.

The problem here is that in some cases these should actually be `Optional`s 
(some fields may or may not be missing) and in some cases it makes sense to 
have `Expected`s, which would leave a weird mixture of these things. But maybe 
carefully thinking about those cases would simplify the code.

How do you feel about leaving this as is and maybe carefully thinking about 
`Optional`s to `Expected` in the next patches?



Comment at: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp:292
   auto Deserialized = ProtobufMarshaller.fromProtobuf(&Serialized);
-  EXPECT_THAT(Deserialized.ProximityPaths,
+  EXPECT_TRUE(Deserialized);
+  EXPECT_THAT(Deserialized->ProximityPaths,

kadircet wrote:
> `ASSERT_TRUE` to ensure we don't try to dereference in case of None.
Good point, I should do `ASSERT_TRUE` in most cases throughout the file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84525



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

[PATCH] D84535: [clangd] Switch from EXPECT_TRUE to ASSERT_TRUE in remote marshalling tests

[Kirill Bobyrev via Phabricator via cfe-commits]

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

When dereferencing Optional's it makes sense to use ASSERT_TRUE for better
test failures readability. Switch from EXPECT_TRUE to ASSERT_TRUE where
it is appropriate.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84535

Files:
  clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp

Index: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
===
--- clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
+++ clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
@@ -49,11 +49,11 @@
   "clangd/unittests/remote/MarshallingTests.cpp",
   Strings);
   auto Serialized = ProtobufMarshaller.toProtobuf(Original);
-  EXPECT_TRUE(Serialized);
+  ASSERT_TRUE(Serialized);
   EXPECT_EQ(Serialized->location().file_path(),
 "clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp");
   auto Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
-  EXPECT_TRUE(Deserialized);
+  ASSERT_TRUE(Deserialized);
   EXPECT_STREQ(Deserialized->Location.FileURI,
testPathURI("home/my-projects/llvm-project/clang-tools-extra/"
"clangd/unittests/remote/MarshallingTests.cpp",
@@ -73,7 +73,7 @@
   // Can not use URIs with scheme different from "file".
   auto UnittestURI =
   URI::create(testPath("project/lib/HelloWorld.cpp"), "unittest");
-  EXPECT_TRUE(bool(UnittestURI));
+  ASSERT_TRUE(bool(UnittestURI));
   WithInvalidURI.Location.FileURI =
   Strings.save(UnittestURI->toString()).begin();
   Serialized = ProtobufMarshaller.toProtobuf(WithInvalidURI);
@@ -92,7 +92,7 @@
   clangd::Symbol Sym;
 
   auto ID = SymbolID::fromStr("057557CEBF6E6B2D");
-  EXPECT_TRUE(bool(ID));
+  ASSERT_TRUE(bool(ID));
   Sym.ID = *ID;
 
   index::SymbolInfo Info;
@@ -140,9 +140,9 @@
   // Check that symbols are exactly the same if the path to indexed project is
   // the same on indexing machine and the client.
   auto Serialized = ProtobufMarshaller.toProtobuf(Sym);
-  EXPECT_TRUE(Serialized);
+  ASSERT_TRUE(Serialized);
   auto Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
-  EXPECT_TRUE(Deserialized);
+  ASSERT_TRUE(Deserialized);
   EXPECT_EQ(toYAML(Sym), toYAML(*Deserialized));
   // Serialized paths are relative and have UNIX slashes.
   EXPECT_EQ(convert_to_slash(Serialized->definition().file_path(),
@@ -154,7 +154,7 @@
   // Missing definition is OK.
   Sym.Definition = clangd::SymbolLocation();
   Serialized = ProtobufMarshaller.toProtobuf(Sym);
-  EXPECT_TRUE(Serialized);
+  ASSERT_TRUE(Serialized);
   Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
   EXPECT_TRUE(Deserialized);
 
@@ -172,7 +172,7 @@
 
   // Schemes other than "file" can not be used.
   auto UnittestURI = URI::create(testPath("home/SomePath.h"), "unittest");
-  EXPECT_TRUE(bool(UnittestURI));
+  ASSERT_TRUE(bool(UnittestURI));
   Location.FileURI = Strings.save(UnittestURI->toString()).begin();
   Sym.Definition = Location;
   Serialized = ProtobufMarshaller.toProtobuf(Sym);
@@ -183,9 +183,9 @@
   Sym.Definition = Location;
   // Check that the symbol is valid and passing the correct path works.
   Serialized = ProtobufMarshaller.toProtobuf(Sym);
-  EXPECT_TRUE(Serialized);
+  ASSERT_TRUE(Serialized);
   Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
-  EXPECT_TRUE(Deserialized);
+  ASSERT_TRUE(Deserialized);
   EXPECT_STREQ(Deserialized->Definition.FileURI,
testPathURI("home/File.h", Strings));
   // Fail with a wrong root.
@@ -214,9 +214,9 @@
 testPath("llvm-project/"));
 
   auto Serialized = ProtobufMarshaller.toProtobuf(Ref);
-  EXPECT_TRUE(Serialized);
+  ASSERT_TRUE(Serialized);
   auto Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
-  EXPECT_TRUE(Deserialized);
+  ASSERT_TRUE(Deserialized);
   EXPECT_EQ(toYAML(Ref), toYAML(*Deserialized));
 }
 
@@ -272,9 +272,9 @@
   auto Serialized = ProtobufMarshaller.toProtobuf(Sym);
   EXPECT_EQ(static_cast(Serialized->headers_size()),
 ValidHeaders.size());
-  EXPECT_TRUE(Serialized);
+  ASSERT_TRUE(Serialized);
   auto Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
-  EXPECT_TRUE(Deserialized);
+  ASSERT_TRUE(Deserialized);
 
   Sym.IncludeHeaders = ValidHeaders;
   EXPECT_EQ(toYAML(Sym), toYAML(*Deserialized));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84192: [OpenMP5.0] map item can be non-contiguous for target update

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84192



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

[PATCH] D84192: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

@ABataev, do I need to wait for the runtime patch to commit this one? If so, do 
you have some recommend reviewers for me to add to that patch? I have pinged 
several times for that patch but haven't got many reviews for it. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84192



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

[PATCH] D84192: [OpenMP5.0] map item can be non-contiguous for target update

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D84192#2172482 , @cchen wrote:

> @ABataev, do I need to wait for the runtime patch to commit this one? If so, 
> do you have some recommend reviewers for me to add to that patch? I have 
> pinged several times for that patch but haven't got many reviews for it. 
> Thanks!


Yes, it must be committed after runtime patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84192



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

[PATCH] D84511: Fix update_cc_test_checks.py --llvm-bin after D78478

[Alexander Richardson via Phabricator via cfe-commits]

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



Comment at: clang/test/utils/update_cc_test_checks/lit.local.cfg:24
+# works as expected
+config.substitutions.append(
+('%update_cc_test_checks_llvm_bin', "%s %s %s" % (

MaskRay wrote:
> The substitution is here just to make a test. Can it be tested with existing 
> substitutions?
I can't think of an easy way since we need to invoke update_cc_test_checks.py 
without the "--clang" argument and we need a substitution for the path to the 
update script.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84511



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

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle updated this revision to Diff 280481.
nhaehnle marked an inline comment as done.
nhaehnle added a comment.

  v6:
  - implement predecessors/successors for all CfgTraits implementations
  - fix error in unwrapRange
  - rename toGeneric/fromGeneric into wrapRef/unwrapRef to have naming
that is consistent with {wrap,unwrap}{Iterator,Range}
  - use getVRegDef instead of getUniqueVRegDef


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

Files:
  clang/include/clang/Analysis/Analyses/Dominators.h
  llvm/include/llvm/CodeGen/MachineCfgTraits.h
  llvm/include/llvm/IR/CFG.h
  llvm/include/llvm/Support/CfgTraits.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/MachineCfgTraits.cpp
  llvm/lib/IR/CFG.cpp
  llvm/lib/IR/CMakeLists.txt
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CfgTraits.cpp
  llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
  mlir/include/mlir/IR/Dominance.h

Index: mlir/include/mlir/IR/Dominance.h
===
--- mlir/include/mlir/IR/Dominance.h
+++ mlir/include/mlir/IR/Dominance.h
@@ -10,8 +10,46 @@
 #define MLIR_IR_DOMINANCE_H
 
 #include "mlir/IR/RegionGraphTraits.h"
+#include "llvm/Support/CfgTraits.h"
 #include "llvm/Support/GenericDomTree.h"
 
+namespace mlir {
+
+/// Partial CFG traits for MLIR's CFG, without a value type.
+class CfgTraitsBase : public llvm::CfgTraitsBase {
+public:
+  using ParentType = Region;
+  using BlockRef = Block *;
+  using ValueRef = void;
+
+  static llvm::CfgBlockRef wrapRef(BlockRef block) {
+return makeOpaque(block);
+  }
+  static BlockRef unwrapRef(llvm::CfgBlockRef block) {
+return static_cast(getOpaque(block));
+  }
+};
+
+class CfgTraits : public llvm::CfgTraits {
+public:
+  static Region *getBlockParent(Block *block) { return block->getParent(); }
+
+  static auto predecessors(Block *block) {
+return llvm::inverse_children(block);
+  }
+
+  static auto successors(Block *block) {
+return llvm::children(block);
+  }
+};
+
+} // namespace mlir
+
+template <>
+struct llvm::CfgTraitsFor {
+  using CfgTraits = mlir::CfgTraits;
+};
+
 extern template class llvm::DominatorTreeBase;
 extern template class llvm::DominatorTreeBase;
 
Index: llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
===
--- llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
+++ llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
@@ -18,9 +18,42 @@
 #include "VPlan.h"
 #include "llvm/ADT/GraphTraits.h"
 #include "llvm/IR/Dominators.h"
+#include "llvm/Support/CfgTraits.h"
 
 namespace llvm {
 
+/// Partial CFG traits for VPlan's CFG, without a value type.
+class VPCfgTraitsBase : public CfgTraitsBase {
+public:
+  using ParentType = VPRegionBlock;
+  using BlockRef = VPBlockBase *;
+  using ValueRef = void;
+
+  static CfgBlockRef wrapRef(BlockRef block) {
+return makeOpaque(block);
+  }
+  static BlockRef unwrapRef(CfgBlockRef block) {
+return static_cast(getOpaque(block));
+  }
+};
+
+class VPCfgTraits : public CfgTraits {
+public:
+  static VPRegionBlock *getBlockParent(VPBlockBase *block) {
+return block->getParent();
+  }
+
+  static auto predecessors(VPBlockBase *block) {
+return llvm::inverse_children(block);
+  }
+
+  static auto successors(VPBlockBase *block) {
+return llvm::children(block);
+  }
+};
+
+template <> struct CfgTraitsFor { using CfgTraits = VPCfgTraits; };
+
 /// Template specialization of the standard LLVM dominator tree utility for
 /// VPBlockBases.
 using VPDominatorTree = DomTreeBase;
Index: llvm/lib/Support/CfgTraits.cpp
===
--- /dev/null
+++ llvm/lib/Support/CfgTraits.cpp
@@ -0,0 +1,14 @@
+//===- CfgTraits.cpp - Traits for generically working on CFGs ---*- 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 "llvm/Support/CfgTraits.h"
+
+using namespace llvm;
+
+void CfgInterface::anchor() {}
+void CfgPrinter::anchor() {}
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -71,6 +71,7 @@
   BranchProbability.cpp
   BuryPointer.cpp
   CachePruning.cpp
+  CfgTraits.cpp
   circular_raw_ostream.cpp
   Chrono.cpp
   COM.cpp
Index: llvm/lib/IR/CMakeLists.txt
===
--- llvm/lib/IR/CMakeLists.txt
+++ llvm/lib/IR/CMakeLists.txt
@@ -4,6 +4,7 @@
   Attributes.cpp
   AutoUpgrade.cpp
   BasicBlock.cpp
+  CFG.cpp
   Comdat.cpp
   ConstantFold.cpp
   ConstantRange.cpp
Index: llvm/lib/IR/CFG.cpp
===

[PATCH] D84371: [DFSan] Add efficient fast16labels instrumentation mode.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: compiler-rt/lib/dfsan/dfsan.cpp:180
 dfsan_label __dfsan_union(dfsan_label l1, dfsan_label l2) {
-  if (flags().fast16labels)
+  if (fast16labels)
 return l1 | l2;

vitalybuka wrote:
> isn't better just create new set of callbacks?
> e.g __dfsan_fast16_union
> and then we don't need any flags or preinit array initialization
Should work for `__dfsan_union` and `__dfsan_union_load`, but what about all 
the other API functions the user can call directly?  We wouldn't be able to 
warn in those cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84371



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

[PATCH] D84371: [DFSan] Add efficient fast16labels instrumentation mode.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse updated this revision to Diff 280485.
morehouse marked 5 inline comments as done.
morehouse added a comment.

- Rename flag
- Clarify doc example
- Use temporary variables.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84371

Files:
  clang/docs/DataFlowSanitizer.rst
  compiler-rt/lib/dfsan/dfsan.cpp
  compiler-rt/lib/dfsan/dfsan_flags.inc
  compiler-rt/lib/dfsan/done_abilist.txt
  compiler-rt/lib/fuzzer/FuzzerDataFlowTrace.cpp
  compiler-rt/lib/fuzzer/dataflow/DataFlow.cpp
  compiler-rt/test/dfsan/fast16labels.c
  compiler-rt/test/fuzzer/dataflow.test
  compiler-rt/test/fuzzer/only-some-bytes-fork.test
  compiler-rt/test/fuzzer/only-some-bytes.test
  llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
  llvm/test/Instrumentation/DataFlowSanitizer/fast16labels.ll

Index: llvm/test/Instrumentation/DataFlowSanitizer/fast16labels.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/DataFlowSanitizer/fast16labels.ll
@@ -0,0 +1,100 @@
+; Test that -dfsan-fast-16-labels mode uses inline ORs rather than calling
+; __dfsan_union or __dfsan_union_load.
+; RUN: opt < %s -dfsan -dfsan-fast-16-labels -S | FileCheck %s --implicit-check-not="call{{.*}}__dfsan_union"
+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i8 @add(i8 %a, i8 %b) {
+  ; CHECK-LABEL: define i8 @"dfs$add"
+  ; CHECK-DAG: %[[ALABEL:.*]] = load{{.*}}__dfsan_arg_tls, i64 0, i64 0
+  ; CHECK-DAG: %[[BLABEL:.*]] = load{{.*}}__dfsan_arg_tls, i64 0, i64 1
+  ; CHECK: %[[ADDLABEL:.*]] = or i16 %[[ALABEL]], %[[BLABEL]]
+  ; CHECK: add i8
+  ; CHECK: store i16 %[[ADDLABEL]], i16* @__dfsan_retval_tls
+  ; CHECK: ret i8
+  %c = add i8 %a, %b
+  ret i8 %c
+}
+
+define i8 @load8(i8* %p) {
+  ; CHECK-LABEL: define i8 @"dfs$load8"
+  ; CHECK: load i16, i16*
+  ; CHECK: ptrtoint i8* {{.*}} to i64
+  ; CHECK: and i64
+  ; CHECK: mul i64
+  ; CHECK: inttoptr i64
+  ; CHECK: load i16, i16*
+  ; CHECK: or i16
+  ; CHECK: load i8, i8*
+  ; CHECK: store i16 {{.*}} @__dfsan_retval_tls
+  ; CHECK: ret i8
+
+  %a = load i8, i8* %p
+  ret i8 %a
+}
+
+define i16 @load16(i16* %p) {
+  ; CHECK-LABEL: define i16 @"dfs$load16"
+  ; CHECK: ptrtoint i16*
+  ; CHECK: and i64
+  ; CHECK: mul i64
+  ; CHECK: inttoptr i64 {{.*}} i16*
+  ; CHECK: getelementptr i16
+  ; CHECK: load i16, i16*
+  ; CHECK: load i16, i16*
+  ; CHECK: or i16
+  ; CHECK: or i16
+  ; CHECK: load i16, i16*
+  ; CHECK: store {{.*}} @__dfsan_retval_tls
+  ; CHECK: ret i16
+
+  %a = load i16, i16* %p
+  ret i16 %a
+}
+
+define i32 @load32(i32* %p) {
+  ; CHECK-LABEL: define i32 @"dfs$load32"
+  ; CHECK: ptrtoint i32*
+  ; CHECK: and i64
+  ; CHECK: mul i64
+  ; CHECK: inttoptr i64 {{.*}} i16*
+  ; CHECK: bitcast i16* {{.*}} i64*
+  ; CHECK: load i64, i64*
+  ; CHECK: lshr i64 {{.*}}, 32
+  ; CHECK: or i64
+  ; CHECK: lshr i64 {{.*}}, 16
+  ; CHECK: or i64
+  ; CHECK: trunc i64 {{.*}} i16
+  ; CHECK: or i16
+  ; CHECK: load i32, i32*
+  ; CHECK: store i16 {{.*}} @__dfsan_retval_tls
+  ; CHECK: ret i32
+
+  %a = load i32, i32* %p
+  ret i32 %a
+}
+
+define i64 @load64(i64* %p) {
+  ; CHECK-LABEL: define i64 @"dfs$load64"
+  ; CHECK: ptrtoint i64*
+  ; CHECK: and i64
+  ; CHECK: mul i64
+  ; CHECK: inttoptr i64 {{.*}} i16*
+  ; CHECK: bitcast i16* {{.*}} i64*
+  ; CHECK: load i64, i64*
+  ; CHECK: getelementptr i64, i64* {{.*}}, i64 1
+  ; CHECK: load i64, i64*
+  ; CHECK: or i64
+  ; CHECK: lshr i64 {{.*}}, 32
+  ; CHECK: or i64
+  ; CHECK: lshr i64 {{.*}}, 16
+  ; CHECK: or i64
+  ; CHECK: trunc i64 {{.*}} i16
+  ; CHECK: or i16
+  ; CHECK: load i64, i64*
+  ; CHECK: store i16 {{.*}} @__dfsan_retval_tls
+  ; CHECK: ret i64
+
+  %a = load i64, i64* %p
+  ret i64 %a
+}
Index: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
@@ -91,6 +91,7 @@
 #include "llvm/Transforms/Instrumentation.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
 #include "llvm/Transforms/Utils/Local.h"
+#include "llvm/Transforms/Utils/ModuleUtils.h"
 #include 
 #include 
 #include 
@@ -176,6 +177,14 @@
 cl::desc("Insert calls to __dfsan_*_callback functions on data events."),
 cl::Hidden, cl::init(false));
 
+// Use a distinct bit for each base label, enabling faster unions with less
+// instrumentation.  Limits the max number of base labels to 16.
+static cl::opt ClFast16Labels(
+"dfsan-fast-16-labels",
+cl::desc("Use more efficient instrumentation, limiting the number of "
+ "labels to 16."),
+cl::Hidden, cl::init(false));
+
 static StringRef GetGlobalTypeString(const GlobalValue

[PATCH] D84348: WIP: Add complete id-expression support to syntax trees

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas updated this revision to Diff 280497.
eduucaldas added a comment.

- Improve getLocalSourceRange
- nested-name-specifier is now a ::-separated list of name-specifiers


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84348

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -867,24 +867,47 @@
   }
   EXPECT_TRUE(treeDumpEqual(
   R"cpp(
-namespace a {
+namespace n {
   struct S {
 template
-static T f(){}
+struct TS {
+  static void f();
+};
   };
 }
+template
+struct TS {
+  struct S {
+template
+static U f();
+  };
+};
 void test() {
-  ::  // global-namespace-specifier
-  a:: // namespace-specifier
-  S:: // type-name-specifier
+  :: // global-namespace-specifier
+  n::// namespace-specifier
+  S::// type-name-specifier
+  template TS:: // type-template-instantiation-specifier
+  f();
+
+  n::// namespace-specifier
+  S::// type-name-specifier
+  TS::  // type-template-instantiation-specifier
+  f();
+
+  TS:: // type-name-specifier
+  S::   // type-name-specifier
   f();
+
+  TS:: // type-name-specifier
+  S::   // type-name-specifier
+  template f();
 }
 )cpp",
   R"txt(
 *: TranslationUnit
 |-NamespaceDefinition
 | |-namespace
-| |-a
+| |-n
 | |-{
 | |-SimpleDeclaration
 | | |-struct
@@ -898,19 +921,58 @@
 | | | | `-T
 | | | |->
 | | | `-SimpleDeclaration
-| | |   |-static
-| | |   |-T
-| | |   |-SimpleDeclarator
-| | |   | |-f
-| | |   | `-ParametersAndQualifiers
-| | |   |   |-(
-| | |   |   `-)
-| | |   `-CompoundStatement
-| | | |-{
-| | | `-}
+| | |   |-struct
+| | |   |-TS
+| | |   |-{
+| | |   |-SimpleDeclaration
+| | |   | |-static
+| | |   | |-void
+| | |   | |-SimpleDeclarator
+| | |   | | |-f
+| | |   | | `-ParametersAndQualifiers
+| | |   | |   |-(
+| | |   | |   `-)
+| | |   | `-;
+| | |   |-}
+| | |   `-;
 | | |-}
 | | `-;
 | `-}
+|-TemplateDeclaration
+| |-template
+| |-<
+| |-UnknownDeclaration
+| | |-typename
+| | `-T
+| |->
+| `-SimpleDeclaration
+|   |-struct
+|   |-TS
+|   |-{
+|   |-SimpleDeclaration
+|   | |-struct
+|   | |-S
+|   | |-{
+|   | |-TemplateDeclaration
+|   | | |-template
+|   | | |-<
+|   | | |-UnknownDeclaration
+|   | | | |-typename
+|   | | | `-U
+|   | | |->
+|   | | `-SimpleDeclaration
+|   | |   |-static
+|   | |   |-U
+|   | |   |-SimpleDeclarator
+|   | |   | |-f
+|   | |   | `-ParametersAndQualifiers
+|   | |   |   |-(
+|   | |   |   `-)
+|   | |   `-;
+|   | |-}
+|   | `-;
+|   |-}
+|   `-;
 `-SimpleDeclaration
   |-void
   |-SimpleDeclarator
@@ -924,14 +986,81 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | |-NameSpecifier
-| | | | | `-::
-| | | | |-NameSpecifier
-| | | | | |-a
-| | | | | `-::
-| | | | `-NameSpecifier
-| | | |   |-S
-| | | |   `-::
+| | | | |-::
+| | | | |-IdentifierNameSpecifier
+| | | | | `-n
+| | | | |-::
+| | | | |-IdentifierNameSpecifier
+| | | | | `-S
+| | | | |-::
+| | | | |-SimpleTemplateNameSpecifier
+| | | | | |-template
+| | | | | |-TS
+| | | | | |-<
+| | | | | |-int
+| | | | | `->
+| | | | `-::
+| | | `-UnqualifiedId
+| | |   `-f
+| | |-(
+| | `-)
+| `-;
+|-ExpressionStatement
+| |-UnknownExpression
+| | |-IdExpression
+| | | |-NestedNameSpecifier
+| | | | |-IdentifierNameSpecifier
+| | | | | `-n
+| | | | |-::
+| | | | |-IdentifierNameSpecifier
+| | | | | `-S
+| | | | |-::
+| | | | |-SimpleTemplateNameSpecifier
+| | | | | |-TS
+| | | | | |-<
+| | | | | |-int
+| | | | | `->
+| | | | `-::
+| | | `-UnqualifiedId
+| | |   `-f
+| | |-(
+| | `-)
+| `-;
+|-ExpressionStatement
+| |-UnknownExpression
+| | |-IdExpression
+| | | |-NestedNameSpecifier
+| | | | |-SimpleTemplateNameSpecifier
+| | | | | |-TS
+| | | | | |-<
+| | | | | |-int
+| | | | | `->
+| | | | |-::
+| | | | |-IdentifierNameSpecifier
+| | | | | `-S
+| | | | `-::
+| | | `-UnqualifiedId
+| | |   |-f
+| | |   |-<
+| | |   |-int
+| | |   `->
+| | |-(
+| | `-)
+| `-;
+|-ExpressionStatement
+| |-UnknownExpression
+| | |-IdExpression
+| | | |-NestedNameSpecifier
+| | | | |-SimpleTemplateNameSpecifier
+| | | | | |-TS
+| | | | | |-<
+| | | | | |-int
+| | | | | `->
+| | | | |-::
+| | | |

[PATCH] D84540: [CodeGen][ObjC] Mark calls to objc_unsafeClaimAutoreleasedReturnValue as notail on x86-64

[Akira Hatanaka via Phabricator via cfe-commits]

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

This is needed because the epilogue code inserted before tail calls on x86-64 
breaks the handshake between the caller and callee.

Calls to objc_retainAutoreleasedReturnValue used to have the same problem, 
which was fixed in https://reviews.llvm.org/D59656.

rdar://problem/66029552


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84540

Files:
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/CodeGen/TargetInfo.h
  clang/test/CodeGenObjC/arc-unsafeclaim.m

Index: clang/test/CodeGenObjC/arc-unsafeclaim.m
===
--- clang/test/CodeGenObjC/arc-unsafeclaim.m
+++ clang/test/CodeGenObjC/arc-unsafeclaim.m
@@ -1,16 +1,16 @@
 //   Make sure it works on x86-64.
-// RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fobjc-runtime=macosx-10.11 -fobjc-arc -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-UNOPTIMIZED
+// RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fobjc-runtime=macosx-10.11 -fobjc-arc -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-UNOPTIMIZED -check-prefix=NOTAIL-CALL
 
 //   Make sure it works on x86-32.
-// RUN: %clang_cc1 -triple i386-apple-darwin11 -fobjc-runtime=macosx-fragile-10.11 -fobjc-arc -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-UNOPTIMIZED -check-prefix=CHECK-MARKED
+// RUN: %clang_cc1 -triple i386-apple-darwin11 -fobjc-runtime=macosx-fragile-10.11 -fobjc-arc -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-UNOPTIMIZED -check-prefix=CHECK-MARKED -check-prefix=CALL
 
 //   Make sure it works on ARM.
-// RUN: %clang_cc1 -triple arm64-apple-ios9 -fobjc-runtime=ios-9.0 -fobjc-arc -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-UNOPTIMIZED -check-prefix=CHECK-MARKED
-// RUN: %clang_cc1 -triple arm64-apple-ios9 -fobjc-runtime=ios-9.0 -fobjc-arc -O -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-OPTIMIZED
+// RUN: %clang_cc1 -triple arm64-apple-ios9 -fobjc-runtime=ios-9.0 -fobjc-arc -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-UNOPTIMIZED -check-prefix=CHECK-MARKED -check-prefix=CALL
+// RUN: %clang_cc1 -triple arm64-apple-ios9 -fobjc-runtime=ios-9.0 -fobjc-arc -O -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-OPTIMIZED -check-prefix=CALL
 
 //   Make sure it works on ARM64.
-// RUN: %clang_cc1 -triple armv7-apple-ios9 -fobjc-runtime=ios-9.0 -fobjc-arc -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-UNOPTIMIZED -check-prefix=CHECK-MARKED
-// RUN: %clang_cc1 -triple armv7-apple-ios9 -fobjc-runtime=ios-9.0 -fobjc-arc -O -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-OPTIMIZED
+// RUN: %clang_cc1 -triple armv7-apple-ios9 -fobjc-runtime=ios-9.0 -fobjc-arc -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-UNOPTIMIZED -check-prefix=CHECK-MARKED -check-prefix=CALL
+// RUN: %clang_cc1 -triple armv7-apple-ios9 -fobjc-runtime=ios-9.0 -fobjc-arc -O -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-OPTIMIZED -check-prefix=CALL
 
 //   Make sure that it's implicitly disabled if the runtime version isn't high enough.
 // RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fobjc-runtime=macosx-10.10 -fobjc-arc -emit-llvm -o - %s | FileCheck %s -check-prefix=DISABLED
@@ -29,7 +29,8 @@
 // CHECK:[[T0:%.*]] = call [[A:.*]]* @makeA()
 // CHECK-MARKED-NEXT:call void asm sideeffect
 // CHECK-NEXT:   [[T1:%.*]] = bitcast [[A]]* [[T0]] to i8*
-// CHECK-NEXT:   [[T2:%.*]] = call i8* @llvm.objc.unsafeClaimAutoreleasedReturnValue(i8* [[T1]])
+// NOTAIL-CALL-NEXT: [[T2:%.*]] = notail call i8* @llvm.objc.unsafeClaimAutoreleasedReturnValue(i8* [[T1]])
+// CALL-NEXT:[[T2:%.*]] = call i8* @llvm.objc.unsafeClaimAutoreleasedReturnValue(i8* [[T1]])
 // CHECK-NEXT:   [[T3:%.*]] = bitcast i8* [[T2]] to [[A]]*
 // CHECK-NEXT:   [[T4:%.*]] = bitcast [[A]]* [[T3]] to i8*
 // CHECK-NEXT:   store i8* [[T4]], i8** [[X]]
@@ -53,7 +54,8 @@
 // CHECK:[[T0:%.*]] = call [[A]]* @makeA()
 // CHECK-MARKED-NEXT:call void asm sideeffect
 // CHECK-NEXT:   [[T1:%.*]] = bitcast [[A]]* [[T0]] to i8*
-// CHECK-NEXT:   [[T2:%.*]] = call i8* @llvm.objc.unsafeClaimAutoreleasedReturnValue(i8* [[T1]])
+// NOTAIL-CALL-NEXT: [[T2:%.*]] = notail call i8* @llvm.objc.unsafeClaimAutoreleasedReturnValue(i8* [[T1]])
+// CALL-NEXT:[[T2:%.*]] = call i8* @llvm.objc.unsafeClaimAutoreleasedReturnValue(i8* [[T1]])
 // CHECK-NEXT:   [[T3:%.*]] = bitcast i8* [[T2]] to [[A]]*

[PATCH] D83961: [Analyzer] Fix bug report source locations in minimal output.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D83961#2166903 , @balazske wrote:

> Every other test failure comes from RetainCount checker except 
> //malloc-plist.c//.


Aha, ok. So, anyway, for retain count checker we ultimately only care about 
plist and html reports, not about text reports. It's also probably easier to 
write more precise tests after your patch: test functions are unlikely to have 
multiple possible uniqueing locations, and even if there are they can be 
discriminated between by warning message text.

But i'd still rather have it explained why does your patch affect the location 
of `RefLeakReport`s in order to make sure we understand what we're doing. This 
consequence of the patch wasn't intended - what other unintended consequences 
does it have? Are we even sure that plist/html reports don't change? Is 
`RefLeakReport` even implemented correctly from our current point of view? Or, 
regardless of correctness, do we want its current implementation to have the 
old behavior or the new behavior?

I should probably investigate this myself from the fairness/justice point of 
view but you'll probably land your patch faster if you don't wait on me to get 
un-busy with other stuff :/ As a bonus, you might be able to get away without 
updating all the tests if you find out that this change is accidental and not 
really intended :) Note that you don't need to know Objective-C or know much 
about the checker to investigate these things; say, CGColorSpace.c is a pure C 
test with a straightforward resource leak bug. The customizations in 
`RefLeakReport` aren't really checker-specific either - we simply didn't ever 
make up our mind on whether they should apply to all leak reports or to none; 
they have visual consequences on plist reports though.




Comment at: clang/test/Analysis/malloc-plist.c:137-139
 if (y)
-y++;
-}//expected-warning{{Potential leak}}
+  y++; //expected-warning{{Potential leak}}
+}

balazske wrote:
> NoQ wrote:
> > This sounds like an expected change: we're now displaying the same report 
> > on a different path. Except it's the longer path rather than the shorter 
> > path, so it still looks suspicious.
> This location may be wrong but it is then another problem. The important 
> thing is that after this patch the same location is used for a warning in 
> `text-minimal` mode as it is in `text` mode. Without the patch in 
> `text-minimal` mode a different location is used for this warning, the one at 
> the end of the function. But still in `text` mode (without the patch) the 
> location at `y++` is shown. (The warning in function `function_with_leak4` in 
> the same test is already at a similar location, not at the end of function.)
Oh, wait, the other path isn't in fact shorter. In both cases it leaks 
immediately after the if-statement, and the path with `y++` is in fact a bit 
shorter (it immediately hits `PreStmtPurgeDeadSymbols` for the `DeclRefExpr` 
whereas the other path hits `CallExitBegin` first, because the call is inlined).

So it's a good and expected change!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83961



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

[PATCH] D83592: [Coverage] Add comment to skipped regions

[Zequan Wu via Phabricator via cfe-commits]

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



Comment at: clang/lib/Lex/Preprocessor.cpp:973
+  if ((LexLevel == 0 || PreprocessToken) &&
+  !Result.getFlag(Token::IsReinjected)) {
+if (LexLevel == 0)

hans wrote:
> zequanwu wrote:
> > @hans , can you take a look on this part? I saw `TokenCount` was introduced 
> > for a warning `-Wmax-tokens`.
> Can you explain why things are changing here?
> 
> The idea of TokenCount is to simply count the preprocessor tokens. At this 
> point I think you have more knowledge here than I did when I added it :)
In `CodeGen::CoverageMappingModuleGen::setUpCoverageCallbacks`, I set `OnToken` 
to a lambda to update `PrevTokLoc` after lexing a new token. But, the original 
condition ` if (LexLevel == 0 && !Result.getFlag(Token::IsReinjected)) ` will 
not call `OnToken` if it's  lexing a directive, like `#if`, `#define` etc, 
because the LexLevel is greater than 0. In order to update `PrevTokLoc` even 
when lexing directive, I add `PreprocessToken` which will be set to true in 
`CodeGen::CoverageMappingModuleGen::setUpCoverageCallbacks` just to invoke 
`OnToken` without increment `TokenCount` when lexing directives. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83592



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

[PATCH] D83592: [Coverage] Add comment to skipped regions

[Zequan Wu via Phabricator via cfe-commits]

zequanwu updated this revision to Diff 280504.
zequanwu added a comment.

Change `newSR.ColumnEnd = 1` to `newSR.ColumnEnd = SR.ColumnStart + 1` to 
ensure ColumnEnd >= ColumnStart.


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

https://reviews.llvm.org/D83592

Files:
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/lib/CodeGen/CoverageMappingGen.h
  clang/lib/Lex/Preprocessor.cpp
  clang/test/CoverageMapping/break.c
  clang/test/CoverageMapping/builtinmacro.c
  clang/test/CoverageMapping/classtemplate.cpp
  clang/test/CoverageMapping/comment-in-macro.c
  clang/test/CoverageMapping/continue.c
  clang/test/CoverageMapping/coroutine.cpp
  clang/test/CoverageMapping/deferred-region.cpp
  clang/test/CoverageMapping/if.cpp
  clang/test/CoverageMapping/includehell.cpp
  clang/test/CoverageMapping/label.cpp
  clang/test/CoverageMapping/logical.cpp
  clang/test/CoverageMapping/loops.cpp
  clang/test/CoverageMapping/macro-expressions.cpp
  clang/test/CoverageMapping/macroparams2.c
  clang/test/CoverageMapping/macros.c
  clang/test/CoverageMapping/macroscopes.cpp
  clang/test/CoverageMapping/moremacros.c
  clang/test/CoverageMapping/objc.m
  clang/test/CoverageMapping/pr32679.cpp
  clang/test/CoverageMapping/preprocessor.c
  clang/test/CoverageMapping/return.c
  clang/test/CoverageMapping/switch.cpp
  clang/test/CoverageMapping/switchmacro.c
  clang/test/CoverageMapping/test.c
  clang/test/CoverageMapping/trycatch.cpp
  clang/test/CoverageMapping/unreachable-macro.c
  clang/test/CoverageMapping/while.c
  clang/test/lit.cfg.py
  compiler-rt/test/profile/Inputs/instrprof-comdat.h
  compiler-rt/test/profile/coverage_comments.cpp
  compiler-rt/test/profile/instrprof-set-file-object-merging.c

Index: compiler-rt/test/profile/instrprof-set-file-object-merging.c
===
--- compiler-rt/test/profile/instrprof-set-file-object-merging.c
+++ compiler-rt/test/profile/instrprof-set-file-object-merging.c
@@ -34,7 +34,7 @@
 // CHECK:   17|  2|
 // CHECK:   18|  2|  FILE *F = fopen(argv[1], "r+b");
 // CHECK:   19|  2|  if (!F) {
-// CHECK:   20|  1|// File might not exist, try opening with truncation
+// CHECK:   20|   |// File might not exist, try opening with truncation
 // CHECK:   21|  1|F = fopen(argv[1], "w+b");
 // CHECK:   22|  1|  }
 // CHECK:   23|  2|  __llvm_profile_set_file_object(F, 1);
Index: compiler-rt/test/profile/coverage_comments.cpp
===
--- /dev/null
+++ compiler-rt/test/profile/coverage_comments.cpp
@@ -0,0 +1,71 @@
+// RUN: %clangxx_profgen -fcoverage-mapping -Wno-comment -o %t %s
+// RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t
+// RUN: llvm-profdata merge -o %t.profdata %t.profraw
+// RUN: llvm-cov show %t -instr-profile %t.profdata -path-equivalence=/tmp,%S 2>&1 | FileCheck %s
+
+int main() {   // CHECK:   [[# @LINE]]| 1|int main() {
+/* comment */ int x = 0;   // CHECK-NEXT:  [[# @LINE]]| 1|
+int y = 0; /* comment */   // CHECK-NEXT:  [[# @LINE]]| 1|
+int z = 0; // comment  // CHECK-NEXT:  [[# @LINE]]| 1|
+// comment // CHECK-NEXT:  [[# @LINE]]|  |
+   // CHECK-NEXT:  [[# @LINE]]|  |
+x = 0; /*  // CHECK-NEXT:  [[# @LINE]]| 1|
+comment// CHECK-NEXT:  [[# @LINE]]|  |
+*/ // CHECK-NEXT:  [[# @LINE]]|  |
+   // CHECK-NEXT:  [[# @LINE]]|  |
+/* // CHECK-NEXT:  [[# @LINE]]|  |
+comment// CHECK-NEXT:  [[# @LINE]]|  |
+*/ x = 0;  // CHECK-NEXT:  [[# @LINE]]| 1|
+   // CHECK-NEXT:  [[# @LINE]]|  |
+/* comment */  // CHECK-NEXT:  [[# @LINE]]|  |
+// comment // CHECK-NEXT:  [[# @LINE]]|  |
+/* comment */  // CHECK-NEXT:  [[# @LINE]]|  |
+z =// CHECK-NEXT:  [[# @LINE]]| 1|
+x // comment   // CHECK-NEXT:  [[# @LINE]]| 1|
+// comment // CHECK-NEXT:  [[# @LINE]]|  |
++ /*   // CHECK-NEXT:  [[# @LINE]]| 1|
+comment// CHECK-NEXT:  [[# @LINE]]|  |
+*/ // CHECK-NEXT:  [[# @LINE]]|  |
+/* // CHECK-NEXT:  [[# @LINE]]|  |
+comment// CHECK-NEXT:  [[# @LINE]]|  |
+*/y;   // CHECK-NEXT:  [[# @LINE]]| 1|
+   // CHECK-NEXT:  [[# @LINE]]|  |
+// Comments inside directives. // 

[PATCH] D83987: [libFuzzer] Disable implicit builtin knowledge about memcmp-like functions when -fsanitize=fuzzer-no-link is given.

[Dokyung Song via Phabricator via cfe-commits]

dokyungs updated this revision to Diff 280507.
dokyungs added a comment.

Relanding this reverted commit. (See summary)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83987

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  compiler-rt/test/fuzzer/noasan-memcmp.test
  compiler-rt/test/fuzzer/noasan-memcmp64.test
  compiler-rt/test/fuzzer/noasan-strcmp.test
  compiler-rt/test/fuzzer/noasan-strncmp.test
  compiler-rt/test/fuzzer/noasan-strstr.test

Index: compiler-rt/test/fuzzer/noasan-strstr.test
===
--- compiler-rt/test/fuzzer/noasan-strstr.test
+++ compiler-rt/test/fuzzer/noasan-strstr.test
@@ -1,9 +1,9 @@
 UNSUPPORTED: darwin, freebsd, windows
 
-RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-strstr %S/StrstrTest.cpp -o %t-NoAsanStrstrTest
+RUN: %cpp_compiler -fno-sanitize=address %S/StrstrTest.cpp -o %t-NoAsanStrstrTest
 RUN: not %run %t-NoAsanStrstrTest -seed=1 -runs=200   2>&1 | FileCheck %s
 
-RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-calloc -fno-builtin-strstr %S/CustomAllocator.cpp %S/StrstrTest.cpp -o %t-NoAsanCustomAllocatorStrstrTest
+RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-calloc %S/CustomAllocator.cpp %S/StrstrTest.cpp -o %t-NoAsanCustomAllocatorStrstrTest
 RUN: not %run %t-NoAsanCustomAllocatorStrstrTest -seed=1 -runs=200   2>&1 | FileCheck %s
 
 CHECK: BINGO
Index: compiler-rt/test/fuzzer/noasan-strncmp.test
===
--- compiler-rt/test/fuzzer/noasan-strncmp.test
+++ compiler-rt/test/fuzzer/noasan-strncmp.test
@@ -1,9 +1,9 @@
 UNSUPPORTED: darwin, freebsd, windows
 
-RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-strncmp %S/StrncmpTest.cpp -o %t-NoAsanStrncmpTest
+RUN: %cpp_compiler -fno-sanitize=address %S/StrncmpTest.cpp -o %t-NoAsanStrncmpTest
 RUN: not %run %t-NoAsanStrncmpTest -seed=2 -runs=1000   2>&1 | FileCheck %s
 
-RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-calloc -fno-builtin-strncmp %S/CustomAllocator.cpp %S/StrncmpTest.cpp -o %t-NoAsanCustomAllocatorStrncmpTest
+RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-calloc %S/CustomAllocator.cpp %S/StrncmpTest.cpp -o %t-NoAsanCustomAllocatorStrncmpTest
 RUN: not %run %t-NoAsanCustomAllocatorStrncmpTest -seed=2 -runs=1000   2>&1 | FileCheck %s
 
 CHECK: BINGO
Index: compiler-rt/test/fuzzer/noasan-strcmp.test
===
--- compiler-rt/test/fuzzer/noasan-strcmp.test
+++ compiler-rt/test/fuzzer/noasan-strcmp.test
@@ -1,9 +1,9 @@
 UNSUPPORTED: darwin, freebsd, windows
 
-RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-strcmp %S/StrcmpTest.cpp -o %t-NoAsanStrcmpTest
+RUN: %cpp_compiler -fno-sanitize=address %S/StrcmpTest.cpp -o %t-NoAsanStrcmpTest
 RUN: not %run %t-NoAsanStrcmpTest -seed=1 -runs=200   2>&1 | FileCheck %s
 
-RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-calloc -fno-builtin-strcmp %S/CustomAllocator.cpp %S/StrcmpTest.cpp -o %t-NoAsanCustomAllocatorStrcmpTest
+RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-calloc %S/CustomAllocator.cpp %S/StrcmpTest.cpp -o %t-NoAsanCustomAllocatorStrcmpTest
 RUN: not %run %t-NoAsanCustomAllocatorStrcmpTest -seed=1 -runs=200   2>&1 | FileCheck %s
 
 CHECK: BINGO
Index: compiler-rt/test/fuzzer/noasan-memcmp64.test
===
--- compiler-rt/test/fuzzer/noasan-memcmp64.test
+++ compiler-rt/test/fuzzer/noasan-memcmp64.test
@@ -1,6 +1,6 @@
 UNSUPPORTED: darwin, freebsd, windows
 
-RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-memcmp %S/Memcmp64BytesTest.cpp -o %t-NoAsanMemcmp64BytesTest
+RUN: %cpp_compiler -fno-sanitize=address %S/Memcmp64BytesTest.cpp -o %t-NoAsanMemcmp64BytesTest
 RUN: not %run %t-NoAsanMemcmp64BytesTest -seed=1 -runs=100   2>&1 | FileCheck %s
 
 CHECK: BINGO
Index: compiler-rt/test/fuzzer/noasan-memcmp.test
===
--- compiler-rt/test/fuzzer/noasan-memcmp.test
+++ compiler-rt/test/fuzzer/noasan-memcmp.test
@@ -1,9 +1,9 @@
 UNSUPPORTED: darwin, freebsd, windows
 
-RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-memcmp %S/MemcmpTest.cpp -o %t-NoAsanMemcmpTest
+RUN: %cpp_compiler -fno-sanitize=address %S/MemcmpTest.cpp -o %t-NoAsanMemcmpTest
 RUN: not %run %t-NoAsanMemcmpTest -seed=1 -runs=1000   2>&1 | FileCheck %s
 
-RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-calloc -fno-builtin-memcmp %S/CustomAllocator.cpp %S/MemcmpTest.cpp -o %t-NoAsanCustomAllocatorMemcmpTest
+RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-calloc %S/CustomAllocator.cpp %S/MemcmpTest.cpp -o %t-NoAsanCustomAllocatorMemcmpTest
 RUN: not %run %t-NoAsanCustomAllocatorMemcmpTest -seed=1 -runs=1000   2>&1 | FileCheck %s
 
 CHECK: BINGO
Index: clang/lib/Driver/SanitizerArgs.cpp
==

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

[Joachim Protze via Phabricator via cfe-commits]

protze.joachim added a comment.

This patch breaks compilation of previously working code.

I added the following to 
`openmp/libomptarget/test/offloading/offloading_success.c`:

  +// RUN: %libomptarget-compile-run-and-check-nvptx64-nvidia-cuda

which results in

  # command stderr:
  ptxas offloading_success-openmp-nvptx64-nvidia-cuda.s, line 173; error   : 
Call has wrong number of parameters
  ptxas fatal   : Ptx assembly aborted due to errors
  clang-12: error: ptxas command failed with exit code 255 (use -v to see 
invocation)

The file `offloading_success-openmp-nvptx64-nvidia-cuda.s` contains:

  .func  (.param .b32 func_retval0) __kmpc_kernel_parallel
  (
  .param .b64 __kmpc_kernel_parallel_param_0,
  .param .b32 __kmpc_kernel_parallel_param_1
  )
  ;

For the clang 11 release, we should either fix the codegen or revert this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83268



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

[PATCH] D84348: WIP: Add complete id-expression support to syntax trees

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas updated this revision to Diff 280511.
eduucaldas added a comment.

- Remove UnknownNameSpecifier, answer to comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84348

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -867,24 +867,47 @@
   }
   EXPECT_TRUE(treeDumpEqual(
   R"cpp(
-namespace a {
+namespace n {
   struct S {
 template
-static T f(){}
+struct TS {
+  static void f();
+};
   };
 }
+template
+struct TS {
+  struct S {
+template
+static U f();
+  };
+};
 void test() {
-  ::  // global-namespace-specifier
-  a:: // namespace-specifier
-  S:: // type-name-specifier
+  :: // global-namespace-specifier
+  n::// namespace-specifier
+  S::// type-name-specifier
+  template TS:: // type-template-instantiation-specifier
+  f();
+
+  n::// namespace-specifier
+  S::// type-name-specifier
+  TS::  // type-template-instantiation-specifier
+  f();
+
+  TS:: // type-name-specifier
+  S::   // type-name-specifier
   f();
+
+  TS:: // type-name-specifier
+  S::   // type-name-specifier
+  template f();
 }
 )cpp",
   R"txt(
 *: TranslationUnit
 |-NamespaceDefinition
 | |-namespace
-| |-a
+| |-n
 | |-{
 | |-SimpleDeclaration
 | | |-struct
@@ -898,19 +921,58 @@
 | | | | `-T
 | | | |->
 | | | `-SimpleDeclaration
-| | |   |-static
-| | |   |-T
-| | |   |-SimpleDeclarator
-| | |   | |-f
-| | |   | `-ParametersAndQualifiers
-| | |   |   |-(
-| | |   |   `-)
-| | |   `-CompoundStatement
-| | | |-{
-| | | `-}
+| | |   |-struct
+| | |   |-TS
+| | |   |-{
+| | |   |-SimpleDeclaration
+| | |   | |-static
+| | |   | |-void
+| | |   | |-SimpleDeclarator
+| | |   | | |-f
+| | |   | | `-ParametersAndQualifiers
+| | |   | |   |-(
+| | |   | |   `-)
+| | |   | `-;
+| | |   |-}
+| | |   `-;
 | | |-}
 | | `-;
 | `-}
+|-TemplateDeclaration
+| |-template
+| |-<
+| |-UnknownDeclaration
+| | |-typename
+| | `-T
+| |->
+| `-SimpleDeclaration
+|   |-struct
+|   |-TS
+|   |-{
+|   |-SimpleDeclaration
+|   | |-struct
+|   | |-S
+|   | |-{
+|   | |-TemplateDeclaration
+|   | | |-template
+|   | | |-<
+|   | | |-UnknownDeclaration
+|   | | | |-typename
+|   | | | `-U
+|   | | |->
+|   | | `-SimpleDeclaration
+|   | |   |-static
+|   | |   |-U
+|   | |   |-SimpleDeclarator
+|   | |   | |-f
+|   | |   | `-ParametersAndQualifiers
+|   | |   |   |-(
+|   | |   |   `-)
+|   | |   `-;
+|   | |-}
+|   | `-;
+|   |-}
+|   `-;
 `-SimpleDeclaration
   |-void
   |-SimpleDeclarator
@@ -924,14 +986,81 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | |-NameSpecifier
-| | | | | `-::
-| | | | |-NameSpecifier
-| | | | | |-a
-| | | | | `-::
-| | | | `-NameSpecifier
-| | | |   |-S
-| | | |   `-::
+| | | | |-::
+| | | | |-IdentifierNameSpecifier
+| | | | | `-n
+| | | | |-::
+| | | | |-IdentifierNameSpecifier
+| | | | | `-S
+| | | | |-::
+| | | | |-SimpleTemplateNameSpecifier
+| | | | | |-template
+| | | | | |-TS
+| | | | | |-<
+| | | | | |-int
+| | | | | `->
+| | | | `-::
+| | | `-UnqualifiedId
+| | |   `-f
+| | |-(
+| | `-)
+| `-;
+|-ExpressionStatement
+| |-UnknownExpression
+| | |-IdExpression
+| | | |-NestedNameSpecifier
+| | | | |-IdentifierNameSpecifier
+| | | | | `-n
+| | | | |-::
+| | | | |-IdentifierNameSpecifier
+| | | | | `-S
+| | | | |-::
+| | | | |-SimpleTemplateNameSpecifier
+| | | | | |-TS
+| | | | | |-<
+| | | | | |-int
+| | | | | `->
+| | | | `-::
+| | | `-UnqualifiedId
+| | |   `-f
+| | |-(
+| | `-)
+| `-;
+|-ExpressionStatement
+| |-UnknownExpression
+| | |-IdExpression
+| | | |-NestedNameSpecifier
+| | | | |-SimpleTemplateNameSpecifier
+| | | | | |-TS
+| | | | | |-<
+| | | | | |-int
+| | | | | `->
+| | | | |-::
+| | | | |-IdentifierNameSpecifier
+| | | | | `-S
+| | | | `-::
+| | | `-UnqualifiedId
+| | |   |-f
+| | |   |-<
+| | |   |-int
+| | |   `->
+| | |-(
+| | `-)
+| `-;
+|-ExpressionStatement
+| |-UnknownExpression
+| | |-IdExpression
+| | | |-NestedNameSpecifier
+| | | | |-SimpleTemplateNameSpecifier
+| | | | | |-TS
+| | | | | |-<
+| | | | | |-int
+| | | | | `->
+| | | | |-::
+| | | | |-IdentifierNameSpecifier
+| | | | | `-S
+   

[PATCH] D84005: Introduce ns_error_domain attribute.

[Doug Gregor via Phabricator via cfe-commits]

doug.gregor added a comment.

In D84005#2171982 , @MForster wrote:

> @milseman, @doug.gregor, could you please help with the open questions on 
> this review?
>
> Specifically:
>
> - Is `ns_error_domain` ever needed for something other than an enum?


No, it only makes sense on enums.

> - Why is this designed in the way it is (requiring an identifier for the 
> domain, not allowing literals and then only using the name of the identifier 
> from Swift)?

It's codifying the design of NSError, which has been this way since... longer 
than Clang has existed, and is independent of Swift. NSErrors have a domain 
(identified by an NSString constant symbol, not a literal, for pointer 
uniqueness and code size)  and a code (an integer, conventionally defined by an 
enum). The two need to be used together, e.g., you create an NSError with a 
domain and a code from that domain. This attribute finally ties those things 
together at the source level.

This is leads to the answer to Aaron's question:

> Are there plans to use this attribute in Clang itself?

It would absolutely make sense to add some warnings if you've mismatched your 
domain and code when constructing an NSError (e.g., uses of 
https://developer.apple.com/documentation/foundation/nserror/1522782-errorwithdomain?language=objc)
 or even if when testing an error in an "if" statement (if checking both domain 
and code, make sure the code enumerator is from the same domain). I bet you'd 
catch some fiddly error-handling bugs this way.

> - Is it ok to make this attribute inheritable?

Sure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84005



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

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

[Joachim Protze via Phabricator via cfe-commits]

protze.joachim reopened this revision.
protze.joachim added a comment.
This revision is now accepted and ready to land.

I carefully made sure, that the freshly built clang was used to execute the 
test. I opened https://bugs.llvm.org/show_bug.cgi?id=46836 to track the issue 
and made it release blocker.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83268



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

[PATCH] D83592: [Coverage] Add comment to skipped regions

[Hans Wennborg via Phabricator via cfe-commits]

hans added inline comments.



Comment at: clang/lib/Lex/Preprocessor.cpp:973
+  if ((LexLevel == 0 || PreprocessToken) &&
+  !Result.getFlag(Token::IsReinjected)) {
+if (LexLevel == 0)

zequanwu wrote:
> hans wrote:
> > zequanwu wrote:
> > > @hans , can you take a look on this part? I saw `TokenCount` was 
> > > introduced for a warning `-Wmax-tokens`.
> > Can you explain why things are changing here?
> > 
> > The idea of TokenCount is to simply count the preprocessor tokens. At this 
> > point I think you have more knowledge here than I did when I added it :)
> In `CodeGen::CoverageMappingModuleGen::setUpCoverageCallbacks`, I set 
> `OnToken` to a lambda to update `PrevTokLoc` after lexing a new token. But, 
> the original condition ` if (LexLevel == 0 && 
> !Result.getFlag(Token::IsReinjected)) ` will not call `OnToken` if it's  
> lexing a directive, like `#if`, `#define` etc, because the LexLevel is 
> greater than 0. In order to update `PrevTokLoc` even when lexing directive, I 
> add `PreprocessToken` which will be set to true in 
> `CodeGen::CoverageMappingModuleGen::setUpCoverageCallbacks` just to invoke 
> `OnToken` without increment `TokenCount` when lexing directives. 
Ah, I see. That sounds okay to me.


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

https://reviews.llvm.org/D83592



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

[PATCH] D84476: Make hip math headers easier to use from C

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/lib/Headers/__clang_hip_math.h:98
 // BEGIN FLOAT
+#ifdef _cplusplus
 __DEVICE__

typo ?



Comment at: clang/lib/Headers/__clang_hip_math.h:558
 // BEGIN DOUBLE
+#ifdef _cplusplus
 __DEVICE__

typo



Comment at: clang/lib/Headers/__clang_hip_math.h:561
 inline double abs(double __x) { return __ocml_fabs_f64(__x); }
+#endif
 __DEVICE__

jdoerfert wrote:
> Nit: You mix the C and C++ math declarations in this file, while possible, I 
> somehow thing the cuda_{cmath/math} split is nicer.
right



Comment at: clang/lib/Headers/__clang_hip_math.h:1185
 
+#ifdef _cplusplus
 __DEVICE__

typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84476



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