[clang-tools-extra] 8080ea4 - [clangd] Enable reflection for clangd-index-server

2021-03-10 Thread Kirill Bobyrev via cfe-commits

Author: Kirill Bobyrev
Date: 2021-03-10T09:07:39+01:00
New Revision: 8080ea4c4b8c456c72c617587cc32f174b3105c1

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

LOG: [clangd] Enable reflection for clangd-index-server

This allows sending requests through CLI and more debugging
opportunities. Example:

```bash
$ grpc_cli ls localhost:50051
clang.clangd.remote.v1.SymbolIndex
grpc.reflection.v1alpha.ServerReflection
grpc.health.v1.Health
```

Added: 


Modified: 
clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
clang-tools-extra/clangd/index/remote/server/Server.cpp
llvm/cmake/modules/FindGRPC.cmake

Removed: 




diff  --git a/clang-tools-extra/clangd/index/remote/server/CMakeLists.txt 
b/clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
index e6959db6bbd8..3ba0c476df0c 100644
--- a/clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
+++ b/clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
@@ -17,4 +17,6 @@ target_link_libraries(clangd-index-server
   RemoteIndexProto
   RemoteIndexServiceProto
   clangdRemoteMarshalling
+
+  grpc++_reflection
   )

diff  --git a/clang-tools-extra/clangd/index/remote/server/Server.cpp 
b/clang-tools-extra/clangd/index/remote/server/Server.cpp
index 3de2c38f7c08..768f3c9fd143 100644
--- a/clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ b/clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -30,6 +30,7 @@
 #include "llvm/Support/VirtualFileSystem.h"
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -313,6 +314,7 @@ void runServerAndWait(clangd::SymbolIndex &Index, 
llvm::StringRef ServerAddress,
   RemoteIndexServer Service(Index, IndexRoot);
 
   grpc::EnableDefaultHealthCheckService(true);
+  grpc::reflection::InitProtoReflectionServerBuilderPlugin();
   grpc::ServerBuilder Builder;
   Builder.AddListeningPort(ServerAddress.str(),
grpc::InsecureServerCredentials());

diff  --git a/llvm/cmake/modules/FindGRPC.cmake 
b/llvm/cmake/modules/FindGRPC.cmake
index 8fdb3506dff1..d39d10e4e93c 100644
--- a/llvm/cmake/modules/FindGRPC.cmake
+++ b/llvm/cmake/modules/FindGRPC.cmake
@@ -22,6 +22,8 @@ if (GRPC_INSTALL_PATH)
   add_library(protobuf ALIAS protobuf::libprotobuf)
   set_target_properties(gRPC::grpc++ PROPERTIES IMPORTED_GLOBAL TRUE)
   add_library(grpc++ ALIAS gRPC::grpc++)
+  set_target_properties(gRPC::grpc++_reflection PROPERTIES IMPORTED_GLOBAL 
TRUE)
+  add_library(grpc++_reflection ALIAS gRPC::grpc++_reflection)
 
   set(GRPC_CPP_PLUGIN $)
   set(PROTOC ${Protobuf_PROTOC_EXECUTABLE})
@@ -71,6 +73,9 @@ else()
   add_library(grpc++ UNKNOWN IMPORTED GLOBAL)
   message(STATUS "Using grpc++: " ${GRPC_LIBRARY})
   set_target_properties(grpc++ PROPERTIES IMPORTED_LOCATION ${GRPC_LIBRARY})
+  find_library(GRPC_REFLECTION_LIBRARY grpc++_reflection $GRPC_OPTS REQUIRED)
+  add_library(grpc++_reflection UNKNOWN IMPORTED GLOBAL)
+  set_target_properties(grpc++_reflection PROPERTIES IMPORTED_LOCATION 
${GRPC_REFLECTION_LIBRARY})
   find_library(PROTOBUF_LIBRARY protobuf $PROTOBUF_OPTS REQUIRED)
   message(STATUS "Using protobuf: " ${PROTOBUF_LIBRARY})
   add_library(protobuf UNKNOWN IMPORTED GLOBAL)



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


[PATCH] D98246: [clangd] Add basic monitoring info request for remote index server

2021-03-10 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 329553.
kbobyrev marked an inline comment as done.
kbobyrev added a comment.

Enabled server reflection in 
https://github.com/llvm/llvm-project/commit/8080ea4c4b8c456c72c617587cc32f174b3105c1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98246

Files:
  clang-tools-extra/clangd/index/remote/Service.proto
  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
@@ -91,7 +91,7 @@
 class RemoteIndexServer final : public v1::SymbolIndex::Service {
 public:
   RemoteIndexServer(clangd::SymbolIndex &Index, llvm::StringRef IndexRoot)
-  : Index(Index) {
+  : Index(Index), ServiceStartTime(std::chrono::system_clock::now()) {
 llvm::SmallString<256> NativePath = IndexRoot;
 llvm::sys::path::native(NativePath);
 ProtobufMarshaller = std::unique_ptr(new Marshaller(
@@ -280,8 +280,23 @@
 Millis);
   }
 
+  grpc::Status MonitoringInfo(grpc::ServerContext *Context,
+  const google::protobuf::Empty *Request,
+  v1::MonitoringInfoReply *Reply) override {
+auto Uptime = std::chrono::duration_cast(
+  std::chrono::system_clock::now() - ServiceStartTime)
+  .count();
+Reply->set_info(llvm::formatv(
+"Service started at {0:%Y-%m-%d %H:%M:%S}. Uptime: {1} h {2} m.",
+ServiceStartTime, Uptime / 60, Uptime % 60));
+logRequest(*Request);
+logResponse(*Reply);
+return grpc::Status::OK;
+  }
+
   std::unique_ptr ProtobufMarshaller;
   clangd::SymbolIndex &Index;
+  llvm::sys::TimePoint<> ServiceStartTime;
 };
 
 // Detect changes in \p IndexPath file and load new versions of the index
Index: clang-tools-extra/clangd/index/remote/Service.proto
===
--- clang-tools-extra/clangd/index/remote/Service.proto
+++ clang-tools-extra/clangd/index/remote/Service.proto
@@ -10,8 +10,11 @@
 
 package clang.clangd.remote.v1;
 
+import "google/protobuf/empty.proto";
 import "Index.proto";
 
+message MonitoringInfoReply { optional string info = 1; }
+
 // Semantics of SymbolIndex match clangd::SymbolIndex with all required
 // structures corresponding to their clangd::* counterparts.
 service SymbolIndex {
@@ -22,5 +25,7 @@
   rpc Refs(RefsRequest) returns (stream RefsReply) {}
 
   rpc Relations(RelationsRequest) returns (stream RelationsReply) {}
+
+  rpc MonitoringInfo(google.protobuf.Empty) returns (MonitoringInfoReply) {}
 }
 


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
@@ -91,7 +91,7 @@
 class RemoteIndexServer final : public v1::SymbolIndex::Service {
 public:
   RemoteIndexServer(clangd::SymbolIndex &Index, llvm::StringRef IndexRoot)
-  : Index(Index) {
+  : Index(Index), ServiceStartTime(std::chrono::system_clock::now()) {
 llvm::SmallString<256> NativePath = IndexRoot;
 llvm::sys::path::native(NativePath);
 ProtobufMarshaller = std::unique_ptr(new Marshaller(
@@ -280,8 +280,23 @@
 Millis);
   }
 
+  grpc::Status MonitoringInfo(grpc::ServerContext *Context,
+  const google::protobuf::Empty *Request,
+  v1::MonitoringInfoReply *Reply) override {
+auto Uptime = std::chrono::duration_cast(
+  std::chrono::system_clock::now() - ServiceStartTime)
+  .count();
+Reply->set_info(llvm::formatv(
+"Service started at {0:%Y-%m-%d %H:%M:%S}. Uptime: {1} h {2} m.",
+ServiceStartTime, Uptime / 60, Uptime % 60));
+logRequest(*Request);
+logResponse(*Reply);
+return grpc::Status::OK;
+  }
+
   std::unique_ptr ProtobufMarshaller;
   clangd::SymbolIndex &Index;
+  llvm::sys::TimePoint<> ServiceStartTime;
 };
 
 // Detect changes in \p IndexPath file and load new versions of the index
Index: clang-tools-extra/clangd/index/remote/Service.proto
===
--- clang-tools-extra/clangd/index/remote/Service.proto
+++ clang-tools-extra/clangd/index/remote/Service.proto
@@ -10,8 +10,11 @@
 
 package clang.clangd.remote.v1;
 
+import "google/protobuf/empty.proto";
 import "Index.proto";
 
+message MonitoringInfoReply { optional string info = 1; }
+
 // Semantics of SymbolIndex match clangd::SymbolIndex with all required
 // structures corresponding to their clangd::* counterparts.
 service SymbolIndex {
@@ -22,5 +25,7 @@
   rpc Refs(RefsRequest) returns (st

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

2021-03-10 Thread Anton Zabaznov via Phabricator via cfe-commits
azabaznov updated this revision to Diff 329554.
azabaznov added a comment.

Corrected some mistakes, added a test for diagnosing undeclared identifiers 
when a extension is unsupported. Generally leaving the change as it is as 
completely removing pragma may break backward compatibility now: let's do it in 
a separate patch and notify everyone before doing that. Also, we should wait 
before spec will be finalized.

Note, that adding a test for diagnosing extended atomic types is hard because 
parser diagnoses a typo (since some types, such as atomic_int, are already 
present), so diagnostic messages look awkward.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97058

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/OpenCLOptions.h
  clang/lib/Basic/OpenCLOptions.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/Parser/opencl-atomics-cl20.cl
  clang/test/SemaOpenCL/access-qualifier.cl
  clang/test/SemaOpenCL/cl20-device-side-enqueue.cl
  clang/test/SemaOpenCL/extension-begin.cl
  clang/test/SemaOpenCL/extensions.cl
  clang/test/SemaOpenCL/intel-subgroup-avc-ext-types.cl

Index: clang/test/SemaOpenCL/intel-subgroup-avc-ext-types.cl
===
--- clang/test/SemaOpenCL/intel-subgroup-avc-ext-types.cl
+++ clang/test/SemaOpenCL/intel-subgroup-avc-ext-types.cl
@@ -1,6 +1,9 @@
-// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=CL1.2 -cl-ext=+cl_intel_device_side_avc_motion_estimation -fsyntax-only -verify %s
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=CL1.2 -cl-ext=+cl_intel_device_side_avc_motion_estimation -fsyntax-only -verify -DEXT %s
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=CL1.2 -cl-ext=-cl_intel_device_side_avc_motion_estimation -fsyntax-only -verify  %s
 
+#ifdef cl_intel_device_side_avc_motion_estimation
 #pragma OPENCL EXTENSION cl_intel_device_side_avc_motion_estimation : enable
+#endif
 
 // All intel_sub_group_avc_* types can only be used as argument or return value
 // of built-in functions defined in the extension.
@@ -16,55 +19,77 @@
 // negative test cases for initializers
 void foo(char c, float f, void* v, struct st ss) {
   intel_sub_group_avc_mce_payload_t payload_mce = 0; // No zero initializer for mce types
-  // expected-error@-1 {{initializing '__private intel_sub_group_avc_mce_payload_t' with an expression of incompatible type 'int'}}
   intel_sub_group_avc_ime_payload_t payload_ime = 1; // No literal initializer for *payload_t types
-  // expected-error@-1 {{initializing '__private intel_sub_group_avc_ime_payload_t' with an expression of incompatible type 'int'}}
   intel_sub_group_avc_ref_payload_t payload_ref = f;
-  // expected-error@-1 {{initializing '__private intel_sub_group_avc_ref_payload_t' with an expression of incompatible type '__private float'}}
   intel_sub_group_avc_sic_payload_t payload_sic = ss;
-  // expected-error@-1 {{initializing '__private intel_sub_group_avc_sic_payload_t' with an expression of incompatible type '__private struct st'}}
-
   intel_sub_group_avc_mce_result_t result_mce = 0; // No zero initializer for mce types
-  // expected-error@-1 {{initializing '__private intel_sub_group_avc_mce_result_t' with an expression of incompatible type 'int'}}
   intel_sub_group_avc_ime_result_t result_ime = 1; // No literal initializer for *result_t types
-  // expected-error@-1 {{initializing '__private intel_sub_group_avc_ime_result_t' with an expression of incompatible type 'int'}}
   intel_sub_group_avc_ref_result_t result_ref = f;
-  // expected-error@-1 {{initializing '__private intel_sub_group_avc_ref_result_t' with an expression of incompatible type '__private float'}}
   intel_sub_group_avc_sic_result_t result_sic = ss;
-  // expected-error@-1 {{initializing '__private intel_sub_group_avc_sic_result_t' with an expression of incompatible type '__private struct st'}}
-
   intel_sub_group_avc_ime_result_single_reference_streamout_t sstreamout = v;
-  // expected-error@-1 {{initializing '__private intel_sub_group_avc_ime_result_single_reference_streamout_t' with an expression of incompatible type '__private void *__private'}}
-
   intel_sub_group_avc_ime_result_dual_reference_streamout_t dstreamin_list = {0x0, 0x1};
-  // expected-warning@-1 {{excess elements in struct initializer}}
   intel_sub_group_avc_ime_dual_reference_streamin_t dstreamin_list2 = {};
-  // expected-error@-1 {{scalar initializer cannot be empty}}
   intel_sub_group_avc_ime_single_reference_streamin_t dstreamin_list3 = {c};
-  // expected-error@-1 {{initializing '__private in

[clang] ea8e5b8 - [NFC] Remove duplicate isNoBuiltinFunc method

2021-03-10 Thread via cfe-commits

Author: serge-sans-paille
Date: 2021-03-10T09:18:55+01:00
New Revision: ea8e5b87acba4b7dad749d697a3969901652b97a

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

LOG: [NFC] Remove duplicate isNoBuiltinFunc method

It's available both in CodeGenOptions and in LangOptions, and LangOptions
implementation is slightly better as it uses a StringRef instead of a char
pointer, so use it.

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

Added: 


Modified: 
clang/include/clang/Basic/CodeGenOptions.h
clang/lib/Basic/CodeGenOptions.cpp
clang/lib/CodeGen/CGCall.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/CodeGenOptions.h 
b/clang/include/clang/Basic/CodeGenOptions.h
index 40068b4f6649..b38df2da97de 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -415,10 +415,6 @@ class CodeGenOptions : public CodeGenOptionsBase {
 
   CodeGenOptions();
 
-  /// Is this a libc/libm function that is no longer recognized as a
-  /// builtin because a -fno-builtin-* option has been specified?
-  bool isNoBuiltinFunc(const char *Name) const;
-
   const std::vector &getNoBuiltinFuncs() const {
 return NoBuiltinFuncs;
   }

diff  --git a/clang/lib/Basic/CodeGenOptions.cpp 
b/clang/lib/Basic/CodeGenOptions.cpp
index 4fc7a535c9eb..0c609cfa61de 100644
--- a/clang/lib/Basic/CodeGenOptions.cpp
+++ b/clang/lib/Basic/CodeGenOptions.cpp
@@ -20,12 +20,4 @@ CodeGenOptions::CodeGenOptions() {
   memcpy(CoverageVersion, "408*", 4);
 }
 
-bool CodeGenOptions::isNoBuiltinFunc(const char *Name) const {
-  StringRef FuncName(Name);
-  for (unsigned i = 0, e = NoBuiltinFuncs.size(); i != e; ++i)
-if (FuncName.equals(NoBuiltinFuncs[i]))
-  return true;
-  return false;
-}
-
 }  // end namespace clang

diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index da993bed7ba0..34ad782d810c 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -1765,8 +1765,7 @@ void 
CodeGenModule::getDefaultFunctionAttributes(StringRef Name,
 
   if (AttrOnCallSite) {
 // Attributes that should go on the call site only.
-if (!CodeGenOpts.SimplifyLibCalls ||
-CodeGenOpts.isNoBuiltinFunc(Name.data()))
+if (!CodeGenOpts.SimplifyLibCalls || LangOpts.isNoBuiltinFunc(Name))
   FuncAttrs.addAttribute(llvm::Attribute::NoBuiltin);
 if (!CodeGenOpts.TrapFuncName.empty())
   FuncAttrs.addAttribute("trap-func-name", CodeGenOpts.TrapFuncName);



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


[PATCH] D98175: [NFC] Remove duplicate isNoBuiltinFunc method

2021-03-10 Thread serge via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGea8e5b87acba: [NFC] Remove duplicate isNoBuiltinFunc method 
(authored by serge-sans-paille).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98175

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/lib/Basic/CodeGenOptions.cpp
  clang/lib/CodeGen/CGCall.cpp


Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1765,8 +1765,7 @@
 
   if (AttrOnCallSite) {
 // Attributes that should go on the call site only.
-if (!CodeGenOpts.SimplifyLibCalls ||
-CodeGenOpts.isNoBuiltinFunc(Name.data()))
+if (!CodeGenOpts.SimplifyLibCalls || LangOpts.isNoBuiltinFunc(Name))
   FuncAttrs.addAttribute(llvm::Attribute::NoBuiltin);
 if (!CodeGenOpts.TrapFuncName.empty())
   FuncAttrs.addAttribute("trap-func-name", CodeGenOpts.TrapFuncName);
Index: clang/lib/Basic/CodeGenOptions.cpp
===
--- clang/lib/Basic/CodeGenOptions.cpp
+++ clang/lib/Basic/CodeGenOptions.cpp
@@ -20,12 +20,4 @@
   memcpy(CoverageVersion, "408*", 4);
 }
 
-bool CodeGenOptions::isNoBuiltinFunc(const char *Name) const {
-  StringRef FuncName(Name);
-  for (unsigned i = 0, e = NoBuiltinFuncs.size(); i != e; ++i)
-if (FuncName.equals(NoBuiltinFuncs[i]))
-  return true;
-  return false;
-}
-
 }  // end namespace clang
Index: clang/include/clang/Basic/CodeGenOptions.h
===
--- clang/include/clang/Basic/CodeGenOptions.h
+++ clang/include/clang/Basic/CodeGenOptions.h
@@ -415,10 +415,6 @@
 
   CodeGenOptions();
 
-  /// Is this a libc/libm function that is no longer recognized as a
-  /// builtin because a -fno-builtin-* option has been specified?
-  bool isNoBuiltinFunc(const char *Name) const;
-
   const std::vector &getNoBuiltinFuncs() const {
 return NoBuiltinFuncs;
   }


Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1765,8 +1765,7 @@
 
   if (AttrOnCallSite) {
 // Attributes that should go on the call site only.
-if (!CodeGenOpts.SimplifyLibCalls ||
-CodeGenOpts.isNoBuiltinFunc(Name.data()))
+if (!CodeGenOpts.SimplifyLibCalls || LangOpts.isNoBuiltinFunc(Name))
   FuncAttrs.addAttribute(llvm::Attribute::NoBuiltin);
 if (!CodeGenOpts.TrapFuncName.empty())
   FuncAttrs.addAttribute("trap-func-name", CodeGenOpts.TrapFuncName);
Index: clang/lib/Basic/CodeGenOptions.cpp
===
--- clang/lib/Basic/CodeGenOptions.cpp
+++ clang/lib/Basic/CodeGenOptions.cpp
@@ -20,12 +20,4 @@
   memcpy(CoverageVersion, "408*", 4);
 }
 
-bool CodeGenOptions::isNoBuiltinFunc(const char *Name) const {
-  StringRef FuncName(Name);
-  for (unsigned i = 0, e = NoBuiltinFuncs.size(); i != e; ++i)
-if (FuncName.equals(NoBuiltinFuncs[i]))
-  return true;
-  return false;
-}
-
 }  // end namespace clang
Index: clang/include/clang/Basic/CodeGenOptions.h
===
--- clang/include/clang/Basic/CodeGenOptions.h
+++ clang/include/clang/Basic/CodeGenOptions.h
@@ -415,10 +415,6 @@
 
   CodeGenOptions();
 
-  /// Is this a libc/libm function that is no longer recognized as a
-  /// builtin because a -fno-builtin-* option has been specified?
-  bool isNoBuiltinFunc(const char *Name) const;
-
   const std::vector &getNoBuiltinFuncs() const {
 return NoBuiltinFuncs;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D98175: [NFC] Remove duplicate isNoBuiltinFunc method

2021-03-10 Thread serge via Phabricator via cfe-commits
serge-sans-paille added a comment.

That was a very convincing argument :-) tag added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98175

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


[PATCH] D98214: [clang-format] Fix aligning with linebreaks

2021-03-10 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments.



Comment at: clang/lib/Format/WhitespaceManager.cpp:332-340
   if (Changes[ScopeStart - 1].Tok->is(TT_FunctionDeclarationName) ||
   (ScopeStart > Start + 1 &&
Changes[ScopeStart - 2].Tok->is(TT_FunctionDeclarationName)) ||
+  (ScopeStart > Start + 1 &&
+   Changes[ScopeStart - 2].Tok->is(tok::identifier) &&
+   Changes[ScopeStart - 1].Tok->is(tok::l_paren)) ||
   Changes[i].Tok->is(TT_ConditionalExpr) ||

HazardyKnusperkeks wrote:
> curdeius wrote:
> > Would it be possible to break up this condition and name it (or name its 
> > parts)? It's getting hard to follow.
> > Suggestion according to my understanding, which might be wrong.
> Can do, but then all those are always checked, there is no short circuit 
> anymore. But I really get your point.
> 
> How about a lambda with different returns (and comments), that way we would 
> still short circuit. Or some thing like
> ```
> bool AddShift = /* checks #1 */;
> AddShift = AddShift || /* checks #2 */;
> ...
> AddShift = AddShoft || /* checks #n */;
> 
> if (AddShift)
>   Changes[i].Spaces += Shift;
> ```
Good point on the short circuiting. A lambda may be a good solution here. But 
the `bool AddShift ...` that you suggested above (with comments too) is ok for 
me as well. I let you choose.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98214

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


[PATCH] D98070: [clang-tidy] Add option to ignore macros in readability-function-cognitive-complexity check.

2021-03-10 Thread Jens Massberg via Phabricator via cfe-commits
massberg updated this revision to Diff 329559.
massberg added a comment.

Sync to head.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98070

Files:
  clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
  clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.h
  
clang-tools-extra/docs/clang-tidy/checks/readability-function-cognitive-complexity.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-function-cognitive-complexity-flags.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-function-cognitive-complexity-flags.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-function-cognitive-complexity-flags.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-function-cognitive-complexity-flags.cpp
@@ -10,9 +10,18 @@
 // RUN:   value: 5}, \
 // RUN:  {key: readability-function-cognitive-complexity.DescribeBasicIncrements, \
 // RUN:   value: "false"} ]}'
+// RUN: %check_clang_tidy -check-suffix=IGNORE-MACROS %s readability-function-cognitive-complexity %t -- \
+// RUN:   -config='{CheckOptions: \
+// RUN: [{key: readability-function-cognitive-complexity.Threshold, \
+// RUN:   value: 0}, \
+// RUN:  {key: readability-function-cognitive-complexity.IgnoreMacros, \
+// RUN:   value: "true"}, \
+// RUN:  {key: readability-function-cognitive-complexity.DescribeBasicIncrements, \
+// RUN:   value: "false"} ]}'
 
 void func_of_complexity_4() {
   // CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'func_of_complexity_4' has cognitive complexity of 4 (threshold 0) [readability-function-cognitive-complexity]
+  // CHECK-NOTES-IGNORE-MACROS: :[[@LINE-2]]:6: warning: function 'func_of_complexity_4' has cognitive complexity of 4 (threshold 0) [readability-function-cognitive-complexity]
   if (1) {
 if (1) {
 }
@@ -34,9 +43,41 @@
 void function_with_macro() {
   // CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'function_with_macro' has cognitive complexity of 11 (threshold 0) [readability-function-cognitive-complexity]
   // CHECK-NOTES-THRESHOLD5: :[[@LINE-2]]:6: warning: function 'function_with_macro' has cognitive complexity of 11 (threshold 5) [readability-function-cognitive-complexity]
+  // CHECK-NOTES-IGNORE-MACROS: :[[@LINE-3]]:6: warning: function 'function_with_macro' has cognitive complexity of 1 (threshold 0) [readability-function-cognitive-complexity]
 
   MacroOfComplexity10;
 
   if (1) {
   }
 }
+
+#define noop \
+  {}
+
+#define SomeMacro(x) \
+  if (1) {   \
+x;   \
+  }
+
+void func_macro_1() {
+  // CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'func_macro_1' has cognitive complexity of 2 (threshold 0) [readability-function-cognitive-complexity]
+  // CHECK-NOTES-IGNORE-MACROS: :[[@LINE-2]]:6: warning: function 'func_macro_1' has cognitive complexity of 1 (threshold 0) [readability-function-cognitive-complexity]
+
+  if (1) {
+  }
+  SomeMacro(noop);
+}
+
+void func_macro_2() {
+  // CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'func_macro_2' has cognitive complexity of 4 (threshold 0) [readability-function-cognitive-complexity]
+  // CHECK-NOTES-IGNORE-MACROS: :[[@LINE-2]]:6: warning: function 'func_macro_2' has cognitive complexity of 1 (threshold 0) [readability-function-cognitive-complexity]
+
+  if (1) {
+  }
+  // Note that if the IgnoreMacro option is set to 'true', currently also macro
+  // arguments are ignored. Optimally, macros should be treated like function
+  // calls, i.e. the arguments account to the complexity so that the overall
+  // complexity of this function is 2 (1 for the if statement above + 1 for
+  // the if statement in the argument).
+  SomeMacro(if (1) { noop; });
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-function-cognitive-complexity.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-function-cognitive-complexity.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-function-cognitive-complexity.rst
@@ -24,6 +24,12 @@
`if` statement, etc.) which contributes to that complexity. See also the
examples below. Default is `true`.
 
+.. option:: IgnoreMacros
+
+   If set to `true`, the check will ignore code inside macros. Note, that also
+   any macro arguments are ignored, even if they should count to the complexity.
+   Default is `false`.
+
 Building blocks
 ---
 
Index: clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.h
===
--- clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.h
+++ clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.h
@

[PATCH] D97743: Define __GCC_HAVE_DWARF2_CFI_ASM if applicable

2021-03-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

There are some side effects: because -D is passed as command line options, it 
has `` in its filename, so `isWrittenInBuiltinFile` will not 
match it (`isWrittenInBuiltinFile` can match other built-in macros)...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97743

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


[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

2021-03-10 Thread Anton Zabaznov via Phabricator via cfe-commits
azabaznov updated this revision to Diff 329560.
azabaznov added a comment.

Replaced atomic_double implicit definition


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97058

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/OpenCLOptions.h
  clang/lib/Basic/OpenCLOptions.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/Parser/opencl-atomics-cl20.cl
  clang/test/SemaOpenCL/access-qualifier.cl
  clang/test/SemaOpenCL/cl20-device-side-enqueue.cl
  clang/test/SemaOpenCL/extension-begin.cl
  clang/test/SemaOpenCL/extensions.cl
  clang/test/SemaOpenCL/intel-subgroup-avc-ext-types.cl

Index: clang/test/SemaOpenCL/intel-subgroup-avc-ext-types.cl
===
--- clang/test/SemaOpenCL/intel-subgroup-avc-ext-types.cl
+++ clang/test/SemaOpenCL/intel-subgroup-avc-ext-types.cl
@@ -1,6 +1,9 @@
-// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=CL1.2 -cl-ext=+cl_intel_device_side_avc_motion_estimation -fsyntax-only -verify %s
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=CL1.2 -cl-ext=+cl_intel_device_side_avc_motion_estimation -fsyntax-only -verify -DEXT %s
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=CL1.2 -cl-ext=-cl_intel_device_side_avc_motion_estimation -fsyntax-only -verify  %s
 
+#ifdef cl_intel_device_side_avc_motion_estimation
 #pragma OPENCL EXTENSION cl_intel_device_side_avc_motion_estimation : enable
+#endif
 
 // All intel_sub_group_avc_* types can only be used as argument or return value
 // of built-in functions defined in the extension.
@@ -16,55 +19,77 @@
 // negative test cases for initializers
 void foo(char c, float f, void* v, struct st ss) {
   intel_sub_group_avc_mce_payload_t payload_mce = 0; // No zero initializer for mce types
-  // expected-error@-1 {{initializing '__private intel_sub_group_avc_mce_payload_t' with an expression of incompatible type 'int'}}
   intel_sub_group_avc_ime_payload_t payload_ime = 1; // No literal initializer for *payload_t types
-  // expected-error@-1 {{initializing '__private intel_sub_group_avc_ime_payload_t' with an expression of incompatible type 'int'}}
   intel_sub_group_avc_ref_payload_t payload_ref = f;
-  // expected-error@-1 {{initializing '__private intel_sub_group_avc_ref_payload_t' with an expression of incompatible type '__private float'}}
   intel_sub_group_avc_sic_payload_t payload_sic = ss;
-  // expected-error@-1 {{initializing '__private intel_sub_group_avc_sic_payload_t' with an expression of incompatible type '__private struct st'}}
-
   intel_sub_group_avc_mce_result_t result_mce = 0; // No zero initializer for mce types
-  // expected-error@-1 {{initializing '__private intel_sub_group_avc_mce_result_t' with an expression of incompatible type 'int'}}
   intel_sub_group_avc_ime_result_t result_ime = 1; // No literal initializer for *result_t types
-  // expected-error@-1 {{initializing '__private intel_sub_group_avc_ime_result_t' with an expression of incompatible type 'int'}}
   intel_sub_group_avc_ref_result_t result_ref = f;
-  // expected-error@-1 {{initializing '__private intel_sub_group_avc_ref_result_t' with an expression of incompatible type '__private float'}}
   intel_sub_group_avc_sic_result_t result_sic = ss;
-  // expected-error@-1 {{initializing '__private intel_sub_group_avc_sic_result_t' with an expression of incompatible type '__private struct st'}}
-
   intel_sub_group_avc_ime_result_single_reference_streamout_t sstreamout = v;
-  // expected-error@-1 {{initializing '__private intel_sub_group_avc_ime_result_single_reference_streamout_t' with an expression of incompatible type '__private void *__private'}}
-
   intel_sub_group_avc_ime_result_dual_reference_streamout_t dstreamin_list = {0x0, 0x1};
-  // expected-warning@-1 {{excess elements in struct initializer}}
   intel_sub_group_avc_ime_dual_reference_streamin_t dstreamin_list2 = {};
-  // expected-error@-1 {{scalar initializer cannot be empty}}
   intel_sub_group_avc_ime_single_reference_streamin_t dstreamin_list3 = {c};
-  // expected-error@-1 {{initializing '__private intel_sub_group_avc_ime_single_reference_streamin_t' with an expression of incompatible type '__private char'}}
   intel_sub_group_avc_ime_dual_reference_streamin_t dstreamin_list4 = {1};
-  // expected-error@-1 {{initializing '__private intel_sub_group_avc_ime_dual_reference_streamin_t' with an expression of incompatible type 'int'}}
+#ifdef EXT
+// expected-error@-14 {{initializing '__private intel_sub_group_avc_mce_payload_t' with an expression of incompatible type 'int'}}
+// expected-error@-14 

[PATCH] D46443: [libc++] Add missing cstdalign header

2021-03-10 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius requested changes to this revision.
curdeius added inline comments.
This revision now requires changes to proceed.



Comment at: libcxx/include/cstdalign:24
+#include <__config>
+#include 
+

hubert.reinterpretcast wrote:
> curdeius wrote:
> > curdeius wrote:
> > > hubert.reinterpretcast wrote:
> > > > sbc100 wrote:
> > > > > hubert.reinterpretcast wrote:
> > > > > > This seems to be assuming that the underlying C library's 
> > > > > > `stdalign.h` is C++ friendly. A C11 `stdalign.h` //does// define 
> > > > > > `alignof` and `alignas` as macros.
> > > > > Should I just remove this `#include` then?
> > > > The idea would be to //add// a `stdalign.h` alongside this header that 
> > > > doesn't `#include_next` the underlying C library's `stdalign.h`.
> > > I'm not sure if that should be the solution. At least gcc's libstdc++ 
> > > assumes that `stdalign.h` is C++-compatbile (cf. 
> > > https://github.com/gcc-mirror/gcc/blob/16e2427f50c208dfe07d07f18009969502c25dc8/libstdc%2B%2B-v3/include/c_global/cstdalign).
> > > 
> > > Clang provides a compatible header: 
> > > https://github.com/llvm/llvm-project/commit/8acb4044d83ecc9df81b1c9f327d5bd4325e1756.
> > > Gcc too of course: 
> > > https://github.com/gcc-mirror/gcc/blob/16e2427f50c208dfe07d07f18009969502c25dc8/gcc/ginclude/stdalign.h.
> > > 
> > > MSVC's STL on the other hand, doesn't include `` 
> > > (https://github.com/microsoft/STL/blob/main/stl/inc/cstdalign).
> > > 
> > > @hubert.reinterpretcast, are you aware of an environment which has 
> > > non-friendly `stdalign.h`?
> > FYI, musl is also C++ friendly: 
> > https://git.musl-libc.org/cgit/musl/tree/include/stdalign.h.
> >>! Quote:
> > @hubert.reinterpretcast, are you aware of an environment which has 
> > non-friendly stdalign.h?
> 
> The one GCC provides disagrees with the interpretation I gave of which macros 
> should be present. The one that Clang provides //does// match my 
> interpretation. It seems the GCC one is non-friendly (albeit a different form 
> of non-friendly than the one I opened with).
Oh, you mean that `__alignas_is_defined` and `__alignof_is_defined` won't be 
defined in this case, right?
In this case, I guess we won't avoid having `stdalign.h` as you had suggested.
And indeed the test fails with gcc:
```
bin/llvm-lit -vv ../../libcxx/test/std/language.support/cstdalign/ 
--param=std=c++17 --param=cxx_under_test=`which g++`
...
libcxx/test/std/language.support/cstdalign/cstdalign.pass.cpp:21:2: error: 
#error __alignas_is_defined not defined
   21 | #error __alignas_is_defined not defined
  |  ^
```

That's unfortunately a configuration which is not tested in the CI.



Comment at: libcxx/test/std/language.support/cstdalign/cstdalign.pass.cpp:16-18
+#ifdef alignas
+#error alignas should not be a macro in C++
+#endif

Please do the same for `alignof`.



Comment at: libcxx/test/std/language.support/cstdalign/cstdalign.pass.cpp:27
+#ifndef __alignof_is_defined
+#error __alignof_is_defined not defined
+#endif

hubert.reinterpretcast wrote:
> sbc100 wrote:
> > hubert.reinterpretcast wrote:
> > > sbc100 wrote:
> > > > ldionne wrote:
> > > > > I'm not seeing `__alignof_is_defined` anywhere in the spec?
> > > > Removed
> > > Seems like a defect in the old standard. The prose doesn't match the 
> > > synopsis. `__alignof_is_defined` is a macro in C11's `stdalign.h` (and so 
> > > is `alignof`). That the C++ committee did not intend for an `alignof` 
> > > macro can probably be assumed. I suspect the lack of an 
> > > `__alignof_is_defined` macro was also unintended.
> > So should I add back the check for `__alignof_is_defined`?
> I think so (in addition to also checking that `alignof` is not defined as a 
> macro). I think the patch needs to be confirmed again either way by the 
> libc++ approvers though.
Please check `__alignof_is_defined` as well, but guard it with `#ifdef 
_LIBCPP_VERSION` to be conforming.
I agree that it seems to be a defect in the old standard.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D46443

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


[PATCH] D98246: [clangd] Add basic monitoring info request for remote index server

2021-03-10 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/index/remote/Service.proto:13
 
+import "google/protobuf/empty.proto";
 import "Index.proto";

sammccall wrote:
> question of style, but unless there's a concrete benefit I'd suggest just 
> defining the request inline.
> Cost is not just the import but also the irregularity of the request type 
> *not* being called MonitoringInfoRequest or whatever.
+1 (i also had a comment about it but forgot to hit submit)



Comment at: clang-tools-extra/clangd/index/remote/Service.proto:16
 
+message MonitoringInfoReply { optional string info = 1; }
+

i am not sure if having more structure here immediately vs. incrementally makes 
much difference, we should at least settle on the proto initially, and can fill 
in those fields in incremental steps if need be (but i don't expect those 
changes to be huge, so it should be fine to just cram them into this patch).

i think we need more structure here, rather than just string. for starters:
- an unsigned for uptime (probably in seconds)
- another unsgined (or timepoint object) for indicating the build time of 
currently used index

i am not sure if we got anything else we can dispatch immediately, but can 
probably incorporate things like QPS and more details about the loaded index if 
turns out to be useful/needed.



Comment at: clang-tools-extra/clangd/index/remote/Service.proto:29
+
+  rpc MonitoringInfo(google.protobuf.Empty) returns (MonitoringInfoReply) {}
 }

sammccall wrote:
> sammccall wrote:
> > Who's going to call this? If clients will, then it probably belongs here, 
> > but should have a different name.
> > If only e.g. a web UI or monitoring system will, it belongs in a separate 
> > `service` IMO.
> > 
> > Depending on what it returns, it may be interesting for clients to call and 
> > log!
> > Or it may be useful to ban clients from calling it (if it has private 
> > details)
> I think we should try harder to find a name that describes the data 
> requested, rather than what we expect the data to be used for.
actually having this available as a separate service makes sense to me, it is 
not directly related to symbol index facilities and i don't think will be 
useful to clangd as is. in the future we might try to expose some meta 
information to the clients about the index being used on the server to enable 
different workflows (e.g. branches/staleness/incremental updates), but that's 
probably a different endpoint.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98246

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


[PATCH] D98246: [clangd] Add basic monitoring info request for remote index server

2021-03-10 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/index/remote/Service.proto:16
 
+message MonitoringInfoReply { optional string info = 1; }
+

sammccall wrote:
> kadircet wrote:
> > i am not sure if having more structure here immediately vs. incrementally 
> > makes much difference, we should at least settle on the proto initially, 
> > and can fill in those fields in incremental steps if need be (but i don't 
> > expect those changes to be huge, so it should be fine to just cram them 
> > into this patch).
> > 
> > i think we need more structure here, rather than just string. for starters:
> > - an unsigned for uptime (probably in seconds)
> > - another unsgined (or timepoint object) for indicating the build time of 
> > currently used index
> > 
> > i am not sure if we got anything else we can dispatch immediately, but can 
> > probably incorporate things like QPS and more details about the loaded 
> > index if turns out to be useful/needed.
> it seems unlikely to me that `string info` is the right format, but I can't 
> tell whether this is a placeholder or not
> we should at least settle on the proto initially, and can fill in those 
> fields in incremental steps

Agree, incremental development is nice, but the scope/responsibility of RPCs is 
fairly important and not that easy to change later.

(Sorry, I hadn't seen this review wasn't actually assigned to me, happy to 
leave this with you!)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98246

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


[PATCH] D98070: [clang-tidy] Add option to ignore macros in readability-function-cognitive-complexity check.

2021-03-10 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

Please also add a test with global `IgnoreMacros=1` and 
`readability-function-cognitive-complexity.IgnoreMacros` unset.
(The code is correct as-is, global `IgnoreMacros` should not affect the check 
here.)

I'm also somewhat worried about forward compatibility.
If in future a mode will be implemented that allows to count the cost of macro 
arguments,
how'd we expose it in check's params, without breaking the existing 
`IgnoreMacros=1` config support?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98070

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


[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-03-10 Thread Ally Tiritoglu via Phabricator via cfe-commits
atirit added a comment.

In D93938#2614825 , 
@HazardyKnusperkeks wrote:

> If the bugs are (very) similar, I could live with one fix for both. Otherwise 
> you should fix the other bug first, if its blocking this change.

The only thing linking the bugs is `AfterEnum`. Other than that I'd say they're 
separate. If the bug should be fixed first (instead of merging incorrect but 
passing tests) then I think it should be a separate diff.

My concern with that course of action is that whoever reviews that diff will 
all but certainly ask for unit tests, and if I wrote unit tests for `AfterEnum` 
and `AllowShortCaseLabelsOnASingleLine` as I have here, they'll still be 
incorrect because this bug is not merged. Would you foresee this being an issue?

I'll start working on a fix regardless.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93938

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


[PATCH] D98237: [clang-format] Option for empty lines after an access modifier.

2021-03-10 Thread Max Sagebaum via Phabricator via cfe-commits
Max_S updated this revision to Diff 329574.
Max_S added a comment.

Updating D98237 : [clang-format] Option for 
empty lines after an access modifier.

- Added additional verification tests
- Added expected formating tests
- Changed comment to reflect the new logic


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98237

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9179,6 +9179,48 @@
Style);
 }
 
+TEST_F(FormatTest, FormatsAfterAccessModifiers) {
+  char const* const test0NL =
+   "class Foo {\n"
+   "private: int i;\n"
+   "};";
+  char const* const test1NL =
+   "class Foo {\n"
+   "private:\n"
+   "  int i;\n"
+   "};";
+  char const* const test2NL =
+   "class Foo {\n"
+   "private:\n"
+   "\n"
+   "  int i;\n"
+   "};";
+  char const* const test3NL =
+   "class Foo {\n"
+   "private:\n"
+   "\n"
+   "\n"
+   "  int i;\n"
+   "};";
+  EXPECT_EQ(test1NL, format(test0NL));
+  verifyFormat(test1NL);
+  EXPECT_EQ(test1NL, format(test2NL));
+  EXPECT_EQ(test1NL, format(test3NL));
+
+  FormatStyle StyleWithLine = getLLVMStyle();
+  StyleWithLine.EmptyLinesAfterAccessModifier = 1u;
+  EXPECT_EQ(test2NL, format(test0NL, StyleWithLine));
+  EXPECT_EQ(test2NL, format(test1NL, StyleWithLine));
+  verifyFormat(test2NL, StyleWithLine);
+  EXPECT_EQ(test2NL, format(test3NL, StyleWithLine));
+
+  StyleWithLine.EmptyLinesAfterAccessModifier = 2u;
+  EXPECT_EQ(test3NL, format(test0NL, StyleWithLine));
+  EXPECT_EQ(test3NL, format(test1NL, StyleWithLine));
+  EXPECT_EQ(test3NL, format(test2NL, StyleWithLine));
+  verifyFormat(test3NL, StyleWithLine);
+}
+
 TEST_F(FormatTest, FormatsArrays) {
   verifyFormat("a[a]\n"
" [b] = c;");
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1278,10 +1278,10 @@
 }
   }
 
-  // Remove empty lines after access specifiers.
+  // Force the configured amount of new lines after an access specifier
   if (PreviousLine && PreviousLine->First->isAccessSpecifier() &&
   (!PreviousLine->InPPDirective || !RootToken.HasUnescapedNewline))
-Newlines = std::min(1u, Newlines);
+Newlines = Style.EmptyLinesAfterAccessModifier + 1u;
 
   if (Newlines)
 Indent = NewlineIndent;
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -586,6 +586,8 @@
 IO.mapOptional("DisableFormat", Style.DisableFormat);
 IO.mapOptional("EmptyLineBeforeAccessModifier",
Style.EmptyLineBeforeAccessModifier);
+IO.mapOptional("EmptyLinesAfterAccessModifier",
+   Style.EmptyLinesAfterAccessModifier);
 IO.mapOptional("ExperimentalAutoDetectBinPacking",
Style.ExperimentalAutoDetectBinPacking);
 IO.mapOptional("FixNamespaceComments", Style.FixNamespaceComments);
@@ -975,6 +977,7 @@
   LLVMStyle.DeriveLineEnding = true;
   LLVMStyle.DerivePointerAlignment = false;
   LLVMStyle.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock;
+  LLVMStyle.EmptyLinesAfterAccessModifier = 0u;
   LLVMStyle.ExperimentalAutoDetectBinPacking = false;
   LLVMStyle.FixNamespaceComments = true;
   LLVMStyle.ForEachMacros.push_back("foreach");
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1953,6 +1953,9 @@
   /// Defines in which cases to put empty line before access modifiers.
   EmptyLineBeforeAccessModifierStyle EmptyLineBeforeAccessModifier;
 
+  /// Defines how many lines are put after access modifiers.
+  unsigned EmptyLinesAfterAccessModifier;
+
   /// If ``true``, clang-format detects whether function calls and
   /// definitions are formatted with one parameter per line.
   ///
@@ -3201,6 +3204,7 @@
DerivePointerAlignment == R.DerivePointerAlignment &&
DisableFormat == R.DisableFormat &&
EmptyLineBeforeAccessModifier == R.EmptyLineBeforeAccessModifier &&
+   EmptyLines

[PATCH] D98237: [clang-format] Option for empty lines after an access modifier.

2021-03-10 Thread Max Sagebaum via Phabricator via cfe-commits
Max_S added a comment.

In D98237#2614844 , 
@HazardyKnusperkeks wrote:

> I would like additional tests:
>
> - With at least 2 empty lines
> - With 0-2 empty lines without `verifyFormat` to demonstrate what is kept, 
> added, and removed.

Thank you for reviewing this, I added new tests, that demonstrate the 
formatting behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98237

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


[clang] 6f912a2 - [OpenCL] Set calling convention for -fdeclare-opencl-builtins

2021-03-10 Thread Sven van Haastregt via cfe-commits

Author: Sven van Haastregt
Date: 2021-03-10T10:03:57Z
New Revision: 6f912a2cd49150813ee467c079201b5ce3dfbbca

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

LOG: [OpenCL] Set calling convention for -fdeclare-opencl-builtins

IR produced using TableGen builtin function declarations
(`fdeclare-opencl-builtins.cl`) did not have the target's calling
convention applied to builtin calls.

Fix this, and update the codegen test to check that IR produced using
opencl-c.h and `-fdeclare-opencl-builtins` is identical with respect
to the builtin calls.

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

Added: 


Modified: 
clang/lib/Sema/SemaLookup.cpp
clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl

Removed: 




diff  --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index 4da81bb44d5b..b6bf88203745 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -755,7 +755,8 @@ static void GetOpenCLBuiltinFctOverloads(
 ASTContext &Context, unsigned GenTypeMaxCnt,
 std::vector &FunctionList, SmallVector &RetTypes,
 SmallVector, 5> &ArgTypes) {
-  FunctionProtoType::ExtProtoInfo PI;
+  FunctionProtoType::ExtProtoInfo PI(
+  Context.getDefaultCallingConvention(false, false, true));
   PI.Variadic = false;
 
   // Create FunctionTypes for each (gen)type.

diff  --git a/clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl 
b/clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl
index a59357e331e7..665297b469f8 100644
--- a/clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl
+++ b/clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl
@@ -1,8 +1,9 @@
+// RUN: %clang_cc1 -emit-llvm -o - -O0 -triple spir-unknown-unknown 
-cl-std=CL1.2 -finclude-default-header %s | FileCheck %s
 // RUN: %clang_cc1 -emit-llvm -o - -O0 -triple spir-unknown-unknown 
-cl-std=CL1.2 -fdeclare-opencl-builtins -finclude-default-header %s | FileCheck 
%s
 
 // Test that mix is correctly defined.
 // CHECK-LABEL: @test_float
-// CHECK: call <4 x float> @_Z3mixDv4_fS_f
+// CHECK: call spir_func <4 x float> @_Z3mixDv4_fS_f
 // CHECK: ret
 void test_float(float4 x, float a) {
   float4 ret = mix(x, x, a);
@@ -10,7 +11,7 @@ void test_float(float4 x, float a) {
 
 // Test that Attr.Const from OpenCLBuiltins.td is lowered to a readnone 
attribute.
 // CHECK-LABEL: @test_const_attr
-// CHECK: call i32 @_Z3maxii({{.*}}) [[ATTR_CONST:#[0-9]]]
+// CHECK: call spir_func i32 @_Z3maxii({{.*}}) [[ATTR_CONST:#[0-9]]]
 // CHECK: ret
 int test_const_attr(int a) {
   return max(a, 2);
@@ -18,7 +19,7 @@ int test_const_attr(int a) {
 
 // Test that Attr.Pure from OpenCLBuiltins.td is lowered to a readonly 
attribute.
 // CHECK-LABEL: @test_pure_attr
-// CHECK: call <4 x float> @_Z11read_imagef{{.*}} [[ATTR_PURE:#[0-9]]]
+// CHECK: call spir_func <4 x float> @_Z11read_imagef{{.*}} 
[[ATTR_PURE:#[0-9]]]
 // CHECK: ret
 kernel void test_pure_attr(read_only image1d_t img) {
   float4 resf = read_imagef(img, 42);
@@ -26,7 +27,7 @@ kernel void test_pure_attr(read_only image1d_t img) {
 
 // Test that builtins with only one prototype are mangled.
 // CHECK-LABEL: @test_mangling
-// CHECK: call i32 @_Z12get_local_idj
+// CHECK: call spir_func i32 @_Z12get_local_idj
 kernel void test_mangling() {
   size_t lid = get_local_id(0);
 }



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


[PATCH] D98039: [OpenCL] Set calling convention for -fdeclare-opencl-builtins

2021-03-10 Thread Sven van Haastregt via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6f912a2cd491: [OpenCL] Set calling convention for 
-fdeclare-opencl-builtins (authored by svenvh).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98039

Files:
  clang/lib/Sema/SemaLookup.cpp
  clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl


Index: clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl
===
--- clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl
+++ clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl
@@ -1,8 +1,9 @@
+// RUN: %clang_cc1 -emit-llvm -o - -O0 -triple spir-unknown-unknown 
-cl-std=CL1.2 -finclude-default-header %s | FileCheck %s
 // RUN: %clang_cc1 -emit-llvm -o - -O0 -triple spir-unknown-unknown 
-cl-std=CL1.2 -fdeclare-opencl-builtins -finclude-default-header %s | FileCheck 
%s
 
 // Test that mix is correctly defined.
 // CHECK-LABEL: @test_float
-// CHECK: call <4 x float> @_Z3mixDv4_fS_f
+// CHECK: call spir_func <4 x float> @_Z3mixDv4_fS_f
 // CHECK: ret
 void test_float(float4 x, float a) {
   float4 ret = mix(x, x, a);
@@ -10,7 +11,7 @@
 
 // Test that Attr.Const from OpenCLBuiltins.td is lowered to a readnone 
attribute.
 // CHECK-LABEL: @test_const_attr
-// CHECK: call i32 @_Z3maxii({{.*}}) [[ATTR_CONST:#[0-9]]]
+// CHECK: call spir_func i32 @_Z3maxii({{.*}}) [[ATTR_CONST:#[0-9]]]
 // CHECK: ret
 int test_const_attr(int a) {
   return max(a, 2);
@@ -18,7 +19,7 @@
 
 // Test that Attr.Pure from OpenCLBuiltins.td is lowered to a readonly 
attribute.
 // CHECK-LABEL: @test_pure_attr
-// CHECK: call <4 x float> @_Z11read_imagef{{.*}} [[ATTR_PURE:#[0-9]]]
+// CHECK: call spir_func <4 x float> @_Z11read_imagef{{.*}} 
[[ATTR_PURE:#[0-9]]]
 // CHECK: ret
 kernel void test_pure_attr(read_only image1d_t img) {
   float4 resf = read_imagef(img, 42);
@@ -26,7 +27,7 @@
 
 // Test that builtins with only one prototype are mangled.
 // CHECK-LABEL: @test_mangling
-// CHECK: call i32 @_Z12get_local_idj
+// CHECK: call spir_func i32 @_Z12get_local_idj
 kernel void test_mangling() {
   size_t lid = get_local_id(0);
 }
Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -755,7 +755,8 @@
 ASTContext &Context, unsigned GenTypeMaxCnt,
 std::vector &FunctionList, SmallVector &RetTypes,
 SmallVector, 5> &ArgTypes) {
-  FunctionProtoType::ExtProtoInfo PI;
+  FunctionProtoType::ExtProtoInfo PI(
+  Context.getDefaultCallingConvention(false, false, true));
   PI.Variadic = false;
 
   // Create FunctionTypes for each (gen)type.


Index: clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl
===
--- clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl
+++ clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl
@@ -1,8 +1,9 @@
+// RUN: %clang_cc1 -emit-llvm -o - -O0 -triple spir-unknown-unknown -cl-std=CL1.2 -finclude-default-header %s | FileCheck %s
 // RUN: %clang_cc1 -emit-llvm -o - -O0 -triple spir-unknown-unknown -cl-std=CL1.2 -fdeclare-opencl-builtins -finclude-default-header %s | FileCheck %s
 
 // Test that mix is correctly defined.
 // CHECK-LABEL: @test_float
-// CHECK: call <4 x float> @_Z3mixDv4_fS_f
+// CHECK: call spir_func <4 x float> @_Z3mixDv4_fS_f
 // CHECK: ret
 void test_float(float4 x, float a) {
   float4 ret = mix(x, x, a);
@@ -10,7 +11,7 @@
 
 // Test that Attr.Const from OpenCLBuiltins.td is lowered to a readnone attribute.
 // CHECK-LABEL: @test_const_attr
-// CHECK: call i32 @_Z3maxii({{.*}}) [[ATTR_CONST:#[0-9]]]
+// CHECK: call spir_func i32 @_Z3maxii({{.*}}) [[ATTR_CONST:#[0-9]]]
 // CHECK: ret
 int test_const_attr(int a) {
   return max(a, 2);
@@ -18,7 +19,7 @@
 
 // Test that Attr.Pure from OpenCLBuiltins.td is lowered to a readonly attribute.
 // CHECK-LABEL: @test_pure_attr
-// CHECK: call <4 x float> @_Z11read_imagef{{.*}} [[ATTR_PURE:#[0-9]]]
+// CHECK: call spir_func <4 x float> @_Z11read_imagef{{.*}} [[ATTR_PURE:#[0-9]]]
 // CHECK: ret
 kernel void test_pure_attr(read_only image1d_t img) {
   float4 resf = read_imagef(img, 42);
@@ -26,7 +27,7 @@
 
 // Test that builtins with only one prototype are mangled.
 // CHECK-LABEL: @test_mangling
-// CHECK: call i32 @_Z12get_local_idj
+// CHECK: call spir_func i32 @_Z12get_local_idj
 kernel void test_mangling() {
   size_t lid = get_local_id(0);
 }
Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -755,7 +755,8 @@
 ASTContext &Context, unsigned GenTypeMaxCnt,
 std::vector &FunctionList, SmallVector &RetTypes,
 SmallVector, 5> &ArgTypes) {
-  FunctionProtoType::ExtProtoInfo PI;
+  FunctionProtoType::ExtProtoInfo PI(
+  Context.getDefaultCalli

[PATCH] D98237: [clang-format] Option for empty lines after an access modifier.

2021-03-10 Thread Max Sagebaum via Phabricator via cfe-commits
Max_S added inline comments.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:1284
   (!PreviousLine->InPPDirective || !RootToken.HasUnescapedNewline))
-Newlines = std::min(1u, Newlines);
+Newlines = Style.EmptyLinesAfterAccessModifier + 1u;
 

HazardyKnusperkeks wrote:
> I don't know, I'm just asking:
> Shouldn't this be `Newlines = std::min(Newlines, 
> Style.EmptyLinesAfterAccessModifier + 1u);`?
This is also possible but then the logic would be how many lines should be kept 
at maximum after an access specifier.

The name would then be `Style.KeepMaximumLinesAfterAccessModifier`.

Currently the logic above:
```
  if (Newlines == 0 && !RootToken.IsFirst)
Newlines = 1;
```
forces Newlines to be always 1 or bigger. Therefore the old logic would always 
add one new line and I decided to implement the setting in the same way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98237

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


[PATCH] D98264: [AArch64] Implement __rndr, __rndrrs intrinsics

2021-03-10 Thread Stelios Ioannou via Phabricator via cfe-commits
stelios-arm added a comment.

@SjoerdMeijer @dmgreen Thanks for your reviews, I will be looking into this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98264

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


[PATCH] D98264: [AArch64] Implement __rndr, __rndrrs intrinsics

2021-03-10 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:1495
   let DecoderNamespace = "Fallback";
+  let Defs = [NZCV];
 }

SjoerdMeijer wrote:
> dmgreen wrote:
> > SjoerdMeijer wrote:
> > > dmgreen wrote:
> > > > SjoerdMeijer wrote:
> > > > > Do all MRS instructions do this?
> > > > No, but some do and it's not obvious which ones do and don't. I think 
> > > > it should be reasonable to always def NZCV here, even if they are just 
> > > > dead. It should be very rare that it would be beneficial for NZCV to be 
> > > > live across a MRS instruction.
> > > True, but since creating another definition is cheap, what would the cons 
> > > be of:
> > > 
> > >   class MRS_NZCV : MRSI {
> > > ..
> > > let Defs = [NZCV];
> > >   }
> > > 
> > > The way I look at it is that the description would be more accurate?
> > I believe that would have an over-lapping definition with the existing MRS 
> > instruction?
> > 
> > It would need to be a pseudo I think, which would eventually be turned into 
> > a MSR proper late enough on the pipeline for it to be valid (or the other 
> > way around, the no-nzcv version gets turned into a the nzcv version to keep 
> > the verifier happy).
> > 
> > It could also be a optional def, but those are only used in the arm backend 
> > and I would not recommend using them anywhere else.  I would probably 
> > suggest just setting MRS as a NZCV setting instruction, unless we find a 
> > reason to need to implement it differently.
> > I believe that would have an over-lapping definition with the existing MRS 
> > instruction?
> 
> Ah yeah, that might be true.
> 
> > It would need to be a pseudo I think
> 
> How much work is adding a pseudo for this case? My first reaction would be 
> just trying to model this correctly, then we potentially don't have to worry 
> again later about this. 
I just wanted to add that I don't have too strong opinions on this, but what I 
suggested seemed more the correct, even though the consequence of setting NCZV 
for all MRS is minimal. So I will leave this one up to you @dmgreen and 
@stelios-arm .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98264

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


[PATCH] D98237: [clang-format] Option for empty lines after an access modifier.

2021-03-10 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

Just out of interest and we are supposed to ask for this but can you point to 
public style guide that uses this style.   (actually I don't mind if other 
formatting tools have this capability and you highlight it, like astyle or 
editorConfig etc)

From my perspective this seems like a reasonable suggestion. (even if I'm 
unlikely to use it myself ;-))




Comment at: clang/unittests/Format/FormatTest.cpp:9205
+   "};";
+  EXPECT_EQ(test1NL, format(test0NL));
+  verifyFormat(test1NL);

why can't you just verifyFormat them all?



Comment at: clang/unittests/Format/FormatTest.cpp:9212
+  StyleWithLine.EmptyLinesAfterAccessModifier = 1u;
+  EXPECT_EQ(test2NL, format(test0NL, StyleWithLine));
+  EXPECT_EQ(test2NL, format(test1NL, StyleWithLine));

yeah I'm not a fan of this like this... sorry... just write the test out in 
long form, when it goes wrong I don't have to be a compiler to understand what 
is going wrong I can just see it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98237

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


[PATCH] D98329: [clangd] Add cache for FID to Header mappings

2021-03-10 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: usaxena95, arphaman.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.

Depends on D98242 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98329

Files:
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/SymbolCollector.h


Index: clang-tools-extra/clangd/index/SymbolCollector.h
===
--- clang-tools-extra/clangd/index/SymbolCollector.h
+++ clang-tools-extra/clangd/index/SymbolCollector.h
@@ -22,6 +22,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/Support/Regex.h"
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -142,6 +143,9 @@
   // File IDs for Symbol.IncludeHeaders.
   // The final spelling is calculated in finish().
   llvm::DenseMap IncludeFiles;
+  // Caches FileID to header mappings, values are either verbatim headers or
+  // URIs. None in case of a bad header, e.g. not include-guarded.
+  llvm::DenseMap> HeaderToInclude;
   void setIncludeLocation(const Symbol &S, SourceLocation);
   // Indexed macros, to be erased if they turned out to be include guards.
   llvm::DenseSet IndexedMacros;
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -781,13 +781,17 @@
 if (Canonical != Filename)
   return toURI(SM, Canonical, Opts);
   }
+  auto FIDMapping = HeaderToInclude.find(FID);
+  if (FIDMapping != HeaderToInclude.end())
+return FIDMapping->second;
   if (!isSelfContainedHeader(FID)) {
 // A .inc or .def file is often included into a real header to define
 // symbols (e.g. LLVM tablegen files).
 if (Filename.endswith(".inc") || Filename.endswith(".def"))
-  return getIncludeHeader(S, SM.getFileID(SM.getIncludeLoc(FID)));
+  return HeaderToInclude[FID] =
+ getIncludeHeader(S, SM.getFileID(SM.getIncludeLoc(FID)));
 // Conservatively refuse to insert #includes to files without guards.
-return llvm::None;
+return HeaderToInclude[FID] = llvm::None;
   }
   // Store system includes as verbatim. This enables making use of the same
   // index in different environments, e.g. a system header like 
@@ -798,9 +802,9 @@
   PP->getHeaderSearchInfo().suggestPathToFileForDiagnostics(FE, "",
 &IsSystem);
   if (IsSystem)
-return "<" + ShorterInclude + ">";
+return HeaderToInclude[FID] = "<" + ShorterInclude + ">";
   // Standard case: just insert the file itself.
-  return toURI(SM, Filename, Opts);
+  return HeaderToInclude[FID] = toURI(SM, Filename, Opts);
 }
 
 bool SymbolCollector::isSelfContainedHeader(FileID FID) {


Index: clang-tools-extra/clangd/index/SymbolCollector.h
===
--- clang-tools-extra/clangd/index/SymbolCollector.h
+++ clang-tools-extra/clangd/index/SymbolCollector.h
@@ -22,6 +22,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/Support/Regex.h"
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -142,6 +143,9 @@
   // File IDs for Symbol.IncludeHeaders.
   // The final spelling is calculated in finish().
   llvm::DenseMap IncludeFiles;
+  // Caches FileID to header mappings, values are either verbatim headers or
+  // URIs. None in case of a bad header, e.g. not include-guarded.
+  llvm::DenseMap> HeaderToInclude;
   void setIncludeLocation(const Symbol &S, SourceLocation);
   // Indexed macros, to be erased if they turned out to be include guards.
   llvm::DenseSet IndexedMacros;
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -781,13 +781,17 @@
 if (Canonical != Filename)
   return toURI(SM, Canonical, Opts);
   }
+  auto FIDMapping = HeaderToInclude.find(FID);
+  if (FIDMapping != HeaderToInclude.end())
+return FIDMapping->second;
   if (!isSelfContainedHeader(FID)) {
 // A .inc or .def file is often included into a real header to define
 // symbols (e.g. LLVM tablegen files).
 if (Filename.endswith(".inc") || Filename.endswith(".def"))
-  return getIncludeHeader(S, SM.getFileID(SM.getIncludeLoc(FID)));
+  return HeaderToInclude[FID] =
+ getIncludeHeader(S, SM.getFileID(SM.getIncludeLoc(FID)));
 // Conservatively refuse to insert #includes to files without guards.
-return llvm::None;
+return HeaderToInclude[FID] = llvm::None;
   }
   // Store system includes as verbatim. This enables making use of the 

[PATCH] D97857: [Matrix] Add support for matrix-by-scalar division.

2021-03-10 Thread Florian Hahn via Phabricator via cfe-commits
fhahn updated this revision to Diff 329596.
fhahn added a comment.

rebase & added a matrix_types_scalar_division feature, which allows users to 
check if the current version of clang is new enough to support  it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97857

Files:
  clang/docs/MatrixTypes.rst
  clang/include/clang/Basic/Features.def
  clang/include/clang/Basic/LangOptions.def
  clang/lib/AST/Type.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CodeGen/matrix-type-operators.c
  clang/test/CodeGen/matrix-type.c
  clang/test/Sema/matrix-type-operators.c
  llvm/include/llvm/IR/MatrixBuilder.h

Index: llvm/include/llvm/IR/MatrixBuilder.h
===
--- llvm/include/llvm/IR/MatrixBuilder.h
+++ llvm/include/llvm/IR/MatrixBuilder.h
@@ -215,6 +215,22 @@
 return B.CreateMul(LHS, RHS);
   }
 
+  /// Divide matrix \p LHS by scalar \p RHS. If the operands are integers, \p
+  /// IsUnsigned indicates whether UDiv or SDiv should be used.
+  Value *CreateScalarDiv(Value *LHS, Value *RHS, bool IsUnsigned) {
+assert(LHS->getType()->isVectorTy() && !RHS->getType()->isVectorTy());
+assert(!isa(LHS->getType()) &&
+   "LHS Assumed to be fixed width");
+RHS =
+B.CreateVectorSplat(cast(LHS->getType())->getElementCount(),
+RHS, "scalar.splat");
+return cast(LHS->getType())
+   ->getElementType()
+   ->isFloatingPointTy()
+   ? B.CreateFDiv(LHS, RHS)
+   : (IsUnsigned ? B.CreateUDiv(LHS, RHS) : B.CreateSDiv(LHS, RHS));
+  }
+
   /// Extracts the element at (\p RowIdx, \p ColumnIdx) from \p Matrix.
   Value *CreateExtractElement(Value *Matrix, Value *RowIdx, Value *ColumnIdx,
   unsigned NumRows, Twine const &Name = "") {
Index: clang/test/Sema/matrix-type-operators.c
===
--- clang/test/Sema/matrix-type-operators.c
+++ clang/test/Sema/matrix-type-operators.c
@@ -94,6 +94,40 @@
   // expected-error@-1 {{assigning to 'float' from incompatible type 'sx10x10_t' (aka 'float __attribute__((matrix_type(10, 10)))')}}
 }
 
+void mat_scalar_divide(sx10x10_t a, sx5x10_t b, float sf, char *p) {
+  // Shape of multiplication result does not match the type of b.
+  b = a / sf;
+  // expected-error@-1 {{assigning to 'sx5x10_t' (aka 'float __attribute__((matrix_type(5, 10)))') from incompatible type 'sx10x10_t' (aka 'float __attribute__((matrix_type(10, 10)))')}}
+  b = sf / a;
+  // expected-error@-1 {{invalid operands to binary expression ('float' and 'sx10x10_t' (aka 'float __attribute__((matrix_type(10, 10)))'))}}
+
+  a = a / p;
+  // expected-error@-1 {{invalid operands to binary expression ('sx10x10_t' (aka 'float __attribute__((matrix_type(10, 10)))') and 'char *')}}
+  a = p / a;
+  // expected-error@-1 {{invalid operands to binary expression ('char *' and 'sx10x10_t' (aka 'float __attribute__((matrix_type(10, 10)))'))}}
+
+  sf = a / sf;
+  // expected-error@-1 {{assigning to 'float' from incompatible type 'sx10x10_t' (aka 'float __attribute__((matrix_type(10, 10)))')}}
+}
+
+void matrix_matrix_divide(sx10x10_t a, sx5x10_t b, ix10x5_t c, ix10x10_t d, float sf, char *p) {
+  // Matrix by matrix division is not supported.
+  a = a / a;
+  // expected-error@-1 {{invalid operands to binary expression ('sx10x10_t' (aka 'float __attribute__((matrix_type(10, 10)))') and 'sx10x10_t')}}
+
+  b = a / a;
+  // expected-error@-1 {{invalid operands to binary expression ('sx10x10_t' (aka 'float __attribute__((matrix_type(10, 10)))') and 'sx10x10_t')}}
+
+  // Check element type mismatches.
+  a = b / c;
+  // expected-error@-1 {{invalid operands to binary expression ('sx5x10_t' (aka 'float __attribute__((matrix_type(5, 10)))') and 'ix10x5_t' (aka 'int __attribute__((matrix_type(10, 5)))'))}}
+  d = a / a;
+  // expected-error@-1 {{invalid operands to binary expression ('sx10x10_t' (aka 'float __attribute__((matrix_type(10, 10)))') and 'sx10x10_t')}}
+
+  p = a / a;
+  // expected-error@-1 {{invalid operands to binary expression ('sx10x10_t' (aka 'float __attribute__((matrix_type(10, 10)))') and 'sx10x10_t')}}
+}
+
 sx5x10_t get_matrix();
 
 void insert(sx5x10_t a, float f) {
Index: clang/test/CodeGen/matrix-type.c
===
--- clang/test/CodeGen/matrix-type.c
+++ clang/test/CodeGen/matrix-type.c
@@ -4,6 +4,10 @@
 #error Expected extension 'matrix_types' to be enabled
 #endif
 
+#if !__has_extension(matrix_types_scalar_division)
+#error Expected extension 'matrix_types_scalar_division' to be enabled
+#endif
+
 typedef double dx5x5_t __attribute__((matrix_type(5, 5)));
 
 // CHECK: %struct.Matrix = type { i8, [12 x float], float }
Index: clang/test/CodeGen/matrix-type-operators.c
===

[PATCH] D97857: [Matrix] Add support for matrix-by-scalar division.

2021-03-10 Thread Florian Hahn via Phabricator via cfe-commits
fhahn added inline comments.



Comment at: clang/test/CodeGen/matrix-type-operators.c:303
 }
 
+// CHECK-LABEL: @divide_double_matrix_scalar_float(

everton.constantino wrote:
> Shouldn't a test for half floating point be added here as well? 
Thanks for taking a look! I don't think we have any tests for half floating 
points at the moment. The tests do not cover each possible combination of types 
I think, because they re-use the existing code-gen for the stuff that's element 
type specific (except deciding whether to use floating point or integer ops).

I think it would probably be good to have some FP16 tests though, but that 
should be a separate change IMO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97857

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


[clang] 57e149d - [analyzer][docs][NFC] Fix typo in checkers.rst

2021-03-10 Thread Balazs Benics via cfe-commits

Author: Balazs Benics
Date: 2021-03-10T12:42:23+01:00
New Revision: 57e149d386286031e0fd118acf162056ce9d4795

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

LOG: [analyzer][docs][NFC] Fix typo in checkers.rst

Move `alpha.core.BoolAssignment` out of the `alpha.clone` enumeration.

Reviewed By: Szelethus

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

Added: 


Modified: 
clang/docs/analyzer/checkers.rst

Removed: 




diff  --git a/clang/docs/analyzer/checkers.rst 
b/clang/docs/analyzer/checkers.rst
index d94296244c63..9a74dffc1658 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1476,6 +1476,9 @@ Reports similar pieces of code.
return y;
  }
 
+alpha.core
+^^
+
 .. _alpha-core-BoolAssignment:
 
 alpha.core.BoolAssignment (ObjC)
@@ -1488,9 +1491,6 @@ Warn about assigning non-{0,1} values to boolean 
variables.
BOOL b = -1; // warn
  }
 
-alpha.core
-^^
-
 .. _alpha-core-C11Lock:
 
 alpha.core.C11Lock



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


[clang] 0dc0e2a - [analyzer][NFC] Add more tests for ArrayBoundCheckerV2

2021-03-10 Thread Balazs Benics via cfe-commits

Author: Balazs Benics
Date: 2021-03-10T12:42:23+01:00
New Revision: 0dc0e2a9ab3cc6be3d4012ea861f54e69854472d

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

LOG: [analyzer][NFC] Add more tests for ArrayBoundCheckerV2

According to a Bugzilla ticket (https://bugs.llvm.org/show_bug.cgi?id=45148),
ArrayBoundCheckerV2 produces a false-positive report.
This patch adds a test demonstrating the current //flawed// behavior.
Also adds several similar test cases just to be on the safe side.

Reviewed By: martong

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

Added: 
clang/test/Analysis/out-of-bounds-false-positive.c

Modified: 


Removed: 




diff  --git a/clang/test/Analysis/out-of-bounds-false-positive.c 
b/clang/test/Analysis/out-of-bounds-false-positive.c
new file mode 100644
index ..3e8071449250
--- /dev/null
+++ b/clang/test/Analysis/out-of-bounds-false-positive.c
@@ -0,0 +1,101 @@
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.security.ArrayBoundV2,debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false -verify %s
+
+void clang_analyzer_eval(int);
+void clang_analyzer_printState();
+
+typedef unsigned long long size_t;
+const char a[] = "abcd"; // extent: 5 bytes
+
+void symbolic_size_t_and_int0(size_t len) {
+  // FIXME: Should not warn for this.
+  (void)a[len + 1]; // expected-warning {{Out of bound memory access}}
+  // We infered that the 'len' must be in a specific range to make the 
previous indexing valid.
+  // len: [0,3]
+  clang_analyzer_eval(len <= 3); // expected - warning {{TRUE}}
+  clang_analyzer_eval(len <= 2); // expected - warning {{UNKNOWN}}
+}
+
+void symbolic_size_t_and_int1(size_t len) {
+  (void)a[len]; // no-warning
+  // len: [0,4]
+  clang_analyzer_eval(len <= 4); // expected-warning {{TRUE}}
+  clang_analyzer_eval(len <= 3); // expected-warning {{UNKNOWN}}
+}
+
+void symbolic_size_t_and_int2(size_t len) {
+  (void)a[len - 1]; // no-warning
+  // len: [1,5]
+  clang_analyzer_eval(1 <= len && len <= 5); // expected-warning {{TRUE}}
+  clang_analyzer_eval(2 <= len); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(len <= 4); // expected-warning {{UNKNOWN}}
+}
+
+void symbolic_uint_and_int0(unsigned len) {
+  (void)a[len + 1]; // no-warning
+  // len: [0,3]
+  clang_analyzer_eval(0 <= len && len <= 3); // expected-warning {{TRUE}}
+  clang_analyzer_eval(1 <= len); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(len <= 2); // expected-warning {{UNKNOWN}}
+}
+
+void symbolic_uint_and_int1(unsigned len) {
+  (void)a[len]; // no-warning
+  // len: [0,4]
+  clang_analyzer_eval(0 <= len && len <= 4); // expected-warning {{TRUE}}
+  clang_analyzer_eval(1 <= len); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(len <= 3); // expected-warning {{UNKNOWN}}
+}
+void symbolic_uint_and_int2(unsigned len) {
+  (void)a[len - 1]; // no-warning
+  // len: [1,5]
+  clang_analyzer_eval(1 <= len && len <= 5); // expected-warning {{TRUE}}
+  clang_analyzer_eval(2 <= len); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(len <= 4); // expected-warning {{UNKNOWN}}
+}
+
+void symbolic_int_and_int0(int len) {
+  (void)a[len + 1]; // no-warning
+  // len: [-1,3]
+  clang_analyzer_eval(-1 <= len && len <= 3); // expected-warning {{TRUE}}
+  clang_analyzer_eval(0 <= len);  // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(len <= 2);  // expected-warning {{UNKNOWN}}
+}
+void symbolic_int_and_int1(int len) {
+  (void)a[len]; // no-warning
+  // len: [0,4]
+  clang_analyzer_eval(0 <= len && len <= 4); // expected-warning {{TRUE}}
+  clang_analyzer_eval(1 <= len); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(len <= 3); // expected-warning {{UNKNOWN}}
+}
+void symbolic_int_and_int2(int len) {
+  (void)a[len - 1]; // no-warning
+  // len: [1,5]
+  clang_analyzer_eval(1 <= len && len <= 5); // expected-warning {{TRUE}}
+  clang_analyzer_eval(2 <= len); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(len <= 4); // expected-warning {{UNKNOWN}}
+}
+
+void symbolic_longlong_and_int0(long long len) {
+  (void)a[len + 1]; // no-warning
+  // len: [-1,3]
+  clang_analyzer_eval(-1 <= len && len <= 3); // expected-warning {{TRUE}}
+  clang_analyzer_eval(0 <= len);  // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(len <= 2);  // expected-warning {{UNKNOWN}}
+}
+
+void symbolic_longlong_and_int1(long long len) {
+  (void)a[len]; // no-warning
+  // len: [0,4]
+  clang_analyzer_eval(0 <= len && len <= 4); // expected-warning {{TRUE}}
+  clang_analyzer_eval(1 <= len); // expected-warning {{UNKNOWN}}
+

[PATCH] D97936: [analyzer][docs][NFC] Fix typo in checkers.rst

2021-03-10 Thread Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG57e149d38628: [analyzer][docs][NFC] Fix typo in checkers.rst 
(authored by Balazs Benics ).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97936

Files:
  clang/docs/analyzer/checkers.rst


Index: clang/docs/analyzer/checkers.rst
===
--- clang/docs/analyzer/checkers.rst
+++ clang/docs/analyzer/checkers.rst
@@ -1476,6 +1476,9 @@
return y;
  }
 
+alpha.core
+^^
+
 .. _alpha-core-BoolAssignment:
 
 alpha.core.BoolAssignment (ObjC)
@@ -1488,9 +1491,6 @@
BOOL b = -1; // warn
  }
 
-alpha.core
-^^
-
 .. _alpha-core-C11Lock:
 
 alpha.core.C11Lock


Index: clang/docs/analyzer/checkers.rst
===
--- clang/docs/analyzer/checkers.rst
+++ clang/docs/analyzer/checkers.rst
@@ -1476,6 +1476,9 @@
return y;
  }
 
+alpha.core
+^^
+
 .. _alpha-core-BoolAssignment:
 
 alpha.core.BoolAssignment (ObjC)
@@ -1488,9 +1491,6 @@
BOOL b = -1; // warn
  }
 
-alpha.core
-^^
-
 .. _alpha-core-C11Lock:
 
 alpha.core.C11Lock
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 0e0ea9f - [analyzer][CTU][NFC] Add an extra regression test

2021-03-10 Thread Balazs Benics via cfe-commits

Author: Balazs Benics
Date: 2021-03-10T12:42:24+01:00
New Revision: 0e0ea9ffb8027b2e785b383d66b06bbd92dc7a46

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

LOG: [analyzer][CTU][NFC] Add an extra regression test

Before `bc713f6a004723d1325bc16e1efc32d0ac82f939` landed, the analyzer
crashed on this reduced example.
It seems important to have bot `ctu` and `-analyzer-opt-analyze-headers`
enabled in the example.

This test file ensures that no regression happens in the future in this regard.

Reviewed By: martong, NoQ

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

Added: 
clang/test/Analysis/Inputs/ctu-inherited-default-ctor-other.cpp
clang/test/Analysis/ctu-inherited-default-ctor.cpp

Modified: 


Removed: 




diff  --git a/clang/test/Analysis/Inputs/ctu-inherited-default-ctor-other.cpp 
b/clang/test/Analysis/Inputs/ctu-inherited-default-ctor-other.cpp
new file mode 100644
index ..b529f416999d
--- /dev/null
+++ b/clang/test/Analysis/Inputs/ctu-inherited-default-ctor-other.cpp
@@ -0,0 +1,27 @@
+namespace llvm {
+template 
+class impl;
+// basecase
+template 
+class impl {};
+// recursion
+template 
+class impl : impl {
+  using child = impl;
+  using child::child; // no-crash
+  impl(T);
+};
+template 
+class container : impl<0, TS...> {};
+} // namespace llvm
+namespace clang {
+class fun {
+  llvm::container k;
+  fun() {}
+};
+class DeclContextLookupResult {
+  static int *const SingleElementDummyList;
+};
+} // namespace clang
+using namespace clang;
+int *const DeclContextLookupResult::SingleElementDummyList = nullptr;

diff  --git a/clang/test/Analysis/ctu-inherited-default-ctor.cpp 
b/clang/test/Analysis/ctu-inherited-default-ctor.cpp
new file mode 100644
index ..e3e7744e493c
--- /dev/null
+++ b/clang/test/Analysis/ctu-inherited-default-ctor.cpp
@@ -0,0 +1,28 @@
+// Should not crash with '-analyzer-opt-analyze-headers' option during CTU 
analysis.
+//
+// RUN: rm -r %t && mkdir -p %t/ctudir
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-inherited-default-ctor-other.cpp.ast \
+// RUN:%S/Inputs/ctu-inherited-default-ctor-other.cpp
+// RUN: echo "c:@N@clang@S@DeclContextLookupResult@SingleElementDummyList 
ctu-inherited-default-ctor-other.cpp.ast" \
+// RUN:   > %t/ctudir/externalDefMap.txt
+//
+// RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
+// RUN:   -analyzer-opt-analyze-headers \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -analyzer-config display-ctu-progress=true \
+// RUN:   -verify %s 2>&1 | FileCheck %s
+//
+// expected-no-diagnostics
+//
+// CHECK: CTU loaded AST file: ctu-inherited-default-ctor-other.cpp.ast
+
+namespace clang {}
+namespace llvm {}
+namespace clang {
+class DeclContextLookupResult {
+  static int *const SingleElementDummyList;
+};
+} // namespace clang



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


[PATCH] D86870: [analyzer] Add more tests for ArrayBoundCheckerV2

2021-03-10 Thread Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0dc0e2a9ab3c: [analyzer][NFC] Add more tests for 
ArrayBoundCheckerV2 (authored by Balazs Benics 
).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86870

Files:
  clang/test/Analysis/out-of-bounds-false-positive.c

Index: clang/test/Analysis/out-of-bounds-false-positive.c
===
--- /dev/null
+++ clang/test/Analysis/out-of-bounds-false-positive.c
@@ -0,0 +1,101 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.security.ArrayBoundV2,debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false -verify %s
+
+void clang_analyzer_eval(int);
+void clang_analyzer_printState();
+
+typedef unsigned long long size_t;
+const char a[] = "abcd"; // extent: 5 bytes
+
+void symbolic_size_t_and_int0(size_t len) {
+  // FIXME: Should not warn for this.
+  (void)a[len + 1]; // expected-warning {{Out of bound memory access}}
+  // We infered that the 'len' must be in a specific range to make the previous indexing valid.
+  // len: [0,3]
+  clang_analyzer_eval(len <= 3); // expected - warning {{TRUE}}
+  clang_analyzer_eval(len <= 2); // expected - warning {{UNKNOWN}}
+}
+
+void symbolic_size_t_and_int1(size_t len) {
+  (void)a[len]; // no-warning
+  // len: [0,4]
+  clang_analyzer_eval(len <= 4); // expected-warning {{TRUE}}
+  clang_analyzer_eval(len <= 3); // expected-warning {{UNKNOWN}}
+}
+
+void symbolic_size_t_and_int2(size_t len) {
+  (void)a[len - 1]; // no-warning
+  // len: [1,5]
+  clang_analyzer_eval(1 <= len && len <= 5); // expected-warning {{TRUE}}
+  clang_analyzer_eval(2 <= len); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(len <= 4); // expected-warning {{UNKNOWN}}
+}
+
+void symbolic_uint_and_int0(unsigned len) {
+  (void)a[len + 1]; // no-warning
+  // len: [0,3]
+  clang_analyzer_eval(0 <= len && len <= 3); // expected-warning {{TRUE}}
+  clang_analyzer_eval(1 <= len); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(len <= 2); // expected-warning {{UNKNOWN}}
+}
+
+void symbolic_uint_and_int1(unsigned len) {
+  (void)a[len]; // no-warning
+  // len: [0,4]
+  clang_analyzer_eval(0 <= len && len <= 4); // expected-warning {{TRUE}}
+  clang_analyzer_eval(1 <= len); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(len <= 3); // expected-warning {{UNKNOWN}}
+}
+void symbolic_uint_and_int2(unsigned len) {
+  (void)a[len - 1]; // no-warning
+  // len: [1,5]
+  clang_analyzer_eval(1 <= len && len <= 5); // expected-warning {{TRUE}}
+  clang_analyzer_eval(2 <= len); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(len <= 4); // expected-warning {{UNKNOWN}}
+}
+
+void symbolic_int_and_int0(int len) {
+  (void)a[len + 1]; // no-warning
+  // len: [-1,3]
+  clang_analyzer_eval(-1 <= len && len <= 3); // expected-warning {{TRUE}}
+  clang_analyzer_eval(0 <= len);  // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(len <= 2);  // expected-warning {{UNKNOWN}}
+}
+void symbolic_int_and_int1(int len) {
+  (void)a[len]; // no-warning
+  // len: [0,4]
+  clang_analyzer_eval(0 <= len && len <= 4); // expected-warning {{TRUE}}
+  clang_analyzer_eval(1 <= len); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(len <= 3); // expected-warning {{UNKNOWN}}
+}
+void symbolic_int_and_int2(int len) {
+  (void)a[len - 1]; // no-warning
+  // len: [1,5]
+  clang_analyzer_eval(1 <= len && len <= 5); // expected-warning {{TRUE}}
+  clang_analyzer_eval(2 <= len); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(len <= 4); // expected-warning {{UNKNOWN}}
+}
+
+void symbolic_longlong_and_int0(long long len) {
+  (void)a[len + 1]; // no-warning
+  // len: [-1,3]
+  clang_analyzer_eval(-1 <= len && len <= 3); // expected-warning {{TRUE}}
+  clang_analyzer_eval(0 <= len);  // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(len <= 2);  // expected-warning {{UNKNOWN}}
+}
+
+void symbolic_longlong_and_int1(long long len) {
+  (void)a[len]; // no-warning
+  // len: [0,4]
+  clang_analyzer_eval(0 <= len && len <= 4); // expected-warning {{TRUE}}
+  clang_analyzer_eval(1 <= len); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(len <= 3); // expected-warning {{UNKNOWN}}
+}
+
+void symbolic_longlong_and_int2(long long len) {
+  (void)a[len - 1]; // no-warning
+  // len: [1,5]
+  clang_analyzer_eval(1 <= len && len <= 5); // expected-warning {{TRUE}}
+  clang_analyzer_eval(2 <= len); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(len <= 4); // expected-warning {{UNKNOWN}}
+}
___
cfe-commits mailing list
cfe-commits@lists.ll

[clang] bcc6624 - [analyzer] Crash fix for alpha.cplusplus.IteratorRange

2021-03-10 Thread Balazs Benics via cfe-commits

Author: Adam Balogh
Date: 2021-03-10T12:42:24+01:00
New Revision: bcc662484a95c95f7d193e6a791fc5d1c4a2c74f

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

LOG: [analyzer] Crash fix for alpha.cplusplus.IteratorRange

If the non-iterator side of an iterator operation
`+`, `+=`, `-` or `-=` is `UndefinedVal` an assertions happens.
This small fix prevents this.

Patch by Adam Balogh.

Reviewed By: NoQ

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
clang/test/Analysis/iterator-range.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
index dd014648eb6f..a47484497771 100644
--- a/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
@@ -228,7 +228,7 @@ void 
IteratorRangeChecker::verifyRandomIncrOrDecr(CheckerContext &C,
 Value = State->getRawSVal(*ValAsLoc);
   }
 
-  if (Value.isUnknown())
+  if (Value.isUnknownOrUndef())
 return;
 
   // Incremention or decremention by 0 is never a bug.

diff  --git a/clang/test/Analysis/iterator-range.cpp 
b/clang/test/Analysis/iterator-range.cpp
index 8d7103929047..849a1e9814ae 100644
--- a/clang/test/Analysis/iterator-range.cpp
+++ b/clang/test/Analysis/iterator-range.cpp
@@ -939,3 +939,10 @@ void ptr_iter_
diff (cont_with_ptr_iterator &c) {
   auto i0 = c.begin(), i1 = c.end();
   ptr
diff _t len = i1 - i0; // no-crash
 }
+
+int uninit_var(int n) {
+  int uninit; // expected-note{{'uninit' declared without an initial value}}
+  return n - uninit; // no-crash
+  // expected-warning@-1 {{The right operand of '-' is a garbage value}}
+  // expected-note@-2 {{The right operand of '-' is a garbage value}}
+}



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


[PATCH] D96586: [analyzer][CTU][NFC] Add an extra regression test

2021-03-10 Thread Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0e0ea9ffb802: [analyzer][CTU][NFC] Add an extra regression 
test (authored by Balazs Benics ).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96586

Files:
  clang/test/Analysis/Inputs/ctu-inherited-default-ctor-other.cpp
  clang/test/Analysis/ctu-inherited-default-ctor.cpp


Index: clang/test/Analysis/ctu-inherited-default-ctor.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-inherited-default-ctor.cpp
@@ -0,0 +1,28 @@
+// Should not crash with '-analyzer-opt-analyze-headers' option during CTU 
analysis.
+//
+// RUN: rm -r %t && mkdir -p %t/ctudir
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-inherited-default-ctor-other.cpp.ast \
+// RUN:%S/Inputs/ctu-inherited-default-ctor-other.cpp
+// RUN: echo "c:@N@clang@S@DeclContextLookupResult@SingleElementDummyList 
ctu-inherited-default-ctor-other.cpp.ast" \
+// RUN:   > %t/ctudir/externalDefMap.txt
+//
+// RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
+// RUN:   -analyzer-opt-analyze-headers \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -analyzer-config display-ctu-progress=true \
+// RUN:   -verify %s 2>&1 | FileCheck %s
+//
+// expected-no-diagnostics
+//
+// CHECK: CTU loaded AST file: ctu-inherited-default-ctor-other.cpp.ast
+
+namespace clang {}
+namespace llvm {}
+namespace clang {
+class DeclContextLookupResult {
+  static int *const SingleElementDummyList;
+};
+} // namespace clang
Index: clang/test/Analysis/Inputs/ctu-inherited-default-ctor-other.cpp
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-inherited-default-ctor-other.cpp
@@ -0,0 +1,27 @@
+namespace llvm {
+template 
+class impl;
+// basecase
+template 
+class impl {};
+// recursion
+template 
+class impl : impl {
+  using child = impl;
+  using child::child; // no-crash
+  impl(T);
+};
+template 
+class container : impl<0, TS...> {};
+} // namespace llvm
+namespace clang {
+class fun {
+  llvm::container k;
+  fun() {}
+};
+class DeclContextLookupResult {
+  static int *const SingleElementDummyList;
+};
+} // namespace clang
+using namespace clang;
+int *const DeclContextLookupResult::SingleElementDummyList = nullptr;


Index: clang/test/Analysis/ctu-inherited-default-ctor.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-inherited-default-ctor.cpp
@@ -0,0 +1,28 @@
+// Should not crash with '-analyzer-opt-analyze-headers' option during CTU analysis.
+//
+// RUN: rm -r %t && mkdir -p %t/ctudir
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-inherited-default-ctor-other.cpp.ast \
+// RUN:%S/Inputs/ctu-inherited-default-ctor-other.cpp
+// RUN: echo "c:@N@clang@S@DeclContextLookupResult@SingleElementDummyList ctu-inherited-default-ctor-other.cpp.ast" \
+// RUN:   > %t/ctudir/externalDefMap.txt
+//
+// RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
+// RUN:   -analyzer-opt-analyze-headers \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -analyzer-config display-ctu-progress=true \
+// RUN:   -verify %s 2>&1 | FileCheck %s
+//
+// expected-no-diagnostics
+//
+// CHECK: CTU loaded AST file: ctu-inherited-default-ctor-other.cpp.ast
+
+namespace clang {}
+namespace llvm {}
+namespace clang {
+class DeclContextLookupResult {
+  static int *const SingleElementDummyList;
+};
+} // namespace clang
Index: clang/test/Analysis/Inputs/ctu-inherited-default-ctor-other.cpp
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-inherited-default-ctor-other.cpp
@@ -0,0 +1,27 @@
+namespace llvm {
+template 
+class impl;
+// basecase
+template 
+class impl {};
+// recursion
+template 
+class impl : impl {
+  using child = impl;
+  using child::child; // no-crash
+  impl(T);
+};
+template 
+class container : impl<0, TS...> {};
+} // namespace llvm
+namespace clang {
+class fun {
+  llvm::container k;
+  fun() {}
+};
+class DeclContextLookupResult {
+  static int *const SingleElementDummyList;
+};
+} // namespace clang
+using namespace clang;
+int *const DeclContextLookupResult::SingleElementDummyList = nullptr;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D85424: [Analyzer] Crash fix for alpha.cplusplus.IteratorRange

2021-03-10 Thread Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbcc662484a95: [analyzer] Crash fix for 
alpha.cplusplus.IteratorRange (authored by baloghadamsoftware, committed by 
Balazs Benics ).

Changed prior to commit:
  https://reviews.llvm.org/D85424?vs=290967&id=329600#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85424

Files:
  clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
  clang/test/Analysis/iterator-range.cpp


Index: clang/test/Analysis/iterator-range.cpp
===
--- clang/test/Analysis/iterator-range.cpp
+++ clang/test/Analysis/iterator-range.cpp
@@ -939,3 +939,10 @@
   auto i0 = c.begin(), i1 = c.end();
   ptrdiff_t len = i1 - i0; // no-crash
 }
+
+int uninit_var(int n) {
+  int uninit; // expected-note{{'uninit' declared without an initial value}}
+  return n - uninit; // no-crash
+  // expected-warning@-1 {{The right operand of '-' is a garbage value}}
+  // expected-note@-2 {{The right operand of '-' is a garbage value}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
@@ -228,7 +228,7 @@
 Value = State->getRawSVal(*ValAsLoc);
   }
 
-  if (Value.isUnknown())
+  if (Value.isUnknownOrUndef())
 return;
 
   // Incremention or decremention by 0 is never a bug.


Index: clang/test/Analysis/iterator-range.cpp
===
--- clang/test/Analysis/iterator-range.cpp
+++ clang/test/Analysis/iterator-range.cpp
@@ -939,3 +939,10 @@
   auto i0 = c.begin(), i1 = c.end();
   ptrdiff_t len = i1 - i0; // no-crash
 }
+
+int uninit_var(int n) {
+  int uninit; // expected-note{{'uninit' declared without an initial value}}
+  return n - uninit; // no-crash
+  // expected-warning@-1 {{The right operand of '-' is a garbage value}}
+  // expected-note@-2 {{The right operand of '-' is a garbage value}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
@@ -228,7 +228,7 @@
 Value = State->getRawSVal(*ValAsLoc);
   }
 
-  if (Value.isUnknown())
+  if (Value.isUnknownOrUndef())
 return;
 
   // Incremention or decremention by 0 is never a bug.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D96586: [analyzer][CTU][NFC] Add an extra regression test

2021-03-10 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

The test doesn't pass: http://45.33.8.238/linux/41341/step_7.txt

I think you just need to say `rm -rf` instead of `rm -r` on line 3 (?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96586

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


[PATCH] D97072: [OpenCL][Docs] Add guidelines for adding new extensions and features

2021-03-10 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia updated this revision to Diff 329602.
Anastasia added a comment.

Added sentence to elaborate the meaning of "useful".


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

https://reviews.llvm.org/D97072

Files:
  clang/docs/OpenCLSupport.rst
  clang/lib/Driver/Types.cpp

Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -396,9 +396,8 @@
 return types::TY_ObjCHeader;
   case types::TY_ObjCXX:
 return types::TY_ObjCXXHeader;
-  case types::TY_CLCXX:
-return types::TY_CLHeader;
   case types::TY_CL:
+  case types::TY_CLCXX:
 return types::TY_CLHeader;
   }
 }
Index: clang/docs/OpenCLSupport.rst
===
--- clang/docs/OpenCLSupport.rst
+++ clang/docs/OpenCLSupport.rst
@@ -168,6 +168,8 @@
 
  $ clang -cc1 -ffake-address-space-map test.cl
 
+.. _opencl_builtins:
+
 OpenCL builtins
 ---
 
@@ -210,6 +212,82 @@
   inserted using ``InsertOCLBuiltinDeclarationsFromTable`` and overload
   resolution takes place.
 
+OpenCL Extensions and Features
+--
+
+Clang implements various extensions to OpenCL kernel languages.
+
+New functionality is accepted as soon as the documentation is detailed to the
+level sufficient to be implemented. There should be an evidence that the
+extension is designed with implementation feasibility in consideration and
+assessment of complexity for C/C++ based compilers. Alternatively, the
+documentation can be accepted in a format of a draft that can be further
+refined during the implementation.
+
+Implementation guidelines
+^
+
+This section explains how to extend clang with the new functionality.
+
+**Parsing functionality**
+
+If an extension modifies the standard parsing it needs to be added to
+the clang frontend source code. This also means that the associated macro
+indicating the presence of the extension should be added to clang.
+
+The default flow for adding a new extension into the frontend is to
+modify `OpenCLExtensions.def
+`_
+
+This will add the macro automatically and also add a field in the target
+options ``clang::TargetOptions::OpenCLFeaturesMap`` to control the exposure
+of the new extension during the compilation.
+
+Note that by default targets like `SPIR` or `X86` expose all the OpenCL
+extensions. For all other targets the configuration has to be made explicitly.
+
+Note that the target extension support performed by clang can be overridden
+with :ref:`-cl-ext ` command-line flags.
+
+**Library functionality**
+
+If an extension adds functionality that does not modify standard language
+parsing it should not require modifying anything other than header files or
+``OpenCLBuiltins.td`` detailed in :ref:``.
+Most commonly such extensions add functionality via libraries (by adding
+non-native types or functions) parsed regularly. Similar to other languages this
+is the most common way to add new functionality.
+
+Clang has standard headers where new types and functions are being added,
+for more details refer to
+:ref:`the section on the OpenCL Header `. The macros indicating
+the presence of such extensions can be added in the standard header files
+conditioned on target specific predefined macros or/and language version
+predefined macros.
+
+**Pragmas**
+
+Some extensions alter standard parsing dynamically via pragmas.
+
+Clang provides a mechanism to add the standard extension pragma
+``OPENCL EXTENSION`` by setting a dedicated flag in the extension list entry of
+``OpenCLExtensions.def``. Note that there is no default behavior for the
+standard extension pragmas as it is not specified (for the standards up to and
+including version 3.0) in a sufficient level of detail and, therefore,
+there is no default functionality provided by clang.
+
+Pragmas without detailed information of their behavior (e.g. an explanation of
+changes it triggers in the parsing) should not be added to clang. Moreover, the
+pragmas should provide useful functionality to the user. For example, such
+functionality should address a practical use case and not be redundant i.e.
+cannot be achieved using existing features.
+
+Note that some legacy extensions (published prior to OpenCL 3.0) still
+provide some non-conformant functionality for pragmas e.g. add diagnostics on
+the use of types or functions. This functionality is not guaranteed to remain in
+future releases. However, any future changes should not affect backward
+compatibility.
+
 .. _opencl_addrsp:
 
 Address spaces attribute
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D82900: [analyzer][Z3-refutation] Add statistics tracking invalidated bug report classes

2021-03-10 Thread Balázs Benics via Phabricator via cfe-commits
steakhal abandoned this revision.
steakhal added a comment.

It might be useful in the future, but right now, I'm not interested in 
upstreaming this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82900

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


[PATCH] D82856: [analyzer][Z3-refutation] Add statistics for refutation visitor

2021-03-10 Thread Balázs Benics via Phabricator via cfe-commits
steakhal abandoned this revision.
steakhal added a comment.

It might be useful in the future, but right now, I'm not interested in 
upstreaming this.


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

https://reviews.llvm.org/D82856

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


[PATCH] D97072: [OpenCL][Docs] Add guidelines for adding new extensions and features

2021-03-10 Thread Sven van Haastregt via Phabricator via cfe-commits
svenvh accepted this revision.
svenvh added a comment.
This revision is now accepted and ready to land.

LGTM with the latest elaboration on "useful".  It seems you accidentally added 
unrelated changes to Types.cpp to this review, so please take care not to 
commit those.


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

https://reviews.llvm.org/D97072

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


[PATCH] D97072: [OpenCL][Docs] Add guidelines for adding new extensions and features

2021-03-10 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia updated this revision to Diff 329605.
Anastasia added a comment.

Fixed patch reupload issue.


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

https://reviews.llvm.org/D97072

Files:
  clang/docs/OpenCLSupport.rst


Index: clang/docs/OpenCLSupport.rst
===
--- clang/docs/OpenCLSupport.rst
+++ clang/docs/OpenCLSupport.rst
@@ -168,6 +168,8 @@
 
  $ clang -cc1 -ffake-address-space-map test.cl
 
+.. _opencl_builtins:
+
 OpenCL builtins
 ---
 
@@ -210,6 +212,82 @@
   inserted using ``InsertOCLBuiltinDeclarationsFromTable`` and overload
   resolution takes place.
 
+OpenCL Extensions and Features
+--
+
+Clang implements various extensions to OpenCL kernel languages.
+
+New functionality is accepted as soon as the documentation is detailed to the
+level sufficient to be implemented. There should be an evidence that the
+extension is designed with implementation feasibility in consideration and
+assessment of complexity for C/C++ based compilers. Alternatively, the
+documentation can be accepted in a format of a draft that can be further
+refined during the implementation.
+
+Implementation guidelines
+^
+
+This section explains how to extend clang with the new functionality.
+
+**Parsing functionality**
+
+If an extension modifies the standard parsing it needs to be added to
+the clang frontend source code. This also means that the associated macro
+indicating the presence of the extension should be added to clang.
+
+The default flow for adding a new extension into the frontend is to
+modify `OpenCLExtensions.def
+`_
+
+This will add the macro automatically and also add a field in the target
+options ``clang::TargetOptions::OpenCLFeaturesMap`` to control the exposure
+of the new extension during the compilation.
+
+Note that by default targets like `SPIR` or `X86` expose all the OpenCL
+extensions. For all other targets the configuration has to be made explicitly.
+
+Note that the target extension support performed by clang can be overridden
+with :ref:`-cl-ext ` command-line flags.
+
+**Library functionality**
+
+If an extension adds functionality that does not modify standard language
+parsing it should not require modifying anything other than header files or
+``OpenCLBuiltins.td`` detailed in :ref:``.
+Most commonly such extensions add functionality via libraries (by adding
+non-native types or functions) parsed regularly. Similar to other languages 
this
+is the most common way to add new functionality.
+
+Clang has standard headers where new types and functions are being added,
+for more details refer to
+:ref:`the section on the OpenCL Header `. The macros indicating
+the presence of such extensions can be added in the standard header files
+conditioned on target specific predefined macros or/and language version
+predefined macros.
+
+**Pragmas**
+
+Some extensions alter standard parsing dynamically via pragmas.
+
+Clang provides a mechanism to add the standard extension pragma
+``OPENCL EXTENSION`` by setting a dedicated flag in the extension list entry of
+``OpenCLExtensions.def``. Note that there is no default behavior for the
+standard extension pragmas as it is not specified (for the standards up to and
+including version 3.0) in a sufficient level of detail and, therefore,
+there is no default functionality provided by clang.
+
+Pragmas without detailed information of their behavior (e.g. an explanation of
+changes it triggers in the parsing) should not be added to clang. Moreover, the
+pragmas should provide useful functionality to the user. For example, such
+functionality should address a practical use case and not be redundant i.e.
+cannot be achieved using existing features.
+
+Note that some legacy extensions (published prior to OpenCL 3.0) still
+provide some non-conformant functionality for pragmas e.g. add diagnostics on
+the use of types or functions. This functionality is not guaranteed to remain 
in
+future releases. However, any future changes should not affect backward
+compatibility.
+
 .. _opencl_addrsp:
 
 Address spaces attribute


Index: clang/docs/OpenCLSupport.rst
===
--- clang/docs/OpenCLSupport.rst
+++ clang/docs/OpenCLSupport.rst
@@ -168,6 +168,8 @@
 
  $ clang -cc1 -ffake-address-space-map test.cl
 
+.. _opencl_builtins:
+
 OpenCL builtins
 ---
 
@@ -210,6 +212,82 @@
   inserted using ``InsertOCLBuiltinDeclarationsFromTable`` and overload
   resolution takes place.
 
+OpenCL Extensions and Features
+--
+
+Clang implements various extensions to OpenCL kernel languages.
+
+New functionality is accepted as soon as the documentation is detailed to the
+level sufficient to be implemented. There should be an evidence that t

[clang] a94ac46 - [analyzer][CTU][NFC] Fix "Add an extra regression test"

2021-03-10 Thread Balazs Benics via cfe-commits

Author: Balazs Benics
Date: 2021-03-10T13:07:49+01:00
New Revision: a94ac467c2974d9afe68f3fe6cff27bd19bcfad0

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

LOG: [analyzer][CTU][NFC] Fix "Add an extra regression test"

As thakis reported, I will replace `rm -r` by `rm -rf`.
I hope it fixes the build bot.

Added: 


Modified: 
clang/test/Analysis/ctu-inherited-default-ctor.cpp

Removed: 




diff  --git a/clang/test/Analysis/ctu-inherited-default-ctor.cpp 
b/clang/test/Analysis/ctu-inherited-default-ctor.cpp
index e3e7744e493c..7c208a86ba93 100644
--- a/clang/test/Analysis/ctu-inherited-default-ctor.cpp
+++ b/clang/test/Analysis/ctu-inherited-default-ctor.cpp
@@ -1,6 +1,6 @@
 // Should not crash with '-analyzer-opt-analyze-headers' option during CTU 
analysis.
 //
-// RUN: rm -r %t && mkdir -p %t/ctudir
+// RUN: rm -rf %t && mkdir -p %t/ctudir
 // RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
 // RUN:   -emit-pch -o %t/ctudir/ctu-inherited-default-ctor-other.cpp.ast \
 // RUN:%S/Inputs/ctu-inherited-default-ctor-other.cpp



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


[PATCH] D96586: [analyzer][CTU][NFC] Add an extra regression test

2021-03-10 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment.

In D96586#2616292 , @thakis wrote:

> The test doesn't pass: http://45.33.8.238/linux/41341/step_7.txt
>
> I think you just need to say `rm -rf` instead of `rm -r` on line 3 (?)

I hope a94ac467c2974d9afe68f3fe6cff27bd19bcfad0 
 fixes 
this.
Thanks for the heads up.

Uh, I missed mentioning this revision in the commit message. I hope it's not 
gonna bite back :|


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96586

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


[PATCH] D97457: [flang][driver] Add `-fdebug-dump-parsing-log`

2021-03-10 Thread Andrzej Warzynski via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG523d7bc6f427: [flang][driver] Add `-fdebug-dump-parsing-log` 
(authored by awarzynski).

Changed prior to commit:
  https://reviews.llvm.org/D97457?vs=328970&id=329612#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97457

Files:
  clang/include/clang/Driver/Options.td
  flang/include/flang/Frontend/FrontendActions.h
  flang/include/flang/Frontend/FrontendOptions.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Driver/debug-parsing-log.f90
  flang/test/Driver/driver-help.f90
  flang/tools/f18/f18.cpp

Index: flang/tools/f18/f18.cpp
===
--- flang/tools/f18/f18.cpp
+++ flang/tools/f18/f18.cpp
@@ -537,7 +537,8 @@
   driver.debugModuleWriter = true;
 } else if (arg == "-fdebug-measure-parse-tree") {
   driver.measureTree = true;
-} else if (arg == "-fdebug-instrumented-parse") {
+} else if (arg == "-fdebug-instrumented-parse" ||
+arg == "-fdebug-dump-parsing-log") {
   options.instrumentedParse = true;
 } else if (arg == "-fdebug-no-semantics") {
   driver.debugNoSemantics = true;
Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -62,6 +62,8 @@
 ! HELP-FC1-NEXT: Enable the old style PARAMETER statement
 ! HELP-FC1-NEXT: -fbackslashSpecify that backslash in string introduces an escape character
 ! HELP-FC1-NEXT: -fdebug-dump-parse-tree Dump the parse tree
+! HELP-FC1-NEXT: -fdebug-dump-parsing-log
+! HELP-FC1-NEXT:   Run instrumented parse and dump the parsing log
 ! HELP-FC1-NEXT: -fdebug-dump-provenance Dump provenance
 ! HELP-FC1-NEXT: -fdebug-dump-symbolsDump symbols after the semantic analysis
 ! HELP-FC1-NEXT: -fdebug-measure-parse-tree
Index: flang/test/Driver/debug-parsing-log.f90
===
--- /dev/null
+++ flang/test/Driver/debug-parsing-log.f90
@@ -0,0 +1,31 @@
+! RUN: %flang_fc1 -fdebug-dump-parsing-log %s  2>&1 | FileCheck %s
+
+!-
+! EXPECTED OUTPUT
+!-
+! Below are just few lines extracted from the dump. The actual output is much _much_ bigger.
+
+! CHECK: {{.*}}/debug-parsing-log.f90:31:1: IMPLICIT statement
+! CHECK-NEXT:  END PROGRAM
+! CHECK-NEXT:  ^
+! CHECK-NEXT:  fail 3
+! CHECK-NEXT: {{.*}}/debug-parsing-log.f90:31:1: error: expected 'IMPLICIT NONE'
+! CHECK-NEXT:   END PROGRAM
+! CHECK-NEXT:   ^
+! CHECK-NEXT: {{.*}}/debug-parsing-log.f90:31:1: in the context: IMPLICIT statement
+! CHECK-NEXT:   END PROGRAM
+! CHECK-NEXT:   ^
+! CHECK-NEXT: {{.*}}/debug-parsing-log.f90:31:1: in the context: implicit part
+! CHECK-NEXT:   END PROGRAM
+! CHECK-NEXT:   ^
+! CHECK-NEXT: {{.*}}/debug-parsing-log.f90:31:1: in the context: specification part
+! CHECK-NEXT:   END PROGRAM
+! CHECK-NEXT:   ^
+! CHECK-NEXT: {{.*}}/debug-parsing-log.f90:31:1: in the context: main program
+! CHECK-NEXT:   END PROGRAM
+! CHECK-NEXT:   ^
+
+!-
+! TEST INPUT
+!-
+END PROGRAM
Index: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -52,6 +52,9 @@
   case DebugDumpProvenance:
 return std::make_unique();
 break;
+  case DebugDumpParsingLog:
+return std::make_unique();
+break;
   case DebugMeasureParseTree:
 return std::make_unique();
 break;
Index: flang/lib/Frontend/FrontendActions.cpp
===
--- flang/lib/Frontend/FrontendActions.cpp
+++ flang/lib/Frontend/FrontendActions.cpp
@@ -307,6 +307,13 @@
   }
 }
 
+void DebugDumpParsingLogAction::ExecuteAction() {
+  CompilerInstance &ci = this->instance();
+
+  ci.parsing().Parse(llvm::errs());
+  ci.parsing().DumpParsingLog(llvm::outs());
+}
+
 void EmitObjAction::ExecuteAction() {
   CompilerInstance &ci = this->instance();
   unsigned DiagID = ci.diagnostics().getCustomDiagID(
Index: flang/lib/Frontend/CompilerInvocation.cpp
===
--- flang/lib/Frontend/CompilerInvocation.cpp
+++ flang/lib/Frontend/CompilerInvocation.cpp
@@ -85,6 +85,15 @@
   return true;
 }
 
+// Tweak the frontend configuration based on the frontend action
+static void setUpFrontendBasedOnAction(FrontendOptions &opts) {
+  assert(opts.programAction_ != Fortran::frontend::InvalidAction &&
+  "Fortran frontend action not set!");
+
+  if (opts.programA

[clang] 523d7bc - [flang][driver] Add `-fdebug-dump-parsing-log`

2021-03-10 Thread Andrzej Warzynski via cfe-commits

Author: Andrzej Warzynski
Date: 2021-03-10T12:09:16Z
New Revision: 523d7bc6f427f9ae32e54dbf1764826cfb269d21

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

LOG: [flang][driver] Add `-fdebug-dump-parsing-log`

This patch adds `-fdebug-dump-parsing-log` in the new driver. This option is
semantically identical to `-fdebug-instrumented-parse` in `f18` (the
former is added as an alias in `f18`).

As dumping the parsing log makes only sense for instrumented parses, we
set Fortran::parser::Options::instrumentedParse to `True` when
`-fdebug-dump-parsing-log` is used. This is consistent with `f18`.

To facilitate tweaking the configuration of the frontend based on the
action being requested, `setUpFrontendBasedOnAction` is introduced in
CompilerInvocation.cpp.

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

Added: 
flang/test/Driver/debug-parsing-log.f90

Modified: 
clang/include/clang/Driver/Options.td
flang/include/flang/Frontend/FrontendActions.h
flang/include/flang/Frontend/FrontendOptions.h
flang/lib/Frontend/CompilerInvocation.cpp
flang/lib/Frontend/FrontendActions.cpp
flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
flang/test/Driver/driver-help.f90
flang/tools/f18/f18.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 8e71aff2e96d..376a3baf0a4b 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4361,6 +4361,8 @@ def fdebug_dump_parse_tree : Flag<["-"], 
"fdebug-dump-parse-tree">, Group;
 def fdebug_dump_provenance : Flag<["-"], "fdebug-dump-provenance">, 
Group,
   HelpText<"Dump provenance">;
+def fdebug_dump_parsing_log : Flag<["-"], "fdebug-dump-parsing-log">, 
Group,
+  HelpText<"Run instrumented parse and dump the parsing log">;
 def fdebug_measure_parse_tree : Flag<["-"], "fdebug-measure-parse-tree">, 
Group,
   HelpText<"Measure the parse tree">;
 def fdebug_pre_fir_tree : Flag<["-"], "fdebug-pre-fir-tree">, 
Group,

diff  --git a/flang/include/flang/Frontend/FrontendActions.h 
b/flang/include/flang/Frontend/FrontendActions.h
index f6dfdd2dc09b..35d1e6f29b0f 100644
--- a/flang/include/flang/Frontend/FrontendActions.h
+++ b/flang/include/flang/Frontend/FrontendActions.h
@@ -54,6 +54,10 @@ class DebugDumpProvenanceAction : public PrescanAction {
   void ExecuteAction() override;
 };
 
+class DebugDumpParsingLogAction : public PrescanAction {
+  void ExecuteAction() override;
+};
+
 class DebugMeasureParseTreeAction : public PrescanAction {
   void ExecuteAction() override;
 };

diff  --git a/flang/include/flang/Frontend/FrontendOptions.h 
b/flang/include/flang/Frontend/FrontendOptions.h
index 98a717dfa6ec..48182f488466 100644
--- a/flang/include/flang/Frontend/FrontendOptions.h
+++ b/flang/include/flang/Frontend/FrontendOptions.h
@@ -50,6 +50,9 @@ enum ActionKind {
   /// Dump provenance
   DebugDumpProvenance,
 
+  /// Parse then output the parsing log
+  DebugDumpParsingLog,
+
   /// Parse then output the number of objects in the parse tree and the overall
   /// size
   DebugMeasureParseTree,
@@ -172,6 +175,9 @@ class FrontendOptions {
   /// Show the -version text.
   unsigned showVersion_ : 1;
 
+  /// Instrument the parse to get a more verbose log
+  unsigned instrumentedParse_ : 1;
+
   /// The input files and their types.
   std::vector inputs_;
 

diff  --git a/flang/lib/Frontend/CompilerInvocation.cpp 
b/flang/lib/Frontend/CompilerInvocation.cpp
index 0dc7e7c3d395..17649700e0d2 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -85,6 +85,15 @@ bool 
Fortran::frontend::ParseDiagnosticArgs(clang::DiagnosticOptions &opts,
   return true;
 }
 
+// Tweak the frontend configuration based on the frontend action
+static void setUpFrontendBasedOnAction(FrontendOptions &opts) {
+  assert(opts.programAction_ != Fortran::frontend::InvalidAction &&
+  "Fortran frontend action not set!");
+
+  if (opts.programAction_ == DebugDumpParsingLog)
+opts.instrumentedParse_ = true;
+}
+
 static InputKind ParseFrontendArgs(FrontendOptions &opts,
 llvm::opt::ArgList &args, clang::DiagnosticsEngine &diags) {
 
@@ -125,6 +134,9 @@ static InputKind ParseFrontendArgs(FrontendOptions &opts,
 case clang::driver::options::OPT_fdebug_dump_provenance:
   opts.programAction_ = DebugDumpProvenance;
   break;
+case clang::driver::options::OPT_fdebug_dump_parsing_log:
+  opts.programAction_ = DebugDumpParsingLog;
+  break;
 case clang::driver::options::OPT_fdebug_measure_parse_tree:
   opts.programAction_ = DebugMeasureParseTree;
   break;
@@ -264,6 +276,9 @@ static InputKind ParseFrontendArgs(FrontendOptions &opts,
   << arg->getAsStrin

[PATCH] D93164: [AST] Add generator for source location introspection

2021-03-10 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

This is almost ready but a few more points need addressing.
Running clang-format over the inc file is pointless and just extends 
compilation time while adding an unnecessary dependency on clang-format.
The inc file should likely live in the include build directory, All tablegen 
files seem to live in there. You could either move the CMake code that 
generates it into the include directory, or alter the directory, this should do 
that if you want and its safer than replace as it would only change the last 
/lib/ detected.

  # Replace the last lib component of the current binary directory with include
  string(FIND ${CMAKE_CURRENT_BINARY_DIR} "/lib/" PATH_LIB_START REVERSE)
  if(PATH_LIB_START EQUAL -1)
message(FATAL_ERROR "Couldn't find lib component in binary directory")
  endif()
  math(EXPR PATH_LIB_END "${PATH_LIB_START}+5")
  string(SUBSTRING ${CMAKE_CURRENT_BINARY_DIR} 0 ${PATH_LIB_START} PATH_HEAD)
  string(SUBSTRING ${CMAKE_CURRENT_BINARY_DIR} ${PATH_LIB_END} -1 PATH_TAIL)
  string(CONCAT BINARY_INCLUDE_DIR ${PATH_HEAD} "/include/" ${PATH_TAIL})

After moving it to the Include output folder, In the cpp file you would need 
`#include "clang/Tooling/NodeIntrospection.inc"`.
This would also remove a lot of those commands in `lib/tooling/CMakeLists.txt`.
Tablegen has a command line option `--write-if-changed` It may be wise to also 
include that in your generator script instead of using copy-if-different in the 
aforementioned CMakeLists.txt.




Comment at: clang/include/clang/Tooling/NodeIntrospection.h:16-17
+
+#include 
+#include 
+

These should be quoted includes



Comment at: clang/lib/Tooling/CMakeLists.txt:29
+
+add_custom_command(
+OUTPUT ${CMAKE_BINARY_DIR}/ASTNodeAPI.json

It may be wise to use the `COMMENT` argument to let the users know that it's 
building the ASTNodeAPI.json.



Comment at: clang/lib/Tooling/CMakeLists.txt:56
+COMMAND
+${PYTHON_EXECUTABLE} 
${CMAKE_CURRENT_SOURCE_DIR}/DumpTool/generate_cxx_src_locs.py
+  --json-input-path ${CMAKE_BINARY_DIR}/ASTNodeAPI.json

Likewise a comment to say building NodeIntrospection.inc.



Comment at: clang/lib/Tooling/CMakeLists.txt:99
+  NodeIntrospection.cpp
+  NodeIntrospection.inc
   Tooling.cpp

This shouldn't appear in the source list.



Comment at: clang/lib/Tooling/DumpTool/APIData.h:10
+
+#ifndef LLVM_CLANG_TOOLING_APIDATA_H
+#define LLVM_CLANG_TOOLING_APIDATA_H

aaron.ballman wrote:
> Might as well fix this lint warning.
Header guard should be `LLVM_CLANG_LIB_TOOLING_DUMPTOOL_APIDATA_H`



Comment at: clang/lib/Tooling/NodeIntrospection.cpp:13-15
+#include 
+
+#include 

Quoted includes and the `NodeIntrospection.h` include is the MainFileInclude so 
should appear first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93164

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


[PATCH] D98076: [OpenCL][Docs] Release 12.0 notes

2021-03-10 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia updated this revision to Diff 329617.
Anastasia added a comment.

Added corrections from Sven.


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

https://reviews.llvm.org/D98076

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -200,10 +200,29 @@
 Objective-C Language Changes in Clang
 -
 
-OpenCL C Language Changes in Clang
---
-
-...
+OpenCL Kernel Language Changes in Clang
+---
+
+- Improved online documentation: :doc:`UsersManual` and :doc:`OpenCLSupport` 
pages.
+- Added ``-cl-std=CL3.0`` and predefined version macro for OpenCL 3.0.
+- Added ``-cl-std=CL1.0`` and mapped to the existing OpenCL 1.0 functionality.
+- Improved OpenCL extension handling per target.
+- Added clang extension for function pointers ``__cl_clang_function_pointers``
+  and variadic functions ``__cl_clang_variadic_functions``, more details can be
+  found in :doc:`LanguageExtensions`.
+- Improved diagnostics for nested pointers and  unevaluated ``vec_step``
+  expression.
+- Added ``global_device`` and ``global_host`` address spaces for USM
+  allocations.
+
+Miscellaneous improvements in C++ for OpenCL support:
+
+- Added diagnostics for pointers to member functions and references to
+  functions.
+- Added support of ``vec_step`` builtin.
+- Fixed ICE on address spaces with forwarding references and templated copy
+  constructors.
+- Removed warning for variadic macro use.
 
 ABI Changes in Clang
 


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -200,10 +200,29 @@
 Objective-C Language Changes in Clang
 -
 
-OpenCL C Language Changes in Clang
---
-
-...
+OpenCL Kernel Language Changes in Clang
+---
+
+- Improved online documentation: :doc:`UsersManual` and :doc:`OpenCLSupport` pages.
+- Added ``-cl-std=CL3.0`` and predefined version macro for OpenCL 3.0.
+- Added ``-cl-std=CL1.0`` and mapped to the existing OpenCL 1.0 functionality.
+- Improved OpenCL extension handling per target.
+- Added clang extension for function pointers ``__cl_clang_function_pointers``
+  and variadic functions ``__cl_clang_variadic_functions``, more details can be
+  found in :doc:`LanguageExtensions`.
+- Improved diagnostics for nested pointers and  unevaluated ``vec_step``
+  expression.
+- Added ``global_device`` and ``global_host`` address spaces for USM
+  allocations.
+
+Miscellaneous improvements in C++ for OpenCL support:
+
+- Added diagnostics for pointers to member functions and references to
+  functions.
+- Added support of ``vec_step`` builtin.
+- Fixed ICE on address spaces with forwarding references and templated copy
+  constructors.
+- Removed warning for variadic macro use.
 
 ABI Changes in Clang
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D98076: [OpenCL][Docs] Release 12.0 notes

2021-03-10 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia marked 3 inline comments as done.
Anastasia added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:214
+- Added ``global_device`` and ``global_host`` address spaces for USM
+  allocations.
+

svenvh wrote:
> Perhaps one more point to mention:
> ```
>  - Allow pointer-to-pointer kernel arguments beyond OpenCL 1.2.
> ```
> (related commit is 523775f96742 ("[OpenCL] Allow pointer-to-pointer kernel 
> args beyond CL 1.2", 2020-12-01)).
I have covered it in this but it is combined with another diagnostic:


```
- Improved diagnostics for nested pointers and unevaluated ``vec_step`` 
expression.
```

But if you think it is better I can change as you have suggested.


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

https://reviews.llvm.org/D98076

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


[PATCH] D98321: [NFC] Unify FIME with FIXME in comments

2021-03-10 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.

Looks good. Thanks for the fix! I'll get it landed for you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98321

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


[clang-tools-extra] 481079e - [NFC] Unify FIME with FIXME in comments

2021-03-10 Thread Alexander Kornienko via cfe-commits

Author: Jinzheng Tu
Date: 2021-03-10T14:00:51+01:00
New Revision: 481079e2841f1d7aafbbd627e7028bcc632a4ef7

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

LOG: [NFC] Unify FIME with FIXME in comments

There are 5 occurrences FIME and 15333 FIXME. All of them should be FIXME.

Reviewed By: alexfh

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp 
b/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
index 12795f0468fd..23765da1b46f 100644
--- a/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
+++ b/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
@@ -230,7 +230,7 @@ void 
ExpandModularHeadersPPCallbacks::HasInclude(SourceLocation Loc, StringRef,
 void ExpandModularHeadersPPCallbacks::PragmaOpenCLExtension(
 SourceLocation NameLoc, const IdentifierInfo *, SourceLocation StateLoc,
 unsigned) {
-  // FIME: Figure out whether it's the right location to parse to.
+  // FIXME: Figure out whether it's the right location to parse to.
   parseToLocation(NameLoc);
 }
 void ExpandModularHeadersPPCallbacks::PragmaWarning(SourceLocation Loc,
@@ -256,7 +256,7 @@ void ExpandModularHeadersPPCallbacks::MacroExpands(const 
Token &MacroNameTok,
const MacroDefinition &,
SourceRange Range,
const MacroArgs *) {
-  // FIME: Figure out whether it's the right location to parse to.
+  // FIXME: Figure out whether it's the right location to parse to.
   parseToLocation(Range.getBegin());
 }
 void ExpandModularHeadersPPCallbacks::MacroDefined(const Token &MacroNameTok,
@@ -271,12 +271,12 @@ void ExpandModularHeadersPPCallbacks::MacroUndefined(
 void ExpandModularHeadersPPCallbacks::Defined(const Token &MacroNameTok,
   const MacroDefinition &,
   SourceRange Range) {
-  // FIME: Figure out whether it's the right location to parse to.
+  // FIXME: Figure out whether it's the right location to parse to.
   parseToLocation(Range.getBegin());
 }
 void ExpandModularHeadersPPCallbacks::SourceRangeSkipped(
 SourceRange Range, SourceLocation EndifLoc) {
-  // FIME: Figure out whether it's the right location to parse to.
+  // FIXME: Figure out whether it's the right location to parse to.
   parseToLocation(EndifLoc);
 }
 void ExpandModularHeadersPPCallbacks::If(SourceLocation Loc, SourceRange,

diff  --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp 
b/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp
index 4015a5a0ce70..1cffe20cbb83 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp
@@ -529,7 +529,7 @@ SDValue VectorLegalizer::LegalizeOp(SDValue Op) {
   return RecursivelyLegalizeResults(Op, ResultVals);
 }
 
-// FIME: This is very similar to the X86 override of
+// FIXME: This is very similar to the X86 override of
 // TargetLowering::LowerOperationWrapper. Can we merge them somehow?
 bool VectorLegalizer::LowerOperationWrapper(SDNode *Node,
 SmallVectorImpl &Results) 
{



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


[clang-tools-extra] 99b01cf - Revert "[clangd] Enable reflection for clangd-index-server"

2021-03-10 Thread Kadir Cetinkaya via cfe-commits

Author: Kadir Cetinkaya
Date: 2021-03-10T14:12:37+01:00
New Revision: 99b01cf28db9db1a3ec0e25367bd325b7aca6d43

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

LOG: Revert "[clangd] Enable reflection for clangd-index-server"

This reverts commit 8080ea4c4b8c456c72c617587cc32f174b3105c1.

As discussed offline we should only do that for debug builds.

Added: 


Modified: 
clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
clang-tools-extra/clangd/index/remote/server/Server.cpp
llvm/cmake/modules/FindGRPC.cmake

Removed: 




diff  --git a/clang-tools-extra/clangd/index/remote/server/CMakeLists.txt 
b/clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
index 3ba0c476df0c..e6959db6bbd8 100644
--- a/clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
+++ b/clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
@@ -17,6 +17,4 @@ target_link_libraries(clangd-index-server
   RemoteIndexProto
   RemoteIndexServiceProto
   clangdRemoteMarshalling
-
-  grpc++_reflection
   )

diff  --git a/clang-tools-extra/clangd/index/remote/server/Server.cpp 
b/clang-tools-extra/clangd/index/remote/server/Server.cpp
index 768f3c9fd143..3de2c38f7c08 100644
--- a/clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ b/clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -30,7 +30,6 @@
 #include "llvm/Support/VirtualFileSystem.h"
 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -314,7 +313,6 @@ void runServerAndWait(clangd::SymbolIndex &Index, 
llvm::StringRef ServerAddress,
   RemoteIndexServer Service(Index, IndexRoot);
 
   grpc::EnableDefaultHealthCheckService(true);
-  grpc::reflection::InitProtoReflectionServerBuilderPlugin();
   grpc::ServerBuilder Builder;
   Builder.AddListeningPort(ServerAddress.str(),
grpc::InsecureServerCredentials());

diff  --git a/llvm/cmake/modules/FindGRPC.cmake 
b/llvm/cmake/modules/FindGRPC.cmake
index d39d10e4e93c..8fdb3506dff1 100644
--- a/llvm/cmake/modules/FindGRPC.cmake
+++ b/llvm/cmake/modules/FindGRPC.cmake
@@ -22,8 +22,6 @@ if (GRPC_INSTALL_PATH)
   add_library(protobuf ALIAS protobuf::libprotobuf)
   set_target_properties(gRPC::grpc++ PROPERTIES IMPORTED_GLOBAL TRUE)
   add_library(grpc++ ALIAS gRPC::grpc++)
-  set_target_properties(gRPC::grpc++_reflection PROPERTIES IMPORTED_GLOBAL 
TRUE)
-  add_library(grpc++_reflection ALIAS gRPC::grpc++_reflection)
 
   set(GRPC_CPP_PLUGIN $)
   set(PROTOC ${Protobuf_PROTOC_EXECUTABLE})
@@ -73,9 +71,6 @@ else()
   add_library(grpc++ UNKNOWN IMPORTED GLOBAL)
   message(STATUS "Using grpc++: " ${GRPC_LIBRARY})
   set_target_properties(grpc++ PROPERTIES IMPORTED_LOCATION ${GRPC_LIBRARY})
-  find_library(GRPC_REFLECTION_LIBRARY grpc++_reflection $GRPC_OPTS REQUIRED)
-  add_library(grpc++_reflection UNKNOWN IMPORTED GLOBAL)
-  set_target_properties(grpc++_reflection PROPERTIES IMPORTED_LOCATION 
${GRPC_REFLECTION_LIBRARY})
   find_library(PROTOBUF_LIBRARY protobuf $PROTOBUF_OPTS REQUIRED)
   message(STATUS "Using protobuf: " ${PROTOBUF_LIBRARY})
   add_library(protobuf UNKNOWN IMPORTED GLOBAL)



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


[PATCH] D98321: [NFC] Unify FIME with FIXME in comments

2021-03-10 Thread Alexander Kornienko via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG481079e2841f: [NFC] Unify FIME with FIXME in comments 
(authored by b1f6c1c4, committed by alexfh).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98321

Files:
  clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
  llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp


Index: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp
+++ llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp
@@ -529,7 +529,7 @@
   return RecursivelyLegalizeResults(Op, ResultVals);
 }
 
-// FIME: This is very similar to the X86 override of
+// FIXME: This is very similar to the X86 override of
 // TargetLowering::LowerOperationWrapper. Can we merge them somehow?
 bool VectorLegalizer::LowerOperationWrapper(SDNode *Node,
 SmallVectorImpl &Results) 
{
Index: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
===
--- clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
+++ clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
@@ -230,7 +230,7 @@
 void ExpandModularHeadersPPCallbacks::PragmaOpenCLExtension(
 SourceLocation NameLoc, const IdentifierInfo *, SourceLocation StateLoc,
 unsigned) {
-  // FIME: Figure out whether it's the right location to parse to.
+  // FIXME: Figure out whether it's the right location to parse to.
   parseToLocation(NameLoc);
 }
 void ExpandModularHeadersPPCallbacks::PragmaWarning(SourceLocation Loc,
@@ -256,7 +256,7 @@
const MacroDefinition &,
SourceRange Range,
const MacroArgs *) {
-  // FIME: Figure out whether it's the right location to parse to.
+  // FIXME: Figure out whether it's the right location to parse to.
   parseToLocation(Range.getBegin());
 }
 void ExpandModularHeadersPPCallbacks::MacroDefined(const Token &MacroNameTok,
@@ -271,12 +271,12 @@
 void ExpandModularHeadersPPCallbacks::Defined(const Token &MacroNameTok,
   const MacroDefinition &,
   SourceRange Range) {
-  // FIME: Figure out whether it's the right location to parse to.
+  // FIXME: Figure out whether it's the right location to parse to.
   parseToLocation(Range.getBegin());
 }
 void ExpandModularHeadersPPCallbacks::SourceRangeSkipped(
 SourceRange Range, SourceLocation EndifLoc) {
-  // FIME: Figure out whether it's the right location to parse to.
+  // FIXME: Figure out whether it's the right location to parse to.
   parseToLocation(EndifLoc);
 }
 void ExpandModularHeadersPPCallbacks::If(SourceLocation Loc, SourceRange,


Index: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp
+++ llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp
@@ -529,7 +529,7 @@
   return RecursivelyLegalizeResults(Op, ResultVals);
 }
 
-// FIME: This is very similar to the X86 override of
+// FIXME: This is very similar to the X86 override of
 // TargetLowering::LowerOperationWrapper. Can we merge them somehow?
 bool VectorLegalizer::LowerOperationWrapper(SDNode *Node,
 SmallVectorImpl &Results) {
Index: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
===
--- clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
+++ clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
@@ -230,7 +230,7 @@
 void ExpandModularHeadersPPCallbacks::PragmaOpenCLExtension(
 SourceLocation NameLoc, const IdentifierInfo *, SourceLocation StateLoc,
 unsigned) {
-  // FIME: Figure out whether it's the right location to parse to.
+  // FIXME: Figure out whether it's the right location to parse to.
   parseToLocation(NameLoc);
 }
 void ExpandModularHeadersPPCallbacks::PragmaWarning(SourceLocation Loc,
@@ -256,7 +256,7 @@
const MacroDefinition &,
SourceRange Range,
const MacroArgs *) {
-  // FIME: Figure out whether it's the right location to parse to.
+  // FIXME: Figure out whether it's the right location to parse to.
   parseToLocation(Range.getBegin());
 }
 void ExpandModularHeadersPPCallbacks::MacroDefined(const Token &MacroNameTok,
@@ -271,12 +271,12 @@
 void ExpandModularHeadersPPCallbacks::

[PATCH] D98296: [clang-tidy] Simplify readability checks to not need ignoring* matchers

2021-03-10 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

Please address the format issues.




Comment at: 
clang-tools-extra/clang-tidy/readability/UniqueptrDeleteReleaseCheck.cpp:45
   callee(
   memberExpr(hasObjectExpression(allOf(
  anyOf(hasType(UniquePtrWithDefaultDelete),

This allOf matcher only contains 1 sub matcher so is breaking the build.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98296

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


[PATCH] D98337: Add support for digit separators in C2x

2021-03-10 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman created this revision.
aaron.ballman added reviewers: rsmith, jyknight, rjmccall.
aaron.ballman requested review of this revision.
Herald added a project: clang.

WG14 adopted N2626 at the meetings this week. This proposal adds support for 
using `'` as a digit separator in a numeric literal which is compatible with 
the C++ feature.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98337

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/Lexer.cpp
  clang/test/Lexer/c2x_digit_separators.c
  clang/test/Sema/pre-c2x-compat.c


Index: clang/test/Sema/pre-c2x-compat.c
===
--- /dev/null
+++ clang/test/Sema/pre-c2x-compat.c
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -std=c2x -Wpre-c2x-compat -pedantic -fsyntax-only -verify
+
+int digit_seps = 123'456; // expected-warning {{digit separators are 
incompatible with C standards before C2x}}
Index: clang/test/Lexer/c2x_digit_separators.c
===
--- /dev/null
+++ clang/test/Lexer/c2x_digit_separators.c
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 -std=c2x -verify %s
+
+_Static_assert(1'2'3 == 12'3, "");
+_Static_assert(1'000'000 == 0xf'4240, "");
+_Static_assert(0'004'000'000 == 0x10', "");
+_Static_assert(0b0101'0100 == 0x54, "");
+
+int a0 = 123'; //'; // expected-error {{expected ';'}}
+int b0 = 0'xff; // expected-error {{digit separator cannot appear at end of 
digit sequence}} expected-error {{suffix 'xff' on integer}}
+int c0 = 0x'ff; // expected-error {{suffix 'x'ff' on integer}}
+int d0 = 0'1234; // ok, octal
+int e0 = 0'b1010; // expected-error {{digit 'b' in octal constant}}
+int f0 = 0b'1010; // expected-error {{invalid digit 'b' in octal}}
+int h0 = 0x1e+1; // expected-error {{invalid suffix '+1' on integer constant}}
+int i0 = 0x1'e+1; // ok, 'e+' is not recognized after a digit separator
+
+float a1 = 1'e1; // expected-error {{digit separator cannot appear at end of 
digit sequence}}
+float b1 = 1'0e1;
+float c1 = 1.'0e1; // expected-error {{digit separator cannot appear at start 
of digit sequence}}
+float d1 = 1.0'e1; // expected-error {{digit separator cannot appear at end of 
digit sequence}}
+float e1 = 1e'1; // expected-error {{digit separator cannot appear at start of 
digit sequence}}
+float g1 = 0.'0; // expected-error {{digit separator cannot appear at start of 
digit sequence}}
+float h1 = .'0; // '; // expected-error {{expected expression}}, lexed as . 
followed by character literal
+float i1 = 0x.'0p0; // expected-error {{digit separator cannot appear at start 
of digit sequence}}
+float j1 = 0x'0.0p0; // expected-error {{invalid suffix 'x'0.0p0'}}
+float k1 = 0x0'.0p0; // '; // expected-error {{expected ';'}}
+float l1 = 0x0.'0p0; // expected-error {{digit separator cannot appear at 
start of digit sequence}}
+float m1 = 0x0.0'p0; // expected-error {{digit separator cannot appear at end 
of digit sequence}}
+float n1 = 0x0.0p'0; // expected-error {{digit separator cannot appear at 
start of digit sequence}}
+float p1 = 0'e1; // expected-error {{digit separator cannot appear at end of 
digit sequence}}
+float q1 = 0'0e1;
+float r1 = 0.'0e1; // expected-error {{digit separator cannot appear at start 
of digit sequence}}
+float s1 = 0.0'e1; // expected-error {{digit separator cannot appear at end of 
digit sequence}}
+float t1 = 0.0e'1; // expected-error {{digit separator cannot appear at start 
of digit sequence}}
+float u1 = 0x.'p1f; // expected-error {{hexadecimal floating constant requires 
a significand}}
+float v1 = 0e'f; // expected-error {{exponent has no digits}}
+float w1 = 0x0p'f; // expected-error {{exponent has no digits}}
+float x1 = 0'e+1; // expected-error {{digit separator cannot appear at end of 
digit sequence}}
+float y1 = 0x0'p+1; // expected-error {{digit separator cannot appear at end 
of digit sequence}}
+
+#line 123'456
+_Static_assert(__LINE__ == 123456, "");
+
+// UCNs can appear before digit separators but not after.
+int a2 = 0\u1234'5; // expected-error {{invalid suffix '\u1234'5' on integer 
constant}}
+int b2 = 0'\u12345; // '; // expected-error {{expected ';'}}
+
+// extended characters can appear before digit separators but not after.
+int a3 = 0ሴ'5; // expected-error {{invalid suffix 'ሴ'5' on integer constant}}
+int b3 = 0'ሴ5; // '; // expected-error {{expected ';'}}
+
Index: clang/lib/Lex/Lexer.cpp
===
--- clang/lib/Lex/Lexer.cpp
+++ clang/lib/Lex/Lexer.cpp
@@ -1788,12 +1788,14 @@
   }
 
   // If we have a digit separator, continue.
-  if (C == '\'' && getLangOpts().CPlusPlus14) {
+  if (C == '\'' && (getLangOpts().CPlusPlus14 || getLangOpts().C2x)) {
 unsigned NextSize;
 char Next = getCharAndSizeNoWarn(CurPtr + Size, NextSize, getLangOpts());
 if (isIdentifierBody(Next)) {
   if (!isLexingRawMode())
-Diag(CurPtr, diag::warn_cxx11_compat_digit_separato

[PATCH] D97717: [SYCL] Rework the SYCL driver options

2021-03-10 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

Ping.


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

https://reviews.llvm.org/D97717

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


[PATCH] D97871: Update diagnostic groups for pre-compat warnings

2021-03-10 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97871

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


[PATCH] D98338: [clang-tidy] Fix bugprone-terminating-continue when continue appears inside a switch

2021-03-10 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision.
njames93 added reviewers: aaron.ballman, alexfh.
Herald added a subscriber: xazax.hun.
njames93 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes https://llvm.org/PR9492.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98338

Files:
  clang-tools-extra/clang-tidy/bugprone/TerminatingContinueCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-terminating-continue.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-terminating-continue.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-terminating-continue.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-terminating-continue.cpp
@@ -62,4 +62,12 @@
   if (n>2) continue;
 }
   } while (false);
+
+  do {
+switch (2) {
+case 1:
+case 2:
+  continue;
+}
+  } while (false);
 }
Index: clang-tools-extra/clang-tidy/bugprone/TerminatingContinueCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/TerminatingContinueCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/TerminatingContinueCheck.cpp
@@ -26,10 +26,11 @@
  equalsBoundNode("closestLoop"));
 
   Finder->addMatcher(
-  continueStmt(hasAncestor(stmt(anyOf(forStmt(), whileStmt(),
-  cxxForRangeStmt(), doStmt()))
-   .bind("closestLoop")),
-   hasAncestor(DoWithFalse))
+  continueStmt(
+  hasAncestor(stmt(anyOf(forStmt(), whileStmt(), cxxForRangeStmt(),
+ doStmt(), switchStmt()))
+  .bind("closestLoop")),
+  hasAncestor(DoWithFalse))
   .bind("continue"),
   this);
 }


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-terminating-continue.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-terminating-continue.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-terminating-continue.cpp
@@ -62,4 +62,12 @@
   if (n>2) continue;
 }
   } while (false);
