[clang] e2a1639 - [test] Fix unused check prefixes in test/Driver

2020-10-31 Thread Fangrui Song via cfe-commits

Author: Fangrui Song
Date: 2020-10-31T00:14:59-07:00
New Revision: e2a1639c738c46f65d978fde161c10bac86392af

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

LOG: [test] Fix unused check prefixes in test/Driver

Note, the deprecated AArch64 -msign-return-address= does not accept b-key. So
delete the incorrect tests.

Added: 


Modified: 
clang/test/Driver/aarch64-security-options.c
clang/test/Driver/arch-specific-libdir-rpath.c
clang/test/Driver/arm-execute-only.c
clang/test/Driver/cl-denorms-are-zero.cl
clang/test/Driver/fopenmp.c
clang/test/Driver/fsanitize-blacklist.c
clang/test/Driver/fuzzer.c
clang/test/Driver/hip-phases.hip
clang/test/Driver/rocm-detect.cl
clang/test/Driver/rocm-detect.hip

Removed: 




diff  --git a/clang/test/Driver/aarch64-security-options.c 
b/clang/test/Driver/aarch64-security-options.c
index 9ba5067cc3cf..00ea1f46da28 100644
--- a/clang/test/Driver/aarch64-security-options.c
+++ b/clang/test/Driver/aarch64-security-options.c
@@ -9,15 +9,6 @@
 // RUN: %clang -target aarch64--none-eabi -c %s -### -msign-return-address=all 
 2>&1 | \
 // RUN: FileCheck %s --check-prefix=RA-ALL  --check-prefix=KEY-A 
--check-prefix=BTE-OFF
 
-// Check that the -msign-return-address= option can also accept the signing key
-// to use.
-
-// RUN: %clang -target aarch64--none-eabi -c %s -### 
-msign-return-address=non-leaf   2>&1 | \
-// RUN: FileCheck %s --check-prefix=RA-NON-LEAF --check-prefix=KEY-B 
--check-prefix=BTE-OFF
-
-// RUN: %clang -target aarch64--none-eabi -c %s -### -msign-return-address=all 
   2>&1 | \
-// RUN: FileCheck %s --check-prefix=RA-ALL  --check-prefix=KEY-B 
--check-prefix=BTE-OFF
-
 // -mbranch-protection with standard
 // RUN: %clang -target aarch64--none-eabi -c %s -### 
-mbranch-protection=standard2>&1 | \
 // RUN: FileCheck %s --check-prefix=RA-NON-LEAF --check-prefix=KEY-A 
--check-prefix=BTE-ON

diff  --git a/clang/test/Driver/arch-specific-libdir-rpath.c 
b/clang/test/Driver/arch-specific-libdir-rpath.c
index 2ea41aade342..1aa5bc88b72d 100644
--- a/clang/test/Driver/arch-specific-libdir-rpath.c
+++ b/clang/test/Driver/arch-specific-libdir-rpath.c
@@ -84,7 +84,7 @@
 // NO-RPATH-X86_64-NOT:   "-rpath" 
"[[RESDIR]]{{(/|)lib(/|)linux(/|)x86_64}}"
 //
 // LIBPATH-AARCH64: -L[[RESDIR]]{{(/|)lib(/|)linux(/|)aarch64}}
-// RPATH-AAARCH4:   "-rpath" 
"[[RESDIR]]{{(/|)lib(/|)linux(/|)aarch64}}"
+// RPATH-AARCH64:   "-rpath" 
"[[RESDIR]]{{(/|)lib(/|)linux(/|)aarch64}}"
 //
 // NO-LIBPATH-NOT: "-L{{[^"]*Inputs(/|)resource_dir}}"
 // NO-RPATH-NOT:   "-rpath" {{.*(/|)Inputs(/|)resource_dir}}

diff  --git a/clang/test/Driver/arm-execute-only.c 
b/clang/test/Driver/arm-execute-only.c
index c73785d38566..81b822f1ac06 100644
--- a/clang/test/Driver/arm-execute-only.c
+++ b/clang/test/Driver/arm-execute-only.c
@@ -11,7 +11,7 @@
 // CHECK-EXECUTE-ONLY-LONG-CALLS: error: option '-mexecute-only' cannot be 
specified with '-mlong-calls'
 
 // RUN: %clang -target armv7m-eabi -x assembler -mexecute-only %s -c -### 2>&1 
\
-// RUN:| FileCheck %s -check-prefix CHECK-NO-EXECUTE-ONLY -check-prefix 
CHECK-NO-EXECUTE-ONLY-ASM
+// RUN:| FileCheck %s --check-prefix=CHECK-NO-EXECUTE-ONLY-ASM
 // CHECK-NO-EXECUTE-ONLY-ASM: warning: argument unused during compilation: 
'-mexecute-only'
 
 // -mpure-code flag for GCC compatibility

diff  --git a/clang/test/Driver/cl-denorms-are-zero.cl 
b/clang/test/Driver/cl-denorms-are-zero.cl
index e3fd095e5831..31c1be8c2517 100644
--- a/clang/test/Driver/cl-denorms-are-zero.cl
+++ b/clang/test/Driver/cl-denorms-are-zero.cl
@@ -18,6 +18,7 @@
 // RUN: %clang -### -target amdgcn--amdhsa -nogpulib -c %s 2>&1 | FileCheck 
-check-prefixes=AMDGCN,AMDGCN-DENORM %s
 // RUN: %clang -### -cl-denorms-are-zero -o - -target amdgcn--amdhsa -nogpulib 
-c %s 2>&1 | FileCheck -check-prefixes=AMDGCN,AMDGCN-FLUSH %s
 
+// AMDGCN: "-triple" "amdgcn-unknown-amdhsa"
 // AMDGCN-FLUSH: "-fdenormal-fp-math-f32=preserve-sign,preserve-sign"
 
 // This should be omitted and default to ieee

diff  --git a/clang/test/Driver/fopenmp.c b/clang/test/Driver/fopenmp.c
index c45dbafbb05c..b467fbed93e2 100644
--- a/clang/test/Driver/fopenmp.c
+++ b/clang/test/Driver/fopenmp.c
@@ -114,7 +114,7 @@
 // CHECK-LD-STATIC-GOMP: "{{.*}}ld{{(.exe)?}}"
 // CHECK-LD-STATIC-GOMP: "-Bstatic" "-lgomp" "-Bdynamic"
 // CHECK-LD-STATIC-GOMP-RT: "-lrt"
-// CHECK-LD-STATIC-NO-GOMP-RT-NOT: "-lrt"
+// CHECK-LD-STATIC-GOMP-NO-RT-NOT: "-lrt"
 //
 // CHECK-LD-STATIC-IOMP5: "{{.*}}ld{{(.exe)?}}"
 // CHECK-LD-STATIC-IOMP5: "-Bstatic" "-liomp5" "-Bdynamic"

diff  --git a/clang/test/Driver/fsanitize

[PATCH] D90516: [clang] Limit scope of CLANG_VENDOR definition

2020-10-31 Thread Petr Hosek via Phabricator via cfe-commits
phosek accepted this revision.
phosek 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/D90516/new/

https://reviews.llvm.org/D90516

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


[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-10-31 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits
glaubitz created this revision.
glaubitz added a reviewer: nemanjai.
Herald added subscribers: cfe-commits, pengfei, fedor.sergeev, jyknight.
Herald added a project: clang.
glaubitz requested review of this revision.

This fixes the Builtins-sparc-linux testsuite failures on Linux
SPARC which occur because clang cannot find the 32-bit runtime
libraries when -m32 is passed on the command line.

The same workaround is already being used on X86 and PPC.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90524

Files:
  clang/lib/Driver/ToolChains/Linux.cpp


Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -185,8 +185,8 @@
 return Triple.isArch32Bit() ? "lib" : "lib64";
   }
 
-  // It happens that only x86 and PPC use the 'lib32' variant of oslibdir, and
-  // using that variant while targeting other architectures causes problems
+  // It happens that only x86, PPC and SPARC use the 'lib32' variant of 
oslibdir,
+  // and using that variant while targeting other architectures causes problems
   // because the libraries are laid out in shared system roots that can't cope
   // with a 'lib32' library search path being considered. So we only enable
   // them when we know we may need it.
@@ -195,7 +195,8 @@
   // reasoning about oslibdir spellings with the lib dir spellings in the
   // GCCInstallationDetector, but that is a more significant refactoring.
   if (Triple.getArch() == llvm::Triple::x86 ||
-  Triple.getArch() == llvm::Triple::ppc)
+  Triple.getArch() == llvm::Triple::ppc ||
+  Triple.getArch() == llvm::Triple::sparc)
 return "lib32";
 
   if (Triple.getArch() == llvm::Triple::x86_64 &&


Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -185,8 +185,8 @@
 return Triple.isArch32Bit() ? "lib" : "lib64";
   }
 
-  // It happens that only x86 and PPC use the 'lib32' variant of oslibdir, and
-  // using that variant while targeting other architectures causes problems
+  // It happens that only x86, PPC and SPARC use the 'lib32' variant of oslibdir,
+  // and using that variant while targeting other architectures causes problems
   // because the libraries are laid out in shared system roots that can't cope
   // with a 'lib32' library search path being considered. So we only enable
   // them when we know we may need it.
