date:20180213

r325103 - Test commit access

2018-02-13 Thread Henry Wong via cfe-commits

Author: henrywong
Date: Tue Feb 13 23:32:27 2018
New Revision: 325103

URL: http://llvm.org/viewvc/llvm-project?rev=325103=rev
Log:
Test commit access

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h?rev=325103=325102=325103=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h Tue 
Feb 13 23:32:27 2018
@@ -393,7 +393,7 @@ public:
   void VisitMemberExpr(const MemberExpr *M, ExplodedNode *Pred, 
ExplodedNodeSet );
 
-  /// VisitMemberExpr - Transfer function for builtin atomic expressions
+  /// VisitAtomicExpr - Transfer function for builtin atomic expressions
   void VisitAtomicExpr(const AtomicExpr *E, ExplodedNode *Pred,
ExplodedNodeSet );
 


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

[PATCH] D42755: [libcxx] Fix last_write_time tests for filesystems that don't support very small times.

2018-02-13 Thread Eric Fiselier via Phabricator via cfe-commits

EricWF added inline comments.



Comment at: 
libcxx/test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp:360
-
-ec = GetTestEC();
-last_write_time(p, Clock::now());

vsapsai wrote:
> EricWF wrote:
> > I would really love to keep this test. I think it has the ability to detect 
> > incorrect integer arithmetic in `last_write_time` when UBSAN is enabled.
> > 
> > Could we maybe add another check like `SupportsAlmostMinTime` where that 
> > checks `::min() + MicroSec(1)`?
> What about nesting removed part in `TimeIsRepresentableByFilesystem` like
> ```lang=c++
> if (TimeIsRepresentableByFilesystem(new_time)) {
> TEST_CHECK(!ec);
> TEST_CHECK(tt >= new_time);
> TEST_CHECK(tt < new_time + Sec(1));
> 
> ec = GetTestEC();
> last_write_time(p, Clock::now());
> 
> new_time = file_time_type::min() + MicroSec(1);
> 
> last_write_time(p, new_time, ec);
> tt = last_write_time(p);
> 
> if (TimeIsRepresentableByFilesystem(new_time)) {
> TEST_CHECK(!ec);
> TEST_CHECK(tt >= new_time);
> TEST_CHECK(tt < new_time + Sec(1));
> }
> }
> ```
> 
> As for me, I don't like how it looks. So maybe the same approach but with a 
> flag like
> ```lang=c++
> bool supports_min_time = false;
> if (TimeIsRepresentableByFilesystem(new_time)) {
> TEST_CHECK(!ec);
> TEST_CHECK(tt >= new_time);
> TEST_CHECK(tt < new_time + Sec(1));
> supports_min_time = true;
> }
> 
> ec = GetTestEC();
> last_write_time(p, Clock::now());
> 
> new_time = file_time_type::min() + MicroSec(1);
> 
> last_write_time(p, new_time, ec);
> tt = last_write_time(p);
> 
> if (supports_min_time && TimeIsRepresentableByFilesystem(new_time)) {
> TEST_CHECK(!ec);
> TEST_CHECK(tt >= new_time);
> TEST_CHECK(tt < new_time + Sec(1));
> }
> ```
> 
> Yet another option is to leverage that APFS truncates time to supported 
> value, something like
> ```if ((old_tt < new_time + Sec(1)) && 
> TimeIsRepresentableByFilesystem(new_time))```
> I don't like it because it implicitly relies on time truncation which is not 
> guaranteed for all filesystems and it is slightly harder to understand than 
> boolean.
> 
> How do you like these ideas? `SupportsAlmostMinTime` can also work but I'd 
> like to avoid repeating `+ MicroSec(1)` part in 2 places.
The first option seems fine to me.


https://reviews.llvm.org/D42755



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

[PATCH] D38639: [clangd] #include statements support for Open definition

2018-02-13 Thread Marc-Andre Laperle via Phabricator via cfe-commits

malaperle added inline comments.

Comment at: unittests/clangd/XRefsTests.cpp:245
+  const char *SourceContents = R"cpp(
+  #include "$2^invalid.h"
+  #include "^foo.h"

ilya-biryukov wrote:
> Could we also add tests for corner cases: cursor before opening quote, cursor 
> after the closing quote, cursor in the middle of `#include` token? (we 
> shouldn't navigate anywhere in the middle of the #include token)
> cursor before opening quote, cursor after the closing quote

I assume we don't want to navigate anywhere for these positions? I don't have 
an opinion.

> (we shouldn't navigate anywhere in the middle of the #include token)

It did in CDT and I thought it worked nicely as it made it easier to click on 
it. You can hold ctrl on the whole line and it underlined it (well except 
trailing comments). But clients don't handle the spaces nicely (underline 
between #include and file name) so I thought I'd work on the client first 
before making the server do it. Anyhow, for now it shouldn't navigate indeed.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D38639

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

[PATCH] D43277: limits: Use `false` instead of `type(0)`.

2018-02-13 Thread Bruce Mitchener via Phabricator via cfe-commits

brucem created this revision.
brucem added reviewers: mclow.lists, EricWF.

This fixes warnings when using clang-tidy and the
`modernize-use-bool-literals` check.


Repository:
  rCXX libc++

https://reviews.llvm.org/D43277

Files:
  include/limits


Index: include/limits
===
--- include/limits
+++ include/limits
@@ -269,8 +269,8 @@
 static _LIBCPP_CONSTEXPR const bool is_integer = true;
 static _LIBCPP_CONSTEXPR const bool is_exact = true;
 static _LIBCPP_CONSTEXPR const int  radix = 2;
-_LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR type epsilon() 
_NOEXCEPT {return type(0);}
-_LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR type round_error() 
_NOEXCEPT {return type(0);}
+_LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR type epsilon() 
_NOEXCEPT {return false;}
+_LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR type round_error() 
_NOEXCEPT {return false;}
 
 static _LIBCPP_CONSTEXPR const int  min_exponent = 0;
 static _LIBCPP_CONSTEXPR const int  min_exponent10 = 0;
@@ -282,10 +282,10 @@
 static _LIBCPP_CONSTEXPR const bool has_signaling_NaN = false;
 static _LIBCPP_CONSTEXPR const float_denorm_style has_denorm = 
denorm_absent;
 static _LIBCPP_CONSTEXPR const bool has_denorm_loss = false;
-_LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR type infinity() 
_NOEXCEPT {return type(0);}
-_LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR type quiet_NaN() 
_NOEXCEPT {return type(0);}
-_LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR type signaling_NaN() 
_NOEXCEPT {return type(0);}
-_LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR type denorm_min() 
_NOEXCEPT {return type(0);}
+_LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR type infinity() 
_NOEXCEPT {return false;}
+_LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR type quiet_NaN() 
_NOEXCEPT {return false;}
+_LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR type signaling_NaN() 
_NOEXCEPT {return false;}
+_LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR type denorm_min() 
_NOEXCEPT {return false;}
 
 static _LIBCPP_CONSTEXPR const bool is_iec559 = false;
 static _LIBCPP_CONSTEXPR const bool is_bounded = true;


Index: include/limits
===
--- include/limits
+++ include/limits
@@ -269,8 +269,8 @@
 static _LIBCPP_CONSTEXPR const bool is_integer = true;
 static _LIBCPP_CONSTEXPR const bool is_exact = true;
 static _LIBCPP_CONSTEXPR const int  radix = 2;
-_LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR type epsilon() _NOEXCEPT {return type(0);}
-_LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR type round_error() _NOEXCEPT {return type(0);}
+_LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR type epsilon() _NOEXCEPT {return false;}
+_LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR type round_error() _NOEXCEPT {return false;}
 
 static _LIBCPP_CONSTEXPR const int  min_exponent = 0;
 static _LIBCPP_CONSTEXPR const int  min_exponent10 = 0;
@@ -282,10 +282,10 @@
 static _LIBCPP_CONSTEXPR const bool has_signaling_NaN = false;
 static _LIBCPP_CONSTEXPR const float_denorm_style has_denorm = denorm_absent;
 static _LIBCPP_CONSTEXPR const bool has_denorm_loss = false;
-_LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR type infinity() _NOEXCEPT {return type(0);}
-_LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR type quiet_NaN() _NOEXCEPT {return type(0);}
-_LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR type signaling_NaN() _NOEXCEPT {return type(0);}
-_LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR type denorm_min() _NOEXCEPT {return type(0);}
+_LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR type infinity() _NOEXCEPT {return false;}
+_LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR type quiet_NaN() _NOEXCEPT {return false;}
+_LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR type signaling_NaN() _NOEXCEPT {return false;}
+_LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR type denorm_min() _NOEXCEPT {return false;}
 
 static _LIBCPP_CONSTEXPR const bool is_iec559 = false;
 static _LIBCPP_CONSTEXPR const bool is_bounded = true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43273: [libcxx] [test] Fix MSVC warnings and errors.

2018-02-13 Thread Stephan T. Lavavej via Phabricator via cfe-commits

STL_MSFT created this revision.
STL_MSFT added reviewers: EricWF, mclow.lists.

[libcxx] [test] Fix MSVC warnings and errors.

test/std/numerics/numeric.ops/exclusive.scan/exclusive_scan.pass.cpp
test/std/numerics/numeric.ops/exclusive.scan/exclusive_scan_init_op.pass.cpp
test/std/numerics/numeric.ops/inclusive.scan/inclusive_scan.pass.cpp
test/std/numerics/numeric.ops/inclusive.scan/inclusive_scan_op.pass.cpp
test/std/numerics/numeric.ops/inclusive.scan/inclusive_scan_op_init.pass.cpp
test/std/numerics/numeric.ops/transform.exclusive.scan/transform_exclusive_scan_init_bop_uop.pass.cpp
test/std/numerics/numeric.ops/transform.inclusive.scan/transform_inclusive_scan_bop_uop.pass.cpp
test/std/numerics/numeric.ops/transform.inclusive.scan/transform_inclusive_scan_bop_uop_init.pass.cpp
Fix MSVC x64 truncation warnings.
warning C4267: conversion from 'size_t' to 'int', possible loss of data

test/std/strings/basic.string/string.modifiers/string_append/push_back.pass.cpp
Fix MSVC uninitialized memory warning.
warning C6001: Using uninitialized memory 'vl'.

test/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_binding_diagnostics.pass.cpp
Include  for the assert() macro.


https://reviews.llvm.org/D43273

Files:
  test/std/numerics/numeric.ops/exclusive.scan/exclusive_scan.pass.cpp
  test/std/numerics/numeric.ops/exclusive.scan/exclusive_scan_init_op.pass.cpp
  test/std/numerics/numeric.ops/inclusive.scan/inclusive_scan.pass.cpp
  test/std/numerics/numeric.ops/inclusive.scan/inclusive_scan_op.pass.cpp
  test/std/numerics/numeric.ops/inclusive.scan/inclusive_scan_op_init.pass.cpp
  
test/std/numerics/numeric.ops/transform.exclusive.scan/transform_exclusive_scan_init_bop_uop.pass.cpp
  
test/std/numerics/numeric.ops/transform.inclusive.scan/transform_inclusive_scan_bop_uop.pass.cpp
  
test/std/numerics/numeric.ops/transform.inclusive.scan/transform_inclusive_scan_bop_uop_init.pass.cpp
  
test/std/strings/basic.string/string.modifiers/string_append/push_back.pass.cpp
  
test/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_binding_diagnostics.pass.cpp

Index: test/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_binding_diagnostics.pass.cpp
===
--- test/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_binding_diagnostics.pass.cpp
+++ test/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_binding_diagnostics.pass.cpp
@@ -16,6 +16,7 @@
 
 #include 
 #include 
+#include 
 #include "test_macros.h"
 
 #if TEST_HAS_BUILTIN_IDENTIFIER(__reference_binds_to_temporary)
Index: test/std/strings/basic.string/string.modifiers/string_append/push_back.pass.cpp
===
--- test/std/strings/basic.string/string.modifiers/string_append/push_back.pass.cpp
+++ test/std/strings/basic.string/string.modifiers/string_append/push_back.pass.cpp
@@ -52,7 +52,7 @@
 {
 // https://bugs.llvm.org/show_bug.cgi?id=31454
 std::basic_string s;
-veryLarge vl;
+veryLarge vl = {};
 s.push_back(vl);
 s.push_back(vl);
 s.push_back(vl);
Index: test/std/numerics/numeric.ops/transform.inclusive.scan/transform_inclusive_scan_bop_uop_init.pass.cpp
===
--- test/std/numerics/numeric.ops/transform.inclusive.scan/transform_inclusive_scan_bop_uop_init.pass.cpp
+++ test/std/numerics/numeric.ops/transform.inclusive.scan/transform_inclusive_scan_bop_uop_init.pass.cpp
@@ -105,15 +105,15 @@
 std::iota(v.begin(), v.end(), 0);
 std::transform_inclusive_scan(v.begin(), v.end(), v.begin(), std::plus<>(), add_one{}, 30);
 for (size_t i = 0; i < v.size(); ++i)
-assert(v[i] == 30 + triangle(i) + (int) (i + 1));
+assert(v[i] == 30 + triangle(static_cast(i)) + static_cast(i + 1));
 }
 
 {
 std::vector v(10);
 std::iota(v.begin(), v.end(), 1);
 std::transform_inclusive_scan(v.begin(), v.end(), v.begin(), std::plus<>(), add_one{}, 40);
 for (size_t i = 0; i < v.size(); ++i)
-assert(v[i] == 40 + triangle(i + 1) + (int) (i + 1));
+assert(v[i] == 40 + triangle(static_cast(i + 1)) + static_cast(i + 1));
 }
 
 {
@@ -134,7 +134,7 @@
 assert(res[0] == 2);
 for (size_t i = 1; i < res.size(); ++i)
 {
-j *= i + 2;
+j = static_cast(j * (i + 2));
 assert(res[i] == j);
 }
 }
Index: test/std/numerics/numeric.ops/transform.inclusive.scan/transform_inclusive_scan_bop_uop.pass.cpp
===
--- test/std/numerics/numeric.ops/transform.inclusive.scan/transform_inclusive_scan_bop_uop.pass.cpp
+++ test/std/numerics/numeric.ops/transform.inclusive.scan/transform_inclusive_scan_bop_uop.pass.cpp
@@ -96,15 +96,15 @@
 std::iota(v.begin(), v.end(), 0);
 std::transform_inclusive_scan(v.begin(), v.end(), v.begin(), std::plus<>(), add_one{});
 for

[clang-tools-extra] r325097 - [clangd] Configure clangd tracing with CLANGD_TRACE env instead of -trace flag

2018-02-13 Thread Sam McCall via cfe-commits

Author: sammccall
Date: Tue Feb 13 19:20:07 2018
New Revision: 325097

URL: http://llvm.org/viewvc/llvm-project?rev=325097=rev
Log:
[clangd] Configure clangd tracing with CLANGD_TRACE env instead of -trace flag

Modified:
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
clang-tools-extra/trunk/test/clangd/trace.test

Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=325097=325096=325097=diff
==
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Tue Feb 13 19:20:07 2018
@@ -17,6 +17,7 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 #include 
 #include 
 #include 
@@ -123,12 +124,6 @@ static llvm::cl::opt InputMirrorFi
 "Mirror all LSP input to the specified file. Useful for debugging."),
 llvm::cl::init(""), llvm::cl::Hidden);
 