+
+  do {
+switch (2) {
+case 1:
+case 2:
+  continue;
+}
+  } while (false);
 }
Index: clang-tools-extra/clang-tidy/bugprone/TerminatingContinueCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/TerminatingContinueCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/TerminatingContinueCheck.cpp
@@ -26,10 +26,11 @@
  equalsBoundNode("closestLoop"));
 
   Finder->addMatcher(
-  continueStmt(hasAncestor(stmt(anyOf(forStmt(), whileStmt(),
-  cxxForRangeStmt(), doStmt()))
-   .bind("closestLoop")),
-   hasAncestor(DoWithFalse))
+  continueStmt(
+  hasAncestor(stmt(anyOf(forStmt(), whileStmt(), cxxForRangeStmt(),
+ doStmt(), switchStmt()))
+  .bind("closestLoop")),
+  hasAncestor(DoWithFalse))
   .bind("continue"),
   this);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D96771: [OpenCL] Add distinct file extension for C++ for OpenCL

2021-03-10 Thread Sven van Haastregt via Phabricator via cfe-commits
svenvh accepted this revision.
svenvh added a comment.
This revision is now accepted and ready to land.

LGTM, but maybe give this another 24h before landing in case there are any 
remaining concerns.


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

https://reviews.llvm.org/D96771

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


