[PATCH] D37538: [libc++] Remove problematic ADL in container implementations.

[David L. Jones via Phabricator via cfe-commits]

dlj updated this revision to Diff 115165.
dlj added a comment.

- Remove deque from the test for now.


https://reviews.llvm.org/D37538

Files:
  include/__split_buffer
  include/memory
  test/std/containers/containers.general/construct_destruct.pass.cpp

Index: test/std/containers/containers.general/construct_destruct.pass.cpp
===
--- /dev/null
+++ test/std/containers/containers.general/construct_destruct.pass.cpp
@@ -0,0 +1,128 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+// Holder is a do-nothing wrapper around a value of the given type.
+//
+// Since unqualified name lookup and operator overload lookup participate in
+// ADL, the dependent type ns::T contributes to the overload set considered by
+// ADL. In other words, ADL will consider all associated namespaces: this
+// includes not only N, but also any inline friend functions declared by ns::T.
+//
+// The purpose of this test is to ensure that simply constructing or destroying
+// a container does not invoke ADL which would be problematic for unknown types.
+// By using the type 'Holder *' as a container value, we can avoid the
+// obvious problems with unknown types: a pointer is trivial, and its size is
+// known. However, any ADL which includes ns::T as an associated namesapce will
+// fail.
+//
+// For example:
+//
+//   namespace ns {
+// class T {
+//   // 13.5.7 [over.inc]:
+//   friend std::list::const_iterator
+//   operator++(std::list::const_iterator it) {
+// return /* ... */;
+//   }
+// };
+//   }
+//
+// The C++ standard stipulates that some functions, such as std::advance, use
+// overloaded operators (in C++14: 24.4.4 [iterator.operations]). The
+// implication is that calls to such a function are dependent on knowing the
+// overload set of operators in all associated namespaces; under ADL, this
+// includes the private friend function in the example above.
+//
+// However, for some operations, such as construction and destruction, no such
+// ADL is required. This can be important, for example, when using the Pimpl
+// pattern:
+//
+//   // Defined in a header file:
+//   class InterfaceList {
+// // Defined in a .cpp file:
+// class Impl;
+// vector impls;
+//   public:
+// ~InterfaceList();
+//   };
+//
+template 
+class Holder { T value; };
+
+namespace ns { class Fwd; }
+
+// TestSequencePtr and TestMappingPtr ensure that a given container type can be
+// default-constructed and destroyed with an incomplete value pointer type.
+template  class Container>
+void TestSequencePtr() {
+  using X = Container;
+  {
+X u;
+assert(u.empty());
+  }
+  {
+auto u = X();
+assert(u.empty());
+  }
+}
+
+template  class Container>
+void TestMappingPtr() {
+  {
+using X = Container;
+X u1;
+assert(u1.empty());
+auto u2 = X();
+assert(u2.empty());
+  }
+  {
+using X = Container;
+X u1;
+assert(u1.empty());
+auto u2 = X();
+assert(u2.empty());
+  }
+}
+
+int main()
+{
+  // Sequence containers:
+  {
+std::array a;
+(void)a;
+  }
+  TestSequencePtr();
+  TestSequencePtr();
+  TestSequencePtr();
+
+  // Associative containers, non-mapping:
+  TestSequencePtr();
+  TestSequencePtr();
+
+  // Associative containers, mapping:
+  TestMappingPtr();
+  TestMappingPtr();
+
+  // Container adapters:
+  TestSequencePtr();
+  TestSequencePtr();
+}
Index: include/memory
===
--- include/memory
+++ include/memory
@@ -1346,9 +1346,9 @@
 struct __has_construct
 : integral_constant(),
-  declval<_Pointer>(),
-  declval<_Args>()...)),
+decltype(_VSTD::__has_construct_test(declval<_Alloc>(),
+ declval<_Pointer>(),
+ declval<_Args>()...)),
 true_type>::value>
 {
 };
@@ -1367,8 +1367,8 @@
 struct __has_destroy
 : integral_constant(),
-declval<_Pointer>())),
+decltype(_VSTD::__has_destroy_test(declval<_Alloc>(),
+   declval<_Pointer>())),
 

[PATCH] D37538: [libc++] Remove problematic ADL in container implementations.

[David L. Jones via Phabricator via cfe-commits]

dlj updated this revision to Diff 115164.
dlj added a comment.

- Remove deque from the test for now.


https://reviews.llvm.org/D37538

Files:
  include/__split_buffer
  include/deque
  include/memory
  test/std/containers/containers.general/construct_destruct.pass.cpp

Index: test/std/containers/containers.general/construct_destruct.pass.cpp
===
--- /dev/null
+++ test/std/containers/containers.general/construct_destruct.pass.cpp
@@ -0,0 +1,128 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+// Holder is a do-nothing wrapper around a value of the given type.
+//
+// Since unqualified name lookup and operator overload lookup participate in
+// ADL, the dependent type ns::T contributes to the overload set considered by
+// ADL. In other words, ADL will consider all associated namespaces: this
+// includes not only N, but also any inline friend functions declared by ns::T.
+//
+// The purpose of this test is to ensure that simply constructing or destroying
+// a container does not invoke ADL which would be problematic for unknown types.
+// By using the type 'Holder *' as a container value, we can avoid the
+// obvious problems with unknown types: a pointer is trivial, and its size is
+// known. However, any ADL which includes ns::T as an associated namesapce will
+// fail.
+//
+// For example:
+//
+//   namespace ns {
+// class T {
+//   // 13.5.7 [over.inc]:
+//   friend std::list::const_iterator
+//   operator++(std::list::const_iterator it) {
+// return /* ... */;
+//   }
+// };
+//   }
+//
+// The C++ standard stipulates that some functions, such as std::advance, use
+// overloaded operators (in C++14: 24.4.4 [iterator.operations]). The
+// implication is that calls to such a function are dependent on knowing the
+// overload set of operators in all associated namespaces; under ADL, this
+// includes the private friend function in the example above.
+//
+// However, for some operations, such as construction and destruction, no such
+// ADL is required. This can be important, for example, when using the Pimpl
+// pattern:
+//
+//   // Defined in a header file:
+//   class InterfaceList {
+// // Defined in a .cpp file:
+// class Impl;
+// vector impls;
+//   public:
+// ~InterfaceList();
+//   };
+//
+template 
+class Holder { T value; };
+
+namespace ns { class Fwd; }
+
+// TestSequencePtr and TestMappingPtr ensure that a given container type can be
+// default-constructed and destroyed with an incomplete value pointer type.
+template  class Container>
+void TestSequencePtr() {
+  using X = Container;
+  {
+X u;
+assert(u.empty());
+  }
+  {
+auto u = X();
+assert(u.empty());
+  }
+}
+
+template  class Container>
+void TestMappingPtr() {
+  {
+using X = Container;
+X u1;
+assert(u1.empty());
+auto u2 = X();
+assert(u2.empty());
+  }
+  {
+using X = Container;
+X u1;
+assert(u1.empty());
+auto u2 = X();
+assert(u2.empty());
+  }
+}
+
+int main()
+{
+  // Sequence containers:
+  {
+std::array a;
+(void)a;
+  }
+  TestSequencePtr();
+  TestSequencePtr();
+  TestSequencePtr();
+
+  // Associative containers, non-mapping:
+  TestSequencePtr();
+  TestSequencePtr();
+
+  // Associative containers, mapping:
+  TestMappingPtr();
+  TestMappingPtr();
+
+  // Container adapters:
+  TestSequencePtr();
+  TestSequencePtr();
+}
Index: include/memory
===
--- include/memory
+++ include/memory
@@ -1346,9 +1346,9 @@
 struct __has_construct
 : integral_constant(),
-  declval<_Pointer>(),
-  declval<_Args>()...)),
+decltype(_VSTD::__has_construct_test(declval<_Alloc>(),
+ declval<_Pointer>(),
+ declval<_Args>()...)),
 true_type>::value>
 {
 };
@@ -1367,8 +1367,8 @@
 struct __has_destroy
 : integral_constant(),
-declval<_Pointer>())),
+decltype(_VSTD::__has_destroy_test(declval<_Alloc>(),
+   declval<_Pointer>())),

[PATCH] D37538: [libc++] Remove problematic ADL in container implementations.

[David L. Jones via Phabricator via cfe-commits]

dlj marked 2 inline comments as done.
dlj added inline comments.



Comment at: include/deque:1167-1168
 allocator_type& __a = __alloc();
-for (iterator __i = begin(), __e = end(); __i != __e; ++__i)
-__alloc_traits::destroy(__a, _VSTD::addressof(*__i));
+for (iterator __i = begin(), __e = end(); __i.__ptr_ != __e.__ptr_; 
__i.operator++())
+__alloc_traits::destroy(__a, _VSTD::addressof(__i.operator*()));
 size() = 0;

EricWF wrote:
> rsmith wrote:
> > The other changes all look like obvious improvements to me. This one is a 
> > little more subtle, but if we want types like `deque` 
> > to be destructible, I think we need to do something equivalent to this.
> That's really yucky! I'm OK with this, but I really don't like it.
> 
> Fundamentally this can't work, at least not generically. When the allocator 
> produces a fancy pointer type, the operator lookups need to be performed 
> using ADL.
> 
> In this specific case, we control the iterator type, but this isn't always 
> true. for example like in `__split_buffer`, which uses the allocators pointer 
> type as an iterator.
> 
> @dlj I updated your test case to demonstrate the fancy-pointer problem: 
> https://gist.github.com/EricWF/b465fc475f55561ba972b6dd87e7e7ea
> 
> So I think we have a couple choices:
> 
> (1) Do nothing, since we can't universally support the behavior, so we 
> shouldn't attempt to support any form of this behavior.
> (2) Fix only the non-fancy pointer case (which is what this patch does).
> (3) Attempt to fix *all* cases, including the fancy-pointer ones. This won't 
> be possible, but perhaps certain containers can tolerate incomplete fancy 
> pointers?
> 
> Personally I'm leaning towards (1), since we can't claim to support the use 
> case, at least not universally. If we select (1) we should probably encode 
> the restriction formally in standardese.
> 
> 
So I think there are at least a couple of issues here, but first I want to get 
clarification on this point:

> Do nothing, since we can't universally support the behavior, so we shouldn't 
> attempt to support any form of this behavior.

To me, this sounds like it could be equivalent to the statement "someone might 
do a bad job of implementing type-erasure for fancy pointers, so we should 
never attempt to preserve type erasure of any pointers." Now, that statement 
seems tenuous //at best// (and a straw man on a slippery slope otherwise), so 
I'm guessing that's not quite what you meant. ;-)

That said, it does sort of make sense to ignore deque for now, so I'll drop it 
from the patch; but for other containers, I don't really see the argument.

As for #3: I'm able to make (most of) your gist pass for everything but 
deque... calls to _VSTD::__to_raw_pointer are enough to avoid ADL around 
operators, although a few other changes are needed, too. That probably makes 
sense to do as a follow-up.

As for deque in particular, I think there may just be some missing functions on 
the __deque_iterator, but it probably warrants a closer look before adding to 
the interface.



Comment at: include/memory:1349
 is_same<