@@ -195,7 +195,8 @@
   // reasoning about oslibdir spellings with the lib dir spellings in the
   // GCCInstallationDetector, but that is a more significant refactoring.
   if (Triple.getArch() == llvm::Triple::x86 ||
-  Triple.getArch() == llvm::Triple::ppc)
+  Triple.getArch() == llvm::Triple::ppc ||
+  Triple.getArch() == llvm::Triple::sparc)
 return "lib32";
 
   if (Triple.getArch() == llvm::Triple::x86_64 &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D88295: [Sema] Fix volatile check when test if a return object can be implicitly move

2020-10-31 Thread Yang Fan via Phabricator via cfe-commits
nullptr.cpp updated this revision to Diff 302073.
nullptr.cpp added a comment.

move test to `class/class.init/class.copy.elision/p1.cpp`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88295

Files:
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CXX/class/class.init/class.copy.elision/p1.cpp


Index: clang/test/CXX/class/class.init/class.copy.elision/p1.cpp
===
--- /dev/null
+++ clang/test/CXX/class/class.init/class.copy.elision/p1.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -std=c++20 -emit-llvm -triple x86_64-unknown-linux-gnu -o - 
%s | FileCheck %s
+// RUN: %clang_cc1 -std=c++17 -emit-llvm -triple x86_64-unknown-linux-gnu -o - 
%s | FileCheck %s
+// RUN: %clang_cc1 -std=c++14 -emit-llvm -triple x86_64-unknown-linux-gnu -o - 
%s | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm -triple x86_64-unknown-linux-gnu -o - 
%s | FileCheck %s
+
+// - volatile object in return statement don't match the rule for using move
+//   operation instead of copy operation. Thus should call the copy constructor
+//   A(const volatile A &).
+//
+// - volatile object in return statement also don't match the rule for copy
+//   elision. Thus the copy constructor A(const volatile A &) cannot be elided.
+namespace test_volatile {
+class A {
+public:
+  A() {}
+  ~A() {}
+  A(const volatile A &);
+  A(volatile A &&);
+};
+
+A test() {
+  volatile A a_copy;
+  // CHECK: call void @_ZN13test_volatile1AC1ERVKS0_
+  return a_copy;
+}
+} // namespace test_volatile
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3059,12 +3059,13 @@
   // variable will no longer be used.
   if (VD->hasAttr()) return false;
 
+  // ...non-volatile...
+  if (VD->getType().isVolatileQualified())
+return false;
+
   if (CESK & CES_AllowDifferentTypes)
 return true;
 
-  // ...non-volatile...
-  if (VD->getType().isVolatileQualified()) return false;
-
   // Variables with higher required alignment than their type's ABI
   // alignment cannot use NRVO.
   if (!VD->getType()->isDependentType() && VD->hasAttr() &&


Index: clang/test/CXX/class/class.init/class.copy.elision/p1.cpp
===
--- /dev/null
+++ clang/test/CXX/class/class.init/class.copy.elision/p1.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -std=c++20 -emit-llvm -triple x86_64-unknown-linux-gnu -o - %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++17 -emit-llvm -triple x86_64-unknown-linux-gnu -o - %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++14 -emit-llvm -triple x86_64-unknown-linux-gnu -o - %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm -triple x86_64-unknown-linux-gnu -o - %s | FileCheck %s
+
+// - volatile object in return statement don't match the rule for using move
+//   operation instead of copy operation. Thus should call the copy constructor
+//   A(const volatile A &).
+//
+// - volatile object in return statement also don't match the rule for copy
+//   elision. Thus the copy constructor A(const volatile A &) cannot be elided.
+namespace test_volatile {
+class A {
+public:
+  A() {}
+  ~A() {}
+  A(const volatile A &);
+  A(volatile A &&);
+};
+
+A test() {
+  volatile A a_copy;
+  // CHECK: call void @_ZN13test_volatile1AC1ERVKS0_
+  return a_copy;
+}
+} // namespace test_volatile
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3059,12 +3059,13 @@
   // variable will no longer be used.
   if (VD->hasAttr()) return false;
 
+  // ...non-volatile...
+  if (VD->getType().isVolatileQualified())
+return false;
+
   if (CESK & CES_AllowDifferentTypes)
 return true;
 
-  // ...non-volatile...
-  if (VD->getType().isVolatileQualified()) return false;
-
   // Variables with higher required alignment than their type's ABI
   // alignment cannot use NRVO.
   if (!VD->getType()->isDependentType() && VD->hasAttr() &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

2020-10-31 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

Can you rebase this against trunk please, having trouble testing it out locally.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86671

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


[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

2020-10-31 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:441-456
+static IdentifierNamingCheck::HungarianPrefixOption
+parseHungarianPrefix(std::string OptionVal) {
+  for (auto &C : OptionVal)
+C = toupper(C);
+
+  if (std::string::npos != OptionVal.find("LOWER_CASE"))
+return IdentifierNamingCheck::HungarianPrefixOption::HPO_LowerCase;

This isn't really needed if you have the mapping defined, `Options.get` works 
with enums, just look at how CaseType is parsed and stored. If you want to map 
multiple strings to a single enum constant that can also work by putting both 
strings in the mapping.
This method also validates inputs and will print out an error if a user 
supplies a value that can't be converted.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:460
 getNamingStyles(const ClangTidyCheck::OptionsView &Options) {
+  static IdentifierNamingCheck::HungarianNotationOption HNOption;
+  HNOption.clearAll();

This function can be called multiple times per translation unit when looking 
through header files if `GetConfigPerFile` is enabled. Making this static will 
mean that each file thats read could potentially alter other files style 
configuration.
Maybe a smarter way about this is rather than this function returning a vector 
of naming styles, it returns a struct which contains the Hungarian options and 
a vector of the styles. Doing this would probably also mean you don't need to 
store a reference to this in the `NamingStyle`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86671

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


[PATCH] D90526: [clangd] Add -log=public to redact all request info from index server logs

2020-10-31 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
sammccall added reviewers: kadircet, kbobyrev.
Herald added subscribers: cfe-commits, usaxena95, arphaman.
Herald added a project: clang.
sammccall requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

The index server has access to potentially-sensitive information, e.g. a
sequence of fuzzyFind requests reveals details about code completions in the
editor which in turn reveals details about the code being edited.
This information is necessary to provide the service, and our intention[1] is it
should never be retained beyond the scope of the request (e.g. not logged).

At the same time, some log messages should be exposed:

- server startup/index reloads etc that don't pertain to a user request
- basic request logs (method, latency, #results, error code) for monitoring
- errors while handling requests, without request-specific data

The -log=public design accommodates these by allowing three types of logs:

- those not associated with any user RPC request (via context-propagation)
- those explicitly tagged as [public] in the log line
- logging of format strings only, with no interpolated data (error level only)

[1] Specifically: Google is likely to run public instances of this server
for LLVM and potentially other projects, they will run in this configuration.
The details agreed in a Google-internal privacy review.
As clangd developers, we'd encourage others to use this configuration for public
instances too.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90526

Files:
  clang-tools-extra/clangd/index/remote/server/Server.cpp
  clang-tools-extra/clangd/support/Logger.cpp
  clang-tools-extra/clangd/support/Logger.h
  clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp

Index: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -67,7 +67,8 @@
 
 private:
   // Color logs so we can distinguish them from test output.
-  void log(Level L, const llvm::formatv_object_base &Message) override {
+  void log(Level L, const char *Fmt,
+   const llvm::formatv_object_base &Message) override {
 raw_ostream::Colors Color;
 switch (L) {
 case Level::Verbose:
Index: clang-tools-extra/clangd/support/Logger.h
===
--- clang-tools-extra/clangd/support/Logger.h
+++ clang-tools-extra/clangd/support/Logger.h
@@ -24,16 +24,19 @@
 public:
   virtual ~Logger() = default;
 
-  enum Level { Debug, Verbose, Info, Error };
+  /// The significance or severity of this message.
+  /// Typically used to filter the output to an interesting level.
+  enum Level : unsigned char { Debug, Verbose, Info, Error };
   static char indicator(Level L) { return "DVIE"[L]; }
 
   /// Implementations of this method must be thread-safe.
-  virtual void log(Level, const llvm::formatv_object_base &Message) = 0;
+  virtual void log(Level, const char *Fmt,
+   const llvm::formatv_object_base &Message) = 0;
 };
 
 namespace detail {
 const char *debugType(const char *Filename);
-void log(Logger::Level, const llvm::formatv_object_base &);
+void logImpl(Logger::Level, const char *Fmt, const llvm::formatv_object_base &);
 
 // We often want to consume llvm::Errors by value when passing them to log().
 // We automatically wrap them in llvm::fmt_consume() as formatv requires.
@@ -43,7 +46,8 @@
 }
 template 
 void log(Logger::Level L, const char *Fmt, Ts &&... Vals) {
-  detail::log(L, llvm::formatv(Fmt, detail::wrap(std::forward(Vals))...));
+  detail::logImpl(L, Fmt,
+  llvm::formatv(Fmt, detail::wrap(std::forward(Vals))...));
 }
 
 llvm::Error error(std::error_code, std::string &&);
@@ -119,7 +123,8 @@
   : MinLevel(MinLevel), Logs(Logs) {}
 
   /// Write a line to the logging stream.
-  void log(Level, const llvm::formatv_object_base &Message) override;
+  void log(Level, const char *Fmt,
+   const llvm::formatv_object_base &Message) override;
 
 private:
   Logger::Level MinLevel;
Index: clang-tools-extra/clangd/support/Logger.cpp
===
--- clang-tools-extra/clangd/support/Logger.cpp
+++ clang-tools-extra/clangd/support/Logger.cpp
@@ -28,10 +28,10 @@
 
 LoggingSession::~LoggingSession() { L = nullptr; }
 
-void detail::log(Logger::Level Level,
- const llvm::formatv_object_base &Message) {
+void detail::logImpl(Logger::Level Level, const char *Fmt,
+ const llvm::formatv_object_base &Message) {
   if (L)
-L->log(Level, Message);
+L->log(Level, Fmt, Message);
   else {
 static std::mutex Mu;
 std::lock_guard Guard(Mu);
@@ -47,7 +47,7 @@
   return Filename;
 }
 
-void StreamLogger::log(Logger::Level Level,
+void StreamLogger::log(Log

[PATCH] D89849: Add option 'exceptions' to pragma clang fp

2020-10-31 Thread Serge Pavlov 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 rG6021cbea4d34: Add option 'exceptions' to pragma 
clang fp (authored by sepavloff).

Changed prior to commit:
  https://reviews.llvm.org/D89849?vs=299887&id=302078#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89849

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/test/CodeGen/pragma-fp-exc.cpp
  clang/test/Parser/pragma-fp.cpp

Index: clang/test/Parser/pragma-fp.cpp
===
--- clang/test/Parser/pragma-fp.cpp
+++ clang/test/Parser/pragma-fp.cpp
@@ -1,14 +1,14 @@
 // RUN: %clang_cc1 -std=c++11 -verify %s
 
 void test_0(int *List, int Length) {
-/* expected-error@+1 {{missing option; expected 'contract' or 'reassociate'}} */
+/* expected-error@+1 {{missing option; expected 'contract', 'reassociate' or 'exceptions'}} */
 #pragma clang fp
   for (int i = 0; i < Length; i++) {
 List[i] = i;
   }
 }
 void test_1(int *List, int Length) {
-/* expected-error@+1 {{invalid option 'blah'; expected 'contract' or 'reassociate'}} */
+/* expected-error@+1 {{invalid option 'blah'; expected 'contract', 'reassociate' or 'exceptions'}} */
 #pragma clang fp blah
   for (int i = 0; i < Length; i++) {
 List[i] = i;
@@ -62,3 +62,14 @@
 #pragma clang fp contract(fast)
   }
 }
+
+void test_9(float *dest, float a, float b) {
+/* expected-error@+1 {{unexpected argument 'on' to '#pragma clang fp exceptions'; expected 'ignore', 'maytrap' or 'strict'}} */
+#pragma clang fp exceptions(on)
+  *dest = a + b;
+}
+
+void test_10(float *dest, float a, float b) {
+#pragma clang fp exceptions(maytrap) contract(fast) reassociate(on)
+  *dest = a + b;
+}
Index: clang/test/CodeGen/pragma-fp-exc.cpp
===
--- /dev/null
+++ clang/test/CodeGen/pragma-fp-exc.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -o - %s | FileCheck --check-prefix=CHECK-DEF %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -ffp-exception-behavior=strict -emit-llvm -o - %s | FileCheck --check-prefix=CHECK-STRICT %s
+
+float func_01(float x, float y, float z) {
+  float res = x + y;
+  {
+#pragma clang fp exceptions(maytrap)
+res += z;
+  }
+  return res;
+}
+// CHECK-DEF-LABEL: @_Z7func_01fff
+// CHECK-DEF:   call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.ignore")
+// CHECK-DEF:   call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
+
+// CHECK-STRICT-LABEL: @_Z7func_01fff
+// CHECK-STRICT:   call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+// CHECK-STRICT:   call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -1022,6 +1022,11 @@
   CurFPFeatures = NewFPFeatures.applyOverrides(LO);
 }
 
+void Sema::ActOnPragmaFPExceptions(SourceLocation Loc,
+   LangOptions::FPExceptionModeKind FPE) {
+  setExceptionMode(Loc, FPE);
+}
+
 void Sema::PushNamespaceVisibilityAttr(const VisibilityAttr *Attr,
SourceLocation Loc) {
   // Visibility calculations will consider the namespace's visibility.
Index: clang/lib/Parse/ParsePragma.cpp
===
--- clang/lib/Parse/ParsePragma.cpp
+++ clang/lib/Parse/ParsePragma.cpp
@@ -2858,11 +2858,12 @@
 namespace {
 /// Used as the annotation value for tok::annot_pragma_fp.
 struct TokFPAnnotValue {
-  enum FlagKinds { Contract, Reassociate };
+  enum FlagKinds { Contract, Reassociate, Exceptions };
   enum FlagValues { On, Off, Fast };
 
-  FlagKinds FlagKind;
-  FlagValues FlagValue;
+  llvm::Optional ContractValue;
+  llvm::Optional ReassociateValue;
+  llvm::Optional ExceptionsValue;
 };
 } // end anonymous namespace
 
@@ -2879,6 +2880,7 @@
 return;
   }
 
+  auto *AnnotValue = new (PP.getPreprocessorAllocator()) TokFPAnnotValue;
   while (Tok.is(tok::identifier)) {
 IdentifierInfo *OptionInfo = Tok.getIdentifierInfo();
 
@@ -2887,6 +2889,7 @@
 OptionInfo->getName())
 .Case("contract", TokFPAnnotValue::Contract)
 .Case("reassociate", TokFPAnnotValue::Reassociate)
+.Case("exceptions", TokFPAnnotValue::Except

[clang] 6021cbe - Add option 'exceptions' to pragma clang fp

2020-10-31 Thread Serge Pavlov via cfe-commits

Author: Serge Pavlov
Date: 2020-10-31T17:36:12+07:00
New Revision: 6021cbea4d346d8ff059f8dd74ba7d520646be03

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

LOG: Add option 'exceptions' to pragma clang fp

Pragma 'clang fp' is extended to support a new option, 'exceptions'. It
allows to specify floating point exception behavior more flexibly.

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

Added: 
clang/test/CodeGen/pragma-fp-exc.cpp

Modified: 
clang/docs/LanguageExtensions.rst
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Parse/ParsePragma.cpp
clang/lib/Sema/SemaAttr.cpp
clang/test/Parser/pragma-fp.cpp

Removed: 




diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index a90485d9f799..e17e4e1c46a7 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -3182,7 +3182,7 @@ The pragma can take two values: ``on`` and ``off``.
   float f(float x, float y, float z)
   {
 // Enable floating point reassociation across statements
-#pragma fp reassociate(on)
+#pragma clang fp reassociate(on)
 float t = x + y;
 float v = t + z;
   }
@@ -3211,6 +3211,31 @@ The pragma can also be used with ``off`` which turns FP 
contraction off for a
 section of the code. This can be useful when fast contraction is otherwise
 enabled for the translation unit with the ``-ffp-contract=fast`` flag.
 
+
+``#pragma clang fp exceptions`` specifies floating point exception behavior. It
+may take one the the values: ``ignore``, ``maytrap`` or ``strict``. Meaning of
+these values is same as for `constrained floating point intrinsics 
`_.
+
+.. code-block:: c++
+
+  {
+// Preserve floating point exceptions
+#pragma clang fp exceptions(strict)
+z = x + y;
+if (fetestexcept(FE_OVERFLOW))
+ ...
+  }
+
+A ``#pragma clang fp`` pragma may contain any number of options:
+
+.. code-block:: c++
+
+  void func(float *dest, float a, float b) {
+#pragma clang fp exceptions(maytrap) contract(fast) reassociate(on)
+...
+  }
+
+
 The ``#pragma float_control`` pragma allows precise floating-point
 semantics and floating-point exception behavior to be specified
 for a section of the source code. This pragma can only appear at file scope or

diff  --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 0fd4eb5323de..c0a5a2b4c144 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1388,12 +1388,13 @@ def err_pragma_loop_invalid_option : Error<
   "pipeline, pipeline_initiation_interval, vectorize_predicate, or 
distribute">;
 
 def err_pragma_fp_invalid_option : Error<
-  "%select{invalid|missing}0 option%select{ %1|}0; expected 'contract' or 
'reassociate'">;
+  "%select{invalid|missing}0 option%select{ %1|}0; expected 'contract', 
'reassociate' or 'exceptions'">;
 def err_pragma_fp_invalid_argument : Error<
-  "unexpected argument '%0' to '#pragma clang fp %1'; "
+  "unexpected argument '%0' to '#pragma clang fp %1'; expected "
   "%select{"
-  "expected 'fast' or 'on' or 'off'|"
-  "expected 'on' or 'off'}2">;
+  "'fast' or 'on' or 'off'|"
+  "'on' or 'off'|"
+  "'ignore', 'maytrap' or 'strict'}2">;
 
 def err_pragma_invalid_keyword : Error<
   "invalid argument; expected 'enable'%select{|, 'full'}0%select{|, 
'assume_safety'}1 or 'disable'">;

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 4be3c23652e1..3afd417bb2a2 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -9896,6 +9896,10 @@ class Sema final {
   /// \#pragma STDC FENV_ACCESS
   void ActOnPragmaFEnvAccess(SourceLocation Loc, bool IsEnabled);
 
+  /// Called on well formed '\#pragma clang fp' that has option 'exceptions'.
+  void ActOnPragmaFPExceptions(SourceLocation Loc,
+   LangOptions::FPExceptionModeKind);
+
   /// Called to set constant rounding mode for floating point operations.
   void setRoundingMode(SourceLocation Loc, llvm::RoundingMode);
 

diff  --git a/clang/lib/Parse/ParsePragma.cpp b/clang/lib/Parse/ParsePragma.cpp
index 278e6f50deba..64de2cf5d7f1 100644
--- a/clang/lib/Parse/ParsePragma.cpp
+++ b/clang/lib/Parse/ParsePragma.cpp
@@ -2858,11 +2858,12 @@ void PragmaOptimizeHandler::HandlePragma(Preprocessor 
&PP,
 namespace {
 /// Used as the annotation value for tok::annot_pragma_fp.
 struct TokFPAnnotValue {
-  enum FlagKinds { Contract, Reassociate };
+  enum FlagKinds { Contract, Reassociate, Exceptions };
   enum FlagValues { On, Off, Fast };
 
-  FlagKi

[PATCH] D90526: [clangd] Add -log=public to redact all request info from index server logs

2020-10-31 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

Couple of notes on this patch.

There's no actual request logging here yet. My plan is something like `[public] 
request v1/Relations => SUCCESS: 3 results in 3ms`. Wanted to keep the patch a 
reasonable size.

There's also no testing, testing is definitely important here. I want to build 
something on top of Kirill's python tests once that lands. Probably in the 
patch that adds request logging.

The `[public]` thing is gross, but I think worse is better here.
I went through lots of iterations, including with explicit "Sensitivity" enum 
passed around, but it was pretty invasive. 
Moreover explicitly supporting it in `Logger.h` made me feel like all the 
callsites in all our libraries should consider whether info is public or not. I 
think spreading that cognitive load around the whole project is a net negative.
With the request-context detection (which was a last-minute realization 
actually), maybe we could even get away without `[public]`, but I didn't want 
to commit to contorting our code so that the request logging happens outside 
the request scope. Happy to drop this part if you think we can make it work.

Weird extension idea: we could have a completely synthetic request ID to stick 
on the request log lines and also the pattern-only elog()s to tie them together.

Changing the Logger API is a bit annoying, but has one side-benefit: error 
count grouped by format string is a nice thing to be able to monitor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90526

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


[PATCH] D89481: [scan-build] Fix clang++ pathname again

2020-10-31 Thread Sylvestre Ledru via Phabricator via cfe-commits
sylvestre.ledru accepted this revision.
sylvestre.ledru added a comment.
This revision is now accepted and ready to land.

Looks good to me


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

https://reviews.llvm.org/D89481

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


[PATCH] D88084: [clang-format] Changed default styles BraceWrappping bool table to directly use variables

2020-10-31 Thread Sylvestre Ledru via Phabricator via cfe-commits
sylvestre.ledru accepted this revision.
sylvestre.ledru added a comment.
This revision is now accepted and ready to land.

Much better


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88084

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


[PATCH] D90518: [clangd] Make tests depend on Clang

2020-10-31 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

This would be pretty unfortunate for iteration times: in my workspace having 
just run `ninja check-clangd`, `ninja clang` has 1450 additional build steps 
remaining. (All of which would get frequently invalidated e.g. by changes to 
common LLVM headers).

`clangd/test/lit.cfg.py` mentions clang, our intent here is configure some 
common required variables etc, not depend clang itself. It's not very 
well-defined though :-(
I'm pretty sure it used to work at some point, but now after a clean:

  llvm-lit: 
/usr/local/google/home/sammccall/src/llvm-project/llvm/utils/lit/lit/llvm/config.py:342:
 fatal: couldn't find 'clang' program, try setting CLANG in your environment

Looking at the contents of llvm/config.py, it's getting the builtin includes 
directory in order to use it in lit substitutions like %clang_cc1. Which of 
course we don't use in clangd tests.
I'm trying to remember exactly why we do need to use this module at all, but I 
suspect there was a real reason.

I think we're going to need to refactor llvm/config.py a little to let us take 
the bits we want. I can have a dig into this soon...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90518

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


[PATCH] D78807: Fix gendered documentation

2020-10-31 Thread Sylvestre Ledru via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG43e451f9f316: Fix gendered documentation (authored by 
pedro.gonnet, committed by sylvestre.ledru).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78807

Files:
  
clang-tools-extra/docs/clang-tidy/checks/google-objc-global-variable-declaration.rst
  lldb/docs/use/variable.rst


Index: lldb/docs/use/variable.rst
===
--- lldb/docs/use/variable.rst
+++ lldb/docs/use/variable.rst
@@ -993,7 +993,7 @@
 user to see.
 
 A filter will solve this issue by only letting the user see those member
-variables he cares about. Of course, the equivalent of a filter can be
+variables they care about. Of course, the equivalent of a filter can be
 implemented easily using synthetic children, but a filter lets you get the job
 done without having to write Python code.
 
Index: 
clang-tools-extra/docs/clang-tidy/checks/google-objc-global-variable-declaration.rst
===
--- 
clang-tools-extra/docs/clang-tidy/checks/google-objc-global-variable-declaration.rst
+++ 
clang-tools-extra/docs/clang-tidy/checks/google-objc-global-variable-declaration.rst
@@ -44,4 +44,4 @@
   static NSString* __anotherString = @"world";
 
 The check will give a warning message but will not be able to suggest a fix. 
The
-user need to fix it on his own.
+user needs to fix it on their own.


Index: lldb/docs/use/variable.rst
===
--- lldb/docs/use/variable.rst
+++ lldb/docs/use/variable.rst
@@ -993,7 +993,7 @@
 user to see.
 
 A filter will solve this issue by only letting the user see those member
-variables he cares about. Of course, the equivalent of a filter can be
+variables they care about. Of course, the equivalent of a filter can be
 implemented easily using synthetic children, but a filter lets you get the job
 done without having to write Python code.
 
Index: clang-tools-extra/docs/clang-tidy/checks/google-objc-global-variable-declaration.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/google-objc-global-variable-declaration.rst
+++ clang-tools-extra/docs/clang-tidy/checks/google-objc-global-variable-declaration.rst
@@ -44,4 +44,4 @@
   static NSString* __anotherString = @"world";
 
 The check will give a warning message but will not be able to suggest a fix. The
-user need to fix it on his own.
+user needs to fix it on their own.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] 43e451f - Fix gendered documentation

2020-10-31 Thread Sylvestre Ledru via cfe-commits

Author: Pedro Gonnet
Date: 2020-10-31T12:17:19+01:00
New Revision: 43e451f9f316fd224d88899d45313a73ff11291e

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

LOG: Fix gendered documentation

Changed two references to developers as "he" or "him" to the more neutral 
"they".

Reviewed By: JDevlieghere, sylvestre.ledru

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

Added: 


Modified: 

clang-tools-extra/docs/clang-tidy/checks/google-objc-global-variable-declaration.rst
lldb/docs/use/variable.rst

Removed: 




diff  --git 
a/clang-tools-extra/docs/clang-tidy/checks/google-objc-global-variable-declaration.rst
 
b/clang-tools-extra/docs/clang-tidy/checks/google-objc-global-variable-declaration.rst
index 15b59996e3d3..84c97767f18f 100644
--- 
a/clang-tools-extra/docs/clang-tidy/checks/google-objc-global-variable-declaration.rst
+++ 
b/clang-tools-extra/docs/clang-tidy/checks/google-objc-global-variable-declaration.rst
@@ -44,4 +44,4 @@ However for code that prefixed with non-alphabetical 
characters like:
   static NSString* __anotherString = @"world";
 
 The check will give a warning message but will not be able to suggest a fix. 
The
-user need to fix it on his own.
+user needs to fix it on their own.

diff  --git a/lldb/docs/use/variable.rst b/lldb/docs/use/variable.rst
index 5b1fef7b06c3..ae6dbf2f4e5e 100644
--- a/lldb/docs/use/variable.rst
+++ b/lldb/docs/use/variable.rst
@@ -993,7 +993,7 @@ have many member variables but not all of these are 
actually necessary for the
 user to see.
 
 A filter will solve this issue by only letting the user see those member
-variables he cares about. Of course, the equivalent of a filter can be
+variables they care about. Of course, the equivalent of a filter can be
 implemented easily using synthetic children, but a filter lets you get the job
 done without having to write Python code.
 



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


[PATCH] D90528: [clangd] Fix check-clangd with no clang built

2020-10-31 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
sammccall added reviewers: smeenai, hokein.
Herald added subscribers: llvm-commits, cfe-commits, usaxena95, kadircet, 
arphaman, delcypher.
Herald added projects: clang, LLVM.
sammccall requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

- pass required=False to use_clang(), as we don't need it
- fix required=False (which was unused and rotted):
  - make derived substitutions conditional on it
  - add a feature so we can disable tests that need it
- conditionally disable our one test that depends on %resource_dir. This 
doesn't seem right from first principles, but isn't a big deal.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90528

Files:
  clang-tools-extra/clangd/test/document-link.test
  clang-tools-extra/clangd/test/lit.cfg.py
  llvm/utils/lit/lit/llvm/config.py


Index: llvm/utils/lit/lit/llvm/config.py
===
--- llvm/utils/lit/lit/llvm/config.py
+++ llvm/utils/lit/lit/llvm/config.py
@@ -396,11 +396,6 @@
 
 self.with_environment('LD_LIBRARY_PATH', paths, append_path=True)
 
-# Discover the 'clang' and 'clangcc' to use.
-
-self.config.clang = self.use_llvm_tool(
-'clang', search_env='CLANG', required=required)
-
 shl = getattr(self.config, 'llvm_shlib_dir', None)
 pext = getattr(self.config, 'llvm_plugin_ext', None)
 if shl:
@@ -408,23 +403,28 @@
 if pext:
 self.config.substitutions.append(('%pluginext', pext))
 
-builtin_include_dir = 
self.get_clang_builtin_include_dir(self.config.clang)
-tool_substitutions = [
-ToolSubst('%clang', command=self.config.clang, 
extra_args=additional_flags),
-ToolSubst('%clang_analyze_cc1', command='%clang_cc1', 
extra_args=['-analyze', '%analyze', '-setup-static-analyzer']+additional_flags),
-ToolSubst('%clang_cc1', command=self.config.clang, 
extra_args=['-cc1', '-internal-isystem', builtin_include_dir, 
'-nostdsysteminc']+additional_flags),
-ToolSubst('%clang_cpp', command=self.config.clang, 
extra_args=['--driver-mode=cpp']+additional_flags),
-ToolSubst('%clang_cl', command=self.config.clang, 
extra_args=['--driver-mode=cl']+additional_flags),
-ToolSubst('%clangxx', command=self.config.clang, 
extra_args=['--driver-mode=g++']+additional_flags),
-]
-self.add_tool_substitutions(tool_substitutions)
+# Discover the 'clang' and 'clangcc' to use.
+self.config.clang = self.use_llvm_tool(
+'clang', search_env='CLANG', required=required)
+if self.config.clang:
+  features.add('clang')
+  builtin_include_dir = 
self.get_clang_builtin_include_dir(self.config.clang)
+  tool_substitutions = [
+  ToolSubst('%clang', command=self.config.clang, 
extra_args=additional_flags),
+  ToolSubst('%clang_analyze_cc1', command='%clang_cc1', 
extra_args=['-analyze', '%analyze', '-setup-static-analyzer']+additional_flags),
+  ToolSubst('%clang_cc1', command=self.config.clang, 
extra_args=['-cc1', '-internal-isystem', builtin_include_dir, 
'-nostdsysteminc']+additional_flags),
+  ToolSubst('%clang_cpp', command=self.config.clang, 
extra_args=['--driver-mode=cpp']+additional_flags),
+  ToolSubst('%clang_cl', command=self.config.clang, 
extra_args=['--driver-mode=cl']+additional_flags),
+  ToolSubst('%clangxx', command=self.config.clang, 
extra_args=['--driver-mode=g++']+additional_flags),
+  ]
+  self.add_tool_substitutions(tool_substitutions)
+  self.config.substitutions.append(
+  ('%resource_dir', builtin_include_dir))
 
 self.config.substitutions.append(('%itanium_abi_triple',
   
self.make_itanium_abi_triple(self.config.target_triple)))
 self.config.substitutions.append(('%ms_abi_triple',
   
self.make_msabi_triple(self.config.target_triple)))
-self.config.substitutions.append(
-('%resource_dir', builtin_include_dir))
 
 # The host triple might not be set, at least if we're compiling clang 
from
 # an already installed llvm.
Index: clang-tools-extra/clangd/test/lit.cfg.py
===
--- clang-tools-extra/clangd/test/lit.cfg.py
+++ clang-tools-extra/clangd/test/lit.cfg.py
@@ -1,7 +1,7 @@
 import lit.llvm
 
 lit.llvm.initialize(lit_config, config)
-lit.llvm.llvm_config.use_clang()
+lit.llvm.llvm_config.use_clang([], [], required=False)
 
 config.name = 'Clangd'
 config.suffixes = ['.test']
Index: clang-tools-extra/clangd/test/document-link.test
===
--- clang-tools-extra/clangd/test/document-link.test
+++ clang-tools-extra/clangd/test/documen

[PATCH] D90518: [clangd] Make tests depend on Clang

2020-10-31 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a subscriber: kbobyrev.
sammccall added a comment.

D90528  should solve this in a cheaper way. 
Thanks for raising this though!

@kbobyrev @kadircet we were thinking about adding a buildbot with grpc/remote 
index enabled. Maybe we should have that one build check-clangd only, and not 
clang?
It would be a little faster and it would ensure this configuration doesn't 
break in future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90518

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


[PATCH] D88609: Use uint64_t for branch weights instead of uint32_t

2020-10-31 Thread dmajor via Phabricator via cfe-commits
dmajor added a comment.

After 10f2a0d662d8 
 our 
self-host PGO builds are hitting:
llvm::BranchProbability::getBranchProbability(uint64_t, uint64_t): Assertion 
`Numerator <= Denominator && "Probability cannot be bigger than 1!"' failed.

https://treeherder.mozilla.org/logviewer?job_id=320324631&repo=try&lineNumber=52597
 has the log as well as the .cpp/.sh reproducers, but I assume you'd also need 
the merged.profdata file, and our bots haven't been taught to save those. I can 
try to add that ability next week if needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88609

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


[PATCH] D88609: Use uint64_t for branch weights instead of uint32_t

2020-10-31 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

The stack trace helped me come up with my own repro, reverting.
It's more uint64_t overflow issues. This might be the reason branch_weights 
were initially i32 with casts in a bunch of places around LLVM. Not sure if I 
should try and fix all usages or just keep branch_weights as i32.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88609

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


[clang] 5963e02 - Temporarily remove test CodeGen/pragma-fp-exc

2020-10-31 Thread Serge Pavlov via cfe-commits

Author: Serge Pavlov
Date: 2020-10-31T19:48:44+07:00
New Revision: 5963e028e7145d68b70be128d13e26a02095a0ad

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

LOG: Temporarily remove test CodeGen/pragma-fp-exc

This test fails on buildbots where CPU architecture does not fully
support constrained intrinsics.

Added: 


Modified: 


Removed: 
clang/test/CodeGen/pragma-fp-exc.cpp



diff  --git a/clang/test/CodeGen/pragma-fp-exc.cpp 
b/clang/test/CodeGen/pragma-fp-exc.cpp
deleted file mode 100644
index c51b7e63204c..
--- a/clang/test/CodeGen/pragma-fp-exc.cpp
+++ /dev/null
@@ -1,18 +0,0 @@
-// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -o - %s | FileCheck 
--check-prefix=CHECK-DEF %s
-// RUN: %clang_cc1 -triple %itanium_abi_triple -ffp-exception-behavior=strict 
-emit-llvm -o - %s | FileCheck --check-prefix=CHECK-STRICT %s
-
-float func_01(float x, float y, float z) {
-  float res = x + y;
-  {
-#pragma clang fp exceptions(maytrap)
-res += z;
-  }
-  return res;
-}
-// CHECK-DEF-LABEL: @_Z7func_01fff
-// CHECK-DEF:   call float @llvm.experimental.constrained.fadd.f32(float 
{{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.ignore")
-// CHECK-DEF:   call float @llvm.experimental.constrained.fadd.f32(float 
{{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
-
-// CHECK-STRICT-LABEL: @_Z7func_01fff
-// CHECK-STRICT:   call float 
@llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata 
!"round.tonearest", metadata !"fpexcept.strict")
-// CHECK-STRICT:   call float 
@llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata 
!"round.tonearest", metadata !"fpexcept.maytrap")



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


[PATCH] D90109: [clang-tidy] Use ANSI escape codes for --use-color on Windows

2020-10-31 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM! Do you need someone to commit on your behalf?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90109

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


[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

2020-10-31 Thread Tom Rix via Phabricator via cfe-commits
trixirt marked 2 inline comments as done.
trixirt added a comment.

I am trying to address problems treewide so around 100 changes per fix.
Doing that requires consensus that the fix is generally ok for every subsystem 
in the code base.
So while clang will fix
switch () {} ; 
it will also fix
for () {

  // tbd : add something here
  ;

}
consensus needs to be found on as narrow a fix as possible.
so there will be a specialized fixer for each problem consensus is reached on.

As for leaving whitespace fixing as an exercise for the user.
No change will be accepted with a whitespace problem.
When there are 100 fixes, there are 100 files to open, find the line, fix the 
line.
This is slow and error prone.

Another slow part is manually commit and submitting the fixes and doing a 
review cycle.
But if your fixer can automagically do everything you can  hook it into a c/i 
and and
it will keep the codebase free of its set of fixers while we all do something 
more 
interesting.

i am working on the kernel build to do this now.
It would be helpful if this and future fixers could live in clang-tidy's 
linuxkernel/
which is already part of the kernel build so i don't have to reinvent how to 
find them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90180

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


[PATCH] D90109: [clang-tidy] Use ANSI escape codes for --use-color on Windows

2020-10-31 Thread David Sanders via Phabricator via cfe-commits
dsanders11 added a comment.

In D90109#2366415 , @aaron.ballman 
wrote:

> LGTM! Do you need someone to commit on your behalf?

Yea, I do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90109

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


[PATCH] D90246: [clang-format] Improve BAS_DontAlign+AllowAllArgumentsOnNextLine=false

2020-10-31 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

This LGTM actually, and something that's been annoying me for ages that it 
breaks like this


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90246

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


[PATCH] D90121: clang-format: Add a consumer to diagnostics engine

2020-10-31 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

@thakis didn't you and I go through this process of removing frontEnd once 
before? do we really want to add this back in for all the reasons you said 
before?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90121

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


[PATCH] D90121: clang-format: Add a consumer to diagnostics engine

2020-10-31 Thread Kirill Dmitrenko via Phabricator via cfe-commits
dmikis added a comment.

@MyDeveloperDay That change introduced a regression: clang-format crashes 
instead of reporting errors in certain situations. See my comment with a test 
case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90121

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


[PATCH] D90109: [clang-tidy] Use ANSI escape codes for --use-color on Windows

2020-10-31 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision.
aaron.ballman added a comment.

In D90109#2366420 , @dsanders11 wrote:

> In D90109#2366415 , @aaron.ballman 
> wrote:
>
>> LGTM! Do you need someone to commit on your behalf?
>
> Yea, I do.

I've commit on your behalf in d915d403d7410b17e8ac52f1f435859713a7b354 
, thank 
you for the patch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90109

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


[PATCH] D90531: [clangd][WIP] Start implementing clang-tidy options into clangd config

2020-10-31 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision.
njames93 added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, mgorny.
Herald added a project: clang.
njames93 requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

First step of implementing clang-tidy configuration into clangd config.
This is just getting a few of the basics working, like reading the 
configuration and a simple implementation to get clang-tidy to use those values.
However right now its achieving the latter in a much less than ideal way so 
definitely need to clean that up.
Also need to add some tests here and there also.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90531

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigTidyProvider.cpp
  clang-tools-extra/clangd/ConfigTidyProvider.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp

Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -8,6 +8,7 @@
 
 #include "ClangdLSPServer.h"
 #include "CodeComplete.h"
+#include "ConfigTidyProvider.h"
 #include "Features.inc"
 #include "PathMapping.h"
 #include "Protocol.h"
@@ -811,10 +812,18 @@
 tidy::ClangTidyOptions OverrideClangTidyOptions;
 if (!ClangTidyChecks.empty())
   OverrideClangTidyOptions.Checks = ClangTidyChecks;
-ClangTidyOptProvider = std::make_unique(
-tidy::ClangTidyGlobalOptions(),
-/* Default */ EmptyDefaults,
-/* Override */ OverrideClangTidyOptions, TFS.view(/*CWD=*/llvm::None));
+if (Opts.ConfigProvider)
+  ClangTidyOptProvider = std::make_unique(
+  tidy::ClangTidyGlobalOptions(),
+  /* Default */ EmptyDefaults, *Opts.ConfigProvider,
+  /* Override */ OverrideClangTidyOptions,
+  TFS.view(/*CWD=*/llvm::None));
+else
+  ClangTidyOptProvider = std::make_unique(
+  tidy::ClangTidyGlobalOptions(),
+  /* Default */ EmptyDefaults,
+  /* Override */ OverrideClangTidyOptions,
+  TFS.view(/*CWD=*/llvm::None));
 Opts.GetClangTidyOptions = [&](llvm::vfs::FileSystem &,
llvm::StringRef File) {
   // This function must be thread-safe and tidy option providers are not.
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -40,6 +40,7 @@
 Dict.handle("CompileFlags", [&](Node &N) { parse(F.CompileFlags, N); });
 Dict.handle("Index", [&](Node &N) { parse(F.Index, N); });
 Dict.handle("Style", [&](Node &N) { parse(F.Style, N); });
+Dict.handle("Tidy", [&](Node &N) { parse(F.Tidy, N); });
 Dict.parse(N);
 return !(N.failed() || HadError);
   }
@@ -82,6 +83,51 @@
 Dict.parse(N);
   }
 
+  void parse(Fragment::TidyBlock &F, Node &N) {
+DictParser Dict("Tidy", this);
+Dict.handle("Enable", [&](Node &N) {
+  if (auto Value = scalarBool(N, "Enable"))
+F.Enable = *Value;
+});
+Dict.handle("Add", [&](Node &N) {
+  if (auto Values = scalarValues(N))
+F.Add = std::move(*Values);
+});
+Dict.handle("Remove", [&](Node &N) {
+  if (auto Values = scalarValues(N))
+F.Remove = std::move(*Values);
+});
+Dict.handle("CheckOptions", [&](Node &N) {
+  if (N.getType() != Node::NK_Mapping) {
+this->error("CheckOptions should be a dictionary", N);
+return;
+  }
+  llvm::SmallSet Seen;
+
+  for (auto &KV : llvm::cast(N)) {
+auto *K = KV.getKey();
+if (!K) // YAMLParser emitted an error.
+  continue;
+auto Key = this->scalarValue(*K, "CheckOption Key");
+if (!Key)
+  continue;
+if (!Seen.insert(**Key).second) {
+  this->warning("Duplicate CheckOption " + **Key + " is ignored", *K);
+  continue;
+}
+auto *V = KV.getValue();
+if (!V) // YAMLParser emitted an error.
+  continue;
+auto Value = this->scalarValue(*V, **Key);
+if (!Value) // Couldn't parse V as a String
+  continue;
+F.CheckOptions.emplace_back(
+std::make_pair(std::move(**Key), std::move(**Value)),
+N.getSourceRange());
+  }
+});
+  }
+
   void parse(Fragment::IndexBlock &F, Node &N) {
 DictParser Dict("Index", this);
 Dict.handle("Background",
@@ -157,6 +203,29 @@
 }
   };
 
+  // Try to parse a single boolean value from the node, warn on failure.
+  llvm::Optional> scalarBool(Node &N, llvm::StringRef Desc) {
+llvm::SmallString<256> Buf;
+llvm::StringR

[clang-tools-extra] d915d40 - Use ANSI escape codes for --use-color on Windows

2020-10-31 Thread Aaron Ballman via cfe-commits

Author: David Sanders
Date: 2020-10-31T10:36:42-04:00
New Revision: d915d403d7410b17e8ac52f1f435859713a7b354

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

LOG: Use ANSI escape codes for --use-color on Windows

On Windows the --use-color option cannot be used for its originally
intended purpose of forcing color when piping stdout, since Windows
does not use ANSI escape codes by default. This change turns on ANSI
escape codes on Windows when forcing color to a non-displayed stdout
(e.g. piped).

Added: 


Modified: 
clang-tools-extra/clang-tidy/ClangTidy.cpp
clang-tools-extra/test/clang-tidy/infrastructure/use-color.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp 
b/clang-tools-extra/clang-tidy/ClangTidy.cpp
index 1f94ab4977c2..b5f2a1c0fbdb 100644
--- a/clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -110,6 +110,9 @@ class ErrorReporter {
 DiagOpts->ShowColors = Context.getOptions().UseColor.getValueOr(
 llvm::sys::Process::StandardOutHasColors());
 DiagPrinter->BeginSourceFile(LangOpts);
+if (DiagOpts->ShowColors && !llvm::sys::Process::StandardOutIsDisplayed()) 
{
+  llvm::sys::Process::UseANSIEscapeCodes(true);
+}
   }
 
   SourceManager &getSourceManager() { return SourceMgr; }

diff  --git a/clang-tools-extra/test/clang-tidy/infrastructure/use-color.cpp 
b/clang-tools-extra/test/clang-tidy/infrastructure/use-color.cpp
index 5f0e3509d008..ace70a4a051e 100644
--- a/clang-tools-extra/test/clang-tidy/infrastructure/use-color.cpp
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/use-color.cpp
@@ -5,7 +5,6 @@
 // RUN: clang-tidy -config='UseColor: false' -dump-config | FileCheck 
-check-prefix=CHECK-CONFIG-NO-COLOR %s
 // RUN: clang-tidy -help | FileCheck -check-prefix=CHECK-OPT-PRESENT %s
 
-// REQUIRES: ansi-escape-sequences
 // RUN: clang-tidy -checks='-*, modernize-use-override' -extra-arg=-std=c++11 
-use-color=false %s | FileCheck -check-prefix=CHECK-NO-COLOR %s
 // RUN: clang-tidy -checks='-*, modernize-use-override' -extra-arg=-std=c++11 
%s | FileCheck -check-prefix=CHECK-NO-COLOR %s
 // RUN: clang-tidy -checks='-*, modernize-use-override' -extra-arg=-std=c++11 
-use-color %s | FileCheck -check-prefix=CHECK-COLOR %s



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


[PATCH] D90531: [clangd][WIP] Start implementing clang-tidy options into clangd config

2020-10-31 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments.



Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:236-237
+StringRef Type(IsPositive ? "Add" : "Remove");
+// Don't support negating here, its handled if the item is in the Add or
+// Remove list.
+if (Str[0] == '-') {

Is this the direction we want to go, the first draft paper on this seems to 
suggest so.



Comment at: clang-tools-extra/clangd/ConfigTidyProvider.cpp:38-41
+  config::Params P;
+  P.Path = AbsoluteFilePath;
+  auto IgnoreDiag = [](const llvm::SMDiagnostic &) {};
+  Config C = ConfigProvider.getConfig(P, IgnoreDiag);

This definitely isn't right, but I'm unsure of the correct way to get the 
config for the specified file.
Is it better to not have this class share the interface of 
ClangTidyOptionsProvider and instead have it take an interface that takes a 
Config and a Filename and works from there instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90531

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


[PATCH] D90533: [clang-format] Always consider option PenaltyBreakBeforeFirstCallParameter

2020-10-31 Thread Mark Nauwelaerts via Phabricator via cfe-commits
mnauw created this revision.
mnauw added a reviewer: sammccall.
mnauw added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
mnauw requested review of this revision.

If AlignAfterOpenBracket is set to BAS_DontAlign, then it turns out that 
PenaltyBreakBeforeFirstCallParameter is not considered (due to hard-coded 
shortcut, so to speak).

This patch attempts to preserve the current (specially crafted) behavior, while 
considering PenaltyBreakBeforeFirstCallParameter when really specified.  There 
may also be other ways to go about this to allow the user to actually set and 
use PenaltyBreakBeforeFirstCallParameter.  Failing all such options, it might 
otherwise be documented that PenaltyBreakBeforeFirstCallParameter has no effect 
in BAS_DontAlign.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90533

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4478,6 +4478,7 @@
   Style.TabWidth = 4;
   Style.UseTab = FormatStyle::UT_Always;
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.PenaltyBreakBeforeFirstCallParameter = 0;
   Style.AlignOperands = FormatStyle::OAS_DontAlign;
   EXPECT_EQ("return someVeryVeryLongConditionThatBarelyFitsOnALine\n"
 "\t&& (someOtherLongishConditionPart1\n"
@@ -4628,6 +4629,7 @@
Style);
 
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.PenaltyBreakBeforeFirstCallParameter = 0;
   verifyFormat("return (a > b\n"
"// comment1\n"
"// comment2\n"
@@ -6028,6 +6030,7 @@
   " a));");
   FormatStyle Style = getLLVMStyle();
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.PenaltyBreakBeforeFirstCallParameter = 0;
   verifyFormat("void aa(\n"
"aaa , a aaa) {}",
Style);
@@ -6106,11 +6109,13 @@
"  bb);",
Style);
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.PenaltyBreakBeforeFirstCallParameter = 0;
   Style.AlignOperands = FormatStyle::OAS_Align;
   verifyFormat("int a = f(aa &&\n"
"  bb);",
Style);
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.PenaltyBreakBeforeFirstCallParameter = 0;
   Style.AlignOperands = FormatStyle::OAS_DontAlign;
   verifyFormat("int a = f(aa &&\n"
"bb);",
@@ -7460,6 +7465,7 @@
 TEST_F(FormatTest, WrapsTemplateParameters) {
   FormatStyle Style = getLLVMStyle();
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.PenaltyBreakBeforeFirstCallParameter = 0;
   Style.BreakBeforeBinaryOperators = FormatStyle::BOS_None;
   verifyFormat(
   "template  struct q {};\n"
@@ -7468,6 +7474,7 @@
   "y;",
   Style);
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.PenaltyBreakBeforeFirstCallParameter = 0;
   Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
   verifyFormat(
   "template  struct r {};\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2740,7 +2740,7 @@
 return 100;
   if (Left.opensScope()) {
 if (Style.AlignAfterOpenBracket == FormatStyle::BAS_DontAlign)
-  return 0;
+  return Style.PenaltyBreakBeforeFirstCallParameter;
 if (Left.is(tok::l_brace) && !Style.Cpp11BracedListStyle)
   return 19;
 return Left.ParameterCount > 1 ? Style.PenaltyBreakBeforeFirstCallParameter
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -579,6 +579,13 @@
 IO.mapOptional("ObjCSpaceBeforeProtocolList",
Style.ObjCSpaceBeforeProtocolList);
 IO.mapOptional("PenaltyBreakAssignment", Style.PenaltyBreakAssignment);
+// If AlignAfterBracket is DontAlign,
+// adjust default PenaltyBreakBeforeFirstCallParameter to favor Newline.
+// Always accept any explicitly specified value though.
+if (!IO.outputting() &&
+Style.AlignAfterOpenBracket == FormatStyle::BAS_DontAlign) {
+  Style.PenaltyBreakBeforeFirstCallParameter = 0;
+}
 IO.mapOptional("PenaltyBreakBeforeFirstCallParameter",
Style.PenaltyBreakBeforeFirstCallParameter);
 IO.mapOptional("PenaltyBreakComment", Style.PenaltyBreakComment);

[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-10-31 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits
glaubitz added a comment.

Hmm, I'm not sure why the CHECK-DEBIAN-SPARC32 test is failing but I assume the 
expected output needs to be updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

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


[PATCH] D90534: [clang-format] Add new option PenaltyIndentedWhitespace

2020-10-31 Thread Mark Nauwelaerts via Phabricator via cfe-commits
mnauw created this revision.
mnauw added a reviewer: sammccall.
mnauw added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
mnauw requested review of this revision.

As the example in the comment within the patch shows, the intention of 
yet-another-penalty is to discourage some "far-right-cascading" indentation 
cases.  Of course, mileage and taste may vary, so the default does not affect 
current behavior in any way.  Likewise, the option may well be named otherwise 
(etc).

I realize the patch is likely incomplete (and requires additional test and 
documentation changes), but it would first have to be considered useful and 
acceptable in the first place.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90534

Files:
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp


Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -597,6 +597,8 @@
 IO.mapOptional("PenaltyExcessCharacter", Style.PenaltyExcessCharacter);
 IO.mapOptional("PenaltyReturnTypeOnItsOwnLine",
Style.PenaltyReturnTypeOnItsOwnLine);
+IO.mapOptional("PenaltyIndentedWhitespace",
+   Style.PenaltyIndentedWhitespace);
 IO.mapOptional("PointerAlignment", Style.PointerAlignment);
 IO.mapOptional("RawStringFormats", Style.RawStringFormats);
 IO.mapOptional("ReflowComments", Style.ReflowComments);
@@ -975,6 +977,7 @@
   LLVMStyle.PenaltyReturnTypeOnItsOwnLine = 60;
   LLVMStyle.PenaltyBreakBeforeFirstCallParameter = 19;
   LLVMStyle.PenaltyBreakTemplateDeclaration = prec::Relational;
+  LLVMStyle.PenaltyIndentedWhitespace = 0;
 
   LLVMStyle.DisableFormat = false;
   LLVMStyle.SortIncludes = true;
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -783,6 +783,22 @@
 
   State.Column = getNewLineColumn(State);
 
+  // Add Penalty proportional to amount of whitespace away from FirstColumn
+  // This tends to penalize several lines that are far-right indented,
+  // and prefers a line-break prior to such a block, e.g:
+  //
+  // Constructor() :
+  //   member(value), long_member(
+  //  looong_call(param_1, param_2, param_3))
+  // would then become
+  // Constructor() :
+  //   member(value),
+  //   long_member(
+  //   looong_call(param_1, param_2, param_3))
+  if (State.Column > State.FirstIndent)
+Penalty +=
+Style.PenaltyIndentedWhitespace * (State.Column - State.FirstIndent);
+
   // Indent nested blocks relative to this column, unless in a very specific
   // JavaScript special case where:
   //
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1913,6 +1913,9 @@
   /// line.
   unsigned PenaltyReturnTypeOnItsOwnLine;
 
+  /// Penalty for whitespace indentation
+  unsigned PenaltyIndentedWhitespace;
+
   /// The ``&`` and ``*`` alignment style.
   enum PointerAlignmentStyle {
 /// Align pointer to the left.


Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -597,6 +597,8 @@
 IO.mapOptional("PenaltyExcessCharacter", Style.PenaltyExcessCharacter);
 IO.mapOptional("PenaltyReturnTypeOnItsOwnLine",
Style.PenaltyReturnTypeOnItsOwnLine);
+IO.mapOptional("PenaltyIndentedWhitespace",
+   Style.PenaltyIndentedWhitespace);
 IO.mapOptional("PointerAlignment", Style.PointerAlignment);
 IO.mapOptional("RawStringFormats", Style.RawStringFormats);
 IO.mapOptional("ReflowComments", Style.ReflowComments);
@@ -975,6 +977,7 @@
   LLVMStyle.PenaltyReturnTypeOnItsOwnLine = 60;
   LLVMStyle.PenaltyBreakBeforeFirstCallParameter = 19;
   LLVMStyle.PenaltyBreakTemplateDeclaration = prec::Relational;
+  LLVMStyle.PenaltyIndentedWhitespace = 0;
 
   LLVMStyle.DisableFormat = false;
   LLVMStyle.SortIncludes = true;
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -783,6 +783,22 @@
 
   State.Column = getNewLineColumn(State);
 
+  // Add Penalty proportional to amount of whitespace away from FirstColumn
+  // This tends to penalize several lines that are far-right indented,
+  // and prefers a line-break prior to such a block, e.g:
+  //
+  // Constructor() :
+  //   member(value), long_member(
+  //  looong_

[PATCH] D90535: [clang-tidy] Allow -warnings-as-errors to be specified from run_clang_tidy.py

2020-10-31 Thread Christian Schärf via Phabricator via cfe-commits
schaerfo created this revision.
schaerfo added a reviewer: alexfh.
schaerfo added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.
schaerfo requested review of this revision.

This patch adds a `-warnings-as-errors` option to run_clang_tidy.py, which is 
forwarded to clang-tidy itself. This is done similar to the `-header-filter` 
option (as well as some others).

The rationale behind this is that previously, promoting warnings to errors when 
using run_clang_tidy was only possible through the .clang-tidy file and the 
`-config` option (which, however, overrides what is specified in .clang-tidy) 
and thus could not be changed without modifying this file. This comes in handy 
e. g. for CI, when you want the run to fail if certain warnings are emitted.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90535

Files:
  clang-tools-extra/clang-tidy/tool/run-clang-tidy.py


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -81,7 +81,8 @@
 
 def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
 header_filter, allow_enabling_alpha_checkers,
-extra_arg, extra_arg_before, quiet, config):
+extra_arg, extra_arg_before, quiet, config,
+warnings_as_errors):
   """Gets a command line for clang-tidy."""
   start = [clang_tidy_binary]
   if allow_enabling_alpha_checkers:
@@ -106,6 +107,8 @@
   start.append('-quiet')
   if config:
   start.append('-config=' + config)
+  if warnings_as_errors:
+  start.append('-warnings-as-errors=' + warnings_as_errors)
   start.append(f)
   return start
 
@@ -165,7 +168,7 @@
  tmpdir, build_path, args.header_filter,
  args.allow_enabling_alpha_checkers,
  args.extra_arg, args.extra_arg_before,
- args.quiet, args.config)
+ args.quiet, args.config, 
args.warnings_as_errors)
 
 proc = subprocess.Popen(invocation, stdout=subprocess.PIPE, 
stderr=subprocess.PIPE)
 output, err = proc.communicate()
@@ -234,6 +237,9 @@
   'command line.')
   parser.add_argument('-quiet', action='store_true',
   help='Run clang-tidy in quiet mode')
+  parser.add_argument('-warnings-as-errors', default=None,
+  help='Upgrades warnings to errors. Does not enable '
+  'any warnings py itself.')
   args = parser.parse_args()
 
   db_path = 'compile_commands.json'


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -81,7 +81,8 @@
 
 def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
 header_filter, allow_enabling_alpha_checkers,
-extra_arg, extra_arg_before, quiet, config):
+extra_arg, extra_arg_before, quiet, config,
+warnings_as_errors):
   """Gets a command line for clang-tidy."""
   start = [clang_tidy_binary]
   if allow_enabling_alpha_checkers:
@@ -106,6 +107,8 @@
   start.append('-quiet')
   if config:
   start.append('-config=' + config)
+  if warnings_as_errors:
+  start.append('-warnings-as-errors=' + warnings_as_errors)
   start.append(f)
   return start
 
@@ -165,7 +168,7 @@
  tmpdir, build_path, args.header_filter,
  args.allow_enabling_alpha_checkers,
  args.extra_arg, args.extra_arg_before,
- args.quiet, args.config)
+ args.quiet, args.config, args.warnings_as_errors)
 
 proc = subprocess.Popen(invocation, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
 output, err = proc.communicate()
@@ -234,6 +237,9 @@
   'command line.')
   parser.add_argument('-quiet', action='store_true',
   help='Run clang-tidy in quiet mode')
+  parser.add_argument('-warnings-as-errors', default=None,
+  help='Upgrades warnings to errors. Does not enable '
+  'any warnings py itself.')
   args = parser.parse_args()
 
   db_path = 'compile_commands.json'
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D90174: [HIP] Fix regressions due to fp contract change

2020-10-31 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 302099.
yaxunl edited the summary of this revision.
yaxunl added a comment.
Herald added subscribers: dexonsmith, dang.

introduce faststd as value for -ffp-contract and use it for HIP by default.


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

https://reviews.llvm.org/D90174

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGenCUDA/fp-contract.cu
  clang/test/Driver/autocomplete.c

Index: clang/test/Driver/autocomplete.c
===
--- clang/test/Driver/autocomplete.c
+++ clang/test/Driver/autocomplete.c
@@ -66,6 +66,7 @@
 // FNOSANICOVERALL-NEXT: trace-pc-guard
 // RUN: %clang --autocomplete=-ffp-contract= | FileCheck %s -check-prefix=FFPALL
 // FFPALL: fast
+// FFPALL-NEXT: faststd 
 // FFPALL-NEXT: off
 // FFPALL-NEXT: on
 // RUN: %clang --autocomplete=-flto= | FileCheck %s -check-prefix=FLTOALL
Index: clang/test/CodeGenCUDA/fp-contract.cu
===
--- clang/test/CodeGenCUDA/fp-contract.cu
+++ clang/test/CodeGenCUDA/fp-contract.cu
@@ -1,32 +1,298 @@
-// REQUIRES: x86-registered-target
-// REQUIRES: nvptx-registered-target
+// REQUIRES: x86-registered-target, nvptx-registered-target, amdgpu-registered-target
 
-// By default we should fuse multiply/add into fma instruction.
+// By default CUDA uses -ffp-contract=fast, HIP uses -ffp-contract=faststd.
+// we should fuse multiply/add into fma instruction.
+// In IR, fmul/fadd instructions with contract flag are emitted.
+// In backend
+//nvptx -  assumes fast fp fuse option, which fuses
+// mult/add insts disregarding contract flag and
+// llvm.fmuladd intrinsics.
+//amdgcn - assumes standard fp fuse option, which only
+// fuses mult/add insts with contract flag and
+// llvm.fmuladd intrinsics.
+
+// RUN: %clang_cc1 -fcuda-is-device -triple nvptx-nvidia-cuda -S \
+// RUN:   -disable-llvm-passes -o - %s \
+// RUN:   | FileCheck -check-prefixes=COMMON,NV-ON %s
+// RUN: %clang_cc1 -fcuda-is-device -triple amdgcn-amd-amdhsa -S \
+// RUN:   -target-cpu gfx906 -disable-llvm-passes -o - -x hip %s \
+// RUN:   | FileCheck -check-prefixes=COMMON,AMD-ON %s
 // RUN: %clang_cc1 -fcuda-is-device -triple nvptx-nvidia-cuda -S \
-// RUN:   -disable-llvm-passes -o - %s | FileCheck -check-prefix ENABLED %s
+// RUN:   -O3 -o - %s \
+// RUN:   | FileCheck -check-prefixes=COMMON,NV-OPT-FAST %s
+// RUN: %clang_cc1 -fcuda-is-device -triple amdgcn-amd-amdhsa -S \
+// RUN:   -O3 -target-cpu gfx906 -o - -x hip %s \
+// RUN:   | FileCheck -check-prefixes=COMMON,AMD-OPT-FASTSTD %s
+
+// Check separate compile/backend steps corresponding to -save-temps.
+
+// RUN: %clang_cc1 -fcuda-is-device -triple amdgcn-amd-amdhsa -emit-llvm \
+// RUN:   -O3 -disable-llvm-passes -target-cpu gfx906 -o %t.ll -x hip %s
+// RUN: cat %t.ll  | FileCheck -check-prefixes=COMMON,AMD-OPT-FAST-IR %s
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -S \
+// RUN:   -O3 -target-cpu gfx906 -o - -x ir %t.ll \
+// RUN:   | FileCheck -check-prefixes=COMMON,AMD-OPT-FASTSTD %s
 
 // Explicit -ffp-contract=fast
+// In IR, fmul/fadd instructions with contract flag are emitted.
+// In backend
+//nvptx/amdgcn - assumes fast fp fuse option, which fuses
+//   mult/add insts disregarding contract flag and
+//   llvm.fmuladd intrinsics.
+
 // RUN: %clang_cc1 -fcuda-is-device -triple nvptx-nvidia-cuda -S \
 // RUN:   -ffp-contract=fast -disable-llvm-passes -o - %s \
-// RUN:   | FileCheck -check-prefix ENABLED %s
+// RUN:   | FileCheck -check-prefixes=COMMON,NV-ON %s
+// RUN: %clang_cc1 -fcuda-is-device -triple amdgcn-amd-amdhsa -S \
+// RUN:   -target-cpu gfx906 -disable-llvm-passes -o - -x hip %s \
+// RUN:   -ffp-contract=fast \
+// RUN:   | FileCheck -check-prefixes=COMMON,AMD-ON %s
+// RUN: %clang_cc1 -fcuda-is-device -triple nvptx-nvidia-cuda -S \
+// RUN:   -O3 -o - %s \
+// RUN:   -ffp-contract=fast \
+// RUN:   | FileCheck -check-prefixes=COMMON,NV-OPT-FAST %s
+// RUN: %clang_cc1 -fcuda-is-device -triple amdgcn-amd-amdhsa -S \
+// RUN:   -O3 -target-cpu gfx906 -o - -x hip %s \
+// RUN:   -ffp-contract=fast \
+// RUN:   | FileCheck -check-prefixes=COMMON,AMD-OPT-FAST %s
+
+// Check separate compile/backend steps corresponding to -save-temps.
+// When input is IR, -ffp-contract has no effect. Backend uses default
+// default FP fuse option.
+
+// RUN: %clang_cc1 -fcuda-is-device -triple amdgcn-amd-amdhsa -emit-llvm \
+// RUN:   -ffp-contract=fast \
+// RUN:   -O3 -disable-llvm-passes -target-cpu gfx906 -o %t.ll -x hip %s
+// RUN: cat %t.ll  | FileCheck -check-prefixes=COMMON,AMD-OPT-FAST-IR %s
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -S \
+// RUN:   -O3 -target-cpu gfx906 -o - -x ir %t.ll \
+// RUN:   | FileCheck -check-pre

[PATCH] D90534: [clang-format] Add new option PenaltyIndentedWhitespace

2020-10-31 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay requested changes to this revision.
MyDeveloperDay added a comment.
This revision now requires changes to proceed.

1. This requires unit tests
2. This requires a fill context diff
3. This requires changes to the ClangFormatStyleOptions.rst by running the 
dump_style tool in clang/docs/tools


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90534

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


[PATCH] D90533: [clang-format] Always consider option PenaltyBreakBeforeFirstCallParameter

2020-10-31 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

You need to supply full context diffs




Comment at: clang/unittests/Format/FormatTest.cpp:4481
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.PenaltyBreakBeforeFirstCallParameter = 0;
   Style.AlignOperands = FormatStyle::OAS_DontAlign;

Why do you need these? what is the default value of this as set in the LLVM 
style?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90533

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


[PATCH] D17993: [CodeGen] Apply 'nonnull' to 'this' pointer arguments.

2020-10-31 Thread CJ Johnson via Phabricator via cfe-commits
CJ-Johnson marked an inline comment as done.
CJ-Johnson added inline comments.



Comment at: 
clang/test/CodeGenCXX/microsoft-abi-multiple-nonvirtual-inheritance.cpp:126
 //
-// CHECK: call x86_thiscallcc void %[[VFUN_VALUE]](i8* %[[RIGHT]])
+// CHECK: call x86_thiscallcc void %[[VFUN_VALUE]](i8* nonnull %[[RIGHT]])
 // CHECK: ret

rsmith wrote:
> Why does this call get `nonnull` but not `dereferenceable`? Seems like we 
> should be able to use at least `dereferenceable(sizeof(Right))` here -- but I 
> think we could actually be more aggressive and pass 
> `dereferenceable(sizeof(ChildOverride) - offsetof(ChildOverride,  class>))`.
Fixed! Added a check for `isVoidPointerType()` :)


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

https://reviews.llvm.org/D17993

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


[PATCH] D90336: [Sema] Diagnose annotating `if constexpr` with a likelihood attribute

2020-10-31 Thread Mark de Wever via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Mordante marked an inline comment as done.
Closed by commit rGb231396122f1: [Sema] Diagnose annotating `if constexpr` with 
a likelihood attribute (authored by Mordante).

Changed prior to commit:
  https://reviews.llvm.org/D90336?vs=301384&id=302101#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90336

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaStmt.cpp
  clang/test/SemaCXX/attr-likelihood.cpp


Index: clang/test/SemaCXX/attr-likelihood.cpp
===
--- clang/test/SemaCXX/attr-likelihood.cpp
+++ clang/test/SemaCXX/attr-likelihood.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=c++17 -fsyntax-only -verify
 // RUN: %clang_cc1 %s -DPEDANTIC -pedantic -fsyntax-only -verify
 
 #if PEDANTIC
@@ -129,4 +129,23 @@
   catch (...) [[likely]] { // expected-error {{expected expression}}
   }
 }
+
+void o()
+{
+  // expected-warning@+2 {{attribute 'likely' has no effect when annotating an 
'if constexpr' statement}}
+  // expected-note@+1 {{annotating the 'if constexpr' statement here}}
+  if constexpr (true) [[likely]];
+
+  // expected-note@+1 {{annotating the 'if constexpr' statement here}}
+  if constexpr (true) {
+  // expected-warning@+1 {{attribute 'unlikely' has no effect when annotating 
an 'if constexpr' statement}}
+  } else [[unlikely]];
+
+  // Annotating both branches with conflicting likelihoods generates no 
diagnostic regarding the conflict.
+  // expected-warning@+2 {{attribute 'likely' has no effect when annotating an 
'if constexpr' statement}}
+  // expected-note@+1 2 {{annotating the 'if constexpr' statement here}}
+  if constexpr (true) [[likely]] {
+  // expected-warning@+1 {{attribute 'likely' has no effect when annotating an 
'if constexpr' statement}}
+  } else [[likely]];
+}
 #endif
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -600,16 +600,31 @@
 DiagnoseEmptyStmtBody(CondExpr->getEndLoc(), thenStmt,
   diag::warn_empty_if_body);
 
-  std::tuple LHC =
-  Stmt::determineLikelihoodConflict(thenStmt, elseStmt);
-  if (std::get<0>(LHC)) {
-const Attr *ThenAttr = std::get<1>(LHC);
-const Attr *ElseAttr = std::get<2>(LHC);
-Diags.Report(ThenAttr->getLocation(),
- diag::warn_attributes_likelihood_ifstmt_conflict)
-<< ThenAttr << ThenAttr->getRange();
-Diags.Report(ElseAttr->getLocation(), diag::note_conflicting_attribute)
-<< ElseAttr << ElseAttr->getRange();
+  if (IsConstexpr) {
+auto DiagnoseLikelihood = [&](const Stmt *S) {
+  if (const Attr *A = Stmt::getLikelihoodAttr(S)) {
+Diags.Report(A->getLocation(),
+ diag::warn_attribute_has_no_effect_on_if_constexpr)
+<< A << A->getRange();
+Diags.Report(IfLoc,
+ diag::note_attribute_has_no_effect_on_if_constexpr_here)
+<< SourceRange(IfLoc, LParenLoc.getLocWithOffset(-1));
+  }
+};
+DiagnoseLikelihood(thenStmt);
+DiagnoseLikelihood(elseStmt);
+  } else {
+std::tuple LHC =
+Stmt::determineLikelihoodConflict(thenStmt, elseStmt);
+if (std::get<0>(LHC)) {
+  const Attr *ThenAttr = std::get<1>(LHC);
+  const Attr *ElseAttr = std::get<2>(LHC);
+  Diags.Report(ThenAttr->getLocation(),
+   diag::warn_attributes_likelihood_ifstmt_conflict)
+  << ThenAttr << ThenAttr->getRange();
+  Diags.Report(ElseAttr->getLocation(), diag::note_conflicting_attribute)
+  << ElseAttr << ElseAttr->getRange();
+}
   }
 
   return BuildIfStmt(IfLoc, IsConstexpr, LParenLoc, InitStmt, Cond, RParenLoc,
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3162,6 +3162,11 @@
InGroup;
 def note_attribute_has_no_effect_on_infinite_loop_here : Note<
   "annotating the infinite loop here">;
+def warn_attribute_has_no_effect_on_if_constexpr : Warning<
+  "attribute %0 has no effect when annotating an 'if constexpr' statement">,
+   InGroup;
+def note_attribute_has_no_effect_on_if_constexpr_here : Note<
+  "annotating the 'if constexpr' statement here">;
 def err_decl_attribute_invalid_on_stmt : Error<
   "%0 attribute cannot be applied to a statement">;
 def err_stmt_attribute_invalid_on_decl : Error<


Index: clang/test/SemaCXX/attr-likelihood.cpp
===
--- clang/test/SemaCXX/attr-likelihood.cpp
+++ clang/test/SemaCXX/attr-

[clang] b231396 - [Sema] Diagnose annotating `if constexpr` with a likelihood attribute

2020-10-31 Thread Mark de Wever via cfe-commits

Author: Mark de Wever
Date: 2020-10-31T17:51:36+01:00
New Revision: b231396122f131ffaff5fc4ba36685a7df554aaa

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

LOG: [Sema] Diagnose annotating `if constexpr` with a likelihood attribute

Adds a diagnostic when the user annotates an `if constexpr` with a
likelihood attribute. The `if constexpr` statement is evaluated at compile
time so the attribute has no effect. Annotating the accompanied `else`
with a likelihood attribute has the same effect as annotating a generic
statement. Since the attribute there is most likely not intended, a
diagnostic will be issued. Since the attributes can't conflict, the
"conflict" won't be diagnosed for an `if constexpr`.

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

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaStmt.cpp
clang/test/SemaCXX/attr-likelihood.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 2b19eac7e0c8..7555a5ebbd34 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3162,6 +3162,11 @@ def warn_attribute_has_no_effect_on_infinite_loop : 
Warning<
InGroup;
 def note_attribute_has_no_effect_on_infinite_loop_here : Note<
   "annotating the infinite loop here">;
+def warn_attribute_has_no_effect_on_if_constexpr : Warning<
+  "attribute %0 has no effect when annotating an 'if constexpr' statement">,
+   InGroup;
+def note_attribute_has_no_effect_on_if_constexpr_here : Note<
+  "annotating the 'if constexpr' statement here">;
 def err_decl_attribute_invalid_on_stmt : Error<
   "%0 attribute cannot be applied to a statement">;
 def err_stmt_attribute_invalid_on_decl : Error<

diff  --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index e6314fd65f8c..2672cadfa421 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -600,16 +600,31 @@ StmtResult Sema::ActOnIfStmt(SourceLocation IfLoc, bool 
IsConstexpr,
 DiagnoseEmptyStmtBody(CondExpr->getEndLoc(), thenStmt,
   diag::warn_empty_if_body);
 
-  std::tuple LHC =
-  Stmt::determineLikelihoodConflict(thenStmt, elseStmt);
-  if (std::get<0>(LHC)) {
-const Attr *ThenAttr = std::get<1>(LHC);
-const Attr *ElseAttr = std::get<2>(LHC);
-Diags.Report(ThenAttr->getLocation(),
- diag::warn_attributes_likelihood_ifstmt_conflict)
-<< ThenAttr << ThenAttr->getRange();
-Diags.Report(ElseAttr->getLocation(), diag::note_conflicting_attribute)
-<< ElseAttr << ElseAttr->getRange();
+  if (IsConstexpr) {
+auto DiagnoseLikelihood = [&](const Stmt *S) {
+  if (const Attr *A = Stmt::getLikelihoodAttr(S)) {
+Diags.Report(A->getLocation(),
+ diag::warn_attribute_has_no_effect_on_if_constexpr)
+<< A << A->getRange();
+Diags.Report(IfLoc,
+ diag::note_attribute_has_no_effect_on_if_constexpr_here)
+<< SourceRange(IfLoc, LParenLoc.getLocWithOffset(-1));
+  }
+};
+DiagnoseLikelihood(thenStmt);
+DiagnoseLikelihood(elseStmt);
+  } else {
+std::tuple LHC =
+Stmt::determineLikelihoodConflict(thenStmt, elseStmt);
+if (std::get<0>(LHC)) {
+  const Attr *ThenAttr = std::get<1>(LHC);
+  const Attr *ElseAttr = std::get<2>(LHC);
+  Diags.Report(ThenAttr->getLocation(),
+   diag::warn_attributes_likelihood_ifstmt_conflict)
+  << ThenAttr << ThenAttr->getRange();
+  Diags.Report(ElseAttr->getLocation(), diag::note_conflicting_attribute)
+  << ElseAttr << ElseAttr->getRange();
+}
   }
 
   return BuildIfStmt(IfLoc, IsConstexpr, LParenLoc, InitStmt, Cond, RParenLoc,

diff  --git a/clang/test/SemaCXX/attr-likelihood.cpp 
b/clang/test/SemaCXX/attr-likelihood.cpp
index c8be00bfcc32..d2f95d17069c 100644
--- a/clang/test/SemaCXX/attr-likelihood.cpp
+++ b/clang/test/SemaCXX/attr-likelihood.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=c++17 -fsyntax-only -verify
 // RUN: %clang_cc1 %s -DPEDANTIC -pedantic -fsyntax-only -verify
 
 #if PEDANTIC
@@ -129,4 +129,23 @@ void n() [[likely]] // expected-error {{'likely' attribute 
cannot be applied to
   catch (...) [[likely]] { // expected-error {{expected expression}}
   }
 }
+
+void o()
+{
+  // expected-warning@+2 {{attribute 'likely' has no effect when annotating an 
'if constexpr' statement}}
+  // expected-note@+1 {{annotating the 'if constexpr' statement here}}
+  if constexpr (true) [[likely]];
+
+  // expected-note@+1 {{annotating the 'if constexpr' statement here}}
+  if constexpr (true) {

[clang] b46fddf - [CodeGen] Implement [[likely]] and [[unlikely]] for while and for loop.

2020-10-31 Thread Mark de Wever via cfe-commits

Author: Mark de Wever
Date: 2020-10-31T17:51:29+01:00
New Revision: b46fddf75fc27ee1545f00ab853d50b10c1d7ee6

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

LOG: [CodeGen] Implement [[likely]] and [[unlikely]] for while and for loop.

The attribute has no effect on a do statement since the path of execution
will always include its substatement.

It adds a diagnostic when the attribute is used on an infinite while loop
since the codegen omits the branch here. Since the likelihood attributes
have no effect on a do statement no diagnostic will be issued for
do [[unlikely]] {...} while(0);

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

Added: 
clang/test/CodeGenCXX/attr-likelihood-iteration-stmt.cpp

Modified: 
clang/include/clang/AST/Stmt.h
clang/include/clang/Basic/AttrDocs.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/AST/Stmt.cpp
clang/lib/CodeGen/CGStmt.cpp
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp

Removed: 




diff  --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index c2452f426566..c2e69a91e55d 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -1183,6 +1183,9 @@ class alignas(void *) Stmt {
   /// \returns the likelihood of a statement.
   static Likelihood getLikelihood(const Stmt *S);
 
+  /// \returns the likelihood attribute of a statement.
+  static const Attr *getLikelihoodAttr(const Stmt *S);
+
   /// \returns the likelihood of the 'then' branch of an 'if' statement. The
   /// 'else' branch is required to determine whether both branches specify the
   /// same likelihood, which affects the result.

diff  --git a/clang/include/clang/Basic/AttrDocs.td 
b/clang/include/clang/Basic/AttrDocs.td
index 912af82dee8c..238b0752a1a2 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -1738,7 +1738,8 @@ PGO (Profile-Guided Optimization) or at optimization 
level 0.
 
 In Clang, the attributes will be ignored if they're not placed on
 * the ``case`` or ``default`` label of a ``switch`` statement,
-* or on the substatement of an ``if`` or ``else`` statement.
+* or on the substatement of an ``if`` or ``else`` statement,
+* or on the substatement of an ``for`` or ``while`` statement.
 The C++ Standard recommends to honor them on every statement in the
 path of execution, but that can be confusing:
 
@@ -1769,8 +1770,7 @@ path of execution, but that can be confusing:
   }
 
 
-At the moment the attributes only have effect when used in an ``if``, ``else``,
-or ``switch`` statement.
+Below are some example usages of the likelihood attributes and their effects:
 
 .. code-block:: c++
 
@@ -1850,6 +1850,23 @@ or ``switch`` statement.
   break;
   }
 
+  for(int i = 0; i != size; ++i) [[likely]] {
+...   // The loop is the likely path of execution
+  }
+
+  for(const auto &E : Elements) [[likely]] {
+...   // The loop is the likely path of execution
+  }
+
+  while(i != size) [[unlikely]] {
+...   // The loop is the unlikely path of execution
+  }   // The generated code will optimize to skip the loop body
+
+  while(true) [[unlikely]] {
+...   // The attribute has no effect
+  }   // Clang elides the comparison and generates an infinite
+  // loop
+
   }];
 }
 

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index fe6b88321f6b..2b19eac7e0c8 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3157,6 +3157,11 @@ def warn_cxx11_gnu_attribute_on_type : Warning<
 def warn_unhandled_ms_attribute_ignored : Warning<
   "__declspec attribute %0 is not supported">,
   InGroup;
+def warn_attribute_has_no_effect_on_infinite_loop : Warning<
+  "attribute %0 has no effect when annotating an infinite loop">,
+   InGroup;
+def note_attribute_has_no_effect_on_infinite_loop_here : Note<
+  "annotating the infinite loop here">;
 def err_decl_attribute_invalid_on_stmt : Error<
   "%0 attribute cannot be applied to a statement">;
 def err_stmt_attribute_invalid_on_decl : Error<

diff  --git a/clang/lib/AST/Stmt.cpp b/clang/lib/AST/Stmt.cpp
index 09503079ffae..8b2564d7fc96 100644
--- a/clang/lib/AST/Stmt.cpp
+++ b/clang/lib/AST/Stmt.cpp
@@ -158,6 +158,10 @@ Stmt::Likelihood Stmt::getLikelihood(const Stmt *S) {
   return ::getLikelihood(S).first;
 }
 
+const Attr *Stmt::getLikelihoodAttr(const Stmt *S) {
+  return ::getLikelihood(S).second;
+}
+
 Stmt::Likelihood Stmt::getLikelihood(const Stmt *Then, const St

[PATCH] D90121: clang-format: Add a consumer to diagnostics engine

2020-10-31 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

@dmikis  just for history sake I introduce the frontEnd when doing D68554: 
[clang-format] Proposal for clang-format to give compiler style warnings 
, D69854: [clang-format] [RELAND] Remove the 
dependency on frontend  was an attempt to 
remove the need for frontEnd while doing the same thing, so its not fair to say 
that D69854 
 was the cause of a regression its actually my belief that it would have failed 
before (unless I'm mistaken)

Having said that I understand what you are trying to do...and agree it being 
move to libBasic would help us here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90121

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


[PATCH] D89899: [CodeGen] Implement [[likely]] and [[unlikely]] for the iteration statements

2020-10-31 Thread Mark de Wever via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Mordante marked an inline comment as done.
Closed by commit rGb46fddf75fc2: [CodeGen] Implement [[likely]] and 
[[unlikely]] for while and for loop. (authored by Mordante).

Changed prior to commit:
  https://reviews.llvm.org/D89899?vs=301332&id=302100#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89899

Files:
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/Stmt.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
  clang/test/CodeGenCXX/attr-likelihood-iteration-stmt.cpp

Index: clang/test/CodeGenCXX/attr-likelihood-iteration-stmt.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/attr-likelihood-iteration-stmt.cpp
@@ -0,0 +1,60 @@
+// RUN: %clang_cc1 -O1 -disable-llvm-passes -emit-llvm %s -o - -triple=x86_64-linux-gnu -verify
+// RUN: %clang_cc1 -O1 -disable-llvm-passes -emit-llvm %s -o - -triple=x86_64-linux-gnu | FileCheck %s
+
+void wl(int e){
+  // CHECK-LABEL: define{{.*}}wl
+  // CHECK: br {{.*}} !prof !6
+  while(e) [[likely]] ++e;
+}
+
+void wu(int e){
+  // CHECK-LABEL: define{{.*}}wu
+  // CHECK: br {{.*}} !prof !9
+  while(e) [[unlikely]] ++e;
+}
+
+void w_branch_elided(unsigned e){
+  // CHECK-LABEL: define{{.*}}w_branch_elided
+  // CHECK-NOT: br {{.*}} !prof
+  // expected-warning@+2 {{attribute 'likely' has no effect when annotating an infinite loop}}
+  // expected-note@+1 {{annotating the infinite loop here}}
+  while(1) [[likely]] ++e;
+}
+
+void fl(unsigned e)
+{
+  // CHECK-LABEL: define{{.*}}fl
+  // CHECK: br {{.*}} !prof !6
+  for(int i = 0; i != e; ++e) [[likely]];
+}
+
+void fu(int e)
+{
+  // CHECK-LABEL: define{{.*}}fu
+  // CHECK: br {{.*}} !prof !9
+  for(int i = 0; i != e; ++e) [[unlikely]];
+}
+
+void f_branch_elided()
+{
+  // CHECK-LABEL: define{{.*}}f_branch_elided
+  // CHECK-NOT: br {{.*}} !prof
+  for(;;) [[likely]];
+}
+
+void frl(int (&&e) [4])
+{
+  // CHECK-LABEL: define{{.*}}frl
+  // CHECK: br {{.*}} !prof !6
+  for(int i : e) [[likely]];
+}
+
+void fru(int (&&e) [4])
+{
+  // CHECK-LABEL: define{{.*}}fru
+  // CHECK: br {{.*}} !prof !9
+  for(int i : e) [[unlikely]];
+}
+
+// CHECK: !6 = !{!"branch_weights", i32 2000, i32 1}
+// CHECK: !9 = !{!"branch_weights", i32 1, i32 2000}
Index: clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
===
--- clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
+++ clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
@@ -69,6 +69,7 @@
 
   // CHECK-NOT: br {{.*}} %if.end{{.*}} !prof
   if (b)
+// CHECK: br {{.*}} !prof !7
 while (B())
   [[unlikely]] { b = false; }
 }
@@ -96,6 +97,7 @@
 
   // CHECK-NOT: br {{.*}} %if.end{{.*}} !prof
   if (b)
+// CHECK: br {{.*}} !prof !7
 for (; B();)
   [[unlikely]] {}
 }
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -1407,6 +1407,13 @@
   llvm::MDNode *createProfileWeightsForLoop(const Stmt *Cond,
 uint64_t LoopCount) const;
 
+  /// Calculate the branch weight for PGO data or the likelihood attribute.
+  /// The function tries to get the weight of \ref createProfileWeightsForLoop.
+  /// If that fails it gets the weight of \ref createBranchWeights.
+  llvm::MDNode *createProfileOrBranchWeightsForLoop(const Stmt *Cond,
+uint64_t LoopCount,
+const Stmt *Body) const;
+
 public:
   /// Increment the profiler's counter for the given statement by \p StepV.
   /// If \p StepV is null, the default increment is 1.
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -2555,3 +2555,12 @@
   llvm::MDBuilder MDHelper(CGM.getLLVMContext());
   return MDHelper.createBranchWeights(LHW->first, LHW->second);
 }
+
+llvm::MDNode *CodeGenFunction::createProfileOrBranchWeightsForLoop(
+const Stmt *Cond, uint64_t LoopCount, const Stmt *Body) const {
+  llvm::MDNode *Weights = createProfileWeightsForLoop(Cond, LoopCount);
+  if (!Weights && CGM.getCodeGenOpts().OptimizationLevel)
+Weights = createBranchWeights(Stmt::getLikelihood(Body));
+
+  return Weights;
+}
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@

[PATCH] D87565: [Sema] Improve const_cast conformance to N4261

2020-10-31 Thread Mark de Wever via Phabricator via cfe-commits
Mordante added a comment.

Friendly ping.


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

https://reviews.llvm.org/D87565

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


[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

2020-10-31 Thread Endre Fülöp via Phabricator via cfe-commits
gamesh411 added a comment.

> Just to make sure we're on the same page -- the current approach is not 
> flow-sensitive, and so my concern is that it won't report any true positives 
> (not that it will be prone to false positives).

Sorry about that. You are absolutely right; what I was trying to say is 
CallGraph-based.

Just some thoughts on this example:

In D83717#2279263 , @aaron.ballman 
wrote:

> One of the concerns I have with this not being a flow-sensitive check is that 
> most of the bad situations are not going to be caught by the clang-tidy 
> version of the check. The CERT rules show contrived code examples, but the 
> more frequent issue looks like:
>
>   void cleanup(struct whatever *ptr) {
> assert(ptr); // This potentially calls abort()
> free(ptr->buffer);
> free(ptr);
>   }
>   ...

What I have in support of this approach is this reasoning:
If a handler is used where either branch can abort then that branch is expected 
to be taken. Otherwise it is dead code. I would argue then, that this abortion 
should be refactored out of the handler function to ensure well-defined 
behaviour in every possible case.

As a counter-argument; suppose that there is a function that is used as both an 
exit-handler and as a simple invocation. In this case, I can understand if one 
would not want to factor the abortion logic out, or possibly pass flags around.

Then to this remark:

> The fact that we're not looking through the call sites (even without cross-TU 
> support) means the check isn't going to catch the most problematic cases. You 
> could modify the called function collector to gather this a bit better, but 
> you'd issue false positives in flow-sensitive situations like:
>
>   void some_cleanup_func(void) {
> for (size_t idx = 0; idx < GlobalElementCount; ++idx) {
>   struct whatever *ptr = GlobalElement[idx];
>   if (ptr) {
> // Now we know abort() won't be called
> cleanup(ptr);
>   }
> }
>   }

The current approach definitely does not take 'adjacent' call-sites into 
account (not to mention CTU ones).
In this regard I also tend to see the benefit of this being a ClangSA checker 
as that would solve 3 problems at once:

1. Being path-sensitive, so we can explain how we got to the erroneous 
program-point
2. It utilizes CTU mode to take callsites from other TU-s into account
3. Runtime-stack building is implicitly done by ExprEngine as a side effect of 
symbolic execution

Counter-argument:
But using ClangSA also introduces a big challenge.
ClangSA analyzes all top-level functions during analysis. However I don't know 
if it understands the concept of exit-handlers, and I don't know a way of 
'triggering' an analysis 'on-exit' so to speak.
So AFAIK this model of analyzing only top-level functions is a limitation when 
it comes to modelling the program behaviour 'on-exit'.
sidenote:
To validate this claim I have dumped the exploded graph of the following file:

  #include 
  #include 
  
  void f() {
std::cout << "handler f";
  };
  
  int main() {
std::atexit(f);
  }

And it has no mention of std::cout being used, so I concluded, that ClangSA 
does not model the 'on-exit' behaviour.

I wanted to clear these issues before I made the documentation.
Thanks for the effort and the tips on evaluating the solution, I will do some 
more exploration.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83717

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


[PATCH] D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels

2020-10-31 Thread Mark de Wever via Phabricator via cfe-commits
Mordante added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6454
 
+static bool validateLikelihoodAttr(Sema &S, Decl *D, const ParsedAttr &A) {
+  if (!isa(D)) {

Mordante wrote:
> aaron.ballman wrote:
> > Mordante wrote:
> > > aaron.ballman wrote:
> > > > This is entering into somewhat novel territory for attributes, so some 
> > > > of this feedback is me thinking out loud.
> > > > 
> > > > Attr.td lists these two attributes as being a `StmtAttr` and not any 
> > > > kind of declaration attribute. We have `DeclOrTypeAttr` for attributes 
> > > > that can be applied to declarations or types, but we do not have 
> > > > something similar for statement attributes yet. We do have some custom 
> > > > semantic handling logic in SemaDeclAttr.cpp for statement attributes, 
> > > > but you avoid hitting that code path by adding a `case` for the two 
> > > > likelihood attributes. These attributes only apply to label 
> > > > declarations currently, and labels cannot be redeclared, so there 
> > > > aren't yet questions about whether this is inheritable or not. So we 
> > > > *might* be okay with this, but I'm not 100% certain. For instance, I 
> > > > don't recall if the pretty printer or AST dumpers will need to 
> > > > distinguish between whether this attribute is written on the statement 
> > > > or the declaration (which is itself a bit of an interesting question: 
> > > > should the attribute attach only to the statement rather than trying to 
> > > > attach to the underlying decl? 
> > > > http://eel.is/c++draft/stmt.stmt#stmt.label-1.sentence-2 is ambiguous, 
> > > > but I don't think of `case` or `default` labels as being declarations 
> > > > so I tend to not think of identifier labels as such either.). There's a 
> > > > part of me that wonders if we have a different issue where the 
> > > > attribute is trying to attach to the declaration rather than the 
> > > > statement and that this should be handled purely as a statement 
> > > > attribute.
> > > > 
> > > > I'm curious what your thoughts are, but I'd also like to see some 
> > > > additional tests for the random other bits that interact with 
> > > > attributes like AST dumping and pretty printing to be sure the behavior 
> > > > is reasonable.
> > > The labels in a switch are indeed different and the code in trunk already 
> > > should allow the attribute there. (I'm still busy with the CodeGen patch.)
> > > I agree that Standard isn't clear whether the attribute is attached to 
> > > the statement or the declaration.
> > > 
> > > The `LabelDecl` expects a pointer to a `LabelStmt` and not to an 
> > > `AttributedStmt`. Since declarations can already have attributes I used 
> > > that feature. I just checked and the `LabelDecl` isn't shown in the AST 
> > > and so the attributes also aren't shown. I can adjust that.
> > > 
> > > Another option would be to change the `LabelDecl` and have two overloads 
> > > of `setStmt`
> > > `void setStmt(LabelStmt *T) { TheStmt = T; }`
> > > `void setStmt(AttributedStmt *T) { TheStmt = T; }`
> > > Then `TheStmt` needs to be a `Stmt` and an extra getter would be required 
> > > to get the generic statement.
> > > 
> > > I think both solutions aren't trivial changes. Currently the attribute 
> > > has no effect on labels so it not being visible isn't a real issue. 
> > > However I feel that's not a proper solution. I expect attributes will be 
> > > used more in C and C++ in the future. For example, I can imagine a 
> > > `[[cold]]` attribute becoming available for labels.
> > > 
> > > So I'm leaning towards biting the bullet and change the implementation of 
> > > `LabelDecl` to allow an `AttributedStmt` instead of a `LabelStmt`.
> > > WDYT?
> > > Currently the attribute has no effect on labels so it not being visible 
> > > isn't a real issue. 
> > 
> > That's not entirely true though -- we have pretty printing capabilities 
> > that will lose the attribute when written on a label, so round-tripping 
> > through the pretty printer will fail. But we have quite a few issues with 
> > pretty printing attributes as it stands, so I'm not super concerned either.
> > 
> > > So I'm leaning towards biting the bullet and change the implementation of 
> > > LabelDecl to allow an AttributedStmt instead of a LabelStmt.
> > > WDYT?
> > 
> > I'm curious if @rsmith feels the same way, but I think something along 
> > those lines makes sense (if not an overload, perhaps a templated function 
> > with SFINAE). We'd have to make sure that the attributed statement 
> > eventually resolves to a `LabelStmt` once we strip the attributes away, but 
> > this would keep the attribute at the statement level rather than making it 
> > a declaration one, which I think is more along the lines of what's intended 
> > for the likelihood attributes (and probably for hot/cold if we add that 
> > support later).
> > > Currently the attribute has no effect on labels so it not being visible 
> > 

[PATCH] D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels

2020-10-31 Thread Mark de Wever via Phabricator via cfe-commits
Mordante updated this revision to Diff 302103.
Mordante added a comment.

`Sema::ActOnLabelStmt` now processes the statement attributes placed on the 
`LabelDecl`. Returning an `AttributedStmt` from this function seems to work as 
intended. This changes the behaviour of `[[nomerge]]` being allowed on labels. 
`[[nomerge]]` has been introduced in  D79121 .


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

https://reviews.llvm.org/D86559

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/AST/ast-dump-attr.cpp
  clang/test/Sema/attr-likelihood.c
  clang/test/Sema/attr-nomerge.cpp
  clang/test/SemaCXX/attr-likelihood.cpp

Index: clang/test/SemaCXX/attr-likelihood.cpp
===
--- clang/test/SemaCXX/attr-likelihood.cpp
+++ clang/test/SemaCXX/attr-likelihood.cpp
@@ -115,8 +115,7 @@
   if (x)
 goto lbl;
 
-  // FIXME: allow the attribute on the label
-  [[unlikely]] lbl : // expected-error {{'unlikely' attribute cannot be applied to a declaration}}
+  [[unlikely]] lbl :
  [[likely]] x = x + 1;
 
   [[likely]]++ x;
@@ -148,4 +147,14 @@
   // expected-warning@+1 {{attribute 'likely' has no effect when annotating an 'if constexpr' statement}}
   } else [[likely]];
 }
+
+void p()
+{
+// Make sure the attributes aren't processed as statement attributes.
+[[likely]] __label__ label; // expected-error {{expected expression}}
+__label__ [[likely]] label; // expected-error {{expected expression}}
+__label__ label [[likely]]; // expected-error {{expected expression}}
+
+[[likely]] label: __attribute__((unused)) int i;
+}
 #endif
Index: clang/test/Sema/attr-nomerge.cpp
===
--- clang/test/Sema/attr-nomerge.cpp
+++ clang/test/Sema/attr-nomerge.cpp
@@ -8,7 +8,7 @@
   int x;
   [[clang::nomerge]] x = 10; // expected-warning {{nomerge attribute is ignored because there exists no call expression inside the statement}}
 
-  [[clang::nomerge]] label: bar(); // expected-error {{'nomerge' attribute cannot be applied to a declaration}}
+  [[clang::nomerge]] label: bar();
 
 }
 
Index: clang/test/Sema/attr-likelihood.c
===
--- clang/test/Sema/attr-likelihood.c
+++ clang/test/Sema/attr-likelihood.c
@@ -43,8 +43,7 @@
   if (x)
 goto lbl;
 
-  // FIXME: allow the attribute on the label
-  [[clang::unlikely]] lbl : // expected-error {{'unlikely' attribute cannot be applied to a declaration}}
+  [[clang::unlikely]] lbl :
   [[clang::likely]] x = x + 1;
 
   [[clang::likely]]++ x;
Index: clang/test/AST/ast-dump-attr.cpp
===
--- clang/test/AST/ast-dump-attr.cpp
+++ clang/test/AST/ast-dump-attr.cpp
@@ -113,6 +113,35 @@
 N: __attribute(()) ;
 // CHECK: LabelStmt {{.*}} 'N'
 // CHECK-NEXT: NullStmt
+
+[[likely]] O:;
+// CHECK: AttributedStmt
+// CHECK-NEXT: LikelyAttr
+// CHECK-NEXT: LabelStmt {{.*}} 'O'
+
+[[likely]] P: __attribute__((unused)) int k;
+// CHECK: AttributedStmt
+// CHECK-NEXT: LikelyAttr
+// CHECK-NEXT: LabelStmt {{.*}} 'P'
+// CHECK: VarDecl{{.*}} k 'int'
+// CHECK-NEXT: UnusedAttr{{.*}}
+}
+
+template
+T TestLabelTemplate() {
+[[likely]] A: return T();
+// CHECK: FunctionTemplateDecl {{.*}} TestLabelTemplate
+// CHECK: AttributedStmt
+// CHECK-NEXT: LikelyAttr
+// CHECK-NEXT: LabelStmt {{.*}} 'A'
+}
+
+void TestLabelTemplateInstantiate() {
+TestLabelTemplate();
+// CHECK: FunctionDecl {{.*}} TestLabelTemplate 'int ()'
+// CHECK: AttributedStmt
+// CHECK-NEXT: LikelyAttr
+// CHECK-NEXT: LabelStmt {{.*}} 'A'
 }
 
 namespace Test {
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -1302,7 +1302,10 @@
   /// Subclasses may override this routine to provide different behavior.
   StmtResult RebuildLabelStmt(SourceLocation IdentLoc, LabelDecl *L,
   SourceLocation ColonLoc, Stmt *SubStmt) {
-return SemaRef.ActOnLabelStmt(IdentLoc, L, ColonLoc, SubStmt);
+// The attributes are already processed in template declaration and are
+// already properly rebuild. Only the LabelStmt itself needs to be rebuild.
+return SemaRef.ActOnLabelStmt(IdentLoc, nullptr, SourceLocation(), L,
+  ColonLoc, SubStmt);
   }
 
   /// Build a new attributed statement.
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -531,9 +531,10 @@
   return DS;
 }
 
-StmtResult
-Sema::ActOnLabelStmt(SourceLocation IdentLoc, LabelDecl *TheDecl,
- SourceLocation ColonLoc, Stmt *SubStmt

[PATCH] D17993: [CodeGen] Apply 'nonnull' to 'this' pointer arguments.

2020-10-31 Thread CJ Johnson via Phabricator via cfe-commits
CJ-Johnson updated this revision to Diff 302107.
CJ-Johnson marked an inline comment as done.

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

https://reviews.llvm.org/D17993

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CXX/except/except.spec/p14-ir.cpp
  clang/test/CodeGen/arm64-microsoft-arguments.cpp
  clang/test/CodeGen/attr-nomerge.cpp
  clang/test/CodeGen/no-builtin.cpp
  clang/test/CodeGen/temporary-lifetime.cpp
  clang/test/CodeGenCUDA/device-var-init.cu
  clang/test/CodeGenCXX/2011-12-19-init-list-ctor.cpp
  
clang/test/CodeGenCXX/RelativeVTablesABI/child-inheritted-from-parent-in-comdat.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/child-vtable-in-comdat.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/cross-translation-unit-1.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/cross-translation-unit-2.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/diamond-virtual-inheritance.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/member-function-pointer.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/multiple-inheritance.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/parent-and-child-in-comdats.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/parent-vtable-in-comdat.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/pass-byval-attributes.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/simple-vtable-definition.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/vbase-offset.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/virtual-function-call.cpp
  clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
  clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp
  clang/test/CodeGenCXX/aix-static-init.cpp
  clang/test/CodeGenCXX/amdgcn-automatic-variable.cpp
  clang/test/CodeGenCXX/amdgcn-func-arg.cpp
  clang/test/CodeGenCXX/apple-kext-indirect-call.cpp
  clang/test/CodeGenCXX/apple-kext-indirect-virtual-dtor-call.cpp
  clang/test/CodeGenCXX/apple-kext.cpp
  clang/test/CodeGenCXX/arm.cpp
  clang/test/CodeGenCXX/arm64-constructor-return.cpp
  clang/test/CodeGenCXX/array-default-argument.cpp
  clang/test/CodeGenCXX/atomicinit.cpp
  clang/test/CodeGenCXX/attr-disable-tail-calls.cpp
  clang/test/CodeGenCXX/attr-target-mv-member-funcs.cpp
  clang/test/CodeGenCXX/attr-target-mv-out-of-line-defs.cpp
  clang/test/CodeGenCXX/attr.cpp
  clang/test/CodeGenCXX/auto-variable-template.cpp
  clang/test/CodeGenCXX/blocks-cxx11.cpp
  clang/test/CodeGenCXX/blocks.cpp
  clang/test/CodeGenCXX/builtin-source-location.cpp
  clang/test/CodeGenCXX/builtin_LINE.cpp
  clang/test/CodeGenCXX/catch-undef-behavior.cpp
  clang/test/CodeGenCXX/cfi-cross-dso.cpp
  clang/test/CodeGenCXX/conditional-gnu-ext.cpp
  clang/test/CodeGenCXX/const-init-cxx11.cpp
  clang/test/CodeGenCXX/constructor-destructor-return-this.cpp
  clang/test/CodeGenCXX/constructor-direct-call.cpp
  clang/test/CodeGenCXX/constructor-init.cpp
  clang/test/CodeGenCXX/constructors.cpp
  clang/test/CodeGenCXX/copy-constructor-elim-2.cpp
  clang/test/CodeGenCXX/copy-constructor-synthesis.cpp
  clang/test/CodeGenCXX/cxx0x-delegating-ctors.cpp
  clang/test/CodeGenCXX/cxx0x-initializer-constructors.cpp
  clang/test/CodeGenCXX/cxx0x-initializer-stdinitializerlist.cpp
  clang/test/CodeGenCXX/cxx11-initializer-aggregate.cpp
  clang/test/CodeGenCXX/cxx11-initializer-array-new.cpp
  clang/test/CodeGenCXX/cxx11-thread-local.cpp
  clang/test/CodeGenCXX/cxx1y-init-captures.cpp
  clang/test/CodeGenCXX/cxx1y-sized-deallocation.cpp
  clang/test/CodeGenCXX/cxx1z-copy-omission.cpp
  clang/test/CodeGenCXX/cxx1z-decomposition.cpp
  clang/test/CodeGenCXX/cxx1z-initializer-aggregate.cpp
  clang/test/CodeGenCXX/cxx1z-lambda-star-this.cpp
  clang/test/CodeGenCXX/cxx2a-destroying-delete.cpp
  clang/test/CodeGenCXX/debug-info-class.cpp
  clang/test/CodeGenCXX/debug-info-ms-dtor-thunks.cpp
  clang/test/CodeGenCXX/default-arg-temps.cpp
  clang/test/CodeGenCXX/default-arguments.cpp
  clang/test/CodeGenCXX/delete.cpp
  clang/test/CodeGenCXX/derived-to-base-conv.cpp
  clang/test/CodeGenCXX/destructors.cpp
  clang/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp
  clang/test/CodeGenCXX/devirtualize-virtual-function-calls.cpp
  clang/test/CodeGenCXX/dllexport-ctor-closure.cpp
  clang/test/CodeGenCXX/dllexport-members.cpp
  clang/test/CodeGenCXX/dllexport.cpp
  clang/test/CodeGenCXX/dllimport-dtor-thunks.cpp
  clang/test/CodeGenCXX/dllimport-members.cpp
  clang/test/CodeGenCXX/dllimport.cpp
  clang/test/CodeGenCXX/duplicate-mangled-name.cpp
  clang/test/CodeGenCXX/eh.cpp
  clang/test/CodeGenCXX/empty-nontrivially-copyable.cpp
  clang/test/CodeGenCXX/exceptions-seh-filter-captures.cpp
  clang/test/CodeGenCXX/exceptions-seh.cpp
  clang/test/CodeGenCXX/exceptions.cpp
  clang/test/CodeGenCXX/ext-int.cpp
  clang/test/CodeGenCXX/float128-declarations.cpp
  clang/test/CodeGenCXX/float16-declarations.cpp
  clang/test/CodeGenCXX/global-dtor-no-atexit.cpp
  clang/test/CodeGenCXX/global-init.cpp
  clang/test/CodeGenCXX/goto.cpp
  clang/test/CodeGenCXX/hidden-dllimport.cpp
  clang/test/CodeGen

[PATCH] D17993: [CodeGen] Apply 'nonnull' to 'this' pointer arguments.

2020-10-31 Thread CJ Johnson via Phabricator via cfe-commits
CJ-Johnson added a comment.

The new tests can be found in this-nonnull.cpp: 
https://reviews.llvm.org/differential/changeset/?ref=2242268


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

https://reviews.llvm.org/D17993

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


[PATCH] D90540: [Syntax] Add minimal TableGen for syntax nodes. NFC

2020-10-31 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
sammccall added reviewers: eduucaldas, gribozavr, hokein.
Herald added subscribers: cfe-commits, mgrang, mgorny.
Herald added a project: clang.
sammccall requested review of this revision.

So far, only used to generate Kind and implement classof().

My plan is to have this general-purpose Nodes.inc in the style of AST
DeclNodes.inc etc, and additionally a special-purpose backend generating
the actual class definitions. But baby steps...


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90540

Files:
  clang/include/clang/CMakeLists.txt
  clang/include/clang/Tooling/Syntax/CMakeLists.txt
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/include/clang/Tooling/Syntax/Nodes.td
  clang/include/clang/Tooling/Syntax/Syntax.td
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/lib/Tooling/Syntax/Tree.cpp
  clang/utils/TableGen/CMakeLists.txt
  clang/utils/TableGen/ClangSyntaxEmitter.cpp
  clang/utils/TableGen/TableGen.cpp
  clang/utils/TableGen/TableGenBackends.h

Index: clang/utils/TableGen/TableGenBackends.h
===
--- clang/utils/TableGen/TableGenBackends.h
+++ clang/utils/TableGen/TableGenBackends.h
@@ -83,6 +83,9 @@
  llvm::raw_ostream &OS);
 void EmitClangOpcodes(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 
+void EmitClangSyntaxNodeList(llvm::RecordKeeper &Records,
+ llvm::raw_ostream &OS);
+
 void EmitNeon(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 void EmitFP16(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 void EmitBF16(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
Index: clang/utils/TableGen/TableGen.cpp
===
--- clang/utils/TableGen/TableGen.cpp
+++ clang/utils/TableGen/TableGen.cpp
@@ -55,6 +55,7 @@
   GenClangTypeWriter,
   GenClangOpcodes,
   GenClangSACheckers,
+  GenClangSyntaxNodeList,
   GenClangCommentHTMLTags,
   GenClangCommentHTMLTagsProperties,
   GenClangCommentHTMLNamedCharacterReferences,
@@ -166,6 +167,8 @@
"Generate Clang constexpr interpreter opcodes"),
 clEnumValN(GenClangSACheckers, "gen-clang-sa-checkers",
"Generate Clang Static Analyzer checkers"),
+clEnumValN(GenClangSyntaxNodeList, "gen-clang-syntax-node-list",
+   "Generate list of Clang Syntax Tree node types"),
 clEnumValN(GenClangCommentHTMLTags, "gen-clang-comment-html-tags",
"Generate efficient matchers for HTML tag "
"names that are used in documentation comments"),
@@ -356,6 +359,9 @@
   case GenClangOpenCLBuiltins:
 EmitClangOpenCLBuiltins(Records, OS);
 break;
+  case GenClangSyntaxNodeList:
+EmitClangSyntaxNodeList(Records, OS);
+break;
   case GenArmNeon:
 EmitNeon(Records, OS);
 break;
Index: clang/utils/TableGen/ClangSyntaxEmitter.cpp
===
--- /dev/null
+++ clang/utils/TableGen/ClangSyntaxEmitter.cpp
@@ -0,0 +1,128 @@
+//===- ClangSyntaxEmitter.cpp - Generate clang Syntax Tree nodes --===//
+//
+// The LLVM Compiler Infrastructure
+//
+// 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
+//
+//===--===//
+//
+// These backends consume the definitions of Syntax Tree nodes.
+// See clang/include/clang/Tooling/Syntax/{Syntax,Nodes}.td
+//
+// The -gen-clang-syntax-node-list backend produces a .inc with macro calls
+//   NODE(Kind, BaseKind)
+//   ABSTRACT_NODE(Type, Base, FirstKind, LastKind)
+// similar to those for AST nodes such as AST/DeclNodes.inc.
+//
+// In future, the class definitions themselves will be moved produced by
+// additional backends.
+//
+//===--===//
+#include "TableGenBackends.h"
+
+#include 
+
+#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/TableGen/Record.h"
+
+namespace {
+
+// The class hierarchy of Node types.
+// We assemble this in order to be able to define the NodeKind enum in a
+// stable and useful way, where abstract Node subclasses correspond to ranges.
+class Hierarchy {
+public:
+  Hierarchy(const llvm::RecordKeeper &Records) {
+for (llvm::Record *T : Records.getAllDerivedDefinitions("NodeType"))
+  add(T);
+for (llvm::Record *Derived : Records.getAllDerivedDefinitions("NodeType"))
+  if (llvm::Record *Base = Derived->getValueAsOptionalDef("base"))
+link(Derived, Base);
+for (NodeType &N : AllTypes)
+  llvm::sort(N.Derived, [](const NodeType *L, const NodeType *R) {
+return L->Record->getName() < R->Record->getName();
+ 

[PATCH] D63852: [Clang] Move assembler into a separate file

2020-10-31 Thread Ayke via Phabricator via cfe-commits
aykevl added a comment.

ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63852

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


[PATCH] D90540: [Syntax] Add minimal TableGen for syntax nodes. NFC

2020-10-31 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clang/lib/Tooling/Syntax/Tree.cpp:389
   case NodeKind::ParameterDeclarationList:
+  case NodeKind::DeclaratorList:
 return clang::tok::comma;

OK, this is not *quite* NFC:
 - before this patch, `List::classof(DeclaratorList*)` incorrectly returned 
false ,now it's true...
 - ...which uncovered these unhandled cases

I'll split these changes into another patch and land that first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90540

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


[PATCH] D90541: [Syntax] DeclaratorList is a List

2020-10-31 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
sammccall added a reviewer: eduucaldas.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
sammccall requested review of this revision.

I think this was just an oversight.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90541

Files:
  clang/lib/Tooling/Syntax/Tree.cpp


Index: clang/lib/Tooling/Syntax/Tree.cpp
===
--- clang/lib/Tooling/Syntax/Tree.cpp
+++ clang/lib/Tooling/Syntax/Tree.cpp
@@ -305,6 +305,7 @@
   case syntax::NodeKind::NestedNameSpecifier:
   case syntax::NodeKind::CallArguments:
   case syntax::NodeKind::ParameterDeclarationList:
+  case syntax::NodeKind::DeclaratorList:
 return true;
   default:
 return false;
@@ -405,6 +406,7 @@
 return clang::tok::coloncolon;
   case NodeKind::CallArguments:
   case NodeKind::ParameterDeclarationList:
+  case NodeKind::DeclaratorList:
 return clang::tok::comma;
   default:
 llvm_unreachable("This is not a subclass of List, thus "
@@ -418,6 +420,7 @@
 return TerminationKind::Terminated;
   case NodeKind::CallArguments:
   case NodeKind::ParameterDeclarationList:
+  case NodeKind::DeclaratorList:
 return TerminationKind::Separated;
   default:
 llvm_unreachable("This is not a subclass of List, thus "
@@ -433,6 +436,8 @@
 return true;
   case NodeKind::ParameterDeclarationList:
 return true;
+  case NodeKind::DeclaratorList:
+return true;
   default:
 llvm_unreachable("This is not a subclass of List, thus canBeEmpty() "
  "cannot be called");


Index: clang/lib/Tooling/Syntax/Tree.cpp
===
--- clang/lib/Tooling/Syntax/Tree.cpp
+++ clang/lib/Tooling/Syntax/Tree.cpp
@@ -305,6 +305,7 @@
   case syntax::NodeKind::NestedNameSpecifier:
   case syntax::NodeKind::CallArguments:
   case syntax::NodeKind::ParameterDeclarationList:
+  case syntax::NodeKind::DeclaratorList:
 return true;
   default:
 return false;
@@ -405,6 +406,7 @@
 return clang::tok::coloncolon;
   case NodeKind::CallArguments:
   case NodeKind::ParameterDeclarationList:
+  case NodeKind::DeclaratorList:
 return clang::tok::comma;
   default:
 llvm_unreachable("This is not a subclass of List, thus "
@@ -418,6 +420,7 @@
 return TerminationKind::Terminated;
   case NodeKind::CallArguments:
   case NodeKind::ParameterDeclarationList:
+  case NodeKind::DeclaratorList:
 return TerminationKind::Separated;
   default:
 llvm_unreachable("This is not a subclass of List, thus "
@@ -433,6 +436,8 @@
 return true;
   case NodeKind::ParameterDeclarationList:
 return true;
+  case NodeKind::DeclaratorList:
+return true;
   default:
 llvm_unreachable("This is not a subclass of List, thus canBeEmpty() "
  "cannot be called");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D89087: [MemProf] Pass down memory profile name with optional path from clang

2020-10-31 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson updated this revision to Diff 302112.
tejohnson added a comment.

Rebase and fix bad merge


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89087

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/memory-profile-filename.c
  clang/test/Driver/fmemprof.cpp
  compiler-rt/test/memprof/TestCases/atexit_stats.cpp
  compiler-rt/test/memprof/TestCases/dump_process_map.cpp
  compiler-rt/test/memprof/TestCases/log_path_test.cpp
  compiler-rt/test/memprof/TestCases/malloc-size-too-big.cpp
  compiler-rt/test/memprof/TestCases/mem_info_cache_entries.cpp
  compiler-rt/test/memprof/TestCases/print_miss_rate.cpp
  compiler-rt/test/memprof/TestCases/stress_dtls.c
  compiler-rt/test/memprof/TestCases/test_malloc_load_store.c
  compiler-rt/test/memprof/TestCases/test_memintrin.cpp
  compiler-rt/test/memprof/TestCases/test_new_load_store.cpp
  compiler-rt/test/memprof/TestCases/test_terse.cpp
  compiler-rt/test/memprof/TestCases/unaligned_loads_and_stores.cpp
  llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
  llvm/test/Instrumentation/HeapProfiler/filename.ll

Index: llvm/test/Instrumentation/HeapProfiler/filename.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HeapProfiler/filename.ll
@@ -0,0 +1,15 @@
+; Test to ensure that the filename provided by clang in the module flags
+; metadata results in the expected __memprof_profile_filename insertion.
+
+; RUN: opt < %s -mtriple=x86_64-unknown-linux -memprof -memprof-module -S | FileCheck --check-prefixes=CHECK %s
+
+define i32 @main() {
+entry:
+  ret i32 0
+}
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"MemProfProfileFilename", !"/tmp/memprof.profraw"}
+
+; CHECK: $__memprof_profile_filename = comdat any
+; CHECK: @__memprof_profile_filename = constant [21 x i8] c"/tmp/memprof.profraw\00", comdat
Index: llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
===
--- llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
+++ llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
@@ -60,6 +60,8 @@
 constexpr char MemProfShadowMemoryDynamicAddress[] =
 "__memprof_shadow_memory_dynamic_address";
 
+constexpr char MemProfFilenameVar[] = "__memprof_profile_filename";
+
 // Command-line flags.
 
 static cl::opt ClInsertVersionCheck(
@@ -486,6 +488,26 @@
   IRB.CreateStore(ShadowValue, ShadowAddr);
 }
 
+// Create the variable for the profile file name.
+void createProfileFileNameVar(Module &M) {
+  const MDString *MemProfFilename =
+  dyn_cast_or_null(M.getModuleFlag("MemProfProfileFilename"));
+  if (!MemProfFilename)
+return;
+  assert(!MemProfFilename->getString().empty() &&
+ "Unexpected MemProfProfileFilename metadata with empty string");
+  Constant *ProfileNameConst = ConstantDataArray::getString(
+  M.getContext(), MemProfFilename->getString(), true);
+  GlobalVariable *ProfileNameVar = new GlobalVariable(
+  M, ProfileNameConst->getType(), /*isConstant=*/true,
+  GlobalValue::WeakAnyLinkage, ProfileNameConst, MemProfFilenameVar);
+  Triple TT(M.getTargetTriple());
+  if (TT.supportsCOMDAT()) {
+ProfileNameVar->setLinkage(GlobalValue::ExternalLinkage);
+ProfileNameVar->setComdat(M.getOrInsertComdat(MemProfFilenameVar));
+  }
+}
+
 bool ModuleMemProfiler::instrumentModule(Module &M) {
   // Create a module constructor.
   std::string MemProfVersion = std::to_string(LLVM_MEM_PROFILER_VERSION);
@@ -500,6 +522,8 @@
   const uint64_t Priority = getCtorAndDtorPriority(TargetTriple);
   appendToGlobalCtors(M, MemProfCtorFunction, Priority);
 
+  createProfileFileNameVar(M);
+
   return true;
 }
 
Index: compiler-rt/test/memprof/TestCases/unaligned_loads_and_stores.cpp
===
--- compiler-rt/test/memprof/TestCases/unaligned_loads_and_stores.cpp
+++ compiler-rt/test/memprof/TestCases/unaligned_loads_and_stores.cpp
@@ -1,4 +1,4 @@
-// RUN: %clangxx_memprof -O0 %s -o %t && %run %t 2>&1 | FileCheck %s
+// RUN: %clangxx_memprof -O0 %s -o %t && %env_memprof_opts=log_path=stderr %run %t 2>&1 | FileCheck %s
 
 // This is actually:
 //  Memory allocation stack id = STACKID
Index: compiler-rt/test/memprof/TestCases/test_terse.cpp
===
--- compiler-rt/test/memprof/TestCases/test_terse.cpp
+++ compiler-rt/test/memprof/TestCases/test_terse.cpp
@@ -3,10 +3,10 @@
 // deallocated before exit.
 
 // RUN: %clangxx_memprof -O0 %s -o %t
-// RUN: %env_memprof_opts=print_terse=1 %run %t 2>&1 | FileCheck %s
+// RUN: %env_memprof

[PATCH] D88410: [clang][AVR] Improve avr-ld command line options

2020-10-31 Thread Ayke via Phabricator via cfe-commits
aykevl added a comment.

I haven't verified this all but this looks reasonable to me, at least until a 
better way is figured out to store MCU specific information in the compiler.

The tests may be a little bit excessive though, they all seem to be testing the 
same thing.


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

https://reviews.llvm.org/D88410

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


[PATCH] D90397: [clangd] Value initialize SymbolIDs

2020-10-31 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment.

Would it have been possible to disallow default-constructing `SymbolID` 
altogether, and preserve the ability to represent "an always-valid symbol id" 
(`SymbolID`) vs. "a maybe-valid symbol id" (`Optional`) as distinct 
types in the type system?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90397

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


[PATCH] D90535: [clang-tidy] Allow -warnings-as-errors to be specified from run_clang_tidy.py

2020-10-31 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

Just a general drive by comment, there have been quite a few patches recently 
that add arguments that just get forwarded straight to clang-tidy, would it not 
be a whole lot simpler if we could just automatically forward arguments 
straight to clang-tidy.
clang-tidy itself uses the `--` argument to signify any arguments after that 
get forwarded to the clang driver, this script could probably do the same.
The whole invocation could be like this:
`py run_clang_tidy.py  --  -- 
`
So a general use case could look like this:
`py run_clang_tidy.py -p ./compile_commands.json -quiet -- -check=-*,bugprone* 
-warnings-as-errors=* -- -UNDEBUG`

Anyway aside from that, please upload diffs with the full context, see 
https://llvm.org/docs/Phabricator.html#phabricator-request-review-web for more 
info.




Comment at: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py:242
+  help='Upgrades warnings to errors. Does not enable '
+  'any warnings py itself.')
   args = parser.parse_args()

s/py/by


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90535

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


[PATCH] D89087: [MemProf] Pass down memory profile name with optional path from clang

2020-10-31 Thread David Li via Phabricator via cfe-commits
davidxl accepted this revision.
davidxl 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/D89087/new/

https://reviews.llvm.org/D89087

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


[PATCH] D89087: [MemProf] Pass down memory profile name with optional path from clang

2020-10-31 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: clang/include/clang/Driver/Options.td:1021
+def fmemory_profile_EQ : Joined<["-"], "fmemory-profile=">,
+Group, Flags<[CC1Option, DriverOption]>, 
MetaVarName<"">,
+HelpText<"Enable heap memory profiling and dump results into ">;

DriverOption has been renamed to NoXarchOption (which is just used to provide a 
diagnostic for -Xarch, which may be removed in the future). Please remove the 
flag



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4316
+  if (MemProfArg) {
+if (MemProfArg->getOption().matches(options::OPT_fmemory_profile))
+  Args.AddLastArg(CmdArgs, options::OPT_fmemory_profile);

if not match  options::OPT_fno_memory_profile, `render(Args, CmdArgs)`



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1042
+  // The memory profile runtime appends the pid to make this name more unique.
+  std::string MemProfileBasename = "memprof.profraw";
+  if (Args.hasArg(OPT_fmemory_profile_EQ)) {

const StringLiteral



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1049
+  } else if (Args.hasArg(OPT_fmemory_profile))
+Opts.MemoryProfileOutput = MemProfileBasename;
 

std::string(MemProfileBasename)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89087

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


[PATCH] D90543: [Syntax] Start to move trivial Node class definitions to TableGen. NFC

2020-10-31 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
sammccall added reviewers: gribozavr2, hokein.
Herald added subscribers: cfe-commits, mgorny.
Herald added a project: clang.
sammccall requested review of this revision.

This defines two node archetypes with trivial class definitions:

- Alternatives: the generated abstract classes are trivial as all functionality 
is via LLVM RTTI
- Unconstrained: this is a placeholder, I think all of these are going to be 
Lists but today they have no special accessors etc, so we just say "could 
contain anything", and migrate them one-by-one to Sequence later.

Compared to Dmitri's prototype, Nodes.td looks more like a class hierarchy and
less like a grammar. (E.g. variants list the Alternatives parent rather than
vice versa).
The main reasons for this:

- the hierarchy is an important part of the API we want direct control over.
  - e.g. we may introduce abstract bases like "loop" that the grammar doesn't 
care about in order to model is-a concepts that might make refactorings more 
expressive. This is less natural in a grammar-like idiom.
  - e.g. we're likely to have to model some alternatives as variants and others 
as class hierarchies, the choice will probably be based on natural is-a 
relationships.
- it reduces the cognitive load of switching from editing *.td to working with 
code that uses the generated classes


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90543

Files:
  clang/include/clang/Tooling/Syntax/CMakeLists.txt
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/include/clang/Tooling/Syntax/Nodes.td
  clang/include/clang/Tooling/Syntax/Syntax.td
  clang/utils/TableGen/ClangSyntaxEmitter.cpp
  clang/utils/TableGen/TableGen.cpp
  clang/utils/TableGen/TableGenBackends.h

Index: clang/utils/TableGen/TableGenBackends.h
===
--- clang/utils/TableGen/TableGenBackends.h
+++ clang/utils/TableGen/TableGenBackends.h
@@ -85,6 +85,8 @@
 
 void EmitClangSyntaxNodeList(llvm::RecordKeeper &Records,
  llvm::raw_ostream &OS);
+void EmitClangSyntaxNodeClasses(llvm::RecordKeeper &Records,
+llvm::raw_ostream &OS);
 
 void EmitNeon(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 void EmitFP16(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
Index: clang/utils/TableGen/TableGen.cpp
===
--- clang/utils/TableGen/TableGen.cpp
+++ clang/utils/TableGen/TableGen.cpp
@@ -56,6 +56,7 @@
   GenClangOpcodes,
   GenClangSACheckers,
   GenClangSyntaxNodeList,
+  GenClangSyntaxNodeClasses,
   GenClangCommentHTMLTags,
   GenClangCommentHTMLTagsProperties,
   GenClangCommentHTMLNamedCharacterReferences,
@@ -169,6 +170,8 @@
"Generate Clang Static Analyzer checkers"),
 clEnumValN(GenClangSyntaxNodeList, "gen-clang-syntax-node-list",
"Generate list of Clang Syntax Tree node types"),
+clEnumValN(GenClangSyntaxNodeClasses, "gen-clang-syntax-node-classes",
+   "Generate definitions of Clang Syntax Tree node clasess"),
 clEnumValN(GenClangCommentHTMLTags, "gen-clang-comment-html-tags",
"Generate efficient matchers for HTML tag "
"names that are used in documentation comments"),
@@ -362,6 +365,9 @@
   case GenClangSyntaxNodeList:
 EmitClangSyntaxNodeList(Records, OS);
 break;
+  case GenClangSyntaxNodeClasses:
+EmitClangSyntaxNodeClasses(Records, OS);
+break;
   case GenArmNeon:
 EmitNeon(Records, OS);
 break;
Index: clang/utils/TableGen/ClangSyntaxEmitter.cpp
===
--- clang/utils/TableGen/ClangSyntaxEmitter.cpp
+++ clang/utils/TableGen/ClangSyntaxEmitter.cpp
@@ -16,19 +16,24 @@
 //   ABSTRACT_NODE(Type, Base, FirstKind, LastKind)
 // similar to those for AST nodes such as AST/DeclNodes.inc.
 //
-// In future, the class definitions themselves will be moved produced by
-// additional backends.
+// The -gen-clang-syntax-node-classes backend produces definitions for the
+// syntax::Node subclasses (except those marked as External).
+//
+// In future, another backend will encode the structure of the various node
+// types in tables so their invariants can be checked and enforced.
 //
 //===--===//
 #include "TableGenBackends.h"
 
 #include 
 
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/TableGen/Record.h"
 
 namespace {
+using llvm::formatv;
 
 // The class hierarchy of Node types.
 // We assemble this in order to be able to define the NodeKind enum in a
@@ -41,10 +46,15 @@
 for (llvm::Record *Derived : Records.getAllDerivedDefinitions("NodeType"))
   if (llvm::Record *Base = Derived->getValueAsOptionalDef("base"))
 link(Derived, 

[PATCH] D90543: [Syntax] Start to move trivial Node class definitions to TableGen. NFC

2020-10-31 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

There are *lots* more nodes that can be converted here, I'll wait to review the 
pattern first :-)

The generated code is identical to the deleted code, except the constructor for 
abstract classes is protected now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90543

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


[PATCH] D90397: [clangd] Value initialize SymbolIDs

2020-10-31 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

In D90397#2366727 , @nridge wrote:

> Would it have been possible to disallow default-constructing `SymbolID` 
> altogether, and preserve the ability to represent "an always-valid symbol id" 
> (`SymbolID`) vs. "a maybe-valid symbol id" (`Optional`) as distinct 
> types in the type system?

Absolutely, except where it matters`sizeof(SymbolID)==8` and 
`sizeof(Optional)==16`.
I think the trigger here was `SymbolID Ref::Container` - we can't afford to use 
`Optional` there.
We could come up with some special-case solution for that and use Optional 
elsewhere - I don't really feel strongly about it.




Comment at: clang-tools-extra/clangd/AST.h:67
 
 /// Gets the symbol ID for a declaration, if possible.
+SymbolID getSymbolID(const Decl *D);

probably want to be a bit more specific about the possibility of returning null 
(and below)



Comment at: clang-tools-extra/clangd/index/SymbolID.h:57
 
+  bool isNull() const { return HashValue != std::array{}; }
+  operator bool() const { return isNull(); }

just `return *this == SymbolID()`?



Comment at: clang-tools-extra/clangd/index/SymbolID.h:58
+  bool isNull() const { return HashValue != std::array{}; }
+  operator bool() const { return isNull(); }
+

nit: I think you want this to be explicit. Note that if(x) **will** perform an 
explicit cast if needed



Comment at: clang-tools-extra/clangd/index/SymbolID.h:58
+  bool isNull() const { return HashValue != std::array{}; }
+  operator bool() const { return isNull(); }
+

sammccall wrote:
> nit: I think you want this to be explicit. Note that if(x) **will** perform 
> an explicit cast if needed
this should be `!isNull()`!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90397

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


[PATCH] D90544: [X86] NOT FOR COMMIT. Emit lifetime markers for MXCSR temporaries.

2020-10-31 Thread Craig Topper via Phabricator via cfe-commits
craig.topper created this revision.
craig.topper added reviewers: andreadb, RKSimon, spatel.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
craig.topper requested review of this revision.

My experimental patch for PR48033 so I can share it in the bug.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90544

Files:
  clang/lib/CodeGen/CGBuiltin.cpp


Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -11895,16 +11895,28 @@
   case X86::BI_mm_setcsr:
   case X86::BI__builtin_ia32_ldmxcsr: {
 Address Tmp = CreateMemTemp(E->getArg(0)->getType());
+llvm::Value *TmpPtr = Tmp.getPointer();
+llvm::Value *TmpSize = EmitLifetimeStart(
+CGM.getDataLayout().getTypeAllocSize(Tmp.getElementType()), TmpPtr);
 Builder.CreateStore(Ops[0], Tmp);
-return Builder.CreateCall(CGM.getIntrinsic(Intrinsic::x86_sse_ldmxcsr),
-  Builder.CreateBitCast(Tmp.getPointer(), Int8PtrTy));
+Value *Call = 
Builder.CreateCall(CGM.getIntrinsic(Intrinsic::x86_sse_ldmxcsr),
+ Builder.CreateBitCast(Tmp.getPointer(), 
Int8PtrTy));
+if (TmpSize)
+  EmitLifetimeEnd(TmpSize, TmpPtr);
+return Call;
   }
   case X86::BI_mm_getcsr:
   case X86::BI__builtin_ia32_stmxcsr: {
 Address Tmp = CreateMemTemp(E->getType());
+llvm::Value *TmpPtr = Tmp.getPointer();
+llvm::Value *TmpSize = EmitLifetimeStart(
+CGM.getDataLayout().getTypeAllocSize(Tmp.getElementType()), TmpPtr);
 Builder.CreateCall(CGM.getIntrinsic(Intrinsic::x86_sse_stmxcsr),
Builder.CreateBitCast(Tmp.getPointer(), Int8PtrTy));
-return Builder.CreateLoad(Tmp, "stmxcsr");
+Value *Call = Builder.CreateLoad(Tmp, "stmxcsr");
+if (TmpSize)
+  EmitLifetimeEnd(TmpSize, TmpPtr);
+return Call;
   }
   case X86::BI__builtin_ia32_xsave:
   case X86::BI__builtin_ia32_xsave64:


Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -11895,16 +11895,28 @@
   case X86::BI_mm_setcsr:
   case X86::BI__builtin_ia32_ldmxcsr: {
 Address Tmp = CreateMemTemp(E->getArg(0)->getType());
+llvm::Value *TmpPtr = Tmp.getPointer();
+llvm::Value *TmpSize = EmitLifetimeStart(
+CGM.getDataLayout().getTypeAllocSize(Tmp.getElementType()), TmpPtr);
 Builder.CreateStore(Ops[0], Tmp);
-return Builder.CreateCall(CGM.getIntrinsic(Intrinsic::x86_sse_ldmxcsr),
-  Builder.CreateBitCast(Tmp.getPointer(), Int8PtrTy));
+Value *Call = Builder.CreateCall(CGM.getIntrinsic(Intrinsic::x86_sse_ldmxcsr),
+ Builder.CreateBitCast(Tmp.getPointer(), Int8PtrTy));
+if (TmpSize)
+  EmitLifetimeEnd(TmpSize, TmpPtr);
+return Call;
   }
   case X86::BI_mm_getcsr:
   case X86::BI__builtin_ia32_stmxcsr: {
 Address Tmp = CreateMemTemp(E->getType());
+llvm::Value *TmpPtr = Tmp.getPointer();
+llvm::Value *TmpSize = EmitLifetimeStart(
+CGM.getDataLayout().getTypeAllocSize(Tmp.getElementType()), TmpPtr);
 Builder.CreateCall(CGM.getIntrinsic(Intrinsic::x86_sse_stmxcsr),
Builder.CreateBitCast(Tmp.getPointer(), Int8PtrTy));
-return Builder.CreateLoad(Tmp, "stmxcsr");
+Value *Call = Builder.CreateLoad(Tmp, "stmxcsr");
+if (TmpSize)
+  EmitLifetimeEnd(TmpSize, TmpPtr);
+return Call;
   }
   case X86::BI__builtin_ia32_xsave:
   case X86::BI__builtin_ia32_xsave64:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 1a51bde - [test] Clean up test/Frontend/gnu-mcount.c and fix unused check prefixes

2020-10-31 Thread Fangrui Song via cfe-commits

Author: Fangrui Song
Date: 2020-10-31T21:33:46-07:00
New Revision: 1a51bde1b62511b0a3ecf0f62807acb4bb860965

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

LOG: [test] Clean up test/Frontend/gnu-mcount.c and fix unused check prefixes

Added: 


Modified: 
clang/test/Frontend/gnu-mcount.c

Removed: 




diff  --git a/clang/test/Frontend/gnu-mcount.c 
b/clang/test/Frontend/gnu-mcount.c
index 6e89dfefb750..85e56f84506a 100644
--- a/clang/test/Frontend/gnu-mcount.c
+++ b/clang/test/Frontend/gnu-mcount.c
@@ -1,41 +1,41 @@
 // REQUIRES: arm-registered-target,aarch64-registered-target
 
-// RUN: %clang -target armv7-unknown-none-eabi -pg -S -emit-llvm -o - %s | 
FileCheck %s -check-prefix CHECK -check-prefix CHECK-ARM-BAREMETAL-EABI
-// RUN: %clang -target armv7-unknown-none-eabi -pg -meabi gnu -S -emit-llvm -o 
- %s | FileCheck %s -check-prefix CHECK -check-prefix 
CHECK-ARM-BAREMETAL-EABI-MEABI-GNU
-// RUN: %clang -target aarch64-unknown-none-eabi -pg -S -emit-llvm -o - %s | 
FileCheck %s -check-prefix CHECK -check-prefix CHECK-ARM64-BAREMETAL-EABI
-// RUN: %clang -target aarch64-unknown-none-eabi -pg -meabi gnu -S -emit-llvm 
-o - %s | FileCheck %s -check-prefix CHECK -check-prefix 
CHECK-ARM64-BAREMETAL-EABI-MEABI-GNU
+// RUN: %clang -target armv7-unknown-none-eabi -pg -S -emit-llvm -o - %s | 
FileCheck %s -check-prefixes=CHECK,UNSUPPORTED
+// RUN: %clang -target armv7-unknown-none-eabi -pg -meabi gnu -S -emit-llvm -o 
- %s | FileCheck %s --check-prefixes=CHECK,UNSUPPORTED
+// RUN: %clang -target aarch64-unknown-none-eabi -pg -S -emit-llvm -o - %s | 
FileCheck %s --check-prefixes=CHECK,MCOUNT
+// RUN: %clang -target aarch64-unknown-none-eabi -pg -meabi gnu -S -emit-llvm 
-o - %s | FileCheck %s --check-prefixes=CHECK,UNDER
 // RUN: %clang -target armv7-unknown-linux-gnueabi -pg -S -emit-llvm -o - %s | 
FileCheck %s -check-prefix CHECK -check-prefix CHECK-ARM-EABI
 // RUN: %clang -target armv7-unknown-linux-gnueabi -meabi gnu -pg -S 
-emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix 
CHECK-ARM-EABI-MEABI-GNU
-// RUN: %clang -target aarch64-unknown-linux-gnueabi -pg -S -emit-llvm -o - %s 
| FileCheck %s -check-prefix CHECK -check-prefix CHECK-ARM64-EABI-LINUX
-// RUN: %clang -target aarch64-unknown-linux-gnueabi -meabi gnu -pg -S 
-emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix 
CHECK-ARM64-EABI-MEABI-GNU
+// RUN: %clang -target aarch64-unknown-linux-gnueabi -pg -S -emit-llvm -o - %s 
| FileCheck %s --check-prefixes=CHECK,UNDER
+// RUN: %clang -target aarch64-unknown-linux-gnueabi -meabi gnu -pg -S 
-emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNDER
 // RUN: %clang -target armv7-unknown-linux-gnueabihf -pg -S -emit-llvm -o - %s 
| FileCheck %s -check-prefix CHECK -check-prefix CHECK-ARM-EABI
 // RUN: %clang -target armv7-unknown-linux-gnueabihf -meabi gnu -pg -S 
-emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix 
CHECK-ARM-EABI-MEABI-GNU
-// RUN: %clang -target aarch64-unknown-linux-gnueabihf -pg -S -emit-llvm -o - 
%s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-ARM64-EABI-LINUX
-// RUN: %clang -target aarch64-unknown-linux-gnueabihf -meabi gnu -pg -S 
-emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix 
CHECK-ARM64-EABI-MEABI-GNU
-// RUN: %clang -target armv7-unknown-freebsd-gnueabihf -pg -S -emit-llvm -o - 
%s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-ARM-EABI-FREEBSD
-// RUN: %clang -target armv7-unknown-freebsd-gnueabihf -meabi gnu -pg -S 
-emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix 
CHECK-ARM-EABI-FREEBSD
+// RUN: %clang -target aarch64-unknown-linux-gnueabihf -pg -S -emit-llvm -o - 
%s | FileCheck %s --check-prefixes=CHECK,UNDER
+// RUN: %clang -target aarch64-unknown-linux-gnueabihf -meabi gnu -pg -S 
-emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNDER
+// RUN: %clang -target armv7-unknown-freebsd-gnueabihf -pg -S -emit-llvm -o - 
%s | FileCheck %s --check-prefixes=CHECK,UNDER_UNDER
+// RUN: %clang -target armv7-unknown-freebsd-gnueabihf -meabi gnu -pg -S 
-emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNDER_UNDER
 // RUN: %clang -target aarch64-unknown-freebsd-gnueabihf -pg -S -emit-llvm -o 
- %s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-ARM64-EABI-FREEBSD
 // RUN: %clang -target aarch64-unknown-freebsd-gnueabihf -meabi gnu -pg -S 
-emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix 
CHECK-ARM64-EABI-FREEBSD
-// RUN: %clang -target armv7-unknown-openbsd-gnueabihf -pg -S -emit-llvm -o - 
%s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-ARM-EABI-OPENBSD
-// RUN: %clang -target armv7-unknown-openbsd-gnueabihf -meabi gnu -pg -S 
-emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-p

[clang] 96289ce - [test] Fix unused check prefixes in test/AST

2020-10-31 Thread Fangrui Song via cfe-commits

Author: Fangrui Song
Date: 2020-10-31T21:46:45-07:00
New Revision: 96289ce6333ebf5a8405f824eaf02ef528e09de9

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

LOG: [test] Fix unused check prefixes in test/AST

Added: 


Modified: 
clang/test/AST/ast-dump-decl.c
clang/test/AST/ast-print-record-decl.c

Removed: 




diff  --git a/clang/test/AST/ast-dump-decl.c b/clang/test/AST/ast-dump-decl.c
index 0d6ef76c5231..0b5a8e93a86d 100644
--- a/clang/test/AST/ast-dump-decl.c
+++ b/clang/test/AST/ast-dump-decl.c
@@ -53,7 +53,7 @@ typedef int TestTypedefDecl;
 // CHECK:  TypedefDecl{{.*}} TestTypedefDecl 'int'
 
 __module_private__ typedef int TestTypedefDeclPrivate;
-// CHECK-MODULE:  TypedefDecl{{.*}} TestTypedefDeclPrivate 'int' 
__module_private__
+// CHECK-MODULES:  TypedefDecl{{.*}} TestTypedefDeclPrivate 'int' 
__module_private__
 
 enum TestEnumDecl {
   testEnumDecl

diff  --git a/clang/test/AST/ast-print-record-decl.c 
b/clang/test/AST/ast-print-record-decl.c
index c27fdf42f337..ed2592c3a392 100644
--- a/clang/test/AST/ast-print-record-decl.c
+++ b/clang/test/AST/ast-print-record-decl.c
@@ -7,7 +7,7 @@
 //   RUN: | FileCheck --check-prefixes=CHECK,LLVM %s
 //
 //   RUN: %clang_cc1 -verify -ast-print -DKW=struct -DBASES= %s > %t.c
-//   RUN: FileCheck --check-prefixes=CHECK,PRINT,PRINT-C -DKW=struct -DBASES= \
+//   RUN: FileCheck --check-prefixes=CHECK,PRINT -DKW=struct -DBASES= \
 //   RUN:   %s --input-file %t.c
 //
 //   Now check compiling and printing of the printed file.
@@ -19,7 +19,7 @@
 //   RUN: | FileCheck --check-prefixes=CHECK,LLVM %s
 //
 //   RUN: %clang_cc1 -verify -ast-print %t.c \
-//   RUN: | FileCheck --check-prefixes=CHECK,PRINT,PRINT-C -DKW=struct \
+//   RUN: | FileCheck --check-prefixes=CHECK,PRINT -DKW=struct \
 //   RUN: -DBASES= %s
 
 // Repeat for union:
@@ -31,7 +31,7 @@
 //   RUN: | FileCheck --check-prefixes=CHECK,LLVM %s
 //
 //   RUN: %clang_cc1 -verify -ast-print -DKW=union -DBASES= %s > %t.c
-//   RUN: FileCheck --check-prefixes=CHECK,PRINT,PRINT-C -DKW=union -DBASES= \
+//   RUN: FileCheck --check-prefixes=CHECK,PRINT -DKW=union -DBASES= \
 //   RUN:   %s --input-file %t.c
 //
 //   Now check compiling and printing of the printed file.
@@ -43,7 +43,7 @@
 //   RUN: | FileCheck --check-prefixes=CHECK,LLVM %s
 //
 //   RUN: %clang_cc1 -verify -ast-print %t.c \
-//   RUN: | FileCheck --check-prefixes=CHECK,PRINT,PRINT-C -DKW=union \
+//   RUN: | FileCheck --check-prefixes=CHECK,PRINT -DKW=union \
 //   RUN: -DBASES= %s
 
 // Repeat for C++ (BASES helps ensure we're printing as C++ not as C):



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


[PATCH] D90397: [clangd] Value initialize SymbolIDs

2020-10-31 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment.

In D90397#2366769 , @sammccall wrote:

> In D90397#2366727 , @nridge wrote:
>
>> Would it have been possible to disallow default-constructing `SymbolID` 
>> altogether, and preserve the ability to represent "an always-valid symbol 
>> id" (`SymbolID`) vs. "a maybe-valid symbol id" (`Optional`) as 
>> distinct types in the type system?
>
> Absolutely, except where it matters`sizeof(SymbolID)==8` and 
> `sizeof(Optional)==16`.
> I think the trigger here was `SymbolID Ref::Container` - we can't afford to 
> use `Optional` there.

Good point!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90397

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


[PATCH] D89087: [MemProf] Pass down memory profile name with optional path from clang

2020-10-31 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson marked 3 inline comments as done.
tejohnson added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1049
+  } else if (Args.hasArg(OPT_fmemory_profile))
+Opts.MemoryProfileOutput = MemProfileBasename;
 

MaskRay wrote:
> std::string(MemProfileBasename)
This change doesn't seem to be needed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89087

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


[PATCH] D89087: [MemProf] Pass down memory profile name with optional path from clang

2020-10-31 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson updated this revision to Diff 302119.
tejohnson added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89087

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/memory-profile-filename.c
  clang/test/Driver/fmemprof.cpp
  compiler-rt/test/memprof/TestCases/atexit_stats.cpp
  compiler-rt/test/memprof/TestCases/dump_process_map.cpp
  compiler-rt/test/memprof/TestCases/log_path_test.cpp
  compiler-rt/test/memprof/TestCases/malloc-size-too-big.cpp
  compiler-rt/test/memprof/TestCases/mem_info_cache_entries.cpp
  compiler-rt/test/memprof/TestCases/print_miss_rate.cpp
  compiler-rt/test/memprof/TestCases/stress_dtls.c
  compiler-rt/test/memprof/TestCases/test_malloc_load_store.c
  compiler-rt/test/memprof/TestCases/test_memintrin.cpp
  compiler-rt/test/memprof/TestCases/test_new_load_store.cpp
  compiler-rt/test/memprof/TestCases/test_terse.cpp
  compiler-rt/test/memprof/TestCases/unaligned_loads_and_stores.cpp
  llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
  llvm/test/Instrumentation/HeapProfiler/filename.ll

Index: llvm/test/Instrumentation/HeapProfiler/filename.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HeapProfiler/filename.ll
@@ -0,0 +1,15 @@
+; Test to ensure that the filename provided by clang in the module flags
+; metadata results in the expected __memprof_profile_filename insertion.
+
+; RUN: opt < %s -mtriple=x86_64-unknown-linux -memprof -memprof-module -S | FileCheck --check-prefixes=CHECK %s
+
+define i32 @main() {
+entry:
+  ret i32 0
+}
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"MemProfProfileFilename", !"/tmp/memprof.profraw"}
+
+; CHECK: $__memprof_profile_filename = comdat any
+; CHECK: @__memprof_profile_filename = constant [21 x i8] c"/tmp/memprof.profraw\00", comdat
Index: llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
===
--- llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
+++ llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
@@ -60,6 +60,8 @@
 constexpr char MemProfShadowMemoryDynamicAddress[] =
 "__memprof_shadow_memory_dynamic_address";
 
+constexpr char MemProfFilenameVar[] = "__memprof_profile_filename";
+
 // Command-line flags.
 
 static cl::opt ClInsertVersionCheck(
@@ -486,6 +488,26 @@
   IRB.CreateStore(ShadowValue, ShadowAddr);
 }
 
+// Create the variable for the profile file name.
+void createProfileFileNameVar(Module &M) {
+  const MDString *MemProfFilename =
+  dyn_cast_or_null(M.getModuleFlag("MemProfProfileFilename"));
+  if (!MemProfFilename)
+return;
+  assert(!MemProfFilename->getString().empty() &&
+ "Unexpected MemProfProfileFilename metadata with empty string");
+  Constant *ProfileNameConst = ConstantDataArray::getString(
+  M.getContext(), MemProfFilename->getString(), true);
+  GlobalVariable *ProfileNameVar = new GlobalVariable(
+  M, ProfileNameConst->getType(), /*isConstant=*/true,
+  GlobalValue::WeakAnyLinkage, ProfileNameConst, MemProfFilenameVar);
+  Triple TT(M.getTargetTriple());
+  if (TT.supportsCOMDAT()) {
+ProfileNameVar->setLinkage(GlobalValue::ExternalLinkage);
+ProfileNameVar->setComdat(M.getOrInsertComdat(MemProfFilenameVar));
+  }
+}
+
 bool ModuleMemProfiler::instrumentModule(Module &M) {
   // Create a module constructor.
   std::string MemProfVersion = std::to_string(LLVM_MEM_PROFILER_VERSION);
@@ -500,6 +522,8 @@
   const uint64_t Priority = getCtorAndDtorPriority(TargetTriple);
   appendToGlobalCtors(M, MemProfCtorFunction, Priority);
 
+  createProfileFileNameVar(M);
+
   return true;
 }
 
Index: compiler-rt/test/memprof/TestCases/unaligned_loads_and_stores.cpp
===
--- compiler-rt/test/memprof/TestCases/unaligned_loads_and_stores.cpp
+++ compiler-rt/test/memprof/TestCases/unaligned_loads_and_stores.cpp
@@ -1,4 +1,4 @@
-// RUN: %clangxx_memprof -O0 %s -o %t && %run %t 2>&1 | FileCheck %s
+// RUN: %clangxx_memprof -O0 %s -o %t && %env_memprof_opts=log_path=stderr %run %t 2>&1 | FileCheck %s
 
 // This is actually:
 //  Memory allocation stack id = STACKID
Index: compiler-rt/test/memprof/TestCases/test_terse.cpp
===
--- compiler-rt/test/memprof/TestCases/test_terse.cpp
+++ compiler-rt/test/memprof/TestCases/test_terse.cpp
@@ -3,10 +3,10 @@
 // deallocated before exit.
 
 // RUN: %clangxx_memprof -O0 %s -o %t
-// RUN: %env_memprof_opts=print_terse=1 %run %t 2>&1 | FileCheck %s
+// RUN: %env_memprof_opts=lo

[PATCH] D89670: [clangd] Store the containing symbol for refs

2020-10-31 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 302121.
nridge marked an inline comment as done.
nridge added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89670

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

Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -70,21 +70,16 @@
 MATCHER_P2(IncludeHeaderWithRef, IncludeHeader, References, "") {
   return (arg.IncludeHeader == IncludeHeader) && (arg.References == References);
 }
+bool rangesMatch(const SymbolLocation &Loc, const Range &R) {
+  return std::make_tuple(Loc.Start.line(), Loc.Start.column(), Loc.End.line(),
+ Loc.End.column()) ==
+ std::make_tuple(R.start.line, R.start.character, R.end.line,
+ R.end.character);
+}
 MATCHER_P(DeclRange, Pos, "") {
-  return std::make_tuple(arg.CanonicalDeclaration.Start.line(),
- arg.CanonicalDeclaration.Start.column(),
- arg.CanonicalDeclaration.End.line(),
- arg.CanonicalDeclaration.End.column()) ==
- std::make_tuple(Pos.start.line, Pos.start.character, Pos.end.line,
- Pos.end.character);
-}
-MATCHER_P(DefRange, Pos, "") {
-  return std::make_tuple(
- arg.Definition.Start.line(), arg.Definition.Start.column(),
- arg.Definition.End.line(), arg.Definition.End.column()) ==
- std::make_tuple(Pos.start.line, Pos.start.character, Pos.end.line,
- Pos.end.character);
+  return rangesMatch(arg.CanonicalDeclaration, Pos);
 }
+MATCHER_P(DefRange, Pos, "") { return rangesMatch(arg.Definition, Pos); }
 MATCHER_P(RefCount, R, "") { return int(arg.References) == R; }
 MATCHER_P(ForCodeCompletion, IsIndexedForCodeCompletion, "") {
   return static_cast(arg.Flags & Symbol::IndexedForCodeCompletion) ==
@@ -100,10 +95,7 @@
 MATCHER(RefRange, "") {
   const Ref &Pos = ::testing::get<0>(arg);
   const Range &Range = ::testing::get<1>(arg);
-  return std::make_tuple(Pos.Location.Start.line(), Pos.Location.Start.column(),
- Pos.Location.End.line(), Pos.Location.End.column()) ==
- std::make_tuple(Range.start.line, Range.start.character,
- Range.end.line, Range.end.character);
+  return rangesMatch(Pos.Location, Range);
 }
 ::testing::Matcher &>
 HaveRanges(const std::vector Ranges) {
@@ -738,6 +730,85 @@
   EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(Symbols, "FUNC").ID, _;
 }
 
+TEST_F(SymbolCollectorTest, RefContainers) {
+  Annotations Code(R"cpp(
+int $toplevel1[[f1]](int);
+void f2() {
+  (void) $ref1a[[f1]](1);
+  auto fptr = &$ref1b[[f1]];
+}
+int $toplevel2[[v1]] = $ref2[[f1]](2);
+void f3(int arg = $ref3[[f1]](3));
+struct S1 {
+  int $classscope1[[member1]] = $ref4[[f1]](4);
+  int $classscope2[[member2]] = 42;
+};
+constexpr int f4(int x) { return x + 1; }
+template  struct S2 {};
+S2<$ref6[[f4]](0)> v2;
+S2<$ref7a[[f4]](0)> f5(S2<$ref7b[[f4]](0)>);
+namespace N {
+  void $namespacescope1[[f6]]();
+  int $namespacescope2[[v3]];
+}
+  )cpp");
+  CollectorOpts.RefFilter = RefKind::All;
+  CollectorOpts.CollectMainFileRefs = true;
+  runSymbolCollector("", Code.code());
+  auto FindRefWithRange = [&](Range R) -> Optional {
+for (auto &Entry : Refs) {
+  for (auto &Ref : Entry.second) {
+if (rangesMatch(Ref.Location, R))
+  return Ref;
+  }
+}
+return llvm::None;
+  };
+  auto AssertContainer = [&](llvm::StringRef RefAnnotation,
+ llvm::StringRef ExpectedContainerName) {
+auto Ref = FindRefWithRange(Code.range(RefAnnotation));
+EXPECT_TRUE(bool(Ref));
+
+auto ExpectedContainer = findSymbol(Symbols, ExpectedContainerName);
+EXPECT_EQ(Ref->Container, ExpectedContainer.ID);
+  };
+  auto AssertSameContainer = [&](llvm::StringRef Ref1Ann,
+ llvm::StringRef Ref2Ann) {
+auto Ref1 = FindRefWithRange(Code.range(Ref1Ann));
+EXPECT_TRUE(bool(Ref1));
+auto Ref2 = FindRefWithRange(Code.range(Ref2Ann));
+EXPECT_TRUE(bool(Ref2));
+
+EXPECT_EQ(Ref1->Container, Ref2->Container);
+  };
+  auto AssertDifferentContainers = [&](llvm::StringRef Ref1Ann,
+   llvm::StringRef Ref2Ann) {
+auto Ref1 = FindRefWithRange(Code.range(Ref1Ann));
+EXPECT_TRUE(bool(Ref1));
+auto Ref2 = FindRefWithRange(Co

[PATCH] D89670: [clangd] Store the containing symbol for refs

2020-10-31 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments.



Comment at: clang-tools-extra/clangd/index/Ref.h:91
   RefKind Kind = RefKind::Unknown;
+  SymbolID Container;
 };

kadircet wrote:
> i am afraid we are going to have an indeterminate value here if for whatever 
> reason `Container` detection fails. (e.g. for macros, or if ASTNode.Parent 
> set to nullptr by libindex).
> 
> Sent out https://reviews.llvm.org/D90397 to address the issue.
Good catch, and thank you for addressing this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89670

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