[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

2021-03-10 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a comment.

In D97058#2615926 , @azabaznov wrote:

> Corrected some mistakes, added a test for diagnosing undeclared identifiers 
> when an extension is unsupported. Generally leaving the change as it is as 
> completely removing pragma may break backward compatibility now: let's do it 
> in a separate patch and notify everyone before doing that.

Ok, addressing in a separate patch is reasonable, but why do you think that we 
will break backward compatibility? My current worry is that the implementation 
is so messy and inconsistent that it will take us longer time if  we do the 
incremental steps. Also, we risk introducing the regressions since it is so 
hard to make sense out of it.

> Also, we should wait before spec will be finalized.

This issue has been reported 2 years ago but there was very little progress 
made since then. So I assume it is not important and I am not convinced it 
would be finalized any time soon. In either case, it would be reasonable for us 
to simplify the implementation. Generally, we don't guarantee backward 
compatibility to non-conformant functionality but however, in this case I don't 
see what would be broken if we remove the redundant diagnostics that were never 
intended and only polluted the source code without providing any benefit to the 
developers. The existing kernels would still compile?

> Note, that adding a test for diagnosing extended atomic types is hard because 
> parser diagnoses a typo (since some types, such as atomic_int, are already 
> present), so diagnostic messages look awkward.






Comment at: clang/lib/Sema/Sema.cpp:339
+  setOpenCLExtensionForType(AtomicDoubleT, "cl_khr_fp64");
+  Atomic64BitTypes.push_back(AtomicDoubleT);
+}