-decltype(__has_construct_test(declval<_Alloc>(),
-  declval<_Pointer>(),
-  declval<_Args>()...)),
+decltype(_VSTD::__has_construct_test(declval<_Alloc>(),
+ declval<_Pointer>(),

EricWF wrote:
> Quuxplusone wrote:
> > EricWF wrote:
> > > Wouldn't the `declval` calls also need qualification?
> > (Sorry I'm jumping in before I know the full backstory here.) I would 
> > expect that the "declval" call doesn't need qualification because its 
> > argument-list is empty. ADL doesn't apply to template arguments AFAIK.
> > (Test case proving it to myself: 
> > https://wandbox.org/permlink/i0YarAfjOYKOhLFP )
> > 
> > Now, IMHO, `__has_construct_test` also doesn't need guarding against ADL, 
> > because it contains a double underscore and is thus firmly in the library's 
> > namespace. If the user has declared an ADL-findable entity 
> > `my::__has_construct_test`, they're already in undefined behavior land and 
> > you don't need to uglify libc++'s code just to coddle such users.
> > 
> > And, IMO, to the extent that ADL *does* work on the old code here, that's a 
> > feature, not a bug. As the implementor of some weird fancy pointer or 
> > iterator type, I might *like* having the power to hook into libc++'s 
> > customization points here. Of course I wouldn't try to hook into 
> > `__has_construct_test`, but `__to_raw_pointer` feels more likely. (At my 
> > current low level of understanding, that is.)
> >  If the user has declared an ADL-findable entity my::__has_construct_test, 
> > they're already in undefined behavior land 
> 
> The problem isn't that it will find a `my::__has_construct_test`, but that 
> 

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF updated this revision to Diff 115162.
EricWF added a comment.

- Remove accidentally committed test files.
- Attempt to remove incidental whitespace changes.


https://reviews.llvm.org/D37035

Files:
  docs/LanguageExtensions.rst
  include/clang/AST/Decl.h
  include/clang/AST/Expr.h
  include/clang/AST/ExprCXX.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/AST/Stmt.h
  include/clang/Basic/StmtNodes.td
  include/clang/Basic/TokenKinds.def
  include/clang/Sema/Sema.h
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/ASTImporter.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprCXX.cpp
  lib/AST/ExprClassification.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/StmtProfile.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprAgg.cpp
  lib/CodeGen/CGExprComplex.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Parse/ParseExpr.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExceptionSpec.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/CodeGenCXX/builtin_FUNCTION.cpp
  test/CodeGenCXX/builtin_LINE.cpp
  test/CodeGenCXX/debug-info-line.cpp
  test/Parser/builtin_source_location.c
  test/Sema/source_location.c
  test/SemaCXX/Inputs/source-location-file.h
  test/SemaCXX/source_location.cpp

Index: test/SemaCXX/source_location.cpp
===
--- /dev/null
+++ test/SemaCXX/source_location.cpp
@@ -0,0 +1,475 @@
+// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s
+// expected-no-diagnostics
+
+#define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42)
+#define CURRENT_FROM_MACRO() SL::current()
+#define FORWARD(...) __VA_ARGS__
+
+namespace std {
+namespace experimental {
+struct source_location {
+private:
+  unsigned int __m_line = 0;
+  unsigned int __m_col = 0;
+  const char *__m_file = nullptr;
+  const char *__m_func = nullptr;
+public:
+  static constexpr source_location current(
+  const char *__file = __builtin_FILE(),
+  const char *__func = __builtin_FUNCTION(),
+  unsigned int __line = __builtin_LINE(),
+  unsigned int __col = __builtin_COLUMN()) noexcept {
+source_location __loc;
+__loc.__m_line = __line;
+__loc.__m_col = __col;
+__loc.__m_file = __file;
+__loc.__m_func = __func;
+return __loc;
+  }
+  constexpr source_location() = default;
+  constexpr source_location(source_location const &) = default;
+  constexpr unsigned int line() const noexcept { return __m_line; }
+  constexpr unsigned int column() const noexcept { return __m_col; }
+  constexpr const char *file() const noexcept { return __m_file; }
+  constexpr const char *function() const noexcept { return __m_func; }
+};
+} // namespace experimental
+} // namespace std
+
+using SL = std::experimental::source_location;
+
+#include "Inputs/source-location-file.h"
+namespace SLF = source_location_file;
+
+constexpr bool is_equal(const char *LHS, const char *RHS) {
+  while (*LHS != 0 && *RHS != 0) {
+if (*LHS != *RHS)
+  return false;
+++LHS;
+++RHS;
+  }
+  return *LHS == 0 && *RHS == 0;
+}
+
+template 
+constexpr T identity(T t) {
+  return t;
+}
+
+template 
+struct Pair {
+  T first;
+  U second;
+};
+
+template 
+constexpr bool is_same = false;
+template 
+constexpr bool is_same = true;
+
+// test types
+static_assert(is_same);
+static_assert(is_same);
+static_assert(is_same);
+static_assert(is_same);
+
+// test noexcept
+static_assert(noexcept(__builtin_LINE()));
+static_assert(noexcept(__builtin_COLUMN()));
+static_assert(noexcept(__builtin_FILE()));
+static_assert(noexcept(__builtin_FUNCTION()));
+
+//===--===//
+//__builtin_LINE()
+//===--===//
+
+namespace test_line {
+
+static_assert(SL::current().line() == __LINE__);
+static_assert(SL::current().line() == CURRENT_FROM_MACRO().line());
+
+static constexpr SL GlobalS = SL::current();
+
+static_assert(GlobalS.line() == __LINE__ - 2);
+
+// clang-format off
+constexpr bool test_line_fn() {
+  constexpr SL S = SL::current();
+  static_assert(S.line() == (__LINE__ - 1), "");
+  // The start of the call expression to `current()` begins at the token `SL`
+  constexpr int ExpectLine = __LINE__ + 3;
+  constexpr SL S2
+  =
+  SL // Call expression starts here
+  ::
+  current
+  (
+
+  )
+  ;
+  static_assert(S2.line() == ExpectLine, "");
+
+  static_assert(
+  FORWARD(
+ __builtin_LINE
+(
+)
+  )
+== __LINE__ - 1, "");
+  static_assert(\
+\
+  __builtin_LINE()\
+\
+  == __LINE__ 

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added inline comments.



Comment at: lib/AST/Expr.cpp:1940
false, false),
-InitExprs(C, initExprs.size()),
-LBraceLoc(lbraceloc), RBraceLoc(rbraceloc), AltForm(nullptr, true)
-{
+  InitExprs(C, initExprs.size()), LBraceLoc(lbraceloc),
+  RBraceLoc(rbraceloc), AltForm(nullptr, true) {

Urg... Not sure how both clang-format and git -w messed up enough to let these 
whitespace changes through (and the ones below this).



Comment at: lib/AST/ExprConstant.cpp:591
 
+SourceLocContextRAIIBase *EvaluatingDefaultArg;
+SourceLocContextRAIIBase *EvaluatingDefaultMemberInit;

These members need documentation.



Comment at: lib/AST/ExprConstant.cpp:6279
 
+const auto *DIE = dyn_cast(InitExpr);
 // Temporarily override This, in case there's a CXXDefaultInitExpr in here.

This change seems unnecessary. 


https://reviews.llvm.org/D37035



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

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added inline comments.



Comment at: test/SemaCXX/loc2.cpp:1
+// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s
+// expected-no-diagnostics

Didn't mean to include this file.



Comment at: test/SemaCXX/test.cpp:1
+struct SL {
+  static constexpr SL current(

Didn't mean to include this file either.


https://reviews.llvm.org/D37035



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

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF updated this revision to Diff 115161.
EricWF marked an inline comment as done.
EricWF added a comment.

- Reimplement without rewriting the AST and instead during the substitution 
during constant expression evaluation and code gen.

I still haven't implemented Richards suggestion to reduce the size of 
`source_location` struct, but I think that would be better done as a separate 
patch. These builtins still need to be implemented, and to act like their GNU 
counterparts.

I haven't cleaned this patch up yet, but I plan to do so tomorrow. There are 
going to be silly mistakes until then.


https://reviews.llvm.org/D37035

Files:
  docs/LanguageExtensions.rst
  include/clang/AST/Decl.h
  include/clang/AST/Expr.h
  include/clang/AST/ExprCXX.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/AST/Stmt.h
  include/clang/Basic/StmtNodes.td
  include/clang/Basic/TokenKinds.def
  include/clang/Sema/Sema.h
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/ASTImporter.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprCXX.cpp
  lib/AST/ExprClassification.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/StmtProfile.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprAgg.cpp
  lib/CodeGen/CGExprComplex.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Parse/ParseExpr.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExceptionSpec.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/CodeGenCXX/builtin_FUNCTION.cpp
  test/CodeGenCXX/builtin_LINE.cpp
  test/CodeGenCXX/debug-info-line.cpp
  test/Parser/builtin_source_location.c
  test/Sema/source_location.c
  test/SemaCXX/Inputs/source-location-file.h
  test/SemaCXX/loc2.cpp
  test/SemaCXX/source_location.cpp
  test/SemaCXX/test.cpp

Index: test/SemaCXX/test.cpp
===
--- /dev/null
+++ test/SemaCXX/test.cpp
@@ -0,0 +1,35 @@
+struct SL {
+  static constexpr SL current(
+  const char* f = __builtin_FUNCTION(),
+  unsigned l = __builtin_LINE()) {
+return {f, l};
+  }
+  const char* function;
+  unsigned line;
+};
+
+constexpr bool is_equal(const char *LHS, const char *RHS) {
+  while (*LHS != 0 && *RHS != 0) {
+if (*LHS != *RHS)
+  return false;
+++LHS;
+++RHS;
+  }
+  return *LHS == 0 && *RHS == 0;
+}
+
+
+template 
+constexpr const char* test_func_template(T, U u = U::current()) {
+  static_assert(is_equal(U::current().function, __func__));
+  static_assert(is_equal(U::current().function, ""));
+  static_assert(U::current().line==  __LINE__);
+  static_assert(U::current().line==  __LINE__);
+
+  return u.function;
+}
+template 
+void func_template_tests() {
+  constexpr auto P = test_func_template(42);
+}
+template void func_template_tests();
Index: test/SemaCXX/source_location.cpp
===
--- /dev/null
+++ test/SemaCXX/source_location.cpp
@@ -0,0 +1,475 @@
+// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s
+// expected-no-diagnostics
+
+#define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42)
+#define CURRENT_FROM_MACRO() SL::current()
+#define FORWARD(...) __VA_ARGS__
+
+namespace std {
+namespace experimental {
+struct source_location {
+private:
+  unsigned int __m_line = 0;
+  unsigned int __m_col = 0;
+  const char *__m_file = nullptr;
+  const char *__m_func = nullptr;
+public:
+  static constexpr source_location current(
+  const char *__file = __builtin_FILE(),
+  const char *__func = __builtin_FUNCTION(),
+  unsigned int __line = __builtin_LINE(),
+  unsigned int __col = __builtin_COLUMN()) noexcept {
+source_location __loc;
+__loc.__m_line = __line;
+__loc.__m_col = __col;
+__loc.__m_file = __file;
+__loc.__m_func = __func;
+return __loc;
+  }
+  constexpr source_location() = default;
+  constexpr source_location(source_location const &) = default;
+  constexpr unsigned int line() const noexcept { return __m_line; }
+  constexpr unsigned int column() const noexcept { return __m_col; }
+  constexpr const char *file() const noexcept { return __m_file; }
+  constexpr const char *function() const noexcept { return __m_func; }
+};
+} // namespace experimental
+} // namespace std
+
+using SL = std::experimental::source_location;
+
+#include "Inputs/source-location-file.h"
+namespace SLF = source_location_file;
+
+constexpr bool is_equal(const char *LHS, const char *RHS) {
+  while (*LHS != 0 && *RHS != 0) {
+if (*LHS != *RHS)
+  return false;
+++LHS;
+++RHS;
+  }
+  return *LHS == 0 && *RHS == 0;
+}
+
+template 
+constexpr T identity(T t) {
+  return t;
+}
+
+template 
+struct Pair {
+  T first;
+  U second;
+};
+
+template 
+constexpr bool is_same = false;
+template 
+constexpr bool is_same = true;
+
+// test types

[PATCH] D36806: Switch to cantFail(), since it does the same assertion.

[Stephen Hines via Phabricator via cfe-commits]

srhines added a comment.

Ping


https://reviews.llvm.org/D36806



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

[PATCH] D37308: Fix the __interface inheritence rules to work better with IUnknown and IDispatch

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 115153.
erichkeane retitled this revision from "Interface class with uuid base record" 
to "Fix the __interface inheritence rules to work better with IUnknown and 
IDispatch".
erichkeane edited the summary of this revision.
erichkeane added a comment.
Herald added a subscriber: eraman.

Based off of what I discovered previously, this is my first run at a solution 
to this problem. @zahiraam (and others), if you can see if I missed anything in 
this patch, it would be appreciated.  I think this is a good place to start 
further discussion than text conversation.


https://reviews.llvm.org/D37308

Files:
  include/clang/AST/DeclCXX.h
  lib/AST/DeclCXX.cpp
  lib/Sema/SemaDeclCXX.cpp
  test/SemaCXX/ms-iunknown-inline-def.cpp
  test/SemaCXX/ms-iunknown-outofline-def.cpp
  test/SemaCXX/ms-iunknown.cpp

Index: test/SemaCXX/ms-iunknown.cpp
===
--- /dev/null
+++ test/SemaCXX/ms-iunknown.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -fms-extensions %s 
+
+struct __declspec(uuid("---C000-0046")) IUnknown {
+  void foo();
+};
+struct IPropertyPageBase : public IUnknown {};
+struct IPropertyPage : public IPropertyPageBase {};
+__interface ISfFileIOPropertyPage : public IPropertyPage {};
+
+
+namespace NS {
+  struct __declspec(uuid("---C000-0046")) IUnknown {};
+  // expected-error@+1 {{interface type cannot inherit from}}
+  __interface IPropertyPageBase : public IUnknown {}; 
+}
+// expected-error@+1 {{interface type cannot inherit from}}
+__interface IPropertyPageBase2 : public NS::IUnknown {}; 
+
+__interface temp_iface {};
+struct bad_base : temp_iface {};
+// expected-error@+1 {{interface type cannot inherit from}}
+__interface cant_inherit: public bad_base{};
+
+struct fine_base : temp_iface, IUnknown {};
+__interface can_inherit: public fine_base{};
+
+struct PageBase : public IUnknown {};
+struct Page3 : public PageBase {};
+struct Page4 : public PageBase {};
+__interface PropertyPage : public Page4 {};
+
+struct Page5 : public Page3, Page4{};
+// expected-error@+1 {{interface type cannot inherit from}}
+__interface PropertyPage2 : public Page5 {}; 
+
+__interface IF1 {};
+__interface PP : IUnknown, IF1{}; 
+__interface PP2 : PP, Page3, Page4{};
Index: test/SemaCXX/ms-iunknown-outofline-def.cpp
===
--- /dev/null
+++ test/SemaCXX/ms-iunknown-outofline-def.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -fms-extensions %s 
+
+struct __declspec(uuid("---C000-0046")) IUnknown {
+  void foo();
+};
+
+__interface NoError : public IUnknown {};
+void IUnknown::foo() {}
+// expected-error@+1{{interface type cannot inherit from}}
+__interface HasError : public IUnknown {}; 
Index: test/SemaCXX/ms-iunknown-inline-def.cpp
===
--- /dev/null
+++ test/SemaCXX/ms-iunknown-inline-def.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -fms-extensions %s 
+
+struct __declspec(uuid("---C000-0046")) IUnknown {
+  void foo() {}
+};
+
+// expected-error@+1{{interface type cannot inherit from}}
+__interface HasError : public IUnknown {}; 
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -2388,7 +2388,7 @@
   if (const RecordType *Record = NewBaseType->getAs()) {
 const CXXRecordDecl *RD = cast(Record->getDecl());
 if (Class->isInterface() &&
-  (!RD->isInterface() ||
+  (!RD->isInterfaceLike() ||
KnownBase->getAccessSpecifier() != AS_public)) {
   // The Microsoft extension __interface does not permit bases that
   // are not themselves public interfaces.
Index: lib/AST/DeclCXX.cpp
===
--- lib/AST/DeclCXX.cpp
+++ lib/AST/DeclCXX.cpp
@@ -1470,6 +1470,61 @@
   return false;
 }
 
+bool CXXRecordDecl::isInterfaceLike() const {
+  // All __interfaces are inheritently interface like.
+  if (isInterface())
+return true;
+
+  // No interface-like types can have a user declared constructor, destructor,
+  // friends, VBases, conersion functions, or fields.  Additionally, lambdas
+  // cannot be interface types.
+  if (isLambda() || hasUserDeclaredConstructor() ||
+  hasUserDeclaredDestructor() || !field_empty() || hasFriends() ||
+  getNumVBases() > 0 || conversion_end() - conversion_begin() > 0)
+return false;
+
+  // No interface-like type can have a method with a definition.
+  for (const auto *const Method : methods())
+if (Method->isDefined())
+  return false;
+
+  // Check "Special" types.
+  const auto *Uuid = getAttr();
+  if (Uuid && isStruct() && getDeclContext()->isTranslationUnit() &&
+  

[PATCH] D37308: Interface class with uuid base record

[Erich Keane via Phabricator via cfe-commits]

erichkeane commandeered this revision.
erichkeane edited reviewers, added: zahiraam; removed: erichkeane.
erichkeane added a comment.

@zahiraam: quickly commendeering, since I have an implementation that I think 
better explains my discovery than my words.  Feel free to commandeer it back 
later (inside the 'Add Action...' box above the comment at the bottom).


https://reviews.llvm.org/D37308



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

[PATCH] D37302: [Headers] Define *_HAS_SUBNORM for FLT, DBL, LDBL

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D37302



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

r313219 - [NFC] [Analyzer] Fix RST markup in documentation.

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Wed Sep 13 17:04:56 2017
New Revision: 313219

URL: http://llvm.org/viewvc/llvm-project?rev=313219=rev
Log:
[NFC] [Analyzer] Fix RST markup in documentation.

Modified:
cfe/trunk/docs/analyzer/DebugChecks.rst

Modified: cfe/trunk/docs/analyzer/DebugChecks.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/analyzer/DebugChecks.rst?rev=313219=313218=313219=diff
==
--- cfe/trunk/docs/analyzer/DebugChecks.rst (original)
+++ cfe/trunk/docs/analyzer/DebugChecks.rst Wed Sep 13 17:04:56 2017
@@ -74,7 +74,7 @@ inspects expressions.)
 ExprInspection checks
 -
 
-- void clang_analyzer_eval(bool);
+- ``void clang_analyzer_eval(bool);``
 
   Prints TRUE if the argument is known to have a non-zero value, FALSE if the
   argument is known to have a zero or null value, and UNKNOWN if the argument
@@ -93,7 +93,7 @@ ExprInspection checks
 clang_analyzer_eval(x); // expected-warning{{TRUE}}
 
 
-- void clang_analyzer_checkInlined(bool);
+- ``void clang_analyzer_checkInlined(bool);``
 
   If a call occurs within an inlined function, prints TRUE or FALSE according 
to
   the value of its argument. If a call occurs outside an inlined function,
@@ -125,7 +125,7 @@ ExprInspection checks
   clang_analyzer_eval(value == 42); // expected-warning{{TRUE}}
 }
 
-- void clang_analyzer_warnIfReached();
+- ``void clang_analyzer_warnIfReached();``
 
   Generate a warning if this line of code gets reached by the analyzer.
 
@@ -138,7 +138,7 @@ ExprInspection checks
   clang_analyzer_warnIfReached();  // no-warning
 }
 
-- void clang_analyzer_numTimesReached();
+- ``void clang_analyzer_numTimesReached();``
 
   Same as above, but include the number of times this call expression
   gets reached by the analyzer during the current analysis.
@@ -149,7 +149,7 @@ ExprInspection checks
   clang_analyzer_numTimesReached(); // expected-warning{{3}}
 }
 
-- void clang_analyzer_warnOnDeadSymbol(int);
+- ``void clang_analyzer_warnOnDeadSymbol(int);``
 
   Subscribe for a delayed warning when the symbol that represents the value of
   the argument is garbage-collected by the analyzer.
@@ -173,7 +173,7 @@ ExprInspection checks
 } while(0);  // expected-warning{{SYMBOL DEAD}}
 
 
-- void clang_analyzer_explain(a single argument of any type);
+- ``void clang_analyzer_explain(a single argument of any type);``
 
   This function explains the value of its argument in a human-readable manner
   in the warning message. You can make as many overrides of its prototype
@@ -197,7 +197,7 @@ ExprInspection checks
 clang_analyzer_explain(ptr); // expected-warning{{memory address '0'}}
 }
 
-- void clang_analyzer_dump(a single argument of any type);
+- ``void clang_analyzer_dump( /* a single argument of any type */);``
 
   Similar to clang_analyzer_explain, but produces a raw dump of the value,
   same as SVal::dump().
@@ -209,7 +209,7 @@ ExprInspection checks
   clang_analyzer_dump(x); // expected-warning{{reg_$0}}
 }
 
-- size_t clang_analyzer_getExtent(void *);
+- ``size_t clang_analyzer_getExtent(void *);``
 
   This function returns the value that represents the extent of a memory region
   pointed to by the argument. This value is often difficult to obtain 
otherwise,
@@ -226,7 +226,7 @@ ExprInspection checks
   clang_analyzer_explain(ys); // expected-warning{{'8'}}
 }
 
-- void clang_analyzer_printState();
+- ``void clang_analyzer_printState();``
 
   Dumps the current ProgramState to the stderr. Quickly lookup the program 
state
   at any execution point without ViewExplodedGraph or re-compiling the program.


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

[PATCH] D37089: [Sema] Error out early for tags defined inside an enumeration.

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

I think this looks good, even without fixing the access control crash, this 
seems like a diagnostic improvement.




Comment at: clang/test/Sema/enum.c:128
+// PR28903
+struct PR28903 { // expected-warning {{empty struct is a GNU extension}}
+  enum {

This is a simpler test case that fixes the extraneous diagnostics:
  struct PR28903 {
enum {
  E1 = (enum {  // expected-error-re {{...}}
  E2, E3 = E2
})0
} e;
  };



https://reviews.llvm.org/D37089



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

[PATCH] D37436: Initial implementation of C attributes (WG14 N2137)

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In https://reviews.llvm.org/D37436#869851, @hfinkel wrote:

> A large fraction of the number of attributes we'll want to use are going to 
> fall into this category (because Clang doesn't have its own attributes, but 
> copied GCC's, for many things). I don't think we'll get good implementation 
> feedback until we have this figured out. If we can't sync with GCC soon, I 
> suggest just making a reasonable guess. My first choice would be to just 
> allowing everything, and then we can see what people found to be useful and 
> why. Our experience here can also provide feedback to GCC (and we can always 
> update late if needed - it is still an experimental feature).


I think I would be more comfortable taking those attributes that we want to 
support in C and making them available in the `clang::` attribute namespace 
(across all languages). Because GCC has distinct C and C++ parsers (sharing 
some, but not all, code) there are often differences between how they handle 
the same construct in C and C++ mode, and accepting all `[[gnu::something]]` 
attributes in C mode seems likely to create incompatibilities, even if GCC 
decides that all `__attribute__`s should be available as `[[gnu::x]]` in C.

Also, your wording paper appears to allow things like

  struct [[foo]] S *p; // ok in c n2137, ill-formed in c++
  struct T {};
  int n = sizeof(struct [[foo]] T); // ok in c n2137, ill-formed in c++

You don't seem to implement those changes; are they bugs in the wording paper? 
(The relevant C++ rule is [dcl.type.elab]p1: "An //attribute-specifier-seq// 
shall not appear in an //elaborated-type-specifier// unless the latter is the 
sole constituent of a //declaration//." Neither of the above cases is 
prohibited by n2137's 6.7.2.1/6: "An //attribute-specifier-seq// shall not 
appear in a //struct-or-union-specifier// of the form //struct-or-union// 
//attribute-specifier-seq//opt //identifier// if the 
//struct-or-union-specifier// is an incomplete type used in a 
//parameter-declaration// or //type-name//." -- the first case is not in a 
//parameter-declaration// or //type-name// and the second case is not an 
incomplete type -- and no other rule seems to disallow this.)




Comment at: include/clang/Basic/LangOptions.def:140
 
+LANGOPT(CAttributes   , 1, 0, "'[[]]' attributes extension to C")
+

aaron.ballman wrote:
> rsmith wrote:
> > Can we give this a more general name, then enable it under `CPlusPlus` too? 
> > That's what we do for other similar features.
> I'm not keen on naming it 'Attributes', but the other names are worse 
> (`GeneralAttributes`, `StandardAttributes`). Do you have a suggestion?
It's kind of long, but how about `DoubleSquareBracketAttributes`



Comment at: include/clang/Driver/Options.td:607
 
+def fc_attributes : Flag<["-"], "fc-attributes">, Group,
+  Flags<[DriverOption, CC1Option]>, HelpText<"Enable '[[]]' attributes in C">;

aaron.ballman wrote:
> rsmith wrote:
> > Hmm. On reflection, if we're going to have a flag to enable C++11 
> > attributes in other language modes, it should probably work in C++98 mode 
> > too, so calling this `-fc-attributes` is probably not the best idea. 
> > `-fc++11-attributes` might make sense, though.
> I can't think of a reason why you'd want to control C and C++ attributes 
> separately, so I think it makes sense to add a more general name for this.
> 
> I'm definitely not keen on that flag name. I wouldn't be opposed to 
> `-fattributes`, but that may lead to confusion about other vendor-specific 
> attributes (which we could always decide to use flags like 
> `-fdeclspec-attributes` etc).
> 
> What should happen if a user compiles with `-std=c++11 
> -fno--attributes`? Do we want to support such a construct?
I wouldn't recommend anyone actually does that, but I'd expect clang to respect 
their wishes if they ask for it, just like we do for `-fms-extensions 
-fno-declspec`.



Comment at: lib/Parse/ParseDecl.cpp:4219
+
+// Attributes are prohibited in this location in C2x (and forward
+// declarations are prohibited in C++).

aaron.ballman wrote:
> rsmith wrote:
> > I don't think we can reasonably say what C2x will do. Also, doesn't this 
> > reject valid code such as:
> > ```
> > struct __attribute__((deprecated)) Foo;
> > ```
> > ?
> Sorry for the lack of context in the patch (TortoiseSVN doesn't make this 
> easy). This has to do with enum specifiers, not struct or unions -- it will 
> reject `enum __attribute__((deprecated)) Foo;` as not allowing an attribute 
> list *and* as being a forward reference in C++ mode, but accepts in C mode.
OK, but GCC accepts that with a warning in the `friend` case, and this would 
also seem to reject valid constructs such as
```
enum __attribute__((deprecated)) Foo : int;
```
But... C permits neither the `TUK_Declaration` nor `TUK_Friend` cases for 
`enum`s. The change in your 

[libcxxabi] r313215 - Reland "When built with ASan, __cxa_throw calls __asan_handle_no_return"

[Petr Hosek via cfe-commits]

Author: phosek
Date: Wed Sep 13 16:35:07 2017
New Revision: 313215

URL: http://llvm.org/viewvc/llvm-project?rev=313215=rev
Log:
Reland "When built with ASan, __cxa_throw calls __asan_handle_no_return"

The ASan runtime on many systems intercepts cxa_throw just so it
can call asan_handle_no_return first. Some newer systems such as
Fuchsia don't use interceptors on standard library functions at all,
but instead use sanitizer-instrumented versions of the standard
libraries. When libc++abi is built with ASan, cxa_throw can just
call asan_handle_no_return itself so no interceptor is required.

Patch by Roland McGrath

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

Modified:
libcxxabi/trunk/src/cxa_exception.cpp

Modified: libcxxabi/trunk/src/cxa_exception.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/src/cxa_exception.cpp?rev=313215=313214=313215=diff
==
--- libcxxabi/trunk/src/cxa_exception.cpp (original)
+++ libcxxabi/trunk/src/cxa_exception.cpp Wed Sep 13 16:35:07 2017
@@ -19,6 +19,10 @@
 #include "cxa_handlers.hpp"
 #include "fallback_malloc.h"
 
+#if __has_feature(address_sanitizer)
+#include 
+#endif
+
 // +---+-+---+
 // | __cxa_exception   | _Unwind_Exception CLNGC++\0 | thrown object |
 // +---+-+---+
@@ -217,6 +221,12 @@ __cxa_throw(void *thrown_object, std::ty
 globals->uncaughtExceptions += 1;   // Not atomically, since globals are 
thread-local
 
 exception_header->unwindHeader.exception_cleanup = exception_cleanup_func;
+
+#if __has_feature(address_sanitizer)
+// Inform the ASan runtime that now might be a good time to clean stuff up.
+__asan_handle_no_return();
+#endif
+
 #ifdef __USING_SJLJ_EXCEPTIONS__
 _Unwind_SjLj_RaiseException(_header->unwindHeader);
 #else


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

[PATCH] D37302: [Headers] Define *_HAS_SUBNORM for FLT, DBL, LDBL

[Pirama Arumuga Nainar via Phabricator via cfe-commits]

pirama added a comment.

Ping


https://reviews.llvm.org/D37302



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

[PATCH] D37196: [Clang] Bug 32352 - Provide a way for OptimizationRemarkEmitter::allowExtraAnalysis to check if (specific) remarks are enabled

[Adam Nemet via Phabricator via cfe-commits]

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

LGTM.


https://reviews.llvm.org/D37196



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

[PATCH] D37308: Interface class with uuid base record

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

SO, the implementation would likely be (~2489):

  if (Class->isInterface() && (KnownBase->getAccessSpecifier() != TTK_public || 
!RD->isInterface() || !RD->isInterfaceLike()) {

with "isInterfaceLike" testing what I outlined above.


https://reviews.llvm.org/D37308



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

[PATCH] D33514: [WIP] Bug 32352 - Provide a way for OptimizationRemarkEmitter::allowExtraAnalysis to check if (specific) remarks are enabled

[Adam Nemet via Phabricator via cfe-commits]

anemet accepted this revision.
anemet added a comment.

Still looks good.




Comment at: lib/IR/LLVMContext.cpp:332
+}
\ No newline at end of file


Has this version of the diff been clang-formatted?


https://reviews.llvm.org/D33514



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

[PATCH] D37308: Interface class with uuid base record

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

A bit more research based on a different implementation:

First, there are TWO special types, not just IUnknown!  There is also 
"IDispatch" with UUID: "00020400---c000-0046"

A type is 'interface like' if:
-it has a ZERO virtual inheritance bases
-it has AT LEAST 1 'interface-like' base (interfaces are not 'interface_like' 
here)
-it has ZERO non-interface/non-interface-like bases.
-it has zero user defined destructors, constructors, operators, or conversions
-it has zero DEFINED methods
-it has ZERO data members
-it does not have 'private' or 'protected' data access specifiers
-it does not have any "friend" decls.

An __interface can inherit from other __interfaces, or interface-like bases.  
However, it is limited to only 1 of the latter.

SO, in your example, "Page5" loses its interface-like-ness because it has 2 
separate interface-like bases.


https://reviews.llvm.org/D37308



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

[PATCH] D37308: Interface class with uuid base record

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Yeah, but

  __interface IF1 {};
  __interface PP : IUnknown, IF1{}; 
  __interface PP2 : PP, Page3, Page4{};

Base PP has siblings here.  It seems the rule is even more complex then.


https://reviews.llvm.org/D37308



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

[PATCH] D31140: [LLVMbugs] [Bug 18710] Only generate .ARM.exidx and .ARM.extab when needed in EHABI

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: lib/Driver/ToolChains/BareMetal.cpp:61
+bool BareMetal::IsUnwindTablesDefault(const ArgList ) const {
+  return getDriver().CCCIsCXX();
+}

This still seems weird.  In most situations, I would expect you want the same 
behavior for C code and C++ with -fno-exceptions.  On other platforms, we get 
that behavior by just returning false (the default).


https://reviews.llvm.org/D31140



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

[PATCH] D37308: Interface class with uuid base record

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.

MSVC and xmain compile this:
struct __declspec(uuid("---C000-0046")) IUnknown {};
struct PageBase : public IUnknown {};
struct Page3 : public PageBase {};
struct Page4 : public PageBase {};
__interface PropertyPage : public Page4 {};

But MSVC doesn't compile this:

struct __declspec(uuid("---C000-0046")) IUnknown {};
struct PageBase : public IUnknown {};
struct Page3 : public PageBase {};
struct Page4 : public PageBase {};
struct Page5 : public Page3, Page4{};
__interface PropertyPage : public Page5 {};

So a base of RD that has siblings and inherits from a Unknown is not permitted. 
Which means that if the number is siblings are greater than one, we should 
fail. I guess the current function doesn't take that into account.


https://reviews.llvm.org/D37308



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

[PATCH] D37308: Interface class with uuid base record

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.

Erich,

The IsOrInheritsFromIUnknown function is needed. 
Wh




Comment at: lib/Sema/SemaDeclCXX.cpp:2377
+/// \brief Tests if the __interface base is public.
+static bool IsBasePublicInterface(const CXXRecordDecl *RD,
+  AccessSpecifier spec) {

erichkeane wrote:
> This function isn't testing the 'base', it is testing the actual record decl.
RD is the base of the interface. But I can rename it to IsRDPublicInterface.


https://reviews.llvm.org/D37308



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

r313179 - [docs] Update LeakSanitizer documentation to reflect OS X support

[Francis Ricci via cfe-commits]

Author: fjricci
Date: Wed Sep 13 12:40:10 2017
New Revision: 313179

URL: http://llvm.org/viewvc/llvm-project?rev=313179=rev
Log:
[docs] Update LeakSanitizer documentation to reflect OS X support

Reviewers: kcc, alekseyshl, kubamracek, glider

Subscribers: llvm-commits

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

Modified:
cfe/trunk/docs/AddressSanitizer.rst
cfe/trunk/docs/LeakSanitizer.rst

Modified: cfe/trunk/docs/AddressSanitizer.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/AddressSanitizer.rst?rev=313179=313178=313179=diff
==
--- cfe/trunk/docs/AddressSanitizer.rst (original)
+++ cfe/trunk/docs/AddressSanitizer.rst Wed Sep 13 12:40:10 2017
@@ -140,7 +140,8 @@ Memory leak detection
 -
 
 For more information on leak detector in AddressSanitizer, see
-:doc:`LeakSanitizer`. The leak detection is turned on by default on Linux;
+:doc:`LeakSanitizer`. The leak detection is turned on by default on Linux,
+and can be enabled using ``ASAN_OPTIONS=detect_leaks=1`` on OS X;
 however, it is not yet supported on other platforms.
 
 Issue Suppression

Modified: cfe/trunk/docs/LeakSanitizer.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LeakSanitizer.rst?rev=313179=313178=313179=diff
==
--- cfe/trunk/docs/LeakSanitizer.rst (original)
+++ cfe/trunk/docs/LeakSanitizer.rst Wed Sep 13 12:40:10 2017
@@ -17,7 +17,7 @@ detection phase.
 Usage
 =
 
-LeakSanitizer is only supported on x86\_64 Linux. In order to use it,
+LeakSanitizer is supported on x86\_64 Linux and OS X. In order to use it,
 simply build your program with :doc:`AddressSanitizer`:
 
 .. code-block:: console
@@ -30,7 +30,7 @@ simply build your program with :doc:`Add
   p = 0; // The memory is leaked here.
   return 0;
 }
-% clang -fsanitize=address -g memory-leak.c ; ./a.out
+% clang -fsanitize=address -g memory-leak.c ; ASAN_OPTIONS=detect_leaks=1 
./a.out
 ==23646==ERROR: LeakSanitizer: detected memory leaks
 Direct leak of 7 byte(s) in 1 object(s) allocated from:
 #0 0x4af01b in __interceptor_malloc 
/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:52:3


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

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6031
   InGroup;
+def ext_gnu_null_ptr_arith : Extension<
+  "arithmetic on a null pointer treated as a cast from integer to pointer is a 
GNU extension">,

andrew.w.kaylor wrote:
> efriedma wrote:
> > andrew.w.kaylor wrote:
> > > efriedma wrote:
> > > > "extension" isn't really right here; this shouldn't be an error in 
> > > > -pedantic-errors mode.  Probably best to just stick this into the 
> > > > NullPointerArithmetic, like the other new warning.
> > > So how should a word the warning?  Just this:
> > > 
> > > "arithmetic on a null pointer treated as a cast from integer to pointer"?
> > That wasn't what I meant; the current wording is fine. I meant this should 
> > be something like `def warn_gnu_null_ptr_arith : Warning<`.
> OK.  I think I understand the behavior you wanted.  I just thought maybe the 
> current wording might be technically incorrect.  I wasn't sure how precisely 
> defined we consider "extension" to be in this context.
The part that makes this a little weird is that unlike most extensions, this 
code is already well-formed.  It's an extension because we're guaranteeing 
runtime behavior for a construct which has undefined behavior at runtime 
according to the standard.  (This is in contrast to "implementation-defined" 
behaviors, which are the gaps in the standard we're allowed to fill in as we 
see fit.)

Given that, I think calling it a "GNU extension" in the text is fine.


https://reviews.llvm.org/D37042



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

[PATCH] D37308: Interface class with uuid base record

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 115125.
zahiraam added a comment.

Hi have made all the changes requested.


https://reviews.llvm.org/D37308

Files:
  lib/Sema/SemaDeclCXX.cpp


Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -2373,6 +2373,40 @@
   return true;
 }
 
+/// \brief Tests if RD is a public interface.
+static bool IsDeclPublicInterface(const CXXRecordDecl *RD,
+  AccessSpecifier spec) {
+  return RD->isInterface() && spec == AS_public;
+}
+
+/// \brief Test if record is a uuid for IUnknown.
+/// This is an MS SDK specific type that has a special
+/// behavior in the CL compiler.
+static bool IsIUnknownType(const CXXRecordDecl *RD) {
+  const auto *Uuid = RD->getAttr();
+
+  return Uuid && RD->isStruct()  && RD->isEmpty() &&
+ RD->getDeclContext()->isTranslationUnit() &&
+ RD->getName() == "IUnknown" &&
+ Uuid->getGuid() =="---C000-0046";
+}
+
+/// \brief Test if RD or its inherited bases is an IUnknown type.
+static bool IsOrInheritsFromIUnknown(const CXXRecordDecl *RD) {
+  bool IsUnknown = IsIUnknownType(RD);
+  for (CXXRecordDecl::base_class_const_iterator Base = RD->bases_begin(),
+BaseEnd = RD->bases_end();
+   Base != BaseEnd; ++Base) {
+   CXXRecordDecl *BaseChild = Base->getType()->getAsCXXRecordDecl();
+   
+   return IsUnknown ||
+  IsOrInheritsFromIUnknown(BaseChild) ||
+  (RD->getNumBases() > 1) &&
+  IsOrInheritsFromIUnknown((CXXRecordDecl*) 
BaseChild->getNextDeclInContext());
+  }
+  return IsUnknown;
+}
+
 /// Use small set to collect indirect bases.  As this is only used
 /// locally, there's no need to abstract the small size parameter.
 typedef llvm::SmallPtrSet IndirectBaseSet;
@@ -2450,10 +2484,10 @@
   if (const RecordType *Record = NewBaseType->getAs()) {
 const CXXRecordDecl *RD = cast(Record->getDecl());
 if (Class->isInterface() &&
-  (!RD->isInterface() ||
-   KnownBase->getAccessSpecifier() != AS_public)) {
-  // The Microsoft extension __interface does not permit bases that
-  // are not themselves public interfaces.
+// The Microsoft extension __interface does not permit bases that
+// are not themselves public interfaces.
+!IsDeclPublicInterface(RD, KnownBase->getAccessSpecifier()) &&
+!IsOrInheritsFromIUnknown(RD)) {
   Diag(KnownBase->getLocStart(), diag::err_invalid_base_in_interface)
 << getRecordDiagFromTagKind(RD->getTagKind()) << RD->getName()
 << RD->getSourceRange();


Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -2373,6 +2373,40 @@
   return true;
 }
 
+/// \brief Tests if RD is a public interface.
+static bool IsDeclPublicInterface(const CXXRecordDecl *RD,
+  AccessSpecifier spec) {
+  return RD->isInterface() && spec == AS_public;
+}
+
+/// \brief Test if record is a uuid for IUnknown.
+/// This is an MS SDK specific type that has a special
+/// behavior in the CL compiler.
+static bool IsIUnknownType(const CXXRecordDecl *RD) {
+  const auto *Uuid = RD->getAttr();
+
+  return Uuid && RD->isStruct()  && RD->isEmpty() &&
+ RD->getDeclContext()->isTranslationUnit() &&
+ RD->getName() == "IUnknown" &&
+ Uuid->getGuid() =="---C000-0046";
+}
+
+/// \brief Test if RD or its inherited bases is an IUnknown type.
+static bool IsOrInheritsFromIUnknown(const CXXRecordDecl *RD) {
+  bool IsUnknown = IsIUnknownType(RD);
+  for (CXXRecordDecl::base_class_const_iterator Base = RD->bases_begin(),
+BaseEnd = RD->bases_end();
+   Base != BaseEnd; ++Base) {
+   CXXRecordDecl *BaseChild = Base->getType()->getAsCXXRecordDecl();
+   
+   return IsUnknown ||
+  IsOrInheritsFromIUnknown(BaseChild) ||
+  (RD->getNumBases() > 1) &&
+  IsOrInheritsFromIUnknown((CXXRecordDecl*) BaseChild->getNextDeclInContext());
+  }
+  return IsUnknown;
+}
+
 /// Use small set to collect indirect bases.  As this is only used
 /// locally, there's no need to abstract the small size parameter.
 typedef llvm::SmallPtrSet IndirectBaseSet;
@@ -2450,10 +2484,10 @@
   if (const RecordType *Record = NewBaseType->getAs()) {
 const CXXRecordDecl *RD = cast(Record->getDecl());
 if (Class->isInterface() &&
-  (!RD->isInterface() ||
-   KnownBase->getAccessSpecifier() != AS_public)) {
-  // The Microsoft extension __interface does not permit bases that
-  // are not themselves public interfaces.
+// The Microsoft extension __interface 

r313201 - Use -- to prevent the driver from confusing paths with flags, should fix Mac bot.

[Peter Collingbourne via cfe-commits]

Author: pcc
Date: Wed Sep 13 14:49:17 2017
New Revision: 313201

URL: http://llvm.org/viewvc/llvm-project?rev=313201=rev
Log:
Use -- to prevent the driver from confusing paths with flags, should fix Mac 
bot.

Modified:
cfe/trunk/test/Driver/whole-program-vtables.c

Modified: cfe/trunk/test/Driver/whole-program-vtables.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/whole-program-vtables.c?rev=313201=313200=313201=diff
==
--- cfe/trunk/test/Driver/whole-program-vtables.c (original)
+++ cfe/trunk/test/Driver/whole-program-vtables.c Wed Sep 13 14:49:17 2017
@@ -1,11 +1,11 @@
 // RUN: %clang -target x86_64-unknown-linux -fwhole-program-vtables -### %s 
2>&1 | FileCheck --check-prefix=NO-LTO %s
-// RUN: %clang_cl --target=x86_64-pc-win32 -fwhole-program-vtables -### %s 
2>&1 | FileCheck --check-prefix=NO-LTO %s
+// RUN: %clang_cl --target=x86_64-pc-win32 -fwhole-program-vtables -### -- %s 
2>&1 | FileCheck --check-prefix=NO-LTO %s
 // NO-LTO: invalid argument '-fwhole-program-vtables' only allowed with '-flto'
 
 // RUN: %clang -target x86_64-unknown-linux -fwhole-program-vtables -flto -### 
%s 2>&1 | FileCheck --check-prefix=LTO %s
-// RUN: %clang_cl --target=x86_64-pc-win32 -fwhole-program-vtables -flto -### 
%s 2>&1 | FileCheck --check-prefix=LTO %s
+// RUN: %clang_cl --target=x86_64-pc-win32 -fwhole-program-vtables -flto -### 
-- %s 2>&1 | FileCheck --check-prefix=LTO %s
 // LTO: "-fwhole-program-vtables"
 
 // RUN: %clang -target x86_64-unknown-linux -fwhole-program-vtables 
-fno-whole-program-vtables -flto -### %s 2>&1 | FileCheck 
--check-prefix=LTO-DISABLE %s
-// RUN: %clang_cl --target=x86_64-pc-win32 -fwhole-program-vtables 
-fno-whole-program-vtables -flto -### %s 2>&1 | FileCheck 
--check-prefix=LTO-DISABLE %s
+// RUN: %clang_cl --target=x86_64-pc-win32 -fwhole-program-vtables 
-fno-whole-program-vtables -flto -### -- %s 2>&1 | FileCheck 
--check-prefix=LTO-DISABLE %s
 // LTO-DISABLE-NOT: "-fwhole-program-vtables"


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

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

[Andy Kaylor via Phabricator via cfe-commits]

andrew.w.kaylor added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6031
   InGroup;
+def ext_gnu_null_ptr_arith : Extension<
+  "arithmetic on a null pointer treated as a cast from integer to pointer is a 
GNU extension">,

efriedma wrote:
> andrew.w.kaylor wrote:
> > efriedma wrote:
> > > "extension" isn't really right here; this shouldn't be an error in 
> > > -pedantic-errors mode.  Probably best to just stick this into the 
> > > NullPointerArithmetic, like the other new warning.
> > So how should a word the warning?  Just this:
> > 
> > "arithmetic on a null pointer treated as a cast from integer to pointer"?
> That wasn't what I meant; the current wording is fine. I meant this should be 
> something like `def warn_gnu_null_ptr_arith : Warning<`.
OK.  I think I understand the behavior you wanted.  I just thought maybe the 
current wording might be technically incorrect.  I wasn't sure how precisely 
defined we consider "extension" to be in this context.


https://reviews.llvm.org/D37042



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

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6031
   InGroup;
+def ext_gnu_null_ptr_arith : Extension<
+  "arithmetic on a null pointer treated as a cast from integer to pointer is a 
GNU extension">,

andrew.w.kaylor wrote:
> efriedma wrote:
> > "extension" isn't really right here; this shouldn't be an error in 
> > -pedantic-errors mode.  Probably best to just stick this into the 
> > NullPointerArithmetic, like the other new warning.
> So how should a word the warning?  Just this:
> 
> "arithmetic on a null pointer treated as a cast from integer to pointer"?
That wasn't what I meant; the current wording is fine. I meant this should be 
something like `def warn_gnu_null_ptr_arith : Warning<`.


https://reviews.llvm.org/D37042



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

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

[Andy Kaylor via Phabricator via cfe-commits]

andrew.w.kaylor added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6031
   InGroup;
+def ext_gnu_null_ptr_arith : Extension<
+  "arithmetic on a null pointer treated as a cast from integer to pointer is a 
GNU extension">,

efriedma wrote:
> "extension" isn't really right here; this shouldn't be an error in 
> -pedantic-errors mode.  Probably best to just stick this into the 
> NullPointerArithmetic, like the other new warning.
So how should a word the warning?  Just this:

"arithmetic on a null pointer treated as a cast from integer to pointer"?


https://reviews.llvm.org/D37042



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

[PATCH] D37804: [OpenCL] Handle address space conversion while setting type alignment

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: lib/CodeGen/CGExpr.cpp:957
 
-return Builder.CreateBitCast(Addr, ConvertType(E->getType()));
+return Builder.CreatePointerBitCastOrAddrSpaceCast(
+Addr, ConvertType(E->getType()));

Better assert that only CK_AddressSpaceConversion allows different addr spaces 
in target type.


https://reviews.llvm.org/D37804



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

[PATCH] D37308: Interface class with uuid base record

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Actually... disregard that... the rule is more complex than that.

Based on some playing around with MSVC on godbolt, it seems that it actually 
marks any type that inherits ONLY from interface types or IUnknown as an 
interface itself.  We may be better off doing that instead.

Additionally, the implementation of "isIUnknown" is actually more complicated 
too, since it contains function declarations.




Comment at: lib/Sema/SemaDeclCXX.cpp:2388
+
+  return Uuid && Uuid->getGuid() =="---C000-0046" &&
+ RD->isStruct() && RD->getName() == "IUnknown" && RD->isEmpty() &&

erichkeane wrote:
> Up to @aaron.ballman, but moving the getGuid string check (a massive 
> string-diff) to 2nd defeats the purpose of moving UUID up front, which is to 
> put the 'easy' things first.  IsTU, isStruct, and isEmpty are likely better.
> 
> Additionally, I'd mentioned it before, this still doesn't check to make sure 
> that the IUnknown doesn't inherit from anywhere, does it?  MSVC also will 
> error if IUnknown's definition includes an inherited type.
Also, "isEmpty" isn't sufficient.  IUnknown actually contains functions as long 
as none have an implementation. (Declarations only).


https://reviews.llvm.org/D37308



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

[PATCH] D37681: [refactor] Simplify the interface and remove some template magic

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

This is very nice! Thanks!

Looks good to me; I'll let Manuel stamp the patch for you.




Comment at: include/clang/Tooling/Refactoring/RefactoringActionRules.h:56
+  virtual Expected
+  createSourceReplacements(RefactoringRuleContext ) = 0;
+

Can this be `private`?



Comment at: include/clang/Tooling/Refactoring/RefactoringActionRules.h:77
+  virtual Expected
+  findSymbolOccurrences(RefactoringRuleContext ) = 0;
 

`private`?


Repository:
  rL LLVM

https://reviews.llvm.org/D37681



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

r313192 - Mark static member functions as static in CodeViewDebug

[Adrian McCarthy via cfe-commits]

Author: amccarth
Date: Wed Sep 13 13:53:55 2017
New Revision: 313192

URL: http://llvm.org/viewvc/llvm-project?rev=313192=rev
Log:
Mark static member functions as static in CodeViewDebug

Summary:
To improve CodeView quality for static member functions, we need to make the
static explicit.  In addition to a small change in LLVM's CodeViewDebug to
return the appropriate MethodKind, this requires a small change in Clang to
note the staticness in the debug info metadata.

Subscribers: aprantl, hiraditya

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

Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/test/CodeGenCXX/debug-info-ms-abi.cpp

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=313192=313191=313192=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Wed Sep 13 13:53:55 2017
@@ -1408,6 +1408,8 @@ llvm::DISubprogram *CGDebugInfo::CreateC
 ContainingType = RecordTy;
   }
 
+  if (Method->isStatic())
+Flags |= llvm::DINode::FlagStaticMember;
   if (Method->isImplicit())
 Flags |= llvm::DINode::FlagArtificial;
   Flags |= getAccessFlag(Method->getAccess(), Method->getParent());

Modified: cfe/trunk/test/CodeGenCXX/debug-info-ms-abi.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-ms-abi.cpp?rev=313192=313191=313192=diff
==
--- cfe/trunk/test/CodeGenCXX/debug-info-ms-abi.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/debug-info-ms-abi.cpp Wed Sep 13 13:53:55 2017
@@ -6,6 +6,7 @@ struct Foo {
   virtual void f();
   virtual void g();
   virtual void h();
+  static void i(int, int);
   struct Nested {};
 };
 Foo f;
@@ -18,7 +19,7 @@ Foo::Nested n;
 // CHECK-SAME: elements: ![[elements:[0-9]+]]
 // CHECK-SAME: identifier: ".?AUFoo@@"
 
-// CHECK: ![[elements]] = !{![[vshape:[0-9]+]], ![[vptr:[0-9]+]], ![[Nested]], 
![[f:[0-9]+]], ![[g:[0-9]+]], ![[h:[0-9]+]]}
+// CHECK: ![[elements]] = !{![[vshape:[0-9]+]], ![[vptr:[0-9]+]], ![[Nested]], 
![[f:[0-9]+]], ![[g:[0-9]+]], ![[h:[0-9]+]], ![[i:[0-9]+]]}
 
 // CHECK: ![[vshape]] = !DIDerivedType(tag: DW_TAG_pointer_type, name: 
"__vtbl_ptr_type", baseType: null, size: 96)
 
@@ -38,3 +39,9 @@ Foo::Nested n;
 // CHECK: ![[h]] = !DISubprogram(name: "h",
 // CHECK-SAME: containingType: ![[Foo]], virtuality: DW_VIRTUALITY_virtual, 
virtualIndex: 2,
 // CHECK-SAME: flags: DIFlagPrototyped | DIFlagIntroducedVirtual,
+
+// CHECK: ![[i]] = !DISubprogram(name: "i",
+// CHECK-SAME: flags: DIFlagPrototyped | DIFlagStaticMember
+// CHECK-NEXT: ![[dummy:[0-9]+]] = !DISubroutineType(types: 
![[Signature:[0-9]+]])
+// CHECK: ![[Signature]] = !{null, ![[BasicInt:[0-9]+]], ![[BasicInt]]}
+// CHECK: ![[BasicInt]] = !DIBasicType(name: "int", size: 32, encoding: 
DW_ATE_signed)


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

[PATCH] D37308: Interface class with uuid base record

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Tighten up the 'IUnknown' check, and do the check I mentioned above, and I 
think this logic is correct.  Searching would be required in the positive case, 
but this is the negative case.




Comment at: lib/Sema/SemaDeclCXX.cpp:2483
+!IsDeclPublicInterface(RD, KnownBase->getAccessSpecifier()) &&
+!IsOrInheritsFromIUnknown(RD)) {
   Diag(KnownBase->getLocStart(), diag::err_invalid_base_in_interface)

So, I just realized... this call here is useless, we got sidetracked because of 
the inversion of logic here.  You ACTUALLY just care that this base is either a 
public interface, OR if it is IUnknown.  This logic can be:

   if (Class->isInterface() && !IsDeclPublicInterface(...) && 
!IsIUnknownType(...)) {...}

The search doesn't actually need to take place, since the current base COULD 
NOT be a valid 'interface' unless it inherited only from interfaces/IUnknown 
anyway.  Unless I'm missing something, you can delete the function 
"IsOrInheritsFromIUnknown" as well.


https://reviews.llvm.org/D37308



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

Re: r304661 - CodeGen: fix section names for different file formats

[Steven Wu via cfe-commits]

Hi Saleem

I just realize there can be an issue with this commit. This breaks the bitcode 
compatibility when LTO linking bitcode file produced by llvm-5.0 vs the older 
versions. Because the objc related module flag has the behavior Module::Error, 
simple whitespace changes will cause libLTO to error upon seeing different 
values.

I wish I caught this earlier so we can fix the issue before 5.0 released. Maybe 
we need another bitcode upgrade path to strip away all the whitespaces in 
"Objective-C Image Info Section" module flag?

Steven


> On Jun 3, 2017, at 9:18 AM, Saleem Abdulrasool via cfe-commits 
>  wrote:
> 
> Author: compnerd
> Date: Sat Jun  3 11:18:09 2017
> New Revision: 304661
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=304661=rev
> Log:
> CodeGen: fix section names for different file formats
> 
> This changes the codegen to match the section names according to the
> ObjC rewriter as well as the runtime.  The changes to the test are
> simply whitespace changes to the section attributes and names and are
> functionally equivalent (the whitespace is ignored by the linker).
> 
> Added:
>cfe/trunk/test/CodeGenObjC/sections.m
> Modified:
>cfe/trunk/lib/CodeGen/CGObjCMac.cpp
>cfe/trunk/test/CodeGenObjC/exceptions-asm-attribute.m
>cfe/trunk/test/CodeGenObjC/image-info.m
>cfe/trunk/test/CodeGenObjC/metadata-symbols-64.m
>cfe/trunk/test/CodeGenObjC/metadata_symbols.m
>cfe/trunk/test/CodeGenObjC/non-lazy-classes.m
> 
> Modified: cfe/trunk/lib/CodeGen/CGObjCMac.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjCMac.cpp?rev=304661=304660=304661=diff
> ==
> --- cfe/trunk/lib/CodeGen/CGObjCMac.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGObjCMac.cpp Sat Jun  3 11:18:09 2017
> @@ -1004,6 +1004,8 @@ protected:
>   const ObjCInterfaceDecl *ID,
>   ObjCCommonTypesHelper );
> 
> +  std::string GetSectionName(StringRef Section, StringRef MachOAttributes);
> +
> public:
>   /// CreateMetadataVar - Create a global variable with internal
>   /// linkage for use by the Objective-C runtime.
> @@ -4786,6 +4788,27 @@ llvm::Value *CGObjCMac::EmitIvarOffset(C
> 
> /* *** Private Interface *** */
> 
> +std::string CGObjCCommonMac::GetSectionName(StringRef Section,
> +StringRef MachOAttributes) {
> +  switch (CGM.getTriple().getObjectFormat()) {
> +  default:
> +llvm_unreachable("unexpected object file format");
> +  case llvm::Triple::MachO: {
> +if (MachOAttributes.empty())
> +  return ("__DATA," + Section).str();
> +return ("__DATA," + Section + "," + MachOAttributes).str();
> +  }
> +  case llvm::Triple::ELF:
> +assert(Section.substr(0, 2) == "__" &&
> +   "expected the name to begin with __");
> +return Section.substr(2).str();
> +  case llvm::Triple::COFF:
> +assert(Section.substr(0, 2) == "__" &&
> +   "expected the name to begin with __");
> +return ("." + Section.substr(2) + "$B").str();
> +  }
> +}
> +
> /// EmitImageInfo - Emit the image info marker used to encode some module
> /// level information.
> ///
> @@ -4809,9 +4832,10 @@ enum ImageInfoFlags {
> 
> void CGObjCCommonMac::EmitImageInfo() {
>   unsigned version = 0; // Version is unused?
> -  const char *Section = (ObjCABI == 1) ?
> -"__OBJC, __image_info,regular" :
> -"__DATA, __objc_imageinfo, regular, no_dead_strip";
> +  std::string Section =
> +  (ObjCABI == 1)
> +  ? "__OBJC,__image_info,regular"
> +  : GetSectionName("__objc_imageinfo", "regular,no_dead_strip");
> 
>   // Generate module-level named metadata to convey this information to the
>   // linker and code-gen.
> @@ -4822,7 +4846,7 @@ void CGObjCCommonMac::EmitImageInfo() {
>   Mod.addModuleFlag(llvm::Module::Error, "Objective-C Image Info Version",
> version);
>   Mod.addModuleFlag(llvm::Module::Error, "Objective-C Image Info Section",
> -llvm::MDString::get(VMContext,Section));
> +llvm::MDString::get(VMContext, Section));
> 
>   if (CGM.getLangOpts().getGC() == LangOptions::NonGC) {
> // Non-GC overrides those files which specify GC.
> @@ -5930,17 +5954,21 @@ void CGObjCNonFragileABIMac::FinishNonFr
>   }
> 
>   AddModuleClassList(DefinedClasses, "OBJC_LABEL_CLASS_$",
> - "__DATA, __objc_classlist, regular, no_dead_strip");
> + GetSectionName("__objc_classlist",
> +"regular,no_dead_strip"));
> 
>   AddModuleClassList(DefinedNonLazyClasses, "OBJC_LABEL_NONLAZY_CLASS_$",
> - "__DATA, __objc_nlclslist, regular, no_dead_strip");
> + GetSectionName("__objc_nlclslist",
> +"regular,no_dead_strip"));
> 
>   // Build list of all 

r313186 - [OPENMP] Fix types for the target specific parameters in debug mode.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Wed Sep 13 13:20:59 2017
New Revision: 313186

URL: http://llvm.org/viewvc/llvm-project?rev=313186=rev
Log:
[OPENMP] Fix types for the target specific parameters in debug mode.

Used incorrect types for target specific parameters in debug mode,
should use original pointers rather than the pointee types.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/test/OpenMP/target_parallel_debug_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp?rev=313186=313185=313186=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Wed Sep 13 13:20:59 2017
@@ -2274,9 +2274,9 @@ CGOpenMPRuntimeNVPTX::getParameterAddres
   QualType NativePointeeTy = cast(NonQualTy)->getPointeeType();
   unsigned NativePointeeAddrSpace =
   NativePointeeTy.getQualifiers().getAddressSpace();
-  QualType TargetPointeeTy = TargetParam->getType()->getPointeeType();
+  QualType TargetTy = TargetParam->getType();
   llvm::Value *TargetAddr = CGF.EmitLoadOfScalar(
-  LocalAddr, /*Volatile=*/false, TargetPointeeTy, SourceLocation());
+  LocalAddr, /*Volatile=*/false, TargetTy, SourceLocation());
   // First cast to generic.
   TargetAddr = CGF.Builder.CreatePointerBitCastOrAddrSpaceCast(
   TargetAddr, TargetAddr->getType()->getPointerElementType()->getPointerTo(
@@ -2287,7 +2287,7 @@ CGOpenMPRuntimeNVPTX::getParameterAddres
   NativePointeeAddrSpace));
   Address NativeParamAddr = CGF.CreateMemTemp(NativeParamType);
   CGF.EmitStoreOfScalar(TargetAddr, NativeParamAddr, /*Volatile=*/false,
-NativeParam->getType());
+NativeParamType);
   return NativeParamAddr;
 }
 

Modified: cfe/trunk/test/OpenMP/target_parallel_debug_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/target_parallel_debug_codegen.cpp?rev=313186=313185=313186=diff
==
--- cfe/trunk/test/OpenMP/target_parallel_debug_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/target_parallel_debug_codegen.cpp Wed Sep 13 13:20:59 
2017
@@ -5,11 +5,13 @@
 int main() {
   /* int(*b)[a]; */
   /* int *(**c)[a]; */
+  bool bb;
   int a;
   int b[10][10];
   int c[10][10][10];
-#pragma omp target parallel firstprivate(a, b) map(tofrom \
-   : c)
+#pragma omp target parallel firstprivate(a, b) map(tofrom  \
+   : c) map(tofrom \
+: bb)
   {
 int  = c[1][1][1];
 int  = a;
@@ -19,9 +21,11 @@ int main() {
 b[0][a] = 10;
 c[0][0][a] = 11;
 b[0][a] = c[0][0][a];
+bb |= b[0][a];
   }
-#pragma omp target parallel firstprivate(a) map(tofrom \
-: c, b)
+#pragma omp target parallel firstprivate(a) map(tofrom \
+: c, b) map(to \
+: bb)
   {
 int  = c[1][1][1];
 int  = a;
@@ -31,9 +35,11 @@ int main() {
 b[0][a] = 10;
 c[0][0][a] = 11;
 b[0][a] = c[0][0][a];
+d = bb;
   }
-#pragma omp target parallel map(tofrom \
-: a, c, b)
+#pragma omp target parallel map(tofrom  \
+: a, c, b) map(from \
+   : bb)
   {
 int  = c[1][1][1];
 int  = a;
@@ -43,64 +49,65 @@ int main() {
 b[0][a] = 10;
 c[0][0][a] = 11;
 b[0][a] = c[0][0][a];
+bb = b[0][a];
   }
   return 0;
 }
 
-// CHECK: define internal void @__omp_offloading{{[^(]+}}([10 x [10 x [10 x 
i32]]] addrspace(1)* {{[^,]+}}, i32 {{[^,]+}}, [10 x [10 x i32]]* {{[^)]+}})
+// CHECK: define internal void @__omp_offloading{{[^(]+}}([10 x [10 x [10 x 
i32]]] addrspace(1)* {{[^,]+}}, i32 {{[^,]+}}, [10 x [10 x i32]]* {{[^,]+}}, i8 
addrspace(1)* noalias{{[^)]+}})
 // CHECK: addrspacecast [10 x [10 x [10 x i32]]] addrspace(1)* %{{.+}} to [10 
x [10 x [10 x i32]]]*
-// CHECK: call void [[NONDEBUG_WRAPPER:.+]](i32* {{[^,]+}}, i32* {{[^,]+}}, 
[10 x [10 x [10 x i32]]]* {{[^,]+}}, i64 {{[^,]+}}, [10 x [10 x i32]]* 
{{[^)]+}})
+// CHECK: call void [[NONDEBUG_WRAPPER:.+]](i32* {{[^,]+}}, i32* {{[^,]+}}, 
[10 x [10 x [10 x i32]]]* {{[^,]+}}, i64 {{[^,]+}}, [10 x [10 x i32]]* 
{{[^,]+}}, i8* {{[^)]+}})
 
-// CHECK: define internal void [[DEBUG_PARALLEL:@.+]](i32* {{[^,]+}}, i32* 
{{[^,]+}}, [10 x [10 x [10 x i32]]] addrspace(1)* noalias{{[^,]+}}, i32 
{{[^,]+}}, [10 x [10 x i32]]* noalias{{[^)]+}})
+// CHECK: define internal void [[DEBUG_PARALLEL:@.+]](i32* {{[^,]+}}, i32* 
{{[^,]+}}, [10 x [10 x [10 x 

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6031
   InGroup;
+def ext_gnu_null_ptr_arith : Extension<
+  "arithmetic on a null pointer treated as a cast from integer to pointer is a 
GNU extension">,

"extension" isn't really right here; this shouldn't be an error in 
-pedantic-errors mode.  Probably best to just stick this into the 
NullPointerArithmetic, like the other new warning.


https://reviews.llvm.org/D37042



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

[PATCH] D37491: [Preamble] Fixed preamble breaking with BOM presence (and particularly, fluctuating BOM presence)

[Cameron via Phabricator via cfe-commits]

cameron314 updated this revision to Diff 115104.
cameron314 edited the summary of this revision.
cameron314 added a comment.

Alright, I've changed the patch so that the preamble takes into account the BOM 
presence and is invalidated when it changes. This automatically fixes all 
clients of `PrecompiledPreamble`, and ensures that all `SourceLocation`s are 
always consistent when using a PCH generated from the preamble.

I think this should do the trick!


https://reviews.llvm.org/D37491

Files:
  include/clang/Frontend/PrecompiledPreamble.h
  include/clang/Lex/Lexer.h
  include/clang/Lex/PreprocessorOptions.h
  lib/Frontend/FrontendActions.cpp
  lib/Frontend/PrecompiledPreamble.cpp
  lib/Lex/Lexer.cpp
  lib/Lex/Preprocessor.cpp
  unittests/Frontend/PCHPreambleTest.cpp

Index: unittests/Frontend/PCHPreambleTest.cpp
===
--- unittests/Frontend/PCHPreambleTest.cpp
+++ unittests/Frontend/PCHPreambleTest.cpp
@@ -153,4 +153,48 @@
   ASSERT_EQ(initialCounts[2], GetFileReadCount(Header2));
 }
 
+TEST_F(PCHPreambleTest, ParseWithBom) {
+  std::string Header = "//./header.h";
+  std::string Main = "//./main.cpp";
+  AddFile(Header, "int random() { return 4; }");
+  AddFile(Main,
+"\xef\xbb\xbf"
+"#include \"//./header.h\"\n"
+"int main() { return random() -2; }");
+
+  std::unique_ptr AST(ParseAST(Main));
+  ASSERT_TRUE(AST.get());
+  ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+
+  unsigned HeaderReadCount = GetFileReadCount(Header);
+
+  ASSERT_TRUE(ReparseAST(AST));
+  ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+  
+  // Check preamble PCH was really reused
+  ASSERT_EQ(HeaderReadCount, GetFileReadCount(Header));
+
+  // Remove BOM
+  RemapFile(Main,
+"#include \"//./header.h\"\n"
+"int main() { return random() -2; }");
+
+  ASSERT_TRUE(ReparseAST(AST));
+  ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+
+  ASSERT_LE(HeaderReadCount, GetFileReadCount(Header));
+  HeaderReadCount = GetFileReadCount(Header);
+
+  // Add BOM back
+  RemapFile(Main,
+"\xef\xbb\xbf"
+"#include \"//./header.h\"\n"
+"int main() { return random() -2; }");
+
+  ASSERT_TRUE(ReparseAST(AST));
+  ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+
+  ASSERT_LE(HeaderReadCount, GetFileReadCount(Header));
+}
+
 } // anonymous namespace
Index: lib/Lex/Preprocessor.cpp
===
--- lib/Lex/Preprocessor.cpp
+++ lib/Lex/Preprocessor.cpp
@@ -516,9 +516,9 @@
 // If we've been asked to skip bytes in the main file (e.g., as part of a
 // precompiled preamble), do so now.
 if (SkipMainFilePreamble.first > 0)
-  CurLexer->SkipBytes(SkipMainFilePreamble.first, 
-  SkipMainFilePreamble.second);
-
+  CurLexer->SetByteOffset(SkipMainFilePreamble.first,
+  SkipMainFilePreamble.second);
+
 // Tell the header info that the main file was entered.  If the file is later
 // #imported, it won't be re-entered.
 if (const FileEntry *FE = SourceMgr.getFileEntryForID(MainFileID))
Index: lib/Lex/Lexer.cpp
===
--- lib/Lex/Lexer.cpp
+++ lib/Lex/Lexer.cpp
@@ -552,9 +552,9 @@
 
 } // end anonymous namespace
 
-std::pair Lexer::ComputePreamble(StringRef Buffer,
- const LangOptions ,
- unsigned MaxLines) {
+PreambleBounds Lexer::ComputePreamble(StringRef Buffer,
+  const LangOptions ,
+  unsigned MaxLines) {
   // Create a lexer starting at the beginning of the file. Note that we use a
   // "fake" file source location at offset 1 so that the lexer will track our
   // position within the file.
@@ -688,7 +688,8 @@
   else
 End = TheTok.getLocation();
 
-  return std::make_pair(End.getRawEncoding() - StartLoc.getRawEncoding(),
+  return PreambleBounds(StartLoc.getRawEncoding() - FileLoc.getRawEncoding(),
+End.getRawEncoding() - StartLoc.getRawEncoding(),
 TheTok.isAtStartOfLine());
 }
 
@@ -1394,9 +1395,9 @@
 // Helper methods for lexing.
 //===--===//
 
-/// \brief Routine that indiscriminately skips bytes in the source file.
-void Lexer::SkipBytes(unsigned Bytes, bool StartOfLine) {
-  BufferPtr += Bytes;
+/// \brief Routine that indiscriminately sets the offset into the source file.
+void Lexer::SetByteOffset(unsigned Offset, bool StartOfLine) {
+  BufferPtr = BufferStart + Offset;
   if (BufferPtr > BufferEnd)
 BufferPtr = BufferEnd;
   // FIXME: What exactly does the StartOfLine bit mean?  There are two
Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- 

r313182 - SplitEmptyFunction should be true in the Mozilla coding style

[Sylvestre Ledru via cfe-commits]

Author: sylvestre
Date: Wed Sep 13 13:03:29 2017
New Revision: 313182

URL: http://llvm.org/viewvc/llvm-project?rev=313182=rev
Log:
SplitEmptyFunction should be true in the Mozilla coding style

Summary:
As defined here: 
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Classes
See for the downstream bug report: 
https://bugzilla.mozilla.org/show_bug.cgi?id=1399359

Reviewers: Typz, djasper

Reviewed By: Typz

Subscribers: klimek, cfe-commits

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

Modified:
cfe/trunk/lib/Format/Format.cpp

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=313182=313181=313182=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Wed Sep 13 13:03:29 2017
@@ -515,7 +515,7 @@ static FormatStyle expandPresets(const F
 Expanded.BraceWrapping.AfterFunction = true;
 Expanded.BraceWrapping.AfterStruct = true;
 Expanded.BraceWrapping.AfterUnion = true;
-Expanded.BraceWrapping.SplitEmptyFunction = false;
+Expanded.BraceWrapping.SplitEmptyFunction = true;
 Expanded.BraceWrapping.SplitEmptyRecord = false;
 break;
   case FormatStyle::BS_Stroustrup:


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

Re: r313104 - Revert "[Driver] MinGW: Remove custom linker detection"

[Rui Ueyama via cfe-commits]

Ah, sorry, you mentioned that. Yes, I think you need it.

On Wed, Sep 13, 2017 at 1:00 PM, Rui Ueyama  wrote:

> This is a wild guess, but don't you need to add `REQUIRES: lld` to your
> test if your test depends on lld? I don't think all bots have lld, and if
> `ld.lld` is not found, clang prints out that error message.
>
> On Tue, Sep 12, 2017 at 7:16 PM, Martell Malone 
> wrote:
>
>> Just to follow up,
>> The test case here was added by yaron in https://reviews.llvm.org/rL253
>> 066
>> As a follow up to the original detection which I originally wrote and
>> reid reviewed https://reviews.llvm.org/rL242121
>> Given that no other target does special checks except msvc for lld-link
>> removing this test case might also be an option.
>>
>> On Wed, Sep 13, 2017 at 2:54 AM, Martell Malone 
>> wrote:
>>
>>> Hey Reid, Rui,
>>>
>>> I had to revert this twice. I'm not sure where the configurations for
>>> the build bots are but there are 2 issues as I mentioned in the commit
>>> message
>>>
>>> Looking through the clang test directory I only se -fuse-ld=lld tested
>>> in 4 locations
>>>
>>> test/Driver/windows-cross.c does some really old tests like
>>> `-fuse-ld=lld-link2` which was back when Rui had just started the new COFF
>>> linker. (maybe we should remove / update these ? )
>>> test/Driver/cl-link.c which checks that for cl -fuse-ld=lld is converted
>>> to lld-link and not ld.lld
>>> test/Driver/cl-options.c just checks an error showing LTO that requires
>>> -fuse-ld=lld
>>> and in
>>> test/Driver/mingw-useld.c where we run into our errors.
>>>
>>> So to begin there does not seem to be any checking for lld in the test
>>> suite because it is not properly tested anywhere (from a clang perspective).
>>>
>>> For the first error `error: invalid linker name in argument
>>> '-fuse-ld=lld'`
>>> I'm not sure exactly how to add requirements to the lit tooling, I did a
>>> few greps build only came up with target checks.
>>> but do you know where I would look to implement something like this?
>>> `// REQUIRES: lld`
>>>
>>> As for the second issue.
>>>
>>> lld-link.exe: warning: ignoring unknown argument: -fuse-ld=lld
>>>
>>> I can't confirm but it seems like a configuration with the buildbot
>>> itself.
>>> Possibly related to setting LLD_SYMLINKS_TO_CREATE within the bot itself
>>> or some bat file on the system itself for symlinking?
>>>
>>> I don't think it is clang itself that is doing this because the only
>>> place where lld is remapped to lld-link is in lib/Driver/ToolChains/MSVC.
>>> cpp
>>> and that is only for msvc targets with a testcase
>>> in test/Driver/cl-link.c
>>>
>>> Best,
>>> Martell
>>>
>>>
>>> On Wed, Sep 13, 2017 at 1:57 AM, Martell Malone via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 Author: martell
 Date: Tue Sep 12 17:57:50 2017
 New Revision: 313104

 URL: http://llvm.org/viewvc/llvm-project?rev=313104=rev
 Log:
 Revert "[Driver] MinGW: Remove custom linker detection"

 This reverts rL313102 because it still fails some build bot tests.

 On many linux bots it fails with the following error.
 error: invalid linker name in argument '-fuse-ld=lld'
 and on some windows bots also because there is no ld.lld.exe
 lld-link.exe: warning: ignoring unknown argument: -fuse-ld=lld

 Modified:
 cfe/trunk/lib/Driver/ToolChains/MinGW.cpp
 cfe/trunk/test/Driver/mingw-useld.c

 Modified: cfe/trunk/lib/Driver/ToolChains/MinGW.cpp
 URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Too
 lChains/MinGW.cpp?rev=313104=313103=313104=diff
 
 ==
 --- cfe/trunk/lib/Driver/ToolChains/MinGW.cpp (original)
 +++ cfe/trunk/lib/Driver/ToolChains/MinGW.cpp Tue Sep 12 17:57:50 2017
 @@ -104,6 +104,14 @@ void tools::MinGW::Linker::ConstructJob(
// handled somewhere else.
Args.ClaimAllArgs(options::OPT_w);

 +  StringRef LinkerName = Args.getLastArgValue(options::OPT_fuse_ld_EQ,
 "ld");
 +  if (LinkerName.equals_lower("lld")) {
 +CmdArgs.push_back("-flavor");
 +CmdArgs.push_back("gnu");
 +  } else if (!LinkerName.equals_lower("ld")) {
 +D.Diag(diag::err_drv_unsupported_linker) << LinkerName;
 +  }
 +
if (!D.SysRoot.empty())
  CmdArgs.push_back(Args.MakeArgString("--sysroot=" + D.SysRoot));

 @@ -233,7 +241,7 @@ void tools::MinGW::Linker::ConstructJob(

if (Args.hasArg(options::OPT_static))
  CmdArgs.push_back("--end-group");
 -  else
 +  else if (!LinkerName.equals_lower("lld"))
  AddLibGCC(Args, CmdArgs);
  }

 @@ -244,7 +252,7 @@ void tools::MinGW::Linker::ConstructJob(
CmdArgs.push_back(Args.MakeArgString(TC.GetFilePath("crtend
 .o")));
  }
}
 -  

Re: r313104 - Revert "[Driver] MinGW: Remove custom linker detection"

[Rui Ueyama via cfe-commits]

This is a wild guess, but don't you need to add `REQUIRES: lld` to your
test if your test depends on lld? I don't think all bots have lld, and if
`ld.lld` is not found, clang prints out that error message.

On Tue, Sep 12, 2017 at 7:16 PM, Martell Malone 
wrote:

> Just to follow up,
> The test case here was added by yaron in https://reviews.llvm.org/rL253066
> As a follow up to the original detection which I originally wrote and reid
> reviewed https://reviews.llvm.org/rL242121
> Given that no other target does special checks except msvc for lld-link
> removing this test case might also be an option.
>
> On Wed, Sep 13, 2017 at 2:54 AM, Martell Malone 
> wrote:
>
>> Hey Reid, Rui,
>>
>> I had to revert this twice. I'm not sure where the configurations for the
>> build bots are but there are 2 issues as I mentioned in the commit message
>>
>> Looking through the clang test directory I only se -fuse-ld=lld tested in
>> 4 locations
>>
>> test/Driver/windows-cross.c does some really old tests like
>> `-fuse-ld=lld-link2` which was back when Rui had just started the new COFF
>> linker. (maybe we should remove / update these ? )
>> test/Driver/cl-link.c which checks that for cl -fuse-ld=lld is converted
>> to lld-link and not ld.lld
>> test/Driver/cl-options.c just checks an error showing LTO that requires
>> -fuse-ld=lld
>> and in
>> test/Driver/mingw-useld.c where we run into our errors.
>>
>> So to begin there does not seem to be any checking for lld in the test
>> suite because it is not properly tested anywhere (from a clang perspective).
>>
>> For the first error `error: invalid linker name in argument
>> '-fuse-ld=lld'`
>> I'm not sure exactly how to add requirements to the lit tooling, I did a
>> few greps build only came up with target checks.
>> but do you know where I would look to implement something like this?
>> `// REQUIRES: lld`
>>
>> As for the second issue.
>>
>> lld-link.exe: warning: ignoring unknown argument: -fuse-ld=lld
>>
>> I can't confirm but it seems like a configuration with the buildbot
>> itself.
>> Possibly related to setting LLD_SYMLINKS_TO_CREATE within the bot itself
>> or some bat file on the system itself for symlinking?
>>
>> I don't think it is clang itself that is doing this because the only
>> place where lld is remapped to lld-link is in lib/Driver/ToolChains/MSVC.
>> cpp
>> and that is only for msvc targets with a testcase in test/Driver/cl-link.c
>>
>> Best,
>> Martell
>>
>>
>> On Wed, Sep 13, 2017 at 1:57 AM, Martell Malone via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: martell
>>> Date: Tue Sep 12 17:57:50 2017
>>> New Revision: 313104
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=313104=rev
>>> Log:
>>> Revert "[Driver] MinGW: Remove custom linker detection"
>>>
>>> This reverts rL313102 because it still fails some build bot tests.
>>>
>>> On many linux bots it fails with the following error.
>>> error: invalid linker name in argument '-fuse-ld=lld'
>>> and on some windows bots also because there is no ld.lld.exe
>>> lld-link.exe: warning: ignoring unknown argument: -fuse-ld=lld
>>>
>>> Modified:
>>> cfe/trunk/lib/Driver/ToolChains/MinGW.cpp
>>> cfe/trunk/test/Driver/mingw-useld.c
>>>
>>> Modified: cfe/trunk/lib/Driver/ToolChains/MinGW.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Too
>>> lChains/MinGW.cpp?rev=313104=313103=313104=diff
>>> 
>>> ==
>>> --- cfe/trunk/lib/Driver/ToolChains/MinGW.cpp (original)
>>> +++ cfe/trunk/lib/Driver/ToolChains/MinGW.cpp Tue Sep 12 17:57:50 2017
>>> @@ -104,6 +104,14 @@ void tools::MinGW::Linker::ConstructJob(
>>>// handled somewhere else.
>>>Args.ClaimAllArgs(options::OPT_w);
>>>
>>> +  StringRef LinkerName = Args.getLastArgValue(options::OPT_fuse_ld_EQ,
>>> "ld");
>>> +  if (LinkerName.equals_lower("lld")) {
>>> +CmdArgs.push_back("-flavor");
>>> +CmdArgs.push_back("gnu");
>>> +  } else if (!LinkerName.equals_lower("ld")) {
>>> +D.Diag(diag::err_drv_unsupported_linker) << LinkerName;
>>> +  }
>>> +
>>>if (!D.SysRoot.empty())
>>>  CmdArgs.push_back(Args.MakeArgString("--sysroot=" + D.SysRoot));
>>>
>>> @@ -233,7 +241,7 @@ void tools::MinGW::Linker::ConstructJob(
>>>
>>>if (Args.hasArg(options::OPT_static))
>>>  CmdArgs.push_back("--end-group");
>>> -  else
>>> +  else if (!LinkerName.equals_lower("lld"))
>>>  AddLibGCC(Args, CmdArgs);
>>>  }
>>>
>>> @@ -244,7 +252,7 @@ void tools::MinGW::Linker::ConstructJob(
>>>CmdArgs.push_back(Args.MakeArgString(TC.GetFilePath("crtend
>>> .o")));
>>>  }
>>>}
>>> -  const char *Exec = Args.MakeArgString(TC.GetLinkerPath());
>>> +  const char *Exec = Args.MakeArgString(TC.GetProgr
>>> amPath(LinkerName.data()));
>>>C.addCommand(llvm::make_unique(JA, *this, Exec, CmdArgs,
>>> Inputs));
>>>  }
>>>
>>>
>>> Modified: 

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

[Andy Kaylor via Phabricator via cfe-commits]

andrew.w.kaylor added a comment.

Does anything else need to be done for this to be ready to land?


https://reviews.llvm.org/D37042



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

Re: [libcxx] r250235 - [libcxx] Capture configuration information when installing the libc++ headers

[Richard Smith via cfe-commits]

On 13 September 2017 at 12:15, Eric Fiselier via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Wed, Sep 13, 2017 at 1:02 PM, Richard Smith 
> wrote:
>
>> On 13 October 2015 at 15:12, Eric Fiselier via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: ericwf
>>> Date: Tue Oct 13 17:12:02 2015
>>> New Revision: 250235
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=250235=rev
>>> Log:
>>> [libcxx] Capture configuration information when installing the libc++
>>> headers
>>>
>>> Summary:
>>> Hi all,
>>>
>>> This patch is a successor to D11963. However it has changed dramatically
>>> and I felt it would be best to start a new review thread.
>>>
>>> Please read the design documentation added in this patch for a
>>> description of how it works.
>>>
>>> Reviewers: mclow.lists, danalbert, jroelofs, EricWF
>>>
>>> Subscribers: vkalintiris, rnk, ed, espositofulvio, asl, eugenis,
>>> cfe-commits
>>>
>>> Differential Revision: http://reviews.llvm.org/D13407
>>>
>>> Added:
>>> libcxx/trunk/docs/DesignDocs/
>>> libcxx/trunk/docs/DesignDocs/CapturingConfigInfo.rst
>>> libcxx/trunk/include/__config_site.in
>>> Modified:
>>> libcxx/trunk/CMakeLists.txt
>>> libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake
>>> libcxx/trunk/docs/index.rst
>>> libcxx/trunk/include/CMakeLists.txt
>>>
>>> Modified: libcxx/trunk/CMakeLists.txt
>>> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/CMakeLists.
>>> txt?rev=250235=250234=250235=diff
>>> 
>>> ==
>>> --- libcxx/trunk/CMakeLists.txt (original)
>>> +++ libcxx/trunk/CMakeLists.txt Tue Oct 13 17:12:02 2015
>>> @@ -260,13 +260,6 @@ endif()
>>>
>>>  # Feature flags ==
>>> =
>>>  define_if(MSVC -D_CRT_SECURE_NO_WARNINGS)
>>> -define_if_not(LIBCXX_ENABLE_GLOBAL_FILESYSTEM_NAMESPACE
>>> -D_LIBCPP_HAS_NO_GLOBAL_FILESYSTEM_NAMESPACE)
>>> -define_if_not(LIBCXX_ENABLE_STDIN -D_LIBCPP_HAS_NO_STDIN)
>>> -define_if_not(LIBCXX_ENABLE_STDOUT -D_LIBCPP_HAS_NO_STDOUT)
>>> -define_if_not(LIBCXX_ENABLE_THREADS -D_LIBCPP_HAS_NO_THREADS)
>>> -define_if_not(LIBCXX_ENABLE_MONOTONIC_CLOCK
>>> -D_LIBCPP_HAS_NO_MONOTONIC_CLOCK)
>>> -define_if_not(LIBCXX_ENABLE_THREAD_UNSAFE_C_FUNCTIONS
>>> -D_LIBCPP_HAS_NO_THREAD_UNSAFE_C_FUNCTIONS)
>>> -
>>>
>>>  # Sanitizer flags ==
>>> ===
>>>
>>> @@ -303,6 +296,22 @@ if (LIBCXX_BUILT_STANDALONE)
>>>  message(WARNING "LLVM_USE_SANITIZER is not supported on this
>>> platform.")
>>>endif()
>>>  endif()
>>> +
>>> +# Configuration file flags ==
>>> ===
>>> +config_define_if_not(LIBCXX_ENABLE_GLOBAL_FILESYSTEM_NAMESPACE
>>> _LIBCPP_HAS_NO_GLOBAL_FILESYSTEM_NAMESPACE)
>>> +config_define_if_not(LIBCXX_ENABLE_STDIN _LIBCPP_HAS_NO_STDIN)
>>> +config_define_if_not(LIBCXX_ENABLE_STDOUT _LIBCPP_HAS_NO_STDOUT)
>>> +config_define_if_not(LIBCXX_ENABLE_THREADS _LIBCPP_HAS_NO_THREADS)
>>> +config_define_if_not(LIBCXX_ENABLE_MONOTONIC_CLOCK
>>> _LIBCPP_HAS_NO_MONOTONIC_CLOCK)
>>> +config_define_if_not(LIBCXX_ENABLE_THREAD_UNSAFE_C_FUNCTIONS
>>> _LIBCPP_HAS_NO_THREAD_UNSAFE_C_FUNCTIONS)
>>> +
>>> +if (LIBCXX_NEEDS_SITE_CONFIG)
>>> +  configure_file(
>>> +include/__config_site.in
>>> +${LIBCXX_BINARY_DIR}/__config_site
>>> +@ONLY)
>>> +endif()
>>> +
>>>  #==
>>> =
>>>  # Setup Source Code And Tests
>>>  #==
>>> =
>>>
>>> Modified: libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake
>>> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/cmake/Modul
>>> es/HandleLibcxxFlags.cmake?rev=250235=250234=250235=diff
>>> 
>>> ==
>>> --- libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake (original)
>>> +++ libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake Tue Oct 13
>>> 17:12:02 2015
>>> @@ -49,6 +49,22 @@ macro(define_if_not condition def)
>>>endif()
>>>  endmacro()
>>>
>>> +macro(config_define_if condition def)
>>> +  if (${condition})
>>> +set(${def} ON)
>>> +add_definitions(-D${def})
>>> +set(LIBCXX_NEEDS_SITE_CONFIG ON)
>>> +  endif()
>>> +endmacro()
>>> +
>>> +macro(config_define_if_not condition def)
>>> +  if (NOT ${condition})
>>> +set(${def} ON)
>>> +add_definitions(-D${def})
>>> +set(LIBCXX_NEEDS_SITE_CONFIG ON)
>>> +  endif()
>>> +endmacro()
>>> +
>>>  # Add a specified list of flags to both 'LIBCXX_COMPILE_FLAGS' and
>>>  # 'LIBCXX_LINK_FLAGS'.
>>>  macro(add_flags)
>>>
>>> Added: libcxx/trunk/docs/DesignDocs/CapturingConfigInfo.rst
>>> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/docs/Design
>>> Docs/CapturingConfigInfo.rst?rev=250235=auto
>>> 

[PATCH] D37822: [OpenCL] Clean up and add missing fields for block struct

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.

Currently block is translated to a structure equivalent to

  struct Block {
void *isa;
int flags;
int reserved;
void *invoke;
void *descriptor;
  };

Except `invoke`, which is the pointer to the block invoke function,
all other fields are useless for OpenCL, which clutter the IR and
also waste memory since the block struct is passed to the block
invoke function as argument.

On the other hand, the size and alignment of the block struct is
not stored in the struct, which causes difficulty to implement
__enqueue_kernel as library function, since the library function
needs to know the size and alignment of the argument which needs
to be passed to the kernel.

This patch removes the useless fields from the block struct and adds
size and align fields. The equivalent block struct will become

  struct Block {
int size;
int align;
generic void *invoke;
  };

It also changes the pointer to the invoke function to be
a generic pointer since the address space of a function
may not be private on certain targets.


https://reviews.llvm.org/D37822

Files:
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGOpenCLRuntime.cpp
  lib/CodeGen/CGOpenCLRuntime.h
  test/CodeGen/blocks-opencl.cl
  test/CodeGenOpenCL/blocks.cl
  test/CodeGenOpenCL/cl20-device-side-enqueue.cl

Index: test/CodeGenOpenCL/cl20-device-side-enqueue.cl
===
--- test/CodeGenOpenCL/cl20-device-side-enqueue.cl
+++ test/CodeGenOpenCL/cl20-device-side-enqueue.cl
@@ -7,7 +7,7 @@
 typedef struct {int a;} ndrange_t;
 
 // N.B. The check here only exists to set BL_GLOBAL
-// COMMON: @block_G =  addrspace(1) constant void (i8 addrspace(3)*) addrspace(4)* addrspacecast (void (i8 addrspace(3)*) addrspace(1)* bitcast ({ i8**, i32, i32, i8*, %struct.__block_descriptor addrspace(2)* } addrspace(1)* [[BL_GLOBAL:@__block_literal_global(\.[0-9]+)?]] to void (i8 addrspace(3)*) addrspace(1)*) to void (i8 addrspace(3)*) addrspace(4)*)
+// COMMON: @block_G =  addrspace(1) constant void (i8 addrspace(3)*) addrspace(4)* addrspacecast (void (i8 addrspace(3)*) addrspace(1)* bitcast ({ i32, i32, i8 addrspace(4)* } addrspace(1)* [[BL_GLOBAL:@__block_literal_global(\.[0-9]+)?]] to void (i8 addrspace(3)*) addrspace(1)*) to void (i8 addrspace(3)*) addrspace(4)*)
 const bl_t block_G = (bl_t) ^ (local void *a) {};
 
 kernel void device_side_enqueue(global int *a, global int *b, int i) {
@@ -27,9 +27,10 @@
   // COMMON: [[NDR:%[a-z0-9]+]] = alloca %struct.ndrange_t, align 4
   // COMMON: [[DEF_Q:%[0-9]+]] = load %opencl.queue_t{{.*}}*, %opencl.queue_t{{.*}}** %default_queue
   // COMMON: [[FLAGS:%[0-9]+]] = load i32, i32* %flags
-  // COMMON: [[BL:%[0-9]+]] = bitcast <{ i8*, i32, i32, i8*, %struct.__block_descriptor addrspace(2)*, i32{{.*}}, i32{{.*}}, i32{{.*}} }>* %block to void ()*
+  // B32: [[BL:%[0-9]+]] = bitcast <{ i32, i32, i8 addrspace(4)*, i32 addrspace(1)*, i32, i32 addrspace(1)* }>* %block to void ()*
+  // B64: [[BL:%[0-9]+]] = bitcast <{ i32, i32, i8 addrspace(4)*, i32 addrspace(1)*, i32 addrspace(1)*, i32 }>* %block to void ()*
   // COMMON: [[BL_I8:%[0-9]+]] = addrspacecast void ()* [[BL]] to i8 addrspace(4)*
-  // COMMON: call i32 @__enqueue_kernel_basic(%opencl.queue_t{{.*}}* [[DEF_Q]], i32 [[FLAGS]], %struct.ndrange_t* byval [[NDR]]{{(.[0-9]+)?}}, i8 addrspace(4)* [[BL_I8]])
+  // COMMON: call i32 @__enqueue_kernel_basic(%opencl.queue_t{{.*}}* [[DEF_Q]], i32 [[FLAGS]], %struct.ndrange_t* byval [[NDR]]{{([0-9]+)?}}, i8 addrspace(4)* [[BL_I8]])
   enqueue_kernel(default_queue, flags, ndrange,
  ^(void) {
a[i] = b[i];
@@ -39,7 +40,7 @@
   // COMMON: [[FLAGS:%[0-9]+]] = load i32, i32* %flags
   // COMMON: [[WAIT_EVNT:%[0-9]+]] = addrspacecast %opencl.clk_event_t{{.*}}** %event_wait_list to %opencl.clk_event_t{{.*}}* addrspace(4)*
   // COMMON: [[EVNT:%[0-9]+]] = addrspacecast %opencl.clk_event_t{{.*}}** %clk_event to %opencl.clk_event_t{{.*}}* addrspace(4)*
-  // COMMON: [[BL:%[0-9]+]] = bitcast <{ i8*, i32, i32, i8*, %struct.__block_descriptor addrspace(2)*, i32{{.*}}, i32{{.*}}, i32{{.*}} }>* %block3 to void ()*
+  // COMMON: [[BL:%[0-9]+]] = bitcast <{ i32, i32, i8 addrspace(4)*, i32{{.*}}, i32{{.*}}, i32{{.*}} }>* %block3 to void ()*
   // COMMON: [[BL_I8:%[0-9]+]] = addrspacecast void ()* [[BL]] to i8 addrspace(4)*
   // COMMON: call i32 @__enqueue_kernel_basic_events(%opencl.queue_t{{.*}}* [[DEF_Q]], i32 [[FLAGS]],  %struct.ndrange_t* {{.*}}, i32 2, %opencl.clk_event_t{{.*}}* addrspace(4)* [[WAIT_EVNT]], %opencl.clk_event_t{{.*}}* addrspace(4)* [[EVNT]], i8 addrspace(4)* [[BL_I8]])
   enqueue_kernel(default_queue, flags, ndrange, 2, _wait_list, _event,
@@ -52,11 +53,11 @@
   // B32: %[[TMP:.*]] = alloca [1 x i32]
   // B32: %[[TMP1:.*]] = getelementptr [1 x i32], [1 x i32]* %[[TMP]], i32 0, i32 0
   // B32: store i32 256, i32* %[[TMP1]], align 4
-  // B32: call i32 @__enqueue_kernel_vaargs(%opencl.queue_t{{.*}}* 

[PATCH] D37818: [lit] Update clang and lld to use the new shared LLVMConfig stuff

[David L. Jones via Phabricator via cfe-commits]

dlj added inline comments.



Comment at: clang/test/lit.cfg:23
 # the test runner updated.
-config.test_format = lit.formats.ShTest(execute_external)
+config.test_format = lit.formats.ShTest(not llvm_config.use_lit_shell)
 

Minor nit: it seems reasonable enough to name this parameter:

  ShTest(execute_external=(not llvm_config.use_lit_shell))

(I don't think that's a problem for ShTest.__init__, and has the benefit of 
failing if the parameter name changes. Imagine if someone added another 
default-valued bool argument to ShTest.__init__...)



Comment at: llvm/utils/lit/lit/llvm/config.py:93
 def with_environment(self, variable, value, append_path = False):
-if append_path and variable in self.config.environment:
+if append_path:
+# For paths, we should be able to take a list of them and process 
all

Looking at the callers of this... should the path-append logic be a separate 
function?

For example:

llvm_config.with_environment('PATH', [config.llvm_tools_dir, 
config.clang_tools_dir], append_path=True)

versus maybe this:

llvm_config.append_env_pathvar('PATH', [config.llvm_tools_dir, 
config.clang_tools_dir])




Comment at: llvm/utils/lit/lit/llvm/config.py:128-129
+def clear_environment(self, variables):
+for name in variables:
+if name in self.config.environment:
+del self.config.environment[name]

You could also say:


```
for name in variables:
self.config.environment.pop(name, None)
```



Comment at: llvm/utils/lit/lit/llvm/config.py:132
+
+def feature_config(self, features, encoding = 'ascii'):
+# Ask llvm-config about the specified feature.

I think the expected format of the 'features' arg is complex enough that you 
should add a docstring that explains it...



Comment at: llvm/utils/lit/lit/llvm/config.py:134
+# Ask llvm-config about the specified feature.
+arguments = [x for (x, _) in features]
 try:

I think you could probably just use features.keys() for this, no?



Comment at: llvm/utils/lit/lit/llvm/config.py:147
+output = output.decode(encoding)
+lines = output.split('\n')
+for (line, (_, patterns)) in zip(lines, features):

You might want to use splitlines() for Windows compatibility.


https://reviews.llvm.org/D37818



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

[PATCH] D37818: [lit] Update clang and lld to use the new shared LLVMConfig stuff

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: llvm/utils/lit/lit/llvm/config.py:92
 
 def with_environment(self, variable, value, append_path = False):
+if append_path:

Rather than having an optional parameter that makes this append, maybe have a 
new method that calls this one after doing the path appending. How about 
`append_environment_path(var, value)` or something?


https://reviews.llvm.org/D37818



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

[PATCH] D37785: [Fuchsia] Set ENABLE_X86_RELAX_RELOCATIONS for Fuchsia builds

[Petr Hosek via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL313173: [Fuchsia] Set ENABLE_X86_RELAX_RELOCATIONS for 
Fuchsia builds (authored by phosek).

Changed prior to commit:
  https://reviews.llvm.org/D37785?vs=114956=115094#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37785

Files:
  cfe/trunk/cmake/caches/Fuchsia-stage2.cmake


Index: cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
===
--- cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
+++ cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
@@ -18,6 +18,13 @@
   set(CLANG_DEFAULT_LINKER lld CACHE STRING "")
 endif()
 
+# This is a "Does your linker support it?" option that only applies
+# to x86-64 ELF targets.  All Fuchsia target linkers do support it.
+# For x86-64 Linux, it's supported by LLD and by GNU linkers since
+# binutils 2.27, so one can hope that all Linux hosts in use handle it.
+# Ideally this would be settable as a per-target option.
+set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL "")
+
 if(APPLE)
   set(LLDB_CODESIGN_IDENTITY "" CACHE STRING "")
 endif()


Index: cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
===
--- cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
+++ cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
@@ -18,6 +18,13 @@
   set(CLANG_DEFAULT_LINKER lld CACHE STRING "")
 endif()
 
+# This is a "Does your linker support it?" option that only applies
+# to x86-64 ELF targets.  All Fuchsia target linkers do support it.
+# For x86-64 Linux, it's supported by LLD and by GNU linkers since
+# binutils 2.27, so one can hope that all Linux hosts in use handle it.
+# Ideally this would be settable as a per-target option.
+set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL "")
+
 if(APPLE)
   set(LLDB_CODESIGN_IDENTITY "" CACHE STRING "")
 endif()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r313173 - [Fuchsia] Set ENABLE_X86_RELAX_RELOCATIONS for Fuchsia builds

[Petr Hosek via cfe-commits]

Author: phosek
Date: Wed Sep 13 12:17:41 2017
New Revision: 313173

URL: http://llvm.org/viewvc/llvm-project?rev=313173=rev
Log:
[Fuchsia] Set ENABLE_X86_RELAX_RELOCATIONS for Fuchsia builds

This is a "Does your linker support it?" option, and all ours do.

Patch by Roland McGrath

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

Modified:
cfe/trunk/cmake/caches/Fuchsia-stage2.cmake

Modified: cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/caches/Fuchsia-stage2.cmake?rev=313173=313172=313173=diff
==
--- cfe/trunk/cmake/caches/Fuchsia-stage2.cmake (original)
+++ cfe/trunk/cmake/caches/Fuchsia-stage2.cmake Wed Sep 13 12:17:41 2017
@@ -18,6 +18,13 @@ if(NOT APPLE)
   set(CLANG_DEFAULT_LINKER lld CACHE STRING "")
 endif()
 
+# This is a "Does your linker support it?" option that only applies
+# to x86-64 ELF targets.  All Fuchsia target linkers do support it.
+# For x86-64 Linux, it's supported by LLD and by GNU linkers since
+# binutils 2.27, so one can hope that all Linux hosts in use handle it.
+# Ideally this would be settable as a per-target option.
+set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL "")
+
 if(APPLE)
   set(LLDB_CODESIGN_IDENTITY "" CACHE STRING "")
 endif()


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

Re: [libcxx] r250235 - [libcxx] Capture configuration information when installing the libc++ headers

[Eric Fiselier via cfe-commits]

On Wed, Sep 13, 2017 at 1:02 PM, Richard Smith 
wrote:

> On 13 October 2015 at 15:12, Eric Fiselier via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: ericwf
>> Date: Tue Oct 13 17:12:02 2015
>> New Revision: 250235
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=250235=rev
>> Log:
>> [libcxx] Capture configuration information when installing the libc++
>> headers
>>
>> Summary:
>> Hi all,
>>
>> This patch is a successor to D11963. However it has changed dramatically
>> and I felt it would be best to start a new review thread.
>>
>> Please read the design documentation added in this patch for a
>> description of how it works.
>>
>> Reviewers: mclow.lists, danalbert, jroelofs, EricWF
>>
>> Subscribers: vkalintiris, rnk, ed, espositofulvio, asl, eugenis,
>> cfe-commits
>>
>> Differential Revision: http://reviews.llvm.org/D13407
>>
>> Added:
>> libcxx/trunk/docs/DesignDocs/
>> libcxx/trunk/docs/DesignDocs/CapturingConfigInfo.rst
>> libcxx/trunk/include/__config_site.in
>> Modified:
>> libcxx/trunk/CMakeLists.txt
>> libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake
>> libcxx/trunk/docs/index.rst
>> libcxx/trunk/include/CMakeLists.txt
>>
>> Modified: libcxx/trunk/CMakeLists.txt
>> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/CMakeLists.
>> txt?rev=250235=250234=250235=diff
>> 
>> ==
>> --- libcxx/trunk/CMakeLists.txt (original)
>> +++ libcxx/trunk/CMakeLists.txt Tue Oct 13 17:12:02 2015
>> @@ -260,13 +260,6 @@ endif()
>>
>>  # Feature flags ==
>> =
>>  define_if(MSVC -D_CRT_SECURE_NO_WARNINGS)
>> -define_if_not(LIBCXX_ENABLE_GLOBAL_FILESYSTEM_NAMESPACE
>> -D_LIBCPP_HAS_NO_GLOBAL_FILESYSTEM_NAMESPACE)
>> -define_if_not(LIBCXX_ENABLE_STDIN -D_LIBCPP_HAS_NO_STDIN)
>> -define_if_not(LIBCXX_ENABLE_STDOUT -D_LIBCPP_HAS_NO_STDOUT)
>> -define_if_not(LIBCXX_ENABLE_THREADS -D_LIBCPP_HAS_NO_THREADS)
>> -define_if_not(LIBCXX_ENABLE_MONOTONIC_CLOCK
>> -D_LIBCPP_HAS_NO_MONOTONIC_CLOCK)
>> -define_if_not(LIBCXX_ENABLE_THREAD_UNSAFE_C_FUNCTIONS
>> -D_LIBCPP_HAS_NO_THREAD_UNSAFE_C_FUNCTIONS)
>> -
>>
>>  # Sanitizer flags ==
>> ===
>>
>> @@ -303,6 +296,22 @@ if (LIBCXX_BUILT_STANDALONE)
>>  message(WARNING "LLVM_USE_SANITIZER is not supported on this
>> platform.")
>>endif()
>>  endif()
>> +
>> +# Configuration file flags ==
>> ===
>> +config_define_if_not(LIBCXX_ENABLE_GLOBAL_FILESYSTEM_NAMESPACE
>> _LIBCPP_HAS_NO_GLOBAL_FILESYSTEM_NAMESPACE)
>> +config_define_if_not(LIBCXX_ENABLE_STDIN _LIBCPP_HAS_NO_STDIN)
>> +config_define_if_not(LIBCXX_ENABLE_STDOUT _LIBCPP_HAS_NO_STDOUT)
>> +config_define_if_not(LIBCXX_ENABLE_THREADS _LIBCPP_HAS_NO_THREADS)
>> +config_define_if_not(LIBCXX_ENABLE_MONOTONIC_CLOCK
>> _LIBCPP_HAS_NO_MONOTONIC_CLOCK)
>> +config_define_if_not(LIBCXX_ENABLE_THREAD_UNSAFE_C_FUNCTIONS
>> _LIBCPP_HAS_NO_THREAD_UNSAFE_C_FUNCTIONS)
>> +
>> +if (LIBCXX_NEEDS_SITE_CONFIG)
>> +  configure_file(
>> +include/__config_site.in
>> +${LIBCXX_BINARY_DIR}/__config_site
>> +@ONLY)
>> +endif()
>> +
>>  #==
>> =
>>  # Setup Source Code And Tests
>>  #==
>> =
>>
>> Modified: libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake
>> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/cmake/Modul
>> es/HandleLibcxxFlags.cmake?rev=250235=250234=250235=diff
>> 
>> ==
>> --- libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake (original)
>> +++ libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake Tue Oct 13
>> 17:12:02 2015
>> @@ -49,6 +49,22 @@ macro(define_if_not condition def)
>>endif()
>>  endmacro()
>>
>> +macro(config_define_if condition def)
>> +  if (${condition})
>> +set(${def} ON)
>> +add_definitions(-D${def})
>> +set(LIBCXX_NEEDS_SITE_CONFIG ON)
>> +  endif()
>> +endmacro()
>> +
>> +macro(config_define_if_not condition def)
>> +  if (NOT ${condition})
>> +set(${def} ON)
>> +add_definitions(-D${def})
>> +set(LIBCXX_NEEDS_SITE_CONFIG ON)
>> +  endif()
>> +endmacro()
>> +
>>  # Add a specified list of flags to both 'LIBCXX_COMPILE_FLAGS' and
>>  # 'LIBCXX_LINK_FLAGS'.
>>  macro(add_flags)
>>
>> Added: libcxx/trunk/docs/DesignDocs/CapturingConfigInfo.rst
>> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/docs/Design
>> Docs/CapturingConfigInfo.rst?rev=250235=auto
>> 
>> ==
>> --- libcxx/trunk/docs/DesignDocs/CapturingConfigInfo.rst (added)
>> +++ libcxx/trunk/docs/DesignDocs/CapturingConfigInfo.rst Tue Oct 13
>> 17:12:02 2015
>> @@ -0,0 +1,88 @@
>> 

Re: [libcxx] r250235 - [libcxx] Capture configuration information when installing the libc++ headers

[Richard Smith via cfe-commits]

On 13 October 2015 at 15:12, Eric Fiselier via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: ericwf
> Date: Tue Oct 13 17:12:02 2015
> New Revision: 250235
>
> URL: http://llvm.org/viewvc/llvm-project?rev=250235=rev
> Log:
> [libcxx] Capture configuration information when installing the libc++
> headers
>
> Summary:
> Hi all,
>
> This patch is a successor to D11963. However it has changed dramatically
> and I felt it would be best to start a new review thread.
>
> Please read the design documentation added in this patch for a description
> of how it works.
>
> Reviewers: mclow.lists, danalbert, jroelofs, EricWF
>
> Subscribers: vkalintiris, rnk, ed, espositofulvio, asl, eugenis,
> cfe-commits
>
> Differential Revision: http://reviews.llvm.org/D13407
>
> Added:
> libcxx/trunk/docs/DesignDocs/
> libcxx/trunk/docs/DesignDocs/CapturingConfigInfo.rst
> libcxx/trunk/include/__config_site.in
> Modified:
> libcxx/trunk/CMakeLists.txt
> libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake
> libcxx/trunk/docs/index.rst
> libcxx/trunk/include/CMakeLists.txt
>
> Modified: libcxx/trunk/CMakeLists.txt
> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/
> CMakeLists.txt?rev=250235=250234=250235=diff
> 
> ==
> --- libcxx/trunk/CMakeLists.txt (original)
> +++ libcxx/trunk/CMakeLists.txt Tue Oct 13 17:12:02 2015
> @@ -260,13 +260,6 @@ endif()
>
>  # Feature flags ==
> =
>  define_if(MSVC -D_CRT_SECURE_NO_WARNINGS)
> -define_if_not(LIBCXX_ENABLE_GLOBAL_FILESYSTEM_NAMESPACE
> -D_LIBCPP_HAS_NO_GLOBAL_FILESYSTEM_NAMESPACE)
> -define_if_not(LIBCXX_ENABLE_STDIN -D_LIBCPP_HAS_NO_STDIN)
> -define_if_not(LIBCXX_ENABLE_STDOUT -D_LIBCPP_HAS_NO_STDOUT)
> -define_if_not(LIBCXX_ENABLE_THREADS -D_LIBCPP_HAS_NO_THREADS)
> -define_if_not(LIBCXX_ENABLE_MONOTONIC_CLOCK -D_LIBCPP_HAS_NO_MONOTONIC_
> CLOCK)
> -define_if_not(LIBCXX_ENABLE_THREAD_UNSAFE_C_FUNCTIONS
> -D_LIBCPP_HAS_NO_THREAD_UNSAFE_C_FUNCTIONS)
> -
>
>  # Sanitizer flags ==
> ===
>
> @@ -303,6 +296,22 @@ if (LIBCXX_BUILT_STANDALONE)
>  message(WARNING "LLVM_USE_SANITIZER is not supported on this
> platform.")
>endif()
>  endif()
> +
> +# Configuration file flags ==
> ===
> +config_define_if_not(LIBCXX_ENABLE_GLOBAL_FILESYSTEM_NAMESPACE
> _LIBCPP_HAS_NO_GLOBAL_FILESYSTEM_NAMESPACE)
> +config_define_if_not(LIBCXX_ENABLE_STDIN _LIBCPP_HAS_NO_STDIN)
> +config_define_if_not(LIBCXX_ENABLE_STDOUT _LIBCPP_HAS_NO_STDOUT)
> +config_define_if_not(LIBCXX_ENABLE_THREADS _LIBCPP_HAS_NO_THREADS)
> +config_define_if_not(LIBCXX_ENABLE_MONOTONIC_CLOCK
> _LIBCPP_HAS_NO_MONOTONIC_CLOCK)
> +config_define_if_not(LIBCXX_ENABLE_THREAD_UNSAFE_C_FUNCTIONS
> _LIBCPP_HAS_NO_THREAD_UNSAFE_C_FUNCTIONS)
> +
> +if (LIBCXX_NEEDS_SITE_CONFIG)
> +  configure_file(
> +include/__config_site.in
> +${LIBCXX_BINARY_DIR}/__config_site
> +@ONLY)
> +endif()
> +
>  #===
> 
>  # Setup Source Code And Tests
>  #===
> 
>
> Modified: libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake
> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/cmake/
> Modules/HandleLibcxxFlags.cmake?rev=250235=250234=250235=diff
> 
> ==
> --- libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake (original)
> +++ libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake Tue Oct 13
> 17:12:02 2015
> @@ -49,6 +49,22 @@ macro(define_if_not condition def)
>endif()
>  endmacro()
>
> +macro(config_define_if condition def)
> +  if (${condition})
> +set(${def} ON)
> +add_definitions(-D${def})
> +set(LIBCXX_NEEDS_SITE_CONFIG ON)
> +  endif()
> +endmacro()
> +
> +macro(config_define_if_not condition def)
> +  if (NOT ${condition})
> +set(${def} ON)
> +add_definitions(-D${def})
> +set(LIBCXX_NEEDS_SITE_CONFIG ON)
> +  endif()
> +endmacro()
> +
>  # Add a specified list of flags to both 'LIBCXX_COMPILE_FLAGS' and
>  # 'LIBCXX_LINK_FLAGS'.
>  macro(add_flags)
>
> Added: libcxx/trunk/docs/DesignDocs/CapturingConfigInfo.rst
> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/docs/DesignDocs/
> CapturingConfigInfo.rst?rev=250235=auto
> 
> ==
> --- libcxx/trunk/docs/DesignDocs/CapturingConfigInfo.rst (added)
> +++ libcxx/trunk/docs/DesignDocs/CapturingConfigInfo.rst Tue Oct 13
> 17:12:02 2015
> @@ -0,0 +1,88 @@
> +===
> +Capturing configuration information during installation
> +===
> +
> +.. contents::
> +   :local:
> +
> +The Problem
> 

[PATCH] D37742: Add more tests for OpenCL atomic builtin functions

[Yaxun Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL313172: Add more tests for OpenCL atomic builtin functions 
(authored by yaxunl).

Changed prior to commit:
  https://reviews.llvm.org/D37742?vs=114824=115090#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37742

Files:
  cfe/trunk/test/CodeGenOpenCL/atomic-ops-libcall.cl
  cfe/trunk/test/CodeGenOpenCL/atomic-ops.cl

Index: cfe/trunk/test/CodeGenOpenCL/atomic-ops.cl
===
--- cfe/trunk/test/CodeGenOpenCL/atomic-ops.cl
+++ cfe/trunk/test/CodeGenOpenCL/atomic-ops.cl
@@ -34,10 +34,13 @@
   // CHECK-LABEL: @fi1
   // CHECK: load atomic i32, i32 addrspace(4)* %{{[.0-9A-Z_a-z]+}} syncscope("workgroup") seq_cst
   int x = __opencl_atomic_load(i, memory_order_seq_cst, memory_scope_work_group);
+
   // CHECK: load atomic i32, i32 addrspace(4)* %{{[.0-9A-Z_a-z]+}} syncscope("agent") seq_cst
   x = __opencl_atomic_load(i, memory_order_seq_cst, memory_scope_device);
+
   // CHECK: load atomic i32, i32 addrspace(4)* %{{[.0-9A-Z_a-z]+}} seq_cst
   x = __opencl_atomic_load(i, memory_order_seq_cst, memory_scope_all_svm_devices);
+
   // CHECK: load atomic i32, i32 addrspace(4)* %{{[.0-9A-Z_a-z]+}} syncscope("subgroup") seq_cst
   x = __opencl_atomic_load(i, memory_order_seq_cst, memory_scope_sub_group);
 }
@@ -48,16 +51,32 @@
   __opencl_atomic_store(i, 1, memory_order_seq_cst, memory_scope_work_group);
 }
 
+void test_addr(global atomic_int *ig, private atomic_int *ip, local atomic_int *il) {
+  // CHECK-LABEL: @test_addr
+  // CHECK: store atomic i32 %{{[.0-9A-Z_a-z]+}}, i32 addrspace(1)* %{{[.0-9A-Z_a-z]+}} syncscope("workgroup") seq_cst
+  __opencl_atomic_store(ig, 1, memory_order_seq_cst, memory_scope_work_group);
+
+  // CHECK: store atomic i32 %{{[.0-9A-Z_a-z]+}}, i32* %{{[.0-9A-Z_a-z]+}} syncscope("workgroup") seq_cst
+  __opencl_atomic_store(ip, 1, memory_order_seq_cst, memory_scope_work_group);
+
+  // CHECK: store atomic i32 %{{[.0-9A-Z_a-z]+}}, i32 addrspace(3)* %{{[.0-9A-Z_a-z]+}} syncscope("workgroup") seq_cst
+  __opencl_atomic_store(il, 1, memory_order_seq_cst, memory_scope_work_group);
+}
+
 void fi3(atomic_int *i, atomic_uint *ui) {
   // CHECK-LABEL: @fi3
   // CHECK: atomicrmw and i32 addrspace(4)* %{{[.0-9A-Z_a-z]+}}, i32 %{{[.0-9A-Z_a-z]+}} syncscope("workgroup") seq_cst
   int x = __opencl_atomic_fetch_and(i, 1, memory_order_seq_cst, memory_scope_work_group);
+
   // CHECK: atomicrmw min i32 addrspace(4)* %{{[.0-9A-Z_a-z]+}}, i32 %{{[.0-9A-Z_a-z]+}} syncscope("workgroup") seq_cst
   x = __opencl_atomic_fetch_min(i, 1, memory_order_seq_cst, memory_scope_work_group);
+
   // CHECK: atomicrmw max i32 addrspace(4)* %{{[.0-9A-Z_a-z]+}}, i32 %{{[.0-9A-Z_a-z]+}} syncscope("workgroup") seq_cst
   x = __opencl_atomic_fetch_max(i, 1, memory_order_seq_cst, memory_scope_work_group);
+
   // CHECK: atomicrmw umin i32 addrspace(4)* %{{[.0-9A-Z_a-z]+}}, i32 %{{[.0-9A-Z_a-z]+}} syncscope("workgroup") seq_cst
   x = __opencl_atomic_fetch_min(ui, 1, memory_order_seq_cst, memory_scope_work_group);
+
   // CHECK: atomicrmw umax i32 addrspace(4)* %{{[.0-9A-Z_a-z]+}}, i32 %{{[.0-9A-Z_a-z]+}} syncscope("workgroup") seq_cst
   x = __opencl_atomic_fetch_max(ui, 1, memory_order_seq_cst, memory_scope_work_group);
 }
Index: cfe/trunk/test/CodeGenOpenCL/atomic-ops-libcall.cl
===
--- cfe/trunk/test/CodeGenOpenCL/atomic-ops-libcall.cl
+++ cfe/trunk/test/CodeGenOpenCL/atomic-ops-libcall.cl
@@ -18,39 +18,64 @@
 #endif
 } memory_scope;
 
-void f(atomic_int *i, atomic_uint *ui, int cmp, int order, int scope) {
+void f(atomic_int *i, global atomic_int *gi, local atomic_int *li, private atomic_int *pi, atomic_uint *ui, int cmp, int order, int scope) {
   int x;
   // SPIR: {{%[^ ]*}} = call i32 @__opencl_atomic_load_4(i8 addrspace(4)* {{%[0-9]+}}, i32 5, i32 1)
   // ARM: {{%[^ ]*}} = call i32 @__opencl_atomic_load_4(i8* {{%[0-9]+}}, i32 5, i32 1)
   x = __opencl_atomic_load(i, memory_order_seq_cst, memory_scope_work_group);
+
   // SPIR: call void @__opencl_atomic_store_4(i8 addrspace(4)* {{%[0-9]+}}, i32 {{%[0-9]+}}, i32 5, i32 1)
   // ARM: call void @__opencl_atomic_store_4(i8* {{%[0-9]+}}, i32 {{%[0-9]+}}, i32 5, i32 1)
   __opencl_atomic_store(i, 1, memory_order_seq_cst, memory_scope_work_group);
+
+  // SPIR: %[[GP:[0-9]+]] = addrspacecast i8 addrspace(1)* {{%[0-9]+}} to i8 addrspace(4)*
+  // SPIR: call void @__opencl_atomic_store_4(i8 addrspace(4)* %[[GP]], i32 {{%[0-9]+}}, i32 5, i32 1)
+  // ARM: call void @__opencl_atomic_store_4(i8* {{%[0-9]+}}, i32 {{%[0-9]+}}, i32 5, i32 1)
+  __opencl_atomic_store(gi, 1, memory_order_seq_cst, memory_scope_work_group);
+
+  // SPIR: %[[GP:[0-9]+]] = addrspacecast i8 addrspace(3)* {{%[0-9]+}} to i8 addrspace(4)*
+  // SPIR: call void @__opencl_atomic_store_4(i8 addrspace(4)* %[[GP]], i32 {{%[0-9]+}}, i32 5, i32 1)
+  // ARM: call void @__opencl_atomic_store_4(i8* 

r313172 - Add more tests for OpenCL atomic builtin functions

[Yaxun Liu via cfe-commits]

Author: yaxunl
Date: Wed Sep 13 11:56:25 2017
New Revision: 313172

URL: http://llvm.org/viewvc/llvm-project?rev=313172=rev
Log:
Add more tests for OpenCL atomic builtin functions

Add tests for different address spaces and insert some blank lines to make them 
more readable.

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

Modified:
cfe/trunk/test/CodeGenOpenCL/atomic-ops-libcall.cl
cfe/trunk/test/CodeGenOpenCL/atomic-ops.cl

Modified: cfe/trunk/test/CodeGenOpenCL/atomic-ops-libcall.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/atomic-ops-libcall.cl?rev=313172=313171=313172=diff
==
--- cfe/trunk/test/CodeGenOpenCL/atomic-ops-libcall.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/atomic-ops-libcall.cl Wed Sep 13 11:56:25 2017
@@ -18,39 +18,64 @@ typedef enum memory_scope {
 #endif
 } memory_scope;
 
-void f(atomic_int *i, atomic_uint *ui, int cmp, int order, int scope) {
+void f(atomic_int *i, global atomic_int *gi, local atomic_int *li, private 
atomic_int *pi, atomic_uint *ui, int cmp, int order, int scope) {
   int x;
   // SPIR: {{%[^ ]*}} = call i32 @__opencl_atomic_load_4(i8 addrspace(4)* 
{{%[0-9]+}}, i32 5, i32 1)
   // ARM: {{%[^ ]*}} = call i32 @__opencl_atomic_load_4(i8* {{%[0-9]+}}, i32 
5, i32 1)
   x = __opencl_atomic_load(i, memory_order_seq_cst, memory_scope_work_group);
+
   // SPIR: call void @__opencl_atomic_store_4(i8 addrspace(4)* {{%[0-9]+}}, 
i32 {{%[0-9]+}}, i32 5, i32 1)
   // ARM: call void @__opencl_atomic_store_4(i8* {{%[0-9]+}}, i32 {{%[0-9]+}}, 
i32 5, i32 1)
   __opencl_atomic_store(i, 1, memory_order_seq_cst, memory_scope_work_group);
+
+  // SPIR: %[[GP:[0-9]+]] = addrspacecast i8 addrspace(1)* {{%[0-9]+}} to i8 
addrspace(4)*
+  // SPIR: call void @__opencl_atomic_store_4(i8 addrspace(4)* %[[GP]], i32 
{{%[0-9]+}}, i32 5, i32 1)
+  // ARM: call void @__opencl_atomic_store_4(i8* {{%[0-9]+}}, i32 {{%[0-9]+}}, 
i32 5, i32 1)
+  __opencl_atomic_store(gi, 1, memory_order_seq_cst, memory_scope_work_group);
+
+  // SPIR: %[[GP:[0-9]+]] = addrspacecast i8 addrspace(3)* {{%[0-9]+}} to i8 
addrspace(4)*
+  // SPIR: call void @__opencl_atomic_store_4(i8 addrspace(4)* %[[GP]], i32 
{{%[0-9]+}}, i32 5, i32 1)
+  // ARM: call void @__opencl_atomic_store_4(i8* {{%[0-9]+}}, i32 {{%[0-9]+}}, 
i32 5, i32 1)
+  __opencl_atomic_store(li, 1, memory_order_seq_cst, memory_scope_work_group);
+
+  // SPIR: %[[GP:[0-9]+]] = addrspacecast i8* {{%[0-9]+}} to i8 addrspace(4)*
+  // SPIR: call void @__opencl_atomic_store_4(i8 addrspace(4)* %[[GP]], i32 
{{%[0-9]+}}, i32 5, i32 1)
+  // ARM: call void @__opencl_atomic_store_4(i8* {{%[0-9]+}}, i32 {{%[0-9]+}}, 
i32 5, i32 1)
+  __opencl_atomic_store(pi, 1, memory_order_seq_cst, memory_scope_work_group);
+
   // SPIR: {{%[^ ]*}} = call i32 @__opencl_atomic_fetch_add_4(i8 addrspace(4)* 
{{%[0-9]+}}, i32 {{%[0-9]+}}, i32 5, i32 1)
   // ARM: {{%[^ ]*}} = call i32 @__opencl_atomic_fetch_add_4(i8* {{%[0-9]+}}, 
i32 {{%[0-9]+}}, i32 5, i32 1)
   x = __opencl_atomic_fetch_add(i, 3, memory_order_seq_cst, 
memory_scope_work_group);
+
   // SPIR: {{%[^ ]*}} = call i32 @__opencl_atomic_fetch_min_4(i8 addrspace(4)* 
{{%[0-9]+}}, i32 {{%[0-9]+}}, i32 5, i32 1)
   // ARM: {{%[^ ]*}} = call i32 @__opencl_atomic_fetch_min_4(i8* {{%[0-9]+}}, 
i32 {{%[0-9]+}}, i32 5, i32 1)
   x = __opencl_atomic_fetch_min(i, 3, memory_order_seq_cst, 
memory_scope_work_group);
+
   // SPIR: {{%[^ ]*}} = call i32 @__opencl_atomic_fetch_umin_4(i8 
addrspace(4)* {{%[0-9]+}}, i32 {{%[0-9]+}}, i32 5, i32 1)
   // ARM: {{%[^ ]*}} = call i32 @__opencl_atomic_fetch_umin_4(i8* {{%[0-9]+}}, 
i32 {{%[0-9]+}}, i32 5, i32 1)
   x = __opencl_atomic_fetch_min(ui, 3, memory_order_seq_cst, 
memory_scope_work_group);
+
   // SPIR: {{%[^ ]*}} = call zeroext i1 @__opencl_atomic_compare_exchange_4(i8 
addrspace(4)* {{%[0-9]+}}, i8 addrspace(4)* {{%[0-9]+}}, i32 {{%[0-9]+}}, i32 
5, i32 5, i32 1)
   // ARM: {{%[^ ]*}} = call zeroext i1 @__opencl_atomic_compare_exchange_4(i8* 
{{%[0-9]+}}, i8* {{%[0-9]+}}, i32 {{%[0-9]+}}, i32 5, i32 5, i32 1)
   x = __opencl_atomic_compare_exchange_strong(i, , 1, 
memory_order_seq_cst, memory_order_seq_cst, memory_scope_work_group);
+
   // SPIR: {{%[^ ]*}} = call zeroext i1 @__opencl_atomic_compare_exchange_4(i8 
addrspace(4)* {{%[0-9]+}}, i8 addrspace(4)* {{%[0-9]+}}, i32 {{%[0-9]+}}, i32 
5, i32 5, i32 1)
   // ARM: {{%[^ ]*}} = call zeroext i1 @__opencl_atomic_compare_exchange_4(i8* 
{{%[0-9]+}}, i8* {{%[0-9]+}}, i32 {{%[0-9]+}}, i32 5, i32 5, i32 1)
   x = __opencl_atomic_compare_exchange_weak(i, , 1, memory_order_seq_cst, 
memory_order_seq_cst, memory_scope_work_group);
+
   // SPIR: {{%[^ ]*}} = call zeroext i1 @__opencl_atomic_compare_exchange_4(i8 
addrspace(4)* {{%[0-9]+}}, i8 addrspace(4)* {{%[0-9]+}}, i32 {{%[0-9]+}}, i32 
5, i32 5, i32 2)
   // ARM: {{%[^ ]*}} = call zeroext i1 @__opencl_atomic_compare_exchange_4(i8* 
{{%[0-9]+}}, i8* 

[PATCH] D37818: [lit] Update clang and lld to use the new shared LLVMConfig stuff

[Zachary Turner via Phabricator via cfe-commits]

zturner created this revision.
Herald added a reviewer: modocache.
Herald added a subscriber: fedor.sergeev.

This is probably the last work I'm going to do here for a while.  I still see 
some room for improvement, but I think after this patch:

a) refactor begins to have diminishing returns
b) There's a sufficient amount of sharing and re-use to validate the design
c) There's enough specific examples of how to use this that other projects 
(e.g. compiler-rt, etc) can use those as a model of how to simplify their own 
configs.