-static llvm::cl::opt TraceFile(
-"trace",
-llvm::cl::desc(
-"Trace internal events and timestamps in chrome://tracing JSON 
format"),
-llvm::cl::init(""), llvm::cl::Hidden);
-
 static llvm::cl::opt EnableIndexBasedCompletion(
 "enable-index-based-completion",
 llvm::cl::desc(
@@ -176,15 +171,18 @@ int main(int argc, char *argv[]) {
 }
   }
 
-  // Setup tracing facilities.
+  // Setup tracing facilities if CLANGD_TRACE is set. In practice enabling a
+  // trace flag in your editor's config is annoying, launching with
+  // `CLANGD_TRACE=trace.json vim` is easier.
   llvm::Optional TraceStream;
   std::unique_ptr Tracer;
-  if (!TraceFile.empty()) {
+  if (auto *TraceFile = getenv("CLANGD_TRACE")) {
 std::error_code EC;
 TraceStream.emplace(TraceFile, /*ref*/ EC, llvm::sys::fs::F_RW);
 if (EC) {
-  TraceFile.reset();
-  llvm::errs() << "Error while opening trace file: " << EC.message();
+  TraceStream.reset();
+  llvm::errs() << "Error while opening trace file " << TraceFile << ": "
+   << EC.message();
 } else {
   Tracer = trace::createJSONTracer(*TraceStream, PrettyPrint);
 }

Modified: clang-tools-extra/trunk/test/clangd/trace.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/trace.test?rev=325097=325096=325097=diff
==
--- clang-tools-extra/trunk/test/clangd/trace.test (original)
+++ clang-tools-extra/trunk/test/clangd/trace.test Tue Feb 13 19:20:07 2018
@@ -1,4 +1,4 @@
-# RUN: clangd -lit-test -trace %t < %s && FileCheck %s < %t
+# RUN: CLANGD_TRACE=%t clangd -lit-test < %s && FileCheck %s < %t
 
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
 ---
 
{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"void
 main() {}"}}}


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

[PATCH] D43272: [clangd] Fix tracing now that spans lifetimes can overlap on a thread.

2018-02-13 Thread Sam McCall via Phabricator via cfe-commits

sammccall created this revision.
sammccall added a reviewer: ioeric.
Herald added subscribers: cfe-commits, jkorous-apple, ilya-biryukov, klimek.

The chrome trace viewer requires events within a thread to strictly nest.
So we need to record the lifetime of the Span objects, not the contexts.

But we still want to show the relationship between spans where a context crosses
threads, so do this with flow events (i.e. arrows).

Before: https://photos.app.goo.gl/q4Dd9u9xtelaXk1v1
After: https://photos.app.goo.gl/5RNLmAMLZR3unvY83

(This could stand some further improvement, in particular I think we want a
container span whenever we schedule work on a thread. But that's another patch)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43272

Files:
  clangd/Trace.cpp
  clangd/Trace.h
  test/clangd/trace.test
  unittests/clangd/TraceTests.cpp

Index: unittests/clangd/TraceTests.cpp
===
--- unittests/clangd/TraceTests.cpp
+++ unittests/clangd/TraceTests.cpp
@@ -114,12 +114,10 @@
 ASSERT_NE(++Event, Events->end()) << "Expected thread name";
 EXPECT_TRUE(VerifyObject(*Event, {{"ph", "M"}, {"name", "thread_name"}}));
   }
-  ASSERT_NE(++Event, Events->end()) << "Expected span start";
-  EXPECT_TRUE(VerifyObject(*Event, {{"ph", "B"}, {"name", "A"}}));
   ASSERT_NE(++Event, Events->end()) << "Expected log message";
   EXPECT_TRUE(VerifyObject(*Event, {{"ph", "i"}, {"name", "Log"}}));
   ASSERT_NE(++Event, Events->end()) << "Expected span end";
-  EXPECT_TRUE(VerifyObject(*Event, {{"ph", "E"}}));
+  EXPECT_TRUE(VerifyObject(*Event, {{"ph", "X"}, {"name", "A"}}));
   ASSERT_EQ(++Event, Events->end());
   ASSERT_EQ(++Prop, Root->end());
 }
Index: test/clangd/trace.test
===
--- test/clangd/trace.test
+++ test/clangd/trace.test
@@ -2,18 +2,22 @@
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
 ---
 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"void main() {}"}}}
-#  CHECK: {"displayTimeUnit":"ns","traceEvents":[
-# Start opening the doc.
-#  CHECK: "name": "textDocument/didOpen"
-#  CHECK: "ph": "B"
-# Start building the preamble.
-#  CHECK: "name": "Preamble"
-#  CHECK: },
-# Finish building the preamble, with filename.
-#  CHECK: "File": "{{.*(/|\\)}}foo.c"
-# CHECK-NEXT: },
-# CHECK-NEXT: "ph": "E"
-# Start building the file.
-#  CHECK: "name": "Build"
+# These assertions are a bit loose, to avoid brittleness.
+# CHECK: {"displayTimeUnit":"ns","traceEvents":[
+# CHECK: {
+# CHECK:   "args": {
+# CHECK: "File": "{{.*(/|\\)}}foo.c"
+# CHECK:   },
+# CHECK:   "name": "Preamble",
+# CHECK:   "ph": "X",
+# CHECK: }
+# CHECK: {
+# CHECK:   "args": {
+# CHECK: "File": "{{.*(/|\\)}}foo.c"
+# CHECK:   },
+# CHECK:   "name": "Build",
+# CHECK:   "ph": "X",
+# CHECK: }
+# CHECK: },
 ---
 {"jsonrpc":"2.0","id":5,"method":"shutdown"}
Index: clangd/Trace.h
===
--- clangd/Trace.h
+++ clangd/Trace.h
@@ -40,6 +40,10 @@
   /// whose destructor records the end of the event.
   /// The args are *Args, only complete when the event ends.
   virtual Context beginSpan(llvm::StringRef Name, json::obj *Args) = 0;
+  // Called before a Span ends. beginSpan() and endSpan() will always form a
+  // proper stack on a given thread.
+  // The Context returned by beginSpan is active, but Args is not ready.
+  virtual void endSpan() {};
 
   /// Called for instant events.
   virtual void instant(llvm::StringRef Name, json::obj &) = 0;
@@ -77,6 +81,7 @@
 class Span {
 public:
   Span(llvm::StringRef Name);
+  ~Span();
 
   /// Mutable metadata, if this span is interested.
   /// Prefer to use SPAN_ATTACH rather than accessing this directly.
Index: clangd/Trace.cpp
===
--- clangd/Trace.cpp
+++ clangd/Trace.cpp
@@ -16,6 +16,7 @@
 #include "llvm/Support/FormatProviders.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Threading.h"
+#include 
 #include 
 
 namespace clang {
@@ -46,40 +47,96 @@
 Out.flush();
   }
 
+  // We stash a Span object in the context. It will record the start/end,
+  // and this also allows us to look up the parent Span's information.
   Context beginSpan(llvm::StringRef Name, json::obj *Args) override {
-jsonEvent("B", json::obj{{"name", Name}});
-return Context::current().derive(make_scope_exit([this, Args] {
-  jsonEvent("E", json::obj{{"args", std::move(*Args)}});
-}));
+return Context::current().derive(SpanKey,
+ make_unique(this, Name, Args));
+  }
+
+  // Trace viewer requires each thread to properly stack events.
+  // So we need to mark only duration that the span was active

Re: [PATCH] D43259: Implement function attribute artificial

2018-02-13 Thread David Blaikie via cfe-commits

Maybe - though that'd actually make for larger debug info & not be much
use.

With nodebug+always_inline (which is how the intrinsics are provided) the
function call dissolves into the raw instruction it's meant to represent.
The debug info describes that instruction as if it had been written at the
point of the call.

With this artificial+always_inline a description of inlining would be
provided (DWARF's inlining description is a bit heavy/verbose) & the
debugger would step over it, rather than into it.

For longer functions (inlined or outlined), marking as artificial makes
sense, I think, but /probably/ not for those places already using
nodebug+always_inline.

On Tue, Feb 13, 2018 at 3:18 PM Reid Kleckner via Phabricator <
revi...@reviews.llvm.org> wrote:

> rnk accepted this revision.
> rnk added subscribers: probinson, aprantl, dblaikie.
> rnk added a comment.
> This revision is now accepted and ready to land.
>
> lgtm
>
> ---
>
> Clang's builtin headers use `__attribute__((__nodebug__))` for this
> purpose. Do you think we should follow this up by using artificial instead?
> It seems like it would be a representational improvement. @aprantl
> @probinson @dblaikie
>
>
>
> 
> Comment at: test/Sema/artificial.c:3
> +
> +void __attribute__((artificial)) bar() {} // expected-warning
> {{'artificial' attribute only applies to inline functions}}
> 
> I think it's worth adding the `foo` function from the CodeGen test here to
> show we don't generate warnings when the function is inline specified.
>
>
> https://reviews.llvm.org/D43259
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r325095 - Fix a couple of places where we assumed that non-type template parameters are always rvalues.

2018-02-13 Thread Richard Smith via cfe-commits

Author: rsmith
Date: Tue Feb 13 18:07:53 2018
New Revision: 325095

URL: http://llvm.org/viewvc/llvm-project?rev=325095=rev
Log:
Fix a couple of places where we assumed that non-type template parameters are 
always rvalues.

Added:
cfe/trunk/test/SemaTemplate/sizeof-pack.cpp
Modified:
cfe/trunk/include/clang/AST/ExprCXX.h
cfe/trunk/lib/AST/ExprCXX.cpp
cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
cfe/trunk/lib/Sema/TreeTransform.h
cfe/trunk/test/SemaTemplate/nested-template.cpp

Modified: cfe/trunk/include/clang/AST/ExprCXX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExprCXX.h?rev=325095=325094=325095=diff
==
--- cfe/trunk/include/clang/AST/ExprCXX.h (original)
+++ cfe/trunk/include/clang/AST/ExprCXX.h Tue Feb 13 18:07:53 2018
@@ -3916,6 +3916,7 @@ class SubstNonTypeTemplateParmPackExpr :
 
 public:
   SubstNonTypeTemplateParmPackExpr(QualType T,
+   ExprValueKind ValueKind,
NonTypeTemplateParmDecl *Param,
SourceLocation NameLoc,
const TemplateArgument );

Modified: cfe/trunk/lib/AST/ExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprCXX.cpp?rev=325095=325094=325095=diff
==
--- cfe/trunk/lib/AST/ExprCXX.cpp (original)
+++ cfe/trunk/lib/AST/ExprCXX.cpp Tue Feb 13 18:07:53 2018
@@ -1330,10 +1330,11 @@ SizeOfPackExpr *SizeOfPackExpr::CreateDe
 
 SubstNonTypeTemplateParmPackExpr::
 SubstNonTypeTemplateParmPackExpr(QualType T, 
+ ExprValueKind ValueKind,
  NonTypeTemplateParmDecl *Param,
  SourceLocation NameLoc,
  const TemplateArgument )
-: Expr(SubstNonTypeTemplateParmPackExprClass, T, VK_RValue, OK_Ordinary,
+: Expr(SubstNonTypeTemplateParmPackExprClass, T, ValueKind, OK_Ordinary,
true, true, true, true),
   Param(Param), Arguments(ArgPack.pack_begin()),
   NumArguments(ArgPack.pack_size()), NameLoc(NameLoc) {}

Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp?rev=325095=325094=325095=diff
==
--- cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp Tue Feb 13 18:07:53 2018
@@ -1212,11 +1212,11 @@ TemplateInstantiator::TransformTemplateP
   NTTP->getDeclName());
   if (TargetType.isNull())
 return ExprError();
-  
-  return new (SemaRef.Context) SubstNonTypeTemplateParmPackExpr(TargetType,
-NTTP, 
-  E->getLocation(),
-Arg);
+
+  return new (SemaRef.Context) SubstNonTypeTemplateParmPackExpr(
+  TargetType.getNonLValueExprType(SemaRef.Context),
+  TargetType->isReferenceType() ? VK_LValue : VK_RValue, NTTP,
+  E->getLocation(), Arg);
 }
 
 Arg = getPackSubstitutedTemplateArgument(getSema(), Arg);

Modified: cfe/trunk/lib/Sema/TreeTransform.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/TreeTransform.h?rev=325095=325094=325095=diff
==
--- cfe/trunk/lib/Sema/TreeTransform.h (original)
+++ cfe/trunk/lib/Sema/TreeTransform.h Tue Feb 13 18:07:53 2018
@@ -11392,8 +11392,10 @@ TreeTransform::TransformSizeOfP
 ArgStorage = TemplateArgument(TemplateName(TTPD), None);
   } else {
 auto *VD = cast(Pack);
-ExprResult DRE = getSema().BuildDeclRefExpr(VD, VD->getType(),
-VK_RValue, 
E->getPackLoc());
+ExprResult DRE = getSema().BuildDeclRefExpr(
+VD, VD->getType().getNonLValueExprType(getSema().Context),
+VD->getType()->isReferenceType() ? VK_LValue : VK_RValue,
+E->getPackLoc());
 if (DRE.isInvalid())
   return ExprError();
 ArgStorage = new (getSema().Context) PackExpansionExpr(

Modified: cfe/trunk/test/SemaTemplate/nested-template.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/nested-template.cpp?rev=325095=325094=325095=diff
==
--- cfe/trunk/test/SemaTemplate/nested-template.cpp (original)
+++ cfe/trunk/test/SemaTemplate/nested-template.cpp Tue Feb 13 18:07:53 2018
@@ -161,3 +161,10 @@ class Outer1 {
 template  struct X;
 template  int

[PATCH] D42755: [libcxx] Fix last_write_time tests for filesystems that don't support very small times.

2018-02-13 Thread Volodymyr Sapsai via Phabricator via cfe-commits

vsapsai added inline comments.

Comment at: 
libcxx/test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp:360
-
-ec = GetTestEC();
-last_write_time(p, Clock::now());

EricWF wrote:
> I would really love to keep this test. I think it has the ability to detect 
> incorrect integer arithmetic in `last_write_time` when UBSAN is enabled.
> 
> Could we maybe add another check like `SupportsAlmostMinTime` where that 
> checks `::min() + MicroSec(1)`?
What about nesting removed part in `TimeIsRepresentableByFilesystem` like
```lang=c++
if (TimeIsRepresentableByFilesystem(new_time)) {
TEST_CHECK(!ec);
TEST_CHECK(tt >= new_time);
TEST_CHECK(tt < new_time + Sec(1));

ec = GetTestEC();
last_write_time(p, Clock::now());

new_time = file_time_type::min() + MicroSec(1);

last_write_time(p, new_time, ec);
tt = last_write_time(p);

if (TimeIsRepresentableByFilesystem(new_time)) {
TEST_CHECK(!ec);
TEST_CHECK(tt >= new_time);
TEST_CHECK(tt < new_time + Sec(1));
}
}
```

As for me, I don't like how it looks. So maybe the same approach but with a 
flag like
```lang=c++
bool supports_min_time = false;
if (TimeIsRepresentableByFilesystem(new_time)) {
TEST_CHECK(!ec);
TEST_CHECK(tt >= new_time);
TEST_CHECK(tt < new_time + Sec(1));
supports_min_time = true;
}

ec = GetTestEC();
last_write_time(p, Clock::now());

new_time = file_time_type::min() + MicroSec(1);

last_write_time(p, new_time, ec);
tt = last_write_time(p);

if (supports_min_time && TimeIsRepresentableByFilesystem(new_time)) {
TEST_CHECK(!ec);
TEST_CHECK(tt >= new_time);
TEST_CHECK(tt < new_time + Sec(1));
}
```

Yet another option is to leverage that APFS truncates time to supported value, 
something like
```if ((old_tt < new_time + Sec(1)) && 
TimeIsRepresentableByFilesystem(new_time))```
I don't like it because it implicitly relies on time truncation which is not 
guaranteed for all filesystems and it is slightly harder to understand than 
boolean.

How do you like these ideas? `SupportsAlmostMinTime` can also work but I'd like 
to avoid repeating `+ MicroSec(1)` part in 2 places.

https://reviews.llvm.org/D42755

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

[PATCH] D42498: [ExprConstant] Fix crash when initialize an indirect field with another field.

2018-02-13 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

rsmith added a comment.

Please handle this by temporarily updating the `This` pointer appropriately 
when evaluating the `CXXDefaultInitExpr` rather than trying to work around the 
`This` value being wrong later (your approach will still go wrong if the `This` 
pointer is used in other ways, such as an explicit mention of `this` or a 
member function call). You can refer to how we do this in CodeGen: look for 
`CodeGenFunction::CXXDefaultInitExprScope` and 
`CodeGenFunction::FieldConstructionScope`.


https://reviews.llvm.org/D42498



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

Re: r325081 - Implement function attribute artificial

2018-02-13 Thread Richard Smith via cfe-commits

On 13 February 2018 at 16:14, Erich Keane via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: erichkeane
> Date: Tue Feb 13 16:14:07 2018
> New Revision: 325081
>
> URL: http://llvm.org/viewvc/llvm-project?rev=325081=rev
> Log:
> Implement function attribute artificial
>
> Added support in clang for GCC function attribute 'artificial'. This
> attribute
> is used to control stepping behavior of debugger with respect to inline
> functions.
>
> Patch By: Elizabeth Andrews (eandrews)
>
> Differential Revision: https://reviews.llvm.org/D43259
>
>
> Added:
> cfe/trunk/test/CodeGen/artificial.c
> cfe/trunk/test/Sema/artificial.c
> Modified:
> cfe/trunk/include/clang/Basic/Attr.td
> cfe/trunk/include/clang/Basic/AttrDocs.td
> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> cfe/trunk/lib/Sema/SemaDeclAttr.cpp
>
> Modified: cfe/trunk/include/clang/Basic/Attr.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/Basic/Attr.td?rev=325081=325080=325081=diff
> 
> ==
> --- cfe/trunk/include/clang/Basic/Attr.td (original)
> +++ cfe/trunk/include/clang/Basic/Attr.td Tue Feb 13 16:14:07 2018
> @@ -111,6 +111,9 @@ def SharedVar : SubsetSubject  def GlobalVar : SubsetSubject   [{S->hasGlobalStorage()}], "global
> variables">;
>
> +def InlineFunction : SubsetSubject + [{S->isInlineSpecified()}], "inline
> functions">;
> +
>  // FIXME: this hack is needed because DeclNodes.td defines the base Decl
> node
>  // type to be a class, not a definition. This makes it impossible to
> create an
>  // attribute subject which accepts a Decl. Normally, this is not a
> problem,
> @@ -588,6 +591,12 @@ def AlwaysInline : InheritableAttr {
>let Documentation = [Undocumented];
>  }
>
> +def Artificial : InheritableAttr {
> +  let Spellings = [GCC<"artificial">];
> +  let Subjects = SubjectList<[InlineFunction], WarnDiag>;
> +  let Documentation = [ArtificialDocs];
> +}
> +
>  def XRayInstrument : InheritableAttr {
>let Spellings = [Clang<"xray_always_instrument">,
> Clang<"xray_never_instrument">];
>
> Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/Basic/AttrDocs.td?rev=325081=325080=325081=diff
> 
> ==
> --- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
> +++ cfe/trunk/include/clang/Basic/AttrDocs.td Tue Feb 13 16:14:07 2018
> @@ -3273,3 +3273,13 @@ For more information see
>  or `msvc documentation  pl-pl/cpp/cpp/selectany>`_.
>  }];
>  }
> +
> +def ArtificialDocs : Documentation {
> +  let Category = DocCatFunction;
> +  let Content = [{
> +The ``artificial`` attribute is used with inline functions to treat the
> inline
> +function as a unit while debugging. For more information see GCC_
> documentation.
>

I find this to be hard to understand. What does "treat the inline function
as a unit" actually mean? How about something like:

The ``artificial`` attribute can be applied to an inline function. If such
a function is inlined, the attribute indicates that debuggers should
associate the resulting instructions with the call site, rather than with
the corresponding line within the inlined callee.

+
> +.. _GCC: https://gcc.gnu.org/onlinedocs/gcc-4.6.4/gcc/
> Function-Attributes.html


If you still want to reference the GCC documentation, could you pick a
newer version? :)

+  }];
> +}
>
> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/
> CGDebugInfo.cpp?rev=325081=325080=325081=diff
> 
> ==
> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Tue Feb 13 16:14:07 2018
> @@ -3235,7 +3235,7 @@ void CGDebugInfo::EmitFunctionStart(Glob
>if (Name.startswith("\01"))
>  Name = Name.substr(1);
>
> -  if (!HasDecl || D->isImplicit()) {
> +  if (!HasDecl || D->isImplicit() || D->hasAttr()) {
>  Flags |= llvm::DINode::FlagArtificial;
>  // Artificial functions should not silently reuse CurLoc.
>  CurLoc = SourceLocation();
>
> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/
> SemaDeclAttr.cpp?rev=325081=325080=325081=diff
> 
> ==
> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Tue Feb 13 16:14:07 2018
> @@ -6057,6 +6057,9 @@ static void ProcessDeclAttribute(Sema 
>case AttributeList::AT_AlwaysInline:
>  handleAlwaysInlineAttr(S, D, Attr);
>  break;
> +  case AttributeList::AT_Artificial:
> +handleSimpleAttribute(S, D, Attr);
> +break;

[libcxxabi] r325093 - [demangler] Support for exception specifications on function types.

2018-02-13 Thread Erik Pilkington via cfe-commits

Author: epilk
Date: Tue Feb 13 17:08:20 2018
New Revision: 325093

URL: http://llvm.org/viewvc/llvm-project?rev=325093=rev
Log:
[demangler] Support for exception specifications on function types.

Modified:
libcxxabi/trunk/src/cxa_demangle.cpp
libcxxabi/trunk/test/test_demangle.pass.cpp

Modified: libcxxabi/trunk/src/cxa_demangle.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/src/cxa_demangle.cpp?rev=325093=325092=325093=diff
==
--- libcxxabi/trunk/src/cxa_demangle.cpp (original)
+++ libcxxabi/trunk/src/cxa_demangle.cpp Tue Feb 13 17:08:20 2018
@@ -175,6 +175,8 @@ public:
 KPointerToMemberType,
 KArrayType,
 KFunctionType,
+KNoexceptSpec,
+KDynamicExceptionSpec,
 KFunctionEncoding,
 KLiteralOperator,
 KSpecialName,
@@ -720,16 +722,21 @@ class FunctionType final : public Node {
   NodeArray Params;
   Qualifiers CVQuals;
   FunctionRefQual RefQual;
+  Node *ExceptionSpec;
 
 public:
   FunctionType(Node *Ret_, NodeArray Params_, Qualifiers CVQuals_,
-   FunctionRefQual RefQual_)
+   FunctionRefQual RefQual_, Node *ExceptionSpec_)
   : Node(KFunctionType, Ret_->ParameterPackSize,
  /*RHSComponentCache=*/Cache::Yes, /*ArrayCache=*/Cache::No,
  /*FunctionCache=*/Cache::Yes),
-Ret(Ret_), Params(Params_), CVQuals(CVQuals_), RefQual(RefQual_) {
+Ret(Ret_), Params(Params_), CVQuals(CVQuals_), RefQual(RefQual_),
+ExceptionSpec(ExceptionSpec_) {
 for (Node *P : Params)
   ParameterPackSize = std::min(ParameterPackSize, P->ParameterPackSize);
+if (ExceptionSpec != nullptr)
+  ParameterPackSize =
+std::min(ParameterPackSize, ExceptionSpec->ParameterPackSize);
   }
 
   bool hasRHSComponentSlow(OutputStream &) const override { return true; }
@@ -764,6 +771,39 @@ public:
   S += " &";
 else if (RefQual == FrefQualRValue)
   S += " &&";
+
+if (ExceptionSpec != nullptr) {
+  S += ' ';
+  ExceptionSpec->print(S);
+}
+  }
+};
+
+class NoexceptSpec : public Node {
+  Node *E;
+public:
+  NoexceptSpec(Node *E_) : Node(KNoexceptSpec, E_->ParameterPackSize), E(E_) {}
+
+  void printLeft(OutputStream ) const override {
+S += "noexcept(";
+E->print(S);
+S += ")";
+  }
+};
+
+class DynamicExceptionSpec : public Node {
+  NodeArray Types;
+public:
+  DynamicExceptionSpec(NodeArray Types_)
+  : Node(KDynamicExceptionSpec), Types(Types_) {
+for (Node *T : Types)
+  ParameterPackSize = std::min(ParameterPackSize, T->ParameterPackSize);
+  }
+
+  void printLeft(OutputStream ) const override {
+S += "throw(";
+Types.printWithComma(S);
+S += ')';
   }
 };
 
@@ -2370,6 +2410,29 @@ StringView Db::parseBareSourceName() {
 //  ::= O   # && ref-qualifier
 Node *Db::parseFunctionType() {
   Qualifiers CVQuals = parseCVQualifiers();
+
+  Node *ExceptionSpec = nullptr;
+  if (consumeIf("Do")) {
+ExceptionSpec = make("noexcept");
+  } else if (consumeIf("DO")) {
+Node *E = parseExpr();
+if (E == nullptr || !consumeIf('E'))
+  return nullptr;
+ExceptionSpec = make(E);
+  } else if (consumeIf("Dw")) {
+size_t SpecsBegin = Names.size();
+while (!consumeIf('E')) {
+  Node *T = parseType();
+  if (T == nullptr)
+return nullptr;
+  Names.push_back(T);
+}
+ExceptionSpec =
+  make(popTrailingNodeArray(SpecsBegin));
+  }
+
+  consumeIf("Dx"); // transaction safe
+
   if (!consumeIf('F'))
 return nullptr;
   consumeIf('Y'); // extern "C"
@@ -2399,7 +2462,8 @@ Node *Db::parseFunctionType() {
   }
 
   NodeArray Params = popTrailingNodeArray(ParamsBegin);
-  return make(ReturnType, Params, CVQuals, ReferenceQualifier);
+  return make(ReturnType, Params, CVQuals,
+ReferenceQualifier, ExceptionSpec);
 }
 
 // extension:
@@ -2599,7 +2663,11 @@ Node *Db::parseType() {
 if (look(AfterQuals) == 'r') ++AfterQuals;
 if (look(AfterQuals) == 'V') ++AfterQuals;
 if (look(AfterQuals) == 'K') ++AfterQuals;
-if (look(AfterQuals) == 'F') {
+
+if (look(AfterQuals) == 'F' ||
+(look(AfterQuals) == 'D' &&
+ (look(AfterQuals + 1) == 'o' || look(AfterQuals + 1) == 'O' ||
+  look(AfterQuals + 1) == 'w' || look(AfterQuals + 1) == 'x'))) {
   Result = parseFunctionType();
   break;
 }
@@ -2761,6 +2829,14 @@ Node *Db::parseType() {
   Result = make(Child);
   break;
 }
+// Exception specifier on a function type.
+case 'o':
+case 'O':
+case 'w':
+// Transaction safe function type.
+case 'x':
+  Result = parseFunctionType();
+  break;
 }
 break;
   // ::= 

Modified: libcxxabi/trunk/test/test_demangle.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/test/test_demangle.pass.cpp?rev=325093=325092=325093=diff

[libcxxabi] r325092 - [demangler] Simplify the AST for function types, NFC.

2018-02-13 Thread Erik Pilkington via cfe-commits

Author: epilk
Date: Tue Feb 13 17:08:17 2018
New Revision: 325092

URL: http://llvm.org/viewvc/llvm-project?rev=325092=rev
Log:
[demangler] Simplify the AST for function types, NFC.

Modified:
libcxxabi/trunk/src/cxa_demangle.cpp

Modified: libcxxabi/trunk/src/cxa_demangle.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/src/cxa_demangle.cpp?rev=325092=325091=325092=diff
==
--- libcxxabi/trunk/src/cxa_demangle.cpp (original)
+++ libcxxabi/trunk/src/cxa_demangle.cpp Tue Feb 13 17:08:17 2018
@@ -176,8 +176,6 @@ public:
 KArrayType,
 KFunctionType,
 KFunctionEncoding,
-KFunctionQualType,
-KFunctionRefQualType,
 KLiteralOperator,
 KSpecialName,
 KCtorVtableSpecialName,
@@ -357,6 +355,12 @@ public:
   }
 };
 
+enum FunctionRefQual : unsigned char {
+  FrefQualNone,
+  FrefQualLValue,
+  FrefQualRValue,
+};
+
 enum Qualifiers {
   QualNone = 0,
   QualConst = 0x1,
@@ -714,13 +718,16 @@ public:
 class FunctionType final : public Node {
   Node *Ret;
   NodeArray Params;
+  Qualifiers CVQuals;
+  FunctionRefQual RefQual;
 
 public:
-  FunctionType(Node *Ret_, NodeArray Params_)
+  FunctionType(Node *Ret_, NodeArray Params_, Qualifiers CVQuals_,
+   FunctionRefQual RefQual_)
   : Node(KFunctionType, Ret_->ParameterPackSize,
  /*RHSComponentCache=*/Cache::Yes, /*ArrayCache=*/Cache::No,
  /*FunctionCache=*/Cache::Yes),
-Ret(Ret_), Params(Params_) {
+Ret(Ret_), Params(Params_), CVQuals(CVQuals_), RefQual(RefQual_) {
 for (Node *P : Params)
   ParameterPackSize = std::min(ParameterPackSize, P->ParameterPackSize);
   }
@@ -745,6 +752,18 @@ public:
 Params.printWithComma(S);
 S += ")";
 Ret->printRight(S);
+
+if (CVQuals & QualConst)
+  S += " const";
+if (CVQuals & QualVolatile)
+  S += " volatile";
+if (CVQuals & QualRestrict)
+  S += " restrict";
+
+if (RefQual == FrefQualLValue)
+  S += " &";
+else if (RefQual == FrefQualRValue)
+  S += " &&";
   }
 };
 
@@ -752,13 +771,17 @@ class FunctionEncoding final : public No
   const Node *Ret;
   const Node *Name;
   NodeArray Params;
+  Qualifiers CVQuals;
+  FunctionRefQual RefQual;
 
 public:
-  FunctionEncoding(Node *Ret_, Node *Name_, NodeArray Params_)
+  FunctionEncoding(Node *Ret_, Node *Name_, NodeArray Params_,
+   Qualifiers CVQuals_, FunctionRefQual RefQual_)
   : Node(KFunctionEncoding, NoParameterPack,
  /*RHSComponentCache=*/Cache::Yes, /*ArrayCache=*/Cache::No,
  /*FunctionCache=*/Cache::Yes),
-Ret(Ret_), Name(Name_), Params(Params_) {
+Ret(Ret_), Name(Name_), Params(Params_), CVQuals(CVQuals_),
+RefQual(RefQual_) {
 for (Node *P : Params)
   ParameterPackSize = std::min(ParameterPackSize, P->ParameterPackSize);
 if (Ret)
@@ -785,66 +808,19 @@ public:
 S += ")";
 if (Ret)
   Ret->printRight(S);
-  }
-};
-
-enum FunctionRefQual : unsigned char {
-  FrefQualNone,
-  FrefQualLValue,
-  FrefQualRValue,
-};
 
-class FunctionRefQualType : public Node {
-  Node *Fn;
-  FunctionRefQual Quals;
-
-  friend class FunctionQualType;
-
-public:
-  FunctionRefQualType(Node *Fn_, FunctionRefQual Quals_)
-  : Node(KFunctionRefQualType, Fn_->ParameterPackSize,
- /*RHSComponentCache=*/Cache::Yes, /*ArrayCache=*/Cache::No,
- /*FunctionCache=*/Cache::Yes),
-Fn(Fn_), Quals(Quals_) {}
-
-  bool hasFunctionSlow(OutputStream &) const override { return true; }
-  bool hasRHSComponentSlow(OutputStream &) const override { return true; }
+if (CVQuals & QualConst)
+  S += " const";
+if (CVQuals & QualVolatile)
+  S += " volatile";
+if (CVQuals & QualRestrict)
+  S += " restrict";
 
-  void printQuals(OutputStream ) const {
-if (Quals == FrefQualLValue)
+if (RefQual == FrefQualLValue)
   S += " &";
-else
+else if (RefQual == FrefQualRValue)
   S += " &&";
   }
-
-  void printLeft(OutputStream ) const override { Fn->printLeft(S); }
-
-  void printRight(OutputStream ) const override {
-Fn->printRight(S);
-printQuals(S);
-  }
-};
-
-class FunctionQualType final : public QualType {
-public:
-  FunctionQualType(Node *Child_, Qualifiers Quals_)
-  : QualType(Child_, Quals_) {
-K = KFunctionQualType;
-  }
-
-  void printLeft(OutputStream ) const override { Child->printLeft(S); }
-
-  void printRight(OutputStream ) const override {
-if (Child->getKind() == KFunctionRefQualType) {
-  auto *RefQuals = static_cast(Child);
-  RefQuals->Fn->printRight(S);
-  printQuals(S);
-  RefQuals->printQuals(S);
-} else {
-  Child->printRight(S);
-  printQuals(S);
-}
-  }
 };
 
 class LiteralOperator : public Node {
@@ -2077,7 +2053,7 @@ struct Db {
   Node *parseArrayType();
   Node *parsePointerToMemberType();
   Node *parseClassEnumType();
-  Node

[PATCH] D42755: [libcxx] Fix last_write_time tests for filesystems that don't support very small times.

2018-02-13 Thread Eric Fiselier via Phabricator via cfe-commits

EricWF added a comment.

LGTM except the removal of the test. I think it's probably valuable to keep 
around on platforms that allow it.

What do you think?




Comment at: 
libcxx/test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp:360
-
-ec = GetTestEC();
-last_write_time(p, Clock::now());

I would really love to keep this test. I think it has the ability to detect 
incorrect integer arithmetic in `last_write_time` when UBSAN is enabled.

Could we maybe add another check like `SupportsAlmostMinTime` where that checks 
`::min() + MicroSec(1)`?


https://reviews.llvm.org/D42755



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

[PATCH] D42498: [ExprConstant] Fix crash when initialize an indirect field with another field.

2018-02-13 Thread Volodymyr Sapsai via Phabricator via cfe-commits

vsapsai added a comment.
Herald added a subscriber: jkorous-apple.

Ping.


https://reviews.llvm.org/D42498



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

[PATCH] D43167: Fix incorrect indentation.

2018-02-13 Thread Bruce Mitchener via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL325087: Fix incorrect indentation. (authored by brucem, 
committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D43167

Files:
  libcxx/trunk/include/ios


Index: libcxx/trunk/include/ios
===
--- libcxx/trunk/include/ios
+++ libcxx/trunk/include/ios
@@ -670,7 +670,7 @@
 void set_rdbuf(basic_streambuf* __sb);
 private:
 basic_ostream* __tie_;
- mutable int_type __fill_;
+mutable int_type __fill_;
 };
 
 template 


Index: libcxx/trunk/include/ios
===
--- libcxx/trunk/include/ios
+++ libcxx/trunk/include/ios
@@ -670,7 +670,7 @@
 void set_rdbuf(basic_streambuf* __sb);
 private:
 basic_ostream* __tie_;
- mutable int_type __fill_;
+mutable int_type __fill_;
 };
 
 template 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43259: Implement function attribute artificial

2018-02-13 Thread Erich Keane via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL325081: Implement function attribute artificial (authored by 
erichkeane, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D43259?vs=134123=134146#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D43259

Files:
  cfe/trunk/include/clang/Basic/Attr.td
  cfe/trunk/include/clang/Basic/AttrDocs.td
  cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
  cfe/trunk/lib/Sema/SemaDeclAttr.cpp
  cfe/trunk/test/CodeGen/artificial.c
  cfe/trunk/test/Sema/artificial.c


Index: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
===
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
@@ -3235,7 +3235,7 @@
   if (Name.startswith("\01"))
 Name = Name.substr(1);
 
-  if (!HasDecl || D->isImplicit()) {
+  if (!HasDecl || D->isImplicit() || D->hasAttr()) {
 Flags |= llvm::DINode::FlagArtificial;
 // Artificial functions should not silently reuse CurLoc.
 CurLoc = SourceLocation();
Index: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
===
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp
@@ -6057,6 +6057,9 @@
   case AttributeList::AT_AlwaysInline:
 handleAlwaysInlineAttr(S, D, Attr);
 break;
+  case AttributeList::AT_Artificial:
+handleSimpleAttribute(S, D, Attr);
+break;
   case AttributeList::AT_AnalyzerNoReturn:
 handleAnalyzerNoReturnAttr(S, D, Attr);
 break;
Index: cfe/trunk/include/clang/Basic/AttrDocs.td
===
--- cfe/trunk/include/clang/Basic/AttrDocs.td
+++ cfe/trunk/include/clang/Basic/AttrDocs.td
@@ -3273,3 +3273,13 @@
 or `msvc documentation `_.
 }];
 }
+
+def ArtificialDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``artificial`` attribute is used with inline functions to treat the inline 
+function as a unit while debugging. For more information see GCC_ 
documentation.
+
+.. _GCC: https://gcc.gnu.org/onlinedocs/gcc-4.6.4/gcc/Function-Attributes.html
+  }];
+}
Index: cfe/trunk/include/clang/Basic/Attr.td
===
--- cfe/trunk/include/clang/Basic/Attr.td
+++ cfe/trunk/include/clang/Basic/Attr.td
@@ -111,6 +111,9 @@
 def GlobalVar : SubsetSubjecthasGlobalStorage()}], "global variables">;
 
+def InlineFunction : SubsetSubjectisInlineSpecified()}], "inline functions">;
+
 // FIXME: this hack is needed because DeclNodes.td defines the base Decl node
 // type to be a class, not a definition. This makes it impossible to create an
 // attribute subject which accepts a Decl. Normally, this is not a problem,
@@ -588,6 +591,12 @@
   let Documentation = [Undocumented];
 }
 
+def Artificial : InheritableAttr {
+  let Spellings = [GCC<"artificial">];
+  let Subjects = SubjectList<[InlineFunction], WarnDiag>;
+  let Documentation = [ArtificialDocs];
+}
+
 def XRayInstrument : InheritableAttr {
   let Spellings = [Clang<"xray_always_instrument">,
Clang<"xray_never_instrument">];
Index: cfe/trunk/test/Sema/artificial.c
===
--- cfe/trunk/test/Sema/artificial.c
+++ cfe/trunk/test/Sema/artificial.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+inline void __attribute__((artificial)) foo() {}
+void __attribute__((artificial)) bar() {} // expected-warning {{'artificial' 
attribute only applies to inline functions}}
Index: cfe/trunk/test/CodeGen/artificial.c
===
--- cfe/trunk/test/CodeGen/artificial.c
+++ cfe/trunk/test/CodeGen/artificial.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -emit-llvm 
-debug-info-kind=limited %s -o - | FileCheck %s
+
+extern void foo();
+// CHECK: !DISubprogram(name: "foo"
+// CHECK-SAME: flags: DIFlagArtificial
+inline void __attribute__((artificial)) foo() {}
+
+void baz() {
+  foo();
+}


Index: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
===
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
@@ -3235,7 +3235,7 @@
   if (Name.startswith("\01"))
 Name = Name.substr(1);
 
-  if (!HasDecl || D->isImplicit()) {
+  if (!HasDecl || D->isImplicit() || D->hasAttr()) {
 Flags |= llvm::DINode::FlagArtificial;
 // Artificial functions should not silently reuse CurLoc.
 CurLoc = SourceLocation();
Index: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
===
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp
@@

r325081 - Implement function attribute artificial

2018-02-13 Thread Erich Keane via cfe-commits

Author: erichkeane
Date: Tue Feb 13 16:14:07 2018
New Revision: 325081

URL: http://llvm.org/viewvc/llvm-project?rev=325081=rev
Log:
Implement function attribute artificial

Added support in clang for GCC function attribute 'artificial'. This attribute 
is used to control stepping behavior of debugger with respect to inline 
functions.

Patch By: Elizabeth Andrews (eandrews)

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


Added:
cfe/trunk/test/CodeGen/artificial.c
cfe/trunk/test/Sema/artificial.c
Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/AttrDocs.td
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=325081=325080=325081=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Tue Feb 13 16:14:07 2018
@@ -111,6 +111,9 @@ def SharedVar : SubsetSubjecthasGlobalStorage()}], "global variables">;
 
+def InlineFunction : SubsetSubjectisInlineSpecified()}], "inline functions">;
+
 // FIXME: this hack is needed because DeclNodes.td defines the base Decl node
 // type to be a class, not a definition. This makes it impossible to create an
 // attribute subject which accepts a Decl. Normally, this is not a problem,
@@ -588,6 +591,12 @@ def AlwaysInline : InheritableAttr {
   let Documentation = [Undocumented];
 }
 
+def Artificial : InheritableAttr {
+  let Spellings = [GCC<"artificial">];
+  let Subjects = SubjectList<[InlineFunction], WarnDiag>;
+  let Documentation = [ArtificialDocs];
+}
+
 def XRayInstrument : InheritableAttr {
   let Spellings = [Clang<"xray_always_instrument">,
Clang<"xray_never_instrument">];

Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=325081=325080=325081=diff
==
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Tue Feb 13 16:14:07 2018
@@ -3273,3 +3273,13 @@ For more information see
 or `msvc documentation `_.
 }];
 }