Side comment: I don't see why would `atomic_double` have anything to do with 
`cl_khr_int64_base_atomics` or `cl_khr_int64_extended_atomics`? If anything the 
extensions should have been named differently...



Comment at: clang/lib/Sema/Sema.cpp:376
+addImplicitTypedef(#ExtType, Context.Id##Ty);  
\
+setOpenCLExtensionForType(Context.Id##Ty, #Ext);   
\
+  }

Anastasia wrote:
> Since we are guarding by the feature/extension support we could even remove 
> the need for pragma in order to use the types?
Ok, image types are reserved so we still need the diagnostic although at some 
point we should just implement them as all other reserved identifiers i.e. 
using keywords. Then we would not need any special handling in OpenCL at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97058

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


[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

2021-03-10 Thread 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! Although some further improvements seem to be necessary they can be done 
separately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97058

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


[clang] 25951c5 - [AArch64] Add missing intrinsics for scalar FP rounding

2021-03-10 Thread Jingu Kang via cfe-commits

Author: Jingu Kang
Date: 2021-03-10T13:22:29Z
New Revision: 25951c5ab8e9d8e4040de163f06c444c73314551

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

LOG: [AArch64] Add missing intrinsics for scalar FP rounding

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

Added: 
clang/test/CodeGen/aarch64-v8.5a-scalar-frint3264-intrinsic.c
llvm/test/CodeGen/AArch64/v8.5a-scalar-frint3264-intrinsic.ll

Modified: 
clang/include/clang/Basic/BuiltinsAArch64.def
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/Headers/arm_acle.h
llvm/include/llvm/IR/IntrinsicsAArch64.td
llvm/lib/Target/AArch64/AArch64InstrInfo.td

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsAArch64.def 
b/clang/include/clang/Basic/BuiltinsAArch64.def
index b35510f8b691..cfd6ff7f410b 100644
--- a/clang/include/clang/Basic/BuiltinsAArch64.def
+++ b/clang/include/clang/Basic/BuiltinsAArch64.def
@@ -99,6 +99,16 @@ BUILTIN(__builtin_arm_tcommit, "v", "n")
 BUILTIN(__builtin_arm_tcancel, "vWUIi", "n")
 BUILTIN(__builtin_arm_ttest, "WUi", "nc")
 
+// Armv8.5-A FP rounding intrinsics
+BUILTIN(__builtin_arm_frint32zf, "ff", "")
+BUILTIN(__builtin_arm_frint32z, "dd", "")
+BUILTIN(__builtin_arm_frint64zf, "ff", "")
+BUILTIN(__builtin_arm_frint64z, "dd", "")
+BUILTIN(__builtin_arm_frint32xf, "ff", "")
+BUILTIN(__builtin_arm_frint32x, "dd", "")
+BUILTIN(__builtin_arm_frint64xf, "ff", "")
+BUILTIN(__builtin_arm_frint64x, "dd", "")
+
 // Armv8.7-A load/store 64-byte intrinsics
 BUILTIN(__builtin_arm_ld64b, "vvC*WUi*", "n")
 BUILTIN(__builtin_arm_st64b, "vv*WUiC*", "n")

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index eb5c430e4df0..d24326162290 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -9149,6 +9149,38 @@ Value *CodeGenFunction::EmitAArch64BuiltinExpr(unsigned 
BuiltinID,
   "cls");
   }
 
+  if (BuiltinID == AArch64::BI__builtin_arm_frint32zf ||
+  BuiltinID == AArch64::BI__builtin_arm_frint32z) {
+llvm::Value *Arg = EmitScalarExpr(E->getArg(0));
+llvm::Type *Ty = Arg->getType();
+return Builder.CreateCall(CGM.getIntrinsic(Intrinsic::aarch64_frint32z, 
Ty),
+  Arg, "frint32z");
+  }
+
+  if (BuiltinID == AArch64::BI__builtin_arm_frint64zf ||
+  BuiltinID == AArch64::BI__builtin_arm_frint64z) {
+llvm::Value *Arg = EmitScalarExpr(E->getArg(0));
+llvm::Type *Ty = Arg->getType();
+return Builder.CreateCall(CGM.getIntrinsic(Intrinsic::aarch64_frint64z, 
Ty),
+  Arg, "frint64z");
+  }
+
+  if (BuiltinID == AArch64::BI__builtin_arm_frint32xf ||
+  BuiltinID == AArch64::BI__builtin_arm_frint32x) {
+llvm::Value *Arg = EmitScalarExpr(E->getArg(0));
+llvm::Type *Ty = Arg->getType();
+return Builder.CreateCall(CGM.getIntrinsic(Intrinsic::aarch64_frint32x, 
Ty),
+  Arg, "frint32x");
+  }
+
+  if (BuiltinID == AArch64::BI__builtin_arm_frint64xf ||
+  BuiltinID == AArch64::BI__builtin_arm_frint64x) {
+llvm::Value *Arg = EmitScalarExpr(E->getArg(0));
+llvm::Type *Ty = Arg->getType();
+return Builder.CreateCall(CGM.getIntrinsic(Intrinsic::aarch64_frint64x, 
Ty),
+  Arg, "frint64x");
+  }
+
   if (BuiltinID == AArch64::BI__builtin_arm_jcvt) {
 assert((getContext().getTypeSize(E->getType()) == 32) &&
"__jcvt of unusual size!");

diff  --git a/clang/lib/Headers/arm_acle.h b/clang/lib/Headers/arm_acle.h
index c156d89c1f84..c34cd6403dbc 100644
--- a/clang/lib/Headers/arm_acle.h
+++ b/clang/lib/Headers/arm_acle.h
@@ -639,6 +639,49 @@ __jcvt(double __a) {
 }
 #endif
 
+/* Armv8.5-A FP rounding intrinsics */
+#if __ARM_64BIT_STATE && defined(__ARM_FEATURE_FRINT)
+static __inline__ float __attribute__((__always_inline__, __nodebug__))
+__frint32zf(float __a) {
+  return __builtin_arm_frint32zf(__a);
+}
+
+static __inline__ double __attribute__((__always_inline__, __nodebug__))
+__frint32z(double __a) {
+  return __builtin_arm_frint32z(__a);
+}
+
+static __inline__ float __attribute__((__always_inline__, __nodebug__))
+__frint64zf(float __a) {
+  return __builtin_arm_frint64zf(__a);
+}
+
+static __inline__ double __attribute__((__always_inline__, __nodebug__))
+__frint64z(double __a) {
+  return __builtin_arm_frint64z(__a);
+}
+
+static __inline__ float __attribute__((__always_inline__, __nodebug__))
+__frint32xf(float __a) {
+  return __builtin_arm_frint32xf(__a);
+}
+
+static __inline__ double __attribute__((__always_inline__, __nodebug__))
+__frint32x(double __a) {
+  return __builtin_arm_frint32x(__a);
+}
+
+static __inline__ float __attribute__((__always_inline__, __nodebug__))
+__frint64xf(float __a) {
+  return __builtin_arm_f

[PATCH] D98269: [AArch64] Add missing intrinsics for scalar fp rounding

2021-03-10 Thread JinGu Kang via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG25951c5ab8e9: [AArch64] Add missing intrinsics for scalar FP 
rounding (authored by jaykang10).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98269

Files:
  clang/include/clang/Basic/BuiltinsAArch64.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/arm_acle.h
  clang/test/CodeGen/aarch64-v8.5a-scalar-frint3264-intrinsic.c
  llvm/include/llvm/IR/IntrinsicsAArch64.td
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/test/CodeGen/AArch64/v8.5a-scalar-frint3264-intrinsic.ll

Index: llvm/test/CodeGen/AArch64/v8.5a-scalar-frint3264-intrinsic.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/v8.5a-scalar-frint3264-intrinsic.ll
@@ -0,0 +1,83 @@
+; RUN: llc < %s -mtriple=aarch64-eabi -mattr=+v8.5a  | FileCheck %s
+
+declare float @llvm.aarch64.frint32z.f32(float)
+declare double @llvm.aarch64.frint32z.f64(double)
+declare float @llvm.aarch64.frint64z.f32(float)
+declare double @llvm.aarch64.frint64z.f64(double)
+
+define dso_local float @t_frint32z(float %a) {
+; CHECK-LABEL: t_frint32z:
+; CHECK: frint32z s0, s0
+; CHECK-NEXT:ret
+entry:
+  %val = tail call float @llvm.aarch64.frint32z.f32(float %a)
+  ret float %val
+}
+
+define dso_local double @t_frint32zf(double %a) {
+; CHECK-LABEL: t_frint32zf:
+; CHECK: frint32z d0, d0
+; CHECK-NEXT:ret
+entry:
+  %val = tail call double @llvm.aarch64.frint32z.f64(double %a)
+  ret double %val
+}
+
+define dso_local float @t_frint64z(float %a) {
+; CHECK-LABEL: t_frint64z:
+; CHECK: frint64z s0, s0
+; CHECK-NEXT:ret
+entry:
+  %val = tail call float @llvm.aarch64.frint64z.f32(float %a)
+  ret float %val
+}
+
+define dso_local double @t_frint64zf(double %a) {
+; CHECK-LABEL: t_frint64zf:
+; CHECK: frint64z d0, d0
+; CHECK-NEXT:ret
+entry:
+  %val = tail call double @llvm.aarch64.frint64z.f64(double %a)
+  ret double %val
+}
+
+declare float @llvm.aarch64.frint32x.f32(float)
+declare double @llvm.aarch64.frint32x.f64(double)
+declare float @llvm.aarch64.frint64x.f32(float)
+declare double @llvm.aarch64.frint64x.f64(double)
+
+define dso_local float @t_frint32x(float %a) {
+; CHECK-LABEL: t_frint32x:
+; CHECK: frint32x s0, s0
+; CHECK-NEXT:ret
+entry:
+  %val = tail call float @llvm.aarch64.frint32x.f32(float %a)
+  ret float %val
+}
+
+define dso_local double @t_frint32xf(double %a) {
+; CHECK-LABEL: t_frint32xf:
+; CHECK: frint32x d0, d0
+; CHECK-NEXT:ret
+entry:
+  %val = tail call double @llvm.aarch64.frint32x.f64(double %a)
+  ret double %val
+}
+
+define dso_local float @t_frint64x(float %a) {
+; CHECK-LABEL: t_frint64x:
+; CHECK: frint64x s0, s0
+; CHECK-NEXT:ret
+entry:
+  %val = tail call float @llvm.aarch64.frint64x.f32(float %a)
+  ret float %val
+}
+
+define dso_local double @t_frint64xf(double %a) {
+; CHECK-LABEL: t_frint64xf:
+; CHECK: frint64x d0, d0
+; CHECK-NEXT:ret
+entry:
+  %val = tail call double @llvm.aarch64.frint64x.f64(double %a)
+  ret double %val
+}
Index: llvm/lib/Target/AArch64/AArch64InstrInfo.td
===
--- llvm/lib/Target/AArch64/AArch64InstrInfo.td
+++ llvm/lib/Target/AArch64/AArch64InstrInfo.td
@@ -3805,10 +3805,10 @@
 }
 
 let Predicates = [HasFRInt3264] in {
-  defm FRINT32Z : FRIntNNT<0b00, "frint32z">;
-  defm FRINT64Z : FRIntNNT<0b10, "frint64z">;
-  defm FRINT32X : FRIntNNT<0b01, "frint32x">;
-  defm FRINT64X : FRIntNNT<0b11, "frint64x">;
+  defm FRINT32Z : FRIntNNT<0b00, "frint32z", int_aarch64_frint32z>;
+  defm FRINT64Z : FRIntNNT<0b10, "frint64z", int_aarch64_frint64z>;
+  defm FRINT32X : FRIntNNT<0b01, "frint32x", int_aarch64_frint32x>;
+  defm FRINT64X : FRIntNNT<0b11, "frint64x", int_aarch64_frint64x>;
 } // HasFRInt3264
 
 let Predicates = [HasFullFP16] in {
Index: llvm/include/llvm/IR/IntrinsicsAArch64.td
===
--- llvm/include/llvm/IR/IntrinsicsAArch64.td
+++ llvm/include/llvm/IR/IntrinsicsAArch64.td
@@ -44,6 +44,19 @@
 def int_aarch64_cls: DefaultAttrsIntrinsic<[llvm_i32_ty], [llvm_i32_ty], [IntrNoMem]>;
 def int_aarch64_cls64: DefaultAttrsIntrinsic<[llvm_i32_ty], [llvm_i64_ty], [IntrNoMem]>;
 
+def int_aarch64_frint32z
+: DefaultAttrsIntrinsic<[ llvm_anyfloat_ty ], [ LLVMMatchType<0> ],
+[ IntrNoMem ]>;
+def int_aarch64_frint64z
+: DefaultAttrsIntrinsic<[ llvm_anyfloat_ty ], [ LLVMMatchType<0> ],
+[ IntrNoMem ]>;
+def int_aarch64_frint32x
+: DefaultAttrsIntrinsic<[ llvm_anyfloat_ty ], [ LLVMMatchType<0> ],
+[ IntrNoMem ]>;
+def int_aarch64_f

[clang-tools-extra] 7044f1d - [clangd] Use Dirty Filesystem for cross file rename.

2021-03-10 Thread Nathan James via cfe-commits

Author: Nathan James
Date: 2021-03-10T13:41:29Z
New Revision: 7044f1d875e37a5badd4e59ee84b56faf7432f68

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

LOG: [clangd] Use Dirty Filesystem for cross file rename.

Refactor cross file rename to use a Filesystem instead of a function for 
getting buffer contents of open files.

Depends on D94554

Reviewed By: sammccall

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

Added: 


Modified: 
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/TUScheduler.h
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/refactor/Rename.h
clang-tools-extra/clangd/unittests/RenameTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index 9a5f4bce2e21..df3dd363800a 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -490,9 +490,10 @@ void ClangdServer::prepareRename(PathRef File, Position 
Pos,
   return CB(InpAST.takeError());
 // prepareRename is latency-sensitive: we don't query the index, as we
 // only need main-file references
-auto Results = clangd::rename(
-{Pos, NewName.getValueOr("__clangd_rename_dummy"), InpAST->AST, File,
- /*Index=*/nullptr, RenameOpts});
+auto Results =
+clangd::rename({Pos, NewName.getValueOr("__clangd_rename_dummy"),
+InpAST->AST, File, /*FS=*/nullptr,
+/*Index=*/nullptr, RenameOpts});
 if (!Results) {
   // LSP says to return null on failure, but that will result in a generic
   // failure message. If we send an LSP error response, clients can surface
@@ -507,25 +508,16 @@ void ClangdServer::prepareRename(PathRef File, Position 
Pos,
 void ClangdServer::rename(PathRef File, Position Pos, llvm::StringRef NewName,
   const RenameOptions &Opts,
   Callback CB) {
-  // A snapshot of all file dirty buffers.
-  llvm::StringMap Snapshot = WorkScheduler->getAllFileContents();
   auto Action = [File = File.str(), NewName = NewName.str(), Pos, Opts,
- CB = std::move(CB), Snapshot = std::move(Snapshot),
+ CB = std::move(CB),
  this](llvm::Expected InpAST) mutable {
 // Tracks number of files edited per invocation.
 static constexpr trace::Metric RenameFiles("rename_files",
trace::Metric::Distribution);
 if (!InpAST)
   return CB(InpAST.takeError());
-auto GetDirtyBuffer =
-[&Snapshot](PathRef AbsPath) -> llvm::Optional {
-  auto It = Snapshot.find(AbsPath);
-  if (It == Snapshot.end())
-return llvm::None;
-  return It->second;
-};
-auto R = clangd::rename(
-{Pos, NewName, InpAST->AST, File, Index, Opts, GetDirtyBuffer});
+auto R = clangd::rename({Pos, NewName, InpAST->AST, File,
+ DirtyFS->view(llvm::None), Index, Opts});
 if (!R)
   return CB(R.takeError());
 

diff  --git a/clang-tools-extra/clangd/ClangdServer.h 
b/clang-tools-extra/clangd/ClangdServer.h
index 152aa5a49e8b..e76ef65922ee 100644
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/clang-tools-extra/clangd/ClangdServer.h
@@ -394,7 +394,6 @@ class ClangdServer {
 
   // Store of the current versions of the open documents.
   // Only written from the main thread (despite being threadsafe).
-  // FIXME: TUScheduler also keeps these, unify?
   DraftStore DraftMgr;
 
   std::unique_ptr DirtyFS;

diff  --git a/clang-tools-extra/clangd/TUScheduler.cpp 
b/clang-tools-extra/clangd/TUScheduler.cpp
index 306f690aac63..435deb6ec162 100644
--- a/clang-tools-extra/clangd/TUScheduler.cpp
+++ b/clang-tools-extra/clangd/TUScheduler.cpp
@@ -1519,13 +1519,6 @@ void TUScheduler::remove(PathRef File) {
   // preamble of several open headers.
 }
 
-llvm::StringMap TUScheduler::getAllFileContents() const {
-  llvm::StringMap Results;
-  for (auto &It : Files)
-Results.try_emplace(It.getKey(), It.getValue()->Contents);
-  return Results;
-}
-
 void TUScheduler::run(llvm::StringRef Name, llvm::StringRef Path,
   llvm::unique_function Action) {
   runWithSemaphore(Name, Path, std::move(Action), Barrier);

diff  --git a/clang-tools-extra/clangd/TUScheduler.h 
b/clang-tools-extra/clangd/TUScheduler.h
index 8749915aa757..8df2c019053e 100644
--- a/clang-tools-extra/clangd/TUScheduler.h
+++ b/clang-tools-extra/clangd/TUScheduler.h
@@ -235,9 +235,6 @@ class TUScheduler {
   /// if requested with WantDiags::Auto or WantDiags::Yes.
   void remove(PathRef File)

[PATCH] D95043: [clangd] Use Dirty Filesystem for cross file rename.

2021-03-10 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7044f1d875e3: [clangd] Use Dirty Filesystem for cross file 
rename. (authored by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95043

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/refactor/Rename.h
  clang-tools-extra/clangd/unittests/RenameTests.cpp

Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -33,6 +33,19 @@
 using testing::UnorderedElementsAre;
 using testing::UnorderedElementsAreArray;
 
+llvm::IntrusiveRefCntPtr
+createOverlay(llvm::IntrusiveRefCntPtr Base,
+  llvm::IntrusiveRefCntPtr Overlay) {
+  auto OFS =
+  llvm::makeIntrusiveRefCnt(std::move(Base));
+  OFS->pushOverlay(std::move(Overlay));
+  return OFS;
+}
+
+llvm::IntrusiveRefCntPtr getVFSFromAST(ParsedAST &AST) {
+  return &AST.getSourceManager().getFileManager().getVirtualFileSystem();
+}
+
 // Convert a Range to a Ref.
 Ref refWithRange(const clangd::Range &Range, const std::string &URI) {
   Ref Result;
@@ -815,7 +828,8 @@
 auto Index = TU.index();
 for (const auto &RenamePos : Code.points()) {
   auto RenameResult =
-  rename({RenamePos, NewName, AST, testPath(TU.Filename), Index.get()});
+  rename({RenamePos, NewName, AST, testPath(TU.Filename),
+  getVFSFromAST(AST), Index.get()});
   ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError();
   ASSERT_EQ(1u, RenameResult->GlobalChanges.size());
   EXPECT_EQ(
@@ -1101,13 +1115,21 @@
   auto AST = TU.build();
 
   auto Main = testPath("main.cc");
+  auto InMemFS = llvm::makeIntrusiveRefCnt();
+  InMemFS->addFile(testPath("main.cc"), 0,
+   llvm::MemoryBuffer::getMemBuffer(Code.code()));
+  InMemFS->addFile(testPath("other.cc"), 0,
+   llvm::MemoryBuffer::getMemBuffer(Code.code()));
 
   auto Rename = [&](const SymbolIndex *Idx) {
-auto GetDirtyBuffer = [&](PathRef Path) -> llvm::Optional {
-  return Code.code().str(); // Every file has the same content.
-};
-RenameInputs Inputs{Code.point(), "xPrime",AST,   Main,
-Idx,  RenameOptions(), GetDirtyBuffer};
+RenameInputs Inputs{Code.point(),
+"xPrime",
+AST,
+Main,
+Idx ? createOverlay(getVFSFromAST(AST), InMemFS)
+: nullptr,
+Idx,
+RenameOptions()};
 auto Results = rename(Inputs);
 EXPECT_TRUE(bool(Results)) << llvm::toString(Results.takeError());
 return std::move(*Results);
@@ -1237,25 +1259,19 @@
 
   Annotations MainCode("class  [[Fo^o]] {};");
   auto MainFilePath = testPath("main.cc");
-  // Dirty buffer for foo.cc.
-  auto GetDirtyBuffer = [&](PathRef Path) -> llvm::Optional {
-if (Path == FooPath)
-  return FooDirtyBuffer.code().str();
-return llvm::None;
-  };
+  llvm::IntrusiveRefCntPtr InMemFS =
+  new llvm::vfs::InMemoryFileSystem;
+  InMemFS->addFile(FooPath, 0,
+   llvm::MemoryBuffer::getMemBuffer(FooDirtyBuffer.code()));
 
   // Run rename on Foo, there is a dirty buffer for foo.cc, rename should
   // respect the dirty buffer.
   TestTU TU = TestTU::withCode(MainCode.code());
   auto AST = TU.build();
   llvm::StringRef NewName = "newName";
-  auto Results = rename({MainCode.point(),
- NewName,
- AST,
- MainFilePath,
- Index.get(),
- {},
- GetDirtyBuffer});
+  auto Results =
+  rename({MainCode.point(), NewName, AST, MainFilePath,
+  createOverlay(getVFSFromAST(AST), InMemFS), Index.get()});
   ASSERT_TRUE(bool(Results)) << Results.takeError();
   EXPECT_THAT(
   applyEdits(std::move(Results->GlobalChanges)),
@@ -1270,13 +1286,8 @@
   // Set a file "bar.cc" on disk.
   TU.AdditionalFiles["bar.cc"] = std::string(BarCode.code());
   AST = TU.build();
-  Results = rename({MainCode.point(),
-NewName,
-AST,
-MainFilePath,
-Index.get(),
-{},
-GetDirtyBuffer});
+  Results = rename({MainCode.point(), NewName, AST, MainFilePath,
+createOverlay(getVFSFromAST(AST), InMemFS), Index.get()});
   ASSERT_TRUE(bool(Results)) << Results.takeError();
   EXPECT_THAT(
   applyEdits(

[PATCH] D98193: [CUDA][HIP] Allow non-ODR use of host var in device

2021-03-10 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl marked an inline comment as done.
yaxunl added inline comments.



Comment at: clang/test/SemaCUDA/device-use-host-var.cu:41
   *out = global_const_var;
+  *out = global_const_struct_var.x;
 

tra wrote:
> I do not think it should be allowed. We end up instantiating the variable on 
> device, even though the variable should be host-only.
> 
> Right now we allow it, but end up with an `.extern .const` which will make 
> ptxas fail:
> https://godbolt.org/z/sx9845
> 
> If we do allow it, we'll need to make sure that we only use the value, but do 
> not allow instantiating the variable.
> 
will only not allow this since it results in ODR-use of the host var.


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

https://reviews.llvm.org/D98193

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


[PATCH] D97874: [analyzer] Improve SVal cast from integer to bool using known RangeSet

2021-03-10 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment.

I would suggest **not** merging this patch.

Circumventing the `assume` machinery could cause potential crashes.
By tracking equivalence classes and whatnot, our solver is becoming more and 
more capable.

Doing a bifurcation, then realizing that the current path is infeasible had 
caused some trouble in the past (D88019 ), and
it still can cause crashes RangeConstraintManager infeasible execution path due 
to EquivalenceClasses .
Reproducing and debugging these is a real headache. And I think there are more 
bugs like these lurking inside.

If we want to have a chance to tackle all of these for once and all, we need a 
common entrypoint for querying assumptions.
Thus, I would suggest not providing an extra API.

@NoQ @vsavchenko What's your take on this?

You could probably put these changes into the assume handler, but that would 
need **really** careful evaluation.
If you think it's worth the effort, consider taking a few measurements on 
several projects.


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

https://reviews.llvm.org/D97874

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


[PATCH] D98193: [CUDA][HIP] Allow non-ODR use of host var in device

2021-03-10 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 329632.
yaxunl marked an inline comment as done.
yaxunl added a comment.

Follow C++ about ODR-use of variables.


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

https://reviews.llvm.org/D98193

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CodeGenCUDA/device-use-host-var.cu
  clang/test/SemaCUDA/device-use-host-var.cu

Index: clang/test/SemaCUDA/device-use-host-var.cu
===
--- clang/test/SemaCUDA/device-use-host-var.cu
+++ clang/test/SemaCUDA/device-use-host-var.cu
@@ -5,37 +5,96 @@
 
 #include "Inputs/cuda.h"
 
-int global_host_var;
+struct A {
+  int x;
+  static int host_var;
+};
+
+int A::host_var;
+
+namespace X {
+  int host_var;
+}
+
+static int static_host_var;
+
 __device__ int global_dev_var;
 __constant__ int global_constant_var;
 __shared__ int global_shared_var;
-constexpr int global_constexpr_var = 1;
+
+int global_host_var;
 const int global_const_var = 1;
+constexpr int global_constexpr_var = 1;
+
+int global_host_array[2] = {1, 2};
+const int global_const_array[2] = {1, 2};
+constexpr int global_constexpr_array[2] = {1, 2};
+
+A global_host_struct_var{1};
+const A global_const_struct_var{1};
+constexpr A global_constexpr_struct_var{1};
 
 template
 __global__ void kernel(F f) { f(); } // dev-note2 {{called by 'kernel<(lambda}}
 
 __device__ void dev_fun(int *out) {
-  int &ref_host_var = global_host_var; // dev-error {{reference to __host__ variable 'global_host_var' in __device__ function}}
+  // Check access device variables are allowed.
   int &ref_dev_var = global_dev_var;
   int &ref_constant_var = global_constant_var;
   int &ref_shared_var = global_shared_var;
-  const int &ref_constexpr_var = global_constexpr_var;
-  const int &ref_const_var = global_const_var;
-
-  *out = global_host_var; // dev-error {{reference to __host__ variable 'global_host_var' in __device__ function}}
+  *out = ref_dev_var;
+  *out = ref_constant_var;
+  *out = ref_shared_var;
   *out = global_dev_var;
   *out = global_constant_var;
   *out = global_shared_var;
-  *out = global_constexpr_var;
+
+  // Check access of non-const host variables are not allowed.
+  *out = global_host_var; // dev-error {{reference to __host__ variable 'global_host_var' in __device__ function}}
   *out = global_const_var;
+  *out = global_constexpr_var;
+  global_host_var = 1; // dev-error {{reference to __host__ variable 'global_host_var' in __device__ function}}
 
+  // Check reference of non-constexpr host variables are not allowed.
+  int &ref_host_var = global_host_var; // dev-error {{reference to __host__ variable 'global_host_var' in __device__ function}}
+  const int &ref_const_var = global_const_var; // dev-error {{reference to __host__ variable 'global_const_var' in __device__ function}}
+  const int &ref_constexpr_var = global_constexpr_var;
   *out = ref_host_var;
-  *out = ref_dev_var;
-  *out = ref_constant_var;
-  *out = ref_shared_var;
   *out = ref_constexpr_var;
   *out = ref_const_var;
+
+  // Check access member of non-constexpr struct type host variable is not allowed.
+  *out = global_host_struct_var.x; // dev-error {{reference to __host__ variable 'global_host_struct_var' in __device__ function}}
+  *out = global_const_struct_var.x; // dev-error {{reference to __host__ variable 'global_const_struct_var' in __device__ function}}
+  *out = global_constexpr_struct_var.x;
+  global_host_struct_var.x = 1; // dev-error {{reference to __host__ variable 'global_host_struct_var' in __device__ function}}
+
+  // Check address taking of non-constexpr host variables is not allowed.
+  int *p = &global_host_var; // dev-error {{reference to __host__ variable 'global_host_var' in __device__ function}}
+  const int *cp = &global_const_var; // dev-error {{reference to __host__ variable 'global_const_var' in __device__ function}}
+  const int *cp2 = &global_constexpr_var;
+
+  // Check access elements of non-constexpr host array is not allowed.
+  *out = global_host_array[1]; // dev-error {{reference to __host__ variable 'global_host_array' in __device__ function}}
+  *out = global_const_array[1]; // dev-error {{reference to __host__ variable 'global_const_array' in __device__ function}}
+  *out = global_constexpr_array[1];
+
+  // Check ODR-use of host variables in namespace is not allowed.
+  *out = X::host_var; // dev-error {{reference to __host__ variable 'host_var' in __device__ function}}
+
+  // Check ODR-use of static host varables in class or file scope is not allowed.
+  *out = A::host_var; // dev-error {{reference to __host__ variable 'host_var' in __device__ function}}
+  *out = static_host_var; // dev-error {{reference to __host__ variable 'static_host_var' in __device__ function}}
+
+  // Check function-scope static variable is allowed.
+  static int static_var;
+  *out = static_var;
+
+  // Check non-ODR use of host varirables are allowed.
+  *out = sizeof(global_host_var);
+  *out = size

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

2021-03-10 Thread Anton Zabaznov via Phabricator via cfe-commits
azabaznov added a comment.

> Ok, addressing in a separate patch is reasonable, but why do you think that 
> we will break backward compatibility? My current worry is that the 
> implementation is so messy and inconsistent that it will take us longer time 
> if we do the incremental steps. Also, we risk introducing the regressions 
> since it is so hard to make sense out of it.

As regarding backward compatibility -- they may be kernels which already rely 
on pragmas. So maybe their users already expect to see something like: 
//some_type requires some_ext to be enabled//. If we remove check for pragma 
their will be no diagnostic at all

> This issue has been reported 2 years ago but there was very little progress 
> made since then.

I see :( Maybe we should introduce diagnostic, something like: //pragma enable 
is deprecated: there is no need to enable extension to use optional 
functionally//. But anyway it seems really impactful as current specification 
allows using pragma...




Comment at: clang/lib/Sema/Sema.cpp:339
+  setOpenCLExtensionForType(AtomicDoubleT, "cl_khr_fp64");
+  Atomic64BitTypes.push_back(AtomicDoubleT);
+}

Anastasia wrote:
> Side comment: I don't see why would `atomic_double` have anything to do with 
> `cl_khr_int64_base_atomics` or `cl_khr_int64_extended_atomics`? If anything 
> the extensions should have been named differently...
Yeah... With the fact given that `atomic_int` for example by default. 
Intuitively I expected to see only `cl_khr_fp64` required.

OpenCL C 3.0 spec, 6.15.12.6. Atomic integer and floating-point types:
...
atomic_double [54]
...
[54]:  //The atomic_double type is only supported if double precision is 
supported and the cl_khr_int64_base_atomics and cl_khr_int64_extended_atomics 
extensions are supported and have been enabled. If this is the case then an 
OpenCL C 3.0 compiler must also define the __opencl_c_fp64 feature//.

Does it seem like spec issue/inaccuracy?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97058

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


[PATCH] D98237: [clang-format] Option for empty lines after an access modifier.

2021-03-10 Thread Max Sagebaum via Phabricator via cfe-commits
Max_S added a comment.

In D98237#2616193 , @MyDeveloperDay 
wrote:

> Just out of interest and we are supposed to ask for this but can you point to 
> public style guide that uses this style.   (actually I don't mind if other 
> formatting tools have this capability and you highlight it, like astyle or 
> editorConfig etc)
>
> From my perspective this seems like a reasonable suggestion. (even if I'm 
> unlikely to use it myself ;-))

I can not specify an other formater or a style guide that uses this style. The 
problem is that clang-format is rather destructive at this point. Other 
formatters like astyle or the one from CLion keep empty lines after an access 
modifier.  The behavior of clang-format before this patch was to remove all 
empty lines after an access modifier. The option `MaxEmptyLinesToKeep` is 
currently ignored at this point.

After thinking a little bit about this:
If the removal of all lines after an access modifier is rather a bug than a 
feature, then the change:

 if (PreviousLine && PreviousLine->First->isAccessSpecifier() &&
 (!PreviousLine->InPPDirective || !RootToken.HasUnescapedNewline))
  -Newlines = std::min(1u, Newlines);
  +Newlines = std::max(1u, Newlines);
  
 if (Newlines)

would solve this problem.




Comment at: clang/unittests/Format/FormatTest.cpp:9205
+   "};";
+  EXPECT_EQ(test1NL, format(test0NL));
+  verifyFormat(test1NL);

MyDeveloperDay wrote:
> why can't you just verifyFormat them all?
Yes. I will change this in the next update.



Comment at: clang/unittests/Format/FormatTest.cpp:9212
+  StyleWithLine.EmptyLinesAfterAccessModifier = 1u;
+  EXPECT_EQ(test2NL, format(test0NL, StyleWithLine));
+  EXPECT_EQ(test2NL, format(test1NL, StyleWithLine));

MyDeveloperDay wrote:
> yeah I'm not a fan of this like this... sorry... just write the test out in 
> long form, when it goes wrong I don't have to be a compiler to understand 
> what is going wrong I can just see it.
I can change this, but the current output of the tests is (I forced the error):
```
//llvm-project/clang/unittests/Format/FormatTest.cpp:72: Failure
  Expected: Expected.str()
  Which is: "class Foo {\nprivate:\n\n  int i;\n};"
To be equal to: format(Expected, Style)
  Which is: "class Foo {\nprivate:\n  int i;\n};"
With diff:
@@ -1,5 @@
 class Foo {
 private:
-
   int i;
 };
```

Which is actually human readable in this case. Shall I still change it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98237

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


[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

2021-03-10 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment.

I'm looking forward to this patch.




Comment at: clang/test/Analysis/string.c:367-373
+void *func_strcpy_no_assertion();
+char ***ptr_strcpy_no_assertion;
+void strcpy_no_assertion() {
+  *(unsigned char **)ptr_strcpy_no_assertion = (unsigned char 
*)(func_strcpy_no_assertion());
+  char c;
+  strcpy(**ptr_strcpy_no_assertion, &c); // no-assertion
+}

steakhal wrote:
> I would prefer slightly more readbale names.
> `func_strcpy_no_assertion()` -> `get_void_ptr()`
> `ptr_strcpy_no_assertion ` -> `type_punned_ptr`
> `no-assertion` -> `no-crash`
> You could also hoist the `char a` as a function parameter to spare a line :)
I think this is 'done'. :)


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

https://reviews.llvm.org/D89055

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


[PATCH] D98341: [analyzer][solver] Prevent infeasible states (PR49490)

2021-03-10 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko created this revision.
vsavchenko added reviewers: steakhal, NoQ, xazax.hun, ASDenysPetrov.
Herald added subscribers: martong, Charusso, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware.
vsavchenko requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch fixes the situation when our knowledge of disequalities
can help us figuring out that some assumption is infeasible, but
the solver still produces a state with inconsistent constraints.

Additionally, this patch adds a couple of assertions to catch this
type of problems easier.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98341

Files:
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/test/Analysis/PR49490.cpp


Index: clang/test/Analysis/PR49490.cpp
===
--- /dev/null
+++ clang/test/Analysis/PR49490.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,alpha.security.ArrayBound 
 -verify %s
+
+struct toggle {
+  bool value;
+};
+
+toggle global_toggle;
+toggle get_global_toggle() { return global_toggle; }
+
+int *ints;
+int oob_access() { return ints[-1]; } // expected-warning{{out-of-bound}}
+
+bool compare(toggle one, bool other) {
+  if (one.value != other)
+return true;
+
+  if (one.value)
+oob_access();
+  return true;
+}
+
+bool coin();
+
+void bar() {
+  bool left = coin();
+  bool right = coin();
+  for (;;)
+compare(get_global_toggle(), left) && compare(get_global_toggle(), right);
+}
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -19,6 +19,8 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/SValVisitor.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/ImmutableSet.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Compiler.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace clang;
@@ -1395,12 +1397,22 @@
 return EquivalenceClass::merge(getBasicVals(), F, State, LHS, RHS);
   }
 
+  LLVM_ATTRIBUTE_UNUSED inline bool areFeasible(ConstraintRangeTy Constraints) 
{
+return llvm::none_of(
+Constraints,
+[](const std::pair ClassConstraint) {
+  return ClassConstraint.second.isEmpty();
+});
+  }
+
   LLVM_NODISCARD inline ProgramStateRef setConstraint(ProgramStateRef State,
   EquivalenceClass Class,
   RangeSet Constraint) {
 ConstraintRangeTy Constraints = State->get();
 ConstraintRangeTy::Factory &CF = State->get_context();
 
+assert(!Constraint.isEmpty() && "New constraint should not be empty");
+
 // Add new constraint.
 Constraints = CF.add(Constraints, Class, Constraint);
 
@@ -1413,9 +1425,18 @@
   for (EquivalenceClass DisequalClass : Class.getDisequalClasses(State)) {
 RangeSet UpdatedConstraint =
 getRange(State, DisequalClass).Delete(getBasicVals(), F, *Point);
+
+// If we end up with at least one of the disequal classes to be
+// constrainted with an empty range-set, the state is infeasible.
+if (UpdatedConstraint.isEmpty())
+  return nullptr;
+
 Constraints = CF.add(Constraints, DisequalClass, UpdatedConstraint);
   }
 
+assert(areFeasible(Constraints) && "Constraint manager shouldn't produce "
+   "a state with infeasible constraints");
+
 return State->set(Constraints);
   }
 


Index: clang/test/Analysis/PR49490.cpp
===
--- /dev/null
+++ clang/test/Analysis/PR49490.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,alpha.security.ArrayBound  -verify %s
+
+struct toggle {
+  bool value;
+};
+
+toggle global_toggle;
+toggle get_global_toggle() { return global_toggle; }
+
+int *ints;
+int oob_access() { return ints[-1]; } // expected-warning{{out-of-bound}}
+
+bool compare(toggle one, bool other) {
+  if (one.value != other)
+return true;
+
+  if (one.value)
+oob_access();
+  return true;
+}
+
+bool coin();
+
+void bar() {
+  bool left = coin();
+  bool right = coin();
+  for (;;)
+compare(get_global_toggle(), left) && compare(get_global_toggle(), right);
+}
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -19,6 +19,8 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/SValVisitor.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/ImmutableSet.h"
+#include "llvm/ADT/STLExtra

[PATCH] D97874: [analyzer] Improve SVal cast from integer to bool using known RangeSet

2021-03-10 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment.

@steakhal I personally don't see any fundamental problems with this patch




Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:878
+  // Integer is known to be non-zero or zero only.
+  auto truth = State->isNonNull(V);
+  if (truth.isConstrained())

nit: LLVM coding standards call for variable names starting with upper-case 
letters. 


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

https://reviews.llvm.org/D97874

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


[PATCH] D96090: [analyzer] Replace StoreManager::CastRetrievedVal with SValBuilder::evalCast

2021-03-10 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment.

In D96090#2607387 , @ASDenysPetrov 
wrote:

> Actually many cases don't need to know an exact **original** type. Some cases 
> can extract the //type// from `SVal` (`SymbolVal`, `ConcreteInt`) Other cases 
> need it from outside (`MemRegionVal`, `LocAsInteger`).
> `dispatchCast`, `evalCastFromNonLoc`, `evalCastFromLoc` have a lot of similar 
> code which is already in `evalCast`. They also calls inside `evalCast` 
> function. I decided to move their functionality into a new splitted version 
> of `evalCast` to decrease complexity of comprehension and try to substitute 
> them in the future with the single `evalCast`. But substitusion needs to deal 
> something with absent **original** type parameter. Then I decided to add 
> support for`evalCast` to work in both modes. Practically, new `evaCast` has 
> additional checks and casts when the **original** type is not null.

Seems reasonable to me.

> Now `evalCastFromNonLoc` and `evalCastFromLoc` are used in 
> `SimpleSValBuilder` functions only. I think it would be better to add an 
> explaination to the doc like `OriginalTy.isNull() is a CastRetrievedVal 
> backdoor hack. Do not use it.` instead of new flags. I'll make a better 
> description, but at first I'd try to investigate how to pass an **original** 
> type to `evalCastFromNonLoc` and `evalCastFromLoc` in `SimpleSValBuilder` 
> functions. Or maybe some other solution.

What about not defaulting that parameter? That would better represent our 
expectation that it **requires** an original-type.
You could still pass a default constructed QualType at each callsite.


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

https://reviews.llvm.org/D96090

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


[PATCH] D98253: [clang][ARM] Refactor ComputeLLVMTriple code for ARM

2021-03-10 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
ostannard accepted this revision.
ostannard 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/D98253/new/

https://reviews.llvm.org/D98253

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


[PATCH] D95244: [clang][AST] Handle overload callee type in CallExpr::getCallReturnType.

2021-03-10 Thread Balázs Kéri via Phabricator via cfe-commits
balazske updated this revision to Diff 329647.
balazske added a comment.

rebase, split the test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95244

Files:
  clang/lib/AST/Expr.cpp
  clang/unittests/Tooling/SourceCodeTest.cpp

Index: clang/unittests/Tooling/SourceCodeTest.cpp
===
--- clang/unittests/Tooling/SourceCodeTest.cpp
+++ clang/unittests/Tooling/SourceCodeTest.cpp
@@ -621,4 +621,63 @@
   };
   Visitor.runOver(Code);
 }
+
+TEST(SourceCodeTest, GetCallReturnType_Callee_UnresolvedLookupExpr) {
+  llvm::StringRef Code = R"cpp(
+template
+void templ(const T& t, F f) {
+  f(t);
+  // CalleeType in getCallReturntype is Overload and dependent
+}
+int f_overload(int) { return 1; }
+int f_overload(double) { return 2; }
+
+void f() {
+  int i = 0;
+  templ(i, [](const auto &p) {
+f_overload(p); // UnresolvedLookupExpr
+// CalleeType in getCallReturntype is Overload and not dependent
+  });
+}
+)cpp";
+
+  CallsVisitor Visitor;
+  Visitor.OnCall = [](CallExpr *Expr, ASTContext *Context) {
+// Should not crash.
+(void)Expr->getCallReturnType(*Context);
+  };
+  Visitor.runOver(Code, CallsVisitor::Lang_CXX14);
+}
+
+TEST(SourceCodeTest, GetCallReturnType_Callee_UnresolvedMemberExpr) {
+  llvm::StringRef Code = R"cpp(
+template
+void templ(const T& t, F f) {
+  f(t);
+  // CalleeType in getCallReturntype is Overload and dependent
+}
+
+struct A {
+  void f_overload(int);
+  void f_overload(double);
+};
+
+void f() {
+  int i = 0;
+  templ(i, [](const auto &p) {
+A a;
+a.f_overload(p); // UnresolvedMemberExpr
+// CalleeType in getCallReturntype is BoundMember and has overloads
+  });
+}
+)cpp";
+
+  CallsVisitor Visitor;
+  Visitor.OnCall = [](CallExpr *Expr, ASTContext *Context) {
+// Should not crash.
+(void)Expr->getCallReturnType(*Context);
+  };
+  Visitor.runOver(Code, CallsVisitor::Lang_CXX14);
+}
+
 } // end anonymous namespace
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -1383,6 +1383,14 @@
 QualType CallExpr::getCallReturnType(const ASTContext &Ctx) const {
   const Expr *Callee = getCallee();
   QualType CalleeType = Callee->getType();
+
+  auto CreateDependentType = [&Ctx]() {
+ASTContext::GetBuiltinTypeError Error;
+QualType RetTy = Ctx.GetBuiltinType(BuiltinType::Dependent, Error);
+assert(Error == ASTContext::GE_None);
+return RetTy;
+  };
+
   if (const auto *FnTypePtr = CalleeType->getAs()) {
 CalleeType = FnTypePtr->getPointeeType();
   } else if (const auto *BPT = CalleeType->getAs()) {
@@ -1391,8 +1399,13 @@
 if (isa(Callee->IgnoreParens()))
   return Ctx.VoidTy;
 
-// This should never be overloaded and so should never return null.
 CalleeType = Expr::findBoundMemberType(Callee);
+if (CalleeType.isNull())
+  return CreateDependentType();
+
+  } else if (CalleeType->isDependentType() ||
+ CalleeType->isSpecificPlaceholderType(BuiltinType::Overload)) {
+return CreateDependentType();
   }
 
   const FunctionType *FnType = CalleeType->castAs();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D97869: [OpenCL][Draft] Add OpenCL builtin test generator

2021-03-10 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a comment.

Thanks for the comprehensive summary. I don't have many other suggestions in 
mind but I would like to add a few thoughts.

We could always consider writing the tests manually from scratch too. But it 
doesn't feel like a good cost/benefit trade-off.

Regarding 2 and 4 I think we should drive towards deprecation of `opencl-c.h` 
as it is a maintenance overhead but we could convert it into a test instead?

Aside from option 3 all other options will require adding non-standard testing 
formats in Clang. This means that if there is any core refactoring changes even 
outside of OpenCL affecting the functionality in the headers, it would be 
required to learn how we do the testing in order to address any breakages. 
Perhaps this can be mitigated by the documentation, but overall it is an extra 
burden to the community. Speaking from my personal experience I was in the 
situations breaking functionality outside of OpenCL and having a unified 
testing format is a big advantage in understanding and addressing the issues in 
unfamiliar code.

>   The output is big, currently already 56k lines, and I expect it will still 
> get a bit bigger when we add conditionals. I don't feel comfortable about 
> putting such big autogenerated files under git (regardless of whether we add 
> it as a single file, or as a collection of smaller files). Ideally, the repo 
> should only contain the source from which we generate things.

What problems do you see with checking in autogenerated files? There are some 
existing tests that check similar functionality and they seem to contain 
repetitive patterns. Honestly, I wouldn't be able to say if they were written 
with copy/paste or auto-generated but I am not sure whether it is very 
important. Of course, it would have been better if we have built up the 
functionality incrementally, and it is still an option. But it hasn't been 
possible in the past and as a matter of fact the original header was committed 
as one large file (around 15K lines).

>   Updates to OpenCLBuiltins.td will give large diffs when regenerating the 
> tests (regardless of whether the file is split or not).

Could you provide more details. I don't follow this.

>   Manually verifying that the generated test is actually correct is not 
> trivial.

I think the way to approach this would be to just assume that the test is 
correct and then fix any issues we discover as we go along with the 
development. There is nothing wrong with this though as even small manually 
written tests have bugs.


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

https://reviews.llvm.org/D97869

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


[PATCH] D98076: [OpenCL][Docs] Release 12.0 notes

2021-03-10 Thread Sven van Haastregt via Phabricator via cfe-commits
svenvh added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:214
+- Added ``global_device`` and ``global_host`` address spaces for USM
+  allocations.
+

Anastasia wrote:
> svenvh wrote:
> > Perhaps one more point to mention:
> > ```
> >  - Allow pointer-to-pointer kernel arguments beyond OpenCL 1.2.
> > ```
> > (related commit is 523775f96742 ("[OpenCL] Allow pointer-to-pointer kernel 
> > args beyond CL 1.2", 2020-12-01)).
> I have covered it in this but it is combined with another diagnostic:
> 
> 
> ```
> - Improved diagnostics for nested pointers and unevaluated ``vec_step`` 
> expression.
> ```
> 
> But if you think it is better I can change as you have suggested.
Ah, that's probably why I missed it, as I was assuming "improved diagnostics" 
means a nicer error message, not that Clang now accepts something that it 
rejected before.  So in my opinion it deserves a point on its own.


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

https://reviews.llvm.org/D98076

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


[PATCH] D93978: [clangd] Use dirty filesystem when performing cross file tweaks

2021-03-10 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 329649.
njames93 added a comment.

Rebased.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93978

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/refactor/Tweak.cpp
  clang-tools-extra/clangd/refactor/Tweak.h
  clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
  clang-tools-extra/clangd/tool/Check.cpp
  clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp

Index: clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
@@ -74,7 +74,8 @@
   SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), Range.first,
 Range.second, [&](SelectionTree ST) {
   Tweak::Selection S(Index, AST, Range.first,
- Range.second, std::move(ST));
+ Range.second, std::move(ST),
+ nullptr);
   if (auto T = prepareTweak(TweakID, S)) {
 Result = (*T)->apply(S);
 return true;
Index: clang-tools-extra/clangd/tool/Check.cpp
===
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -203,7 +203,8 @@
   vlog("  {0} {1}", Pos, Tok.text(AST->getSourceManager()));
   auto Tree = SelectionTree::createRight(AST->getASTContext(),
  AST->getTokens(), Start, End);
-  Tweak::Selection Selection(&Index, *AST, Start, End, std::move(Tree));
+  Tweak::Selection Selection(&Index, *AST, Start, End, std::move(Tree),
+ nullptr);
   for (const auto &T : prepareTweaks(Selection, Opts.TweakFilter)) {
 auto Result = T->apply(Selection);
 if (!Result) {
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -63,10 +63,9 @@
 }
 
 llvm::Optional getSourceFile(llvm::StringRef FileName,
-   const Tweak::Selection &Sel) {
-  if (auto Source = getCorrespondingHeaderOrSource(
-  FileName,
-  &Sel.AST->getSourceManager().getFileManager().getVirtualFileSystem()))
+   const Tweak::Selection &Sel,
+   llvm::vfs::FileSystem *FS) {
+  if (auto Source = getCorrespondingHeaderOrSource(FileName, FS))
 return *Source;
   return getCorrespondingHeaderOrSource(FileName, *Sel.AST, Sel.Index);
 }
@@ -409,13 +408,14 @@
 if (!MainFileName)
   return error("Couldn't get absolute path for main file.");
 
-auto CCFile = getSourceFile(*MainFileName, Sel);
+auto *FS = Sel.FS;
+assert(FS && "FS Must be set in apply");
+
+auto CCFile = getSourceFile(*MainFileName, Sel, FS);
+
 if (!CCFile)
   return error("Couldn't find a suitable implementation file.");
-
-auto &FS =
-Sel.AST->getSourceManager().getFileManager().getVirtualFileSystem();
-auto Buffer = FS.getBufferForFile(*CCFile);
+auto Buffer = FS->getBufferForFile(*CCFile);
 // FIXME: Maybe we should consider creating the implementation file if it
 // doesn't exist?
 if (!Buffer)
Index: clang-tools-extra/clangd/refactor/Tweak.h
===
--- clang-tools-extra/clangd/refactor/Tweak.h
+++ clang-tools-extra/clangd/refactor/Tweak.h
@@ -48,7 +48,8 @@
   /// Input to prepare and apply tweaks.
   struct Selection {
 Selection(const SymbolIndex *Index, ParsedAST &AST, unsigned RangeBegin,
-  unsigned RangeEnd, SelectionTree ASTSelection);
+  unsigned RangeEnd, SelectionTree ASTSelection,
+  llvm::vfs::FileSystem *VFS);
 /// The text of the active document.
 llvm::StringRef Code;
 /// The Index for handling codebase related queries.
@@ -64,6 +65,11 @@
 unsigned SelectionEnd;
 /// The AST nodes that were selected.
 SelectionTree ASTSelection;
+/// File system used to access source code (for cross-file tweaks).
+/// This can be used to overlay the "dirty" contents of files open in the
+/// editor, which (in case of headers) may not match the saved contents used
+/// for building the AST.
+llvm::vfs::FileSystem *FS = nullptr;
 // FIXME: provide a way to get sources and ASTs for other files.
   };
 
Index: clang-tools-extra/clangd/refactor/Tweak.cpp

[PATCH] D77923: OpenCL: Fix some missing predefined macros

2021-03-10 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a comment.
Herald added a subscriber: dexonsmith.

FYI there has been a related spec change that makes this functionality optional 
in the offline compilation: 
https://github.com/KhronosGroup/OpenCL-Docs/pull/522/files


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

https://reviews.llvm.org/D77923

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


[PATCH] D93978: [clangd] Use dirty filesystem when performing cross file tweaks

2021-03-10 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments.



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:575
   return CB(InpAST.takeError());
-auto Selections = tweakSelection(Sel, *InpAST);
+// FIXME: Should we use the dirty fs here?
+auto FS = TFS.view(llvm::None);

Regarding this, Is it wise to set the contract so that Tweak::prepare isn't 
allowed to do IO so we should just pass a nullptr here, inline with how 
prepareRename works.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93978

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


[PATCH] D97874: [analyzer] Improve SVal cast from integer to bool using known RangeSet

2021-03-10 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment.

In D97874#2616660 , @vsavchenko wrote:

> @steakhal I personally don't see any fundamental problems with this patch

Then, I guess, it should be fine :)


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

https://reviews.llvm.org/D97874

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


[PATCH] D98076: [OpenCL][Docs] Release 12.0 notes

2021-03-10 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:214
+- Added ``global_device`` and ``global_host`` address spaces for USM
+  allocations.
+

svenvh wrote:
> Anastasia wrote:
> > svenvh wrote:
> > > Perhaps one more point to mention:
> > > ```
> > >  - Allow pointer-to-pointer kernel arguments beyond OpenCL 1.2.
> > > ```
> > > (related commit is 523775f96742 ("[OpenCL] Allow pointer-to-pointer 
> > > kernel args beyond CL 1.2", 2020-12-01)).
> > I have covered it in this but it is combined with another diagnostic:
> > 
> > 
> > ```
> > - Improved diagnostics for nested pointers and unevaluated ``vec_step`` 
> > expression.
> > ```
> > 
> > But if you think it is better I can change as you have suggested.
> Ah, that's probably why I missed it, as I was assuming "improved diagnostics" 
> means a nicer error message, not that Clang now accepts something that it 
> rejected before.  So in my opinion it deserves a point on its own.
Ok, do you think we should use `nested pointers` instead of 
`pointer-to-pointer` because your change seems to work with any number of 
pointers. Although I appreciate that the latter one is a spec terminology.


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

https://reviews.llvm.org/D98076

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


[PATCH] D98076: [OpenCL][Docs] Release 12.0 notes

2021-03-10 Thread Sven van Haastregt via Phabricator via cfe-commits
svenvh added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:214
+- Added ``global_device`` and ``global_host`` address spaces for USM
+  allocations.
+

Anastasia wrote:
> svenvh wrote:
> > Anastasia wrote:
> > > svenvh wrote:
> > > > Perhaps one more point to mention:
> > > > ```
> > > >  - Allow pointer-to-pointer kernel arguments beyond OpenCL 1.2.
> > > > ```
> > > > (related commit is 523775f96742 ("[OpenCL] Allow pointer-to-pointer 
> > > > kernel args beyond CL 1.2", 2020-12-01)).
> > > I have covered it in this but it is combined with another diagnostic:
> > > 
> > > 
> > > ```
> > > - Improved diagnostics for nested pointers and unevaluated ``vec_step`` 
> > > expression.
> > > ```
> > > 
> > > But if you think it is better I can change as you have suggested.
> > Ah, that's probably why I missed it, as I was assuming "improved 
> > diagnostics" means a nicer error message, not that Clang now accepts 
> > something that it rejected before.  So in my opinion it deserves a point on 
> > its own.
> Ok, do you think we should use `nested pointers` instead of 
> `pointer-to-pointer` because your change seems to work with any number of 
> pointers. Although I appreciate that the latter one is a spec terminology.
No preference, but we could even mention both terms for completeness perhaps?


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

https://reviews.llvm.org/D98076

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


[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

2021-03-10 Thread Deep Majumder via Phabricator via cfe-commits
RedDocMD added a comment.

In D97183#2615096 , @NoQ wrote:

>> why does this not work?
>
> How does this not work? What does it say?

Sorry, my bad! I had made a typo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97183

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


[PATCH] D96976: [analyzer] Fix reinterpret_cast handling for pointer-to-member

2021-03-10 Thread Deep Majumder via Phabricator via cfe-commits
RedDocMD added a comment.

Your view, @steakhal?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96976

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


[PATCH] D98291: [compiler-rt] Fix stale incremental builds when using `LLVM_BUILD_EXTERNAL_COMPILER_RT=ON`.

2021-03-10 Thread Kuba (Brecka) Mracek via Phabricator via cfe-commits
kubamracek accepted this revision.
kubamracek added a comment.

Looks good!

By the way, Apple Clang releases also build compiler-rt this way, so it would 
be worth checking with @arphaman that he's okay with the change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98291

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


[PATCH] D98076: [OpenCL][Docs] Release 12.0 notes

2021-03-10 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:214
+- Added ``global_device`` and ``global_host`` address spaces for USM
+  allocations.
+

svenvh wrote:
> Anastasia wrote:
> > svenvh wrote:
> > > Anastasia wrote:
> > > > svenvh wrote:
> > > > > Perhaps one more point to mention:
> > > > > ```
> > > > >  - Allow pointer-to-pointer kernel arguments beyond OpenCL 1.2.
> > > > > ```
> > > > > (related commit is 523775f96742 ("[OpenCL] Allow pointer-to-pointer 
> > > > > kernel args beyond CL 1.2", 2020-12-01)).
> > > > I have covered it in this but it is combined with another diagnostic:
> > > > 
> > > > 
> > > > ```
> > > > - Improved diagnostics for nested pointers and unevaluated ``vec_step`` 
> > > > expression.
> > > > ```
> > > > 
> > > > But if you think it is better I can change as you have suggested.
> > > Ah, that's probably why I missed it, as I was assuming "improved 
> > > diagnostics" means a nicer error message, not that Clang now accepts 
> > > something that it rejected before.  So in my opinion it deserves a point 
> > > on its own.
> > Ok, do you think we should use `nested pointers` instead of 
> > `pointer-to-pointer` because your change seems to work with any number of 
> > pointers. Although I appreciate that the latter one is a spec terminology.
> No preference, but we could even mention both terms for completeness perhaps?
Do you mean something like:

```
- Allow nested pointers (e.g. pointer-to-pointer) kernel arguments beyond 
OpenCL 1.2.
```


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

https://reviews.llvm.org/D98076

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


[PATCH] D96771: [OpenCL] Add distinct file extension for C++ for OpenCL

2021-03-10 Thread Andrzej Warzynski via Phabricator via cfe-commits
awarzynski accepted this revision.
awarzynski added a comment.

Thank you for addressing my comments and for working on this! I've left one 
small suggestion, but that's a [nit].




Comment at: clang/test/Driver/cxx_for_opencl.cppcl:1
+// RUN: %clang %s -Xclang -verify -fsyntax-only
+// RUN: %clang %s -cl-std=clc++ -Xclang -verify -fsyntax-only

This is a very neat test. Would it make sense to add something more basic too? 
For example (in a separate file):

```
// RUN: %clang %s -fstynax-only -### | FileCheck %s

// CHECK: -x -cppcl
```

This way you would have a bit more explicit test to verifiy that the compiler 
driver picks the right language based on the extension.


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

https://reviews.llvm.org/D96771

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


[PATCH] D89942: Disable LTO and LLD for bootstrap builds on systems unsupported by LLD

2021-03-10 Thread serge via Phabricator via cfe-commits
serge-sans-paille added inline comments.



Comment at: clang/CMakeLists.txt:639
+  # We want LLD for LTO, but LLD does not support SystemZ, so disable
+  # LTO here and use the installed system linker
+  if ("${LLVM_NATIVE_ARCH}" MATCHES "SystemZ")

I guess you're using `FORCE` here to change the user default, right? In that 
case wouldn't that make sense to warn only if it's an actual change?


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

https://reviews.llvm.org/D89942

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


[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

2021-03-10 Thread Whisperity via Phabricator via cfe-commits
whisperity updated this revision to Diff 329666.
whisperity edited the summary of this revision.
whisperity added reviewers: hokein, njames93.
whisperity added a comment.

- Massively refactored and modernised the implementation
- Removed spaghetti code related to the check options and made their storage 
and defaults clearer
- Fixed modelling issues that caused false positives around lambdas, overloaded 
call operators, recursive calls, enumconstant arguments
- Made the abbreviation dictionary for the //Abbreviations// heuristic a 
check-option
- Made the check message much more legible
- Fixed a severe modelling issue about not respecting or handling record types 
passed by copy (`CXXConstructExpr`)
- Wrote the documentation for real, including all heuristics, default values, 
options, etc. The original documentation was basically empty. For the docs, I 
used @varjujan's thesis (which subject was this check), which is unfortunately 
only available in Hungarian, but it helped me understand what some of the enums 
and the //Bound// doing on the inside.
- Thanks to having written a proper documentation, I also renamed a few symbols 
and check options to better tell what they are about.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D20689

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp
  clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability-suspicious-call-argument.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-suspicious-call-argument.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-suspicious-call-argument.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-suspicious-call-argument.cpp
@@ -0,0 +1,468 @@
+// RUN: %check_clang_tidy %s readability-suspicious-call-argument %t -- -- -std=c++11
+
+void foo_1(int aa, int bb) {}
+
+void foo_2(int source, int aa) {}
+
+void foo_3(int valToRet, int aa) {}
+
+void foo_4(int pointer, int aa) {}
+
+void foo_5(int aa, int bb, int cc, ...) {}
+
+void foo_6(const int dd, bool &ee) {}
+
+void foo_7(int aa, int bb, int cc, int ff = 7) {}
+
+// Test functions for convertible argument--parameter types.
+void fun(const int &m);
+void fun2() {
+  int m = 3;
+  fun(m);
+}
+
+// Test cases for parameters of const reference and value.
+void value_const_reference(int ll, const int &kk);
+
+void const_ref_value_swapped() {
+  const int &kk = 42;
+  const int &ll = 42;
+  value_const_reference(kk, ll);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'kk' (passed to 'll') looks like it might be swapped with the 2nd, 'll' (passed to 'kk') [readability-suspicious-call-argument]
+  // CHECK-MESSAGES: :[[@LINE-7]]:6: note: in the call to 'value_const_reference', declared here
+}
+
+// Const, non const references.
+void const_nonconst_parameters(const int &mm, int &nn);
+
+void const_nonconst_swap1() {
+  const int &nn = 42;
+  int mm;
+  // Do not check, because non-const reference parameter cannot bind to const reference argument.
+  const_nonconst_parameters(nn, mm);
+}
+
+void const_nonconst_swap3() {
+  const int nn = 42;
+  int m = 42;
+  int &mm = m;
+  // Do not check, const int does not bind to non const reference.
+  const_nonconst_parameters(nn, mm);
+}
+
+void const_nonconst_swap2() {
+  int nn;
+  int mm;
+  // Check for swapped arguments. (Both arguments are non-const.)
+  const_nonconst_parameters(nn, mm);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'nn' (passed to 'mm') looks like it might be swapped with the 2nd, 'mm' (passed to 'nn')
+}
+
+void const_nonconst_pointers(const int *mm, int *nn);
+void const_nonconst_pointers2(const int *mm, const int *nn);
+
+void const_nonconst_pointers_swapped() {
+  int *mm;
+  const int *nn;
+  const_nonconst_pointers(nn, mm);
+}
+
+void const_nonconst_pointers_swapped2() {
+  const int *mm;
+  int *nn;
+  const_nonconst_pointers2(nn, mm);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'nn' (passed to 'mm') looks like it might be swapped with the 2nd, 'mm' (passed to 'nn')
+}
+
+// Test cases for pointers and arrays.
+void pointer_array_parameters(
+int *pp, int qq[4]);
+
+void pointer_array_swap() {
+  int qq[5];
+  int *pp;
+  // Check for swapped arguments. An array implicitly converts to a pointer.
+  pointer_array_

[PATCH] D98358: [OpenMP] Restore backwards compatibility for libomptarget

2021-03-10 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 created this revision.
jhuber6 added a reviewer: jdoerfert.
Herald added subscribers: guansong, yaxunl.
jhuber6 requested review of this revision.
Herald added subscribers: llvm-commits, openmp-commits, cfe-commits, sstefan1.
Herald added projects: clang, OpenMP, LLVM.

The changes introduced in D87946  changed the 
API for libomptarget
functions. `__kmpc_push_target_tripcount` was a function in Clang 11.x
but was not given a backward-compatible interface accidentally. This 
will make it backwards compatible, but will require Clang 12 to recompile.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98358

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  openmp/libomptarget/include/omptarget.h
  openmp/libomptarget/src/interface.cpp


Index: openmp/libomptarget/src/interface.cpp
===
--- openmp/libomptarget/src/interface.cpp
+++ openmp/libomptarget/src/interface.cpp
@@ -437,8 +437,13 @@
   MapComponentInfoTy(base, begin, size, type, name));
 }
 
-EXTERN void __kmpc_push_target_tripcount(ident_t *loc, int64_t device_id,
+EXTERN void __kmpc_push_target_tripcount(int64_t device_id,
  uint64_t loop_tripcount) {
+  __kmpc_push_target_tripcount_mapper(nullptr, device_id, loop_tripcount);
+}
+
+EXTERN void __kmpc_push_target_tripcount_mapper(ident_t *loc, int64_t 
device_id,
+uint64_t loop_tripcount) {
   TIMESCOPE_WITH_IDENT(loc);
   if (checkDeviceAndCtors(device_id, loc) != OFFLOAD_SUCCESS) {
 DP("Not offloading to device %" PRId64 "\n", device_id);
Index: openmp/libomptarget/include/omptarget.h
===
--- openmp/libomptarget/include/omptarget.h
+++ openmp/libomptarget/include/omptarget.h
@@ -312,9 +312,12 @@
 int32_t thread_limit, int32_t depNum, void *depList, int32_t noAliasDepNum,
 void *noAliasDepList);
 
-void __kmpc_push_target_tripcount(ident_t *loc, int64_t device_id,
+void __kmpc_push_target_tripcount(int64_t device_id,
   uint64_t loop_tripcount);
 
+void __kmpc_push_target_tripcount_mapper(ident_t *loc, int64_t device_id,
+ uint64_t loop_tripcount);
+
 #ifdef __cplusplus
 }
 #endif
Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -375,7 +375,7 @@
 __OMP_RTL(__kmpc_destroy_allocator, false, Void, /* Int */ Int32,
   /* omp_allocator_handle_t */ VoidPtr)
 
-__OMP_RTL(__kmpc_push_target_tripcount, false, Void, IdentPtr, Int64, Int64)
+__OMP_RTL(__kmpc_push_target_tripcount_mapper, false, Void, IdentPtr, Int64, 
Int64)
 __OMP_RTL(__tgt_target_mapper, false, Int32, IdentPtr, Int64, VoidPtr, Int32, 
VoidPtrPtr,
   VoidPtrPtr, Int64Ptr, Int64Ptr, VoidPtrPtr, VoidPtrPtr)
 __OMP_RTL(__tgt_target_nowait_mapper, false, Int32, IdentPtr, Int64, VoidPtr, 
Int32,
@@ -844,7 +844,7 @@
 __OMP_RTL_ATTRS(__kmpc_init_allocator, DefaultAttrs, ReturnPtrAttrs, {})
 __OMP_RTL_ATTRS(__kmpc_destroy_allocator, AllocAttrs, AttributeSet(), {})
 
-__OMP_RTL_ATTRS(__kmpc_push_target_tripcount, SetterAttrs, AttributeSet(), {})
+__OMP_RTL_ATTRS(__kmpc_push_target_tripcount_mapper, SetterAttrs, 
AttributeSet(), {})
 __OMP_RTL_ATTRS(__tgt_target_mapper, ForkAttrs, AttributeSet(), {})
 __OMP_RTL_ATTRS(__tgt_target_nowait_mapper, ForkAttrs, AttributeSet(), {})
 __OMP_RTL_ATTRS(__tgt_target_teams_mapper, ForkAttrs, AttributeSet(), {})
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -9948,7 +9948,7 @@
   llvm::Value *Args[] = {RTLoc, DeviceID, NumIterations};
   CGF.EmitRuntimeCall(
   OMPBuilder.getOrCreateRuntimeFunction(
-  CGM.getModule(), OMPRTL___kmpc_push_target_tripcount),
+  CGM.getModule(), OMPRTL___kmpc_push_target_tripcount_mapper),
   Args);
 }
   };


Index: openmp/libomptarget/src/interface.cpp
===
--- openmp/libomptarget/src/interface.cpp
+++ openmp/libomptarget/src/interface.cpp
@@ -437,8 +437,13 @@
   MapComponentInfoTy(base, begin, size, type, name));
 }
 
-EXTERN void __kmpc_push_target_tripcount(ident_t *loc, int64_t device_id,
+EXTERN void __kmpc_push_target_tripcount(int64_t device_id,
  uint64_t loop_tripcount) {
+  __kmpc_push_target_tripcount_mapper(nullptr, device_id, loop_tripcount);
+}
+
+EXTERN void __kmpc_push_target_tripcount_mapper(ident_t *loc, int64_t device_id,
+   

[PATCH] D97869: [OpenCL][Draft] Add OpenCL builtin test generator

2021-03-10 Thread Sven van Haastregt via Phabricator via cfe-commits
svenvh added a comment.

In D97869#2616772 , @Anastasia wrote:

> Regarding 2 and 4 I think we should drive towards deprecation of `opencl-c.h` 
> as it is a maintenance overhead but we could convert it into a test instead?

Perhaps you misunderstood option 4, as that does not require maintaining 
`opencl-c.h`.  Only for option 2 we would have to maintain `opencl-c.h`.  For 
option 4, the checked in files could look something like this:

name=clang/test/SemaOpenCL/builtins-opencl2.0.check
  ; RUN: clang-tblgen -emit-opencl-builtin-count -cl-std=cl2.0 
OpenCLBuiltins.td | FileCheck '%s'
  
  ; This file lists the number of builtin function declarations per builtin 
function name
  ; available in the OpenCL 2.0 language mode.
  
  CHECK: absdiff 48
  CHECK: abs 48
  CHECK: acos 18
  CHECK: acosh 18
  CHECK: acospi 18
  ...

To illustrate the use, suppose a developer writes a patch that accidentally 
makes the `double` variants of `acos` unavailable in OpenCL 2.0.  The reported 
number of `acos` builtins would be different so FileCheck would fail on the 
line containing `acos 18`.  If we use option 4 in combination with option 1, a 
Clang developer can regenerate the test and observe the differences for the 
`acos` tests: from there it becomes obvious that the double variants of `acos` 
disappeared.

> Aside from option 3 all other options will require adding non-standard 
> testing formats in Clang. This means that if there is any core refactoring 
> changes even outside of OpenCL affecting the functionality in the headers, it 
> would be required to learn how we do the testing in order to address any 
> breakages. Perhaps this can be mitigated by the documentation, but overall it 
> is an extra burden to the community. Speaking from my personal experience I 
> was in the situations breaking functionality outside of OpenCL and having a 
> unified testing format is a big advantage in understanding and addressing the 
> issues in unfamiliar code.

Do you expect this to be a problem in practice for the other proposals, and if 
so could you elaborate in more detail about any particular concerns for the 
other proposals?

> What problems do you see with checking in autogenerated files? There are some 
> existing tests that check similar functionality and they seem to contain 
> repetitive patterns. Honestly, I wouldn't be able to say if they were written 
> with copy/paste or auto-generated but I am not sure whether it is very 
> important. Of course, it would have been better if we have built up the 
> functionality incrementally, and it is still an option. But it hasn't been 
> possible in the past and as a matter of fact the original header was 
> committed as one large file (around 15K lines).
>
>>   Updates to OpenCLBuiltins.td will give large diffs when regenerating the 
>> tests (regardless of whether the file is split or not).
>
> Could you provide more details. I don't follow this.

There are two problems here that combine to make the problem bigger: checking 
in autogenerated files, and the fact that those files are large.  The problem I 
anticipate with regenerating the test files (at least with the current 
generator), is that inserting a new builtin in the .td file results in 
renumbering of the test functions, which results in a large diff.

Other problems I see with checking in large autogenerated files:

- Merge conflicts in the generated files when reordering/rebasing/porting 
patches.  A single conflict in the .td description could expand to many 
conflicts in the autogenerated file(s).
- Diffs in the autogenerated files clutter the output of for example `git log 
-p`.
- Code reviews of changes to those files are harder due to the size.
- It bloats the repository.

Disclaimer: I may be biased by my previous experience on projects that had 
large autogenerated files checked in, that's why I feel that we should avoid 
doing so if we reasonably can.  I have probably revealed now that option 3 is 
my least favorite option. :-)

>>   Manually verifying that the generated test is actually correct is not 
>> trivial.
>
> I think the way to approach this would be to just assume that the test is 
> correct and then fix any issues we discover as we go along with the 
> development. There is nothing wrong with this though as even small manually 
> written tests have bugs.

Fair enough.


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

https://reviews.llvm.org/D97869

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


[PATCH] D98076: [OpenCL][Docs] Release 12.0 notes

2021-03-10 Thread Sven van Haastregt via Phabricator via cfe-commits
svenvh accepted this revision.
svenvh added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/docs/ReleaseNotes.rst:214
+- Added ``global_device`` and ``global_host`` address spaces for USM
+  allocations.
+

Anastasia wrote:
> svenvh wrote:
> > Anastasia wrote:
> > > svenvh wrote:
> > > > Anastasia wrote:
> > > > > svenvh wrote:
> > > > > > Perhaps one more point to mention:
> > > > > > ```
> > > > > >  - Allow pointer-to-pointer kernel arguments beyond OpenCL 1.2.
> > > > > > ```
> > > > > > (related commit is 523775f96742 ("[OpenCL] Allow pointer-to-pointer 
> > > > > > kernel args beyond CL 1.2", 2020-12-01)).
> > > > > I have covered it in this but it is combined with another diagnostic:
> > > > > 
> > > > > 
> > > > > ```
> > > > > - Improved diagnostics for nested pointers and unevaluated 
> > > > > ``vec_step`` expression.
> > > > > ```
> > > > > 
> > > > > But if you think it is better I can change as you have suggested.
> > > > Ah, that's probably why I missed it, as I was assuming "improved 
> > > > diagnostics" means a nicer error message, not that Clang now accepts 
> > > > something that it rejected before.  So in my opinion it deserves a 
> > > > point on its own.
> > > Ok, do you think we should use `nested pointers` instead of 
> > > `pointer-to-pointer` because your change seems to work with any number of 
> > > pointers. Although I appreciate that the latter one is a spec terminology.
> > No preference, but we could even mention both terms for completeness 
> > perhaps?
> Do you mean something like:
> 
> ```
> - Allow nested pointers (e.g. pointer-to-pointer) kernel arguments beyond 
> OpenCL 1.2.
> ```
Looks good to me.


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

https://reviews.llvm.org/D98076

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


[PATCH] D94942: [clangd] Add tweak for implementing abstract class

2021-03-10 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 329680.
njames93 added a comment.

Tweak the prepare method to just check for methods that need implementations, 
this saves some work that could be deferred to the apply method.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94942

Files:
  clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
  clang-tools-extra/clangd/refactor/tweaks/ImplementAbstract.cpp
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/tweaks/ImplementAbstractTests.cpp

Index: clang-tools-extra/clangd/unittests/tweaks/ImplementAbstractTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/tweaks/ImplementAbstractTests.cpp
@@ -0,0 +1,415 @@
+//===-- ImplementAbstractTests.cpp --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "TestTU.h"
+#include "TweakTesting.h"
+#include "gmock/gmock-matchers.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using ::testing::Not;
+
+namespace clang {
+namespace clangd {
+namespace {
+
+bool stringEqIgnoreWs(StringRef LHS, StringRef RHS) {
+  auto TrimmedL = LHS.trim();
+  auto TrimmedR = RHS.trim();
+  static constexpr llvm::StringLiteral WS(" \t\r\n\f\v");
+
+  while (!TrimmedL.empty() && !TrimmedR.empty()) {
+auto LPos = TrimmedL.find_first_of(WS);
+auto RPos = TrimmedR.find_first_of(WS);
+if (TrimmedL.take_front(LPos) != TrimmedR.take_front(RPos))
+  return false;
+TrimmedL =
+TrimmedL.substr(LPos).drop_while([](char C) { return WS.contains(C); });
+TrimmedR =
+TrimmedR.substr(RPos).drop_while([](char C) { return WS.contains(C); });
+  }
+  return TrimmedL == TrimmedR;
+}
+
+MATCHER_P(STREQWS, EqualTo, "") {
+  if (stringEqIgnoreWs(arg, EqualTo))
+return true;
+
+  auto Result =
+  testing::internal::EqFailure("", "", arg, std::string(EqualTo), false);
+  *result_listener << Result.message();
+  return false;
+}
+
+TWEAK_TEST(ImplementAbstract);
+
+TEST_F(ImplementAbstractTest, TestUnavailable) {
+
+  StringRef Cases[]{
+  // Not a pure virtual method.
+  R"cpp(
+class A {
+  virtual void Foo();
+};
+class ^B : public A {};
+  )cpp",
+  // Pure virtual method overridden in class.
+  R"cpp(
+class A {
+  virtual void Foo() = 0;
+};
+class ^B : public A {
+  void Foo() override;
+};
+  )cpp",
+  // Pure virtual method overridden in class with virtual keyword
+  R"cpp(
+class A {
+  virtual void Foo() = 0;
+};
+class ^B : public A {
+  virtual void Foo() override;
+};
+  )cpp",
+  // Pure virtual method overridden in class without override keyword
+  R"cpp(
+class A {
+  virtual void Foo() = 0;
+};
+class ^B : public A {
+  void Foo();
+};
+  )cpp",
+  // Pure virtual method overriden in base class.
+  R"cpp(
+class A {
+  virtual void Foo() = 0;
+};
+class B : public A {
+  void Foo() override;
+};
+class ^C : public B {};
+  )cpp"};
+  for (const auto &Case : Cases) {
+EXPECT_THAT(Case, Not(isAvailable()));
+  }
+}
+
+TEST_F(ImplementAbstractTest, NormalAvailable) {
+  struct Case {
+llvm::StringRef TestHeader;
+llvm::StringRef TestSource;
+llvm::StringRef ExpectedSource;
+  };
+
+  Case Cases[]{
+  {
+  R"cpp(
+class A {
+  virtual void Foo() = 0;
+};)cpp",
+  R"cpp(
+class B : public A {^};
+  )cpp",
+  R"cpp(
+class B : public A {
+  void Foo() override {
+  }
+};
+  )cpp",
+  },
+  {
+  R"cpp(
+class A {
+public:
+  virtual int Foo() = 0;
+};)cpp",
+  R"cpp(
+class ^B : public A {};
+  )cpp",
+  R"cpp(
+class B : public A {
+public:
+  int Foo() override {
+return {};
+  }
+};
+  )cpp",
+  },
+  {
+  R"cpp(
+class A {
+  virtual void Foo(int Param) = 0;
+};)cpp",
+  R"cpp(
+class ^B : public A {};
+  )cpp",
+  R"cpp(
+class B : public A {
+  void Foo(int Param) override {
+  }
+};
+  )cpp",
+  },
+  {
+  R"cpp(
+class 

[PATCH] D98341: [analyzer][solver] Prevent infeasible states (PR49490)

2021-03-10 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

You're testing it with the help of an alpha checker. Is this checker doing 
something special that we don't do normally? In particular, both array bound 
checkers that we have are very likely to be re-done from scratch if they are 
ever to be completed. A lot of decisions in them are questionable. So i'm 
worried that this test will become stale in that process. Maybe a unittest 
would be more appropriate so that to guarantee the specific functionality 
without relying on implementation details of the specific checker(?)

Code looks great and I guess assertions are a test on their own.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98341

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


[PATCH] D98358: [OpenMP] Restore backwards compatibility for libomptarget

2021-03-10 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

LG. We should have a test that references the entire interface so we realize if 
something breaks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98358

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


[PATCH] D97854: [RFC][nsan] A Floating-point numerical sanitizer.

2021-03-10 Thread Steve Canon via Phabricator via cfe-commits
scanon added a comment.

Is there a mechanism to instruct the sanitizer to ignore a specific expression 
or function? From a cursory reading, I am mildly concerned about a deluge of 
false positives from primitives that compute exact (or approximate) residuals; 
these are acting to eliminate or precisely control floating-point errors, but 
tend to show up as "unstable" in a naive analysis that isn't aware of them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97854

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


[PATCH] D98193: [CUDA][HIP] Allow non-ODR use of host var in device

2021-03-10 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 329686.
yaxunl added a comment.

minor bug fix


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

https://reviews.llvm.org/D98193

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CodeGenCUDA/device-use-host-var.cu
  clang/test/SemaCUDA/device-use-host-var.cu

Index: clang/test/SemaCUDA/device-use-host-var.cu
===
--- clang/test/SemaCUDA/device-use-host-var.cu
+++ clang/test/SemaCUDA/device-use-host-var.cu
@@ -5,37 +5,96 @@
 
 #include "Inputs/cuda.h"
 
-int global_host_var;
+struct A {
+  int x;
+  static int host_var;
+};
+
+int A::host_var;
+
+namespace X {
+  int host_var;
+}
+
+static int static_host_var;
+
 __device__ int global_dev_var;
 __constant__ int global_constant_var;
 __shared__ int global_shared_var;
-constexpr int global_constexpr_var = 1;
+
+int global_host_var;
 const int global_const_var = 1;
+constexpr int global_constexpr_var = 1;
+
+int global_host_array[2] = {1, 2};
+const int global_const_array[2] = {1, 2};
+constexpr int global_constexpr_array[2] = {1, 2};
+
+A global_host_struct_var{1};
+const A global_const_struct_var{1};
+constexpr A global_constexpr_struct_var{1};
 
 template
 __global__ void kernel(F f) { f(); } // dev-note2 {{called by 'kernel<(lambda}}
 
 __device__ void dev_fun(int *out) {
-  int &ref_host_var = global_host_var; // dev-error {{reference to __host__ variable 'global_host_var' in __device__ function}}
+  // Check access device variables are allowed.
   int &ref_dev_var = global_dev_var;
   int &ref_constant_var = global_constant_var;
   int &ref_shared_var = global_shared_var;
-  const int &ref_constexpr_var = global_constexpr_var;
-  const int &ref_const_var = global_const_var;
-
-  *out = global_host_var; // dev-error {{reference to __host__ variable 'global_host_var' in __device__ function}}
+  *out = ref_dev_var;
+  *out = ref_constant_var;
+  *out = ref_shared_var;
   *out = global_dev_var;
   *out = global_constant_var;
   *out = global_shared_var;
-  *out = global_constexpr_var;
+
+  // Check access of non-const host variables are not allowed.
+  *out = global_host_var; // dev-error {{reference to __host__ variable 'global_host_var' in __device__ function}}
   *out = global_const_var;
+  *out = global_constexpr_var;
+  global_host_var = 1; // dev-error {{reference to __host__ variable 'global_host_var' in __device__ function}}
 
+  // Check reference of non-constexpr host variables are not allowed.
+  int &ref_host_var = global_host_var; // dev-error {{reference to __host__ variable 'global_host_var' in __device__ function}}
+  const int &ref_const_var = global_const_var; // dev-error {{reference to __host__ variable 'global_const_var' in __device__ function}}
+  const int &ref_constexpr_var = global_constexpr_var;
   *out = ref_host_var;
-  *out = ref_dev_var;
-  *out = ref_constant_var;
-  *out = ref_shared_var;
   *out = ref_constexpr_var;
   *out = ref_const_var;
+
+  // Check access member of non-constexpr struct type host variable is not allowed.
+  *out = global_host_struct_var.x; // dev-error {{reference to __host__ variable 'global_host_struct_var' in __device__ function}}
+  *out = global_const_struct_var.x; // dev-error {{reference to __host__ variable 'global_const_struct_var' in __device__ function}}
+  *out = global_constexpr_struct_var.x;
+  global_host_struct_var.x = 1; // dev-error {{reference to __host__ variable 'global_host_struct_var' in __device__ function}}
+
+  // Check address taking of non-constexpr host variables is not allowed.
+  int *p = &global_host_var; // dev-error {{reference to __host__ variable 'global_host_var' in __device__ function}}
+  const int *cp = &global_const_var; // dev-error {{reference to __host__ variable 'global_const_var' in __device__ function}}
+  const int *cp2 = &global_constexpr_var;
+
+  // Check access elements of non-constexpr host array is not allowed.
+  *out = global_host_array[1]; // dev-error {{reference to __host__ variable 'global_host_array' in __device__ function}}
+  *out = global_const_array[1]; // dev-error {{reference to __host__ variable 'global_const_array' in __device__ function}}
+  *out = global_constexpr_array[1];
+
+  // Check ODR-use of host variables in namespace is not allowed.
+  *out = X::host_var; // dev-error {{reference to __host__ variable 'host_var' in __device__ function}}
+
+  // Check ODR-use of static host varables in class or file scope is not allowed.
+  *out = A::host_var; // dev-error {{reference to __host__ variable 'host_var' in __device__ function}}
+  *out = static_host_var; // dev-error {{reference to __host__ variable 'static_host_var' in __device__ function}}
+
+  // Check function-scope static variable is allowed.
+  static int static_var;
+  *out = static_var;
+
+  // Check non-ODR use of host varirables are allowed.
+  *out = sizeof(global_host_var);
+  *out = sizeof(global_host_struct_var.x);
+  decltype(global_host_var) var1;
+

[PATCH] D91556: [OpenMPIRBuilder} Add capturing of parameters to pass to omp::parallel

2021-03-10 Thread Kiran Chandramohan via Phabricator via cfe-commits
kiranchandramohan added a comment.
Herald added subscribers: dcaballe, cota.

Ping @llitchev. Would you have time to take this forward?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91556

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


[PATCH] D98358: [OpenMP] Restore backwards compatibility for libomptarget

2021-03-10 Thread Chi Chun Chen via Phabricator via cfe-commits
cchen added a comment.

I think some existing Clang OpenMP codegen tests will break if they are testing 
against `__kmpc_push_target_tripcount`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98358

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


[PATCH] D98358: [OpenMP] Restore backwards compatibility for libomptarget

2021-03-10 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added a comment.

In D98358#2617152 , @cchen wrote:

> I think some existing Clang OpenMP codegen tests will break if they are 
> testing against `__kmpc_push_target_tripcount`.

I'm working on fixing the tests, I also forgot to include it in the `exports`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98358

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


  1   2   3   >