In other words, I'm not brave enough to touch compiler-rt's config files :)

There's some minor changes in the config api that increase flexibility in how 
you override environment variables and spawn an external `llvm-config`, but 
otherwise this is mostly just deleting code.


https://reviews.llvm.org/D37818

Files:
  clang/test/lit.cfg
  clang/test/lit.site.cfg.in
  lld/test/lit.cfg
  lld/test/lit.site.cfg.in
  llvm/test/lit.cfg
  llvm/utils/lit/lit/llvm/config.py

Index: llvm/utils/lit/lit/llvm/config.py
===
--- llvm/utils/lit/lit/llvm/config.py
+++ llvm/utils/lit/lit/llvm/config.py
@@ -1,4 +1,5 @@
 import os
+import platform
 import re
 import subprocess
 import sys
@@ -23,35 +24,40 @@
 
 # Tweak PATH for Win32 to decide to use bash.exe or not.
 if sys.platform == 'win32':
-# For tests that require Windows to run.
-features.add('system-windows')
-
 # Seek sane tools in directories and set to $PATH.
 path = self.lit_config.getToolsPath(config.lit_tools_dir,
config.environment['PATH'],
['cmp.exe', 'grep.exe', 'sed.exe'])
 self.with_environment('PATH', path, append_path=True)
 