+
+def ArtificialDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``artificial`` attribute is used with inline functions to treat the inline 
+function as a unit while debugging. For more information see GCC_ 
documentation.
+
+.. _GCC: https://gcc.gnu.org/onlinedocs/gcc-4.6.4/gcc/Function-Attributes.html
+  }];
+}

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=325081=325080=325081=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Tue Feb 13 16:14:07 2018
@@ -3235,7 +3235,7 @@ void CGDebugInfo::EmitFunctionStart(Glob
   if (Name.startswith("\01"))
 Name = Name.substr(1);
 
-  if (!HasDecl || D->isImplicit()) {
+  if (!HasDecl || D->isImplicit() || D->hasAttr()) {
 Flags |= llvm::DINode::FlagArtificial;
 // Artificial functions should not silently reuse CurLoc.
 CurLoc = SourceLocation();

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=325081=325080=325081=diff
==
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Tue Feb 13 16:14:07 2018
@@ -6057,6 +6057,9 @@ static void ProcessDeclAttribute(Sema 
   case AttributeList::AT_AlwaysInline:
 handleAlwaysInlineAttr(S, D, Attr);
 break;
+  case AttributeList::AT_Artificial:
+handleSimpleAttribute(S, D, Attr);
+break;
   case AttributeList::AT_AnalyzerNoReturn:
 handleAnalyzerNoReturnAttr(S, D, Attr);
 break;

Added: cfe/trunk/test/CodeGen/artificial.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/artificial.c?rev=325081=auto
==
--- cfe/trunk/test/CodeGen/artificial.c (added)
+++ cfe/trunk/test/CodeGen/artificial.c Tue Feb 13 16:14:07 2018
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -emit-llvm 
-debug-info-kind=limited %s -o - | FileCheck %s
+
+extern void foo();
+// CHECK: !DISubprogram(name: "foo"
+// CHECK-SAME: flags: DIFlagArtificial
+inline void __attribute__((artificial)) foo() {}
+
+void baz() {
+  foo();
+}

Added: cfe/trunk/test/Sema/artificial.c
URL:

[PATCH] D40925: Add option -fkeep-static-consts

2018-02-13 Thread Erich Keane via Phabricator via cfe-commits

erichkeane added a comment.

>> OK. My concern is that users need to use __attribute__((used)) or something 
>> more robust if they want SVN identifiers to reliably appear in the output. 
>> Adding this flag just creates a trap that will fail once they turn on >>-O2. 
>> I'd rather not have it in the interface to avoid that user confusion.

Since the idea here is to have a global option for 'always emit these things', 
could a viable solution simply add all of these things to @llvm.used.  This 
wouldn't affect the usefulness of __attribute((used)), but could be implemented 
similarly, right?


https://reviews.llvm.org/D40925



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

r325070 - [analyzer] [tests] Update CmpRuns to write to stdout correctly in multithreaded environment

2018-02-13 Thread George Karpenkov via cfe-commits

Author: george.karpenkov
Date: Tue Feb 13 15:36:01 2018
New Revision: 325070

URL: http://llvm.org/viewvc/llvm-project?rev=325070=rev
Log:
[analyzer] [tests] Update CmpRuns to write to stdout correctly in multithreaded 
environment

Modified:
cfe/trunk/utils/analyzer/CmpRuns.py
cfe/trunk/utils/analyzer/SATestBuild.py

Modified: cfe/trunk/utils/analyzer/CmpRuns.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/analyzer/CmpRuns.py?rev=325070=325069=325070=diff
==
--- cfe/trunk/utils/analyzer/CmpRuns.py (original)
+++ cfe/trunk/utils/analyzer/CmpRuns.py Tue Feb 13 15:36:01 2018
@@ -26,6 +26,7 @@ Usage:
 
 """
 
+import sys
 import os
 import plistlib
 from math import log
@@ -264,7 +265,8 @@ def compareResults(A, B, opts):
 return res
 
 
-def dumpScanBuildResultsDiff(dirA, dirB, opts, deleteEmpty=True):
+def dumpScanBuildResultsDiff(dirA, dirB, opts, deleteEmpty=True,
+ Stdout=sys.stdout):
 # Load the run results.
 resultsA = loadResults(dirA, opts, opts.rootA, deleteEmpty)
 resultsB = loadResults(dirB, opts, opts.rootB, deleteEmpty)
@@ -282,30 +284,30 @@ def dumpScanBuildResultsDiff(dirA, dirB,
 for res in diff:
 a, b = res
 if a is None:
-print "ADDED: %r" % b.getReadableName()
+Stdout.write("ADDED: %r\n" % b.getReadableName())
 foundDiffs += 1
 totalAdded += 1
 if auxLog:
-print >>auxLog, ("('ADDED', %r, %r)" % (b.getReadableName(),
-b.getReport()))
+auxLog.write("('ADDED', %r, %r)\n" % (b.getReadableName(),
+  b.getReport()))
 elif b is None:
-print "REMOVED: %r" % a.getReadableName()
+Stdout.write("REMOVED: %r\n" % a.getReadableName())
 foundDiffs += 1
 totalRemoved += 1
 if auxLog:
-print >>auxLog, ("('REMOVED', %r, %r)" % (a.getReadableName(),
-  a.getReport()))
+auxLog.write("('REMOVED', %r, %r)\n" % (a.getReadableName(),
+a.getReport()))
 else:
 pass
 
 TotalReports = len(resultsB.diagnostics)
-print "TOTAL REPORTS: %r" % TotalReports
-print "TOTAL DIFFERENCES: %r" % foundDiffs
-print "TOTAL ADDED: %r" % totalAdded
-print "TOTAL REMOVED: %r" % totalRemoved
+Stdout.write("TOTAL REPORTS: %r\n" % TotalReports)
+Stdout.write("TOTAL ADDED: %r\n" % totalAdded)
+Stdout.write("TOTAL REMOVED: %r\n" % totalRemoved)
 if auxLog:
-print >>auxLog, "('TOTAL NEW REPORTS', %r)" % TotalReports
-print >>auxLog, "('TOTAL DIFFERENCES', %r)" % foundDiffs
+auxLog.write("('TOTAL NEW REPORTS', %r)\n" % TotalReports)
+auxLog.write("('TOTAL DIFFERENCES', %r)\n" % foundDiffs)
+auxLog.close()
 
 return foundDiffs, len(resultsA.diagnostics), len(resultsB.diagnostics)
 

Modified: cfe/trunk/utils/analyzer/SATestBuild.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/analyzer/SATestBuild.py?rev=325070=325069=325070=diff
==
--- cfe/trunk/utils/analyzer/SATestBuild.py (original)
+++ cfe/trunk/utils/analyzer/SATestBuild.py Tue Feb 13 15:36:01 2018
@@ -570,7 +570,9 @@ def runCmpResults(Dir, Strictness=0):
 ["--rootA", "", "--rootB", PatchedSourceDirPath])
 # Scan the results, delete empty plist files.
 NumDiffs, ReportsInRef, ReportsInNew = \
-CmpRuns.dumpScanBuildResultsDiff(RefDir, NewDir, Opts, False)
+CmpRuns.dumpScanBuildResultsDiff(RefDir, NewDir, Opts,
+ deleteEmpty=False,
+ Stdout=Local.stdout)
 if (NumDiffs > 0):
 Local.stdout.write("Warning: %s differences in diagnostics.\n"
% NumDiffs)


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

[PATCH] D43259: Implement function attribute artificial

2018-02-13 Thread Reid Kleckner via Phabricator via cfe-commits

rnk accepted this revision.
rnk added subscribers: probinson, aprantl, dblaikie.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm

---

Clang's builtin headers use `__attribute__((__nodebug__))` for this purpose. Do 
you think we should follow this up by using artificial instead? It seems like 
it would be a representational improvement. @aprantl @probinson @dblaikie




Comment at: test/Sema/artificial.c:3
+
+void __attribute__((artificial)) bar() {} // expected-warning {{'artificial' 
attribute only applies to inline functions}}

I think it's worth adding the `foo` function from the CodeGen test here to show 
we don't generate warnings when the function is inline specified.


https://reviews.llvm.org/D43259



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

[PATCH] D41228: [ObjC] Enable __strong pointers in structs under ARC

2018-02-13 Thread John McCall via Phabricator via cfe-commits

rjmccall added inline comments.



Comment at: include/clang/AST/Type.h:1102
+PDK_Struct   // non-trivial C struct.
+  };
+

This is unused.



Comment at: lib/CodeGen/CGBlocks.cpp:1565
 // For all other types, the memcpy is fine.
 return std::make_pair(BlockCaptureEntityKind::None, Flags);
 

Please restructure this code to use the result of 
isNonTrivialToPrimitiveCopy(), preferably in a way that involves an exhaustive 
switch.  That should be as easy as breaking out in the PCK_Strong case and in 
the PCK_None case when !isObjCRetainableType().



Comment at: lib/CodeGen/CGBlocks.cpp:1773
+return std::make_pair(BlockCaptureEntityKind::NonTrivialCStruct,
+  BlockFieldFlags());
+

Same as above with using the result of isDestructedType().



Comment at: lib/CodeGen/CGBlocks.cpp:2070
+  bool needsDispose() const override {
+return VarType.isDestructedType() == QualType::DK_nontrivial_c_struct;
+  }

Just isDestructedType() should be fine.



Comment at: lib/CodeGen/CGDecl.cpp:1299
+return;
+  }
+

Again, it would be nice to restructure this code to use the result of 
isNonTrivialToPrimitiveDefaultInitialize() exhaustively.



Comment at: lib/CodeGen/CGNonTrivialStruct.cpp:38
+  template 
+  static void visit(Visitor , QualType FT, bool IsVolatile, unsigned Offset,
+Ts... Args) {

These visitors would be re-usable components outside of IRGen if they just took 
a QualType and then forwarded an arbitrary set of other arguments.  I would 
suggest structuring them like this:

  template  struct DestructedTypeVisitor {
Derived () { return static_cast(*this); }

template 
RetTy visit(QualType Ty, Ts &&...Args) {
  return asDerived().visit(Ty.isDestructedType(), Ty, 
std::forward(Args)...);
}

template 
RetTy visit(QualType::DestructionKind DK, QualType Ty, Ts &&...Args) {
  switch (DK) {
  case QualType::DK_none: return asDerived().visitTrivial(Ty, 
std::forward(Args)...);
  case QualType::DK_cxx_destructor: return 
asDerived().visitCXXDestructor(Ty, std::forward(Args)...);
  ...
  }
}
  };

You can then pretty easily add the special behavior for IsVolatile and arrays 
(when DK is not DK_none) in an IRGen-specific subclass.  visitArray would take 
a DK and then recurse by calling back into visit with that same DK.



Comment at: lib/CodeGen/CGNonTrivialStruct.cpp:55
+  break;
+default:
+  break;

Better not to have a default case here.  You can add an assert for the C++ case 
that it should never get here.



Comment at: lib/CodeGen/CGNonTrivialStruct.cpp:92
+  }
+};
+

This entire template is dead.



Comment at: lib/CodeGen/CGNonTrivialStruct.cpp:110
+  unsigned FOffset =
+  Offset + RL.getFieldOffset(FieldNo++) / Ctx.getCharWidth();
+  FieldVisitor::visit(getDerived(), FT,

Please pass around offsets as CharUnits until you actually need to interact 
with LLVM.



Comment at: lib/CodeGen/CGNonTrivialStruct.cpp:131
+
+template  struct BinaryFuncStructVisitor {
+  typedef BinaryFieldVisitor FieldVisitorTy;

I think you should just call this CopyStructVisitor.



Comment at: lib/CodeGen/CGNonTrivialStruct.cpp:145
+if (PCK)
+  getDerived().flushTrivialFields(Args...);
+

This would also be a good place to check for arrays so you don't have to do it 
in every non-trivial case below.

There's a couple other places in this file where I would suggest the same thing.



Comment at: lib/CodeGen/CGNonTrivialStruct.cpp:289
+/// Return an address with the specified offset from the passed address.
+static Address getAddrWithOffset(Address Addr, unsigned Offset,
+ CodeGenFunction ) {

Offset should definitely be a CharUnits here.



Comment at: lib/CodeGen/CGNonTrivialStruct.cpp:432
+// If the special function already exists in the module, return it.
+if (llvm::Function *F = CGM.getModule().getFunction(FuncName))
+  return F;

You should at least make sure this has the right function type; it's possible, 
even if not really allowed, to declare a C function that collides with one of 
these names.



Comment at: lib/CodeGen/CodeGenFunction.h:3407
+ QualType QT, bool IsVolatile);
+  void callCStructDestructor(Address DstPtr, QualType QT, bool IsVolatile);
+

I think we're moving towards generally taking LValues instead of Addresses 
unless you're talking about a very low-level routine.


https://reviews.llvm.org/D41228

[PATCH] D43259: Implement function attribute artificial

2018-02-13 Thread Elizabeth Andrews via Phabricator via cfe-commits

eandrews created this revision.
eandrews added reviewers: erichkeane, aaron.ballman.

Added support in clang for GCC function attribute 'artificial'. This attribute 
is used to control stepping behavior of debugger with respect to inline 
functions.


https://reviews.llvm.org/D43259

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  lib/CodeGen/CGDebugInfo.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGen/artificial.c
  test/Sema/artificial.c


Index: test/Sema/artificial.c
===
--- /dev/null
+++ test/Sema/artificial.c
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+void __attribute__((artificial)) bar() {} // expected-warning {{'artificial' 
attribute only applies to inline functions}}
Index: test/CodeGen/artificial.c
===
--- /dev/null
+++ test/CodeGen/artificial.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -emit-llvm 
-debug-info-kind=limited %s -o - | FileCheck %s
+
+extern void foo();
+// CHECK: !DISubprogram(name: "foo"
+// CHECK-SAME: flags: DIFlagArtificial
+inline void __attribute__((artificial)) foo() {}
+
+void baz() {
+  foo();
+}
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -6057,6 +6057,9 @@
   case AttributeList::AT_AlwaysInline:
 handleAlwaysInlineAttr(S, D, Attr);
 break;
+  case AttributeList::AT_Artificial:
+handleSimpleAttribute(S, D, Attr);
+break;
   case AttributeList::AT_AnalyzerNoReturn:
 handleAnalyzerNoReturnAttr(S, D, Attr);
 break;
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -3229,7 +3229,7 @@
   if (Name.startswith("\01"))
 Name = Name.substr(1);
 
-  if (!HasDecl || D->isImplicit()) {
+  if (!HasDecl || D->isImplicit() || D->hasAttr()) {
 Flags |= llvm::DINode::FlagArtificial;
 // Artificial functions should not silently reuse CurLoc.
 CurLoc = SourceLocation();
Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -3273,3 +3273,13 @@
 or `msvc documentation `_.
 }];
 }
+
+def ArtificialDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``artificial`` attribute is used with inline functions to treat the inline 
+function as a unit while debugging. For more information see GCC_ 
documentation.
+
+.. _GCC: https://gcc.gnu.org/onlinedocs/gcc-4.6.4/gcc/Function-Attributes.html
+  }];
+}
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -111,6 +111,9 @@
 def GlobalVar : SubsetSubjecthasGlobalStorage()}], "global variables">;
 
+def InlineFunction : SubsetSubjectisInlineSpecified()}], "inline functions">;
+
 // FIXME: this hack is needed because DeclNodes.td defines the base Decl node
 // type to be a class, not a definition. This makes it impossible to create an
 // attribute subject which accepts a Decl. Normally, this is not a problem,
@@ -588,6 +591,12 @@
   let Documentation = [Undocumented];
 }
 
+def Artificial : InheritableAttr {
+  let Spellings = [GCC<"artificial">];
+  let Subjects = SubjectList<[InlineFunction], WarnDiag>;
+  let Documentation = [ArtificialDocs];
+}
+
 def XRayInstrument : InheritableAttr {
   let Spellings = [Clang<"xray_always_instrument">,
Clang<"xray_never_instrument">];


Index: test/Sema/artificial.c
===
--- /dev/null
+++ test/Sema/artificial.c
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+void __attribute__((artificial)) bar() {} // expected-warning {{'artificial' attribute only applies to inline functions}}
Index: test/CodeGen/artificial.c
===
--- /dev/null
+++ test/CodeGen/artificial.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -emit-llvm -debug-info-kind=limited %s -o - | FileCheck %s
+
+extern void foo();
+// CHECK: !DISubprogram(name: "foo"
+// CHECK-SAME: flags: DIFlagArtificial
+inline void __attribute__((artificial)) foo() {}
+
+void baz() {
+  foo();
+}
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -6057,6 +6057,9 @@
   case AttributeList::AT_AlwaysInline:
 handleAlwaysInlineAttr(S, D, Attr);
 break;
+  case AttributeList::AT_Artificial:
+

[libclc] r325061 - amdgpu/half_recip: Switch implementation to native_recip

2018-02-13 Thread Jan Vesely via cfe-commits

Author: jvesely
Date: Tue Feb 13 14:09:46 2018
New Revision: 325061

URL: http://llvm.org/viewvc/llvm-project?rev=325061=rev
Log:
amdgpu/half_recip: Switch implementation to native_recip

Reviewer: Tom Stellard 
Signed-off-by: Jan Vesely 

Added:
libclc/trunk/amdgpu/lib/math/half_recip.cl
Modified:
libclc/trunk/amdgpu/lib/SOURCES

Modified: libclc/trunk/amdgpu/lib/SOURCES
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgpu/lib/SOURCES?rev=325061=325060=325061=diff
==
--- libclc/trunk/amdgpu/lib/SOURCES (original)
+++ libclc/trunk/amdgpu/lib/SOURCES Tue Feb 13 14:09:46 2018
@@ -7,6 +7,7 @@ math/half_exp2.cl
 math/half_log.cl
 math/half_log10.cl
 math/half_log2.cl
+math/half_recip.cl
 math/half_rsqrt.cl
 math/half_sqrt.cl
 math/nextafter.cl

Added: libclc/trunk/amdgpu/lib/math/half_recip.cl
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgpu/lib/math/half_recip.cl?rev=325061=auto
==
--- libclc/trunk/amdgpu/lib/math/half_recip.cl (added)
+++ libclc/trunk/amdgpu/lib/math/half_recip.cl Tue Feb 13 14:09:46 2018
@@ -0,0 +1,6 @@
+#include 
+ 
+#define __CLC_FUNC recip
+#define __FLOAT_ONLY
+#define __CLC_BODY 
+#include 


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

[libclc] r325060 - amdgpu/half_log2: Switch implementation to native_log2

2018-02-13 Thread Jan Vesely via cfe-commits

Author: jvesely
Date: Tue Feb 13 14:09:44 2018
New Revision: 325060

URL: http://llvm.org/viewvc/llvm-project?rev=325060=rev
Log:
amdgpu/half_log2: Switch implementation to native_log2

Reviewer: Tom Stellard 
Signed-off-by: Jan Vesely 

Added:
libclc/trunk/amdgpu/lib/math/half_log2.cl
Modified:
libclc/trunk/amdgpu/lib/SOURCES

Modified: libclc/trunk/amdgpu/lib/SOURCES
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgpu/lib/SOURCES?rev=325060=325059=325060=diff
==
--- libclc/trunk/amdgpu/lib/SOURCES (original)
+++ libclc/trunk/amdgpu/lib/SOURCES Tue Feb 13 14:09:44 2018
@@ -6,6 +6,7 @@ math/half_exp10.cl
 math/half_exp2.cl
 math/half_log.cl
 math/half_log10.cl
+math/half_log2.cl
 math/half_rsqrt.cl
 math/half_sqrt.cl
 math/nextafter.cl

Added: libclc/trunk/amdgpu/lib/math/half_log2.cl
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgpu/lib/math/half_log2.cl?rev=325060=auto
==
--- libclc/trunk/amdgpu/lib/math/half_log2.cl (added)
+++ libclc/trunk/amdgpu/lib/math/half_log2.cl Tue Feb 13 14:09:44 2018
@@ -0,0 +1,6 @@
+#include 
+ 
+#define __CLC_FUNC log2
+#define __FLOAT_ONLY
+#define __CLC_BODY 
+#include 


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

[libclc] r325058 - amdgpu/half_log: Switch implementation to native_log

2018-02-13 Thread Jan Vesely via cfe-commits

Author: jvesely
Date: Tue Feb 13 14:09:41 2018
New Revision: 325058

URL: http://llvm.org/viewvc/llvm-project?rev=325058=rev
Log:
amdgpu/half_log: Switch implementation to native_log

Reviewer: Tom Stellard 
Signed-off-by: Jan Vesely 

Added:
libclc/trunk/amdgpu/lib/math/half_log.cl
Modified:
libclc/trunk/amdgpu/lib/SOURCES

Modified: libclc/trunk/amdgpu/lib/SOURCES
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgpu/lib/SOURCES?rev=325058=325057=325058=diff
==
--- libclc/trunk/amdgpu/lib/SOURCES (original)
+++ libclc/trunk/amdgpu/lib/SOURCES Tue Feb 13 14:09:41 2018
@@ -4,6 +4,7 @@ math/native_log10.cl
 math/half_exp.cl
 math/half_exp10.cl
 math/half_exp2.cl
+math/half_log.cl
 math/half_rsqrt.cl
 math/half_sqrt.cl
 math/nextafter.cl

Added: libclc/trunk/amdgpu/lib/math/half_log.cl
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgpu/lib/math/half_log.cl?rev=325058=auto
==
--- libclc/trunk/amdgpu/lib/math/half_log.cl (added)
+++ libclc/trunk/amdgpu/lib/math/half_log.cl Tue Feb 13 14:09:41 2018
@@ -0,0 +1,6 @@
+#include 
+ 
+#define __CLC_FUNC log
+#define __FLOAT_ONLY
+#define __CLC_BODY 
+#include 


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

[libclc] r325055 - amdgpu/half_exp: Switch implementation to native_exp

2018-02-13 Thread Jan Vesely via cfe-commits

Author: jvesely
Date: Tue Feb 13 14:09:35 2018
New Revision: 325055

URL: http://llvm.org/viewvc/llvm-project?rev=325055=rev
Log:
amdgpu/half_exp: Switch implementation to native_exp

Reviewer: Tom Stellard 
Signed-off-by: Jan Vesely 

Added:
libclc/trunk/amdgpu/lib/math/half_exp.cl
Modified:
libclc/trunk/amdgpu/lib/SOURCES

Modified: libclc/trunk/amdgpu/lib/SOURCES
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgpu/lib/SOURCES?rev=325055=325054=325055=diff
==
--- libclc/trunk/amdgpu/lib/SOURCES (original)
+++ libclc/trunk/amdgpu/lib/SOURCES Tue Feb 13 14:09:35 2018
@@ -1,6 +1,7 @@
 math/native_exp.cl
 math/native_log.cl
 math/native_log10.cl
+math/half_exp.cl
 math/half_rsqrt.cl
 math/half_sqrt.cl
 math/nextafter.cl

Added: libclc/trunk/amdgpu/lib/math/half_exp.cl
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgpu/lib/math/half_exp.cl?rev=325055=auto
==
--- libclc/trunk/amdgpu/lib/math/half_exp.cl (added)
+++ libclc/trunk/amdgpu/lib/math/half_exp.cl Tue Feb 13 14:09:35 2018
@@ -0,0 +1,6 @@
+#include 
+ 
+#define __CLC_FUNC exp
+#define __FLOAT_ONLY
+#define __CLC_BODY 
+#include 


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

[libclc] r325059 - amdgpu/half_log10: Switch implementation to native_log10

2018-02-13 Thread Jan Vesely via cfe-commits

Author: jvesely
Date: Tue Feb 13 14:09:42 2018
New Revision: 325059

URL: http://llvm.org/viewvc/llvm-project?rev=325059=rev
Log:
amdgpu/half_log10: Switch implementation to native_log10

Reviewer: Tom Stellard 
Signed-off-by: Jan Vesely 

Added:
libclc/trunk/amdgpu/lib/math/half_log10.cl
Modified:
libclc/trunk/amdgpu/lib/SOURCES

Modified: libclc/trunk/amdgpu/lib/SOURCES
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgpu/lib/SOURCES?rev=325059=325058=325059=diff
==
--- libclc/trunk/amdgpu/lib/SOURCES (original)
+++ libclc/trunk/amdgpu/lib/SOURCES Tue Feb 13 14:09:42 2018
@@ -5,6 +5,7 @@ math/half_exp.cl
 math/half_exp10.cl
 math/half_exp2.cl
 math/half_log.cl
+math/half_log10.cl
 math/half_rsqrt.cl
 math/half_sqrt.cl
 math/nextafter.cl

Added: libclc/trunk/amdgpu/lib/math/half_log10.cl
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgpu/lib/math/half_log10.cl?rev=325059=auto
==
--- libclc/trunk/amdgpu/lib/math/half_log10.cl (added)
+++ libclc/trunk/amdgpu/lib/math/half_log10.cl Tue Feb 13 14:09:42 2018
@@ -0,0 +1,6 @@
+#include 
+ 
+#define __CLC_FUNC log10
+#define __FLOAT_ONLY
+#define __CLC_BODY 
+#include 


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

[libclc] r325053 - amdgpu/half_rsqrt: Switch implementation to native_rsqrt

2018-02-13 Thread Jan Vesely via cfe-commits

Author: jvesely
Date: Tue Feb 13 14:09:31 2018
New Revision: 325053

URL: http://llvm.org/viewvc/llvm-project?rev=325053=rev
Log:
amdgpu/half_rsqrt: Switch implementation to native_rsqrt

Reviewer: Tom Stellard 
Signed-off-by: Jan Vesely 

Added:
libclc/trunk/amdgpu/lib/math/half_native_unary.inc
libclc/trunk/amdgpu/lib/math/half_rsqrt.cl
Modified:
libclc/trunk/amdgpu/lib/SOURCES

Modified: libclc/trunk/amdgpu/lib/SOURCES
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgpu/lib/SOURCES?rev=325053=325052=325053=diff
==
--- libclc/trunk/amdgpu/lib/SOURCES (original)
+++ libclc/trunk/amdgpu/lib/SOURCES Tue Feb 13 14:09:31 2018
@@ -1,5 +1,6 @@
 math/native_exp.cl
 math/native_log.cl
 math/native_log10.cl
+math/half_rsqrt.cl
 math/nextafter.cl
 math/sqrt.cl

Added: libclc/trunk/amdgpu/lib/math/half_native_unary.inc
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgpu/lib/math/half_native_unary.inc?rev=325053=auto
==
--- libclc/trunk/amdgpu/lib/math/half_native_unary.inc (added)
+++ libclc/trunk/amdgpu/lib/math/half_native_unary.inc Tue Feb 13 14:09:31 2018
@@ -0,0 +1,11 @@
+#include 
+
+#define __CLC_HALF_FUNC(x) __CLC_CONCAT(half_, x)
+#define __CLC_NATIVE_FUNC(x) __CLC_CONCAT(native_, x)
+
+_CLC_OVERLOAD _CLC_DEF __CLC_GENTYPE __CLC_HALF_FUNC(__CLC_FUNC)(__CLC_GENTYPE 
val) {
+   return __CLC_NATIVE_FUNC(__CLC_FUNC)(val);
+}
+
+#undef __CLC_NATIVE_FUNC
+#undef __CLC_HALF_FUNC

Added: libclc/trunk/amdgpu/lib/math/half_rsqrt.cl
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgpu/lib/math/half_rsqrt.cl?rev=325053=auto
==
--- libclc/trunk/amdgpu/lib/math/half_rsqrt.cl (added)
+++ libclc/trunk/amdgpu/lib/math/half_rsqrt.cl Tue Feb 13 14:09:31 2018
@@ -0,0 +1,6 @@
+#include 
+ 
+#define __CLC_FUNC rsqrt
+#define __FLOAT_ONLY
+#define __CLC_BODY 
+#include 


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

[libclc] r325056 - amdgpu/half_exp10: Switch implementation to native_exp10

2018-02-13 Thread Jan Vesely via cfe-commits

Author: jvesely
Date: Tue Feb 13 14:09:37 2018
New Revision: 325056

URL: http://llvm.org/viewvc/llvm-project?rev=325056=rev
Log:
amdgpu/half_exp10: Switch implementation to native_exp10

Reviewer: Tom Stellard 
Signed-off-by: Jan Vesely 

Added:
libclc/trunk/amdgpu/lib/math/half_exp10.cl
Modified:
libclc/trunk/amdgpu/lib/SOURCES

Modified: libclc/trunk/amdgpu/lib/SOURCES
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgpu/lib/SOURCES?rev=325056=325055=325056=diff
==
--- libclc/trunk/amdgpu/lib/SOURCES (original)
+++ libclc/trunk/amdgpu/lib/SOURCES Tue Feb 13 14:09:37 2018
@@ -2,6 +2,7 @@ math/native_exp.cl
 math/native_log.cl
 math/native_log10.cl
 math/half_exp.cl
+math/half_exp10.cl
 math/half_rsqrt.cl
 math/half_sqrt.cl
 math/nextafter.cl

Added: libclc/trunk/amdgpu/lib/math/half_exp10.cl
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgpu/lib/math/half_exp10.cl?rev=325056=auto
==
--- libclc/trunk/amdgpu/lib/math/half_exp10.cl (added)
+++ libclc/trunk/amdgpu/lib/math/half_exp10.cl Tue Feb 13 14:09:37 2018
@@ -0,0 +1,6 @@
+#include 
+ 
+#define __CLC_FUNC exp10
+#define __FLOAT_ONLY
+#define __CLC_BODY 
+#include 


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

[libclc] r325057 - amdgpu/half_exp2: Switch implementation to native_exp2

2018-02-13 Thread Jan Vesely via cfe-commits

Author: jvesely
Date: Tue Feb 13 14:09:38 2018
New Revision: 325057

URL: http://llvm.org/viewvc/llvm-project?rev=325057=rev
Log:
amdgpu/half_exp2: Switch implementation to native_exp2

Reviewer: Tom Stellard 
Signed-off-by: Jan Vesely 

Added:
libclc/trunk/amdgpu/lib/math/half_exp2.cl
Modified:
libclc/trunk/amdgpu/lib/SOURCES

Modified: libclc/trunk/amdgpu/lib/SOURCES
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgpu/lib/SOURCES?rev=325057=325056=325057=diff
==
--- libclc/trunk/amdgpu/lib/SOURCES (original)
+++ libclc/trunk/amdgpu/lib/SOURCES Tue Feb 13 14:09:38 2018
@@ -3,6 +3,7 @@ math/native_log.cl
 math/native_log10.cl
 math/half_exp.cl
 math/half_exp10.cl
+math/half_exp2.cl
 math/half_rsqrt.cl
 math/half_sqrt.cl
 math/nextafter.cl

Added: libclc/trunk/amdgpu/lib/math/half_exp2.cl
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgpu/lib/math/half_exp2.cl?rev=325057=auto
==
--- libclc/trunk/amdgpu/lib/math/half_exp2.cl (added)
+++ libclc/trunk/amdgpu/lib/math/half_exp2.cl Tue Feb 13 14:09:38 2018
@@ -0,0 +1,6 @@
+#include 
+ 
+#define __CLC_FUNC exp2
+#define __FLOAT_ONLY
+#define __CLC_BODY 
+#include 


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

[libclc] r325054 - amdgpu/half_sqrt: Switch implementation to native_sqrt

2018-02-13 Thread Jan Vesely via cfe-commits

Author: jvesely
Date: Tue Feb 13 14:09:33 2018
New Revision: 325054

URL: http://llvm.org/viewvc/llvm-project?rev=325054=rev
Log:
amdgpu/half_sqrt: Switch implementation to native_sqrt

Reviewer: Tom Stellard 
Signed-off-by: Jan Vesely 

Added:
libclc/trunk/amdgpu/lib/math/half_sqrt.cl
Modified:
libclc/trunk/amdgpu/lib/SOURCES

Modified: libclc/trunk/amdgpu/lib/SOURCES
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgpu/lib/SOURCES?rev=325054=325053=325054=diff
==
--- libclc/trunk/amdgpu/lib/SOURCES (original)
+++ libclc/trunk/amdgpu/lib/SOURCES Tue Feb 13 14:09:33 2018
@@ -2,5 +2,6 @@ math/native_exp.cl
 math/native_log.cl
 math/native_log10.cl
 math/half_rsqrt.cl
+math/half_sqrt.cl
 math/nextafter.cl
 math/sqrt.cl

Added: libclc/trunk/amdgpu/lib/math/half_sqrt.cl
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgpu/lib/math/half_sqrt.cl?rev=325054=auto
==
--- libclc/trunk/amdgpu/lib/math/half_sqrt.cl (added)
+++ libclc/trunk/amdgpu/lib/math/half_sqrt.cl Tue Feb 13 14:09:33 2018
@@ -0,0 +1,6 @@
+#include 
+ 
+#define __CLC_FUNC sqrt
+#define __FLOAT_ONLY
+#define __CLC_BODY 
+#include 


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

[PATCH] D43221: Teach Wreturn-type, Wunreachable-code, and alpha.deadcode.UnreachableCode to treat assume(0) like builtin_unreachable.

2018-02-13 Thread Nico Weber via Phabricator via cfe-commits

thakis closed this revision.
thakis added a comment.

r325052, thanks!


https://reviews.llvm.org/D43221



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

[PATCH] D42800: Let CUDA toolchain support amdgpu target

2018-02-13 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl updated this revision to Diff 134107.
yaxunl added a comment.

Update with Greg's change.


https://reviews.llvm.org/D42800

Files:
  include/clang/Basic/Cuda.h
  include/clang/Driver/ToolChain.h
  lib/Basic/Cuda.cpp
  lib/Basic/Targets/AMDGPU.cpp
  lib/Basic/Targets/AMDGPU.h
  lib/Basic/Targets/NVPTX.cpp
  lib/Driver/Driver.cpp
  lib/Driver/SanitizerArgs.cpp
  lib/Driver/ToolChain.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/Cuda.cpp
  lib/Driver/ToolChains/Cuda.h
  test/Driver/cuda-phases.cu

Index: test/Driver/cuda-phases.cu
===
--- test/Driver/cuda-phases.cu
+++ test/Driver/cuda-phases.cu
@@ -7,22 +7,25 @@
 // REQUIRES: clang-driver
 // REQUIRES: powerpc-registered-target
 // REQUIRES: nvptx-registered-target
-
+// REQUIRES: amdgpu-registered-target
 //
 // Test single gpu architecture with complete compilation.
 //
 // RUN: %clang -target powerpc64le-ibm-linux-gnu -ccc-print-phases --cuda-gpu-arch=sm_30 %s 2>&1 \
-// RUN: | FileCheck -check-prefix=BIN %s
+// RUN: | FileCheck -check-prefixes=BIN,BIN_NV %s
+// RUN: %clang -target powerpc64le-ibm-linux-gnu -ccc-print-phases --cuda-gpu-arch=gfx803 %s 2>&1 \
+// RUN: | FileCheck -check-prefixes=BIN,BIN_AMD %s
 // BIN-DAG: [[P0:[0-9]+]]: input, "{{.*}}cuda-phases.cu", cuda, (host-cuda)
 // BIN-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, cuda-cpp-output, (host-cuda)
 // BIN-DAG: [[P2:[0-9]+]]: compiler, {[[P1]]}, ir, (host-cuda)
-// BIN-DAG: [[P3:[0-9]+]]: input, "{{.*}}cuda-phases.cu", cuda, (device-cuda, sm_30)
-// BIN-DAG: [[P4:[0-9]+]]: preprocessor, {[[P3]]}, cuda-cpp-output, (device-cuda, sm_30)
-// BIN-DAG: [[P5:[0-9]+]]: compiler, {[[P4]]}, ir, (device-cuda, sm_30)
-// BIN-DAG: [[P6:[0-9]+]]: backend, {[[P5]]}, assembler, (device-cuda, sm_30)
-// BIN-DAG: [[P7:[0-9]+]]: assembler, {[[P6]]}, object, (device-cuda, sm_30)
-// BIN-DAG: [[P8:[0-9]+]]: offload, "device-cuda (nvptx64-nvidia-cuda:sm_30)" {[[P7]]}, object
-// BIN-DAG: [[P9:[0-9]+]]: offload, "device-cuda (nvptx64-nvidia-cuda:sm_30)" {[[P6]]}, assembler
+// BIN_NV-DAG: [[P3:[0-9]+]]: input, "{{.*}}cuda-phases.cu", cuda, (device-cuda, [[ARCH:sm_30]])
+// BIN_AMD-DAG: [[P3:[0-9]+]]: input, "{{.*}}cuda-phases.cu", cuda, (device-cuda, [[ARCH:gfx803]])
+// BIN-DAG: [[P4:[0-9]+]]: preprocessor, {[[P3]]}, cuda-cpp-output, (device-cuda, [[ARCH]])
+// BIN-DAG: [[P5:[0-9]+]]: compiler, {[[P4]]}, ir, (device-cuda, [[ARCH]])
+// BIN-DAG: [[P6:[0-9]+]]: backend, {[[P5]]}, assembler, (device-cuda, [[ARCH]])
+// BIN-DAG: [[P7:[0-9]+]]: assembler, {[[P6]]}, object, (device-cuda, [[ARCH]])
+// BIN-DAG: [[P8:[0-9]+]]: offload, "device-cuda (nvptx64-nvidia-cuda:[[ARCH]])" {[[P7]]}, object
+// BIN-DAG: [[P9:[0-9]+]]: offload, "device-cuda (nvptx64-nvidia-cuda:[[ARCH]])" {[[P6]]}, assembler
 // BIN-DAG: [[P10:[0-9]+]]: linker, {[[P8]], [[P9]]}, cuda-fatbin, (device-cuda)
 // BIN-DAG: [[P11:[0-9]+]]: offload, "host-cuda (powerpc64le-ibm-linux-gnu)" {[[P2]]}, "device-cuda (nvptx64-nvidia-cuda)" {[[P10]]}, ir
 // BIN-DAG: [[P12:[0-9]+]]: backend, {[[P11]]}, assembler, (host-cuda)
@@ -34,11 +37,13 @@
 //
 // RUN: %clang -target powerpc64le-ibm-linux-gnu -ccc-print-phases --cuda-gpu-arch=sm_30 %s -S 2>&1 \
 // RUN: | FileCheck -check-prefix=ASM %s
-// ASM-DAG: [[P0:[0-9]+]]: input, "{{.*}}cuda-phases.cu", cuda, (device-cuda, sm_30)
-// ASM-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, cuda-cpp-output, (device-cuda, sm_30)
-// ASM-DAG: [[P2:[0-9]+]]: compiler, {[[P1]]}, ir, (device-cuda, sm_30)
-// ASM-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (device-cuda, sm_30)
-// ASM-DAG: [[P4:[0-9]+]]: offload, "device-cuda (nvptx64-nvidia-cuda:sm_30)" {[[P3]]}, assembler
+// RUN: %clang -target powerpc64le-ibm-linux-gnu -ccc-print-phases --cuda-gpu-arch=gfx803 %s -S 2>&1 \
+// RUN: | FileCheck -check-prefix=ASM %s
+// ASM-DAG: [[P0:[0-9]+]]: input, "{{.*}}cuda-phases.cu", cuda, (device-cuda, [[ARCH:sm_30|gfx803]])
+// ASM-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, cuda-cpp-output, (device-cuda, [[ARCH]])
+// ASM-DAG: [[P2:[0-9]+]]: compiler, {[[P1]]}, ir, (device-cuda, [[ARCH]])
+// ASM-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (device-cuda, [[ARCH]])
+// ASM-DAG: [[P4:[0-9]+]]: offload, "device-cuda (nvptx64-nvidia-cuda:[[ARCH]])" {[[P3]]}, assembler
 // ASM-DAG: [[P5:[0-9]+]]: input, "{{.*}}cuda-phases.cu", cuda, (host-cuda)
 // ASM-DAG: [[P6:[0-9]+]]: preprocessor, {[[P5]]}, cuda-cpp-output, (host-cuda)
 // ASM-DAG: [[P7:[0-9]+]]: compiler, {[[P6]]}, ir, (host-cuda)
@@ -49,23 +54,25 @@
 //
 // RUN: %clang -target powerpc64le-ibm-linux-gnu -ccc-print-phases --cuda-gpu-arch=sm_30 --cuda-gpu-arch=sm_35 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=BIN2 %s
+// RUN: %clang -target powerpc64le-ibm-linux-gnu -ccc-print-phases --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s 2>&1 \
+// RUN: | FileCheck -check-prefix=BIN2 %s
 // BIN2-DAG: [[P0:[0-9]+]]: input, "{{.*}}cuda-phases.cu", cuda, (host-cuda)
 // BIN2-DAG: [[P1:[0-9]+]]:

r325052 - Teach Wreturn-type, Wunreachable-code, and alpha.deadcode.UnreachableCode to treat assume(0) like builtin_unreachable.

2018-02-13 Thread Nico Weber via cfe-commits

Author: nico
Date: Tue Feb 13 13:31:47 2018
New Revision: 325052

URL: http://llvm.org/viewvc/llvm-project?rev=325052=rev
Log:
Teach Wreturn-type, Wunreachable-code, and alpha.deadcode.UnreachableCode to 
treat __assume(0) like __builtin_unreachable.

Fixes PR29134.
https://reviews.llvm.org/D43221

Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/lib/AST/Expr.cpp
cfe/trunk/lib/Analysis/CFG.cpp
cfe/trunk/lib/Analysis/ReachableCode.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
cfe/trunk/test/Analysis/unreachable-code-path.c
cfe/trunk/test/Sema/return.c
cfe/trunk/test/Sema/warn-unreachable.c

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=325052=325051=325052=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Tue Feb 13 13:31:47 2018
@@ -2357,6 +2357,10 @@ public:
   SourceLocation getLocStart() const LLVM_READONLY;
   SourceLocation getLocEnd() const LLVM_READONLY;
 
+  /// Return true if this is a call to __assume() or __builtin_assume() with
+  /// a non-value-dependent constant parameter evaluating as false.
+  bool isBuiltinAssumeFalse(const ASTContext ) const;
+
   bool isCallToStdMove() const {
 const FunctionDecl* FD = getDirectCallee();
 return getNumArgs() == 1 && FD && FD->isInStdNamespace() &&

Modified: cfe/trunk/lib/AST/Expr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=325052=325051=325052=diff
==
--- cfe/trunk/lib/AST/Expr.cpp (original)
+++ cfe/trunk/lib/AST/Expr.cpp Tue Feb 13 13:31:47 2018
@@ -2917,6 +2917,18 @@ bool Expr::isConstantInitializer(ASTCont
   return false;
 }
 
+bool CallExpr::isBuiltinAssumeFalse(const ASTContext ) const {
+  const FunctionDecl* FD = getDirectCallee();
+  if (!FD || (FD->getBuiltinID() != Builtin::BI__assume &&
+  FD->getBuiltinID() != Builtin::BI__builtin_assume))
+return false;
+  
+  const Expr* Arg = getArg(0);
+  bool ArgVal;
+  return !Arg->isValueDependent() &&
+ Arg->EvaluateAsBooleanCondition(ArgVal, Ctx) && !ArgVal;
+}
+
 namespace {
   /// \brief Look for any side effects within a Stmt.
   class SideEffectFinder : public ConstEvaluatedExprVisitor {

Modified: cfe/trunk/lib/Analysis/CFG.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=325052=325051=325052=diff
==
--- cfe/trunk/lib/Analysis/CFG.cpp (original)
+++ cfe/trunk/lib/Analysis/CFG.cpp Tue Feb 13 13:31:47 2018
@@ -2134,7 +2134,7 @@ CFGBlock *CFGBuilder::VisitCallExpr(Call
   bool OmitArguments = false;
 
   if (FunctionDecl *FD = C->getDirectCallee()) {
-if (FD->isNoReturn())
+if (FD->isNoReturn() || C->isBuiltinAssumeFalse(*Context))
   NoReturn = true;
 if (FD->hasAttr())
   AddEHEdge = false;

Modified: cfe/trunk/lib/Analysis/ReachableCode.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ReachableCode.cpp?rev=325052=325051=325052=diff
==
--- cfe/trunk/lib/Analysis/ReachableCode.cpp (original)
+++ cfe/trunk/lib/Analysis/ReachableCode.cpp Tue Feb 13 13:31:47 2018
@@ -66,6 +66,21 @@ static bool isBuiltinUnreachable(const S
   return false;
 }
 
+static bool isBuiltinAssumeFalse(const CFGBlock *B, const Stmt *S,
+ ASTContext ) {
+  if (B->empty())  {
+// Happens if S is B's terminator and B contains nothing else
+// (e.g. a CFGBlock containing only a goto).
+return false;
+  }
+  if (Optional CS = B->back().getAs()) {
+if (const auto *CE = dyn_cast(CS->getStmt())) {
+  return CE->getCallee()->IgnoreCasts() == S && 
CE->isBuiltinAssumeFalse(C);
+}
+  }
+  return false;
+}
+
 static bool isDeadReturn(const CFGBlock *B, const Stmt *S) {
   // Look to see if the current control flow ends with a 'return', and see if
   // 'S' is a substatement. The 'return' may not be the last element in the
@@ -372,6 +387,7 @@ namespace {
 llvm::BitVector 
 SmallVector WorkList;
 Preprocessor 
+ASTContext 
 
 typedef SmallVector
 DeferredLocsTy;
@@ -379,10 +395,10 @@ namespace {
 DeferredLocsTy DeferredLocs;
 
   public:
-DeadCodeScan(llvm::BitVector , Preprocessor )
+DeadCodeScan(llvm::BitVector , Preprocessor , ASTContext )
 : Visited(reachable.size()),
   Reachable(reachable),
-  PP(PP) {}
+  PP(PP), C(C) {}
 
 void enqueue(const CFGBlock *block);
 unsigned scanBackwards(const CFGBlock *Start,
@@ -600,7 +616,8 @@ void DeadCodeScan::reportDeadCode(const
 
   if (isa(S)) {
 UK = reachable_code::UK_Break;
-  } else if (isTrivialDoWhile(B, S) || isBuiltinUnreachable(S))

[PATCH] D43221: Teach Wreturn-type, Wunreachable-code, and alpha.deadcode.UnreachableCode to treat assume(0) like builtin_unreachable.

2018-02-13 Thread Reid Kleckner via Phabricator via cfe-commits

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

lgtm

It looks like we already generate llvm.assume() for these and that eventually 
behaves the same as the unreachable instruction. Neat. :)


https://reviews.llvm.org/D43221



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

[PATCH] D41880: Adding nocf_check attribute for cf-protection fine tuning

2018-02-13 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a reviewer: aaron.ballman.
aaron.ballman added inline comments.



Comment at: include/clang/Basic/Attr.td:2089
+def AnyX86NoCfCheck : InheritableAttr, TargetSpecificAttr{
+  let Spellings = [GCC<"nocf_check">];
+  let Documentation = [AnyX86NoCfCheckDocs];

This attribute doesn't appear to be supported by GCC. Did you mean to use the 
Clang spelling instead?



Comment at: include/clang/Basic/AttrDocs.td:2876
+  let Content = [{
+Jump Oriented Programming attacks rely on tampering addresses used by
+indirect call / jmp, e.g. redirect control-flow to non-programmer

tampering with addresses



Comment at: include/clang/Basic/AttrDocs.td:2878
+indirect call / jmp, e.g. redirect control-flow to non-programmer
+intended bytes in binary.
+X86 Supports Indirect Branch Tracking (IBT) as part of Control-Flow

bytes in the binary



Comment at: lib/Sema/SemaDeclAttr.cpp:1990
 
-bool Sema::CheckNoReturnAttr(const AttributeList ) {
-  if (!checkAttributeNumArgs(*this, Attrs, 0)) {
-Attrs.setInvalid();
+static void handleNoCfCheckAttr(Sema , Decl *D, const AttributeList ) {
+  if (S.CheckAttrTarget(attr) || S.CheckAttrNoArgs(attr))

`attr` doesn't follow the proper naming conventions.



Comment at: lib/Sema/SemaDeclAttr.cpp:1991-1992
+static void handleNoCfCheckAttr(Sema , Decl *D, const AttributeList ) {
+  if (S.CheckAttrTarget(attr) || S.CheckAttrNoArgs(attr))
+return;
+

Why do you need to manually check the attribute has no args -- doesn't the 
common handling in `handleCommonAttributeFeatures()` already cover that?



Comment at: lib/Sema/SemaDeclAttr.cpp:1994-2002
+  if (!isFunctionOrMethod(D)) {
+ValueDecl *VD = dyn_cast(D);
+if (!VD || (!VD->getType()->isFunctionPointerType())) {
+  S.Diag(attr.getLoc(), diag::warn_attribute_wrong_decl_type)
+  << attr.getName() << ExpectedFunctionOrMethod;
+  return;
+}

I think all of this can be replaced by setting the proper subject on the 
attribute (`FunctionLike`) in Attr.td.



Comment at: test/Sema/attr-nocf_check.c:7-9
+// Allow function declaration and definition mismatch.
+void __attribute__((nocf_check)) testNoCfCheck();   // no-warning
+void __attribute__((nocf_check)) testNoCfCheck(){}; // no-warning

How is this mismatched?


Repository:
  rL LLVM

https://reviews.llvm.org/D41880



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

[PATCH] D39571: [clangd] DidChangeConfiguration Notification

2018-02-13 Thread Simon Marchi via Phabricator via cfe-commits

simark updated this revision to Diff 134097.
simark added a comment.

Add tests, work in progress

Can you take a look at the "TEST(DidChangeConfiguration, DifferentDeclaration)"
test, and tell me if you see anything wrong with it?  It seems to me like
changes in command line arguments (adding -DMACRO=1) are not taken into account
when changing configuration.

The other test (also very much work-in-progress) would be to verify that
changing configuration gets us the right diagnostics.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D39571

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/DraftStore.cpp
  clangd/DraftStore.h
  clangd/GlobalCompilationDatabase.cpp
  clangd/GlobalCompilationDatabase.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  unittests/clangd/XRefsTests.cpp

Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -257,6 +257,86 @@
SourceAnnotations.range()}));
 }
 
+TEST(DidChangeConfiguration, DifferentDeclaration) {
+  Annotations SourceAnnotations(R"cpp(
+#ifndef MACRO
+$before[[static void bob() {}]]
+#else
+$after[[static void bob() {}]]
+#endif
+
+int main() { b^ob(); }
+)cpp");
+
+  MockCompilationDatabase CDB(/*UseRelPaths=*/true);
+  MockFSProvider FS;
+
+  auto FooCpp = getVirtualTestFilePath("foo.cpp");
+  FS.Files[FooCpp] = "";
+
+  IgnoreDiagnostics DiagConsumer;
+  ClangdServer Server(CDB, DiagConsumer, FS, /*AsyncThreadsCount=*/0,
+  /*StorePreambleInMemory=*/true);
+
+  Server.addDocument(FooCpp, SourceAnnotations.code());
+
+  /* Without MACRO defined.  */
+  auto Locations = Server.findDefinitions(FooCpp, SourceAnnotations.point());
+  EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error";
+  EXPECT_THAT(Locations->Value,
+  ElementsAre(Location{URIForFile{FooCpp.str()},
+   SourceAnnotations.range("before")}));
+
+  CDB.ExtraClangFlags.push_back("-DMACRO=1");
+  Server.reparseOpenedFiles();
+
+  /* With MACRO defined.  */
+  Locations = Server.findDefinitions(FooCpp, SourceAnnotations.point());
+  EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error";
+  EXPECT_THAT(Locations->Value,
+  ElementsAre(Location{URIForFile{FooCpp.str()},
+   SourceAnnotations.range("after")}));
+}
+
+class MyDiags : public DiagnosticsConsumer {
+  void onDiagnosticsReady(
+  PathRef File, Tagged Diagnostics) override {
+std::cout << "There are " << Diagnostics.Value.size() << std::endl;
+for (DiagWithFixIts  : Diagnostics.Value) {
+  std::cout << "  " << Diag.Diag.message << std::endl;
+}
+  }
+};
+
+TEST(DidChangeConfiguration, Diagnostics) {
+  Annotations SourceAnnotations(R"cpp(
+#ifdef WITH_ERROR
+this is an error
+#endif
+
+int main() { return 0; }
+)cpp");
+
+  MockCompilationDatabase CDB(/*UseRelPaths=*/true);
+  CDB.ExtraClangFlags.push_back("-DWITH_ERROR=1");
+  MockFSProvider FS;
+
+  auto FooCpp = getVirtualTestFilePath("foo.cpp");
+  FS.Files[FooCpp] = "";
+
+  MyDiags DiagConsumer;
+  ClangdServer Server(CDB, DiagConsumer, FS, /*AsyncThreadsCount=*/0,
+  /*StorePreambleInMemory=*/true);
+
+  Server.addDocument(FooCpp, SourceAnnotations.code());
+
+
+  CDB.ExtraClangFlags.clear();
+
+  Server.reparseOpenedFiles();
+
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/ProtocolHandlers.h
===
--- clangd/ProtocolHandlers.h
+++ clangd/ProtocolHandlers.h
@@ -51,6 +51,7 @@
   virtual void onCommand(ExecuteCommandParams ) = 0;
   virtual void onRename(RenameParams ) = 0;
   virtual void onDocumentHighlight(TextDocumentPositionParams ) = 0;
+  virtual void onChangeConfiguration(DidChangeConfigurationParams ) = 0;
 };
 
 void registerCallbackHandlers(JSONRPCDispatcher , JSONOutput ,
Index: clangd/ProtocolHandlers.cpp
===
--- clangd/ProtocolHandlers.cpp
+++ clangd/ProtocolHandlers.cpp
@@ -71,4 +71,6 @@
   Register("workspace/executeCommand", ::onCommand);
   Register("textDocument/documentHighlight",
::onDocumentHighlight);
+  Register("workspace/didChangeConfiguration",
+   ::onChangeConfiguration);
 }
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -265,6 +265,20 @@
 };
 bool fromJSON(const json::Expr &, DidChangeWatchedFilesParams &);
 
+/// Clangd extension to manage a workspace/didChangeConfiguration notification
+/// since the data received is described as 'any' type in LSP.
+struct ClangdConfigurationParamsChange {
+

[PATCH] D39571: [clangd] DidChangeConfiguration Notification

2018-02-13 Thread Simon Marchi via Phabricator via cfe-commits

simark added a comment.

I think I managed to make some tests by using the `MockCompilationDatabase`.  
Basically with some code like:

  #ifndef MACRO
  static void func () {}  // 1
  #else
  static void func () {}  // 2
  #endif

and these steps:

1. Server.addDocument(...)
2. Server.findDefinitions (assert that it returns definition 1)
3. CDB.ExtraClangFlags.push_back("-DMACRO=1")
4. Server.reparseOpenedFiles()
5. Server.findDefinitions (assert that it returns definition 2)

Right now that test fails, but it's not clear to me whether it's because the 
test is wrong or there's really a bug in there.  I'll upload a work-in-progress 
version.




Comment at: clangd/ClangdServer.cpp:541
 
+std::vector
+ClangdServer::reparseOpenedFiles() {

ilya-biryukov wrote:
> We're not returning futures from `forceReparse` anymore, this function has to 
> be updated accordingly.
Indeed, I found this by rebasing.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D39571



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

[PATCH] D41880: Adding nocf_check attribute for cf-protection fine tuning

2018-02-13 Thread Craig Topper via Phabricator via cfe-commits

craig.topper added inline comments.



Comment at: lib/Sema/SemaDeclAttr.cpp:2007
+
+bool Sema::CheckAttrNoArgs(const AttributeList ) {
+  if (!checkAttributeNumArgs(*this, Attr, 0)) {

Wy did this get renamed?



Comment at: lib/Sema/SemaDeclAttr.cpp:2016
 
-bool Sema::CheckNoCallerSavedRegsAttr(const AttributeList ) {
+bool Sema::CheckAttrTarget(const AttributeList ) {
   // Check whether the attribute is valid on the current target.

Why did this get renamed?


Repository:
  rL LLVM

https://reviews.llvm.org/D41880



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

r325040 - Update StmtProfile.cpp to handle zero template arguments.

2018-02-13 Thread Richard Trieu via cfe-commits

Author: rtrieu
Date: Tue Feb 13 11:53:40 2018
New Revision: 325040

URL: http://llvm.org/viewvc/llvm-project?rev=325040=rev
Log:
Update StmtProfile.cpp to handle zero template arguments.

Treat having no templates arguments differently than having zero template
arguments when profiling.

Modified:
cfe/trunk/lib/AST/StmtProfile.cpp
cfe/trunk/test/Modules/odr_hash.cpp

Modified: cfe/trunk/lib/AST/StmtProfile.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/StmtProfile.cpp?rev=325040=325039=325040=diff
==
--- cfe/trunk/lib/AST/StmtProfile.cpp (original)
+++ cfe/trunk/lib/AST/StmtProfile.cpp Tue Feb 13 11:53:40 2018
@@ -966,8 +966,11 @@ void StmtProfiler::VisitDeclRefExpr(cons
   if (!Canonical)
 VisitNestedNameSpecifier(S->getQualifier());
   VisitDecl(S->getDecl());
-  if (!Canonical)
-VisitTemplateArguments(S->getTemplateArgs(), S->getNumTemplateArgs());
+  if (!Canonical) {
+ID.AddBoolean(S->hasExplicitTemplateArgs());
+if (S->hasExplicitTemplateArgs())
+  VisitTemplateArguments(S->getTemplateArgs(), S->getNumTemplateArgs());
+  }
 }
 
 void StmtProfiler::VisitPredefinedExpr(const PredefinedExpr *S) {

Modified: cfe/trunk/test/Modules/odr_hash.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/odr_hash.cpp?rev=325040=325039=325040=diff
==
--- cfe/trunk/test/Modules/odr_hash.cpp (original)
+++ cfe/trunk/test/Modules/odr_hash.cpp Tue Feb 13 11:53:40 2018
@@ -2924,6 +2924,21 @@ int I10 = F10();
 // expected-note@first.h:* {{but in 'FirstModule' found a different body}}
 }  // namespace FunctionDecl
 
+namespace DeclTemplateArguments {
+#if defined(FIRST)
+int foo() { return 1; }
+int bar() { return foo(); }
+#elif defined(SECOND)
+template 
+int foo() { return 2; }
+int bar() { return foo<>(); }
+#else
+int num = bar();
+// expected-error@second.h:* {{'DeclTemplateArguments::bar' has different 
definitions in different modules; definition in module 'SecondModule' first 
difference is function body}}
+// expected-note@first.h:* {{but in 'FirstModule' found a different body}}
+#endif
+}
+
 // Keep macros contained to one file.
 #ifdef FIRST
 #undef FIRST


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

[PATCH] D43246: [clangd] Assert path is absolute when assigning to URIForFile.

2018-02-13 Thread Sam McCall via Phabricator via cfe-commits

sammccall added inline comments.



Comment at: clangd/Protocol.h:52
 
 struct URIForFile {
+  URIForFile() = default;

I don't like how the API changes here take us further away from the other 
structs in this file.
Why does this one enforce invariants, when the others don't?

If we do want to make this more "safe", I'd suggest making it look more like a 
URI string (since that's what its protocol role is), like:

```
class LSPURI {
  LSPURI(StringRef URI) { assert if bad }
  static LSPURI ForFile(StringRef AbsPath) { assert if bad }
  operator string() { return URI::createFile(File).toString(); }
  StringRef file() { return AbsFile; }
  operator bool() { return !File.empty(); } 
  // move/copy constructor/assignment defaulted

 private:
  string AbsFile;
}
```

But the approach Eric suggested does seem promising: more consistent with how 
we handle invariants in protocol structs.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43246



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

[PATCH] D43227: [clangd] Make functions of ClangdServer callback-based

2018-02-13 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.

LG, suggest a tweak to capture() though.

I wonder whether we want to introduce `using Callback = 
UniqueFunction` for readability at some point...




Comment at: clangd/ClangdServer.cpp:209
+  auto Action = [Contents, Pos, TaggedFS,
+ PCHs](Path File, decltype(Callback) Callback,
+   llvm::Expected IP) {

ioeric wrote:
> ilya-biryukov wrote:
> > ioeric wrote:
> > > nit: I'd probably use a different name than `Callback` for this parameter 
> > > for clarity.
> > I actually think we should keep it this way. It's a bit tricky to grasp, 
> > but it has two important advantages:
> >   - Makes referencing `Callback` from outer scope impossible
> >   - saves us from coming up with names for that superficial variable, i.e. 
> > should it be called `InnerCallback`, `Callback2`, `C` or something else?
> > 
> > Is it that too confusing or do you feel we can keep it?
> > Makes referencing Callback from outer scope impossible. 
> For lambdas, I think we should rely on captures for this instead of the 
> parameter names.
> >saves us from coming up with names for that superficial variable, i.e. 
> >should it be called InnerCallback, Callback2, C or something else?
> I thought this is a common practice with llvm coding style :P I would vote 
> for `C` :) 
> 
> > Is it that too confusing or do you feel we can keep it?
> It is a bit confusing when I first looked at it, but nothing is *too* 
> confusing as long as the code is correct ;) I just think it would make the 
> code easier to read.
I agree with you both :-)
Conceptually, this *is* a capture - BindWithForward simulates move-capture that 
C++11 doesn't have native syntax for. So the same name seems appropriate to me 
- you want shadowing  for the same reasons you want captures to shadow.



Comment at: unittests/clangd/SyncAPI.cpp:69
+  llvm::Expected Result = Tagged();
+  (void)(bool)Result; // Expected has to be checked.
+  Server.signatureHelp(File, Pos, capture(Result), OverridenContents);

ioeric wrote:
> ilya-biryukov wrote:
> > ioeric wrote:
> > > I'd expect this to be checked by callers. Would `return 
> > > std::move(Result);` work?
> > The reason why we need it is because `capture(Result)` writes return value 
> > of the callback to the `Result` variable. But we have to first check the 
> > default-constructed value that was there **before** calling 
> > `Server.signatureHelp`
> I think we should probably fix the behavior of `capture` since this changes 
> the behavior of `llvm::Expected` in user code - the check is there to make 
> sure users always check the status. But here we always do this for users.
+1

I suggested `capture(T&)` was the right API because I thought it'd be used 
directly by test code (but we wrap it here) and because I thought T would be 
default-constructible without problems (but it's not).

I'd suggest changing to `capture(Optional&)` instead, so the caller can just 
create an empty optional. That makes the callsites here slightly less obscure.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43227



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

[PATCH] D43248: [Attr] Fix printing of parameter indices in attributes

2018-02-13 Thread Joel E. Denny via Phabricator via cfe-commits

jdenny created this revision.
jdenny added reviewers: aaron.ballman, hfinkel.

Parameter indices in some attributes (argument_with_type_tag,
pointer_with_type_tag, nonnull, ownership_takes, ownership_holds, and 
ownership_returns) are specified in source as one-origin including any 
this parameter, are stored as zero-origin excluding any this
parameter, and were erroneously printing (-ast-print) as the stored
values.  This patch fixes those cases by printing re-incremented
values.

An alternative solution is to store only the original values, but that
requires modifying all users.  I tried that for nonnull and found that
it required repeating index adjustment logic in many places while this
patch encapsulates that logic in relatively few places.

Another alternative is to store both sets of values, but that would be
less efficient.

This patch also fixes argument_with_type_tag and pointer_with_type_tag
not to print their fake IsPointer arguments.


https://reviews.llvm.org/D43248

Files:
  include/clang/Basic/Attr.td
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/attr-print.cpp
  utils/TableGen/ClangAttrEmitter.cpp

Index: utils/TableGen/ClangAttrEmitter.cpp
===
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -738,6 +738,42 @@
 }
   };
 
+  class VariadicParamIndexArgument : public VariadicArgument {
+std::string DisallowImplicitThisParamName;
+
+  protected:
+// Assumed to receive a parameter: raw_ostream OS.
+virtual void writeValueImpl(raw_ostream ) const {
+  OS << "OS << (Val + 1";
+  if (!DisallowImplicitThisParamName.empty())
+OS << " + get" << DisallowImplicitThisParamName << "()";
+  OS << ");\n";
+}
+  public:
+VariadicParamIndexArgument(const Record , StringRef Attr)
+: VariadicArgument(Arg, Attr, "unsigned"),
+  DisallowImplicitThisParamName(
+  Arg.getValueAsString("DisallowImplicitThisParamName")) {}
+  };
+
+  class ParamIndexArgument : public SimpleArgument {
+std::string DisallowImplicitThisParamName;
+
+  protected:
+void writeValue(raw_ostream ) const override {
+  OS << "\" << (get" << getUpperName() << "() + 1";
+  if (!DisallowImplicitThisParamName.empty())
+OS << " + get" << DisallowImplicitThisParamName << "()";
+  OS << ") << \"";
+}
+
+  public:
+ParamIndexArgument(const Record , StringRef Attr)
+: SimpleArgument(Arg, Attr, "unsigned"),
+  DisallowImplicitThisParamName(
+  Arg.getValueAsString("DisallowImplicitThisParamName")) {}
+  };
+
   // Unique the enums, but maintain the original declaration ordering.
   std::vector
   uniqueEnumsInOrder(const std::vector ) {
@@ -1237,6 +1273,10 @@
 Ptr = llvm::make_unique(Arg, Attr);
   else if (ArgName == "VariadicExprArgument")
 Ptr = llvm::make_unique(Arg, Attr);
+  else if (ArgName == "VariadicParamIndexArgument")
+Ptr = llvm::make_unique(Arg, Attr);
+  else if (ArgName == "ParamIndexArgument")
+Ptr = llvm::make_unique(Arg, Attr);
   else if (ArgName == "VersionArgument")
 Ptr = llvm::make_unique(Arg, Attr);
 
Index: test/Sema/attr-print.cpp
===
--- /dev/null
+++ test/Sema/attr-print.cpp
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 %s -ast-print | FileCheck %s
+
+void awtt(int, int, ...) __attribute__((argument_with_type_tag(foo, 3, 2)));
+// CHECK: void awtt(int, int, ...) __attribute__((argument_with_type_tag(foo, 3, 2)));
+void pwtt(void *, int) __attribute__((pointer_with_type_tag(foo, 1, 2)));
+// CHECK: void pwtt(void *, int) __attribute__((pointer_with_type_tag(foo, 1, 2)));
+
+void nn(int *, int *) __attribute__((nonnull(1, 2)));
+// CHECK: void nn(int *, int *) __attribute__((nonnull(1, 2)));
+
+void ownt(int *, int *) __attribute__((ownership_takes(foo, 1, 2)));
+// CHECK: void ownt(int *, int *) __attribute__((ownership_takes(foo, 1, 2)));
+void ownh(int *, int *) __attribute__((ownership_holds(foo, 1, 2)));
+// CHECK: void ownh(int *, int *) __attribute__((ownership_holds(foo, 1, 2)));
+void ownr(int) __attribute__((ownership_returns(foo, 1)));
+// CHECK: void ownr(int) __attribute__((ownership_returns(foo, 1)));
+
+class C {
+  void awtt(int, int, ...) __attribute__((argument_with_type_tag(foo, 4, 3)));
+  // CHECK: void awtt(int, int, ...) __attribute__((argument_with_type_tag(foo, 4, 3)));
+  void pwtt(void *, int) __attribute__((pointer_with_type_tag(foo, 2, 3)));
+  // CHECK: void pwtt(void *, int) __attribute__((pointer_with_type_tag(foo, 2, 3)));
+
+  void nn(int *, int *) __attribute__((nonnull(2, 3)));
+  // CHECK: void nn(int *, int *) __attribute__((nonnull(2, 3)));
+
+  void ownt(int *, int *) __attribute__((ownership_takes(foo, 2, 3)));
+  // CHECK: void ownt(int *, int *) __attribute__((ownership_takes(foo, 2, 3)));
+  void ownh(int *, int *) __attribute__((ownership_holds(foo, 2, 3)));
+  // CHECK: void

[PATCH] D43227: [clangd] Make functions of ClangdServer callback-based

2018-02-13 Thread Eric Liu via Phabricator via cfe-commits

ioeric added inline comments.



Comment at: clangd/ClangdLSPServer.cpp:194
+if (!Replacements)
+  return replyError(ErrorCode::InternalError,
+llvm::toString(Replacements.takeError()));

ilya-biryukov wrote:
> ioeric wrote:
> > nit: since we are not spelling out the return type here, it might be 
> > clearer if we do:
> > ```
> > replyError(...);
> > return;
> > ```
> > 
> > `return replyError(...)` makes me wonder what the return type is.
> This trick was used in the code before (there are usages below), so I figured 
> it's ok to do this.
> If it's confusing I'll change it everywhere.
I guess it's not confusing after figuring out what `replyError` returns. There 
is obviously tradeoff between LOC and readability. Either is not too bad. So up 
to you :)



Comment at: clangd/ClangdServer.cpp:209
+  auto Action = [Contents, Pos, TaggedFS,
+ PCHs](Path File, decltype(Callback) Callback,
+   llvm::Expected IP) {

ilya-biryukov wrote:
> ioeric wrote:
> > nit: I'd probably use a different name than `Callback` for this parameter 
> > for clarity.
> I actually think we should keep it this way. It's a bit tricky to grasp, but 
> it has two important advantages:
>   - Makes referencing `Callback` from outer scope impossible
>   - saves us from coming up with names for that superficial variable, i.e. 
> should it be called `InnerCallback`, `Callback2`, `C` or something else?
> 
> Is it that too confusing or do you feel we can keep it?
> Makes referencing Callback from outer scope impossible. 
For lambdas, I think we should rely on captures for this instead of the 
parameter names.
>saves us from coming up with names for that superficial variable, i.e. should 
>it be called InnerCallback, Callback2, C or something else?
I thought this is a common practice with llvm coding style :P I would vote for 
`C` :) 

> Is it that too confusing or do you feel we can keep it?
It is a bit confusing when I first looked at it, but nothing is *too* confusing 
as long as the code is correct ;) I just think it would make the code easier to 
read.



Comment at: unittests/clangd/SyncAPI.cpp:69
+  llvm::Expected Result = Tagged();
+  (void)(bool)Result; // Expected has to be checked.
+  Server.signatureHelp(File, Pos, capture(Result), OverridenContents);

ilya-biryukov wrote:
> ioeric wrote:
> > I'd expect this to be checked by callers. Would `return std::move(Result);` 
> > work?
> The reason why we need it is because `capture(Result)` writes return value of 
> the callback to the `Result` variable. But we have to first check the 
> default-constructed value that was there **before** calling 
> `Server.signatureHelp`
I think we should probably fix the behavior of `capture` since this changes the 
behavior of `llvm::Expected` in user code - the check is there to make sure 
users always check the status. But here we always do this for users.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43227



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

[PATCH] D43187: [AST] Refine the condition for element-dependent array fillers

2018-02-13 Thread John McCall via Phabricator via cfe-commits

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

LGTM.


Repository:
  rL LLVM

https://reviews.llvm.org/D43187



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

[PATCH] D43181: [CodeGen] Initialize large arrays by copying from a global

2018-02-13 Thread John McCall via Phabricator via cfe-commits

rjmccall added inline comments.



Comment at: lib/CodeGen/CGExprAgg.cpp:421
+  // with explicit initializers should be large enough.
+  if (NumInitElements > 8 && elementType->isBuiltinType()) {
+CodeGen::CodeGenModule  = CGF.CGM;

kosarev wrote:
> rjmccall wrote:
> > Is there a good reason to use an element-count heuristic instead of a 
> > total-size heuristic here?
> > 
> > Why only builtin types?  That seems to pointlessly rule out nested arrays, 
> > complex types, vectors, C structs, and so on.  I think the predicate you 
> > probably want here is isTriviallyCopyableType.
> > Is there a good reason to use an element-count heuristic instead of a 
> > total-size heuristic here?
> 
> Yes, the code below generates per-element initialization only for explicitly 
> specified initializers. The rest, if any, is initialized with a filler, so it 
> doesn't affect the size of the resulting code much.
That makes sense, but you could still base it on the total size being 
initialized with explicit initializers.  Such initializers, even when constant, 
are likely to require code size basically proportionate to the number of bytes 
initialized — sizes of immediate operands and all that.


https://reviews.llvm.org/D43181



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

[PATCH] D43227: [clangd] Make functions of ClangdServer callback-based

2018-02-13 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov updated this revision to Diff 134080.
ilya-biryukov added a comment.

- Capture only needed vars in lambda, not everything.
- Rebase onto head.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43227

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/SyncAPI.cpp
  unittests/clangd/SyncAPI.h
  unittests/clangd/XRefsTests.cpp

Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -10,6 +10,7 @@
 #include "ClangdUnit.h"
 #include "Compiler.h"
 #include "Matchers.h"
+#include "SyncAPI.h"
 #include "TestFS.h"
 #include "XRefs.h"
 #include "clang/Frontend/CompilerInvocation.h"
@@ -249,7 +250,8 @@
   FS.Files[FooCpp] = "";
 
   Server.addDocument(FooCpp, SourceAnnotations.code());
-  auto Locations = Server.findDefinitions(FooCpp, SourceAnnotations.point());
+  auto Locations =
+  runFindDefinitions(Server, FooCpp, SourceAnnotations.point());
   EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error";
 
   EXPECT_THAT(Locations->Value,
Index: unittests/clangd/SyncAPI.h
===
--- unittests/clangd/SyncAPI.h
+++ unittests/clangd/SyncAPI.h
@@ -22,6 +22,22 @@
 runCodeComplete(ClangdServer , PathRef File, Position Pos,
 clangd::CodeCompleteOptions Opts,
 llvm::Optional OverridenContents = llvm::None);
+
+llvm::Expected
+runSignatureHelp(ClangdServer , PathRef File, Position Pos,
+ llvm::Optional OverridenContents = llvm::None);
+
+llvm::Expected>
+runFindDefinitions(ClangdServer , PathRef File, Position Pos);
+
+llvm::Expected>
+runFindDocumentHighlights(ClangdServer , PathRef File, Position Pos);
+
+llvm::Expected
+runRename(ClangdServer , PathRef File, Position Pos, StringRef NewName);
+
+std::string runDumpAST(ClangdServer , PathRef File);
+
 } // namespace clangd
 } // namespace clang
 
Index: unittests/clangd/SyncAPI.cpp
===
--- unittests/clangd/SyncAPI.cpp
+++ unittests/clangd/SyncAPI.cpp
@@ -62,5 +62,47 @@
   return Result;
 }
 
+llvm::Expected
+runSignatureHelp(ClangdServer , PathRef File, Position Pos,
+ llvm::Optional OverridenContents) {
+  llvm::Expected Result = Tagged();
+  (void)(bool)Result; // Expected has to be checked.
+  Server.signatureHelp(File, Pos, capture(Result), OverridenContents);
+  return Result;
+}
+
+llvm::Expected>
+runFindDefinitions(ClangdServer , PathRef File, Position Pos) {
+  llvm::Expected> Result =
+  Tagged();
+  (void)(bool)Result; // Expected has to be checked.
+  Server.findDefinitions(File, Pos, capture(Result));
+  return Result;
+}
+
+llvm::Expected>
+runFindDocumentHighlights(ClangdServer , PathRef File, Position Pos) {
+  llvm::Expected> Result =
+  Tagged();
+  (void)(bool)Result; // Expected has to be checked.
+  Server.findDocumentHighlights(File, Pos, capture(Result));
+  return Result;
+}
+
+llvm::Expected
+runRename(ClangdServer , PathRef File, Position Pos, StringRef NewName) {
+  llvm::Expected Result =
+  std::vector();
+  (void)(bool)Result; // Expected has to be checked.
+  Server.rename(File, Pos, NewName, capture(Result));
+  return Result;
+}
+
+std::string runDumpAST(ClangdServer , PathRef File) {
+  std::string Result;
+  Server.dumpAST(File, capture(Result));
+  return Result;
+}
+
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -626,7 +626,7 @@
   Annotations Test(Text);
   Server.addDocument(File, Test.code());
   EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble";
-  auto R = Server.signatureHelp(File, Test.point());
+  auto R = runSignatureHelp(Server, File, Test.point());
   assert(R);
   return R.get().Value;
 }
Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -112,7 +112,7 @@
 }
 
 std::string dumpASTWithoutMemoryLocs(ClangdServer , PathRef File) {
-  auto DumpWithMemLocs = Server.dumpAST(File);
+  auto DumpWithMemLocs = runDumpAST(Server, File);
   return replacePtrsInDump(DumpWithMemLocs);
 }
 
@@ -450,17 +450,17 @@
   // Clang can't parse command args in that case, but we shouldn't crash.
   Server.addDocument(FooCpp, "int main() {}");
 
-  EXPECT_EQ(Server.dumpAST(FooCpp), "");
-

[PATCH] D43227: [clangd] Make functions of ClangdServer callback-based

2018-02-13 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added inline comments.



Comment at: clangd/ClangdLSPServer.cpp:194
+if (!Replacements)
+  return replyError(ErrorCode::InternalError,
+llvm::toString(Replacements.takeError()));

ioeric wrote:
> nit: since we are not spelling out the return type here, it might be clearer 
> if we do:
> ```
> replyError(...);
> return;
> ```
> 
> `return replyError(...)` makes me wonder what the return type is.
This trick was used in the code before (there are usages below), so I figured 
it's ok to do this.
If it's confusing I'll change it everywhere.



Comment at: clangd/ClangdServer.cpp:209
+  auto Action = [Contents, Pos, TaggedFS,
+ PCHs](Path File, decltype(Callback) Callback,
+   llvm::Expected IP) {

ioeric wrote:
> nit: I'd probably use a different name than `Callback` for this parameter for 
> clarity.
I actually think we should keep it this way. It's a bit tricky to grasp, but it 
has two important advantages:
  - Makes referencing `Callback` from outer scope impossible
  - saves us from coming up with names for that superficial variable, i.e. 
should it be called `InnerCallback`, `Callback2`, `C` or something else?

Is it that too confusing or do you feel we can keep it?



Comment at: clangd/ClangdServer.cpp:441
 
-  using RetType = llvm::Expected>;
-  auto Action = [=](llvm::Expected InpAST) -> RetType {
+  auto Action = [=](decltype(Callback) Callback,
+llvm::Expected InpAST) {

ioeric wrote:
> Consider spelling out the captured values, just in case new variables which 
> are not expected to be captured are added in the future.
Thanks for spotting this!



Comment at: unittests/clangd/SyncAPI.cpp:69
+  llvm::Expected Result = Tagged();
+  (void)(bool)Result; // Expected has to be checked.
+  Server.signatureHelp(File, Pos, capture(Result), OverridenContents);

ioeric wrote:
> I'd expect this to be checked by callers. Would `return std::move(Result);` 
> work?
The reason why we need it is because `capture(Result)` writes return value of 
the callback to the `Result` variable. But we have to first check the 
default-constructed value that was there **before** calling 
`Server.signatureHelp`


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43227



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

[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h

2018-02-13 Thread Sam McCall via Phabricator via cfe-commits

sammccall added inline comments.



Comment at: clangd/Protocol.h:80
 struct Position {
+  Position() = default;
+  Position(int line, int character) : line(line), character(character) {}

I'd lean to making callers initialize field-by-field here rather than provide 
constructors.
It's not terrible here (you can get it wrong, but only one way), but it's a 
pattern that doesn't scale so well I think.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43230



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

[PATCH] D43246: [clangd] Assert path is absolute when assigning to URIForFile.

2018-02-13 Thread Eric Liu via Phabricator via cfe-commits

ioeric added a comment.

I think another option to prevent the bug in r325029 from happening would be 
providing a canonical approach for retrieving (absolute) paths from 
`FileManager`. We already have code in symbol collector that does this, and we 
have similar code in XRefs.cpp now. We should probably move them to 
`clangd/Path.h` and share the code?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43246



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

[PATCH] D42995: [ThinLTO] Ignore object files with empty ThinLTO index

2018-02-13 Thread Vitaly Buka via Phabricator via cfe-commits

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




https://reviews.llvm.org/D42995



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

[PATCH] D42983: [clang-tidy] Add readability-simd-intrinsics check.

2018-02-13 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay marked 8 inline comments as done.
MaskRay added inline comments.



Comment at: clang-tidy/readability/SIMDIntrinsicsCheck.cpp:46
+
+  static const llvm::StringMap Mapping{
+// [simd.alg]

hokein wrote:
> consider using `llvm::StringSwitch`?
The list is currently a scaffold and more operations will be added. It may be 
more efficient to use a lookup table instead of cascading `.Case("add", ...)`?



Comment at: docs/clang-tidy/checks/readability-simd-intrinsics.rst:38
+
+   If set to non-zero, the check will be enabled and it will suggest 
``std::experimental`` alternatives. Default is ``0``.
+

hokein wrote:
> We don't use check's option to enable the check in clang-tidy, if users want 
> to enable the check, they need to pass the check explicitly to clang-tidy 
> (e.g. -checks=""). I'd suggest to remove this option.
Removed.I don't think the option is necessary.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42983



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

[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h

2018-02-13 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.

Yup, I got bitten recently from some of our plain-c-style structs with no 
default initializers (in Index).

Definitely a fan of this change. Main downside is you can't use aggregate 
initialization, but the field-by-field initialization is often more readable 
anyway.

And +1 to avoiding explicit `= None` for optionals, etc.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43230



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

[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h

2018-02-13 Thread Eric Liu via Phabricator via cfe-commits

ioeric added a comment.

In https://reviews.llvm.org/D43230#1006469, @ilya-biryukov wrote:

> In https://reviews.llvm.org/D43230#1006104, @ioeric wrote:
>
> > But I think it's safe and probably easier to rely on default values of 
> > primitive types like int, bool etc
>
>
> It's not always safe, as primitive types are sometimes left uninitialized 
> (e.g. when constructed on the stack) and reading an uninitialized value is UB.

Oh, you are absolutely right! I I think l had protos get into my mind...

> 
> 
>> but do we really want to make this a requirement for future changes or even 
>> in our coding style?
> 
> I think we should, default values are less surprising than UB. Other people 
> may disagree, though. 
>  @sammccall , @hokein , WDYT? Should we always initialize primitive types in 
> our code?

I think it would probably depend on whether we could find a sensible default 
value for a field (e.g. is 0 a better default value than UINT_MAX for line 
number?) and whether we expect users to always initialize a field (e.g. would 
it be better to pollute the field instead of providing a default value so that 
uninitialized values would be caught more easily).

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43230

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

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

2018-02-13 Thread Russell McClellan via Phabricator via cfe-commits

russellmcc added a comment.

Ping!  I believe all feedback has been addressed - further consideration would 
be much appreciated.


https://reviews.llvm.org/D40988



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

[PATCH] D42624: [clang-tidy] Add a utility Matcher to match the next statement within a statement sequence

2018-02-13 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang-tidy/utils/Matchers.h:16
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Analysis/CFG.h"
 

tbourvon wrote:
> aaron.ballman wrote:
> > This will require linking in the clangAnalysis library as well; are we sure 
> > we want to take on this dependency here for all matchers?
> Do you have a specific solution in mind? We could make the matcher local to 
> the check it is being used in (see D37014), but I think it could prove useful 
> for other checks...
No solution in mind -- I was mostly wondering if @alexfh was okay picking up 
this dependency or not, because it will impact all the clang-tidy modules.



Comment at: clang-tidy/utils/Matchers.h:58-60
+  // We get the first parent, making sure that we're not in a case statement
+  // not in a compound statement directly inside a switch, because this causes
+  // the buildCFG call to crash.

tbourvon wrote:
> aaron.ballman wrote:
> > Why does it cause the crash? Should that crash not be fixed instead of 
> > applying this workaround?
> I'm not entirely sure if this is expected behavior or not. In terms of AST, 
> `switch` statements are a bit special in terms of how they are represented 
> (each case contains all the subsequent cases as its children IIRC).
> There probably is a way to make the CFG work in these cases, but I honestly 
> don't have the time to look into that and attempt a fix. Couldn't this be 
> good enough for now, maybe with a FIXME?
I'm uncomfortable simply working around known crashes rather than fixing them, 
even with FIXME comments, because that just means we accrue more technical debt 
with no plan in place to pay it down in the future. However, I'll let @alexfh 
make the final call on it.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42624



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

[PATCH] D42983: [clang-tidy] Add readability-simd-intrinsics check.

2018-02-13 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay updated this revision to Diff 134076.
MaskRay marked an inline comment as done.
MaskRay added a comment.

Remove UseStdExperimental


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42983

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/readability/SIMDIntrinsicsCheck.cpp
  clang-tidy/readability/SIMDIntrinsicsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-simd-intrinsics.rst
  test/clang-tidy/readability-simd-intrinsics-ppc.cpp
  test/clang-tidy/readability-simd-intrinsics-x86.cpp

Index: test/clang-tidy/readability-simd-intrinsics-x86.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-simd-intrinsics-x86.cpp
@@ -0,0 +1,26 @@
+// RUN: %check_clang_tidy %s readability-simd-intrinsics %t -- \
+// RUN:  -config='{CheckOptions: [ \
+// RUN:{key: readability-simd-intrinsics.UseStdExperimental, value: 1} \
+// RUN:  ]}' -- -target x86_64 -std=c++11
+
+typedef long long __m128i __attribute__((vector_size(16)));
+typedef double __m256 __attribute__((vector_size(32)));
+
+__m128i _mm_add_epi32(__m128i, __m128i);
+__m256 _mm256_load_pd(double const *);
+void _mm256_store_pd(double *, __m256);
+
+int _mm_add_fake(int, int);
+
+void X86() {
+  __m128i i0, i1;
+  __m256 d0;
+
+  _mm_add_epi32(i0, i1);
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: '_mm_add_epi32' can be replaced by operator+ on std::experimental::simd objects [readability-simd-intrinsics]
+  d0 = _mm256_load_pd(0);
+  _mm256_store_pd(0, d0);
+
+  _mm_add_fake(0, 1);
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:
+}
Index: test/clang-tidy/readability-simd-intrinsics-ppc.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-simd-intrinsics-ppc.cpp
@@ -0,0 +1,13 @@
+// RUN: %check_clang_tidy %s readability-simd-intrinsics %t -- \
+// RUN:  -config='{CheckOptions: [ \
+// RUN:{key: readability-simd-intrinsics.UseStdExperimental, value: 1} \
+// RUN:  ]}' -- -target ppc64le -maltivec -std=c++11
+
+vector int vec_add(vector int, vector int);
+
+void PPC() {
+  vector int i0, i1;
+
+  vec_add(i0, i1);
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'vec_add' can be replaced by operator+ on std::experimental::simd objects [readability-simd-intrinsics]
+}
Index: docs/clang-tidy/checks/readability-simd-intrinsics.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-simd-intrinsics.rst
@@ -0,0 +1,33 @@
+.. title:: clang-tidy - readability-simd-intrinsics
+
+readability-simd-intrinsics
+===
+
+Finds SIMD intrinsics calls and suggests ``std::experimental::simd`` (`P0214`_) alternatives.
+
+If the option ``UseStdExperimental`` is set to non-zero, for
+
+.. code-block:: c++
+
+  _mm_add_epi32(a, b); // x86
+  vec_add(a, b);   // Power
+
+the check suggests an alternative:
+
+.. code-block::
+
+  operator+ on std::experimental::simd objects
+
+Many architectures provide SIMD operations (e.g. x86 SSE/AVX, Power AltiVec/VSX,
+ARM NEON). It is common that SIMD code implementing the same algorithm, is
+written in multiple target-dispatching pieces to optimize for different
+architectures or micro-architectures.
+
+The C++ standard proposal `P0214`_ and its extensions cover many common SIMD
+operations. By migrating from target-dependent intrinsics to `P0214` operations,
+the SIMD code can be simplified and pieces for different targets can be unified.
+
+Refer to `P0214`_ for introduction and motivation for the data-parallel standard
+library.
+
+.. _P0214: http://wg21.link/p0214
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -216,6 +216,7 @@
readability-redundant-smartptr-get
readability-redundant-string-cstr
readability-redundant-string-init
+   readability-simd-intrinsics
readability-simplify-boolean-expr
readability-static-accessed-through-instance
readability-static-definition-in-anonymous-namespace
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -88,6 +88,12 @@
   Functions that have trailing returns are disallowed, except for those 
   using decltype specifiers and lambda with otherwise unutterable 
   return types.
+
+- New `readability-simd-intrinsics
+  `_ check
+
+  Warns if SIMD intrinsics are used which can be replaced by
+  ``std::experimental::simd`` operations.
 
 - New alias `hicpp-avoid-goto
   `_ to 
Index: clang-tidy/readability/SIMDIntrinsicsCheck.h

[PATCH] D35894: [clangd] Code hover for Clangd

2018-02-13 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

In https://reviews.llvm.org/D35894#1006124, @simark wrote:

> Is there a way to get the macro name from the MacroInfo object?  I couldn't 
> find any, so the solution I'm considering is to make 
> `DeclarationAndMacrosFinder::takeMacroInfos` return an 
> `std::vector>`, where the first 
> member of the pair is the macro name.  It would come from 
> `IdentifierInfo->getName()` in `DeclarationAndMacrosFinder::finish`.  Does 
> that make sense, or is there a simpler way?

I don't think there's a way to get macro name from `MacroInfo`. `pair` sounds good to me, I'd probably even use a named struct here: 
`struct MacroDecl { StringRef Name; const MacroInfo  }`

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D35894

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

[PATCH] D38639: [clangd] #include statements support for Open definition

2018-02-13 Thread Marc-Andre Laperle via Phabricator via cfe-commits

malaperle added inline comments.



Comment at: unittests/clangd/XRefsTests.cpp:53
 
+class IgnoreDiagnostics : public DiagnosticsConsumer {
+  void onDiagnosticsReady(

ilya-biryukov wrote:
> NIT: remove this class, use `IgnoreDiagnostics` from `Compiler.h` instead.
It looks like IgnoreDiagnostics from Compiler.h implements DiagnosticConsumer 
from Compiler.h, and not DiagnosticsConsumer from ClangdServer.h so that 
wouldn't work.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D38639



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

[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h

2018-02-13 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

In https://reviews.llvm.org/D43230#1006104, @ioeric wrote:

> But I think it's safe and probably easier to rely on default values of 
> primitive types like int, bool etc

It's not always safe, as primitive types are sometimes left uninitialized (e.g. 
when constructed on the stack) and reading an uninitialized value is UB.

> but do we really want to make this a requirement for future changes or even 
> in our coding style?

I think we should, default values are less surprising than UB. Other people may 
disagree, though. 
@sammccall , @hokein , WDYT? Should we always initialize primitive types in our 
code?

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43230

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

[PATCH] D43171: [AMDGPU] Change constant addr space to 4 for clang

2018-02-13 Thread Yaxun Liu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rC325031: [AMDGPU] Change constant addr space to 4 (authored 
by yaxunl, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D43171?vs=133802=134071#toc

Repository:
  rC Clang

https://reviews.llvm.org/D43171

Files:
  include/clang/Basic/BuiltinsAMDGPU.def
  lib/Basic/Targets/AMDGPU.cpp
  test/CodeGen/target-data.c
  test/CodeGenOpenCL/address-space-constant-initializers.cl
  test/CodeGenOpenCL/address-spaces.cl
  test/CodeGenOpenCL/amdgpu-debug-info-variable-expression.cl
  test/CodeGenOpenCL/amdgpu-env-amdgcn.cl
  test/CodeGenOpenCL/amdgpu-env-amdgiz.cl
  test/CodeGenOpenCL/amdgpu-nullptr.cl
  test/CodeGenOpenCL/builtins-amdgcn.cl
  test/CodeGenOpenCL/cast_image.cl
  test/CodeGenOpenCL/opencl_types.cl
  test/CodeGenOpenCL/private-array-initialization.cl
  test/CodeGenOpenCL/size_t.cl
  test/CodeGenOpenCL/vla.cl

Index: include/clang/Basic/BuiltinsAMDGPU.def
===
--- include/clang/Basic/BuiltinsAMDGPU.def
+++ include/clang/Basic/BuiltinsAMDGPU.def
@@ -21,9 +21,9 @@
 // SI+ only builtins.
 //===--===//
 
-BUILTIN(__builtin_amdgcn_dispatch_ptr, "Uc*2", "nc")
-BUILTIN(__builtin_amdgcn_kernarg_segment_ptr, "Uc*2", "nc")
-BUILTIN(__builtin_amdgcn_implicitarg_ptr, "Uc*2", "nc")
+BUILTIN(__builtin_amdgcn_dispatch_ptr, "Uc*4", "nc")
+BUILTIN(__builtin_amdgcn_kernarg_segment_ptr, "Uc*4", "nc")
+BUILTIN(__builtin_amdgcn_implicitarg_ptr, "Uc*4", "nc")
 
 BUILTIN(__builtin_amdgcn_workgroup_id_x, "Ui", "nc")
 BUILTIN(__builtin_amdgcn_workgroup_id_y, "Ui", "nc")
Index: test/CodeGenOpenCL/vla.cl
===
--- test/CodeGenOpenCL/vla.cl
+++ test/CodeGenOpenCL/vla.cl
@@ -1,13 +1,14 @@
 // RUN: %clang_cc1 -emit-llvm -triple "spir-unknown-unknown" -O0 -cl-std=CL2.0 -o - %s | FileCheck -check-prefixes=CHECK,SPIR %s
-// RUN: %clang_cc1 -emit-llvm -triple amdgcn-amd-amdhsa-opencl -O0 -cl-std=CL2.0 -o - %s | FileCheck -check-prefixes=CHECK,SPIR %s
-// RUN: %clang_cc1 -emit-llvm -triple amdgcn-amd-amdhsa-amdgizcl -O0 -cl-std=CL2.0 -o - %s | FileCheck -check-prefixes=CHECK,GIZ %s
+// RUN: %clang_cc1 -emit-llvm -triple amdgcn-amd-amdhsa -O0 -cl-std=CL2.0 -o - %s | FileCheck -check-prefixes=CHECK,AMD %s
 
 constant int sz0 = 5;
-// CHECK: @sz0 = addrspace(2) constant i32 5
+// SPIR: @sz0 = addrspace(2) constant i32 5
+// AMD: @sz0 = addrspace(4) constant i32 5
 const global int sz1 = 16;
 // CHECK: @sz1 = addrspace(1) constant i32 16
 const constant int sz2 = 8;
-// CHECK: @sz2 = addrspace(2) constant i32 8
+// SPIR: @sz2 = addrspace(2) constant i32 8
+// AMD: @sz2 = addrspace(4) constant i32 8
 // CHECK: @testvla.vla2 = internal addrspace(3) global [8 x i16] undef
 
 kernel void testvla()
Index: test/CodeGenOpenCL/private-array-initialization.cl
===
--- test/CodeGenOpenCL/private-array-initialization.cl
+++ test/CodeGenOpenCL/private-array-initialization.cl
@@ -10,7 +10,7 @@
 
 // PRIVATE5: %arr = alloca [3 x i32], align 4, addrspace(5)
 // PRIVATE5: %0 = bitcast [3 x i32] addrspace(5)* %arr to i8 addrspace(5)*
-// PRIVATE5: call void @llvm.memcpy.p5i8.p2i8.i64(i8 addrspace(5)* align 4 %0, i8 addrspace(2)* align 4 bitcast ([3 x i32] addrspace(2)* @test.arr to i8 addrspace(2)*), i64 12, i1 false)
+// PRIVATE5: call void @llvm.memcpy.p5i8.p4i8.i64(i8 addrspace(5)* align 4 %0, i8 addrspace(4)* align 4 bitcast ([3 x i32] addrspace(4)* @test.arr to i8 addrspace(4)*), i64 12, i1 false)
 }
 
 __kernel void initializer_cast_is_valid_crash() {
Index: test/CodeGenOpenCL/builtins-amdgcn.cl
===
--- test/CodeGenOpenCL/builtins-amdgcn.cl
+++ test/CodeGenOpenCL/builtins-amdgcn.cl
@@ -434,22 +434,22 @@
 }
 
 // CHECK-LABEL: @test_dispatch_ptr
-// CHECK: call i8 addrspace(2)* @llvm.amdgcn.dispatch.ptr()
-void test_dispatch_ptr(__attribute__((address_space(2))) unsigned char ** out)
+// CHECK: call i8 addrspace(4)* @llvm.amdgcn.dispatch.ptr()
+void test_dispatch_ptr(__attribute__((address_space(4))) unsigned char ** out)
 {
   *out = __builtin_amdgcn_dispatch_ptr();
 }
 
 // CHECK-LABEL: @test_kernarg_segment_ptr
-// CHECK: call i8 addrspace(2)* @llvm.amdgcn.kernarg.segment.ptr()
-void test_kernarg_segment_ptr(__attribute__((address_space(2))) unsigned char ** out)
+// CHECK: call i8 addrspace(4)* @llvm.amdgcn.kernarg.segment.ptr()
+void test_kernarg_segment_ptr(__attribute__((address_space(4))) unsigned char ** out)
 {
   *out = __builtin_amdgcn_kernarg_segment_ptr();
 }
 
 // CHECK-LABEL: @test_implicitarg_ptr
-// CHECK: call i8 addrspace(2)* @llvm.amdgcn.implicitarg.ptr()
-void test_implicitarg_ptr(__attribute__((address_space(2))) unsigned char ** out)
+// CHECK: call i8 addrspace(4)* @llvm.amdgcn.implicitarg.ptr()
+void

r325031 - [AMDGPU] Change constant addr space to 4

2018-02-13 Thread Yaxun Liu via cfe-commits

Author: yaxunl
Date: Tue Feb 13 10:01:21 2018
New Revision: 325031

URL: http://llvm.org/viewvc/llvm-project?rev=325031=rev
Log:
[AMDGPU] Change constant addr space to 4

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

Added:
cfe/trunk/test/CodeGenOpenCL/amdgpu-env-amdgcn.cl
Removed:
cfe/trunk/test/CodeGenOpenCL/amdgpu-env-amdgiz.cl
Modified:
cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def
cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
cfe/trunk/test/CodeGen/target-data.c
cfe/trunk/test/CodeGenOpenCL/address-space-constant-initializers.cl
cfe/trunk/test/CodeGenOpenCL/address-spaces.cl
cfe/trunk/test/CodeGenOpenCL/amdgpu-debug-info-variable-expression.cl
cfe/trunk/test/CodeGenOpenCL/amdgpu-nullptr.cl
cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn.cl
cfe/trunk/test/CodeGenOpenCL/cast_image.cl
cfe/trunk/test/CodeGenOpenCL/opencl_types.cl
cfe/trunk/test/CodeGenOpenCL/private-array-initialization.cl
cfe/trunk/test/CodeGenOpenCL/size_t.cl
cfe/trunk/test/CodeGenOpenCL/vla.cl

Modified: cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def?rev=325031=325030=325031=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def Tue Feb 13 10:01:21 2018
@@ -21,9 +21,9 @@
 // SI+ only builtins.
 
//===--===//
 
-BUILTIN(__builtin_amdgcn_dispatch_ptr, "Uc*2", "nc")
-BUILTIN(__builtin_amdgcn_kernarg_segment_ptr, "Uc*2", "nc")
-BUILTIN(__builtin_amdgcn_implicitarg_ptr, "Uc*2", "nc")
+BUILTIN(__builtin_amdgcn_dispatch_ptr, "Uc*4", "nc")
+BUILTIN(__builtin_amdgcn_kernarg_segment_ptr, "Uc*4", "nc")
+BUILTIN(__builtin_amdgcn_implicitarg_ptr, "Uc*4", "nc")
 
 BUILTIN(__builtin_amdgcn_workgroup_id_x, "Ui", "nc")
 BUILTIN(__builtin_amdgcn_workgroup_id_y, "Ui", "nc")

Modified: cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/AMDGPU.cpp?rev=325031=325030=325031=diff
==
--- cfe/trunk/lib/Basic/Targets/AMDGPU.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/AMDGPU.cpp Tue Feb 13 10:01:21 2018
@@ -38,7 +38,7 @@ static const char *const DataLayoutStrin
 "-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64";
 
 static const char *const DataLayoutStringSIGenericIsZero =
-"e-p:64:64-p1:64:64-p2:64:64-p3:32:32-p4:32:32-p5:32:32-p6:32:32"
+"e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32"
 "-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128"
 "-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-A5";
 
@@ -46,11 +46,11 @@ static const LangASMap AMDGPUPrivIsZeroD
 4, // Default
 1, // opencl_global
 3, // opencl_local
-2, // opencl_constant
+4, // opencl_constant
 0, // opencl_private
 4, // opencl_generic
 1, // cuda_device
-2, // cuda_constant
+4, // cuda_constant
 3  // cuda_shared
 };
 
@@ -58,11 +58,11 @@ static const LangASMap AMDGPUGenIsZeroDe
 0, // Default
 1, // opencl_global
 3, // opencl_local
-2, // opencl_constant
+4, // opencl_constant
 5, // opencl_private
 0, // opencl_generic
 1, // cuda_device
-2, // cuda_constant
+4, // cuda_constant
 3  // cuda_shared
 };
 
@@ -70,11 +70,11 @@ static const LangASMap AMDGPUPrivIsZeroD
 0, // Default
 1, // opencl_global
 3, // opencl_local
-2, // opencl_constant
+4, // opencl_constant
 0, // opencl_private
 4, // opencl_generic
 1, // cuda_device
-2, // cuda_constant
+4, // cuda_constant
 3  // cuda_shared
 };
 
@@ -82,11 +82,11 @@ static const LangASMap AMDGPUGenIsZeroDe
 5, // Default
 1, // opencl_global
 3, // opencl_local
-2, // opencl_constant
+4, // opencl_constant
 5, // opencl_private
 0, // opencl_generic
 1, // cuda_device
-2, // cuda_constant
+4, // cuda_constant
 3  // cuda_shared
 };
 } // namespace targets

Modified: cfe/trunk/test/CodeGen/target-data.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/target-data.c?rev=325031=325030=325031=diff
==
--- cfe/trunk/test/CodeGen/target-data.c (original)
+++ cfe/trunk/test/CodeGen/target-data.c Tue Feb 13 10:01:21 2018
@@ -132,12 +132,12 @@
 
 // RUN: %clang_cc1 -triple amdgcn-unknown -target-cpu hawaii -o - -emit-llvm 
%s \
 // RUN: | FileCheck %s -check-prefix=R600SI
-// R600SI: target datalayout = 
"e-p:64:64-p1:64:64-p2:64:64-p3:32:32-p4:32:32-p5:32:32-p6:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-A5"
+// R600SI: target datalayout =

[PATCH] D43171: [AMDGPU] Change constant addr space to 4 for clang

2018-02-13 Thread Yaxun Liu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL325031: [AMDGPU] Change constant addr space to 4 (authored 
by yaxunl, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D43171?vs=133802=134070#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D43171

Files:
  cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def
  cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
  cfe/trunk/test/CodeGen/target-data.c
  cfe/trunk/test/CodeGenOpenCL/address-space-constant-initializers.cl
  cfe/trunk/test/CodeGenOpenCL/address-spaces.cl
  cfe/trunk/test/CodeGenOpenCL/amdgpu-debug-info-variable-expression.cl
  cfe/trunk/test/CodeGenOpenCL/amdgpu-env-amdgcn.cl
  cfe/trunk/test/CodeGenOpenCL/amdgpu-env-amdgiz.cl
  cfe/trunk/test/CodeGenOpenCL/amdgpu-nullptr.cl
  cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn.cl
  cfe/trunk/test/CodeGenOpenCL/cast_image.cl
  cfe/trunk/test/CodeGenOpenCL/opencl_types.cl
  cfe/trunk/test/CodeGenOpenCL/private-array-initialization.cl
  cfe/trunk/test/CodeGenOpenCL/size_t.cl
  cfe/trunk/test/CodeGenOpenCL/vla.cl

Index: cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def
===
--- cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def
+++ cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def
@@ -21,9 +21,9 @@
 // SI+ only builtins.
 //===--===//
 
-BUILTIN(__builtin_amdgcn_dispatch_ptr, "Uc*2", "nc")
-BUILTIN(__builtin_amdgcn_kernarg_segment_ptr, "Uc*2", "nc")
-BUILTIN(__builtin_amdgcn_implicitarg_ptr, "Uc*2", "nc")
+BUILTIN(__builtin_amdgcn_dispatch_ptr, "Uc*4", "nc")
+BUILTIN(__builtin_amdgcn_kernarg_segment_ptr, "Uc*4", "nc")
+BUILTIN(__builtin_amdgcn_implicitarg_ptr, "Uc*4", "nc")
 
 BUILTIN(__builtin_amdgcn_workgroup_id_x, "Ui", "nc")
 BUILTIN(__builtin_amdgcn_workgroup_id_y, "Ui", "nc")
Index: cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn.cl
===
--- cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn.cl
+++ cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn.cl
@@ -434,22 +434,22 @@
 }
 
 // CHECK-LABEL: @test_dispatch_ptr
-// CHECK: call i8 addrspace(2)* @llvm.amdgcn.dispatch.ptr()
-void test_dispatch_ptr(__attribute__((address_space(2))) unsigned char ** out)
+// CHECK: call i8 addrspace(4)* @llvm.amdgcn.dispatch.ptr()
+void test_dispatch_ptr(__attribute__((address_space(4))) unsigned char ** out)
 {
   *out = __builtin_amdgcn_dispatch_ptr();
 }
 
 // CHECK-LABEL: @test_kernarg_segment_ptr
-// CHECK: call i8 addrspace(2)* @llvm.amdgcn.kernarg.segment.ptr()
-void test_kernarg_segment_ptr(__attribute__((address_space(2))) unsigned char ** out)
+// CHECK: call i8 addrspace(4)* @llvm.amdgcn.kernarg.segment.ptr()
+void test_kernarg_segment_ptr(__attribute__((address_space(4))) unsigned char ** out)
 {
   *out = __builtin_amdgcn_kernarg_segment_ptr();
 }
 
 // CHECK-LABEL: @test_implicitarg_ptr
-// CHECK: call i8 addrspace(2)* @llvm.amdgcn.implicitarg.ptr()
-void test_implicitarg_ptr(__attribute__((address_space(2))) unsigned char ** out)
+// CHECK: call i8 addrspace(4)* @llvm.amdgcn.implicitarg.ptr()
+void test_implicitarg_ptr(__attribute__((address_space(4))) unsigned char ** out)
 {
   *out = __builtin_amdgcn_implicitarg_ptr();
 }
Index: cfe/trunk/test/CodeGenOpenCL/amdgpu-nullptr.cl
===
--- cfe/trunk/test/CodeGenOpenCL/amdgpu-nullptr.cl
+++ cfe/trunk/test/CodeGenOpenCL/amdgpu-nullptr.cl
@@ -30,7 +30,7 @@
 // CHECK: @global_p = local_unnamed_addr addrspace(1) global i8 addrspace(1)* null, align 8
 global char *global_p = 0;
 
-// CHECK: @constant_p = local_unnamed_addr addrspace(1) global i8 addrspace(2)* null, align 8
+// CHECK: @constant_p = local_unnamed_addr addrspace(1) global i8 addrspace(4)* null, align 8
 constant char *constant_p = 0;
 
 // CHECK: @generic_p = local_unnamed_addr addrspace(1) global i8* null, align 8
@@ -47,7 +47,7 @@
 // CHECK: @global_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(1)* null, align 8
 global char *global_p_NULL = NULL;
 
-// CHECK: @constant_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(2)* null, align 8
+// CHECK: @constant_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(4)* null, align 8
 constant char *constant_p_NULL = NULL;
 
 // CHECK: @generic_p_NULL = local_unnamed_addr addrspace(1) global i8* null, align 8
@@ -104,7 +104,7 @@
 // NOOPT: @test_static_var_private.sp3 = internal addrspace(1) global i8 addrspace(5)* null, align 4
 // NOOPT: @test_static_var_private.sp4 = internal addrspace(1) global i8 addrspace(5)* null, align 4
 // NOOPT: @test_static_var_private.sp5 = internal addrspace(1) global i8 addrspace(5)* null, align 4
-// NOOPT: @test_static_var_private.SS1 = internal addrspace(1) global %struct.StructTy1 { i8 addrspace(5)* null, i8 addrspace(3)* addrspacecast (i8* null to i8

[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h

2018-02-13 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added inline comments.



Comment at: clangd/Protocol.h:190
   /// (see exit notification) its process.
-  llvm::Optional processId;
+  llvm::Optional processId = 0;
 

jkorous-apple wrote:
> Sorry if I am asking stupid question but since the type is Optional<> isn't 
> it's default-constructed value more appropriate here?
Totally, my mistake. Fixed now.



Comment at: clangd/Protocol.h:211
   /// The initial trace setting. If omitted trace is disabled ('off').
-  llvm::Optional trace;
+  llvm::Optional trace = TraceLevel::Off;
 };

jkorous-apple wrote:
> Does it still make more sense to use Optional than plain 
> TraceLevel?
Optional here makes sense (LSP defines the field as nullable), the intializer 
didn't. Fixed now.




Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43230



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

[PATCH] D43227: [clangd] Make functions of ClangdServer callback-based

2018-02-13 Thread Eric Liu via Phabricator via cfe-commits

ioeric added inline comments.



Comment at: clangd/ClangdLSPServer.cpp:194
+if (!Replacements)
+  return replyError(ErrorCode::InternalError,
+llvm::toString(Replacements.takeError()));

nit: since we are not spelling out the return type here, it might be clearer if 
we do:
```
replyError(...);
return;
```

`return replyError(...)` makes me wonder what the return type is.



Comment at: clangd/ClangdServer.cpp:209
+  auto Action = [Contents, Pos, TaggedFS,
+ PCHs](Path File, decltype(Callback) Callback,
+   llvm::Expected IP) {

nit: I'd probably use a different name than `Callback` for this parameter for 
clarity.



Comment at: clangd/ClangdServer.cpp:441
 
-  using RetType = llvm::Expected>;
-  auto Action = [=](llvm::Expected InpAST) -> RetType {
+  auto Action = [=](decltype(Callback) Callback,
+llvm::Expected InpAST) {

Consider spelling out the captured values, just in case new variables which are 
not expected to be captured are added in the future.



Comment at: unittests/clangd/SyncAPI.cpp:69
+  llvm::Expected Result = Tagged();
+  (void)(bool)Result; // Expected has to be checked.
+  Server.signatureHelp(File, Pos, capture(Result), OverridenContents);

I'd expect this to be checked by callers. Would `return std::move(Result);` 
work?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43227



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

[PATCH] D43120: [clang-tidy] New checker for exceptions that are created but not thrown

2018-02-13 Thread Aaron Ballman via Phabricator via cfe-commits

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

I apologize for not noticing this important detail earlier -- I think this 
check should live under `bugprone` rather than `misc`. Other than where it 
lives (and the related naming issues), I think this is looking good!




Comment at: test/clang-tidy/misc-throw-keyword-missing.cpp:38
+
+// Classname contains the substring "exception", in certain cases using this 
class should emit a warning.
+struct RegularException {

Classname -> Class name


https://reviews.llvm.org/D43120



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

[PATCH] D43246: [clangd] Assert path is absolute when assigning to URIForFile.

2018-02-13 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov created this revision.
ilya-biryukov added reviewers: hokein, ioeric, sammccall.
Herald added subscribers: jkorous-apple, klimek.

The assertion will point directly to misbehaving code, so that
debugging related problems (like the one fixed by r325029) is easier.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43246

Files:
  clangd/ClangdLSPServer.cpp
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/XRefs.cpp

Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -149,7 +149,7 @@
 }
   }
 
-  L.uri.file = FilePath.str();
+  L.uri.setFile(FilePath.str());
   L.range = R;
   return L;
 }
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -25,6 +25,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_PROTOCOL_H
 
 #include "JSONExpr.h"
+#include "Path.h"
 #include "URI.h"
 #include "llvm/ADT/Optional.h"
 #include 
@@ -49,21 +50,28 @@
 };
 
 struct URIForFile {
-  std::string file;
+  URIForFile() = default;
+  URIForFile(Path AbsPath) { setFile(AbsPath); }
 
-  std::string uri() const { return URI::createFile(file).toString(); }
+  Path getFile() const { return File; }
+  void setFile(Path AbsPath);
+
+  std::string uri() const { return URI::createFile(File).toString(); }
 
   friend bool operator==(const URIForFile , const URIForFile ) {
-return LHS.file == RHS.file;
+return LHS.File == RHS.File;
   }
 
   friend bool operator!=(const URIForFile , const URIForFile ) {
 return !(LHS == RHS);
   }
 
   friend bool operator<(const URIForFile , const URIForFile ) {
-return LHS.file < RHS.file;
+return LHS.File < RHS.File;
   }
+
+private:
+  Path File;
 };
 
 /// Serialize/deserialize \p URIForFile to/from a string URI.
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -12,8 +12,8 @@
 //===--===//
 
 #include "Protocol.h"
-#include "URI.h"
 #include "Logger.h"
+#include "URI.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/Format.h"
@@ -24,6 +24,11 @@
 namespace clang {
 namespace clangd {
 
+void URIForFile::setFile(Path AbsPath) {
+  assert(llvm::sys::path::is_absolute(AbsPath) && "the path is relative");
+  File = std::move(AbsPath);
+}
+
 bool fromJSON(const json::Expr , URIForFile ) {
   if (auto S = E.asString()) {
 auto U = URI::parse(*S);
@@ -40,15 +45,13 @@
   log(llvm::toString(Path.takeError()));
   return false;
 }
-R.file = *Path;
+R.setFile(*Path);
 return true;
   }
   return false;
 }
 
-json::Expr toJSON(const URIForFile ) {
-  return U.uri();
-}
+json::Expr toJSON(const URIForFile ) { return U.uri(); }
 
 llvm::raw_ostream <<(llvm::raw_ostream , const URIForFile ) {
   return OS << U.uri();
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -116,8 +116,8 @@
  {"commands", {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND}},
  }},
 );
-  if (Params.rootUri && !Params.rootUri->file.empty())
-Server.setRootPath(Params.rootUri->file);
+  if (Params.rootUri && !Params.rootUri->getFile().empty())
+Server.setRootPath(Params.rootUri->getFile());
   else if (Params.rootPath && !Params.rootPath->empty())
 Server.setRootPath(*Params.rootPath);
 }
@@ -132,17 +132,17 @@
 
 void ClangdLSPServer::onDocumentDidOpen(DidOpenTextDocumentParams ) {
   if (Params.metadata && !Params.metadata->extraFlags.empty())
-CDB.setExtraFlagsForFile(Params.textDocument.uri.file,
+CDB.setExtraFlagsForFile(Params.textDocument.uri.getFile(),
  std::move(Params.metadata->extraFlags));
-  Server.addDocument(Params.textDocument.uri.file, Params.textDocument.text);
+  Server.addDocument(Params.textDocument.uri.getFile(), Params.textDocument.text);
 }
 
 void ClangdLSPServer::onDocumentDidChange(DidChangeTextDocumentParams ) {
   if (Params.contentChanges.size() != 1)
 return replyError(ErrorCode::InvalidParams,
   "can only apply one change at a time");
   // We only support full syncing right now.
-  Server.addDocument(Params.textDocument.uri.file,
+  Server.addDocument(Params.textDocument.uri.getFile(),
  Params.contentChanges[0].text);
 }
 
@@ -180,7 +180,7 @@
 }
 
 void ClangdLSPServer::onRename(RenameParams ) {
-  auto File = Params.textDocument.uri.file;
+  auto File = Params.textDocument.uri.getFile();
   auto Code = Server.getDocument(File);
   if (!Code)
 return replyError(ErrorCode::InvalidParams,
@@ -200,12 +200,12 @@
 }
 
 void ClangdLSPServer::onDocumentDidClose(DidCloseTextDocumentParams ) {
-

[PATCH] D42606: [Coroutines] Use allocator overload when available

2018-02-13 Thread Gor Nishanov via Phabricator via cfe-commits

GorNishanov added inline comments.



Comment at: lib/Sema/SemaCoroutine.cpp:1062
+  // an argument list."
+  for (auto *PD : FD.parameters()) {
+if (PD->getType()->isDependentType())

GorNishanov wrote:
> This does not implement TS specified behavior for non static member functions:
> 
> [dcl.fct.def.coroutine]/7 states that  an argument list is build up as 
> follows:
> 
> >   The first argument is the amount of space requested, and has type 
> > std::size_t. The lvalues p1 ... pn are the succeeding arguments. 
> 
> Where p1 ... pn are defined earlier in
> 
> [dcl.fct.def.coroutine]/3 as:
> 
> > For a coroutine f that is a non-static member function, let P1 denote the 
> > type of the implicit object parameter (13.3.1) and P2 ... Pn be the types 
> > of the function parameters; otherwise let P1 ... Pn be the types of the 
> > function parameters. 
> 
> Essentially for non-static member functions, we need to insert implicit 
> object parameter.
> 
> Note that lookupPromiseType in SemaCoroutine.cpp when building specialization 
> of `std::experimental::coroutine_traits<...>` includes implicit object 
> parameter:
> 
> ```
>   // If the function is a non-static member function, add the type
>   // of the implicit object parameter before the formal parameters.
>   if (auto *MD = dyn_cast(FD)) {
> if (MD->isInstance()) {
>   // [over.match.funcs]4
>   // For non-static member functions, the type of the implicit object
>   // parameter is
>   //  -- "lvalue reference to cv X" for functions declared without a
>   //  ref-qualifier or with the & ref-qualifier
>   //  -- "rvalue reference to cv X" for functions declared with the &&
>   //  ref-qualifier
>   QualType T =
>   MD->getThisType(S.Context)->getAs()->getPointeeType();
>   T = FnType->getRefQualifier() == RQ_RValue
>   ? S.Context.getRValueReferenceType(T)
>   : S.Context.getLValueReferenceType(T, /*SpelledAsLValue*/ true);
>   AddArg(T);
> }
>   }
> ```
I think promise constructor argument preview is also missing the implicit 
object parameter.


Repository:
  rC Clang

https://reviews.llvm.org/D42606



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

[PATCH] D42606: [Coroutines] Use allocator overload when available

2018-02-13 Thread Gor Nishanov via Phabricator via cfe-commits

GorNishanov added inline comments.



Comment at: lib/Sema/SemaCoroutine.cpp:1062
+  // an argument list."
+  for (auto *PD : FD.parameters()) {
+if (PD->getType()->isDependentType())

This does not implement TS specified behavior for non static member functions:

[dcl.fct.def.coroutine]/7 states that  an argument list is build up as follows:

>   The first argument is the amount of space requested, and has type 
> std::size_t. The lvalues p1 ... pn are the succeeding arguments. 

Where p1 ... pn are defined earlier in

[dcl.fct.def.coroutine]/3 as:

> For a coroutine f that is a non-static member function, let P1 denote the 
> type of the implicit object parameter (13.3.1) and P2 ... Pn be the types of 
> the function parameters; otherwise let P1 ... Pn be the types of the function 
> parameters. 

Essentially for non-static member functions, we need to insert implicit object 
parameter.

Note that lookupPromiseType in SemaCoroutine.cpp when building specialization 
of `std::experimental::coroutine_traits<...>` includes implicit object 
parameter:

```
  // If the function is a non-static member function, add the type
  // of the implicit object parameter before the formal parameters.
  if (auto *MD = dyn_cast(FD)) {
if (MD->isInstance()) {
  // [over.match.funcs]4
  // For non-static member functions, the type of the implicit object
  // parameter is
  //  -- "lvalue reference to cv X" for functions declared without a
  //  ref-qualifier or with the & ref-qualifier
  //  -- "rvalue reference to cv X" for functions declared with the &&
  //  ref-qualifier
  QualType T =
  MD->getThisType(S.Context)->getAs()->getPointeeType();
  T = FnType->getRefQualifier() == RQ_RValue
  ? S.Context.getRValueReferenceType(T)
  : S.Context.getLValueReferenceType(T, /*SpelledAsLValue*/ true);
  AddArg(T);
}
  }
```


Repository:
  rC Clang

https://reviews.llvm.org/D42606



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

[clang-tools-extra] r325029 - [clangd] Fixed findDefinitions to always return absolute paths.

2018-02-13 Thread Ilya Biryukov via cfe-commits

Author: ibiryukov
Date: Tue Feb 13 09:47:16 2018
New Revision: 325029

URL: http://llvm.org/viewvc/llvm-project?rev=325029=rev
Log:
[clangd] Fixed findDefinitions to always return absolute paths.

Relative paths could be returned in some cases, e.g. when relative
path is used in compilation arguments. This led to crash when trying
to convert the path to URI.

Modified:
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/unittests/clangd/TestFS.cpp
clang-tools-extra/trunk/unittests/clangd/TestFS.h
clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp

Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=325029=325028=325029=diff
==
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Tue Feb 13 09:47:16 2018
@@ -11,6 +11,7 @@
 #include "URI.h"
 #include "clang/Index/IndexDataConsumer.h"
 #include "clang/Index/IndexingAction.h"
+#include "llvm/Support/Path.h"
 namespace clang {
 namespace clangd {
 using namespace llvm;
@@ -138,10 +139,17 @@ getDeclarationLocation(ParsedAST , c
   Range R = {Begin, End};
   Location L;
 
-  StringRef FilePath = F->tryGetRealPathName();
+  SmallString<64> FilePath = F->tryGetRealPathName();
   if (FilePath.empty())
 FilePath = F->getName();
-  L.uri.file = FilePath;
+  if (!llvm::sys::path::is_absolute(FilePath)) {
+if (!SourceMgr.getFileManager().makeAbsolutePath(FilePath)) {
+  log("Could not turn relative path to absolute: " + FilePath);
+  return llvm::None;
+}
+  }
+
+  L.uri.file = FilePath.str();
   L.range = R;
   return L;
 }

Modified: clang-tools-extra/trunk/unittests/clangd/TestFS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TestFS.cpp?rev=325029=325028=325029=diff
==
--- clang-tools-extra/trunk/unittests/clangd/TestFS.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/TestFS.cpp Tue Feb 13 09:47:16 2018
@@ -33,8 +33,10 @@ MockFSProvider::getTaggedFileSystem(Path
   return make_tagged(FS, Tag);
 }
 
-MockCompilationDatabase::MockCompilationDatabase()
-: ExtraClangFlags({"-ffreestanding"}) {} // Avoid implicit stdc-predef.h.
+MockCompilationDatabase::MockCompilationDatabase(bool UseRelPaths)
+: ExtraClangFlags({"-ffreestanding"}), UseRelPaths(UseRelPaths) {
+  // -ffreestanding avoids implicit stdc-predef.h.
+}
 
 llvm::Optional
 MockCompilationDatabase::getCompileCommand(PathRef File) const {
@@ -42,10 +44,10 @@ MockCompilationDatabase::getCompileComma
 return llvm::None;
 
   auto CommandLine = ExtraClangFlags;
+  auto FileName = llvm::sys::path::filename(File);
   CommandLine.insert(CommandLine.begin(), "clang");
-  CommandLine.insert(CommandLine.end(), File.str());
-  return {tooling::CompileCommand(llvm::sys::path::parent_path(File),
-  llvm::sys::path::filename(File),
+  CommandLine.insert(CommandLine.end(), UseRelPaths ? FileName : File);
+  return {tooling::CompileCommand(llvm::sys::path::parent_path(File), FileName,
   std::move(CommandLine), "")};
 }
 

Modified: clang-tools-extra/trunk/unittests/clangd/TestFS.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TestFS.h?rev=325029=325028=325029=diff
==
--- clang-tools-extra/trunk/unittests/clangd/TestFS.h (original)
+++ clang-tools-extra/trunk/unittests/clangd/TestFS.h Tue Feb 13 09:47:16 2018
@@ -37,12 +37,15 @@ public:
 // A Compilation database that returns a fixed set of compile flags.
 class MockCompilationDatabase : public GlobalCompilationDatabase {
 public:
-  MockCompilationDatabase();
+  /// When \p UseRelPaths is true, uses relative paths in compile commands.
+  /// When \p UseRelPaths is false, uses absoulte paths.
+  MockCompilationDatabase(bool UseRelPaths = false);
 
   llvm::Optional
   getCompileCommand(PathRef File) const override;
 
   std::vector ExtraClangFlags;
+  const bool UseRelPaths;
 };
 
 // Returns an absolute (fake) test directory for this OS.

Modified: clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp?rev=325029=325028=325029=diff
==
--- clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp Tue Feb 13 09:47:16 
2018
@@ -8,6 +8,7 @@
 
//===--===//
 #include "Annotations.h"
 #include "ClangdUnit.h"
+#include "Compiler.h"
 #include "Matchers.h"
 #include "TestFS.h"
 #include "XRefs.h"
@@ -37,6 +38,11

[libcxx] r325028 - Make the ctype_byname::widen test cases pass on FreeBSD.

2018-02-13 Thread Dimitry Andric via cfe-commits

Author: dim
Date: Tue Feb 13 09:43:24 2018
New Revision: 325028

URL: http://llvm.org/viewvc/llvm-project?rev=325028=rev
Log:
Make the ctype_byname::widen test cases pass on FreeBSD.

Modified:

libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/widen_1.pass.cpp

libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/widen_many.pass.cpp

Modified: 
libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/widen_1.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/widen_1.pass.cpp?rev=325028=325027=325028=diff
==
--- 
libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/widen_1.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/widen_1.pass.cpp
 Tue Feb 13 09:43:24 2018
@@ -55,7 +55,7 @@ int main()
 assert(f.widen('.') == L'.');
 assert(f.widen('a') == L'a');
 assert(f.widen('1') == L'1');
-#ifdef __APPLE__
+#if defined(__APPLE__) || defined(__FreeBSD__)
 assert(f.widen(char(-5)) == L'\u00fb');
 #else
 assert(f.widen(char(-5)) == wchar_t(-1));

Modified: 
libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/widen_many.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/widen_many.pass.cpp?rev=325028=325027=325028=diff
==
--- 
libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/widen_many.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/widen_many.pass.cpp
 Tue Feb 13 09:43:24 2018
@@ -61,7 +61,7 @@ int main()
 assert(v[3] == L'.');
 assert(v[4] == L'a');
 assert(v[5] == L'1');
-#ifdef __APPLE__
+#if defined(__APPLE__) || defined(__FreeBSD__)
 assert(v[6] == L'\x85');
 #else
 assert(v[6] == wchar_t(-1));


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

[PATCH] D43209: Put type attributes after class keyword

2018-02-13 Thread Dimitry Andric via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rCXX325027: Put type attributes after class keyword (authored 
by dim, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D43209?vs=133932=134065#toc

Repository:
  rCXX libc++

https://reviews.llvm.org/D43209

Files:
  include/functional


Index: include/functional
===
--- include/functional
+++ include/functional
@@ -2497,8 +2497,7 @@
 
 // default searcher
 template>
-_LIBCPP_TYPE_VIS
-class default_searcher {
+class _LIBCPP_TYPE_VIS default_searcher {
 public:
 _LIBCPP_INLINE_VISIBILITY
 default_searcher(_ForwardIterator __f, _ForwardIterator __l, 


Index: include/functional
===
--- include/functional
+++ include/functional
@@ -2497,8 +2497,7 @@
 
 // default searcher
 template>
-_LIBCPP_TYPE_VIS
-class default_searcher {
+class _LIBCPP_TYPE_VIS default_searcher {
 public:
 _LIBCPP_INLINE_VISIBILITY
 default_searcher(_ForwardIterator __f, _ForwardIterator __l, 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r325027 - Put type attributes after class keyword

2018-02-13 Thread Dimitry Andric via cfe-commits

Author: dim
Date: Tue Feb 13 09:40:59 2018
New Revision: 325027

URL: http://llvm.org/viewvc/llvm-project?rev=325027=rev
Log:
Put type attributes after class keyword

Summary:
Compiling `` in C++17 or higher mode results in:

```
functional:2500:1: warning: attribute '__visibility__' is ignored, place it 
after "class" to apply attribute to type declaration [-Wignored-attributes]
_LIBCPP_TYPE_VIS
^
__config:701:46: note: expanded from macro '_LIBCPP_TYPE_VIS'
#define _LIBCPP_TYPE_VIS __attribute__ ((__visibility__("default")))
 ^
1 warning generated.
```

Fix it by putting the attribute after the `class` keyword.

Reviewers: EricWF, mclow.lists

Reviewed By: EricWF

Subscribers: cfe-commits

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

Modified:
libcxx/trunk/include/functional

Modified: libcxx/trunk/include/functional
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/functional?rev=325027=325026=325027=diff
==
--- libcxx/trunk/include/functional (original)
+++ libcxx/trunk/include/functional Tue Feb 13 09:40:59 2018
@@ -2497,8 +2497,7 @@ __search(_RandomAccessIterator1 __first1
 
 // default searcher
 template>
-_LIBCPP_TYPE_VIS
-class default_searcher {
+class _LIBCPP_TYPE_VIS default_searcher {
 public:
 _LIBCPP_INLINE_VISIBILITY
 default_searcher(_ForwardIterator __f, _ForwardIterator __l, 


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

[clang-tools-extra] r325024 - [clangd] Log if CWD could not be changed. NFC.

2018-02-13 Thread Ilya Biryukov via cfe-commits

Author: ibiryukov
Date: Tue Feb 13 09:15:06 2018
New Revision: 325024

URL: http://llvm.org/viewvc/llvm-project?rev=325024=rev
Log:
[clangd] Log if CWD could not be changed. NFC.

Modified:
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/CodeComplete.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=325024=325023=325024=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Tue Feb 13 09:15:06 2018
@@ -379,7 +379,11 @@ CppFile::rebuild(ParseInputs &) {
   for (const auto  : Inputs.CompileCommand.CommandLine)
 ArgStrs.push_back(S.c_str());
 
-  Inputs.FS->setCurrentWorkingDirectory(Inputs.CompileCommand.Directory);
+  if (Inputs.FS->setCurrentWorkingDirectory(Inputs.CompileCommand.Directory)) {
+log("Couldn't set working directory");
+// We run parsing anyway, our lit-tests rely on results for non-existing
+// working dirs.
+  }
 
   // Prepare CompilerInvocation.
   std::unique_ptr CI;

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=325024=325023=325024=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Tue Feb 13 09:15:06 2018
@@ -642,7 +642,11 @@ bool semaCodeComplete(std::unique_ptrsetCurrentWorkingDirectory(Input.Command.Directory);
+  if (Input.VFS->setCurrentWorkingDirectory(Input.Command.Directory)) {
+log("Couldn't set working directory");
+// We run parsing anyway, our lit-tests rely on results for non-existing
+// working dirs.
+  }
 
   IgnoreDiagnostics DummyDiagsConsumer;
   auto CI = createInvocationFromCommandLine(


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

[libcxxabi] r325022 - [demangler] Rewrite parse_nested_name in the new style.

2018-02-13 Thread Erik Pilkington via cfe-commits

Author: epilk
Date: Tue Feb 13 09:09:03 2018
New Revision: 325022

URL: http://llvm.org/viewvc/llvm-project?rev=325022=rev
Log:
[demangler] Rewrite parse_nested_name in the new style.

Modified:
libcxxabi/trunk/src/cxa_demangle.cpp

Modified: libcxxabi/trunk/src/cxa_demangle.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/src/cxa_demangle.cpp?rev=325022=325021=325022=diff
==
--- libcxxabi/trunk/src/cxa_demangle.cpp (original)
+++ libcxxabi/trunk/src/cxa_demangle.cpp Tue Feb 13 09:09:03 2018
@@ -1983,12 +1983,28 @@ struct Db {
   FunctionRefQual RefQuals = FrefQualNone;
   unsigned EncodingDepth = 0;
   bool ParsedCtorDtorCV = false;
+  bool EndsWithTemplateArgs = false;
   bool TagTemplates = true;
   bool FixForwardReferences = false;
   bool TryToParseTemplateArgs = true;
 
   BumpPointerAllocator ASTAllocator;
 
+  // A couple of members of Db are local to a specific name. When recursively
+  // parsing names we need to swap and restore them all.
+  struct SwapAndRestoreNameState {
+SwapAndRestore SaveQualifiers;
+SwapAndRestore SaveRefQualifiers;
+SwapAndRestore SaveEndsWithTemplateArgs;
+SwapAndRestore SaveParsedCtorDtorCV;
+
+SwapAndRestoreNameState(Db )
+: SaveQualifiers(Parser.CV, QualNone),
+  SaveRefQualifiers(Parser.RefQuals, FrefQualNone),
+  SaveEndsWithTemplateArgs(Parser.EndsWithTemplateArgs, false),
+  SaveParsedCtorDtorCV(Parser.ParsedCtorDtorCV, false) {}
+  };
+
   template  T *make(Args &&... args) {
 return new (ASTAllocator.allocate(sizeof(T)))
 T(std::forward(args)...);
@@ -2063,6 +2079,10 @@ struct Db {
   Node *parseClassEnumType();
   Node *parseQualifiedType(bool );
 
+  Node *parseNestedName();
+  Node *parseCtorDtorName(Node *);
+  Node *parseAbiTags(Node *N);
+
   // FIXME: remove this when all the parse_* functions have been rewritten.
   template 
   Node *legacyParse() {
@@ -2090,6 +2110,19 @@ struct Db {
   }
 };
 
+const char *parse_nested_name(const char *first, const char *last, Db ,
+  bool *endsWithTemplateArgs) {
+  db.First = first;
+  db.Last = last;
+  Node *R = db.parseNestedName();
+  if (endsWithTemplateArgs)
+*endsWithTemplateArgs = db.EndsWithTemplateArgs;
+  if (R == nullptr)
+return first;
+  db.Names.push_back(R);
+  return db.First;
+}
+
 const char *parse_expression(const char *first, const char *last, Db ) {
   db.First = first;
   db.Last = last;
@@ -2143,6 +2176,176 @@ const char *parse_decltype(const char *f
 const char *parse_unresolved_name(const char *, const char *, Db &);
 const char *parse_substitution(const char *, const char *, Db &);
 
+
+//  ::= C1  # complete object constructor
+//  ::= C2  # base object constructor
+//  ::= C3  # complete object allocating constructor
+//   extension  ::= C5# ?
+//  ::= D0  # deleting destructor
+//  ::= D1  # complete object destructor
+//  ::= D2  # base object destructor
+//   extension  ::= D5# ?
+Node *Db::parseCtorDtorName(Node *) {
+  if (SoFar->K == Node::KSpecialSubstitution) {
+auto SSK = static_cast(SoFar)->SSK;
+switch (SSK) {
+case SpecialSubKind::string:
+case SpecialSubKind::istream:
+case SpecialSubKind::ostream:
+case SpecialSubKind::iostream:
+  SoFar = make(SSK);
+default:
+  break;
+}
+  }
+
+  if (consumeIf('C')) {
+if (look() != '1' && look() != '2' && look() != '3' && look() != '5')
+  return nullptr;
+++First;
+ParsedCtorDtorCV = true;
+return make(SoFar, false);
+  }
+
+  if (look() == 'D' &&
+  (look(1) == '0' || look(1) == '1' || look(1) == '2' || look(1) == '5')) {
+First += 2;
+ParsedCtorDtorCV = true;
+return make(SoFar, true);
+  }
+
+  return nullptr;
+}
+
+//  ::= N [] []  
 E
+//   ::= N [] []  
 E
+//
+//  ::=  
+//  ::=  
+//  ::= 
+//  ::= 
+//  ::= # empty
+//  ::= 
+//  ::=  
+//  extension ::= L
+//
+//  ::=  
+//   ::= 
+//   ::= 
+Node *Db::parseNestedName() {
+  if (!consumeIf('N'))
+return nullptr;
+
+  CV = parseCVQualifiers();
+  if  (consumeIf('O')) RefQuals = FrefQualRValue;
+  else if (consumeIf('R')) RefQuals = FrefQualLValue;
+  else RefQuals = FrefQualNone;
+
+  Node *SoFar = nullptr;
+  auto PushComponent = [&](Node *Comp) {
+if (SoFar) SoFar = make(SoFar, Comp);
+else   SoFar = Comp;
+EndsWithTemplateArgs = false;
+  };
+
+  if (consumeIf("St"))
+SoFar = make("std");
+
+  while (!consumeIf('E')) {
+consumeIf('L'); // extension
+
+//  ::= 
+if (look() == 'T') {
+  Node *TP = legacyParse();
+  if (TP == nullptr)
+return nullptr;
+  PushComponent(TP);
+  Subs.push_back(SoFar);
+  continue;
+}
+
+//

[libcxxabi] r325023 - [demangler] Support for inheriting constructors.

2018-02-13 Thread Erik Pilkington via cfe-commits

Author: epilk
Date: Tue Feb 13 09:09:07 2018
New Revision: 325023

URL: http://llvm.org/viewvc/llvm-project?rev=325023=rev
Log:
[demangler] Support for inheriting constructors.

Fixes PR33223.

Modified:
libcxxabi/trunk/src/cxa_demangle.cpp
libcxxabi/trunk/test/test_demangle.pass.cpp

Modified: libcxxabi/trunk/src/cxa_demangle.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/src/cxa_demangle.cpp?rev=325023=325022=325023=diff
==
--- libcxxabi/trunk/src/cxa_demangle.cpp (original)
+++ libcxxabi/trunk/src/cxa_demangle.cpp Tue Feb 13 09:09:07 2018
@@ -2200,10 +2200,15 @@ Node *Db::parseCtorDtorName(Node *
   }
 
   if (consumeIf('C')) {
+bool IsInherited = consumeIf('I');
 if (look() != '1' && look() != '2' && look() != '3' && look() != '5')
   return nullptr;
 ++First;
 ParsedCtorDtorCV = true;
+if (IsInherited) {
+  if (legacyParse() == nullptr)
+return nullptr;
+}
 return make(SoFar, false);
   }
 

Modified: libcxxabi/trunk/test/test_demangle.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/test/test_demangle.pass.cpp?rev=325023=325022=325023=diff
==
--- libcxxabi/trunk/test/test_demangle.pass.cpp (original)
+++ libcxxabi/trunk/test/test_demangle.pass.cpp Tue Feb 13 09:09:07 2018
@@ -29690,6 +29690,10 @@ const char* cases[][2] =
 // Designated init expressions
 {"_ZN15designated_init1fINS_1AEEEvDTtlT_di1adi1bdxLi3EdXLi1ELi4ELi9EEE", 
"void 
designated_init::f(decltype(designated_init::A{.a.b[3][1 
... 4] = 9}))"},
 {"_Z1fIXtl1Xdi1adi1bdxLi3ELi1", "f"},
+
+// Inheriting constructors:
+{"_ZN1BCI21AEi", "B::B(int)"},
+{"_ZN1DCI21CIiEET_", "D::D(int)"},
 };
 
 const unsigned N = sizeof(cases) / sizeof(cases[0]);


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

[clang-tools-extra] r325021 - [clangd] Remove the RealFS layer from test VFS. NFC.

2018-02-13 Thread Ilya Biryukov via cfe-commits

Author: ibiryukov
Date: Tue Feb 13 09:08:13 2018
New Revision: 325021

URL: http://llvm.org/viewvc/llvm-project?rev=325021=rev
Log:
[clangd] Remove the RealFS layer from test VFS. NFC.

It was required before because preambles could only be created on
disk. All tests use in-memory preambles now.

Modified:
clang-tools-extra/trunk/unittests/clangd/TestFS.cpp

Modified: clang-tools-extra/trunk/unittests/clangd/TestFS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TestFS.cpp?rev=325021=325020=325021=diff
==
--- clang-tools-extra/trunk/unittests/clangd/TestFS.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/TestFS.cpp Tue Feb 13 09:08:13 2018
@@ -12,112 +12,6 @@
 
 namespace clang {
 namespace clangd {
-namespace {
-
-/// An implementation of vfs::FileSystem that only allows access to
-/// files and folders inside a set of whitelisted directories.
-///
-/// FIXME(ibiryukov): should it also emulate access to parents of whitelisted
-/// directories with only whitelisted contents?
-class FilteredFileSystem : public vfs::FileSystem {
-public:
-  /// The paths inside \p WhitelistedDirs should be absolute
-  FilteredFileSystem(std::vector WhitelistedDirs,
- IntrusiveRefCntPtr InnerFS)
-  : WhitelistedDirs(std::move(WhitelistedDirs)), InnerFS(InnerFS) {
-assert(std::all_of(WhitelistedDirs.begin(), WhitelistedDirs.end(),
-   [](const std::string ) -> bool {
- return llvm::sys::path::is_absolute(Path);
-   }) &&
-   "Not all WhitelistedDirs are absolute");
-  }
-
-  virtual llvm::ErrorOr status(const Twine ) {
-if (!isInsideWhitelistedDir(Path))
-  return llvm::errc::no_such_file_or_directory;
-return InnerFS->status(Path);
-  }
-
-  virtual llvm::ErrorOr
-  openFileForRead(const Twine ) {
-if (!isInsideWhitelistedDir(Path))
-  return llvm::errc::no_such_file_or_directory;
-return InnerFS->openFileForRead(Path);
-  }
-
-  llvm::ErrorOr
-  getBufferForFile(const Twine , int64_t FileSize = -1,
-   bool RequiresNullTerminator = true,
-   bool IsVolatile = false) {
-if (!isInsideWhitelistedDir(Name))
-  return llvm::errc::no_such_file_or_directory;
-return InnerFS->getBufferForFile(Name, FileSize, RequiresNullTerminator,
- IsVolatile);
-  }
-
-  virtual vfs::directory_iterator dir_begin(const Twine ,
-std::error_code ) {
-if (!isInsideWhitelistedDir(Dir)) {
-  EC = llvm::errc::no_such_file_or_directory;
-  return vfs::directory_iterator();
-}
-return InnerFS->dir_begin(Dir, EC);
-  }
-
-  virtual std::error_code setCurrentWorkingDirectory(const Twine ) {
-return InnerFS->setCurrentWorkingDirectory(Path);
-  }
-
-  virtual llvm::ErrorOr getCurrentWorkingDirectory() const {
-return InnerFS->getCurrentWorkingDirectory();
-  }
-
-  bool exists(const Twine ) {
-if (!isInsideWhitelistedDir(Path))
-  return false;
-return InnerFS->exists(Path);
-  }
-
-  std::error_code makeAbsolute(SmallVectorImpl ) const {
-return InnerFS->makeAbsolute(Path);
-  }
-
-private:
-  bool isInsideWhitelistedDir(const Twine ) const {
-SmallString<128> Path;
-InputPath.toVector(Path);
-
-if (makeAbsolute(Path))
-  return false;
-
-for (const auto  : WhitelistedDirs) {
-  if (Path.startswith(Dir))
-return true;
-}
-return false;
-  }
-
-  std::vector WhitelistedDirs;
-  IntrusiveRefCntPtr InnerFS;
-};
-
-/// Create a vfs::FileSystem that has access only to temporary directories
-/// (obtained by calling system_temp_directory).
-IntrusiveRefCntPtr getTempOnlyFS() {
-  llvm::SmallString<128> TmpDir1;
-  llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/false, TmpDir1);
-  llvm::SmallString<128> TmpDir2;
-  llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/true, TmpDir2);
-
-  std::vector TmpDirs;
-  TmpDirs.push_back(TmpDir1.str());
-  if (TmpDir1 != TmpDir2)
-TmpDirs.push_back(TmpDir2.str());
-  return new FilteredFileSystem(std::move(TmpDirs), vfs::getRealFileSystem());
-}
-
-} // namespace
-
 IntrusiveRefCntPtr
 buildTestFS(llvm::StringMap const ) {
   IntrusiveRefCntPtr MemFS(
@@ -126,11 +20,7 @@ buildTestFS(llvm::StringMap
 MemFS->addFile(FileAndContents.first(), time_t(),
llvm::MemoryBuffer::getMemBuffer(FileAndContents.second,
 FileAndContents.first()));
-
-  auto OverlayFS = IntrusiveRefCntPtr(
-  new vfs::OverlayFileSystem(getTempOnlyFS()));
-  OverlayFS->pushOverlay(std::move(MemFS));
-  return OverlayFS;
+  return MemFS;
 }
 
 Tagged


___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Francois Ferrand via Phabricator via cfe-commits

Typz added a comment.

Hum, not sure I fully got your proposal. So you actually mean that clang-format 
you format like this (with 4-space indent for clarity):

  switch (x) {
  case 0:
  break;
  case 1: {
  foo();
  break;
  }
  case 2: {
  foo();
  } break;
  case 3:
  {
  foo();
  }
  bar();
  break;
  }

Is this right?

If so it does not really help me: I don't care so much how it is formatted, but 
I think the current way is way too error prone (and I cannot change the style 
to indent the case blocks themself). So i'll have to keep a patch in my fork :-(
Or maybe the behavior should be dependant on `IndentCaseLabels` (though this 
would change LLVM style formatting) ?


Repository:
  rC Clang

https://reviews.llvm.org/D43183



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

[PATCH] D43171: [AMDGPU] Change constant addr space to 4 for clang

2018-02-13 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl added inline comments.



Comment at: test/CodeGenOpenCL/address-spaces.cl:37
+// SPIR: i32 addrspace(2)* %arg
+// GIZ: i32 addrspace(4)* %arg
 void f__c(__constant int *arg) {}

t-tye wrote:
> Suggest using the same name across all the OpenCL tests as some are using 
> GIZ, some AMD and some AMDGCN. AMDGCN seems the clearer now that only have a 
> single address space mapping? Similar comment for other places GIZ/AMD is 
> being mentioned.
> 
> 
Will have a separate patch to cleanup the tests.



Comment at: test/CodeGenOpenCL/amdgpu-env-amdgiz.cl:4
 
-// CHECK: target datalayout = 
"e-p:64:64-p1:64:64-p2:64:64-p3:32:32-p4:32:32-p5:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-A5"
+// CHECK: target datalayout = 
"e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-A5"
 void foo(void) {}

t-tye wrote:
> Should this test be renamed to amdgpu-env-amdgcn.cl now there is a single 
> address space mapping (and delete any relating to the old mapping if they 
> exist)?
will rename it when committing.


https://reviews.llvm.org/D43171



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

[PATCH] D43240: [OpenCL] Fix __enqueue_block for block with captures

2018-02-13 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl created this revision.
yaxunl added reviewers: Anastasia, bader.

The following test case causes issue with codegen of __enqueue_block

  void (^block)(void) = ^{ callee(id, out); };
  
  enqueue_kernel(queue, 0, ndrange, block); 

Clang first does codegen for block expression in the first line and deletes its 
block info.
Clang then tries to do codegen for the same block expression again for the 
second line,
and fails because the block info is gone.

The fix is to do normal codegen for both lines. Introduce an API to OpenCL 
runtime to
record llvm block invoke function and llvm block literal emitted for each AST 
block
expression, and use the recorded information for generating the wrapper kernel.

The EmitBlockLiteral APIs are cleaned up to minimize changes to the normal 
codegen
of blocks.

Another minor issue is that some clean up AST expression is generated for block
with captures, which can be stripped by IgnoreImplicit.


https://reviews.llvm.org/D43240

Files:
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGOpenCLRuntime.cpp
  lib/CodeGen/CGOpenCLRuntime.h
  lib/CodeGen/CodeGenFunction.h
  test/CodeGenOpenCL/enqueue-block-with-captures.cl

Index: test/CodeGenOpenCL/enqueue-block-with-captures.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/enqueue-block-with-captures.cl
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -emit-llvm -O0 -o - -triple spir-unknown-unknown | FileCheck %s
+
+typedef struct {int a;} ndrange_t;
+
+void callee(int id, __global int* out) {
+  out[id] = id;
+}
+
+kernel void test(int id, __global int* out) {
+
+  void (^block)(void) = ^{ callee(id, out); };
+
+  queue_t queue;
+  ndrange_t ndrange;
+  // CHECK: call i32 @__enqueue_kernel_basic
+  enqueue_kernel(queue, 0, ndrange, block);
+}
+
+// CHECK: define internal spir_kernel void @__test_block_invoke_kernel(i8 addrspace(4)*)
+// CHECK:  call void @__test_block_invoke(i8 addrspace(4)* %0)
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -1583,10 +1583,7 @@
   /// \return an LLVM value which is a pointer to a struct which contains
   /// information about the block, including the block invoke function, the
   /// captured variables, etc.
-  /// \param InvokeF will contain the block invoke function if it is not
-  /// nullptr.
-  llvm::Value *EmitBlockLiteral(const BlockExpr *,
-llvm::Function **InvokeF = nullptr);
+  llvm::Value *EmitBlockLiteral(const BlockExpr *);
   static void destroyBlockInfos(CGBlockInfo *info);
 
   llvm::Function *GenerateBlockFunction(GlobalDecl GD,
@@ -3010,11 +3007,8 @@
   LValue EmitOMPSharedLValue(const Expr *E);
 
 private:
-  /// Helpers for blocks. Returns invoke function by \p InvokeF if it is not
-  /// nullptr. It should be called without \p InvokeF if the caller does not
-  /// need invoke function to be returned.
-  llvm::Value *EmitBlockLiteral(const CGBlockInfo ,
-llvm::Function **InvokeF = nullptr);
+  /// Helpers for blocks.
+  llvm::Value *EmitBlockLiteral(const CGBlockInfo );
 
   /// struct with the values to be passed to the OpenMP loop-related functions
   struct OMPLoopArguments {
Index: lib/CodeGen/CGOpenCLRuntime.h
===
--- lib/CodeGen/CGOpenCLRuntime.h
+++ lib/CodeGen/CGOpenCLRuntime.h
@@ -23,6 +23,7 @@
 
 namespace clang {
 
+class BlockExpr;
 class Expr;
 class VarDecl;
 
@@ -39,8 +40,9 @@
 
   /// Structure for enqueued block information.
   struct EnqueuedBlockInfo {
-llvm::Function *Kernel; /// Enqueued block kernel.
-llvm::Value *BlockArg;  /// The first argument to enqueued block kernel.
+llvm::Function *InvokeFunc; /// Block invoke function.
+llvm::Function *Kernel; /// Enqueued block kernel.
+llvm::Value *BlockArg;  /// The first argument to enqueued block kernel.
   };
   /// Maps block expression to block information.
   llvm::DenseMap EnqueuedBlockMap;
@@ -76,6 +78,15 @@
   /// \return enqueued block information for enqueued block.
   EnqueuedBlockInfo emitOpenCLEnqueuedBlock(CodeGenFunction ,
 const Expr *E);
+
+  /// \brief Record invoke function and block literal emitted during normal
+  /// codegen for a block expression. The information is used by
+  /// emitOpenCLEnqueuedBlock to emit wrapper kernel.
+  ///
+  /// \param InvokeF invoke function emitted for the block expression.
+  /// \param Block block literal emitted for the block expression.
+  void recordBlockInfo(const BlockExpr *E, llvm::Function *InvokeF,
+   llvm::Value *Block);
 };
 
 }
Index: lib/CodeGen/CGOpenCLRuntime.cpp
===
--- lib/CodeGen/CGOpenCLRuntime.cpp
+++ lib/CodeGen/CGOpenCLRuntime.cpp
@@ -112,37 +112,50 @@

[PATCH] D43181: [CodeGen] Initialize large arrays by copying from a global

2018-02-13 Thread Ivan Kosarev via Phabricator via cfe-commits

kosarev updated this revision to Diff 134042.
kosarev added a comment.

Improved as suggested to cover all trivially-copyable types.


https://reviews.llvm.org/D43181

Files:
  lib/CodeGen/CGExprAgg.cpp
  test/CodeGen/init.c

Index: test/CodeGen/init.c
===
--- test/CodeGen/init.c
+++ test/CodeGen/init.c
@@ -8,8 +8,9 @@
 // CHECK-DAG: %struct.M = type { [2 x %struct.I] }
 // CHECK-DAG: %struct.I = type { [3 x i32] }
 
-// CHECK: [1 x %struct.M] [%struct.M { [2 x %struct.I] [%struct.I { [3 x i32] [i32 4, i32 4, i32 0] }, %struct.I { [3 x i32] [i32 4, i32 4, i32 5] }] }],
-// CHECK: [2 x [3 x i32]] {{[[][[]}}3 x i32] [i32 , i32 , i32 0], [3 x i32] [i32 , i32 , i32 ]],
+// CHECK-DAG: [1 x %struct.M] [%struct.M { [2 x %struct.I] [%struct.I { [3 x i32] [i32 4, i32 4, i32 0] }, %struct.I { [3 x i32] [i32 4, i32 4, i32 5] }] }],
+// CHECK-DAG: [2 x [3 x i32]] {{[[][[]}}3 x i32] [i32 , i32 , i32 0], [3 x i32] [i32 , i32 , i32 ]],
+// CHECK-DAG: [[INIT14:.*]] = private global [16 x i32] [i32 0, i32 0, i32 0, i32 0, i32 0, i32 17, i32 17, i32 17, i32 17, i32 17, i32 17, i32 17, i32 0, i32 0, i32 0, i32 0], align 4
 
 void f1() {
   // Scalars in braces.
@@ -33,8 +34,8 @@
 }
 
 // Constants
-// CHECK: @g3 = constant i32 10
-// CHECK: @f4.g4 = internal constant i32 12
+// CHECK-DAG: @g3 = constant i32 10
+// CHECK-DAG: @f4.g4 = internal constant i32 12
 const int g3 = 10;
 int f4() {
   static const int g4 = 12;
@@ -61,7 +62,7 @@
 
 
 
-// CHECK: @test7 = global{{.*}}{ i32 0, [4 x i8] c"bar\00" }
+// CHECK-DAG: @test7 = global{{.*}}{ i32 0, [4 x i8] c"bar\00" }
 // PR8217
 struct a7 {
   int  b;
@@ -151,3 +152,15 @@
   // CHECK: memcpy{{.*}}getelementptr inbounds ([3 x i8], [3 x i8]* @
   bar((char[3]) {""});
 }
+
+// Test that we initialize large member arrays by copying from a global and not
+// with a series of stores.
+struct S14 { int a[16]; };
+
+void test14(struct S14 *s14) {
+// CHECK-LABEL: @test14
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 {{.*}}, i8* align 4 {{.*}} [[INIT14]] {{.*}}, i32 64, i1 false)
+// CHECK-NOT: store
+// CHECK: ret void
+  *s14 = (struct S14) { { [5 ... 11] = 17 } };
+}
Index: lib/CodeGen/CGExprAgg.cpp
===
--- lib/CodeGen/CGExprAgg.cpp
+++ lib/CodeGen/CGExprAgg.cpp
@@ -14,6 +14,7 @@
 #include "CodeGenFunction.h"
 #include "CGObjCRuntime.h"
 #include "CodeGenModule.h"
+#include "ConstantEmitter.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
@@ -85,7 +86,7 @@
   void EmitMoveFromReturnSlot(const Expr *E, RValue Src);
 
   void EmitArrayInit(Address DestPtr, llvm::ArrayType *AType,
- QualType elementType, InitListExpr *E);
+ QualType ArrayQTy, InitListExpr *E);
 
   AggValueSlot::NeedsGCBarriers_t needsGC(QualType T) {
 if (CGF.getLangOpts().getGC() && TypeRequiresGCollection(T))
@@ -394,12 +395,15 @@
 
 /// \brief Emit initialization of an array from an initializer list.
 void AggExprEmitter::EmitArrayInit(Address DestPtr, llvm::ArrayType *AType,
-   QualType elementType, InitListExpr *E) {
+   QualType ArrayQTy, InitListExpr *E) {
   uint64_t NumInitElements = E->getNumInits();
 
   uint64_t NumArrayElements = AType->getNumElements();
   assert(NumInitElements <= NumArrayElements);
 
+  QualType elementType =
+  CGF.getContext().getAsArrayType(ArrayQTy)->getElementType();
+
   // DestPtr is an array*.  Construct an elementType* by drilling
   // down a level.
   llvm::Value *zero = llvm::ConstantInt::get(CGF.SizeTy, 0);
@@ -411,6 +415,29 @@
   CharUnits elementAlign =
 DestPtr.getAlignment().alignmentOfArrayElement(elementSize);
 
+  // Consider initializing the array by copying from a global. For this to be
+  // more efficient than per-element initialization, the number of the elements
+  // with explicit initializers should be large enough.
+  if (NumInitElements > 8 &&
+  elementType.isTriviallyCopyableType(CGF.getContext())) {
+CodeGen::CodeGenModule  = CGF.CGM;
+ConstantEmitter Emitter(CGM);
+LangAS AS = ArrayQTy.getAddressSpace();
+if (llvm::Constant *C = Emitter.tryEmitForInitializer(E, AS, ArrayQTy)) {
+  auto GV = new llvm::GlobalVariable(
+  CGM.getModule(), C->getType(),
+  CGM.isTypeConstant(ArrayQTy, /* ExcludeCtorDtor= */ true),
+  llvm::GlobalValue::PrivateLinkage, C, "constinit",
+  /* InsertBefore= */ nullptr, llvm::GlobalVariable::NotThreadLocal,
+  CGM.getContext().getTargetAddressSpace(AS));
+  Emitter.finalize(GV);
+  CharUnits Align = CGM.getContext().getTypeAlignInChars(ArrayQTy);
+  GV->setAlignment(Align.getQuantity());
+  EmitFinalDestCopy(ArrayQTy, CGF.MakeAddrLValue(GV, ArrayQTy, Align));
+  return;
+}
+  }
+
   //

[PATCH] D43187: [AST] Refine the condition for element-dependent array fillers

2018-02-13 Thread Ivan Kosarev via Phabricator via cfe-commits

kosarev added a comment.

John, maybe you can review this or suggest some other reviewers? Thanks.


Repository:
  rL LLVM

https://reviews.llvm.org/D43187



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

[PATCH] D43181: [CodeGen] Initialize large arrays by copying from a global

2018-02-13 Thread Ivan Kosarev via Phabricator via cfe-commits

kosarev added inline comments.



Comment at: lib/CodeGen/CGExprAgg.cpp:421
+  // with explicit initializers should be large enough.
+  if (NumInitElements > 8 && elementType->isBuiltinType()) {
+CodeGen::CodeGenModule  = CGF.CGM;

rjmccall wrote:
> Is there a good reason to use an element-count heuristic instead of a 
> total-size heuristic here?
> 
> Why only builtin types?  That seems to pointlessly rule out nested arrays, 
> complex types, vectors, C structs, and so on.  I think the predicate you 
> probably want here is isTriviallyCopyableType.
> Is there a good reason to use an element-count heuristic instead of a 
> total-size heuristic here?

Yes, the code below generates per-element initialization only for explicitly 
specified initializers. The rest, if any, is initialized with a filler, so it 
doesn't affect the size of the resulting code much.


Repository:
  rL LLVM

https://reviews.llvm.org/D43181



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

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Daniel Jasper via Phabricator via cfe-commits

djasper added a comment.

In https://reviews.llvm.org/D43183#1006224, @Typz wrote:

> It is explicitly documented in google style guide: 
> https://google.github.io/styleguide/cppguide.html#Loops_and_Switch_Statements 
> :
>
> > case blocks in switch statements can have curly braces or not, depending on 
> > your preference. If you do include curly braces they should be placed as 
> > shown below.
> > 
> > If not conditional on an enumerated value, switch statements should always 
> > have a default case (in the case of an enumerated value, the compiler will 
> > warn you if any values are not handled). If the default case should never 
> > execute, simply assert:
> > 
> >   switch (var) {
> > case 0: {  // 2 space indent
> >   ...  // 4 space indent
> >   break;
> > }
> > case 1: {
> >   ...
> >   break;
> > }
> > default: {
> >   assert(false);
> > }
> >   }
>
> So IMHO we cannot just change the current (or default) behaviour.

My proposal does not contradict this style guide as the case in question is not 
included in the example. It gives absolutely no guidance on how to format this 
case.

If anything, you could argue that it tells the user never to have something 
outside of the braces that make up a case statement (keep in mind that the 
style guide does not only give formatting advise).

Repository:
  rC Clang

https://reviews.llvm.org/D43183

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

[PATCH] D43223: [clang-tidy] Update fuchsia-multiple-inheritance to not fail

2018-02-13 Thread Julie Hockett via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE325015: [clang-tidy] Update fuchsia-multiple-inheritance 
to not fail (authored by juliehockett, committed by ).

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43223

Files:
  clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
  test/clang-tidy/fuchsia-multiple-inheritance.cpp


Index: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
===
--- clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -66,7 +66,7 @@
   for (const auto  : Node->bases()) {
 if (I.isVirtual()) continue;
 const auto *Ty = I.getType()->getAs();
-assert(Ty && "RecordType of base class is unknown");
+if (!Ty) continue;
 const RecordDecl *D = Ty->getDecl()->getDefinition();
 if (!D) continue;
 const auto *Base = cast(D);
@@ -96,18 +96,18 @@
 // concrete classes
 unsigned NumConcrete = 0;
 for (const auto  : D->bases()) {
-  if (I.isVirtual() || I.getType()->getAs()) 
continue;
+  if (I.isVirtual()) continue;
   const auto *Ty = I.getType()->getAs();
-  assert(Ty && "RecordType of base class is unknown");
+  if (!Ty) continue;
   const auto *Base = cast(Ty->getDecl()->getDefinition());
   if (!isInterface(Base)) NumConcrete++;
 }
 
 // Check virtual bases to see if there is more than one concrete 
 // non-virtual base.
 for (const auto  : D->vbases()) {
   const auto *Ty = V.getType()->getAs();
-  assert(Ty && "RecordType of base class is unknown");
+  if (!Ty) continue;
   const auto *Base = cast(Ty->getDecl()->getDefinition());
   if (!isInterface(Base)) NumConcrete++;
 }
Index: test/clang-tidy/fuchsia-multiple-inheritance.cpp
===
--- test/clang-tidy/fuchsia-multiple-inheritance.cpp
+++ test/clang-tidy/fuchsia-multiple-inheritance.cpp
@@ -131,3 +131,6 @@
 
 template struct A : T {};
 template struct B : virtual T {};
+
+template struct C {};
+template struct D : C {};


Index: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
===
--- clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -66,7 +66,7 @@
   for (const auto  : Node->bases()) {
 if (I.isVirtual()) continue;
 const auto *Ty = I.getType()->getAs();
-assert(Ty && "RecordType of base class is unknown");
+if (!Ty) continue;
 const RecordDecl *D = Ty->getDecl()->getDefinition();
 if (!D) continue;
 const auto *Base = cast(D);
@@ -96,18 +96,18 @@
 // concrete classes
 unsigned NumConcrete = 0;
 for (const auto  : D->bases()) {
-  if (I.isVirtual() || I.getType()->getAs()) continue;
+  if (I.isVirtual()) continue;
   const auto *Ty = I.getType()->getAs();
-  assert(Ty && "RecordType of base class is unknown");
+  if (!Ty) continue;
   const auto *Base = cast(Ty->getDecl()->getDefinition());
   if (!isInterface(Base)) NumConcrete++;
 }
 
 // Check virtual bases to see if there is more than one concrete 
 // non-virtual base.
 for (const auto  : D->vbases()) {
   const auto *Ty = V.getType()->getAs();
-  assert(Ty && "RecordType of base class is unknown");
+  if (!Ty) continue;
   const auto *Base = cast(Ty->getDecl()->getDefinition());
   if (!isInterface(Base)) NumConcrete++;
 }
Index: test/clang-tidy/fuchsia-multiple-inheritance.cpp
===
--- test/clang-tidy/fuchsia-multiple-inheritance.cpp
+++ test/clang-tidy/fuchsia-multiple-inheritance.cpp
@@ -131,3 +131,6 @@
 
 template struct A : T {};
 template struct B : virtual T {};
+
+template struct C {};
+template struct D : C {};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r325015 - [clang-tidy] Update fuchsia-multiple-inheritance to not fail

2018-02-13 Thread Julie Hockett via cfe-commits

Author: juliehockett
Date: Tue Feb 13 07:40:40 2018
New Revision: 325015

URL: http://llvm.org/viewvc/llvm-project?rev=325015=rev
Log:
[clang-tidy] Update fuchsia-multiple-inheritance to not fail

Updating the fuchsia-multiple-inheritance to gracefully handle unknown
record types (e.g. templatized classes) by simply continuing, rather
than asserting and failing.

Fixes PR36052.

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

Modified:
clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp?rev=325015=325014=325015=diff
==
--- clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp Tue 
Feb 13 07:40:40 2018
@@ -66,7 +66,7 @@ bool MultipleInheritanceCheck::isInterfa
   for (const auto  : Node->bases()) {
 if (I.isVirtual()) continue;
 const auto *Ty = I.getType()->getAs();
-assert(Ty && "RecordType of base class is unknown");
+if (!Ty) continue;
 const RecordDecl *D = Ty->getDecl()->getDefinition();
 if (!D) continue;
 const auto *Base = cast(D);
@@ -96,9 +96,9 @@ void MultipleInheritanceCheck::check(con
 // concrete classes
 unsigned NumConcrete = 0;
 for (const auto  : D->bases()) {
-  if (I.isVirtual() || I.getType()->getAs()) 
continue;
+  if (I.isVirtual()) continue;
   const auto *Ty = I.getType()->getAs();
-  assert(Ty && "RecordType of base class is unknown");
+  if (!Ty) continue;
   const auto *Base = cast(Ty->getDecl()->getDefinition());
   if (!isInterface(Base)) NumConcrete++;
 }
@@ -107,7 +107,7 @@ void MultipleInheritanceCheck::check(con
 // non-virtual base.
 for (const auto  : D->vbases()) {
   const auto *Ty = V.getType()->getAs();
-  assert(Ty && "RecordType of base class is unknown");
+  if (!Ty) continue;
   const auto *Base = cast(Ty->getDecl()->getDefinition());
   if (!isInterface(Base)) NumConcrete++;
 }

Modified: 
clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp?rev=325015=325014=325015=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp 
Tue Feb 13 07:40:40 2018
@@ -131,3 +131,6 @@ struct D8 : V13, V14 {};
 
 template struct A : T {};
 template struct B : virtual T {};
+
+template struct C {};
+template struct D : C {};


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

r325011 - An updated test to show the current warnings produced for implicit conversions from 'double' to 'float'.

2018-02-13 Thread Andrew V. Tischenko via cfe-commits

Author: avt77
Date: Tue Feb 13 07:20:29 2018
New Revision: 325011

URL: http://llvm.org/viewvc/llvm-project?rev=325011=rev
Log:
An updated test to show the current warnings produced for implicit conversions 
from 'double' to 'float'.

Modified:
cfe/trunk/test/Sema/conversion.c

Modified: cfe/trunk/test/Sema/conversion.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/conversion.c?rev=325011=325010=325011=diff
==
--- cfe/trunk/test/Sema/conversion.c (original)
+++ cfe/trunk/test/Sema/conversion.c Tue Feb 13 07:20:29 2018
@@ -429,3 +429,17 @@ void test27(ushort16 constants) {
 ushort16 crCbScale = pairedConstants.s4; // expected-warning {{implicit 
conversion loses integer precision: 'uint32_t' (aka 'unsigned int') to 
'ushort16'}}
 ushort16 brBias = pairedConstants.s6; // expected-warning {{implicit 
conversion loses integer precision: 'uint32_t' (aka 'unsigned int') to 
'ushort16'}}
 }
+
+
+float double2float_test1(double a) {
+return a; // expected-warning {{implicit conversion loses floating-point 
precision: 'double' to 'float'}}
+}
+
+void double2float_test2(double a, float *b) {
+*b += a;
+}
+
+float sinf (float x);
+double double2float_test3(double a) {
+return sinf(a); // expected-warning {{implicit conversion loses 
floating-point precision: 'double' to 'float'}}
+}


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

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Francois Ferrand via Phabricator via cfe-commits

Typz added a comment.

It is explicitly documented in google style guide: 
https://google.github.io/styleguide/cppguide.html#Loops_and_Switch_Statements :

> case blocks in switch statements can have curly braces or not, depending on 
> your preference. If you do include curly braces they should be placed as 
> shown below.
> 
> If not conditional on an enumerated value, switch statements should always 
> have a default case (in the case of an enumerated value, the compiler will 
> warn you if any values are not handled). If the default case should never 
> execute, simply assert:
> 
>   switch (var) {
> case 0: {  // 2 space indent
>   ...  // 4 space indent
>   break;
> }
> case 1: {
>   ...
>   break;
> }
> default: {
>   assert(false);
> }
>   }

So IMHO we cannot just change the current (or default) behaviour.


Repository:
  rC Clang

https://reviews.llvm.org/D43183



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

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Daniel Jasper via Phabricator via cfe-commits

djasper added a comment.

In https://reviews.llvm.org/D43183#1006170, @Typz wrote:

> > We'll just format those cases in a somewhat weird way and users can either 
> > accept that or change their code to not need it.
>
> I think we have a really diverging opinion on this. From my experience, 
> people will mostly accept the output of the formatter without question : 
> therefore I think we should strive to have the formatter format things as 
> "correctly" as possible. And looking at the existing options and various 
> complex formatting algorithms implemented I think this reasoning has been 
> applied to some extent :-)
>  So I personnally would be willing to add some code/options even for such 
> kind of corner cases; but then I am not the one maintaining the clang-format 
> project.


I have no problem with making clang-format format things "correctly" and I am 
happy to add code. But I think we are doing the average clang-format user a 
disservice if we provide options for every such corner case. Let's settle on 
one good-enough behavior and go with that for everything/everyone.

>> I don't particularly care which of these options we go with, but I would be 
>> really hesitant to introduce an style flag for it. This is so rare, that 
>> almost no one needs this flag (or should need this flag). Thus, the cost of 
>> the flag (discoverability of flags for users, increased maintenance cost, 
>> etc.) strongly outweigh the benefit.
> 
> Any change will still require a new flag somewhere, since the currently 
> implemented behavior is :
>  1- the documented way to format in Google style
>  2- the expected format in LLVM style, according to the clang-format unit test
> 
> It should thus probably not be changed.

I don't think that's true for the alternative I am suggesting.

>> IMO, for the same reason, this specific issue will not become part of the 
>> style guide of projects.
> 
> I definitely agree on this, but it is actually part of some styles (e.g. 
> Google's)
> 
>> If you want to change something, I'd vote for making clang fall back to this 
>> behavior:
>> 
>>   case A:
>> {
>>   stuff();
>> }
>> moreStuff();
>> break;
>>   }
>>
>> 
>> i.e. not let it put the "{" on the same line as the "case ..." if there is a 
>> trailing statement (other than "break;" maybe).
> 
> I am fine with that formatting (though I did not implement it since it 
> requires tweaking the code in more places, instead of essentially modifying a 
> single function like I did).

As we can implement this without additional flags (it doesn't contradict any 
style guide I know of), I think this would be strictly preferable.


Repository:
  rC Clang

https://reviews.llvm.org/D43183



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

[PATCH] D43231: [clang-format] Refactor ObjC tests

2018-02-13 Thread Jacek Olesiak via Phabricator via cfe-commits

jolesiak abandoned this revision.
jolesiak added a comment.

In https://reviews.llvm.org/D43231#1006123, @krasimir wrote:

> I don't believe this is needed: test fails before would fail at the line 
> where test instance is checked, and after they will fail at the checkLanguage 
> function.

Thank you for the comment!
That's a valid argument. I'm reverting as a macro is a no go.

Repository:
  rC Clang

https://reviews.llvm.org/D43231

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

[PATCH] D43108: Support for the mno-stack-arg-probe flag

2018-02-13 Thread Hans Wennborg via Phabricator via cfe-commits

hans added a comment.

In https://reviews.llvm.org/D43108#1005904, @nruslan wrote:

> @hans: One real-world example is when it is used to compile UEFI code using 
> PE/COFF targets natively. Obviously, UEFI uses ABI which is basically almost 
> the same as MS ABI, except that chkstk is not used. It mostly works (I 
> actually was able to get it running) except the cases when the code contains 
> variable-sized arrays allocated on stacks. Unfortunately, stack-probe-size 
> will only help with fixed sized array but will not help to solve the problem 
> described in the bug description since stack usage is unknown at compile 
> time. MinGW does not have this problem because it provides this flag.


I see, interesting. Might be worth mentioning in the commit message for others 
wondering what the flag is useful for.

> @MatzeB : there is a test on LLVM side (related review). Do you think the 
> test is needed for clang side? If so, please let me know, what kind of test 
> it is supposed to be.

Yes please, I think think there should be on in test/Driver/ to check that 
forwarding the flag to cc1 (and if we have a -mno-foo, there should maybe be a 
-mfoo variant too?), and a test in test/CodeGen/ to check that the attribute 
gets put on the functions correctly. Perhaps r321992 is a good example to look 
at.


Repository:
  rC Clang

https://reviews.llvm.org/D43108



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

[PATCH] D42623: [clang-tidy] Add a Lexer util to get the source text of a statement

2018-02-13 Thread Tristan Bourvon via Phabricator via cfe-commits

tbourvon abandoned this revision.
tbourvon added inline comments.



Comment at: clang-tidy/utils/LexerUtils.h:26
+/// Get source code text for statement.
+Optional getStmtText(const Stmt* Statement, const SourceManager& 
SM);
+

alexfh wrote:
> aaron.ballman wrote:
> > Formatting (elsewhere as well).
> > 
> > You should run the patch under clang-format to handle this sort of thing.
> Have you seen clang::tooling::fixit::getText? It should cover this use case.
Hadn't seen that! Seems to be making this whole patch obsolete, I'm closing it


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42623



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

[PATCH] D42624: [clang-tidy] Add a utility Matcher to match the next statement within a statement sequence

2018-02-13 Thread Tristan Bourvon via Phabricator via cfe-commits

tbourvon added inline comments.



Comment at: clang-tidy/utils/Matchers.h:16
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Analysis/CFG.h"
 

aaron.ballman wrote:
> This will require linking in the clangAnalysis library as well; are we sure 
> we want to take on this dependency here for all matchers?
Do you have a specific solution in mind? We could make the matcher local to the 
check it is being used in (see D37014), but I think it could prove useful for 
other checks...



Comment at: clang-tidy/utils/Matchers.h:58-60
+  // We get the first parent, making sure that we're not in a case statement
+  // not in a compound statement directly inside a switch, because this causes
+  // the buildCFG call to crash.

aaron.ballman wrote:
> Why does it cause the crash? Should that crash not be fixed instead of 
> applying this workaround?
I'm not entirely sure if this is expected behavior or not. In terms of AST, 
`switch` statements are a bit special in terms of how they are represented 
(each case contains all the subsequent cases as its children IIRC).
There probably is a way to make the CFG work in these cases, but I honestly 
don't have the time to look into that and attempt a fix. Couldn't this be good 
enough for now, maybe with a FIXME?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42624



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

[PATCH] D39571: [clangd] DidChangeConfiguration Notification

2018-02-13 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

In https://reviews.llvm.org/D39571#1005926, @malaperle wrote:

> I haven't looked at the newest patch yet but I gave it a quick try and saw 
> something odd. If I change the configuration to something invalid (say I 
> specify the path to a CMakeLists.txt), then I get many errors/diagnostics, 
> which is normal. But then when I change the config to something valid, the 
> same diagnostics re-emitted, as if the configuration failed to change. I'll 
> check the code tomorrow a bit but I thought I'd share this with you early.

Maybe you were still seeing diagnostics from the older versions of the files? 
clangd should **eventually** produce the right diagnostics, but if processing 
of some of the requests is in-flight at the time there results may still be 
reported.
BTW, I don't know if you've seen it before, but there's `-input-mirror-file` 
option to clangd that records the lsp input which can be used to rerun clangd 
later.

  # This should be run by VSCode (or Theia)
  clangd -input-mirror-file=/tmp/clangd.input

  # Later we can rerun from command line
  $ clangd < /tmp/clangd.input
  # Pass the flag to execute everyhing on one thread
  $ clangd -run-synchronously < /tmp/clangd.input

This is useful to debug/rerun and see the logs, etc.

This patch is good to go after remove `std::future<>`  from 
`reparseOpenedFiles`, unless @malaperle discovers there are other problems. 
Ideally, it'd be also nice to have a test for it.

Comment at: clangd/ClangdServer.cpp:541

+std::vector
+ClangdServer::reparseOpenedFiles() {

We're not returning futures from `forceReparse` anymore, this function has to 
be updated accordingly.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D39571

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

[PATCH] D43167: Fix incorrect indentation.

2018-02-13 Thread Marshall Clow via Phabricator via cfe-commits

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

This is fine.


Repository:
  rCXX libc++

https://reviews.llvm.org/D43167



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

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Francois Ferrand via Phabricator via cfe-commits

Typz added a comment.

> We'll just format those cases in a somewhat weird way and users can either 
> accept that or change their code to not need it.

I think we have a really diverging opinion on this. From my experience, people 
will mostly accept the output of the formatter without question : therefore I 
think we should strive to have the formatter format things as "correctly" as 
possible. And looking at the existing options and various complex formatting 
algorithms implemented I think this reasoning has been applied to some extent 
:-)
So I personnally would be willing to add some code/options even for such kind 
of corner cases; but then I am not the one maintaining the clang-format project.

> I don't particularly care which of these options we go with, but I would be 
> really hesitant to introduce an style flag for it. This is so rare, that 
> almost no one needs this flag (or should need this flag). Thus, the cost of 
> the flag (discoverability of flags for users, increased maintenance cost, 
> etc.) strongly outweigh the benefit.

Any change will still require a new flag somewhere, since the currently 
implemented behavior is :
1- the documented way to format in Google style
2- the expected format in LLVM style, according to the clang-format unit test

It should thus probably not be changed.

> IMO, for the same reason, this specific issue will not become part of the 
> style guide of projects.

I definitely agree on this, but it is actually part of some styles (e.g. 
Google's)

> If you want to change something, I'd vote for making clang fall back to this 
> behavior:
> 
>   case A:
> {
>   stuff();
> }
> moreStuff();
> break;
>   }
>
> 
> i.e. not let it put the "{" on the same line as the "case ..." if there is a 
> trailing statement (other than "break;" maybe).

I am fine with that formatting (though I did not implement it since it requires 
tweaking the code in more places, instead of essentially modifying a single 
function like I did).


Repository:
  rC Clang

https://reviews.llvm.org/D43183



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

[PATCH] D38639: [clangd] #include statements support for Open definition

2018-02-13 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added inline comments.

Comment at: clangd/ClangdUnit.cpp:103
+
+if (SourceMgr.isInMainFile(FilenameRange.getAsRange().getBegin())) {
+  // Only inclusion directives in the main file make sense. The user cannot

NIT: replace `FilenameRange.getAsRange()` with `SR`

Comment at: clangd/ClangdUnit.cpp:126
+
+  CppFilePreambleCallbacks() : SourceMgr(nullptr) {}
+

NIT: remove ctor, use initializer on the member instead:
```
private:
  SourceManager *SourceMgr = nullptr;
```

Comment at: clangd/ClangdUnit.cpp:160
+  SourceManager *SourceMgr;
+  InclusionLocations IncLocations;
 };

Maybe swap `IncLocations` and `SourceMgr`?  Grouping `TopLevelDecls`, 
`TopLevelDeclIDs` and `IncLocations` together seems reasonable, as all of them 
are essentially output parameters.

Comment at: clangd/ClangdUnit.cpp:296
   StoreDiagsConsumer UnitDiagsConsumer(/*ref*/ ASTDiags);
+  InclusionLocations IncLocations;
+  // Copy over the includes from the preamble, then combine with the

NIT: move `IncLocations` closer to their first use, i.e. create the variable 
right before `addPPCallbacks()`

Comment at: clangd/ClangdUnit.h:51

+using IncludeReferenceMap = std::unordered_map;
+

malaperle wrote:
> ilya-biryukov wrote:
> > We use `unordered_map` as a `vector>` here. (i.e. we never look up 
> > values by key, because we don't only the range, merely a point in the range)
> > Replace map with `vector>` and remove `RangeHash` that we don't need 
> > anymore.
> Done. I also renamed, IncludeReferenceMap  to InclusionLocations. I thought I 
> was more suitable.
LG

Comment at: clangd/ClangdUnit.h:105
+  const InclusionLocations () const {
+return IncLocations;
+  };

NIT: move to .cpp file to be consisten with other decls in this file.

Comment at: clangd/XRefs.cpp:171
+Range R = IncludeLoc.first;
+const SourceManager  = AST.getASTContext().getSourceManager();
+Position Pos = sourceLocToPosition(SourceMgr, SourceLocationBeg);

NIT: `SourceMgr` does not depend on the loop variable, declare it out of the 
loop.

Comment at: clangd/XRefs.cpp:174
+
+if (R.start.line == Pos.line && R.start.character <= Pos.character &&
+Pos.character <= R.end.character) {

NIT: define `operator <=` for `Position` and do this instead: `R.start <= Pos 
&& Pos < R.end` (note that LSP ranges are half-open, i.e. the end is excluded).
Even better: define a `bool contains(Position Pos) const` helper in the `Range` 
class and use it here (will abstract away to half-open nature of the range)

Comment at: clangd/XRefs.cpp:178
+Range{Position{0, 0}, Position{0, 
0}}});
+  return IncludeTargets;
+}

Let's follow the pattern of decls and macros here. I.e. not return from the 
function, merely collect all the results.

Comment at: unittests/clangd/XRefsTests.cpp:53

+class IgnoreDiagnostics : public DiagnosticsConsumer {
+  void onDiagnosticsReady(

NIT: remove this class, use `IgnoreDiagnostics` from `Compiler.h` instead.

Comment at: unittests/clangd/XRefsTests.cpp:245
+  const char *SourceContents = R"cpp(
+  #include "$2^invalid.h"
+  #include "^foo.h"

Could we also add tests for corner cases: cursor before opening quote, cursor 
after the closing quote, cursor in the middle of `#include` token? (we 
shouldn't navigate anywhere in the middle of the #include token)

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D38639

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

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Daniel Jasper via Phabricator via cfe-commits

djasper added a comment.

To me none of these options make sense. For the case you are describing, I 
agree that the current behavior is not ideal, but neither are any of the 
alternatives. However, I think that's fine. We'll just format those cases in a 
somewhat weird way and users can either accept that or change their code to not 
need it. I don't particularly care which of these options we go with, but I 
would be really hesitant to introduce an style flag for it. This is so rare, 
that almost no one needs this flag (or should need this flag). Thus, the cost 
of the flag (discoverability of flags for users, increased maintenance cost, 
etc.) strongly outweigh the benefit. IMO, for the same reason, this specific 
issue will not become part of the style guide of projects.

If you want to change something, I'd vote for making clang fall back to this 
behavior:

  case A:
{
  stuff();
}
moreStuff();
break;
  }

i.e. not let it put the "{" on the same line as the "case ..." if there is a 
trailing statement (other than "break;" maybe).


Repository:
  rC Clang

https://reviews.llvm.org/D43183



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

1 2 >

1 - 100 of 137 matches

Mail list logo