-if use_lit_shell:
-# 0 is external, "" is default, and everything else is internal.
-self.use_lit_shell = (use_lit_shell != "0")
-else:
-# Otherwise we default to internal on Windows and external elsewhere, as
-# bash on Windows is usually very slow.
-self.use_lit_shell = (sys.platform in ['win32'])
+self.with_environment('LLVM_SRC_ROOT', config.llvm_src_root)
 
 self.use_lit_shell = litsh
-if not self.litsh:
+if not litsh:
 features.add('shell')
 
+
+# Running on Darwin OS
+if platform.system() in ['Darwin']:
+# FIXME: lld uses the first, other projects use the second.
+# We should standardize on the former.
+features.add('system-linker-mach-o')
+features.add('system-darwin')
+elif platform.system() in ['Windows']:
+# For tests that require Windows to run.
+features.add('system-windows')
+
 # Native compilation: host arch == default triple arch
-# FIXME: Consider cases that target can be executed
-# even if host_triple were different from target_triple.
-if config.host_triple == config.target_triple:
+# Both of these values should probably be in every site config (e.g. as
+# part of the standard header.  But currently they aren't)
+host_triple = getattr(config, 'host_triple', None)
+target_triple = getattr(config, 'target_triple', None)
+if host_triple and host_triple == target_triple:
 features.add("native")
 
 # Sanitizers.
-sanitizers = frozenset(x.lower() for x in getattr(config, 'llvm_use_sanitizer', []).split(';'))
+sanitizers = getattr(config, 'llvm_use_sanitizer', '')
+sanitizers = frozenset(x.lower() for x in sanitizers.split(';'))
 features.add(binary_feature('address' in sanitizers, 'asan', 'not_'))
 features.add(binary_feature('memory' in sanitizers, 'msan', 'not_'))
 features.add(binary_feature('undefined' in sanitizers, 'ubsan', 'not_'))
@@ -64,7 +70,6 @@
 if lit.util.pythonize_bool(long_tests):
 features.add("long_tests")
 
-target_triple = getattr(config, 'target_triple', None)
 if target_triple:
 if re.match(r'^x86_64.*-linux', target_triple):
 features.add("x86_64-linux")
@@ -80,25 +85,34 @@
 if gmalloc_path_str is not None:
 self.with_environment('DYLD_INSERT_LIBRARIES', gmalloc_path_str)
 
-breaking_checks = getattr(config, 'enable_abi_breaking_checks')
-if lit.util.pythonize_bool(lit_config, breaking_checks):
+breaking_checks = getattr(config, 'enable_abi_breaking_checks', None)
+if lit.util.pythonize_bool(breaking_checks):
 features.add('abi-breaking-checks')
 
 def with_environment(self, variable, value, append_path = False):
-if append_path and variable in self.config.environment:
+if append_path:
+# For paths, we should be able to take a 

[PATCH] D37703: [AMDGPU] Change addr space of clk_event_t, queue_t and reserve_id_t to global

[Yaxun Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL313171: [AMDGPU] Change addr space of clk_event_t, queue_t 
and reserve_id_t to global (authored by yaxunl).

Changed prior to commit:
  https://reviews.llvm.org/D37703?vs=114642=115088#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37703

Files:
  cfe/trunk/lib/Basic/Targets/AMDGPU.h
  cfe/trunk/test/CodeGenOpenCL/opencl_types.cl


Index: cfe/trunk/lib/Basic/Targets/AMDGPU.h
===
--- cfe/trunk/lib/Basic/Targets/AMDGPU.h
+++ cfe/trunk/lib/Basic/Targets/AMDGPU.h
@@ -202,6 +202,10 @@
   case BuiltinType::Id:
\
 return LangAS::opencl_constant;
 #include "clang/Basic/OpenCLImageTypes.def"
+case BuiltinType::OCLClkEvent:
+case BuiltinType::OCLQueue:
+case BuiltinType::OCLReserveID:
+  return LangAS::opencl_global;
 
 default:
   return TargetInfo::getOpenCLTypeAddrSpace(T);
Index: cfe/trunk/test/CodeGenOpenCL/opencl_types.cl
===
--- cfe/trunk/test/CodeGenOpenCL/opencl_types.cl
+++ cfe/trunk/test/CodeGenOpenCL/opencl_types.cl
@@ -1,13 +1,13 @@
-// RUN: %clang_cc1 -cl-std=CL2.0 %s -triple "spir-unknown-unknown" -emit-llvm 
-o - -O0 | FileCheck %s --check-prefix=CHECK-SPIR
-// RUN: %clang_cc1 -cl-std=CL2.0 %s -triple "amdgcn--amdhsa" -emit-llvm -o - 
-O0 | FileCheck %s --check-prefix=CHECK-AMDGCN
+// RUN: %clang_cc1 -cl-std=CL2.0 %s -triple "spir-unknown-unknown" -emit-llvm 
-o - -O0 | FileCheck %s --check-prefixes=CHECK-COM,CHECK-SPIR
+// RUN: %clang_cc1 -cl-std=CL2.0 %s -triple "amdgcn--amdhsa" -emit-llvm -o - 
-O0 | FileCheck %s --check-prefixes=CHECK-COM,CHECK-AMDGCN
 
 #define CLK_ADDRESS_CLAMP_TO_EDGE   2
 #define CLK_NORMALIZED_COORDS_TRUE  1
 #define CLK_FILTER_NEAREST  0x10
 #define CLK_FILTER_LINEAR   0x20
 
 constant sampler_t glb_smp = 
CLK_ADDRESS_CLAMP_TO_EDGE|CLK_NORMALIZED_COORDS_TRUE|CLK_FILTER_NEAREST;
-// CHECK-SPIR-NOT: constant i32
+// CHECK-COM-NOT: constant i32
 
 void fnc1(image1d_t img) {}
 // CHECK-SPIR: @fnc1(%opencl.image1d_ro_t addrspace(1)*
@@ -39,20 +39,23 @@
 
 kernel void foo(image1d_t img) {
   sampler_t smp = 
CLK_ADDRESS_CLAMP_TO_EDGE|CLK_NORMALIZED_COORDS_TRUE|CLK_FILTER_LINEAR;
-  // CHECK-SPIR: alloca %opencl.sampler_t addrspace(2)*
+  // CHECK-COM: alloca %opencl.sampler_t addrspace(2)*
   event_t evt;
-  // CHECK-SPIR: alloca %opencl.event_t*
+  // CHECK-COM: alloca %opencl.event_t*
   clk_event_t clk_evt;
   // CHECK-SPIR: alloca %opencl.clk_event_t*
+  // CHECK-AMDGCN: alloca %opencl.clk_event_t addrspace(1)*
   queue_t queue;
   // CHECK-SPIR: alloca %opencl.queue_t*
+  // CHECK-AMDGCN: alloca %opencl.queue_t addrspace(1)*
   reserve_id_t rid;
   // CHECK-SPIR: alloca %opencl.reserve_id_t*
-  // CHECK-SPIR: store %opencl.sampler_t addrspace(2)*
+  // CHECK-AMDGCN: alloca %opencl.reserve_id_t addrspace(1)*
+  // CHECK-COM: store %opencl.sampler_t addrspace(2)*
   fnc4smp(smp);
-  // CHECK-SPIR: call {{.*}}void @fnc4smp(%opencl.sampler_t addrspace(2)*
+  // CHECK-COM: call {{.*}}void @fnc4smp(%opencl.sampler_t addrspace(2)*
   fnc4smp(glb_smp);
-  // CHECK-SPIR: call {{.*}}void @fnc4smp(%opencl.sampler_t addrspace(2)*
+  // CHECK-COM: call {{.*}}void @fnc4smp(%opencl.sampler_t addrspace(2)*
 }
 
 kernel void foo_pipe(read_only pipe int p) {}


Index: cfe/trunk/lib/Basic/Targets/AMDGPU.h
===
--- cfe/trunk/lib/Basic/Targets/AMDGPU.h
+++ cfe/trunk/lib/Basic/Targets/AMDGPU.h
@@ -202,6 +202,10 @@
   case BuiltinType::Id:\
 return LangAS::opencl_constant;
 #include "clang/Basic/OpenCLImageTypes.def"
+case BuiltinType::OCLClkEvent:
+case BuiltinType::OCLQueue:
+case BuiltinType::OCLReserveID:
+  return LangAS::opencl_global;
 
 default:
   return TargetInfo::getOpenCLTypeAddrSpace(T);
Index: cfe/trunk/test/CodeGenOpenCL/opencl_types.cl
===
--- cfe/trunk/test/CodeGenOpenCL/opencl_types.cl
+++ cfe/trunk/test/CodeGenOpenCL/opencl_types.cl
@@ -1,13 +1,13 @@
-// RUN: %clang_cc1 -cl-std=CL2.0 %s -triple "spir-unknown-unknown" -emit-llvm -o - -O0 | FileCheck %s --check-prefix=CHECK-SPIR
-// RUN: %clang_cc1 -cl-std=CL2.0 %s -triple "amdgcn--amdhsa" -emit-llvm -o - -O0 | FileCheck %s --check-prefix=CHECK-AMDGCN
+// RUN: %clang_cc1 -cl-std=CL2.0 %s -triple "spir-unknown-unknown" -emit-llvm -o - -O0 | FileCheck %s --check-prefixes=CHECK-COM,CHECK-SPIR
+// RUN: %clang_cc1 -cl-std=CL2.0 %s -triple "amdgcn--amdhsa" -emit-llvm -o - -O0 | FileCheck %s --check-prefixes=CHECK-COM,CHECK-AMDGCN
 
 #define CLK_ADDRESS_CLAMP_TO_EDGE   2
 #define CLK_NORMALIZED_COORDS_TRUE  1
 #define CLK_FILTER_NEAREST  0x10
 #define CLK_FILTER_LINEAR   

r313171 - [AMDGPU] Change addr space of clk_event_t, queue_t and reserve_id_t to global

[Yaxun Liu via cfe-commits]

Author: yaxunl
Date: Wed Sep 13 11:50:42 2017
New Revision: 313171

URL: http://llvm.org/viewvc/llvm-project?rev=313171=rev
Log:
[AMDGPU] Change addr space of clk_event_t, queue_t and reserve_id_t to global

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

Modified:
cfe/trunk/lib/Basic/Targets/AMDGPU.h
cfe/trunk/test/CodeGenOpenCL/opencl_types.cl

Modified: cfe/trunk/lib/Basic/Targets/AMDGPU.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/AMDGPU.h?rev=313171=313170=313171=diff
==
--- cfe/trunk/lib/Basic/Targets/AMDGPU.h (original)
+++ cfe/trunk/lib/Basic/Targets/AMDGPU.h Wed Sep 13 11:50:42 2017
@@ -202,6 +202,10 @@ public:
   case BuiltinType::Id:
\
 return LangAS::opencl_constant;
 #include "clang/Basic/OpenCLImageTypes.def"
+case BuiltinType::OCLClkEvent:
+case BuiltinType::OCLQueue:
+case BuiltinType::OCLReserveID:
+  return LangAS::opencl_global;
 
 default:
   return TargetInfo::getOpenCLTypeAddrSpace(T);

Modified: cfe/trunk/test/CodeGenOpenCL/opencl_types.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/opencl_types.cl?rev=313171=313170=313171=diff
==
--- cfe/trunk/test/CodeGenOpenCL/opencl_types.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/opencl_types.cl Wed Sep 13 11:50:42 2017
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -cl-std=CL2.0 %s -triple "spir-unknown-unknown" -emit-llvm 
-o - -O0 | FileCheck %s --check-prefix=CHECK-SPIR
-// RUN: %clang_cc1 -cl-std=CL2.0 %s -triple "amdgcn--amdhsa" -emit-llvm -o - 
-O0 | FileCheck %s --check-prefix=CHECK-AMDGCN
+// RUN: %clang_cc1 -cl-std=CL2.0 %s -triple "spir-unknown-unknown" -emit-llvm 
-o - -O0 | FileCheck %s --check-prefixes=CHECK-COM,CHECK-SPIR
+// RUN: %clang_cc1 -cl-std=CL2.0 %s -triple "amdgcn--amdhsa" -emit-llvm -o - 
-O0 | FileCheck %s --check-prefixes=CHECK-COM,CHECK-AMDGCN
 
 #define CLK_ADDRESS_CLAMP_TO_EDGE   2
 #define CLK_NORMALIZED_COORDS_TRUE  1
@@ -7,7 +7,7 @@
 #define CLK_FILTER_LINEAR   0x20
 
 constant sampler_t glb_smp = 
CLK_ADDRESS_CLAMP_TO_EDGE|CLK_NORMALIZED_COORDS_TRUE|CLK_FILTER_NEAREST;
-// CHECK-SPIR-NOT: constant i32
+// CHECK-COM-NOT: constant i32
 
 void fnc1(image1d_t img) {}
 // CHECK-SPIR: @fnc1(%opencl.image1d_ro_t addrspace(1)*
@@ -39,20 +39,23 @@ void fnc4smp(sampler_t s) {}
 
 kernel void foo(image1d_t img) {
   sampler_t smp = 
CLK_ADDRESS_CLAMP_TO_EDGE|CLK_NORMALIZED_COORDS_TRUE|CLK_FILTER_LINEAR;
-  // CHECK-SPIR: alloca %opencl.sampler_t addrspace(2)*
+  // CHECK-COM: alloca %opencl.sampler_t addrspace(2)*
   event_t evt;
-  // CHECK-SPIR: alloca %opencl.event_t*
+  // CHECK-COM: alloca %opencl.event_t*
   clk_event_t clk_evt;
   // CHECK-SPIR: alloca %opencl.clk_event_t*
+  // CHECK-AMDGCN: alloca %opencl.clk_event_t addrspace(1)*
   queue_t queue;
   // CHECK-SPIR: alloca %opencl.queue_t*
+  // CHECK-AMDGCN: alloca %opencl.queue_t addrspace(1)*
   reserve_id_t rid;
   // CHECK-SPIR: alloca %opencl.reserve_id_t*
-  // CHECK-SPIR: store %opencl.sampler_t addrspace(2)*
+  // CHECK-AMDGCN: alloca %opencl.reserve_id_t addrspace(1)*
+  // CHECK-COM: store %opencl.sampler_t addrspace(2)*
   fnc4smp(smp);
-  // CHECK-SPIR: call {{.*}}void @fnc4smp(%opencl.sampler_t addrspace(2)*
+  // CHECK-COM: call {{.*}}void @fnc4smp(%opencl.sampler_t addrspace(2)*
   fnc4smp(glb_smp);
-  // CHECK-SPIR: call {{.*}}void @fnc4smp(%opencl.sampler_t addrspace(2)*
+  // CHECK-COM: call {{.*}}void @fnc4smp(%opencl.sampler_t addrspace(2)*
 }
 
 kernel void foo_pipe(read_only pipe int p) {}


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

[PATCH] D33514: [WIP] Bug 32352 - Provide a way for OptimizationRemarkEmitter::allowExtraAnalysis to check if (specific) remarks are enabled

[Vivek Pandya via Phabricator via cfe-commits]

vivekvpandya updated this revision to Diff 115086.
vivekvpandya added a comment.

Added method to detach unique_ptr from LLVMContext.


https://reviews.llvm.org/D33514

Files:
  include/llvm/Analysis/OptimizationDiagnosticInfo.h
  include/llvm/CodeGen/MachineOptimizationRemarkEmitter.h
  include/llvm/IR/DiagnosticHandler.h
  include/llvm/IR/DiagnosticInfo.h
  include/llvm/IR/LLVMContext.h
  include/llvm/LTO/Config.h
  include/llvm/LTO/legacy/LTOCodeGenerator.h
  lib/IR/CMakeLists.txt
  lib/IR/Core.cpp
  lib/IR/DiagnosticHandler.cpp
  lib/IR/DiagnosticInfo.cpp
  lib/IR/LLVMContext.cpp
  lib/IR/LLVMContextImpl.cpp
  lib/IR/LLVMContextImpl.h
  lib/LTO/LTOCodeGenerator.cpp
  lib/Transforms/Scalar/GVN.cpp
  lib/Transforms/Vectorize/LoopVectorize.cpp
  test/Transforms/GVN/opt-remarks.ll
  tools/llc/llc.cpp
  tools/llvm-dis/llvm-dis.cpp
  tools/llvm-link/llvm-link.cpp
  tools/llvm-lto/llvm-lto.cpp
  tools/lto/lto.cpp

Index: tools/lto/lto.cpp
===
--- tools/lto/lto.cpp
+++ tools/lto/lto.cpp
@@ -75,20 +75,23 @@
 
 static LLVMContext *LTOContext = nullptr;
 
-static void diagnosticHandler(const DiagnosticInfo , void *Context) {
-  if (DI.getSeverity() != DS_Error) {
-DiagnosticPrinterRawOStream DP(errs());
-DI.print(DP);
-errs() << '\n';
-return;
-  }
-  sLastErrorString = "";
-  {
-raw_string_ostream Stream(sLastErrorString);
-DiagnosticPrinterRawOStream DP(Stream);
-DI.print(DP);
+struct LTOToolDiagnosticHandler : public DiagnosticHandler {
+  bool handleDiagnostics(const DiagnosticInfo ) override {
+if (DI.getSeverity() != DS_Error) {
+  DiagnosticPrinterRawOStream DP(errs());
+  DI.print(DP);
+  errs() << '\n';
+  return true;
+}
+sLastErrorString = "";
+{
+  raw_string_ostream Stream(sLastErrorString);
+  DiagnosticPrinterRawOStream DP(Stream);
+  DI.print(DP);
+}
+return true;
   }
-}
+};
 
 // Initialize the configured targets if they have not been initialized.
 static void lto_initialize() {
@@ -108,7 +111,8 @@
 
 static LLVMContext Context;
 LTOContext = 
-LTOContext->setDiagnosticHandler(diagnosticHandler, nullptr, true);
+LTOContext->setDiagnosticHandler(
+llvm::make_unique(), true);
 initialized = true;
   }
 }
@@ -274,7 +278,8 @@
 
   // Create a local context. Ownership will be transferred to LTOModule.
   std::unique_ptr Context = llvm::make_unique();
-  Context->setDiagnosticHandler(diagnosticHandler, nullptr, true);
+  Context->setDiagnosticHandler(llvm::make_unique(),
+true);
 
   ErrorOr M = LTOModule::createInLocalContext(
   std::move(Context), mem, length, Options, StringRef(path));
Index: tools/llvm-lto/llvm-lto.cpp
===
--- tools/llvm-lto/llvm-lto.cpp
+++ tools/llvm-lto/llvm-lto.cpp
@@ -235,34 +235,40 @@
 }
 
 static std::string CurrentActivity;
-static void diagnosticHandler(const DiagnosticInfo , void *Context) {
-  raw_ostream  = errs();
-  OS << "llvm-lto: ";
-  switch (DI.getSeverity()) {
-  case DS_Error:
-OS << "error";
-break;
-  case DS_Warning:
-OS << "warning";
-break;
-  case DS_Remark:
-OS << "remark";
-break;
-  case DS_Note:
-OS << "note";
-break;
-  }
-  if (!CurrentActivity.empty())
-OS << ' ' << CurrentActivity;
-  OS << ": ";
-
-  DiagnosticPrinterRawOStream DP(OS);
-  DI.print(DP);
-  OS << '\n';
 
-  if (DI.getSeverity() == DS_Error)
-exit(1);
-}
+namespace {
+  struct LLVMLTODiagnosticHandler : public DiagnosticHandler {
+bool handleDiagnostics(const DiagnosticInfo ) override {
+  raw_ostream  = errs();
+  OS << "llvm-lto: ";
+  switch (DI.getSeverity()) {
+  case DS_Error:
+OS << "error";
+break;
+  case DS_Warning:
+OS << "warning";
+break;
+  case DS_Remark:
+OS << "remark";
+break;
+  case DS_Note:
+OS << "note";
+break;
+  }
+  if (!CurrentActivity.empty())
+OS << ' ' << CurrentActivity;
+  OS << ": ";
+  
+  DiagnosticPrinterRawOStream DP(OS);
+  DI.print(DP);
+  OS << '\n';
+  
+  if (DI.getSeverity() == DS_Error)
+exit(1);
+  return true;
+}
+  };
+  }
 
 static void error(const Twine ) {
   errs() << "llvm-lto: " << Msg << '\n';
@@ -293,7 +299,8 @@
   Buffer = std::move(BufferOrErr.get());
   CurrentActivity = ("loading file '" + Path + "'").str();
   std::unique_ptr Context = llvm::make_unique();
-  Context->setDiagnosticHandler(diagnosticHandler, nullptr, true);
+  Context->setDiagnosticHandler(llvm::make_unique(),
+true);
   ErrorOr Ret = LTOModule::createInLocalContext(
   std::move(Context), Buffer->getBufferStart(), Buffer->getBufferSize(),
   Options, Path);
@@ -837,7 +844,8 @@
   unsigned BaseArg = 0;
 
   LLVMContext Context;
-  

[PATCH] D37196: [Clang] Bug 32352 - Provide a way for OptimizationRemarkEmitter::allowExtraAnalysis to check if (specific) remarks are enabled

[Vivek Pandya via Phabricator via cfe-commits]

vivekvpandya updated this revision to Diff 115085.
vivekvpandya added a comment.

update


https://reviews.llvm.org/D37196

Files:
  lib/CodeGen/CodeGenAction.cpp


Index: lib/CodeGen/CodeGenAction.cpp
===
--- lib/CodeGen/CodeGenAction.cpp
+++ lib/CodeGen/CodeGenAction.cpp
@@ -46,6 +46,31 @@
 using namespace llvm;
 
 namespace clang {
+  class BackendConsumer;
+  class ClangDiagnosticHandler final : public DiagnosticHandler {
+  public:
+ClangDiagnosticHandler(const CodeGenOptions , BackendConsumer *BCon)
+: CodeGenOpts(CGOpts), BackendCon(BCon) {}
+  
+bool handleDiagnostics(const DiagnosticInfo ) override;
+bool isAnalysisRemarkEnable(const std::string ) {
+  return (CodeGenOpts.OptimizationRemarkAnalysisPattern &&
+  CodeGenOpts.OptimizationRemarkAnalysisPattern->match(PassName));
+}
+bool isMissedOptRemarkEnable(const std::string ) {
+  return (CodeGenOpts.OptimizationRemarkMissedPattern &&
+  CodeGenOpts.OptimizationRemarkMissedPattern->match(PassName));
+}
+bool isPassedOptRemarkEnable(const std::string ) {
+  return (CodeGenOpts.OptimizationRemarkPattern &&
+  CodeGenOpts.OptimizationRemarkPattern->match(PassName));
+}
+  
+  private:
+const CodeGenOptions 
+BackendConsumer *BackendCon;
+  };
+
   class BackendConsumer : public ASTConsumer {
 using LinkModule = CodeGenAction::LinkModule;
 
@@ -224,10 +249,10 @@
   void *OldContext = Ctx.getInlineAsmDiagnosticContext();
   Ctx.setInlineAsmDiagnosticHandler(InlineAsmDiagHandler, this);
 
-  LLVMContext::DiagnosticHandlerTy OldDiagnosticHandler =
+  std::unique_ptr OldDiagnosticHandler =
   Ctx.getDiagnosticHandler();
-  void *OldDiagnosticContext = Ctx.getDiagnosticContext();
-  Ctx.setDiagnosticHandler(DiagnosticHandler, this);
+  Ctx.setDiagnosticHandler(llvm::make_unique(
+CodeGenOpts, this));
   Ctx.setDiagnosticsHotnessRequested(CodeGenOpts.DiagnosticsWithHotness);
   if (CodeGenOpts.DiagnosticsHotnessThreshold != 0)
 Ctx.setDiagnosticsHotnessThreshold(
@@ -264,7 +289,7 @@
 
   Ctx.setInlineAsmDiagnosticHandler(OldHandler, OldContext);
 
-  Ctx.setDiagnosticHandler(OldDiagnosticHandler, OldDiagnosticContext);
+  Ctx.setDiagnosticHandler(std::move(OldDiagnosticHandler));
 
   if (OptRecordFile)
 OptRecordFile->keep();
@@ -299,11 +324,6 @@
   ((BackendConsumer*)Context)->InlineAsmDiagHandler2(SM, Loc);
 }
 
-static void DiagnosticHandler(const llvm::DiagnosticInfo ,
-  void *Context) {
-  ((BackendConsumer *)Context)->DiagnosticHandlerImpl(DI);
-}
-
 /// Get the best possible source location to represent a diagnostic that
 /// may have associated debug info.
 const FullSourceLoc
@@ -343,6 +363,11 @@
   void BackendConsumer::anchor() {}
 }
 
+bool ClangDiagnosticHandler::handleDiagnostics(const DiagnosticInfo ) {
+  BackendCon->DiagnosticHandlerImpl(DI);
+  return true;
+}
+
 /// ConvertBackendLocation - Convert a location in a temporary llvm::SourceMgr
 /// buffer to be a valid FullSourceLoc.
 static FullSourceLoc ConvertBackendLocation(const llvm::SMDiagnostic ,


Index: lib/CodeGen/CodeGenAction.cpp
===
--- lib/CodeGen/CodeGenAction.cpp
+++ lib/CodeGen/CodeGenAction.cpp
@@ -46,6 +46,31 @@
 using namespace llvm;
 
 namespace clang {
+  class BackendConsumer;
+  class ClangDiagnosticHandler final : public DiagnosticHandler {
+  public:
+ClangDiagnosticHandler(const CodeGenOptions , BackendConsumer *BCon)
+: CodeGenOpts(CGOpts), BackendCon(BCon) {}
+  
+bool handleDiagnostics(const DiagnosticInfo ) override;
+bool isAnalysisRemarkEnable(const std::string ) {
+  return (CodeGenOpts.OptimizationRemarkAnalysisPattern &&
+  CodeGenOpts.OptimizationRemarkAnalysisPattern->match(PassName));
+}
+bool isMissedOptRemarkEnable(const std::string ) {
+  return (CodeGenOpts.OptimizationRemarkMissedPattern &&
+  CodeGenOpts.OptimizationRemarkMissedPattern->match(PassName));
+}
+bool isPassedOptRemarkEnable(const std::string ) {
+  return (CodeGenOpts.OptimizationRemarkPattern &&
+  CodeGenOpts.OptimizationRemarkPattern->match(PassName));
+}
+  
+  private:
+const CodeGenOptions 
+BackendConsumer *BackendCon;
+  };
+
   class BackendConsumer : public ASTConsumer {
 using LinkModule = CodeGenAction::LinkModule;
 
@@ -224,10 +249,10 @@
   void *OldContext = Ctx.getInlineAsmDiagnosticContext();
   Ctx.setInlineAsmDiagnosticHandler(InlineAsmDiagHandler, this);
 
-  LLVMContext::DiagnosticHandlerTy OldDiagnosticHandler =
+  std::unique_ptr OldDiagnosticHandler =
   Ctx.getDiagnosticHandler();
-  void *OldDiagnosticContext = Ctx.getDiagnosticContext();
-  

[PATCH] D37322: [Sema] Correct typos in LHS, RHS before building a binop expression.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Correcting typo in Sema::ActOnMemberAccessExpr is an interesting idea and it 
almost works. After such change there is a single failure in 
clang/test/SemaCXX/typo-correction-cxx11.cpp

  void run(A *annotations) {
map new_annotations;
  
auto  = *annotations;
auto new_it = new_annotations.find(5);
auto _anotation = new_it.second;  // expected-note {{'new_anotation' 
declared here}}
new_annotation->Swap();  // expected-error {{use of undeclared 
identifier 'new_annotation'; did you mean 'new_anotation'?}}
  }

because we don't mention `did you mean 'new_anotation'?` anymore. I haven't 
investigated in depth what's causing it but my guess is that general typo 
correction doesn't suggest replacement due to ambiguity between new_anotation 
and new_annotations, and prevents member-specific typo correction from handling 
it.

I think having CXXDependentScopeMemberExpr for an ObjC property should be fine 
as we are dealing with invalid code and all typos are already type-dependent 
and instantiation-dependent though it doesn't apply to ObjC.

I agree that my fix isn't specific to the code triggering the assertion. But 
broader fix can be useful as it covers more cases. After checking SemaExpr.cpp 
and its multiple CorrectDelayedTyposInExpr, CorrectTypo, CorrectTypoDelayed I 
have an impression that typo correction absent in Sema::BuildBinOp is an 
omission worth fixing.


https://reviews.llvm.org/D37322



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

r313169 - Driver: Make -fwhole-program-vtables a core option so it can be used from clang-cl.

[Peter Collingbourne via cfe-commits]

Author: pcc
Date: Wed Sep 13 11:36:07 2017
New Revision: 313169

URL: http://llvm.org/viewvc/llvm-project?rev=313169=rev
Log:
Driver: Make -fwhole-program-vtables a core option so it can be used from 
clang-cl.

Also add some missing driver tests for the regular clang driver.

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

Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/test/Driver/whole-program-vtables.c

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=313169=313168=313169=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Wed Sep 13 11:36:07 2017
@@ -1502,9 +1502,10 @@ def fvisibility_ms_compat : Flag<["-"],
   HelpText<"Give global types 'default' visibility and global functions and "
"variables 'hidden' visibility by default">;
 def fwhole_program_vtables : Flag<["-"], "fwhole-program-vtables">, 
Group,
-  Flags<[CC1Option]>,
+  Flags<[CoreOption, CC1Option]>,
   HelpText<"Enables whole-program vtable optimization. Requires -flto">;
-def fno_whole_program_vtables : Flag<["-"], "fno-whole-program-vtables">, 
Group;
+def fno_whole_program_vtables : Flag<["-"], "fno-whole-program-vtables">, 
Group,
+  Flags<[CoreOption]>;
 def fwrapv : Flag<["-"], "fwrapv">, Group, Flags<[CC1Option]>,
   HelpText<"Treat signed integer overflow as two's complement">;
 def fwritable_strings : Flag<["-"], "fwritable-strings">, Group, 
Flags<[CC1Option]>,

Modified: cfe/trunk/test/Driver/whole-program-vtables.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/whole-program-vtables.c?rev=313169=313168=313169=diff
==
--- cfe/trunk/test/Driver/whole-program-vtables.c (original)
+++ cfe/trunk/test/Driver/whole-program-vtables.c Wed Sep 13 11:36:07 2017
@@ -1,2 +1,11 @@
 // RUN: %clang -target x86_64-unknown-linux -fwhole-program-vtables -### %s 
2>&1 | FileCheck --check-prefix=NO-LTO %s
+// RUN: %clang_cl --target=x86_64-pc-win32 -fwhole-program-vtables -### %s 
2>&1 | FileCheck --check-prefix=NO-LTO %s
 // NO-LTO: invalid argument '-fwhole-program-vtables' only allowed with '-flto'
+
+// RUN: %clang -target x86_64-unknown-linux -fwhole-program-vtables -flto -### 
%s 2>&1 | FileCheck --check-prefix=LTO %s
+// RUN: %clang_cl --target=x86_64-pc-win32 -fwhole-program-vtables -flto -### 
%s 2>&1 | FileCheck --check-prefix=LTO %s
+// LTO: "-fwhole-program-vtables"
+
+// RUN: %clang -target x86_64-unknown-linux -fwhole-program-vtables 
-fno-whole-program-vtables -flto -### %s 2>&1 | FileCheck 
--check-prefix=LTO-DISABLE %s
+// RUN: %clang_cl --target=x86_64-pc-win32 -fwhole-program-vtables 
-fno-whole-program-vtables -flto -### %s 2>&1 | FileCheck 
--check-prefix=LTO-DISABLE %s
+// LTO-DISABLE-NOT: "-fwhole-program-vtables"


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

[PATCH] D37787: Driver: Make -fwhole-program-vtables a core option so it can be used from clang-cl.

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL313169: Driver: Make -fwhole-program-vtables a core option 
so it can be used from clang… (authored by pcc).

Changed prior to commit:
  https://reviews.llvm.org/D37787?vs=114960=115082#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37787

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/test/Driver/whole-program-vtables.c


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1502,9 +1502,10 @@
   HelpText<"Give global types 'default' visibility and global functions and "
"variables 'hidden' visibility by default">;
 def fwhole_program_vtables : Flag<["-"], "fwhole-program-vtables">, 
Group,
-  Flags<[CC1Option]>,
+  Flags<[CoreOption, CC1Option]>,
   HelpText<"Enables whole-program vtable optimization. Requires -flto">;
-def fno_whole_program_vtables : Flag<["-"], "fno-whole-program-vtables">, 
Group;
+def fno_whole_program_vtables : Flag<["-"], "fno-whole-program-vtables">, 
Group,
+  Flags<[CoreOption]>;
 def fwrapv : Flag<["-"], "fwrapv">, Group, Flags<[CC1Option]>,
   HelpText<"Treat signed integer overflow as two's complement">;
 def fwritable_strings : Flag<["-"], "fwritable-strings">, Group, 
Flags<[CC1Option]>,
Index: cfe/trunk/test/Driver/whole-program-vtables.c
===
--- cfe/trunk/test/Driver/whole-program-vtables.c
+++ cfe/trunk/test/Driver/whole-program-vtables.c
@@ -1,2 +1,11 @@
 // RUN: %clang -target x86_64-unknown-linux -fwhole-program-vtables -### %s 
2>&1 | FileCheck --check-prefix=NO-LTO %s
+// RUN: %clang_cl --target=x86_64-pc-win32 -fwhole-program-vtables -### %s 
2>&1 | FileCheck --check-prefix=NO-LTO %s
 // NO-LTO: invalid argument '-fwhole-program-vtables' only allowed with '-flto'
+
+// RUN: %clang -target x86_64-unknown-linux -fwhole-program-vtables -flto -### 
%s 2>&1 | FileCheck --check-prefix=LTO %s
+// RUN: %clang_cl --target=x86_64-pc-win32 -fwhole-program-vtables -flto -### 
%s 2>&1 | FileCheck --check-prefix=LTO %s
+// LTO: "-fwhole-program-vtables"
+
+// RUN: %clang -target x86_64-unknown-linux -fwhole-program-vtables 
-fno-whole-program-vtables -flto -### %s 2>&1 | FileCheck 
--check-prefix=LTO-DISABLE %s
+// RUN: %clang_cl --target=x86_64-pc-win32 -fwhole-program-vtables 
-fno-whole-program-vtables -flto -### %s 2>&1 | FileCheck 
--check-prefix=LTO-DISABLE %s
+// LTO-DISABLE-NOT: "-fwhole-program-vtables"


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1502,9 +1502,10 @@
   HelpText<"Give global types 'default' visibility and global functions and "
"variables 'hidden' visibility by default">;
 def fwhole_program_vtables : Flag<["-"], "fwhole-program-vtables">, Group,
-  Flags<[CC1Option]>,
+  Flags<[CoreOption, CC1Option]>,
   HelpText<"Enables whole-program vtable optimization. Requires -flto">;
-def fno_whole_program_vtables : Flag<["-"], "fno-whole-program-vtables">, Group;
+def fno_whole_program_vtables : Flag<["-"], "fno-whole-program-vtables">, Group,
+  Flags<[CoreOption]>;
 def fwrapv : Flag<["-"], "fwrapv">, Group, Flags<[CC1Option]>,
   HelpText<"Treat signed integer overflow as two's complement">;
 def fwritable_strings : Flag<["-"], "fwritable-strings">, Group, Flags<[CC1Option]>,
Index: cfe/trunk/test/Driver/whole-program-vtables.c
===
--- cfe/trunk/test/Driver/whole-program-vtables.c
+++ cfe/trunk/test/Driver/whole-program-vtables.c
@@ -1,2 +1,11 @@
 // RUN: %clang -target x86_64-unknown-linux -fwhole-program-vtables -### %s 2>&1 | FileCheck --check-prefix=NO-LTO %s
+// RUN: %clang_cl --target=x86_64-pc-win32 -fwhole-program-vtables -### %s 2>&1 | FileCheck --check-prefix=NO-LTO %s
 // NO-LTO: invalid argument '-fwhole-program-vtables' only allowed with '-flto'
+
+// RUN: %clang -target x86_64-unknown-linux -fwhole-program-vtables -flto -### %s 2>&1 | FileCheck --check-prefix=LTO %s
+// RUN: %clang_cl --target=x86_64-pc-win32 -fwhole-program-vtables -flto -### %s 2>&1 | FileCheck --check-prefix=LTO %s
+// LTO: "-fwhole-program-vtables"
+
+// RUN: %clang -target x86_64-unknown-linux -fwhole-program-vtables -fno-whole-program-vtables -flto -### %s 2>&1 | FileCheck --check-prefix=LTO-DISABLE %s
+// RUN: %clang_cl --target=x86_64-pc-win32 -fwhole-program-vtables -fno-whole-program-vtables -flto -### %s 2>&1 | FileCheck --check-prefix=LTO-DISABLE %s
+// LTO-DISABLE-NOT: "-fwhole-program-vtables"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37787: Driver: Make -fwhole-program-vtables a core option so it can be used from clang-cl.

[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


https://reviews.llvm.org/D37787



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

[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D37808#869879, @JonasToth wrote:

> In https://reviews.llvm.org/D37808#869823, @lebedev.ri wrote:
>
> > What about GNU extension `case 1 ... 3:` ?
>
>
> Strictly speaking, the coding standard (which should be enforced by the 
> patch) requires strict ISO C++11, therefore this extension is not considered 
> directly.




> Does clang support this extension?

It does:

  test.cpp:3:12: warning: use of GNU case range extension [-Wgnu-case-range]
  case 1 ... 3:
 ^



> When this would be a warning, then that case should be added as well.




https://reviews.llvm.org/D37808



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

[PATCH] D37703: [AMDGPU] Change addr space of clk_event_t, queue_t and reserve_id_t to global

[Brian Sumner via Phabricator via cfe-commits]

b-sumner accepted this revision.
b-sumner added a comment.
This revision is now accepted and ready to land.

Looks good to me.


https://reviews.llvm.org/D37703



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

[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In https://reviews.llvm.org/D37808#869823, @lebedev.ri wrote:

> What about GNU extension `case 1 ... 3:` ?


Strictly speaking, the coding standard (which should be enforced by the patch) 
requires strict ISO C++11, therefore this extension is not considered directly.
Does clang support this extension? When this would be a warning, then that case 
should be added as well.


https://reviews.llvm.org/D37808



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

[PATCH] D37436: Initial implementation of C attributes (WG14 N2137)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D37436#869851, @hfinkel wrote:

> In https://reviews.llvm.org/D37436#869467, @aaron.ballman wrote:
>
> > In https://reviews.llvm.org/D37436#869462, @hfinkel wrote:
> >
> > > I think that I misunderstood your concern. Let me see if I can summarize 
> > > your position: You believe that, when GCC implements this syntax in C, 
> > > they will audit their attributes and not support all of their existing 
> > > `gnu::` attributes in C. You only want us to support these when we know 
> > > what that list will be (which we don't yet). Is that correct?
> >
> >
> > Yes, that is correct.
>
>
> Okay. A large fraction of the number of attributes we'll want to use are 
> going to fall into this category (because Clang doesn't have its own 
> attributes, but copied GCC's, for many things). I don't think we'll get good 
> implementation feedback until we have this figured out. If we can't sync with 
> GCC soon, I suggest just making a reasonable guess. My first choice would be 
> to just allowing everything, and then we can see what people found to be 
> useful and why. Our experience here can also provide feedback to GCC (and we 
> can always update late if needed - it is still an experimental feature).


I think this direction is a reasonable one, but not for the initial syntax 
patch. I would prefer to be conservative and get the basic syntax in first. My 
plan is that once the syntax is available, I would start adding more 
attributes, starting with the ones proposed to WG14, followed by the existing 
C++ ones in the clang namespace. I think the gnu attributes would likely be 
immediately following the clang ones, so they're definitely a high priority for 
me to support.

Btw, because I wasn't explicit about this, we're in violent agreement that 
getting implementation experience about how well we can share attributes 
between C and C++ code bases is also an important question to answer.


https://reviews.llvm.org/D37436



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

[PATCH] D37436: Initial implementation of C attributes (WG14 N2137)

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D37436#869467, @aaron.ballman wrote:

> In https://reviews.llvm.org/D37436#869462, @hfinkel wrote:
>
> > I think that I misunderstood your concern. Let me see if I can summarize 
> > your position: You believe that, when GCC implements this syntax in C, they 
> > will audit their attributes and not support all of their existing `gnu::` 
> > attributes in C. You only want us to support these when we know what that 
> > list will be (which we don't yet). Is that correct?
>
>
> Yes, that is correct.


Okay. A large fraction of the number of attributes we'll want to use are going 
to fall into this category (because Clang doesn't have its own attributes, but 
copied GCC's, for many things). I don't think we'll get good implementation 
feedback until we have this figured out. If we can't sync with GCC soon, I 
suggest just making a reasonable guess. My first choice would be to just 
allowing everything, and then we can see what people found to be useful and 
why. Our experience here can also provide feedback to GCC (and we can always 
update late if needed - it is still an experimental feature).


https://reviews.llvm.org/D37436



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

r313162 - Attempt to fix MSVC build.

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Wed Sep 13 10:45:51 2017
New Revision: 313162

URL: http://llvm.org/viewvc/llvm-project?rev=313162=rev
Log:
Attempt to fix MSVC build.

Modified:
cfe/trunk/lib/Driver/Job.cpp

Modified: cfe/trunk/lib/Driver/Job.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Job.cpp?rev=313162=313161=313162=diff
==
--- cfe/trunk/lib/Driver/Job.cpp (original)
+++ cfe/trunk/lib/Driver/Job.cpp Wed Sep 13 10:45:51 2017
@@ -14,6 +14,7 @@
 #include "clang/Driver/Tool.h"
 #include "clang/Driver/ToolChain.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
@@ -307,7 +308,7 @@ void Command::setEnvironment(llvm::Array
   Environment.push_back(nullptr);
 }
 
-int Command::Execute(ArrayRef Redirects,
+int Command::Execute(ArrayRef Redirects,
  std::string *ErrMsg, bool *ExecutionFailed) const {
   SmallVector Argv;
 
@@ -378,7 +379,7 @@ static bool ShouldFallback(int ExitCode)
   return ExitCode != 0;
 }
 
-int FallbackCommand::Execute(ArrayRef Redirects,
+int FallbackCommand::Execute(ArrayRef Redirects,
  std::string *ErrMsg, bool *ExecutionFailed) const 
{
   int PrimaryStatus = Command::Execute(Redirects, ErrMsg, ExecutionFailed);
   if (!ShouldFallback(PrimaryStatus))
@@ -410,7 +411,7 @@ void ForceSuccessCommand::Print(raw_ostr
   OS << " || (exit 0)" << Terminator;
 }
 
-int ForceSuccessCommand::Execute(ArrayRef Redirects,
+int ForceSuccessCommand::Execute(ArrayRef Redirects,
  std::string *ErrMsg,
  bool *ExecutionFailed) const {
   int Status = Command::Execute(Redirects, ErrMsg, ExecutionFailed);


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

[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

What about GNU extension `case 1 ... 3:` ?


https://reviews.llvm.org/D37808



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

[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

In https://reviews.llvm.org/D37808#869612, @JonasToth wrote:

> In https://reviews.llvm.org/D37808#869602, @Eugene.Zelenko wrote:
>
> > I think will be good idea to extend -Wswitch diagnostics.
>
>
> Ok. But it will introduce new warnings to llvm codebase itself. I prepare 
> some example output i found right now.


If number of them will not be huge, it'll be worth to fix before extended 
-Wswitch will be committed.


https://reviews.llvm.org/D37808



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

[PATCH] D37785: [Fuchsia] Set ENABLE_X86_RELAX_RELOCATIONS for Fuchsia builds

[Petr Hosek via Phabricator via cfe-commits]

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

LGTM


Repository:
  rL LLVM

https://reviews.llvm.org/D37785



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

[PATCH] D37564: Update users of llvm::sys::ExecuteAndWait etc.

[Alexander Kornienko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL313156: Update users of llvm::sys::ExecuteAndWait etc. 
(authored by alexfh).

Repository:
  rL LLVM

https://reviews.llvm.org/D37564

Files:
  cfe/trunk/include/clang/Driver/Compilation.h
  cfe/trunk/include/clang/Driver/Job.h
  cfe/trunk/lib/Driver/Compilation.cpp
  cfe/trunk/lib/Driver/Job.cpp
  cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

Index: cfe/trunk/include/clang/Driver/Compilation.h
===
--- cfe/trunk/include/clang/Driver/Compilation.h
+++ cfe/trunk/include/clang/Driver/Compilation.h
@@ -99,8 +99,8 @@
   /// only be removed if we crash.
   ArgStringMap FailureResultFiles;
 
-  /// Redirection for stdout, stderr, etc.
-  const StringRef **Redirects;
+  /// Optional redirection for stdin, stdout, stderr.
+  std::vector Redirects;
 
   /// Whether we're compiling for diagnostic purposes.
   bool ForDiagnostics;
@@ -283,12 +283,10 @@
 
   /// Redirect - Redirect output of this compilation. Can only be done once.
   ///
-  /// \param Redirects - array of pointers to paths. The array
-  /// should have a size of three. The inferior process's
-  /// stdin(0), stdout(1), and stderr(2) will be redirected to the
-  /// corresponding paths. This compilation instance becomes
-  /// the owner of Redirects and will delete the array and StringRef's.
-  void Redirect(const StringRef** Redirects);
+  /// \param Redirects - array of optional paths. The array should have a size
+  /// of three. The inferior process's stdin(0), stdout(1), and stderr(2) will
+  /// be redirected to the corresponding paths, if provided (not llvm::None).
+  void Redirect(ArrayRef Redirects);
 };
 
 } // end namespace driver
Index: cfe/trunk/include/clang/Driver/Job.h
===
--- cfe/trunk/include/clang/Driver/Job.h
+++ cfe/trunk/include/clang/Driver/Job.h
@@ -97,8 +97,8 @@
   virtual void Print(llvm::raw_ostream , const char *Terminator, bool Quote,
  CrashReportInfo *CrashInfo = nullptr) const;
 
-  virtual int Execute(const StringRef **Redirects, std::string *ErrMsg,
-  bool *ExecutionFailed) const;
+  virtual int Execute(ArrayRef Redirects,
+  std::string *ErrMsg, bool *ExecutionFailed) const;
 
   /// getSource - Return the Action which caused the creation of this job.
   const Action () const { return Source; }
@@ -141,7 +141,7 @@
   void Print(llvm::raw_ostream , const char *Terminator, bool Quote,
  CrashReportInfo *CrashInfo = nullptr) const override;
 
-  int Execute(const StringRef **Redirects, std::string *ErrMsg,
+  int Execute(ArrayRef Redirects, std::string *ErrMsg,
   bool *ExecutionFailed) const override;
 
 private:
@@ -158,7 +158,7 @@
   void Print(llvm::raw_ostream , const char *Terminator, bool Quote,
  CrashReportInfo *CrashInfo = nullptr) const override;
 
-  int Execute(const StringRef **Redirects, std::string *ErrMsg,
+  int Execute(ArrayRef Redirects, std::string *ErrMsg,
   bool *ExecutionFailed) const override;
 };
 
Index: cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -854,8 +854,7 @@
 Ubiviz = *Path;
   const char *args[] = {Ubiviz.c_str(), Filename.c_str(), nullptr};
 
-  if (llvm::sys::ExecuteAndWait(Ubiviz, [0], nullptr, nullptr, 0, 0,
-)) {
+  if (llvm::sys::ExecuteAndWait(Ubiviz, [0], nullptr, {}, 0, 0, )) {
 llvm::errs() << "Error viewing graph: " << ErrMsg << "\n";
   }
 
Index: cfe/trunk/lib/Driver/Compilation.cpp
===
--- cfe/trunk/lib/Driver/Compilation.cpp
+++ cfe/trunk/lib/Driver/Compilation.cpp
@@ -26,8 +26,8 @@
  InputArgList *_Args, DerivedArgList *_TranslatedArgs,
  bool ContainsError)
 : TheDriver(D), DefaultToolChain(_DefaultToolChain), ActiveOffloadMask(0u),
-  Args(_Args), TranslatedArgs(_TranslatedArgs), Redirects(nullptr),
-  ForDiagnostics(false), ContainsError(ContainsError) {
+  Args(_Args), TranslatedArgs(_TranslatedArgs), ForDiagnostics(false),
+  ContainsError(ContainsError) {
   // The offloading host toolchain is the default tool chain.
   OrderedOffloadingToolchains.insert(
   std::make_pair(Action::OFK_Host, ));
@@ -41,14 +41,6 @@
   for (auto Arg : TCArgs)
 if (Arg.second != TranslatedArgs)
   delete Arg.second;
-
-  // Free redirections of stdout/stderr.
-  if (Redirects) {
-delete Redirects[0];
-delete Redirects[1];
-delete Redirects[2];
-delete [] Redirects;
-  }
 }
 
 const 

r313156 - Update users of llvm::sys::ExecuteAndWait etc.

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Wed Sep 13 10:03:58 2017
New Revision: 313156

URL: http://llvm.org/viewvc/llvm-project?rev=313156=rev
Log:
Update users of llvm::sys::ExecuteAndWait etc.

Summary: Clang part of https://reviews.llvm.org/D37563

Reviewers: bkramer

Subscribers: vsk, cfe-commits

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

Modified:
cfe/trunk/include/clang/Driver/Compilation.h
cfe/trunk/include/clang/Driver/Job.h
cfe/trunk/lib/Driver/Compilation.cpp
cfe/trunk/lib/Driver/Job.cpp
cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

Modified: cfe/trunk/include/clang/Driver/Compilation.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Compilation.h?rev=313156=313155=313156=diff
==
--- cfe/trunk/include/clang/Driver/Compilation.h (original)
+++ cfe/trunk/include/clang/Driver/Compilation.h Wed Sep 13 10:03:58 2017
@@ -99,8 +99,8 @@ class Compilation {
   /// only be removed if we crash.
   ArgStringMap FailureResultFiles;
 
-  /// Redirection for stdout, stderr, etc.
-  const StringRef **Redirects;
+  /// Optional redirection for stdin, stdout, stderr.
+  std::vector Redirects;
 
   /// Whether we're compiling for diagnostic purposes.
   bool ForDiagnostics;
@@ -283,12 +283,10 @@ public:
 
   /// Redirect - Redirect output of this compilation. Can only be done once.
   ///
-  /// \param Redirects - array of pointers to paths. The array
-  /// should have a size of three. The inferior process's
-  /// stdin(0), stdout(1), and stderr(2) will be redirected to the
-  /// corresponding paths. This compilation instance becomes
-  /// the owner of Redirects and will delete the array and StringRef's.
-  void Redirect(const StringRef** Redirects);
+  /// \param Redirects - array of optional paths. The array should have a size
+  /// of three. The inferior process's stdin(0), stdout(1), and stderr(2) will
+  /// be redirected to the corresponding paths, if provided (not llvm::None).
+  void Redirect(ArrayRef Redirects);
 };
 
 } // end namespace driver

Modified: cfe/trunk/include/clang/Driver/Job.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Job.h?rev=313156=313155=313156=diff
==
--- cfe/trunk/include/clang/Driver/Job.h (original)
+++ cfe/trunk/include/clang/Driver/Job.h Wed Sep 13 10:03:58 2017
@@ -97,8 +97,8 @@ public:
   virtual void Print(llvm::raw_ostream , const char *Terminator, bool Quote,
  CrashReportInfo *CrashInfo = nullptr) const;
 
-  virtual int Execute(const StringRef **Redirects, std::string *ErrMsg,
-  bool *ExecutionFailed) const;
+  virtual int Execute(ArrayRef Redirects,
+  std::string *ErrMsg, bool *ExecutionFailed) const;
 
   /// getSource - Return the Action which caused the creation of this job.
   const Action () const { return Source; }
@@ -141,7 +141,7 @@ public:
   void Print(llvm::raw_ostream , const char *Terminator, bool Quote,
  CrashReportInfo *CrashInfo = nullptr) const override;
 
-  int Execute(const StringRef **Redirects, std::string *ErrMsg,
+  int Execute(ArrayRef Redirects, std::string *ErrMsg,
   bool *ExecutionFailed) const override;
 
 private:
@@ -158,7 +158,7 @@ public:
   void Print(llvm::raw_ostream , const char *Terminator, bool Quote,
  CrashReportInfo *CrashInfo = nullptr) const override;
 
-  int Execute(const StringRef **Redirects, std::string *ErrMsg,
+  int Execute(ArrayRef Redirects, std::string *ErrMsg,
   bool *ExecutionFailed) const override;
 };
 

Modified: cfe/trunk/lib/Driver/Compilation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Compilation.cpp?rev=313156=313155=313156=diff
==
--- cfe/trunk/lib/Driver/Compilation.cpp (original)
+++ cfe/trunk/lib/Driver/Compilation.cpp Wed Sep 13 10:03:58 2017
@@ -26,8 +26,8 @@ Compilation::Compilation(const Driver 
  InputArgList *_Args, DerivedArgList *_TranslatedArgs,
  bool ContainsError)
 : TheDriver(D), DefaultToolChain(_DefaultToolChain), ActiveOffloadMask(0u),
-  Args(_Args), TranslatedArgs(_TranslatedArgs), Redirects(nullptr),
-  ForDiagnostics(false), ContainsError(ContainsError) {
+  Args(_Args), TranslatedArgs(_TranslatedArgs), ForDiagnostics(false),
+  ContainsError(ContainsError) {
   // The offloading host toolchain is the default tool chain.
   OrderedOffloadingToolchains.insert(
   std::make_pair(Action::OFK_Host, ));
@@ -41,14 +41,6 @@ Compilation::~Compilation() {
   for (auto Arg : TCArgs)
 if (Arg.second != TranslatedArgs)
   delete Arg.second;
-
-  // Free redirections of stdout/stderr.
-  if (Redirects) {
-delete 

[PATCH] D37564: Update users of llvm::sys::ExecuteAndWait etc.

[Benjamin Kramer via Phabricator via cfe-commits]

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

lgtm too


https://reviews.llvm.org/D37564



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

[PATCH] D37813: clang-format: better handle namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz created this revision.

Other macros are used to declare namespaces, and should thus be handled
similarly. This is the case for crpcut's TESTSUITE macro, or for
unittest-cpp's SUITE macro:

  TESTSUITE(Foo) {
  TEST(MyFirstTest) {
assert(0);
  }
  } // TESTSUITE(Foo)

This patch deals with this cases by introducing a new option to specify
lists of namespace macros. Internally, it re-uses the system already in
place for foreach and statement macros, to ensure there is no impact on
performance.


https://reviews.llvm.org/D37813

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/FormatTokenLexer.cpp
  lib/Format/NamespaceEndCommentsFixer.cpp
  lib/Format/TokenAnnotator.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp
  unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -53,6 +53,7 @@
 "  int i;\n"
 "  int j;\n"
 "}"));
+
   EXPECT_EQ("namespace {\n"
 "  int i;\n"
 "  int j;\n"
@@ -249,6 +250,85 @@
 "// unrelated"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, AddsMacroEndComment) {
+  FormatStyle Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("TESTSUITE");
+
+  EXPECT_EQ("TESTSUITE() {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE()",
+fixNamespaceEndComments("TESTSUITE() {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+
+  EXPECT_EQ("TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("inline TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("inline TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(::A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A)",
+fixNamespaceEndComments("TESTSUITE(::A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(::A::B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A::B)",
+fixNamespaceEndComments("TESTSUITE(::A::B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(/**/::/**/A/**/::/**/B/**/) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A::B)",
+fixNamespaceEndComments("TESTSUITE(/**/::/**/A/**/::/**/B/**/) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(A, B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("TESTSUITE(A, B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(\"Test1\") {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(\"Test1\")",
+fixNamespaceEndComments("TESTSUITE(\"Test1\") {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+}
+
 TEST_F(NamespaceEndCommentsFixerTest, AddsNewlineIfNeeded) {
   EXPECT_EQ("namespace A {\n"
 "  int i;\n"
@@ -381,6 +461,54 @@
 "}; /* unnamed namespace */"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, KeepsValidMacroEndComment) {
+  FormatStyle Style = getLLVMStyle();
+  

[PATCH] D33440: clang-format: better handle statement and namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 115057.
Typz added a comment.

Split diff: handle only statements in here, namespace macros will be moved to 
another one.


https://reviews.llvm.org/D33440

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/FormatTokenLexer.cpp
  lib/Format/FormatTokenLexer.h
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -2385,6 +2385,26 @@
getLLVMStyleWithColumns(40)));
 
   verifyFormat("MACRO(>)");
+
+  // Some macros contain an implicit semicolon
+  FormatStyle Style = getLLVMStyle();
+  Style.StatementMacros.push_back("FOO");
+  verifyFormat("FOO(a) int b = 0;");
+  verifyFormat("FOO(a)\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO(a);\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO(argc, argv, \"4.0.2\")\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO()\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO\n"
+   "int b = 0;",
+   Style);
 }
 
 TEST_F(FormatTest, LayoutMacroDefinitionsStatementsSpanningBlocks) {
@@ -10185,6 +10205,12 @@
   CHECK_PARSE("ForEachMacros: [BOOST_FOREACH, Q_FOREACH]", ForEachMacros,
   BoostAndQForeach);
 
+  Style.StatementMacros.clear();
+  CHECK_PARSE("StatementMacros: [QUNUSED]", StatementMacros,
+  std::vector{"QUNUSED"});
+  CHECK_PARSE("StatementMacros: [QUNUSED, QT_REQUIRE_VERSION]", StatementMacros,
+  std::vector({"QUNUSED", "QT_REQUIRE_VERSION"}));
+
   Style.IncludeCategories.clear();
   std::vector ExpectedCategories = {{"abc/.*", 2},
   {".*", 1}};
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1089,6 +1089,15 @@
 return;
   }
 }
+if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+  nextToken();
+  if (FormatTok->is(tok::l_paren))
+parseParens();
+  if (FormatTok->is(tok::semi))
+nextToken();
+  addUnwrappedLine();
+  return;
+}
 // In all other cases, parse the declaration.
 break;
   default:
Index: lib/Format/FormatTokenLexer.h
===
--- lib/Format/FormatTokenLexer.h
+++ lib/Format/FormatTokenLexer.h
@@ -97,7 +97,29 @@
   // Index (in 'Tokens') of the last token that starts a new line.
   unsigned FirstInLineIndex;
   SmallVector Tokens;
-  SmallVector ForEachMacros;
+
+  struct MacroInfo {
+MacroInfo() : Identifier(NULL), TokType(TT_Unknown) {}
+MacroInfo(IdentifierInfo *Identifier, TokenType TokType)
+: Identifier(Identifier), TokType(TokType) {}
+
+IdentifierInfo *Identifier;
+TokenType TokType;
+
+bool operator==(const MacroInfo ) const {
+  return Identifier == Other.Identifier;
+}
+bool operator==(const IdentifierInfo *Other) const {
+  return Identifier == Other;
+}
+bool operator<(const MacroInfo ) const {
+  return Identifier < Other.Identifier;
+}
+bool operator<(const IdentifierInfo *Other) const {
+  return Identifier < Other;
+}
+  };
+  SmallVector Macros;
 
   bool FormattingDisabled;
 
Index: lib/Format/FormatTokenLexer.cpp
===
--- lib/Format/FormatTokenLexer.cpp
+++ lib/Format/FormatTokenLexer.cpp
@@ -37,8 +37,10 @@
   Lex->SetKeepWhitespaceMode(true);
 
   for (const std::string  : Style.ForEachMacros)
-ForEachMacros.push_back((ForEachMacro));
-  std::sort(ForEachMacros.begin(), ForEachMacros.end());
+Macros.emplace_back((ForEachMacro), TT_ForEachMacro);
+  for (const std::string  : Style.StatementMacros)
+Macros.emplace_back((StatementMacro), TT_StatementMacro);
+  std::sort(Macros.begin(), Macros.end());
 }
 
 ArrayRef FormatTokenLexer::lex() {
@@ -633,12 +635,13 @@
   }
 
   if (Style.isCpp()) {
+decltype(Macros)::iterator it;
 if (!(Tokens.size() > 0 && Tokens.back()->Tok.getIdentifierInfo() &&
   Tokens.back()->Tok.getIdentifierInfo()->getPPKeywordID() ==
   tok::pp_define) &&
-std::find(ForEachMacros.begin(), ForEachMacros.end(),
-  FormatTok->Tok.getIdentifierInfo()) != ForEachMacros.end()) {
-  FormatTok->Type = TT_ForEachMacro;
+(it = std::find(Macros.begin(), Macros.end(),
+FormatTok->Tok.getIdentifierInfo())) != Macros.end()) {
+  FormatTok->Type = it->TokType;
 } else if (FormatTok->is(tok::identifier)) {
   if (MacroBlockBeginRegex.match(Text)) {
 

[PATCH] D37812: [analyzer] PthreadLock: Escape the pointers.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.

As usual, we need to invalidate mutex states when something may touch them. 
Implement this boilerplate for the thread lock checker.

The previous refactoring is handy for listing functions on which we don't need 
to invalidate mutex states because we model them instead.

Additionally, i don't invalidate mutex states when any system header function 
is called, unless the mutex is passed directly into it. The TODO here would be 
to model *all* system functions that may change mutex states, and then disable 
invalidation for the rest of them even if they take a mutex; that's what other 
checkers do.


https://reviews.llvm.org/D37812

Files:
  lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
  test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h
  test/Analysis/pthreadlock.c

Index: test/Analysis/pthreadlock.c
===
--- test/Analysis/pthreadlock.c
+++ test/Analysis/pthreadlock.c
@@ -221,6 +221,27 @@
   lck_rw_unlock_exclusive(); // no-warning
 }
 
+void escape_mutex(pthread_mutex_t *m);
+void ok30(void) {
+  pthread_mutex_t local_mtx;
+  pthread_mutex_init(_mtx, NULL);
+  pthread_mutex_lock(_mtx);
+  escape_mutex(_mtx);
+  pthread_mutex_lock(_mtx); // no-warning
+  pthread_mutex_unlock(_mtx);
+  pthread_mutex_destroy(_mtx);
+}
+
+void ok31(void) {
+  pthread_mutex_t local_mtx;
+  pthread_mutex_init(_mtx, NULL);
+  pthread_mutex_lock(_mtx);
+  fake_system_function_that_takes_a_mutex(_mtx);
+  pthread_mutex_lock(_mtx); // no-warning
+  pthread_mutex_unlock(_mtx);
+  pthread_mutex_destroy(_mtx);
+}
+
 void
 bad1(void)
 {
@@ -486,3 +507,9 @@
   lck_rw_lock_exclusive();
   lck_rw_unlock_shared(); // FIXME: warn - should be exclusive?
 }
+
+void bad33(void) {
+  pthread_mutex_lock(pmtx);
+  fake_system_function();
+  pthread_mutex_lock(pmtx); // expected-warning{{This lock has already been acquired}}
+}
Index: test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h
===
--- test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h
+++ test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h
@@ -3,6 +3,8 @@
 // should not invalidate regions of their arguments.
 #pragma clang system_header
 
+extern void fake_system_function();
+
 typedef struct {
   void *foo;
 } OpaqueThing;
@@ -12,6 +14,8 @@
 typedef OpaqueThing pthread_rwlock_t;
 typedef OpaqueThing pthread_mutexattr_t;
 
+extern void fake_system_function_that_takes_a_mutex(pthread_mutex_t *mtx);
+
 // XNU.
 typedef OpaqueThing lck_mtx_t;
 typedef OpaqueThing lck_rw_t;
Index: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
+++ lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
@@ -66,8 +66,8 @@
   }
 };
 
-class PthreadLockChecker
-: public Checker {
+class PthreadLockChecker : public Checker {
   mutable std::unique_ptr BT_doublelock;
   mutable std::unique_ptr BT_doubleunlock;
   mutable std::unique_ptr BT_destroylock;
@@ -124,6 +124,11 @@
 
   void checkPostCall(const CallEvent , CheckerContext ) const;
   void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
+  ProgramStateRef
+  checkRegionChanges(ProgramStateRef State, const InvalidatedSymbols *Symbols,
+ ArrayRef ExplicitRegions,
+ ArrayRef Regions,
+ const LocationContext *LCtx, const CallEvent *Call) const;
   void printState(raw_ostream , ProgramStateRef State,
   const char *NL, const char *Sep) const override;
 };
@@ -567,6 +572,38 @@
   C.addTransition(State);
 }
 
+ProgramStateRef PthreadLockChecker::checkRegionChanges(
+ProgramStateRef State, const InvalidatedSymbols *Symbols,
+ArrayRef ExplicitRegions,
+ArrayRef Regions, const LocationContext *LCtx,
+const CallEvent *Call) const {
+
+  bool IsLibraryFunction = false;
+  if (Call && Call->isGlobalCFunction()) {
+// Avoid invalidating mutex state when a known supported function is called.
+for (auto I : SupportedAPIs)
+  if (Call->isCalled(I.Name))
+return State;
+if (Call->isInSystemHeader())
+  IsLibraryFunction = true;
+  }
+
+  for (auto R : Regions) {
+// We assume that system library function wouldn't touch the mutex unless
+// it takes the mutex explicitly as an argument.
+if (IsLibraryFunction &&
+std::find(ExplicitRegions.begin(), ExplicitRegions.end(), R) ==
+ExplicitRegions.end())
+  continue;
+
+State = State->remove(R);
+State = State->remove(R);
+// TODO: Destroy the lock stack as well.
+  }
+
+  return State;
+}
+
 void ento::registerPthreadLockChecker(CheckerManager ) {
   mgr.registerChecker();
 }

[PATCH] D33440: clang-format: better handle statement and namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 115054.
Typz marked 2 inline comments as done.
Typz added a comment.

Fix review comments, before splitting the commit.


https://reviews.llvm.org/D33440

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/FormatTokenLexer.cpp
  lib/Format/FormatTokenLexer.h
  lib/Format/NamespaceEndCommentsFixer.cpp
  lib/Format/TokenAnnotator.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp
  unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -53,6 +53,7 @@
 "  int i;\n"
 "  int j;\n"
 "}"));
+
   EXPECT_EQ("namespace {\n"
 "  int i;\n"
 "  int j;\n"
@@ -249,6 +250,85 @@
 "// unrelated"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, AddsMacroEndComment) {
+  FormatStyle Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("TESTSUITE");
+
+  EXPECT_EQ("TESTSUITE() {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE()",
+fixNamespaceEndComments("TESTSUITE() {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+
+  EXPECT_EQ("TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("inline TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("inline TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(::A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A)",
+fixNamespaceEndComments("TESTSUITE(::A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(::A::B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A::B)",
+fixNamespaceEndComments("TESTSUITE(::A::B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(/**/::/**/A/**/::/**/B/**/) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A::B)",
+fixNamespaceEndComments("TESTSUITE(/**/::/**/A/**/::/**/B/**/) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(A, B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("TESTSUITE(A, B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(\"Test1\") {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(\"Test1\")",
+fixNamespaceEndComments("TESTSUITE(\"Test1\") {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+}
+
 TEST_F(NamespaceEndCommentsFixerTest, AddsNewlineIfNeeded) {
   EXPECT_EQ("namespace A {\n"
 "  int i;\n"
@@ -381,6 +461,54 @@
 "}; /* unnamed namespace */"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, KeepsValidMacroEndComment) {
+  FormatStyle Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("TESTSUITE");
+
+  EXPECT_EQ("TESTSUITE() {\n"
+"  int i;\n"
+"} // end anonymous TESTSUITE()",
+fixNamespaceEndComments("TESTSUITE() {\n"
+"  int i;\n"
+"} // end anonymous TESTSUITE()",
+  

[PATCH] D37806: [analyzer] PthreadLock: Fix return values of XNU lock functions.

[Anna Zaks via Phabricator via cfe-commits]

zaks.anna added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp:271
 }
-assert(lockFail && lockSucc);
-C.addTransition(lockFail);
-
+// We might want to handle the case when the mutex lock function was 
inlined
+// and returned an Unknown or Undefined value.

This comment is repeated several times...



Comment at: test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h:29
+extern void lck_mtx_unlock(lck_mtx_t *);
+extern boolean_t lck_mtx_try_lock(lck_mtx_t *);
 extern void lck_mtx_destroy(lck_mtx_t *lck, lck_grp_t *grp);

Should there be a test added that uses the function?


https://reviews.llvm.org/D37806



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

[PATCH] D37809: [analyzer] PthreadLock: Refactor, use PostCall API. NFC.

[Anna Zaks via Phabricator via cfe-commits]

zaks.anna added a comment.

How about committing the refactor of the code without test modifications. And 
committing changes to the test separately?


https://reviews.llvm.org/D37809



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

[PATCH] D33440: clang-format: better handle statement and namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

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



Comment at: lib/Format/NamespaceEndCommentsFixer.cpp:155
   const FormatToken *NamespaceTok = AnnotatedLines[StartLineIndex]->First;
-  // Detect "(inline)? namespace" in the beginning of a line.
-  if (NamespaceTok->is(tok::kw_inline))
-NamespaceTok = NamespaceTok->getNextNonComment();
-  if (!NamespaceTok || NamespaceTok->isNot(tok::kw_namespace))
-return nullptr;
-  return NamespaceTok;
+  return NamespaceTok->getNamespaceToken();
+}

krasimir wrote:
> What happened to the old `// Detect "(inline)? namespace" in the beginning of 
> a line.`
Moved to `FormatToken::getNamespaceToken()`



Comment at: unittests/Format/FormatTest.cpp:1588
+  Style.NamespaceIndentation = FormatStyle::NI_All;
+  verifyFormat("TESTSUITE(A) {\n"
+   "  int foo();\n"

krasimir wrote:
> Hm, what would happen if you have a namespace macro with two or more 
> parameters?
only the first argument is used, as seen in previous test case.


https://reviews.llvm.org/D33440



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

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 115051.
Typz added a comment.

Reorder the functions to minimize diff.


https://reviews.llvm.org/D33589

Files:
  lib/Format/ContinuationIndenter.cpp
  lib/Format/ContinuationIndenter.h
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -9809,6 +9809,60 @@
   EXPECT_EQ("#pragma option -C -A", format("#pragmaoption   -C   -A"));
 }
 
+TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) {
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 20;
+
+  verifyFormat("int a; // the\n"
+   "   // comment", Style);
+  EXPECT_EQ("int a; /* first line\n"
+"* second\n"
+"* line third\n"
+"* line\n"
+"*/",
+			format("int a; /* first line\n"
+   "* second\n"
+   "* line third\n"
+   "* line\n"
+   "*/", Style));
+  EXPECT_EQ("int a; // first line\n"
+"   // second\n"
+"   // line third\n"
+"   // line",
+format("int a; // first line\n"
+   "   // second line\n"
+   "   // third line",
+   Style));
+
+  Style.PenaltyExcessCharacter = 90;
+  verifyFormat("int a; // the comment", Style);
+  EXPECT_EQ("int a; // the\n"
+"   // comment aa",
+format("int a; // the comment aa", Style));
+  EXPECT_EQ("int a; /* first line\n"
+"* second line\n"
+"* third line\n"
+"*/",
+			format("int a; /* first line\n"
+	   "* second line\n"
+			   "* third line\n"
+	   "*/", Style));
+  EXPECT_EQ("int a; // first line\n"
+"   // second line\n"
+"   // third line",
+format("int a; // first line\n"
+   "   // second line\n"
+   "   // third line",
+   Style));
+  EXPECT_EQ("int a; /* first line\n"
+"* second\n"
+"* line third\n"
+"* line\n"
+"*/",
+format("int a; /* first line second line third line"
+   "\n*/", Style));
+}
+
 #define EXPECT_ALL_STYLES_EQUAL(Styles)\
   for (size_t i = 1; i < Styles.size(); ++i)   \
   EXPECT_EQ(Styles[0], Styles[i]) << "Style #" << i << " of " << Styles.size() \
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -951,6 +951,7 @@
 for (std::deque::iterator I = Path.begin(), E = Path.end();
  I != E; ++I) {
   unsigned Penalty = 0;
+  State.Reflow = (*I)->State.Reflow;
   formatChildren(State, (*I)->NewLine, /*DryRun=*/false, Penalty);
   Penalty += Indenter->addTokenToState(State, (*I)->NewLine, false);
 
Index: lib/Format/ContinuationIndenter.h
===
--- lib/Format/ContinuationIndenter.h
+++ lib/Format/ContinuationIndenter.h
@@ -27,6 +27,7 @@
 namespace format {
 
 class AnnotatedLine;
+class BreakableToken;
 struct FormatToken;
 struct LineState;
 struct ParenState;
@@ -100,6 +101,16 @@
   unsigned breakProtrudingToken(const FormatToken , LineState ,
 bool DryRun);
 
+  /// \brief Perform the reflowing of a BreakableToken.
+  /// If State.Reflow=true, then the function will reflow the token as needed.
+  /// Otherwise, it simply computes the penalty caused by this tokens characters.
+  ///
+  /// \returns The penalty of reflowing the token if State.Reflow=true; otherwise
+  /// the penalty of characters going beyond the column limit.
+  unsigned reflowProtrudingToken(const FormatToken & Current, LineState & State,
+ std::unique_ptr & Token,
+ unsigned ColumnLimit, bool DryRun);
+
   /// \brief Appends the next token to \p State and updates information
   /// necessary for indentation.
   ///
@@ -350,6 +361,11 @@
   /// \brief The indent of the first token.
   unsigned FirstIndent;
 
+  /// \brief Indicates if comments are reflown.
+  /// This value is set when breakProtrudingToken() is called with DryRun=true,
+  /// and simply used otherwise.
+  bool Reflow = true;
+
   /// \brief The line that is being formatted.
   ///
   /// Does not need to be considered for memoization because it doesn't change.
Index: lib/Format/ContinuationIndenter.cpp
===
--- 

r313152 - This adds the _Float16 preprocessor macro definitions.

[Sjoerd Meijer via cfe-commits]

Author: sjoerdmeijer
Date: Wed Sep 13 08:23:19 2017
New Revision: 313152

URL: http://llvm.org/viewvc/llvm-project?rev=313152=rev
Log:
This adds the _Float16 preprocessor macro definitions.

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

Added:
cfe/trunk/test/Headers/float16.c
Modified:
cfe/trunk/lib/Frontend/InitPreprocessor.cpp
cfe/trunk/lib/Headers/float.h
cfe/trunk/test/Preprocessor/init.c

Modified: cfe/trunk/lib/Frontend/InitPreprocessor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/InitPreprocessor.cpp?rev=313152=313151=313152=diff
==
--- cfe/trunk/lib/Frontend/InitPreprocessor.cpp (original)
+++ cfe/trunk/lib/Frontend/InitPreprocessor.cpp Wed Sep 13 08:23:19 2017
@@ -110,9 +110,11 @@ static void AddImplicitIncludePCH(MacroB
 /// PickFP - This is used to pick a value based on the FP semantics of the
 /// specified FP model.
 template 
-static T PickFP(const llvm::fltSemantics *Sem, T IEEESingleVal,
+static T PickFP(const llvm::fltSemantics *Sem, T IEEEHalfVal, T IEEESingleVal,
 T IEEEDoubleVal, T X87DoubleExtendedVal, T PPCDoubleDoubleVal,
 T IEEEQuadVal) {
+  if (Sem == (const llvm::fltSemantics*)::APFloat::IEEEhalf())
+return IEEEHalfVal;
   if (Sem == (const llvm::fltSemantics*)::APFloat::IEEEsingle())
 return IEEESingleVal;
   if (Sem == (const llvm::fltSemantics*)::APFloat::IEEEdouble())
@@ -128,26 +130,26 @@ static T PickFP(const llvm::fltSemantics
 static void DefineFloatMacros(MacroBuilder , StringRef Prefix,
   const llvm::fltSemantics *Sem, StringRef Ext) {
   const char *DenormMin, *Epsilon, *Max, *Min;
-  DenormMin = PickFP(Sem, "1.40129846e-45", "4.9406564584124654e-324",
- "3.64519953188247460253e-4951",
+  DenormMin = PickFP(Sem, "5.9604644775390625e-8", "1.40129846e-45",
+ "4.9406564584124654e-324", "3.64519953188247460253e-4951",
  "4.94065645841246544176568792868221e-324",
  "6.47517511943802511092443895822764655e-4966");
-  int Digits = PickFP(Sem, 6, 15, 18, 31, 33);
-  int DecimalDigits = PickFP(Sem, 9, 17, 21, 33, 36);
-  Epsilon = PickFP(Sem, "1.19209290e-7", "2.2204460492503131e-16",
-   "1.08420217248550443401e-19",
+  int Digits = PickFP(Sem, 3, 6, 15, 18, 31, 33);
+  int DecimalDigits = PickFP(Sem, 5, 9, 17, 21, 33, 36);
+  Epsilon = PickFP(Sem, "9.765625e-4", "1.19209290e-7",
+   "2.2204460492503131e-16", "1.08420217248550443401e-19",
"4.94065645841246544176568792868221e-324",
"1.92592994438723585305597794258492732e-34");
-  int MantissaDigits = PickFP(Sem, 24, 53, 64, 106, 113);
-  int Min10Exp = PickFP(Sem, -37, -307, -4931, -291, -4931);
-  int Max10Exp = PickFP(Sem, 38, 308, 4932, 308, 4932);
-  int MinExp = PickFP(Sem, -125, -1021, -16381, -968, -16381);
-  int MaxExp = PickFP(Sem, 128, 1024, 16384, 1024, 16384);
-  Min = PickFP(Sem, "1.17549435e-38", "2.2250738585072014e-308",
+  int MantissaDigits = PickFP(Sem, 11, 24, 53, 64, 106, 113);
+  int Min10Exp = PickFP(Sem, -13, -37, -307, -4931, -291, -4931);
+  int Max10Exp = PickFP(Sem, 4, 38, 308, 4932, 308, 4932);
+  int MinExp = PickFP(Sem, -14, -125, -1021, -16381, -968, -16381);
+  int MaxExp = PickFP(Sem, 15, 128, 1024, 16384, 1024, 16384);
+  Min = PickFP(Sem, "6.103515625e-5", "1.17549435e-38", 
"2.2250738585072014e-308",
"3.36210314311209350626e-4932",
"2.00416836000897277799610805135016e-292",
"3.36210314311209350626267781732175260e-4932");
-  Max = PickFP(Sem, "3.40282347e+38", "1.7976931348623157e+308",
+  Max = PickFP(Sem, "6.5504e+4", "3.40282347e+38", "1.7976931348623157e+308",
"1.18973149535723176502e+4932",
"1.79769313486231580793728971405301e+308",
"1.18973149535723176508575932662800702e+4932");
@@ -802,6 +804,7 @@ static void InitializePredefinedMacros(c
   DefineFmt("__UINTPTR", TI.getUIntPtrType(), TI, Builder);
   DefineTypeWidth("__UINTPTR_WIDTH__", TI.getUIntPtrType(), TI, Builder);
 
+  DefineFloatMacros(Builder, "FLT16", (), "F16");
   DefineFloatMacros(Builder, "FLT", (), "F");
   DefineFloatMacros(Builder, "DBL", (), "");
   DefineFloatMacros(Builder, "LDBL", (), "L");

Modified: cfe/trunk/lib/Headers/float.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/float.h?rev=313152=313151=313152=diff
==
--- cfe/trunk/lib/Headers/float.h (original)
+++ cfe/trunk/lib/Headers/float.h Wed Sep 13 08:23:19 2017
@@ -143,4 +143,18 @@
 #  define LDBL_DECIMAL_DIG __LDBL_DECIMAL_DIG__
 #endif
 
+#ifdef __STDC_WANT_IEC_60559_TYPES_EXT__
+#  define FLT16_MANT_DIG__FLT16_MANT_DIG__
+#  define FLT16_DECIMAL_DIG __FLT16_DECIMAL_DIG__
+#  define FLT16_DIG __FLT16_DIG__
+#  define 

[PATCH] D34695: _Float16 preprocessor macro definitions

[Sjoerd Meijer via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL313152: This adds the _Float16 preprocessor macro 
definitions. (authored by SjoerdMeijer).

Changed prior to commit:
  https://reviews.llvm.org/D34695?vs=114998=115050#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D34695

Files:
  cfe/trunk/lib/Frontend/InitPreprocessor.cpp
  cfe/trunk/lib/Headers/float.h
  cfe/trunk/test/Headers/float16.c
  cfe/trunk/test/Preprocessor/init.c

Index: cfe/trunk/lib/Headers/float.h
===
--- cfe/trunk/lib/Headers/float.h
+++ cfe/trunk/lib/Headers/float.h
@@ -143,4 +143,18 @@
 #  define LDBL_DECIMAL_DIG __LDBL_DECIMAL_DIG__
 #endif
 
+#ifdef __STDC_WANT_IEC_60559_TYPES_EXT__
+#  define FLT16_MANT_DIG__FLT16_MANT_DIG__
+#  define FLT16_DECIMAL_DIG __FLT16_DECIMAL_DIG__
+#  define FLT16_DIG __FLT16_DIG__
+#  define FLT16_MIN_EXP __FLT16_MIN_EXP__
+#  define FLT16_MIN_10_EXP  __FLT16_MIN_10_EXP__
+#  define FLT16_MAX_EXP __FLT16_MAX_EXP__
+#  define FLT16_MAX_10_EXP  __FLT16_MAX_10_EXP__
+#  define FLT16_MAX __FLT16_MAX__
+#  define FLT16_EPSILON __FLT16_EPSILON__
+#  define FLT16_MIN __FLT16_MIN__
+#  define FLT16_TRUE_MIN__FLT16_TRUE_MIN__
+#endif /* __STDC_WANT_IEC_60559_TYPES_EXT__ */
+
 #endif /* __FLOAT_H */
Index: cfe/trunk/lib/Frontend/InitPreprocessor.cpp
===
--- cfe/trunk/lib/Frontend/InitPreprocessor.cpp
+++ cfe/trunk/lib/Frontend/InitPreprocessor.cpp
@@ -110,9 +110,11 @@
 /// PickFP - This is used to pick a value based on the FP semantics of the
 /// specified FP model.
 template 
-static T PickFP(const llvm::fltSemantics *Sem, T IEEESingleVal,
+static T PickFP(const llvm::fltSemantics *Sem, T IEEEHalfVal, T IEEESingleVal,
 T IEEEDoubleVal, T X87DoubleExtendedVal, T PPCDoubleDoubleVal,
 T IEEEQuadVal) {
+  if (Sem == (const llvm::fltSemantics*)::APFloat::IEEEhalf())
+return IEEEHalfVal;
   if (Sem == (const llvm::fltSemantics*)::APFloat::IEEEsingle())
 return IEEESingleVal;
   if (Sem == (const llvm::fltSemantics*)::APFloat::IEEEdouble())
@@ -128,26 +130,26 @@
 static void DefineFloatMacros(MacroBuilder , StringRef Prefix,
   const llvm::fltSemantics *Sem, StringRef Ext) {
   const char *DenormMin, *Epsilon, *Max, *Min;
-  DenormMin = PickFP(Sem, "1.40129846e-45", "4.9406564584124654e-324",
- "3.64519953188247460253e-4951",
+  DenormMin = PickFP(Sem, "5.9604644775390625e-8", "1.40129846e-45",
+ "4.9406564584124654e-324", "3.64519953188247460253e-4951",
  "4.94065645841246544176568792868221e-324",
  "6.47517511943802511092443895822764655e-4966");
-  int Digits = PickFP(Sem, 6, 15, 18, 31, 33);
-  int DecimalDigits = PickFP(Sem, 9, 17, 21, 33, 36);
-  Epsilon = PickFP(Sem, "1.19209290e-7", "2.2204460492503131e-16",
-   "1.08420217248550443401e-19",
+  int Digits = PickFP(Sem, 3, 6, 15, 18, 31, 33);
+  int DecimalDigits = PickFP(Sem, 5, 9, 17, 21, 33, 36);
+  Epsilon = PickFP(Sem, "9.765625e-4", "1.19209290e-7",
+   "2.2204460492503131e-16", "1.08420217248550443401e-19",
"4.94065645841246544176568792868221e-324",
"1.92592994438723585305597794258492732e-34");
-  int MantissaDigits = PickFP(Sem, 24, 53, 64, 106, 113);
-  int Min10Exp = PickFP(Sem, -37, -307, -4931, -291, -4931);
-  int Max10Exp = PickFP(Sem, 38, 308, 4932, 308, 4932);
-  int MinExp = PickFP(Sem, -125, -1021, -16381, -968, -16381);
-  int MaxExp = PickFP(Sem, 128, 1024, 16384, 1024, 16384);
-  Min = PickFP(Sem, "1.17549435e-38", "2.2250738585072014e-308",
+  int MantissaDigits = PickFP(Sem, 11, 24, 53, 64, 106, 113);
+  int Min10Exp = PickFP(Sem, -13, -37, -307, -4931, -291, -4931);
+  int Max10Exp = PickFP(Sem, 4, 38, 308, 4932, 308, 4932);
+  int MinExp = PickFP(Sem, -14, -125, -1021, -16381, -968, -16381);
+  int MaxExp = PickFP(Sem, 15, 128, 1024, 16384, 1024, 16384);
+  Min = PickFP(Sem, "6.103515625e-5", "1.17549435e-38", "2.2250738585072014e-308",
"3.36210314311209350626e-4932",
"2.00416836000897277799610805135016e-292",
"3.36210314311209350626267781732175260e-4932");
-  Max = PickFP(Sem, "3.40282347e+38", "1.7976931348623157e+308",
+  Max = PickFP(Sem, "6.5504e+4", "3.40282347e+38", "1.7976931348623157e+308",
"1.18973149535723176502e+4932",
"1.79769313486231580793728971405301e+308",
"1.18973149535723176508575932662800702e+4932");
@@ -802,6 +804,7 @@
   DefineFmt("__UINTPTR", TI.getUIntPtrType(), TI, Builder);
   DefineTypeWidth("__UINTPTR_WIDTH__", TI.getUIntPtrType(), TI, Builder);
 
+  DefineFloatMacros(Builder, "FLT16", (), "F16");
   DefineFloatMacros(Builder, "FLT", (), "F");
   DefineFloatMacros(Builder, "DBL", (), 

[PATCH] D34695: _Float16 preprocessor macro definitions

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

many thanks for reviewing and your help.


https://reviews.llvm.org/D34695



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

[PATCH] D34695: _Float16 preprocessor macro definitions

[Steve Canon via Phabricator via cfe-commits]

scanon accepted this revision.
scanon added a comment.

LGTM as well.


https://reviews.llvm.org/D34695



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

[PATCH] D34695: _Float16 preprocessor macro definitions

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 accepted this revision.
rogfer01 added a subscriber: rsmith.
rogfer01 added a comment.
This revision is now accepted and ready to land.

This LGTM, but wait a couple of days before comitting in case @rsmith or 
@scanon (or others!) have further comments.


https://reviews.llvm.org/D34695



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

[PATCH] D37566: [clang-tidy] fixed misc-unused-parameters omitting parameters default value

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In https://reviews.llvm.org/D37566#866096, @PriMee wrote:

> Done :) Could you please commit this for me?


Sure, just committed the patch.


Repository:
  rL LLVM

https://reviews.llvm.org/D37566



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

[PATCH] D37566: [clang-tidy] fixed misc-unused-parameters omitting parameters default value

[Alexander Kornienko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL313150: [clang-tidy] fixed misc-unused-parameters omitting 
parameters default value (authored by alexfh).

Changed prior to commit:
  https://reviews.llvm.org/D37566?vs=114538=115047#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37566

Files:
  clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
  clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp


Index: clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -138,7 +138,8 @@
   if (Function->isExternallyVisible() ||
   !Result.SourceManager->isInMainFile(Function->getLocation()) ||
   !Indexer->getOtherRefs(Function).empty() || isOverrideMethod(Function)) {
-SourceRange RemovalRange(Param->getLocation(), Param->getLocEnd());
+SourceRange RemovalRange(Param->getLocation(),
+ Param->DeclaratorDecl::getSourceRange().getEnd());
 // Note: We always add a space before the '/*' to not accidentally create a
 // '*/*' for pointer types, which doesn't start a comment. clang-format 
will
 // clean this up afterwards.
Index: clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp
@@ -14,15 +14,16 @@
 
 void b(int i = 1) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: parameter 'i' is unused 
[misc-unused-parameters]
-// CHECK-FIXES: {{^}}void b(int  /*i*/) {}{{$}}
+// CHECK-FIXES: {{^}}void b(int  /*i*/ = 1) {}{{$}}
 
 void c(int *i) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: parameter 'i' is unused 
[misc-unused-parameters]
 // CHECK-FIXES: {{^}}void c(int * /*i*/) {}{{$}}
 
 // Unchanged cases
 // ===
-void g(int i); // Don't remove stuff in declarations
+void f(int i); // Don't remove stuff in declarations
+void g(int i = 1);
 void h(int i) { (void)i; } // Don't remove used parameters
 
 bool useLambda(int (*fn)(int));
@@ -52,6 +53,11 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:33: warning
 // CHECK-FIXES: {{^}}static void staticFunctionE()
 
+static void staticFunctionF(int i = 4);
+// CHECK-FIXES: {{^}}static void staticFunctionF();
+static void staticFunctionF(int i) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:33: warning
+// CHECK-FIXES: {{^}}static void staticFunctionF()
 
 static void someCallSites() {
   staticFunctionA(1);
@@ -62,7 +68,12 @@
 // CHECK-FIXES: staticFunctionC(2);
   staticFunctionD(1, 2, 3);
 // CHECK-FIXES: staticFunctionD(1, 3);
-  staticFunctionE();
+  staticFunctionE(1);
+// CHECK-FIXES: staticFunctionE();
+  staticFunctionF(1);
+// CHECK-FIXES: staticFunctionF();
+  staticFunctionF();
+// CHECK-FIXES: staticFunctionF();
 }
 
 /*
@@ -95,6 +106,9 @@
   static void f(int i) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:21: warning
 // CHECK-FIXES: static void f(int  /*i*/) {}
+  static void g(int i = 1) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning
+// CHECK-FIXES: static void g(int  /*i*/ = 1) {}
 };
 
 namespace {
@@ -108,6 +122,9 @@
   void h(int i) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:14: warning
 // CHECK-FIXES: void h(int  /*i*/) {}
+  void s(int i = 1) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning
+// CHECK-FIXES: void s(int  /*i*/ = 1) {}
 };
 
 void C::f(int i) {}
@@ -125,6 +142,7 @@
 // CHECK-FIXES: c.g();
 
   useFunction(::h);
+  useFunction(::s);;
 }
 
 class Base {


Index: clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -138,7 +138,8 @@
   if (Function->isExternallyVisible() ||
   !Result.SourceManager->isInMainFile(Function->getLocation()) ||
   !Indexer->getOtherRefs(Function).empty() || isOverrideMethod(Function)) {
-SourceRange RemovalRange(Param->getLocation(), Param->getLocEnd());
+SourceRange RemovalRange(Param->getLocation(),
+ Param->DeclaratorDecl::getSourceRange().getEnd());
 // Note: We always add a space before the '/*' to not accidentally create a
 // '*/*' for pointer types, which doesn't start a comment. clang-format will
 // clean this up afterwards.
Index: clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp
@@ -14,15 +14,16 @@
 
 void b(int i = 1) {}
 // CHECK-MESSAGES: 

[clang-tools-extra] r313150 - [clang-tidy] fixed misc-unused-parameters omitting parameters default value

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Wed Sep 13 07:55:13 2017
New Revision: 313150

URL: http://llvm.org/viewvc/llvm-project?rev=313150=rev
Log:
[clang-tidy] fixed misc-unused-parameters omitting parameters default value

Summary:
Bug: https://bugs.llvm.org/show_bug.cgi?id=34450

**Problem:**

Clang-tidy check misc-unused-parameters omits parameter default value what 
results in its complete removal. Compilation errors might occur after 
clang-tidy fix.

**Patch description:**

Changed removal range. The range should end after parameter declarator, not 
after whole parameter declaration (which might contain a default value).

Reviewers: alexfh, xazax.hun

Reviewed By: alexfh

Subscribers: JDevlieghere, cfe-commits

Tags: #clang-tools-extra

Patch by Pawel Maciocha!

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

Modified:
clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp

Modified: clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp?rev=313150=313149=313150=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp Wed Sep 
13 07:55:13 2017
@@ -138,7 +138,8 @@ void UnusedParametersCheck::warnOnUnused
   if (Function->isExternallyVisible() ||
   !Result.SourceManager->isInMainFile(Function->getLocation()) ||
   !Indexer->getOtherRefs(Function).empty() || isOverrideMethod(Function)) {
-SourceRange RemovalRange(Param->getLocation(), Param->getLocEnd());
+SourceRange RemovalRange(Param->getLocation(),
+ Param->DeclaratorDecl::getSourceRange().getEnd());
 // Note: We always add a space before the '/*' to not accidentally create a
 // '*/*' for pointer types, which doesn't start a comment. clang-format 
will
 // clean this up afterwards.

Modified: clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp?rev=313150=313149=313150=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp Wed Sep 
13 07:55:13 2017
@@ -14,7 +14,7 @@ void a(int i) {}
 
 void b(int i = 1) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: parameter 'i' is unused 
[misc-unused-parameters]
-// CHECK-FIXES: {{^}}void b(int  /*i*/) {}{{$}}
+// CHECK-FIXES: {{^}}void b(int  /*i*/ = 1) {}{{$}}
 
 void c(int *i) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: parameter 'i' is unused 
[misc-unused-parameters]
@@ -22,7 +22,8 @@ void c(int *i) {}
 
 // Unchanged cases
 // ===
-void g(int i); // Don't remove stuff in declarations
+void f(int i); // Don't remove stuff in declarations
+void g(int i = 1);
 void h(int i) { (void)i; } // Don't remove used parameters
 
 bool useLambda(int (*fn)(int));
@@ -52,6 +53,11 @@ static void staticFunctionE(int i = 4) {
 // CHECK-MESSAGES: :[[@LINE-1]]:33: warning
 // CHECK-FIXES: {{^}}static void staticFunctionE()
 
+static void staticFunctionF(int i = 4);
+// CHECK-FIXES: {{^}}static void staticFunctionF();
+static void staticFunctionF(int i) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:33: warning
+// CHECK-FIXES: {{^}}static void staticFunctionF()
 
 static void someCallSites() {
   staticFunctionA(1);
@@ -62,7 +68,12 @@ static void someCallSites() {
 // CHECK-FIXES: staticFunctionC(2);
   staticFunctionD(1, 2, 3);
 // CHECK-FIXES: staticFunctionD(1, 3);
-  staticFunctionE();
+  staticFunctionE(1);
+// CHECK-FIXES: staticFunctionE();
+  staticFunctionF(1);
+// CHECK-FIXES: staticFunctionF();
+  staticFunctionF();
+// CHECK-FIXES: staticFunctionF();
 }
 
 /*
@@ -95,6 +106,9 @@ class SomeClass {
   static void f(int i) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:21: warning
 // CHECK-FIXES: static void f(int  /*i*/) {}
+  static void g(int i = 1) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning
+// CHECK-FIXES: static void g(int  /*i*/ = 1) {}
 };
 
 namespace {
@@ -108,6 +122,9 @@ public:
   void h(int i) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:14: warning
 // CHECK-FIXES: void h(int  /*i*/) {}
+  void s(int i = 1) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning
+// CHECK-FIXES: void s(int  /*i*/ = 1) {}
 };
 
 void C::f(int i) {}
@@ -125,6 +142,7 @@ void someMoreCallSites() {
 // CHECK-FIXES: c.g();
 
   useFunction(::h);
+  useFunction(::s);;
 }
 
 class Base {


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

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

[Francois Ferrand via Phabricator via cfe-commits]

Typz added inline comments.



Comment at: lib/Format/ContinuationIndenter.cpp:1339
 
+unsigned ContinuationIndenter::breakProtrudingToken(const FormatToken ,
+LineState ,

djasper wrote:
> Can you create a patch that doesn't move the code around so much? Seems 
> unnecessary and hard to review.
Moving the code around is an unfortunate requirement for this patch: we must 
actually do the "reflowing" twice, to find out which solution (actually 
reflowing or not) gives the least penalty.

Therefore the function that reflowing must be moved out of 
`breakProtrudingToken()`...

All I can do is try moving the new function after `breakProtrudingToken()`, 
maybe Phabricator will better show the change.



Comment at: lib/Format/ContinuationIndenter.cpp:1446
+  // Do not count the penalty twice, it will be added afterwards
+  if (State.Column > getColumnLimit(State)) {
+unsigned ExcessCharacters = State.Column - getColumnLimit(State);

djasper wrote:
> I believe that this is incorrect. reflowProtrudingToken counts the length of 
> the unbreakable tail and here you just remove the penalty of the token 
> itself. E.g. in:
> 
>   string s = f("aaa");
> 
> the ");" is the unbreakable tail of the stringl
This behavior has not changed : before the commit, the last token was not 
included in the penalty [c.f. `if` at line 1338 in original code].

To make the comparison significative, the last token's penalty is included in 
the penalty returned by `reflowProtrudingToken()` (hence the removal of the 
test at line 1260); and it is removed before returning from this function, to 
keep the same behavior as before.


https://reviews.llvm.org/D33589



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

[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In https://reviews.llvm.org/D37808#869602, @Eugene.Zelenko wrote:

> I think will be good idea to extend -Wswitch diagnostics.


Ok. But it will introduce new warnings to llvm codebase itself. I prepare some 
example output i found right now.


https://reviews.llvm.org/D37808



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

[PATCH] D37809: [analyzer] PthreadLock: Refactor, use PostCall API. NFC.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 115042.
NoQ added a comment.

Don't forget to check that the function is a global C function in post-call.


https://reviews.llvm.org/D37809

Files:
  lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
  test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h

Index: test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h
===
--- test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h
+++ test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h
@@ -4,36 +4,49 @@
 #pragma clang system_header
 
 typedef struct {
-	void	*foo;
-} pthread_mutex_t;
-
-typedef struct {
-	void	*foo;
-} pthread_mutexattr_t;
-
-typedef struct {
-	void	*foo;
-} lck_grp_t;
-
-typedef struct {
   void *foo;
-} lck_rw_t;
+} OpaqueThing;
+
+// Pthread.
+typedef OpaqueThing pthread_mutex_t;
+typedef OpaqueThing pthread_rwlock_t;
+typedef OpaqueThing pthread_mutexattr_t;
+
+// XNU.
+typedef OpaqueThing lck_mtx_t;
+typedef OpaqueThing lck_rw_t;
+typedef OpaqueThing lck_grp_t;
+typedef OpaqueThing lck_attr_t;
+typedef int boolean_t;
 
-typedef pthread_mutex_t lck_mtx_t;
+// Init.
+extern int pthread_mutex_init(pthread_mutex_t *mutex,
+  const pthread_mutexattr_t *mutexattr);
 
+// Acquire.
 extern int pthread_mutex_lock(pthread_mutex_t *);
-extern int pthread_mutex_unlock(pthread_mutex_t *);
-extern int pthread_mutex_trylock(pthread_mutex_t *);
-extern int pthread_mutex_destroy(pthread_mutex_t *);
-extern int pthread_mutex_init(pthread_mutex_t  *mutex, const pthread_mutexattr_t *mutexattr);
-
-typedef int boolean_t;
+// TODO: pthread_rwlock_rdlock.
+// TODO: pthread_rwlock_wrlock.
 extern void lck_mtx_lock(lck_mtx_t *);
-extern void lck_mtx_unlock(lck_mtx_t *);
+extern void lck_rw_lock_exclusive(lck_rw_t *lck);
+extern void lck_rw_lock_shared(lck_rw_t *lck);
+
+// Try.
+extern int pthread_mutex_trylock(pthread_mutex_t *);
+// TODO: pthread_rwlock_tryrdlock.
+// TODO: pthread_rwlock_trywrlock.
 extern boolean_t lck_mtx_try_lock(lck_mtx_t *);
-extern void lck_mtx_destroy(lck_mtx_t *lck, lck_grp_t *grp);
+// TODO: lck_rw_try_lock_exclusive.
+// TODO: lck_rw_try_lock_shared.
 
-extern void lck_rw_lock_exclusive(lck_rw_t *lck);
+// Release.
+extern int pthread_mutex_unlock(pthread_mutex_t *);
+// TODO: pthread_rwlock_tryrdlock.
+// TODO: pthread_rwlock_trywrlock.
+extern void lck_mtx_unlock(lck_mtx_t *);
 extern void lck_rw_unlock_exclusive(lck_rw_t *lck);
-extern void lck_rw_lock_shared(lck_rw_t *lck);
 extern void lck_rw_unlock_shared(lck_rw_t *lck);
+
+// Destroy.
+extern int pthread_mutex_destroy(pthread_mutex_t *);
+extern void lck_mtx_destroy(lck_mtx_t *lck, lck_grp_t *grp);
Index: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
+++ lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
@@ -17,7 +17,7 @@
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 
 using namespace clang;
 using namespace ento;
@@ -67,7 +67,7 @@
 };
 
 class PthreadLockChecker
-: public Checker {
+: public Checker {
   mutable std::unique_ptr BT_doublelock;
   mutable std::unique_ptr BT_doubleunlock;
   mutable std::unique_ptr BT_destroylock;
@@ -78,23 +78,54 @@
 PthreadSemantics,
 XNUSemantics
   };
-public:
-  void checkPostStmt(const CallExpr *CE, CheckerContext ) const;
-  void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
-  void printState(raw_ostream , ProgramStateRef State,
-  const char *NL, const char *Sep) const override;
 
-  void AcquireLock(CheckerContext , const CallExpr *CE, SVal lock,
-   bool isTryLock, enum LockingSemantics semantics) const;
+  typedef void (PthreadLockChecker::*FnCheck)(const CallEvent ,
+  CheckerContext ) const;
+  struct SupportedAPI {
+CallDescription Name;
+FnCheck Callback;
+  };
+  typedef SmallVector SupportedAPIList;
+  SupportedAPIList SupportedAPIs;
 
-  void ReleaseLock(CheckerContext , const CallExpr *CE, SVal lock) const;
-  void DestroyLock(CheckerContext , const CallExpr *CE, SVal Lock,
-   enum LockingSemantics semantics) const;
-  void InitLock(CheckerContext , const CallExpr *CE, SVal Lock) const;
-  void reportUseDestroyedBug(CheckerContext , const CallExpr *CE) const;
   ProgramStateRef resolvePossiblyDestroyedMutex(ProgramStateRef state,
 const MemRegion *lockR,
 const 

[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

I think will be good idea to extend -Wswitch diagnostics.


https://reviews.llvm.org/D37808



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

[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

fixed first stuff, sorry for noise.


https://reviews.llvm.org/D37808



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

[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 115037.
JonasToth marked 3 inline comments as done.
JonasToth added a comment.

- fix the first issues i found


https://reviews.llvm.org/D37808

Files:
  clang-tidy/hicpp/CMakeLists.txt
  clang-tidy/hicpp/HICPPTidyModule.cpp
  clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp
  clang-tidy/hicpp/MultiwayPathsCoveredCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/hicpp-multiway-paths-covered.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/hicpp-multiway-paths-covered-else.cpp
  test/clang-tidy/hicpp-multiway-paths-covered.cpp

Index: test/clang-tidy/hicpp-multiway-paths-covered.cpp
===
--- /dev/null
+++ test/clang-tidy/hicpp-multiway-paths-covered.cpp
@@ -0,0 +1,406 @@
+// RUN: %check_clang_tidy %s hicpp-multiway-paths-covered %t
+
+enum OS { Mac,
+  Windows,
+  Linux };
+
+/// All of these cases will not emit a warning per default, but with explicit activation.
+void problematic_if(int i, enum OS os) {
+  if (i > 0) {
+return;
+  } else if (i < 0) {
+return;
+  }
+
+  if (os == Mac) {
+return;
+  } else if (os == Linux) {
+if (true) {
+  return;
+} else if (false) {
+  return;
+}
+return;
+  } else {
+/* unreachable */
+if (true) // check if the parent would match here as well
+  return;
+  }
+
+  // Ok, because all paths are covered
+  if (i > 0) {
+return;
+  } else if (i < 0) {
+return;
+  } else {
+/* error, maybe precondition failed */
+  }
+}
+
+int return_integer() { return 42; }
+
+void problematic_switch(int i) {
+  // No default in this switch
+  switch (i) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered codepath found; add a default branch
+  case 0:
+break;
+  case 1:
+break;
+  case 2:
+break;
+  }
+
+  // degenerate, maybe even warning
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerate switch statement without any cases found; add cases and a default branch
+  }
+
+  switch (int j = return_integer()) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered codepath found; add a default branch
+  case 0:
+  case 1:
+  case 2:
+break;
+  }
+
+  // Degenerated, only default branch.
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerated switch stmt found; only the default branch exists
+  default:
+break;
+  }
+
+  // Degenerated, only one case label and default branch -> Better as if-stmt.
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch case could be better written with single if stmt
+  case 0:
+break;
+  default:
+break;
+  }
+
+  unsigned long long BigNumber = 0;
+  switch (BigNumber) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered codepath found; add a default branch
+  case 0:
+  case 1:
+break;
+  }
+
+  const int  = i;
+  switch (IntRef) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered codepath found; add a default branch
+  case 0:
+  case 1:
+break;
+  }
+
+  char C = 'A';
+  switch (C) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered codepath found; add a default branch
+  case 'A':
+break;
+  case 'B':
+break;
+  }
+}
+
+void unproblematic_switch(unsigned char c) {
+  // The switch stmt covers all paths explicitly.
+  // FIXME: False positive, determine the count of different possible values correctly.
+  // On the other hand, char is not defined to be one byte big, so there could be
+  // architectural differences, which would make the 'default:' case more portable.
+  switch (c) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered codepath found; add a default branch
+  case 0:
+  case 1:
+  case 2:
+  case 3:
+  case 4:
+  case 5:
+  case 6:
+  case 7:
+  case 8:
+  case 9:
+  case 10:
+  case 11:
+  case 12:
+  case 13:
+  case 14:
+  case 15:
+  case 16:
+  case 17:
+  case 18:
+  case 19:
+  case 20:
+  case 21:
+  case 22:
+  case 23:
+  case 24:
+  case 25:
+  case 26:
+  case 27:
+  case 28:
+  case 29:
+  case 30:
+  case 31:
+  case 32:
+  case 33:
+  case 34:
+  case 35:
+  case 36:
+  case 37:
+  case 38:
+  case 39:
+  case 40:
+  case 41:
+  case 42:
+  case 43:
+  case 44:
+  case 45:
+  case 46:
+  case 47:
+  case 48:
+  case 49:
+  case 50:
+  case 51:
+  case 52:
+  case 53:
+  case 54:
+  case 55:
+  case 56:
+  case 57:
+  case 58:
+  case 59:
+  case 60:
+  case 61:
+  case 62:
+  case 63:
+  case 64:
+  case 65:
+  case 66:
+  case 67:
+  case 68:
+  case 69:
+  case 70:
+  case 71:
+  case 72:
+  case 73:
+  case 74:
+  case 75:
+  case 76:
+  case 77:
+  case 78:
+  case 79:
+  case 80:
+  case 81:
+  case 82:
+  case 83:
+  case 84:
+  case 85:
+  case 86:
+  case 87:
+  case 88:
+  case 89:
+  case 90:
+  case 91:
+  case 92:
+  case 93:
+  case 94:
+  case 95:
+  case 96:
+  case 97:
+  case 98:
+  case 99:
+  case 100:
+  case 101:
+  case 102:
+  case 

[PATCH] D37809: [analyzer] PthreadLock: Refactor, use PostCall API. NFC.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.

Use `CallEvent` and `CallDescription` everywhere. Unhardcode argument numbers 
in AcquireLock() etc. Have a list of supported functions in one place. Other 
misc cleanup. No functional change intended anywhere.


https://reviews.llvm.org/D37809

Files:
  lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
  test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h

Index: test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h
===
--- test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h
+++ test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h
@@ -4,36 +4,49 @@
 #pragma clang system_header
 
 typedef struct {
-	void	*foo;
-} pthread_mutex_t;
-
-typedef struct {
-	void	*foo;
-} pthread_mutexattr_t;
-
-typedef struct {
-	void	*foo;
-} lck_grp_t;
-
-typedef struct {
   void *foo;
-} lck_rw_t;
+} OpaqueThing;
+
+// Pthread.
+typedef OpaqueThing pthread_mutex_t;
+typedef OpaqueThing pthread_rwlock_t;
+typedef OpaqueThing pthread_mutexattr_t;
+
+// XNU.
+typedef OpaqueThing lck_mtx_t;
+typedef OpaqueThing lck_rw_t;
+typedef OpaqueThing lck_grp_t;
+typedef OpaqueThing lck_attr_t;
+typedef int boolean_t;
 
-typedef pthread_mutex_t lck_mtx_t;
+// Init.
+extern int pthread_mutex_init(pthread_mutex_t *mutex,
+  const pthread_mutexattr_t *mutexattr);
 
+// Acquire.
 extern int pthread_mutex_lock(pthread_mutex_t *);
-extern int pthread_mutex_unlock(pthread_mutex_t *);
-extern int pthread_mutex_trylock(pthread_mutex_t *);
-extern int pthread_mutex_destroy(pthread_mutex_t *);
-extern int pthread_mutex_init(pthread_mutex_t  *mutex, const pthread_mutexattr_t *mutexattr);
-
-typedef int boolean_t;
+// TODO: pthread_rwlock_rdlock.
+// TODO: pthread_rwlock_wrlock.
 extern void lck_mtx_lock(lck_mtx_t *);
-extern void lck_mtx_unlock(lck_mtx_t *);
+extern void lck_rw_lock_exclusive(lck_rw_t *lck);
+extern void lck_rw_lock_shared(lck_rw_t *lck);
+
+// Try.
+extern int pthread_mutex_trylock(pthread_mutex_t *);
+// TODO: pthread_rwlock_tryrdlock.
+// TODO: pthread_rwlock_trywrlock.
 extern boolean_t lck_mtx_try_lock(lck_mtx_t *);
-extern void lck_mtx_destroy(lck_mtx_t *lck, lck_grp_t *grp);
+// TODO: lck_rw_try_lock_exclusive.
+// TODO: lck_rw_try_lock_shared.
 
-extern void lck_rw_lock_exclusive(lck_rw_t *lck);
+// Release.
+extern int pthread_mutex_unlock(pthread_mutex_t *);
+// TODO: pthread_rwlock_tryrdlock.
+// TODO: pthread_rwlock_trywrlock.
+extern void lck_mtx_unlock(lck_mtx_t *);
 extern void lck_rw_unlock_exclusive(lck_rw_t *lck);
-extern void lck_rw_lock_shared(lck_rw_t *lck);
 extern void lck_rw_unlock_shared(lck_rw_t *lck);
+
+// Destroy.
+extern int pthread_mutex_destroy(pthread_mutex_t *);
+extern void lck_mtx_destroy(lck_mtx_t *lck, lck_grp_t *grp);
Index: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
+++ lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
@@ -17,7 +17,7 @@
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 
 using namespace clang;
 using namespace ento;
@@ -67,7 +67,7 @@
 };
 
 class PthreadLockChecker
-: public Checker {
+: public Checker {
   mutable std::unique_ptr BT_doublelock;
   mutable std::unique_ptr BT_doubleunlock;
   mutable std::unique_ptr BT_destroylock;
@@ -78,23 +78,54 @@
 PthreadSemantics,
 XNUSemantics
   };
-public:
-  void checkPostStmt(const CallExpr *CE, CheckerContext ) const;
-  void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
-  void printState(raw_ostream , ProgramStateRef State,
-  const char *NL, const char *Sep) const override;
 
-  void AcquireLock(CheckerContext , const CallExpr *CE, SVal lock,
-   bool isTryLock, enum LockingSemantics semantics) const;
+  typedef void (PthreadLockChecker::*FnCheck)(const CallEvent ,
+  CheckerContext ) const;
+  struct SupportedAPI {
+CallDescription Name;
+FnCheck Callback;
+  };
+  typedef SmallVector SupportedAPIList;
+  SupportedAPIList SupportedAPIs;
 
-  void ReleaseLock(CheckerContext , const CallExpr *CE, SVal lock) const;
-  void DestroyLock(CheckerContext , const CallExpr *CE, SVal Lock,
-   enum LockingSemantics semantics) const;
-  void InitLock(CheckerContext , const CallExpr *CE, SVal Lock) const;
-  void reportUseDestroyedBug(CheckerContext , const CallExpr *CE) const;
   ProgramStateRef resolvePossiblyDestroyedMutex(ProgramStateRef state,
   

[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.

[Peter Siket via Phabricator via cfe-commits]

MontyKutyi added a comment.

Ping.


https://reviews.llvm.org/D34030



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

[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

I added a clarifying comment for some outcommented code still in the patch. I 
would like to have a bit of discussion on it.
Maybe some parts of the check could be warning-area, i just implemented it here 
to have it on one place.




Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:47
+
+  /// This option is noise, therefore matching is configurable.
+  if (WarnOnMissingElse) {

s/noise/noisy/



Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:68-78
+#if 0
+/// Get the number of different values for the Type T, that is switched on.
+std::size_t getNumberOfPossibleValues(const Type *T,
+  const ASTContext ) {
+  // This Assertion fails in clang and llvm, when matching on enum types.
+  assert(T->isIntegralType(Context) &&
+ "expecting integral type for discrete set of values");

I originally tried to implement some logic, that will realize when all paths 
are covered explicitly.
It didn't work and i think it is irrelevant anyway, since manually covering all 
paths for integertypes is IMHO unrealistic.

Iam willing to remove the code i currently commented out, just wanted it to be 
there if there is explicit need for a 'is everything covered'-check.



Comment at: docs/clang-tidy/checks/list.rst:41
cert-oop11-cpp (redirects to misc-move-constructor-init) 
-   cppcoreguidelines-c-copy-assignment-signature
+   cppcoreguidelines-c-copy-assignment-signature (redirects to 
misc-unconventional-assign-operator) 

cppcoreguidelines-interfaces-global-init

remove



Comment at: test/clang-tidy/hicpp-multiway-paths-covered.cpp:7
+int return_integer() { return 42; }
+
+void problematic_switch(int i) {

explicitly test, that the warnings for `else` do not occur here.


Repository:
  rL LLVM

https://reviews.llvm.org/D37808



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