Re: [PATCH] D24374: [libc++] Avoid include in locale_win32.h

[Eric Fiselier via cfe-commits]

EricWF added a comment.

LGTM.  I'm OK with things that are "technically" regressions in Windows support 
if it helps us get it working today.


https://reviews.llvm.org/D24374



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

Re: [PATCH] D21515: Update clang for D21514. NFC

[Amaury SECHET via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL281018: Update clang for D21514. NFC (authored by deadalnix).

Changed prior to commit:
  https://reviews.llvm.org/D21515?vs=62791=70783#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D21515

Files:
  cfe/trunk/lib/CodeGen/CGExpr.cpp

Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -2758,10 +2758,11 @@
 llvm::CallInst *CodeGenFunction::EmitTrapCall(llvm::Intrinsic::ID IntrID) {
   llvm::CallInst *TrapCall = Builder.CreateCall(CGM.getIntrinsic(IntrID));
 
-  if (!CGM.getCodeGenOpts().TrapFuncName.empty())
-TrapCall->addAttribute(llvm::AttributeSet::FunctionIndex,
-   "trap-func-name",
-   CGM.getCodeGenOpts().TrapFuncName);
+  if (!CGM.getCodeGenOpts().TrapFuncName.empty()) {
+auto A = llvm::Attribute::get(getLLVMContext(), "trap-func-name",
+  CGM.getCodeGenOpts().TrapFuncName);
+TrapCall->addAttribute(llvm::AttributeSet::FunctionIndex, A);
+  }
 
   return TrapCall;
 }


Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -2758,10 +2758,11 @@
 llvm::CallInst *CodeGenFunction::EmitTrapCall(llvm::Intrinsic::ID IntrID) {
   llvm::CallInst *TrapCall = Builder.CreateCall(CGM.getIntrinsic(IntrID));
 
-  if (!CGM.getCodeGenOpts().TrapFuncName.empty())
-TrapCall->addAttribute(llvm::AttributeSet::FunctionIndex,
-   "trap-func-name",
-   CGM.getCodeGenOpts().TrapFuncName);
+  if (!CGM.getCodeGenOpts().TrapFuncName.empty()) {
+auto A = llvm::Attribute::get(getLLVMContext(), "trap-func-name",
+  CGM.getCodeGenOpts().TrapFuncName);
+TrapCall->addAttribute(llvm::AttributeSet::FunctionIndex, A);
+  }
 
   return TrapCall;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24374: [libc++] Avoid include in locale_win32.h

[Shoaib Meenai via cfe-commits]

smeenai added a comment.

I'm aware that my replacement code isn't entirely equivalent, since it won't 
restore the locale in case `MB_CUR_MAX` throws. However, I'm quite certain the 
`MB_CUR_MAX` implementation won't throw. I can make my own RAII wrapper if 
desired, however.

Alternatively, MSDN documents an ___mb_cur_max_l_func 
, which 
is available Visual Studio 2013 onwards and does exactly what we want. It does 
warn that it's an internal CRT function and recommends against its use, hence 
my current implementation avoiding it. However, if we're okay using internal 
CRT functions, it would make for a cleaner implementation, and avoid the locale 
restoration issues altogether.


https://reviews.llvm.org/D24374



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

[PATCH] D24374: [libc++] Avoid include in locale_win32.h

[Shoaib Meenai via cfe-commits]

smeenai created this revision.
smeenai added reviewers: compnerd, EricWF, mclow.lists.
smeenai added a subscriber: cfe-commits.

When `_LIBCPP_NO_EXCEPTIONS` is defined, we end up with compile errors
when targeting MSVCRT:

* Code includes ``
* `` includes `` in order to get `abort`
* `` includes ``, _before_ the `using ::abort`
* `` includes `locale_win32.h`
* `locale_win32.h` includes ``
* `` includes ``
* `` includes ` __locale_raii;
 inline _LIBCPP_ALWAYS_INLINE
 decltype(MB_CUR_MAX) MB_CUR_MAX_L( locale_t __l )
 {
-  __locale_raii __current( uselocale(__l), uselocale );
-  return MB_CUR_MAX;
+  locale_t prev_locale = uselocale(__l);
+  auto cur_max = MB_CUR_MAX;
+  uselocale(prev_locale);
+  return cur_max;
 }
 
 // the *_l functions are prefixed on Windows, only available for msvcr80+, 
VS2005+


Index: include/support/win32/locale_win32.h
===
--- include/support/win32/locale_win32.h
+++ include/support/win32/locale_win32.h
@@ -17,7 +17,6 @@
 #include "support/win32/support.h"
 #include "support/win32/locale_mgmt_win32.h"
 #include 
-#include 
 
 lconv *localeconv_l( locale_t loc );
 size_t mbrlen_l( const char *__restrict s, size_t n,
@@ -34,13 +33,13 @@
  size_t nwc, size_t len, mbstate_t *__restrict ps, locale_t loc);
 wint_t btowc_l( int c, locale_t loc );
 int wctob_l( wint_t c, locale_t loc );
-typedef _VSTD::remove_pointer::type __locale_struct;
-typedef _VSTD::unique_ptr<__locale_struct, decltype()> __locale_raii;
 inline _LIBCPP_ALWAYS_INLINE
 decltype(MB_CUR_MAX) MB_CUR_MAX_L( locale_t __l )
 {
-  __locale_raii __current( uselocale(__l), uselocale );
-  return MB_CUR_MAX;
+  locale_t prev_locale = uselocale(__l);
+  auto cur_max = MB_CUR_MAX;
+  uselocale(prev_locale);
+  return cur_max;
 }
 
 // the *_l functions are prefixed on Windows, only available for msvcr80+, VS2005+
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23932: [XRay] ARM 32-bit no-Thumb support in Clang

[Dean Michael Berris via cfe-commits]

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

See comments in https://reviews.llvm.org/D23931 for more details.


https://reviews.llvm.org/D23932



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

Re: [PATCH] D23932: [XRay] ARM 32-bit no-Thumb support in Clang

[Dean Michael Berris via cfe-commits]

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

Reverted in https://reviews.llvm.org/rL280968 -- we should resolve 
https://reviews.llvm.org/D23931 before attempting to land again.


https://reviews.llvm.org/D23932



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

[PATCH] D24372: [libcxx] Sprinkle constexpr over compressed_pair

[Keno Fischer via cfe-commits]

loladiro created this revision.
loladiro added reviewers: EricWF, mclow.lists.
loladiro added a subscriber: cfe-commits.
loladiro set the repository for this revision to rL LLVM.

Without this, unique_ptr is not constant initialized. I've added a test to that 
extent.
Unfortunately, I believe there's additional work required for C++11, since 
unique_ptr
uses std::forward internally which is not constexpr until C++14. Doing 
something about
that is not part of this patch. unique_ptr not being constant initialized is 
particularly problematic
because it is in libstdc++, so there's applications that rely on this behavior.

Repository:
  rL LLVM

https://reviews.llvm.org/D24372

Files:
  include/memory
  
test/std/utilities/memory/unique.ptr/unique.ptr.runtime/unique.ptr.runtime.ctor/constinit.pass.cpp

Index: test/std/utilities/memory/unique.ptr/unique.ptr.runtime/unique.ptr.runtime.ctor/constinit.pass.cpp
===
--- /dev/null
+++ test/std/utilities/memory/unique.ptr/unique.ptr.runtime/unique.ptr.runtime.ctor/constinit.pass.cpp
@@ -0,0 +1,23 @@
+//===--===//
+//
+// 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.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03, c++11
+
+#include 
+#include 
+
+#ifndef _LIBCPP_SAFE_STATIC
+#define _LIBCPP_SAFE_STATIC
+#endif
+
+_LIBCPP_SAFE_STATIC static std::unique_ptr a;
+
+int main() {
+assert(a == nullptr);
+}
Index: include/memory
===
--- include/memory
+++ include/memory
@@ -2096,24 +2096,28 @@
 typedef const typename remove_reference<_T1>::type& _T1_const_reference;
 typedef const typename remove_reference<_T2>::type& _T2_const_reference;
 
-_LIBCPP_INLINE_VISIBILITY __libcpp_compressed_pair_imp() : __first_(), __second_() {}
-_LIBCPP_INLINE_VISIBILITY explicit __libcpp_compressed_pair_imp(_T1_param __t1)
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
+__libcpp_compressed_pair_imp() : __first_(), __second_() {}
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
+explicit __libcpp_compressed_pair_imp(_T1_param __t1)
 : __first_(_VSTD::forward<_T1_param>(__t1)), __second_() {}
-_LIBCPP_INLINE_VISIBILITY explicit __libcpp_compressed_pair_imp(_T2_param __t2)
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
+explicit __libcpp_compressed_pair_imp(_T2_param __t2)
 : __first_(), __second_(_VSTD::forward<_T2_param>(__t2)) {}
-_LIBCPP_INLINE_VISIBILITY __libcpp_compressed_pair_imp(_T1_param __t1, _T2_param __t2)
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
+__libcpp_compressed_pair_imp(_T1_param __t1, _T2_param __t2)
 : __first_(_VSTD::forward<_T1_param>(__t1)), __second_(_VSTD::forward<_T2_param>(__t2)) {}
 
 #if defined(_LIBCPP_HAS_NO_DEFAULTED_FUNCTIONS) && !defined(_LIBCPP_HAS_NO_RVALUE_REFERENCES)
 
-_LIBCPP_INLINE_VISIBILITY
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
 __libcpp_compressed_pair_imp(const __libcpp_compressed_pair_imp& __p)
 _NOEXCEPT_(is_nothrow_copy_constructible<_T1>::value &&
is_nothrow_copy_constructible<_T2>::value)
 : __first_(__p.first()),
   __second_(__p.second()) {}
 
-_LIBCPP_INLINE_VISIBILITY
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
 __libcpp_compressed_pair_imp& operator=(const __libcpp_compressed_pair_imp& __p)
 _NOEXCEPT_(is_nothrow_copy_assignable<_T1>::value &&
is_nothrow_copy_assignable<_T2>::value)
@@ -2123,14 +2127,14 @@
 return *this;
 }
 
-_LIBCPP_INLINE_VISIBILITY
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
 __libcpp_compressed_pair_imp(__libcpp_compressed_pair_imp&& __p)
 _NOEXCEPT_(is_nothrow_move_constructible<_T1>::value &&
is_nothrow_move_constructible<_T2>::value)
 : __first_(_VSTD::forward<_T1>(__p.first())),
   __second_(_VSTD::forward<_T2>(__p.second())) {}
 
-_LIBCPP_INLINE_VISIBILITY
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
 __libcpp_compressed_pair_imp& operator=(__libcpp_compressed_pair_imp&& __p)
 _NOEXCEPT_(is_nothrow_move_assignable<_T1>::value &&
is_nothrow_move_assignable<_T2>::value)
@@ -2145,7 +2149,7 @@
 #ifndef _LIBCPP_HAS_NO_VARIADICS
 
 template 
-_LIBCPP_INLINE_VISIBILITY
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
 __libcpp_compressed_pair_imp(piecewise_construct_t __pc,
  tuple<_Args1...> __first_args,
  tuple<_Args2...> __second_args,
@@ -2189,23 +2193,27 @@
 typedef const _T1&

[PATCH] D24371: Add diagnostics to require_constant_initialization

[Keno Fischer via cfe-commits]

loladiro created this revision.
loladiro added reviewers: EricWF, aaron.ballman, rsmith.
loladiro added a subscriber: cfe-commits.
loladiro set the repository for this revision to rL LLVM.

This hooks up the detailed diagnostics why constant initialization was not 
possible if require_constant_initialization reports an error. I have updated 
the test to account for the new notes.
Everything works fine, except that in C++11 mode we get:
```
error: 'note' diagnostics expected but not seen:
  File 
/data/llvm/tools/clang/test/SemaCXX/attr-require-constant-initialization.cpp 
Line 229 (directive at 
/data/llvm/tools/clang/test/SemaCXX/attr-require-constant-initialization.cpp:231):
 non-constexpr constructor 'NonLit' cannot be used in a constant expression
error: 'note' diagnostics seen but not expected:
  (frontend): non-literal type 'NonLit' cannot be used in a constant expression
```
This is because of an ImplicitValueInitExpr that gets passed into 
CheckLiteralType, but since ImplicitValueInitExpr doesn't have source 
information we get an invalid source location. I'm not really sure how to fix 
that (Is it possible to test for a note without source location?). Adding the 
proper source locations to ImplicitValueInitExpr seemed like a bigger 
undertaking than was warranted for this patch, so I'd appreciate guidance on 
how to proceed.

Repository:
  rL LLVM

https://reviews.llvm.org/D24371

Files:
  lib/Sema/SemaDecl.cpp
  test/SemaCXX/attr-require-constant-initialization.cpp

Index: test/SemaCXX/attr-require-constant-initialization.cpp
===
--- test/SemaCXX/attr-require-constant-initialization.cpp
+++ test/SemaCXX/attr-require-constant-initialization.cpp
@@ -7,9 +7,9 @@
 
 #define ATTR __attribute__((require_constant_initialization)) // expected-note 0+ {{expanded from macro}}
 
-int ReturnInt();
+int ReturnInt(); // expected-note 0+ {{declared here}}
 
-struct PODType {
+struct PODType { // expected-note 0+ {{declared here}}
   int value;
   int value2;
 };
@@ -20,17 +20,17 @@
 struct LitType {
   constexpr LitType() : value(0) {}
   constexpr LitType(int x) : value(x) {}
-  LitType(void *) : value(-1) {}
+  LitType(void *) : value(-1) {} // expected-note 0+ {{declared here}}
   int value;
 };
 #endif
 
-struct NonLit {
+struct NonLit { // expected-note 0+ {{declared here}}
 #if __cplusplus >= 201402L
   constexpr NonLit() : value(0) {}
   constexpr NonLit(int x) : value(x) {}
 #else
-  NonLit() : value(0) {}
+  NonLit() : value(0) {}  // expected-note 0+ {{declared here}}
   NonLit(int x) : value(x) {}
 #endif
   NonLit(void *) : value(-1) {}
@@ -82,23 +82,44 @@
   const int non_global = 42;
   ATTR static const int _init = non_global; // expected-error {{variable does not have a constant initializer}}
   // expected-note@-1 {{required by 'require_constant_initializer' attribute here}}
+#if __cplusplus >= 201103L
+  // expected-note@-3 {{reference to 'non_global' is not a constant expression}}
+  // expected-note@-5 {{declared here}}
+#else
+  // expected-note@-6 {{subexpression not valid in a constant expression}}
+#endif
   ATTR static const int _init = glvalue_int;
   ATTR static const int _init = 42;
 }
 
 ATTR const int _ref = 42;
 ATTR const int _ref2 = ReturnInt(); // expected-error {{variable does not have a constant initializer}}
 // expected-note@-1 {{required by 'require_constant_initializer' attribute here}}
+#if __cplusplus >= 201103L
+// expected-note@-3 {{non-constexpr function 'ReturnInt' cannot be used in a constant expression}}
+#else
+// expected-note@-5 {{subexpression not valid in a constant expression}}
+#endif
 ATTR const NonLit _temp_ref = 42; // expected-error {{variable does not have a constant initializer}}
 // expected-note@-1 {{required by 'require_constant_initializer' attribute here}}
+#if __cplusplus >= 201103L
+// expected-note@-3 {{non-literal type 'const NonLit' cannot be used in a constant expression}}
+#else
+// expected-note@-5 {{subexpression not valid in a constant expression}}
+#endif
 
 #if __cplusplus >= 201103L
 ATTR const LitType _temp_ref = 42;
 ATTR const int _ref = LitType{}.value;
 #endif
 
 ATTR const int _subobj_ref = NonLit().value; // expected-error {{variable does not have a constant initializer}}
 // expected-note@-1 {{required by 'require_constant_initializer' attribute here}}
+#if __cplusplus >= 201103L
+// expected-note-re@-3 {{non-literal type '{{.*}}' cannot be used in a constant expression}}
+#else
+// expected-note@-5 {{subexpression not valid in a constant expression}}
+#endif
 
 struct TT1 {
   ATTR static const int _init;
@@ -116,6 +137,8 @@
 #if __cplusplus >= 201103L
 thread_local const int ::tl_glvalue_init = glvalue_int;
 thread_local const int ::tl_temp_init = 42; // expected-error {{variable does not have a constant initializer}}
+// expected-note@-1 {{reference to temporary is not a constant expression}}
+// expected-note@-2 {{temporary created here}}
 #endif
 
 // 

r281017 - [Docs] Fix typos, remove trailing whitespace.

[George Burgess IV via cfe-commits]

Author: gbiv
Date: Thu Sep  8 21:45:48 2016
New Revision: 281017

URL: http://llvm.org/viewvc/llvm-project?rev=281017=rev
Log:
[Docs] Fix typos, remove trailing whitespace.

Avoided wrapping NullabilityDocs at 80cols, since that would've made
this diff much bigger, and never-ending lines seems to be the style for
many of the null-related docs.

Modified:
cfe/trunk/include/clang/Basic/AttrDocs.td

Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=281017=281016=281017=diff
==
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Thu Sep  8 21:45:48 2016
@@ -119,7 +119,7 @@ The ``carries_dependency`` attribute spe
 out of functions.
 
 When specified on a function or Objective-C method, the ``carries_dependency``
-attribute means that the return value carries a dependency out of the 
function, 
+attribute means that the return value carries a dependency out of the function,
 so that the implementation need not constrain ordering upon return from that
 function. Implementations of the function and its caller may choose to preserve
 dependencies instead of emitting memory ordering instructions such as fences.
@@ -691,10 +691,10 @@ This attribute specifies that the Object
 def ObjCBoxableDocs : Documentation {
 let Category = DocCatFunction;
 let Content = [{
-Structs and unions marked with the ``objc_boxable`` attribute can be used 
+Structs and unions marked with the ``objc_boxable`` attribute can be used
 with the Objective-C boxed expression syntax, ``@(...)``.
 
-**Usage**: ``__attribute__((objc_boxable))``. This attribute 
+**Usage**: ``__attribute__((objc_boxable))``. This attribute
 can only be placed on a declaration of a trivially-copyable struct or union:
 
 .. code-block:: objc
@@ -847,8 +847,8 @@ is not a constant initializer, an error
 only be used in C++.
 
 Note that in C++03 strict constant expression checking is not done. Instead
-the attribute reports if Clang can emit the the variable as a constant, even
-if it's not technically a 'constant initializer'. This behavior is 
non-portable.
+the attribute reports if Clang can emit the variable as a constant, even if 
it's
+not technically a 'constant initializer'. This behavior is non-portable.
 
 Static storage duration variables with constant initializers avoid hard-to-find
 bugs caused by the indeterminate order of dynamic initialization. They can also
@@ -920,7 +920,7 @@ potentially-evaluated discarded-value ex
 launch_missiles();
   }
   error_info ();
-  void f() { foo(); } // Does not diagnose, error_info is a reference.  
+  void f() { foo(); } // Does not diagnose, error_info is a reference.
   }];
 }
 
@@ -1238,7 +1238,7 @@ passed in RCX, RDX, R8, and R9 as is don
 convention.
 
 On both 32-bit x86 and x86_64 targets, vector and floating point arguments are
-passed in XMM0-XMM5. Homogenous vector aggregates of up to four elements are
+passed in XMM0-XMM5. Homogeneous vector aggregates of up to four elements are
 passed in sequential SSE registers if enough are available. If AVX is enabled,
 256 bit vectors are passed in YMM0-YMM5. Any vector or aggregate type that
 cannot be passed in registers for any reason is passed by reference, which
@@ -1373,7 +1373,7 @@ def NoSanitizeMemoryDocs : Documentation
 .. _langext-memory_sanitizer:
 
 Use ``__attribute__((no_sanitize_memory))`` on a function declaration to
-specify that checks for uninitialized memory should not be inserted 
+specify that checks for uninitialized memory should not be inserted
 (e.g. by MemorySanitizer). The function may still be instrumented by the tool
 to avoid false positives in other places.
   }];
@@ -1991,7 +1991,7 @@ by Clang.
 }
 def NullabilityDocs : DocumentationCategory<"Nullability Attributes"> {
   let Content = [{
-Whether a particular pointer may be "null" is an important concern when 
working with pointers in the C family of languages. The various nullability 
attributes indicate whether a particular pointer can be null or not, which 
makes APIs more expressive and can help static analysis tools identify bugs 
involving null pointers. Clang supports several kinds of nullability 
attributes: the ``nonnull`` and ``returns_nonnull`` attributes indicate which 
function or method parameters and result types can never be null, while 
nullability type qualifiers indicate which pointer types can be null 
(``_Nullable``) or cannot be null (``_Nonnull``). 
+Whether a particular pointer may be "null" is an important concern when 
working with pointers in the C family of languages. The various nullability 
attributes indicate whether a particular pointer can be null or not, which 
makes APIs more expressive and can help static analysis tools identify bugs 
involving null pointers. Clang supports several kinds of nullability 
attributes: the 

Re: [PATCH] D22494: [analyzer] Explain why analyzer report is not generated (fix for PR12421).

[Anton Yartsev via cfe-commits]

ayartsev updated this revision to Diff 70772.
ayartsev added a comment.

Updated the patch, added help entry for the "--analyzer-output" driver option. 
Please review.


https://reviews.llvm.org/D22494

Files:
  lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  test/Analysis/diagnostics/diag-cross-file-boundaries.c
  test/Analysis/diagnostics/diag-cross-file-boundaries.h

Index: test/Analysis/diagnostics/diag-cross-file-boundaries.h
===
--- test/Analysis/diagnostics/diag-cross-file-boundaries.h
+++ test/Analysis/diagnostics/diag-cross-file-boundaries.h
@@ -0,0 +1,4 @@
+static void f() {
+  int *p = 0;
+  *p = 1;   // expected-warning{{Dereference of null pointer}}
+}
Index: test/Analysis/diagnostics/diag-cross-file-boundaries.c
===
--- test/Analysis/diagnostics/diag-cross-file-boundaries.c
+++ test/Analysis/diagnostics/diag-cross-file-boundaries.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-output=html -o 
PR12421.html %s 2>&1 | FileCheck %s
+
+// Test for PR12421
+#include "diag-cross-file-boundaries.h"
+
+int main(){
+  f();
+  return 0;
+}
+
+// CHECK: warning: Path diagnostic report is not generated.
Index: lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===
--- lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -211,6 +211,12 @@
 const SourceManager  = D->path.front()->getLocation().getManager();
 SmallVector WorkList;
 WorkList.push_back(>path);
+SmallString<128> buf;
+llvm::raw_svector_ostream warning(buf);
+warning << "warning: Path diagnostic report is not generated. Current "
+<< "output format does not support diagnostics that cross file "
+<< "boundaries. Refer to --analyzer-output for valid output "
+   << "formats\n";
 
 while (!WorkList.empty()) {
   const PathPieces  = *WorkList.pop_back_val();
@@ -222,19 +228,25 @@
 
 if (FID.isInvalid()) {
   FID = SMgr.getFileID(L);
-} else if (SMgr.getFileID(L) != FID)
-  return; // FIXME: Emit a warning?
+} else if (SMgr.getFileID(L) != FID) {
+  llvm::errs() << warning.str();
+  return;
+}
 
 // Check the source ranges.
 ArrayRef Ranges = piece->getRanges();
 for (ArrayRef::iterator I = Ranges.begin(),
  E = Ranges.end(); I != E; ++I) {
   SourceLocation L = SMgr.getExpansionLoc(I->getBegin());
-  if (!L.isFileID() || SMgr.getFileID(L) != FID)
-return; // FIXME: Emit a warning?
+  if (!L.isFileID() || SMgr.getFileID(L) != FID) {
+llvm::errs() << warning.str();
+return;
+  }
   L = SMgr.getExpansionLoc(I->getEnd());
-  if (!L.isFileID() || SMgr.getFileID(L) != FID)
-return; // FIXME: Emit a warning?
+  if (!L.isFileID() || SMgr.getFileID(L) != FID) {
+llvm::errs() << warning.str();
+return;
+  }
 }
 
 if (const PathDiagnosticCallPiece *call =


Index: test/Analysis/diagnostics/diag-cross-file-boundaries.h
===
--- test/Analysis/diagnostics/diag-cross-file-boundaries.h
+++ test/Analysis/diagnostics/diag-cross-file-boundaries.h
@@ -0,0 +1,4 @@
+static void f() {
+  int *p = 0;
+  *p = 1;   // expected-warning{{Dereference of null pointer}}
+}
Index: test/Analysis/diagnostics/diag-cross-file-boundaries.c
===
--- test/Analysis/diagnostics/diag-cross-file-boundaries.c
+++ test/Analysis/diagnostics/diag-cross-file-boundaries.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-output=html -o PR12421.html %s 2>&1 | FileCheck %s
+
+// Test for PR12421
+#include "diag-cross-file-boundaries.h"
+
+int main(){
+  f();
+  return 0;
+}
+
+// CHECK: warning: Path diagnostic report is not generated.
Index: lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===
--- lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -211,6 +211,12 @@
 const SourceManager  = D->path.front()->getLocation().getManager();
 SmallVector WorkList;
 WorkList.push_back(>path);
+SmallString<128> buf;
+llvm::raw_svector_ostream warning(buf);
+warning << "warning: Path diagnostic report is not generated. Current "
+<< "output format does not support diagnostics that cross file "
+<< "boundaries. Refer to --analyzer-output for valid output "
+			<< "formats\n";
 
  

Re: [PATCH] D24289: Add warning when assigning enums to bitfields without an explicit unsigned underlying type

[Sasha Bermeister via cfe-commits]

sashab updated this revision to Diff 70760.
sashab marked an inline comment as done.
sashab added a comment.

Thanks for your feedback everyone; left the flag as DefaultIgnore but added it 
to -Wall.

Keep in mind I am planning on adding two additional warnings after this, namely
"%0 is too small to hold all values of %1"
and
"%0 is too large to hold all values of %1"
for all enum bitfields.

Also, just going back to rnk's original comment, I don't think I properly 
replied -- you are correct that the following code behaves strangely:

  enum E : signed { E1, E2 };
  struct { E e1 : 1; E e2; F f1 : 1; F f2; } s;
  s.e2 = E2; // comes out as -1 instead of 1

If you are storing signed enums in a bitfield, you need 1 additional bit to 
store the sign bit otherwise the enums will not serialize back correctly. This 
is equivalent to storing unsigned enums in bitfields that are too small; the 
whole value is not stored.

This is handled by the warning I'm adding next "%0 is too small to hold all 
values of %1", which can fire for signed enum bitfields when you don't store 
max(numberOfBitsNeededForUnsignedValues, numberOfBitsNeededForSignedValues) + 1.


https://reviews.llvm.org/D24289

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/SemaCXX/warn-msvc-enum-bitfield.cpp

Index: test/SemaCXX/warn-msvc-enum-bitfield.cpp
===
--- test/SemaCXX/warn-msvc-enum-bitfield.cpp
+++ test/SemaCXX/warn-msvc-enum-bitfield.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -fsyntax-only -Wsigned-enum-bitfield -verify %s --std=c++11
+
+// Enums used in bitfields with no explicitly specified underlying type.
+void test0() {
+  enum E { E1, E2 };
+  enum F { F1, F2 };
+  struct { E e1 : 1; E e2; F f1 : 1; F f2; } s;
+
+  s.e1 = E1; // expected-warning {{enums in the Microsoft ABI are signed integers by default; consider giving the enum E an unsigned underlying type to make this code portable}}
+  s.f1 = F1; // expected-warning {{enums in the Microsoft ABI are signed integers by default; consider giving the enum F an unsigned underlying type to make this code portable}}
+
+  s.e2 = E2;
+  s.f2 = F2;
+}
+
+// Enums used in bitfields with an explicit signed underlying type.
+void test1() {
+  enum E : signed { E1, E2 };
+  enum F : long { F1, F2 };
+  struct { E e1 : 1; E e2; F f1 : 1; F f2; } s;
+  
+  s.e1 = E1;
+  s.f1 = F1;
+
+  s.e2 = E2;
+  s.f2 = F2;
+}
+
+// Enums used in bitfields with an explicitly unsigned underlying type.
+void test3() {
+  enum E : unsigned { E1, E2 };
+  enum F : unsigned long { F1, F2 };
+  struct { E e1 : 1; E e2; F f1 : 1; F f2; } s;
+  
+  s.e1 = E1;
+  s.f1 = F1;
+
+  s.e2 = E2;
+  s.f2 = F2;
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -7828,6 +7828,24 @@
 return false;
 
   // White-list bool bitfields.
+  QualType BitfieldType = Bitfield->getType();
+  if (BitfieldType->isBooleanType())
+ return false;
+ 
+  if (BitfieldType->isEnumeralType()) {
+EnumDecl *BitfieldEnumDecl = BitfieldType->getAs()->getDecl();
+// If the underlying enum type was not explicitly specified as an unsigned
+// type and the enum contain only positive values, MSVC++ will cause an
+// inconsistency by storing this as a signed type.
+if (S.getLangOpts().CPlusPlus11 &&
+!BitfieldEnumDecl->getIntegerTypeSourceInfo() &&
+BitfieldEnumDecl->getNumPositiveBits() > 0 &&
+BitfieldEnumDecl->getNumNegativeBits() == 0) {
+  S.Diag(InitLoc, diag::warn_no_underlying_type_specified_for_enum_bitfield)
+<< BitfieldEnumDecl->getNameAsString();
+}
+  }
+
   if (Bitfield->getType()->isBooleanType())
 return false;
 
@@ -7858,7 +7876,7 @@
 
   // Compute the value which the bitfield will contain.
   llvm::APSInt TruncatedValue = Value.trunc(FieldWidth);
-  TruncatedValue.setIsSigned(Bitfield->getType()->isSignedIntegerType());
+  TruncatedValue.setIsSigned(BitfieldType->isSignedIntegerType());
 
   // Check whether the stored value is equal to the original value.
   TruncatedValue = TruncatedValue.extend(OriginalWidth);
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -2956,6 +2956,10 @@
   "cast to %1 from smaller integer type %0">,
   InGroup;
 
+def warn_no_underlying_type_specified_for_enum_bitfield : Warning<
+  "enums in the Microsoft ABI are signed integers by default; consider giving "
+  "the enum %0 an unsigned underlying type to make this code portable">,
+  InGroup>, DefaultIgnore;
 def warn_attribute_packed_for_bitfield : Warning<
   "'packed' attribute was ignored on bit-fields with single-byte alignment "
   

r280999 - C++ Modules TS: Add parsing and some semantic analysis support for

[Richard Smith via cfe-commits]

Author: rsmith
Date: Thu Sep  8 18:14:54 2016
New Revision: 280999

URL: http://llvm.org/viewvc/llvm-project?rev=280999=rev
Log:
C++ Modules TS: Add parsing and some semantic analysis support for
export-declarations. These don't yet have an effect on name visibility;
we still export everything by default.

Added:
cfe/trunk/test/CodeGenCXX/modules-ts.cppm
cfe/trunk/test/SemaCXX/modules-ts.cppm
Modified:
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/include/clang/AST/DeclBase.h
cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
cfe/trunk/include/clang/Basic/DeclNodes.td
cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/include/clang/Sema/Template.h
cfe/trunk/include/clang/Serialization/ASTBitCodes.h
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/AST/DeclBase.cpp
cfe/trunk/lib/CodeGen/CGDecl.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.h
cfe/trunk/lib/Parse/ParseDeclCXX.cpp
cfe/trunk/lib/Parse/Parser.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaLookup.cpp
cfe/trunk/lib/Sema/SemaTemplate.cpp
cfe/trunk/lib/Serialization/ASTCommon.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
cfe/trunk/test/Parser/cxx-modules-interface.cppm
cfe/trunk/tools/libclang/CIndex.cpp

Modified: cfe/trunk/include/clang/AST/Decl.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=280999=280998=280999=diff
==
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Thu Sep  8 18:14:54 2016
@@ -3792,6 +3792,55 @@ public:
   static bool classofKind(Kind K) { return K == Import; }
 };
 
+/// \brief Represents a C++ Modules TS module export declaration.
+///
+/// For example:
+/// \code
+///   export void foo();
+/// \endcode
+class ExportDecl final : public Decl, public DeclContext {
+  virtual void anchor();
+private:
+  /// \brief The source location for the right brace (if valid).
+  SourceLocation RBraceLoc;
+
+  ExportDecl(DeclContext *DC, SourceLocation ExportLoc)
+: Decl(Export, DC, ExportLoc), DeclContext(Export),
+  RBraceLoc(SourceLocation()) { }
+
+  friend class ASTDeclReader;
+
+public:
+  static ExportDecl *Create(ASTContext , DeclContext *DC,
+SourceLocation ExportLoc);
+  static ExportDecl *CreateDeserialized(ASTContext , unsigned ID);
+  
+  SourceLocation getExportLoc() const { return getLocation(); }
+  SourceLocation getRBraceLoc() const { return RBraceLoc; }
+  void setRBraceLoc(SourceLocation L) { RBraceLoc = L; }
+
+  SourceLocation getLocEnd() const LLVM_READONLY {
+if (RBraceLoc.isValid())
+  return RBraceLoc;
+// No braces: get the end location of the (only) declaration in context
+// (if present).
+return decls_empty() ? getLocation() : decls_begin()->getLocEnd();
+  }
+
+  SourceRange getSourceRange() const override LLVM_READONLY {
+return SourceRange(getLocation(), getLocEnd());
+  }
+
+  static bool classof(const Decl *D) { return classofKind(D->getKind()); }
+  static bool classofKind(Kind K) { return K == Export; }
+  static DeclContext *castToDeclContext(const ExportDecl *D) {
+return static_cast(const_cast(D));
+  }
+  static ExportDecl *castFromDeclContext(const DeclContext *DC) {
+return static_cast(const_cast(DC));
+  }
+};
+
 /// \brief Represents an empty-declaration.
 class EmptyDecl : public Decl {
   virtual void anchor();

Modified: cfe/trunk/include/clang/AST/DeclBase.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=280999=280998=280999=diff
==
--- cfe/trunk/include/clang/AST/DeclBase.h (original)
+++ cfe/trunk/include/clang/AST/DeclBase.h Thu Sep  8 18:14:54 2016
@@ -33,6 +33,7 @@ class DeclContext;
 class DeclarationName;
 class DependentDiagnostic;
 class EnumDecl;
+class ExportDecl;
 class FunctionDecl;
 class FunctionType;
 enum Linkage : unsigned char;
@@ -1135,6 +1136,7 @@ public:
 ///   ObjCMethodDecl
 ///   ObjCContainerDecl
 ///   LinkageSpecDecl
+///   ExportDecl
 ///   BlockDecl
 ///   OMPDeclareReductionDecl
 ///
@@ -1279,7 +1281,8 @@ public:
 
   /// \brief Test whether the context supports looking up names.
   bool isLookupContext() const {
-return !isFunctionOrMethod() && DeclKind != Decl::LinkageSpec;
+return !isFunctionOrMethod() && DeclKind != Decl::LinkageSpec &&
+   DeclKind != Decl::Export;
   }
 
   bool isFileContext() const {

Modified: cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/RecursiveASTVisitor.h?rev=280999=280998=280999=diff

Re: [PATCH] D23643: [Driver] Report invalid -mtune/-mcpu parameters when -arch=arm64

[Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL280998: [Driver] Report invalid -mtune/-mcpu parameters when 
-arch=arm64 (authored by vedantk).

Changed prior to commit:
  https://reviews.llvm.org/D23643?vs=68747=70761#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D23643

Files:
  cfe/trunk/lib/Driver/Tools.cpp
  cfe/trunk/test/Driver/arm-cortex-cpus.c

Index: cfe/trunk/lib/Driver/Tools.cpp
===
--- cfe/trunk/lib/Driver/Tools.cpp
+++ cfe/trunk/lib/Driver/Tools.cpp
@@ -1140,9 +1140,9 @@
 // ARM tools end.
 
 /// getAArch64TargetCPU - Get the (LLVM) name of the AArch64 cpu we are
-/// targeting.
-static std::string getAArch64TargetCPU(const ArgList ) {
-  Arg *A;
+/// targeting. Set \p A to the Arg corresponding to the -mcpu or -mtune
+/// arguments if they are provided, or to nullptr otherwise.
+static std::string getAArch64TargetCPU(const ArgList , Arg *) {
   std::string CPU;
   // If we have -mtune or -mcpu, use that.
   if ((A = Args.getLastArg(options::OPT_mtune_EQ))) {
@@ -1919,13 +1919,15 @@
 
 static std::string getCPUName(const ArgList , const llvm::Triple ,
   bool FromAs = false) {
+  Arg *A;
+
   switch (T.getArch()) {
   default:
 return "";
 
   case llvm::Triple::aarch64:
   case llvm::Triple::aarch64_be:
-return getAArch64TargetCPU(Args);
+return getAArch64TargetCPU(Args, A);
 
   case llvm::Triple::arm:
   case llvm::Triple::armeb:
@@ -2443,18 +2445,18 @@
   else if ((A = Args.getLastArg(options::OPT_mcpu_EQ)))
 success = getAArch64ArchFeaturesFromMcpu(D, A->getValue(), Args, Features);
   else if (Args.hasArg(options::OPT_arch))
-success = getAArch64ArchFeaturesFromMcpu(D, getAArch64TargetCPU(Args), 
Args,
- Features);
+success = getAArch64ArchFeaturesFromMcpu(D, getAArch64TargetCPU(Args, A),
+ Args, Features);
 
   if (success && (A = Args.getLastArg(options::OPT_mtune_EQ)))
 success =
 getAArch64MicroArchFeaturesFromMtune(D, A->getValue(), Args, Features);
   else if (success && (A = Args.getLastArg(options::OPT_mcpu_EQ)))
 success =
 getAArch64MicroArchFeaturesFromMcpu(D, A->getValue(), Args, Features);
-  else if (Args.hasArg(options::OPT_arch))
-success = getAArch64MicroArchFeaturesFromMcpu(D, getAArch64TargetCPU(Args),
-  Args, Features);
+  else if (success && Args.hasArg(options::OPT_arch))
+success = getAArch64MicroArchFeaturesFromMcpu(
+D, getAArch64TargetCPU(Args, A), Args, Features);
 
   if (!success)
 D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
Index: cfe/trunk/test/Driver/arm-cortex-cpus.c
===
--- cfe/trunk/test/Driver/arm-cortex-cpus.c
+++ cfe/trunk/test/Driver/arm-cortex-cpus.c
@@ -303,7 +303,10 @@
 
 // == Check that a bogus CPU gives an error
 // RUN: %clang -target arm -mcpu=bogus -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-BOGUS-CPU %s
+// RUN: %clang -target armv8-apple-darwin -arch arm64 -mcpu=bogus -### -c %s 
2>&1 | FileCheck -check-prefix=CHECK-BOGUS-CPU %s
 // CHECK-BOGUS-CPU: error: {{.*}} does not support '-mcpu=bogus'
+// RUN: %clang -target armv8-apple-darwin -arch arm64 -mtune=bogus -### -c %s 
2>&1 | FileCheck -check-prefix=CHECK-BOGUS-TUNE %s
+// CHECK-BOGUS-TUNE: error: {{.*}} does not support '-mtune=bogus'
 
 // == Check default Architecture on each ARM11 CPU
 // RUN: %clang -target arm-linux-gnueabi -mcpu=arm1136j-s -### -c %s 2>&1 | 
FileCheck -check-prefix=CHECK-CPUV6 %s


Index: cfe/trunk/lib/Driver/Tools.cpp
===
--- cfe/trunk/lib/Driver/Tools.cpp
+++ cfe/trunk/lib/Driver/Tools.cpp
@@ -1140,9 +1140,9 @@
 // ARM tools end.
 
 /// getAArch64TargetCPU - Get the (LLVM) name of the AArch64 cpu we are
-/// targeting.
-static std::string getAArch64TargetCPU(const ArgList ) {
-  Arg *A;
+/// targeting. Set \p A to the Arg corresponding to the -mcpu or -mtune
+/// arguments if they are provided, or to nullptr otherwise.
+static std::string getAArch64TargetCPU(const ArgList , Arg *) {
   std::string CPU;
   // If we have -mtune or -mcpu, use that.
   if ((A = Args.getLastArg(options::OPT_mtune_EQ))) {
@@ -1919,13 +1919,15 @@
 
 static std::string getCPUName(const ArgList , const llvm::Triple ,
   bool FromAs = false) {
+  Arg *A;
+
   switch (T.getArch()) {
   default:
 return "";
 
   case llvm::Triple::aarch64:
   case llvm::Triple::aarch64_be:
-return getAArch64TargetCPU(Args);
+return getAArch64TargetCPU(Args, A);
 
   case llvm::Triple::arm:
   case llvm::Triple::armeb:
@@ -2443,18 +2445,18 @@
   else if ((A = Args.getLastArg(options::OPT_mcpu_EQ)))
 success = 

r280998 - [Driver] Report invalid -mtune/-mcpu parameters when -arch=arm64

[Vedant Kumar via cfe-commits]

Author: vedantk
Date: Thu Sep  8 17:53:19 2016
New Revision: 280998

URL: http://llvm.org/viewvc/llvm-project?rev=280998=rev
Log:
[Driver] Report invalid -mtune/-mcpu parameters when -arch=arm64

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

Modified:
cfe/trunk/lib/Driver/Tools.cpp
cfe/trunk/test/Driver/arm-cortex-cpus.c

Modified: cfe/trunk/lib/Driver/Tools.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=280998=280997=280998=diff
==
--- cfe/trunk/lib/Driver/Tools.cpp (original)
+++ cfe/trunk/lib/Driver/Tools.cpp Thu Sep  8 17:53:19 2016
@@ -1140,9 +1140,9 @@ void Clang::AddARMTargetArgs(const llvm:
 // ARM tools end.
 
 /// getAArch64TargetCPU - Get the (LLVM) name of the AArch64 cpu we are
-/// targeting.
-static std::string getAArch64TargetCPU(const ArgList ) {
-  Arg *A;
+/// targeting. Set \p A to the Arg corresponding to the -mcpu or -mtune
+/// arguments if they are provided, or to nullptr otherwise.
+static std::string getAArch64TargetCPU(const ArgList , Arg *) {
   std::string CPU;
   // If we have -mtune or -mcpu, use that.
   if ((A = Args.getLastArg(options::OPT_mtune_EQ))) {
@@ -1919,13 +1919,15 @@ static StringRef getWebAssemblyTargetCPU
 
 static std::string getCPUName(const ArgList , const llvm::Triple ,
   bool FromAs = false) {
+  Arg *A;
+
   switch (T.getArch()) {
   default:
 return "";
 
   case llvm::Triple::aarch64:
   case llvm::Triple::aarch64_be:
-return getAArch64TargetCPU(Args);
+return getAArch64TargetCPU(Args, A);
 
   case llvm::Triple::arm:
   case llvm::Triple::armeb:
@@ -2443,8 +2445,8 @@ static void getAArch64TargetFeatures(con
   else if ((A = Args.getLastArg(options::OPT_mcpu_EQ)))
 success = getAArch64ArchFeaturesFromMcpu(D, A->getValue(), Args, Features);
   else if (Args.hasArg(options::OPT_arch))
-success = getAArch64ArchFeaturesFromMcpu(D, getAArch64TargetCPU(Args), 
Args,
- Features);
+success = getAArch64ArchFeaturesFromMcpu(D, getAArch64TargetCPU(Args, A),
+ Args, Features);
 
   if (success && (A = Args.getLastArg(options::OPT_mtune_EQ)))
 success =
@@ -2452,9 +2454,9 @@ static void getAArch64TargetFeatures(con
   else if (success && (A = Args.getLastArg(options::OPT_mcpu_EQ)))
 success =
 getAArch64MicroArchFeaturesFromMcpu(D, A->getValue(), Args, Features);
-  else if (Args.hasArg(options::OPT_arch))
-success = getAArch64MicroArchFeaturesFromMcpu(D, getAArch64TargetCPU(Args),
-  Args, Features);
+  else if (success && Args.hasArg(options::OPT_arch))
+success = getAArch64MicroArchFeaturesFromMcpu(
+D, getAArch64TargetCPU(Args, A), Args, Features);
 
   if (!success)
 D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);

Modified: cfe/trunk/test/Driver/arm-cortex-cpus.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/arm-cortex-cpus.c?rev=280998=280997=280998=diff
==
--- cfe/trunk/test/Driver/arm-cortex-cpus.c (original)
+++ cfe/trunk/test/Driver/arm-cortex-cpus.c Thu Sep  8 17:53:19 2016
@@ -303,7 +303,10 @@
 
 // == Check that a bogus CPU gives an error
 // RUN: %clang -target arm -mcpu=bogus -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-BOGUS-CPU %s
+// RUN: %clang -target armv8-apple-darwin -arch arm64 -mcpu=bogus -### -c %s 
2>&1 | FileCheck -check-prefix=CHECK-BOGUS-CPU %s
 // CHECK-BOGUS-CPU: error: {{.*}} does not support '-mcpu=bogus'
+// RUN: %clang -target armv8-apple-darwin -arch arm64 -mtune=bogus -### -c %s 
2>&1 | FileCheck -check-prefix=CHECK-BOGUS-TUNE %s
+// CHECK-BOGUS-TUNE: error: {{.*}} does not support '-mtune=bogus'
 
 // == Check default Architecture on each ARM11 CPU
 // RUN: %clang -target arm-linux-gnueabi -mcpu=arm1136j-s -### -c %s 2>&1 | 
FileCheck -check-prefix=CHECK-CPUV6 %s


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

Re: [PATCH] D24307: calculate extent size for memory regions allocated by C++ new expression

[Anna Zaks via cfe-commits]

zaks.anna added inline comments.


Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1003
@@ +1002,3 @@
+//
+ProgramStateRef MallocChecker::addExtentSize(CheckerContext ,
+ const CXXNewExpr *NE,

NoQ wrote:
> dkrupp wrote:
> > xazax.hun wrote:
> > > zaks.anna wrote:
> > > > I am not sure this code belongs to the malloc checker since it only 
> > > > supports the array bounds checker. Is there a reason it's not part of 
> > > > that checker?
> > > I think it is part of the malloc checker because it already does 
> > > something very very similar to malloc, see the MallocMemAux function. So 
> > > in fact, for the array bounds checker to work properly, the malloc 
> > > checker should be turned on.
> > Extent size is used by ArrayBoundChecker, ArrayBoundCheckerV2 and 
> > CStringChecker checkers currently. New expression in case of simple 
> > allocations (0 allocation) was already handled in Malloc checker , that's 
> > why I implemented it there. But I agree it feels odd that one has to switch 
> > on unix.Malloc checker to get the size of new allocated heap regions. 
> > Should I move this to ArrayBoundChecker or ArrayBoundCheckerV2?
> 1. Well, in my dreams, this code should be in the silent operator-new 
> modelling checker, which would be separate from the stateless operator-new 
> bug-finding checker. Then all the checkers that rely on extent would 
> automatically load the modelling checker as a dependency.
> 
> 2. Yeah, i think many checkers may consider extents, not just array bound; 
> other checkers may appear in the future. This info is very useful, which is 
> why we have the whole symbol class just for that (however, checker 
> communication through constraints on this symbol class wasn't IMHO a good 
> idea, because it's harder for the constraint manager to deal with symbols and 
> constraints rather than with concrete values).
> 
> //Just wanted to speak out, thoughts below might perhaps be more on-topic//
> 
> 3. The MallocChecker is not just unix.Malloc, but also cplusplus.NewDelete, 
> etc., which makes a lot more sense to leave it here.
> 
> 4. Consider another part of MallocChecker's job - modeling the memory spaces 
> for symbolic regions (heap vs. unknown). For malloc(), this is done in the 
> checker. For operator-new, it is done in the ExprEngine(!), because this is 
> part of the language rather than a library function. Perhaps ExprEngine would 
> be the proper place for that code as well.
> Well, in my dreams, this code should be in the silent operator-new modelling 
> checker, which would be 
> separate from the stateless operator-new bug-finding checker. Then all the 
> checkers that rely on extent 
> would automatically load the modelling checker as a dependency.

I agree. This sounds like the best approach! (Though it's not a must have to 
get the patch in.)

> Consider another part of MallocChecker's job - modeling the memory spaces for 
> symbolic regions (heap vs. 
> unknown). For malloc(), this is done in the checker. For operator-new, it is 
> done in the ExprEngine(!), because 
> this is part of the language rather than a library function. Perhaps 
> ExprEngine would be the proper place for 
> that code as well.

Interesting point. Can you clarify the last sentence?


Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1020
@@ +1019,3 @@
+  } else {
+Size = svalBuilder.makeIntVal(1, true);
+Region = (State->getSVal(NE, LCtx)).getAsRegion()->getAs();

Please, be more explicit that this is not a size of allocation (as in not 1 
byte)? Maybe call this ElementCount or something like that.


Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1021
@@ +1020,3 @@
+Size = svalBuilder.makeIntVal(1, true);
+Region = (State->getSVal(NE, LCtx)).getAsRegion()->getAs();
+  }

A bit of code repetition from above. Please, add comments explaining why we 
need subregion in one case and super region in the other.

Are there test cases for this branch?


Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1049
@@ +1048,3 @@
+   CharUnits Scaling, SValBuilder ) {
+  return Sb.evalBinOpNN(State, BO_Mul, BaseVal,
+Sb.makeArrayIndex(Scaling.getQuantity()),

This should probably be inline/have different name since it talks about 
ArrayIndexType and is not reused elsewhere.


Comment at: test/Analysis/out-of-bounds.cpp:1
@@ +1,2 @@
+// RUN: %clang_cc1 -std=c++11 -Wno-array-bounds -analyze 
-analyzer-checker=unix,core,alpha.security.ArrayBoundV2 -verify %s
+

Let's use a more specific file name.


https://reviews.llvm.org/D24307



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

Re: [PATCH] D24311: Implement MS _rot intrinsics

[Albert Gutowski via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL280997: Implement MS _rot intrinsics (authored by agutowski).

Changed prior to commit:
  https://reviews.llvm.org/D24311?vs=70747=70759#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24311

Files:
  cfe/trunk/include/clang/Basic/Builtins.def
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/lib/Headers/intrin.h
  cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c

Index: cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c
===
--- cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c
+++ cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c
@@ -0,0 +1,181 @@
+// RUN: %clang_cc1 -ffreestanding -fms-extensions -fms-compatibility -fms-compatibility-version=17.00 \
+// RUN: -triple i686--windows -emit-llvm %s -o - \
+// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
+// RUN: %clang_cc1 -ffreestanding -fms-extensions -fms-compatibility -fms-compatibility-version=17.00 \
+// RUN: -triple thumbv7--windows -emit-llvm %s -o - \
+// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
+// RUN: %clang_cc1 -ffreestanding -fms-extensions -fms-compatibility -fms-compatibility-version=17.00 \
+// RUN: -triple x86_64--windows -emit-llvm %s -o - \
+// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
+// RUN: %clang_cc1 -ffreestanding -fms-extensions -fms-compatibility -fms-compatibility-version=17.00 \
+// RUN: -triple i686--linux -emit-llvm %s -o - \
+// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
+// RUN: %clang_cc1 -ffreestanding -fms-extensions -fms-compatibility -fms-compatibility-version=17.00 \
+// RUN: -triple x86_64--linux -emit-llvm %s -o - \
+// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-64BIT-LONG
+
+// rotate left
+
+unsigned char test_rotl8(unsigned char value, unsigned char shift) {
+  return _rotl8(value, shift);
+}
+// CHECK: i8 @test_rotl8
+// CHECK:   [[SHIFT:%[0-9]+]] = and i8 %{{[0-9]+}}, 7
+// CHECK:   [[NEGSHIFT:%[0-9]+]] = sub i8 8, [[SHIFT]]
+// CHECK:   [[HIGH:%[0-9]+]] = shl i8 [[VALUE:%[0-9]+]], [[SHIFT]]
+// CHECK:   [[LOW:%[0-9]+]] = lshr i8 [[VALUE]], [[NEGSHIFT]]
+// CHECK:   [[ROTATED:%[0-9]+]] = or i8 [[HIGH]], [[LOW]]
+// CHECK:   [[ISZERO:%[0-9]+]] = icmp eq i8 [[SHIFT]], 0
+// CHECK:   [[RESULT:%[0-9]+]] = select i1 [[ISZERO]], i8 [[VALUE]], i8 [[ROTATED]]
+// CHECK:   ret i8 [[RESULT]]
+// CHECK  }
+
+unsigned short test_rotl16(unsigned short value, unsigned char shift) {
+  return _rotl16(value, shift);
+}
+// CHECK: i16 @test_rotl16
+// CHECK:   [[SHIFT:%[0-9]+]] = and i16 %{{[0-9]+}}, 15
+// CHECK:   [[NEGSHIFT:%[0-9]+]] = sub i16 16, [[SHIFT]]
+// CHECK:   [[HIGH:%[0-9]+]] = shl i16 [[VALUE:%[0-9]+]], [[SHIFT]]
+// CHECK:   [[LOW:%[0-9]+]] = lshr i16 [[VALUE]], [[NEGSHIFT]]
+// CHECK:   [[ROTATED:%[0-9]+]] = or i16 [[HIGH]], [[LOW]]
+// CHECK:   [[ISZERO:%[0-9]+]] = icmp eq i16 [[SHIFT]], 0
+// CHECK:   [[RESULT:%[0-9]+]] = select i1 [[ISZERO]], i16 [[VALUE]], i16 [[ROTATED]]
+// CHECK:   ret i16 [[RESULT]]
+// CHECK  }
+
+unsigned int test_rotl(unsigned int value, int shift) {
+  return _rotl(value, shift);
+}
+// CHECK: i32 @test_rotl
+// CHECK:   [[SHIFT:%[0-9]+]] = and i32 %{{[0-9]+}}, 31
+// CHECK:   [[NEGSHIFT:%[0-9]+]] = sub i32 32, [[SHIFT]]
+// CHECK:   [[HIGH:%[0-9]+]] = shl i32 [[VALUE:%[0-9]+]], [[SHIFT]]
+// CHECK:   [[LOW:%[0-9]+]] = lshr i32 [[VALUE]], [[NEGSHIFT]]
+// CHECK:   [[ROTATED:%[0-9]+]] = or i32 [[HIGH]], [[LOW]]
+// CHECK:   [[ISZERO:%[0-9]+]] = icmp eq i32 [[SHIFT]], 0
+// CHECK:   [[RESULT:%[0-9]+]] = select i1 [[ISZERO]], i32 [[VALUE]], i32 [[ROTATED]]
+// CHECK:   ret i32 [[RESULT]]
+// CHECK  }
+
+unsigned long test_lrotl(unsigned long value, int shift) {
+  return _lrotl(value, shift);
+}
+// CHECK-32BIT-LONG: i32 @test_lrotl
+// CHECK-32BIT-LONG:   [[SHIFT:%[0-9]+]] = and i32 %{{[0-9]+}}, 31
+// CHECK-32BIT-LONG:   [[NEGSHIFT:%[0-9]+]] = sub i32 32, [[SHIFT]]
+// CHECK-32BIT-LONG:   [[HIGH:%[0-9]+]] = shl i32 [[VALUE:%[0-9]+]], [[SHIFT]]
+// CHECK-32BIT-LONG:   [[LOW:%[0-9]+]] = lshr i32 [[VALUE]], [[NEGSHIFT]]
+// CHECK-32BIT-LONG:   [[ROTATED:%[0-9]+]] = or i32 [[HIGH]], [[LOW]]
+// CHECK-32BIT-LONG:   [[ISZERO:%[0-9]+]] = icmp eq i32 [[SHIFT]], 0
+// CHECK-32BIT-LONG:   [[RESULT:%[0-9]+]] = select i1 [[ISZERO]], i32 [[VALUE]], i32 [[ROTATED]]
+// CHECK-32BIT-LONG:   ret i32 [[RESULT]]
+// CHECK-32BIT-LONG  }
+
+// CHECK-64BIT-LONG: i64 @test_lrotl
+// CHECK-64BIT-LONG:   [[SHIFT:%[0-9]+]] = and i64 %{{[0-9]+}}, 63
+// CHECK-64BIT-LONG:   [[NEGSHIFT:%[0-9]+]] = sub i64 64, [[SHIFT]]
+// CHECK-64BIT-LONG:   [[HIGH:%[0-9]+]] = shl i64 [[VALUE:%[0-9]+]], [[SHIFT]]
+// CHECK-64BIT-LONG:   [[LOW:%[0-9]+]] = lshr i64 [[VALUE]], [[NEGSHIFT]]
+// CHECK-64BIT-LONG:   [[ROTATED:%[0-9]+]] = or i64 [[HIGH]], [[LOW]]
+// CHECK-64BIT-LONG:   [[ISZERO:%[0-9]+]] = icmp eq i64 

r280997 - Implement MS _rot intrinsics

[Albert Gutowski via cfe-commits]

Author: agutowski
Date: Thu Sep  8 17:32:19 2016
New Revision: 280997

URL: http://llvm.org/viewvc/llvm-project?rev=280997=rev
Log:
Implement MS _rot intrinsics

Reviewers: thakis, Prazek, compnerd, rnk

Subscribers: majnemer, cfe-commits

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

Added:
cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c
Modified:
cfe/trunk/include/clang/Basic/Builtins.def
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/Headers/intrin.h

Modified: cfe/trunk/include/clang/Basic/Builtins.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=280997=280996=280997=diff
==
--- cfe/trunk/include/clang/Basic/Builtins.def (original)
+++ cfe/trunk/include/clang/Basic/Builtins.def Thu Sep  8 17:32:19 2016
@@ -725,6 +725,16 @@ LANGBUILTIN(_InterlockedExchangePointer,
 LANGBUILTIN(_InterlockedIncrement,   "LiLiD*", "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(__noop,   "i.",  "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(__readfsdword,"ULiULi", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_rotl8,  "UcUcUc","n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_rotl16, "UsUsUc","n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_rotl,   "UiUii", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_lrotl,  "ULiULii",   "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_rotl64, "ULLiULLii", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_rotr8,  "UcUcUc","n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_rotr16, "UsUsUc","n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_rotr,   "UiUii", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_lrotr,  "ULiULii",   "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_rotr64, "ULLiULLii", "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(__va_start,   "vc**.", "nt", ALL_MS_LANGUAGES)
 
 // Microsoft library builtins.

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=280997=280996=280997=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Thu Sep  8 17:32:19 2016
@@ -696,6 +696,58 @@ RValue CodeGenFunction::EmitBuiltinExpr(
  "cast");
 return RValue::get(Result);
   }
+  case Builtin::BI_rotr8:
+  case Builtin::BI_rotr16:
+  case Builtin::BI_rotr:
+  case Builtin::BI_lrotr:
+  case Builtin::BI_rotr64: {
+Value *Val = EmitScalarExpr(E->getArg(0));
+Value *Shift = EmitScalarExpr(E->getArg(1));
+
+llvm::Type *ArgType = Val->getType();
+Shift = Builder.CreateIntCast(Shift, ArgType, false);
+unsigned ArgWidth = cast(ArgType)->getBitWidth();
+Value *ArgTypeSize = llvm::ConstantInt::get(ArgType, ArgWidth);
+Value *ArgZero = llvm::Constant::getNullValue(ArgType);
+
+Value *Mask = llvm::ConstantInt::get(ArgType, ArgWidth - 1);
+Shift = Builder.CreateAnd(Shift, Mask);
+Value *LeftShift = Builder.CreateSub(ArgTypeSize, Shift);
+
+Value *RightShifted = Builder.CreateLShr(Val, Shift);
+Value *LeftShifted = Builder.CreateShl(Val, LeftShift);
+Value *Rotated = Builder.CreateOr(LeftShifted, RightShifted);
+
+Value *ShiftIsZero = Builder.CreateICmpEQ(Shift, ArgZero);
+Value *Result = Builder.CreateSelect(ShiftIsZero, Val, Rotated);
+return RValue::get(Result);
+  }
+  case Builtin::BI_rotl8:
+  case Builtin::BI_rotl16:
+  case Builtin::BI_rotl:
+  case Builtin::BI_lrotl:
+  case Builtin::BI_rotl64: {
+Value *Val = EmitScalarExpr(E->getArg(0));
+Value *Shift = EmitScalarExpr(E->getArg(1));
+
+llvm::Type *ArgType = Val->getType();
+Shift = Builder.CreateIntCast(Shift, ArgType, false);
+unsigned ArgWidth = cast(ArgType)->getBitWidth();
+Value *ArgTypeSize = llvm::ConstantInt::get(ArgType, ArgWidth);
+Value *ArgZero = llvm::Constant::getNullValue(ArgType);
+
+Value *Mask = llvm::ConstantInt::get(ArgType, ArgWidth - 1);
+Shift = Builder.CreateAnd(Shift, Mask);
+Value *RightShift = Builder.CreateSub(ArgTypeSize, Shift);
+
+Value *LeftShifted = Builder.CreateShl(Val, Shift);
+Value *RightShifted = Builder.CreateLShr(Val, RightShift);
+Value *Rotated = Builder.CreateOr(LeftShifted, RightShifted);
+
+Value *ShiftIsZero = Builder.CreateICmpEQ(Shift, ArgZero);
+Value *Result = Builder.CreateSelect(ShiftIsZero, Val, Rotated);
+return RValue::get(Result);
+  }
   case Builtin::BI__builtin_unpredictable: {
 // Always return the argument of __builtin_unpredictable. LLVM does not
 // handle this builtin. Metadata for this builtin should be added directly

Modified: cfe/trunk/lib/Headers/intrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/intrin.h?rev=280997=280996=280997=diff
==
--- cfe/trunk/lib/Headers/intrin.h (original)
+++ cfe/trunk/lib/Headers/intrin.h Thu Sep  8 17:32:19 2016
@@ -447,61 +447,6 @@ 

Re: [PATCH] D23643: [Driver] Report invalid -mtune/-mcpu parameters when -arch=arm64

[Akira Hatanaka via cfe-commits]

ahatanak accepted this revision.
ahatanak added a reviewer: ahatanak.
ahatanak added a comment.
This revision is now accepted and ready to land.

LGTM. We should fix this longstanding bug.


https://reviews.llvm.org/D23643



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

Re: [PATCH] D24349: [clang-tidy] Extend readability-container-size-empty to arbitrary class with size() and empty()

[Piotr Padlewski via cfe-commits]

Prazek added inline comments.


Comment at: clang-tidy/readability/ContainerSizeEmptyCheck.cpp:34
@@ +33,3 @@
+  has(functionDecl(
+  isPublic(), hasName("size"), returns(isInteger()),
+  unless(anyOf(returns(isAnyCharacter()), returns(booleanType()),

Would be nice to have 'isStrictlyInteger' matcher to do this thing.


Repository:
  rL LLVM

https://reviews.llvm.org/D24349



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

Re: [PATCH] D23643: [Driver] Report invalid -mtune/-mcpu parameters when -arch=arm64

[Vedant Kumar via cfe-commits]

vsk added a comment.

Ping.


https://reviews.llvm.org/D23643



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

Re: [PATCH] D24330: Add some MS aliases for existing intrinsics

[Reid Kleckner via cfe-commits]

rnk added inline comments.


Comment at: include/clang/Basic/BuiltinsX86.def:304
@@ -303,2 +303,3 @@
 TARGET_BUILTIN(__builtin_ia32_ldmxcsr, "vUi", "", "sse")
+TARGET_BUILTIN(_mm_setcsr, "vUi", "", "sse")
 TARGET_BUILTIN(__builtin_ia32_stmxcsr, "Ui", "", "sse")

rsmith wrote:
> majnemer wrote:
> > rnk wrote:
> > > Part of the idea behind the Intel *mmintrin.h headers is that they define 
> > > symbols outside of the implementor's namespace. Users should be able to 
> > > write code that uses these _mm_* names if they don't include those 
> > > headers. Having this builtin always be available on x86 makes that 
> > > impossible.
> > > 
> > > That said, I can see that winnt.h uses these directly, rather than 
> > > including xmmintrin.h, so we need them to be builtins in MSVC mode, and 
> > > it's annoying to have them sometimes be builtins and sometimes be 
> > > functions.
> > We could have the header file use a pragma which turns the _mm intrinsics 
> > on along the same lines as `#pragma intrinsic`.  For MSVC compat, we could 
> > treat them as-if they were enabled by some similar mechanism.
> Names starting with an underscore are reserved to the implementation for use 
> in the global namespace. Also, we declare these lazily, on-demand, only if 
> name lookup would otherwise fail, so (outside of odd SFINAE cases in C++) 
> this shouldn't affect the meaning of any valid programs. While I find it 
> distasteful to model these as builtins, the only significant problem I see 
> here is that we'll accept non-portable code that forgets to include the right 
> header if we do so.
> 
> Perhaps we could model this somewhat similarly to `LIBBUILTIN`, so that we 
> allow use of the builtin without a declaration, but produce a diagnostic 
> about the missing `#include` when we do so?
+1 for doing something like LIBBUILTIN


https://reviews.llvm.org/D24330



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

Re: [PATCH] D24311: Implement MS _rot intrinsics

[Reid Kleckner 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/D24311



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

Re: [PATCH] D24289: Add warning when assigning enums to bitfields without an explicit unsigned underlying type

[Reid Kleckner via cfe-commits]

rnk added a comment.

I was thinking of suggesting to put it under -Wextra, but -Wall is fine too.

I still don't think it should be on by default. I think there are a lot of 
Unix-only projects out there that aren't concerned with MSVC compatibility, and 
a warning about how things with in the Microsoft ABI would be surprising and 
noisy. Maybe we could tack on "... to make this code portable" or something to 
the diagnostic text to make it more clear why the user should be concerned 
about this?


https://reviews.llvm.org/D24289



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

Re: [PATCH] D24349: [clang-tidy] Extend readability-container-size-empty to arbitrary class with size() and empty()

[Kirill Bobyrev via cfe-commits]

omtcyfz added a comment.

In https://reviews.llvm.org/D24349#537624, @aaron.ballman wrote:

> I think that's reasonable, depending on whether we find false positives with 
> the warning as well (I have a slight concern about `size()` and `empty()` 
> being unrelated operations on a non-container class that someone writes).


Agreed. I'd like to try LLVM as an example of a huge codebase tomorrow to see 
how many false-positives there actually are.

Generally, I can see the point of requiring `const` and providing options to 
reduce the set of allowed classes to `std::container`'s, but at the same time 
my thoughts are as follows:

1. Chances of people misusing C++ so much are very low. That's something I can 
check tomorrow just by running the check on few large open source codebases to 
see whether it's true.
2. If people are misusing C++ it's their fault anyway. One can do whatever he 
likes to do. One can rewrite all interfaces to STL containers and make `size()` 
and `empty()` most sophisticated functions the world has ever seen while 
performing whatever strange operations inside of them. Thus, I am not very 
supportive of the idea "let's try to think about all weird cornercases".

I'm very curious to see how many false-positives there would be in LLVM 
codebase, for example. I'll try it tomorrow.


Repository:
  rL LLVM

https://reviews.llvm.org/D24349



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

Re: [PATCH] D24289: Add warning when assigning enums to bitfields without an explicit unsigned underlying type

[Aaron Ballman via cfe-commits]

aaron.ballman added a subscriber: aaron.ballman.


Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2962
@@ -2959,1 +2961,3 @@
+  "the enum %0 an unsigned underlying type">,
+  InGroup>, DefaultIgnore;
 def warn_attribute_packed_for_bitfield : Warning<

thakis wrote:
> Hm, isn't the more safe default to have it on by default? A "what you're 
> doing isn't portable" warning seems useful, and people who don't care can 
> easily turn it off. And if it's not off by default, it's hard for people who 
> do care to turn it on (they'll have to know about the warning).
> 
> Maybe it should be off-by-default but part of -Wall?
I would also vote for this being on by default rather than off by default. We 
usually try to avoid adding new, off-by-default warnings because they are 
basically undiscoverable warnings that no one enables (unless we're adding them 
for GCC compatibility and GCC default ignores). I would be fine if it was only 
enabled with -Wall.


https://reviews.llvm.org/D24289



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

Re: [PATCH] D24048: [Driver] [Darwin] Add sanitizer libraries even if -nodefaultlibs is passed

[Richard Smith via cfe-commits]

On Thu, Sep 8, 2016 at 12:06 PM, Filipe Cabecinhas <
filcab+llvm.phabrica...@gmail.com> wrote:

> It seems some people on this thread (I'm sorry if everyone is taking this
> into account, but it seemed to me that this was going unnoticed, I want to
> make sure everyone is on the same page) is only talking about linking (or
> not) the sanitizer runtimes. But that's not the only thing.
>
> Depending on the OS, -fsanitize will also link in the sanitizer's
> dependencies, which might include libdl, libc++, and others.
>

I think this is actually consistent across OSs: on non-Darwin,
-nodefaultlibs turns off implicit linking of the sanitizer runtimes'
dependencies. On Darwin, there are no additional dependencies (the
sanitizer runtimes are linked as DSOs). Also, libc++ is not one of the
dependencies that we implicitly link due to -fsanitize. (Those are
-lpthread, -lrt, -lm, and sometimes -ldl.)

That was the reason why the patch was reverted last time we tried to change
> this flag's behavior: a sanitized libc++ needs the sanitizer libs, but not
> libc++... And it might need other dependencies for the sanitizers.
>
> Maybe have something like:
> -fsanitize=blah links all libs
> -fsanitize=blah -nodefaultlibs links nothing
> And then have two flags for explicitly linking either the sanitizer flag
> or the dependencies? Libc++ would still have the problem of wanting some
> dependencies but not all, though :-(
>

If the goal is to make it easier to link standard library implementations,
maybe instead we should add a -nodefaultlibs++, mirroring -nostdinc++, to
only remove the C++ standard library parts from the link and leave all
other implicit libs alone?

Thank you,
>
>  Filipe
>
> On Thursday, 8 September 2016, Anna Zaks  wrote:
>
>> zaks.anna added a comment.
>>
>> > I don't see the point of adding another flag to control this when we
>> already have a perfectly good set of
>>
>> >  flags that already do the right thing -- that takes us three levels
>> deep in flags overriding the behavior of
>>
>> >  other flags, and I don't see how it actually helps our users in any
>> way.
>>
>>
>> Going with silently linking the sanitizer runtimes when -nostdlib is
>> passed will cause regressions in user experience. They will start getting
>> inexplicable run time failures instead of build failures. Which solution do
>> you propose to fix this problem?
>>
>> This specific situation comes up often on internal mailing lists, so we
>> should not go for a solution that regresses the behavior on Darwin.
>>
>>
>> https://reviews.llvm.org/D24048
>>
>>
>>
>>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24278: [analyzer] Extend bug reports with extra notes.

[Vassil Vassilev via cfe-commits]

v.g.vassilev added a comment.

Thanks for working on this!

On my browser the note "Detected code clone" note appears slightly off the 
highlighted range which was a bit confusing to me.

Given my limited understanding of the SA bug reports, this looks good to me.



Comment at: lib/StaticAnalyzer/Checkers/CloneChecker.cpp:83
@@ +82,3 @@
+static PathDiagnosticLocation makeLocation(const StmtSequence ,
+  AnalysisManager ) {
+  ASTContext  = Mgr.getASTContext();

Probably an indent here would make this look more consistent.


Comment at: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp:348
@@ +347,3 @@
+
+
+bool AnalyzerOptions::shouldDisplayExtraNotesAsEvents() {

Extra new line?


https://reviews.llvm.org/D24278



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

Re: [PATCH] D24349: [clang-tidy] Extend readability-container-size-empty to arbitrary class with size() and empty()

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D24349#537595, @xazax.hun wrote:

> In https://reviews.llvm.org/D24349#537594, @omtcyfz wrote:
>
> > In https://reviews.llvm.org/D24349#537589, @Eugene.Zelenko wrote:
> >
> > > If size() and empty() change object's state, it may be not equivalent 
> > > replacement.
> >
> >
> > True. But my point is that they are not required to do that if they're just 
> > not marked `const`.
>
>
> I agree with Eugene. But I think a good consensus could be to warn on the 
> non-const case but do not do the rewrite. What do you think?


I think that's reasonable, depending on whether we find false positives with 
the warning as well (I have a slight concern about `size()` and `empty()` being 
unrelated operations on a non-container class that someone writes).


Repository:
  rL LLVM

https://reviews.llvm.org/D24349



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

Re: [PATCH] D24048: [Driver] [Darwin] Add sanitizer libraries even if -nodefaultlibs is passed

[Richard Smith via cfe-commits]

On Thu, Sep 8, 2016 at 11:19 AM, Anna Zaks  wrote:

> zaks.anna added a comment.
>
> > I don't see the point of adding another flag to control this when we
> already have a perfectly good set of
>
> >  flags that already do the right thing -- that takes us three levels
> deep in flags overriding the behavior of
>
> >  other flags, and I don't see how it actually helps our users in any way.
>
>
> Going with silently linking the sanitizer runtimes when -nostdlib is
> passed will cause regressions in user experience. They will start getting
> inexplicable run time failures instead of build failures. Which solution do
> you propose to fix this problem?

This specific situation comes up often on internal mailing lists, so we
> should not go for a solution that regresses the behavior on Darwin.


Ultimately, the clang driver should do whatever makes sense for Darwin when
targeting Darwin, and in this case perhaps being compatible with earlier
versions of the driver is the right thing. However, if we reach consensus
that the non-Darwin behavior is preferable, we do have reasonable options
to transition. For instance, we could change the driver to produce a
warning if -nodefaultlibs is enabled with a -fsanitize= flag, suggesting
adding -fno-sanitize=all to avoid linking the sanitizer runtimes, and then
after a release or two, change the behavior for that case to link the
sanitizer runtimes. We've done similar things for other changes to
sanitizer flags in the past and it's worked out pretty well. If we go that
way, we could add a -f flag to force linking the sanitizer runtime even
with -nodefaultlibs to tide libc++ over until the Darwin driver is brought
in line with our other modes. Likewise, if we decide that the current
Darwin behavior is preferable, we can transition the driver behavior for
non-Darwin targets in an analogous way.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24314: [libc++] Clean up MSVC support

[Saleem Abdulrasool via cfe-commits]

compnerd closed this revision.
compnerd added a comment.

SVN r280988


https://reviews.llvm.org/D24314



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

[libcxx] r280988 - support: clean up MSVC support

[Saleem Abdulrasool via cfe-commits]

Author: compnerd
Date: Thu Sep  8 15:52:48 2016
New Revision: 280988

URL: http://llvm.org/viewvc/llvm-project?rev=280988=rev
Log:
support: clean up MSVC support

Visual Studio 2013 (CRT version 12) added support for many C99 long long and
long double functions. Visual Studio 2015 (CRT version 14) increased C99 and C11
compliance further. Since we don't support Visual Studio versions older than
2013, we can considerably clean up the support header.

Patch by Shoaib Meenai!

Modified:
libcxx/trunk/include/support/win32/support.h

Modified: libcxx/trunk/include/support/win32/support.h
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/support/win32/support.h?rev=280988=280987=280988=diff
==
--- libcxx/trunk/include/support/win32/support.h (original)
+++ libcxx/trunk/include/support/win32/support.h Thu Sep  8 15:52:48 2016
@@ -22,7 +22,7 @@
 #include 
 #endif
 #if defined(_LIBCPP_MSVCRT)
-#include 
+#include 
 #endif
 #define swprintf _snwprintf
 #define vswprintf _vsnwprintf
@@ -44,26 +44,8 @@ size_t wcsnrtombs(char *__restrict dst,
 }
 #endif // __MINGW32__
 
-#if defined(_LIBCPP_MSVCRT)
+#if defined(_VC_CRT_MAJOR_VERSION) && _VC_CRT_MAJOR_VERSION < 14
 #define snprintf _snprintf
-#define atoll _atoi64
-#define strtoll _strtoi64
-#define strtoull _strtoui64
-#define wcstoll _wcstoi64
-#define wcstoull _wcstoui64
-_LIBCPP_ALWAYS_INLINE float strtof(const char *nptr, char **endptr)
-{
-  return _Stof(nptr, endptr, 0);
-}
-_LIBCPP_ALWAYS_INLINE double strtod(const char *nptr, char **endptr)
-{
-  return _Stod(nptr, endptr, 0);
-}
-_LIBCPP_ALWAYS_INLINE long double strtold(const char *nptr, char **endptr)
-{
-  return _Stold(nptr, endptr, 0);
-}
-
 #define _Exit _exit
 #endif
 


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

Re: [PATCH] D24311: Implement MS _rot intrinsics

[Albert Gutowski via cfe-commits]

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

https://reviews.llvm.org/D24311



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

Re: [PATCH] D24311: Implement MS _rot intrinsics

[Albert Gutowski via cfe-commits]

agutowski updated this revision to Diff 70747.
agutowski added a comment.

Remove evaluating values for constant arguments


https://reviews.llvm.org/D24311

Files:
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/intrin.h
  test/CodeGen/ms-intrinsics-rotations.c

Index: lib/Headers/intrin.h
===
--- lib/Headers/intrin.h
+++ lib/Headers/intrin.h
@@ -447,61 +447,6 @@
   return (unsigned __int64)__in1 * (unsigned __int64)__in2;
 }
 /**\
-|* Bit Twiddling
-\**/
-static __inline__ unsigned char __DEFAULT_FN_ATTRS
-_rotl8(unsigned char _Value, unsigned char _Shift) {
-  _Shift &= 0x7;
-  return _Shift ? (_Value << _Shift) | (_Value >> (8 - _Shift)) : _Value;
-}
-static __inline__ unsigned char __DEFAULT_FN_ATTRS
-_rotr8(unsigned char _Value, unsigned char _Shift) {
-  _Shift &= 0x7;
-  return _Shift ? (_Value >> _Shift) | (_Value << (8 - _Shift)) : _Value;
-}
-static __inline__ unsigned short __DEFAULT_FN_ATTRS
-_rotl16(unsigned short _Value, unsigned char _Shift) {
-  _Shift &= 0xf;
-  return _Shift ? (_Value << _Shift) | (_Value >> (16 - _Shift)) : _Value;
-}
-static __inline__ unsigned short __DEFAULT_FN_ATTRS
-_rotr16(unsigned short _Value, unsigned char _Shift) {
-  _Shift &= 0xf;
-  return _Shift ? (_Value >> _Shift) | (_Value << (16 - _Shift)) : _Value;
-}
-static __inline__ unsigned int __DEFAULT_FN_ATTRS
-_rotl(unsigned int _Value, int _Shift) {
-  _Shift &= 0x1f;
-  return _Shift ? (_Value << _Shift) | (_Value >> (32 - _Shift)) : _Value;
-}
-static __inline__ unsigned int __DEFAULT_FN_ATTRS
-_rotr(unsigned int _Value, int _Shift) {
-  _Shift &= 0x1f;
-  return _Shift ? (_Value >> _Shift) | (_Value << (32 - _Shift)) : _Value;
-}
-static __inline__ unsigned long __DEFAULT_FN_ATTRS
-_lrotl(unsigned long _Value, int _Shift) {
-  _Shift &= 0x1f;
-  return _Shift ? (_Value << _Shift) | (_Value >> (32 - _Shift)) : _Value;
-}
-static __inline__ unsigned long __DEFAULT_FN_ATTRS
-_lrotr(unsigned long _Value, int _Shift) {
-  _Shift &= 0x1f;
-  return _Shift ? (_Value >> _Shift) | (_Value << (32 - _Shift)) : _Value;
-}
-static
-__inline__ unsigned __int64 __DEFAULT_FN_ATTRS
-_rotl64(unsigned __int64 _Value, int _Shift) {
-  _Shift &= 0x3f;
-  return _Shift ? (_Value << _Shift) | (_Value >> (64 - _Shift)) : _Value;
-}
-static
-__inline__ unsigned __int64 __DEFAULT_FN_ATTRS
-_rotr64(unsigned __int64 _Value, int _Shift) {
-  _Shift &= 0x3f;
-  return _Shift ? (_Value >> _Shift) | (_Value << (64 - _Shift)) : _Value;
-}
-/**\
 |* Bit Counting and Testing
 \**/
 static __inline__ unsigned char __DEFAULT_FN_ATTRS
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -696,6 +696,58 @@
  "cast");
 return RValue::get(Result);
   }
+  case Builtin::BI_rotr8:
+  case Builtin::BI_rotr16:
+  case Builtin::BI_rotr:
+  case Builtin::BI_lrotr:
+  case Builtin::BI_rotr64: {
+Value *Val = EmitScalarExpr(E->getArg(0));
+Value *Shift = EmitScalarExpr(E->getArg(1));
+
+llvm::Type *ArgType = Val->getType();
+Shift = Builder.CreateIntCast(Shift, ArgType, false);
+unsigned ArgWidth = cast(ArgType)->getBitWidth();
+Value *ArgTypeSize = llvm::ConstantInt::get(ArgType, ArgWidth);
+Value *ArgZero = llvm::Constant::getNullValue(ArgType);
+
+Value *Mask = llvm::ConstantInt::get(ArgType, ArgWidth - 1);
+Shift = Builder.CreateAnd(Shift, Mask);
+Value *LeftShift = Builder.CreateSub(ArgTypeSize, Shift);
+
+Value *RightShifted = Builder.CreateLShr(Val, Shift);
+Value *LeftShifted = Builder.CreateShl(Val, LeftShift);
+Value *Rotated = Builder.CreateOr(LeftShifted, RightShifted);
+
+Value *ShiftIsZero = Builder.CreateICmpEQ(Shift, ArgZero);
+Value *Result = Builder.CreateSelect(ShiftIsZero, Val, Rotated);
+return RValue::get(Result);
+  }
+  case Builtin::BI_rotl8:
+  case Builtin::BI_rotl16:
+  case Builtin::BI_rotl:
+  case Builtin::BI_lrotl:
+  case Builtin::BI_rotl64: {
+Value *Val = EmitScalarExpr(E->getArg(0));
+Value *Shift = EmitScalarExpr(E->getArg(1));
+
+llvm::Type *ArgType = Val->getType();
+Shift = Builder.CreateIntCast(Shift, ArgType, false);
+unsigned ArgWidth = cast(ArgType)->getBitWidth();
+Value *ArgTypeSize = llvm::ConstantInt::get(ArgType, ArgWidth);
+Value *ArgZero = llvm::Constant::getNullValue(ArgType);
+
+Value *Mask = llvm::ConstantInt::get(ArgType, ArgWidth - 1);
+Shift = Builder.CreateAnd(Shift, Mask);
+Value *RightShift = Builder.CreateSub(ArgTypeSize, Shift);
+
+Value 

Re: [PATCH] D24311: Implement MS _rot intrinsics

[Reid Kleckner via cfe-commits]

rnk added inline comments.


Comment at: lib/AST/ExprConstant.cpp:7024-7050
@@ -7023,1 +7023,29 @@
 
+  case Builtin::BI_rotl8:
+  case Builtin::BI_rotl16:
+  case Builtin::BI_rotl:
+  case Builtin::BI_lrotl:
+  case Builtin::BI_rotl64: {
+APSInt Val, Shift;
+if (!EvaluateInteger(E->getArg(0), Val, Info) ||
+!EvaluateInteger(E->getArg(1), Shift, Info))
+  return false;
+
+APSInt BitWidth(llvm::APInt(32, Val.getBitWidth()));
+return Success(Val.rotl(Shift % BitWidth), E);
+  }
+
+  case Builtin::BI_rotr8:
+  case Builtin::BI_rotr16:
+  case Builtin::BI_rotr:
+  case Builtin::BI_lrotr:
+  case Builtin::BI_rotr64: {
+APSInt Val, Shift;
+if (!EvaluateInteger(E->getArg(0), Val, Info) ||
+!EvaluateInteger(E->getArg(1), Shift, Info))
+  return false;
+
+APSInt BitWidth(llvm::APInt(32, Val.getBitWidth()));
+return Success(Val.rotr(Shift % BitWidth), E);
+  }
+

majnemer wrote:
> agutowski wrote:
> > majnemer wrote:
> > > Any reason why we need this?
> > > 
> > > Given:
> > >   #include 
> > > 
> > >   constexpr int x = _rotl8(1, 2);
> > > 
> > > MSVC 2015 reports:
> > >   error C2131: expression did not evaluate to a constant
> > Hm, I don't know. Is there any reason why we shouldn't do this? I mean, I 
> > just had the feeling that if we can evaluate something during compilation 
> > time, we should to it.
> The best reason I can think of is that it allows people to use a 
> Microsoft-specific intrinsic in ways which won't compile with Microsoft's 
> compiler.
Yeah, I agree, we should drop the constexpr part of this. The use case for 
these intrinsics is really to get at the x86 instructions.


https://reviews.llvm.org/D24311



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

r280984 - [modules] Apply ODR merging for function scoped tags only in C++ mode.

[Vassil Vassilev via cfe-commits]

Author: vvassilev
Date: Thu Sep  8 15:34:41 2016
New Revision: 280984

URL: http://llvm.org/viewvc/llvm-project?rev=280984=rev
Log:
[modules] Apply ODR merging for function scoped tags only in C++ mode.

In C mode, if we have a visible declaration but not a visible definition, a tag
defined in the declaration should be have a visible definition. In C++ we rely
on the ODR merging, whereas in C we cannot because each declaration of a
function gets its own set of declarations in its prototype scope.

Patch developed in collaboration with Richard Smith!

Added:
cfe/trunk/test/Modules/Inputs/merge-fn-prototype-tags/
cfe/trunk/test/Modules/Inputs/merge-fn-prototype-tags/a.h
cfe/trunk/test/Modules/Inputs/merge-fn-prototype-tags/b.h
cfe/trunk/test/Modules/Inputs/merge-fn-prototype-tags/c.h
cfe/trunk/test/Modules/Inputs/merge-fn-prototype-tags/module.modulemap
cfe/trunk/test/Modules/merge-fn-prototype-tags.c
Modified:
cfe/trunk/lib/Sema/SemaLookup.cpp

Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=280984=280983=280984=diff
==
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Thu Sep  8 15:34:41 2016
@@ -1543,7 +1543,12 @@ bool LookupResult::isVisibleSlow(Sema 
 // For a parameter, check whether our current template declaration's
 // lexical context is visible, not whether there's some other visible
 // definition of it, because parameters aren't "within" the definition.
-if ((D->isTemplateParameter() || isa(D))
+//
+// In C++ we need to check for a visible definition due to ODR merging,
+// and in C we must not because each declaration of a function gets its own
+// set of declarations for tags in prototype scope.
+if ((D->isTemplateParameter() || isa(D)
+ || (isa(DC) && !SemaRef.getLangOpts().CPlusPlus))
 ? isVisible(SemaRef, cast(DC))
 : SemaRef.hasVisibleDefinition(cast(DC))) {
   if (SemaRef.ActiveTemplateInstantiations.empty() &&

Added: cfe/trunk/test/Modules/Inputs/merge-fn-prototype-tags/a.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-fn-prototype-tags/a.h?rev=280984=auto
==
--- cfe/trunk/test/Modules/Inputs/merge-fn-prototype-tags/a.h (added)
+++ cfe/trunk/test/Modules/Inputs/merge-fn-prototype-tags/a.h Thu Sep  8 
15:34:41 2016
@@ -0,0 +1,2 @@
+#include "c.h"
+void ftw(struct stat);

Added: cfe/trunk/test/Modules/Inputs/merge-fn-prototype-tags/b.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-fn-prototype-tags/b.h?rev=280984=auto
==
--- cfe/trunk/test/Modules/Inputs/merge-fn-prototype-tags/b.h (added)
+++ cfe/trunk/test/Modules/Inputs/merge-fn-prototype-tags/b.h Thu Sep  8 
15:34:41 2016
@@ -0,0 +1 @@
+struct stat;

Added: cfe/trunk/test/Modules/Inputs/merge-fn-prototype-tags/c.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-fn-prototype-tags/c.h?rev=280984=auto
==
--- cfe/trunk/test/Modules/Inputs/merge-fn-prototype-tags/c.h (added)
+++ cfe/trunk/test/Modules/Inputs/merge-fn-prototype-tags/c.h Thu Sep  8 
15:34:41 2016
@@ -0,0 +1,4 @@
+#ifndef _STAT_H_
+#define _STAT_H_
+struct stat {};
+#endif

Added: cfe/trunk/test/Modules/Inputs/merge-fn-prototype-tags/module.modulemap
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-fn-prototype-tags/module.modulemap?rev=280984=auto
==
--- cfe/trunk/test/Modules/Inputs/merge-fn-prototype-tags/module.modulemap 
(added)
+++ cfe/trunk/test/Modules/Inputs/merge-fn-prototype-tags/module.modulemap Thu 
Sep  8 15:34:41 2016
@@ -0,0 +1,5 @@
+module M {
+  header "a.h"
+  module mod_c { header "c.h" }
+  header "b.h"
+}

Added: cfe/trunk/test/Modules/merge-fn-prototype-tags.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-fn-prototype-tags.c?rev=280984=auto
==
--- cfe/trunk/test/Modules/merge-fn-prototype-tags.c (added)
+++ cfe/trunk/test/Modules/merge-fn-prototype-tags.c Thu Sep  8 15:34:41 2016
@@ -0,0 +1,8 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -x c -I%S/Inputs/merge-fn-prototype-tags -verify %s
+// RUN: %clang_cc1 -fmodules 
-fmodule-map-file=%S/Inputs/merge-fn-prototype-tags/module.modulemap 
-fmodules-cache-path=%t -x c -I%S/Inputs/merge-fn-prototype-tags -verify %s
+
+#include "c.h"
+void mmalloc_attach() { struct stat sbuf; }
+
+// expected-no-diagnostics


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

Re: [PATCH] D22642: CodeGen: Clean up implementation of vtable initializer builder. NFC.

[Richard Smith via cfe-commits]

On Wed, Sep 7, 2016 at 7:14 PM, Richard Smith  wrote:

> On 7 Sep 2016 6:59 pm, "Peter Collingbourne"  wrote:
>
> On Wed, Sep 7, 2016 at 6:44 PM, Richard Smith 
> wrote:
>
>> On 7 Sep 2016 6:23 pm, "Peter Collingbourne"  wrote:
>>
>> pcc marked 4 inline comments as done.
>>
>> 
>> Comment at: lib/CodeGen/CGVTables.cpp:588
>> @@ +587,3 @@
>> +if (auto *F = dyn_cast(Cache))
>> +  F->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
>> +Cache = llvm::ConstantExpr::getBitCast(Cache, CGM.Int8PtrTy);
>> 
>> rsmith wrote:
>> > Do you have any idea why we're doing this? It looks wrong to me. These
>> ABI entry points are exposed and could certainly have their addresses taken
>> and used in this translation unit.
>> I introduced this in D18071. Although a translation unit can take the
>> address of one of these functions, that would involve declaring a function
>> with a reserved name, so I believe we'd be allowed to impose restrictions
>> such as `unnamed_addr` on the address.
>>
>>
>> These are declared by , and thus presumably intended to be
>> useable by programs, so I'm not convinced that reasoning applies.
>>
>
> cxxabi.h isn't part of the standard, is it? So whatever it does shouldn't
> have an effect on the standard.
>
>
> It's part of the itanium c++ abi standard.
>

OK, so this is actually not completely clear. On the one hand,
__cxa_pure_virtual is declared by GCC's  and libc++abi's
, and documented in the ABI description as a function that is
part of the API provided by the implementation, but on the other hand, it's
not part of the "reference implementation" of  provided as part
of the Itanium C++ ABI.

Reasonable code is not going to use the address of this function, and even
in unreasonable code, the only really plausible use would be comparing the
address of __cxa_pure_virtual to the contents of a vtable slot, which is
already not guaranteed to work since implementations aren't required to put
that function pointer there.

Do we gain anything from this?
>>
>
> Not at present (the relative ABI is the only user I know of and that isn't
> fully implemented yet on trunk), although something like this may be
> required if we ever fully implement the relative ABI.
>
>
Since the idea here is to allow use of a relative address for the function,
it strikes me that there's another option: we could emit an unnamed_addr
linkonce_odr hidden visibility forwarding thunk for the function
(effectively, doing the lowering to a PLT thunk in the frontend). That'd
let us form a relative address for the function without disturbing other
uses of the symbol. However, the complexity of that solution doesn't seem
justified by the (largely theoretical) benefit to me.

Let's leave this alone, then. (We could presumably restrict it to just the
relative-ABI case, but I think we'd prefer to find out sooner if this
actually causes problems for someone.)

It also seems like there's an LLVM IR deficiency here to me -- there's
apparently no way to state that one particular use of a global is
unnamed_addr without making the global itself be unnamed_addr.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24349: [clang-tidy] Extend readability-container-size-empty to arbitrary class with size() and empty()

[Gábor Horváth via cfe-commits]

xazax.hun added a comment.

In https://reviews.llvm.org/D24349#537594, @omtcyfz wrote:

> In https://reviews.llvm.org/D24349#537589, @Eugene.Zelenko wrote:
>
> > If size() and empty() change object's state, it may be not equivalent 
> > replacement.
>
>
> True. But my point is that they are not required to do that if they're just 
> not marked `const`.


I agree with Eugene. But I think a good consensus could be to warn on the 
non-const case but do not do the rewrite. What do you think?


Repository:
  rL LLVM

https://reviews.llvm.org/D24349



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

Re: [PATCH] D24349: [clang-tidy] Extend readability-container-size-empty to arbitrary class with size() and empty()

[Kirill Bobyrev via cfe-commits]

omtcyfz added a comment.

In https://reviews.llvm.org/D24349#537589, @Eugene.Zelenko wrote:

> If size() and empty() change object's state, it may be not equivalent 
> replacement.


True. But my point is that they are not required to do that if they're just not 
marked `const`.


Repository:
  rL LLVM

https://reviews.llvm.org/D24349



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

Re: [PATCH] D24349: [clang-tidy] Extend readability-container-size-empty to arbitrary class with size() and empty()

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko added a comment.

If size() and empty() change object's state, it may be not equivalent 
replacement.


Repository:
  rL LLVM

https://reviews.llvm.org/D24349



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

Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation

[Tavian Barnes via cfe-commits]

tavianator added inline comments.


Comment at: src/cxa_thread_atexit.cpp:70
@@ +69,3 @@
+while (auto head = dtors) {
+  dtors = head->next;
+  head->dtor(head->obj);

EricWF wrote:
> tavianator wrote:
> > EricWF wrote:
> > > There is a bug here. If `head->next == nullptr` and if 
> > > `head->dtor(head->obj))` creates a TL variable in the destructor then 
> > > that destructor will not be invoked.
> > > 
> > > Here's an updated test case which catches the bug: 
> > > https://gist.github.com/EricWF/3bb50d4f28b91aa28d2adefea0e94a0e
> > I can't reproduce that failure here, your exact test case passes (even with 
> > `#undef HAVE___CXA_THREAD_ATEXIT_IMPL` and the weak symbol test commented 
> > out).
> > 
> > Tracing the implementation logic, it seems correct.  If `head->next == 
> > nullptr` then this line does `dtors = nullptr`.  Then if 
> > `head->dtor(head->obj)` registers a new `thread_local`, 
> > `__cxa_thread_atexit()` does `head = malloc(...); ... dtors = head;`.  Then 
> > the next iteration of the loop `while (auto head = dtors) {` picks up that 
> > new node.
> > 
> > Have I missed something?
> I can't reproduce this morning either, I must have been doing something 
> funny. I'll look at this with a fresh head tomorrow. If I can't find anything 
> this will be good to go. Thanks for working on this. 
No problem!  I can integrate your updated test case anyway if you want.


https://reviews.llvm.org/D21803



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

Re: [PATCH] D24349: [clang-tidy] Extend readability-container-size-empty to arbitrary class with size() and empty()

[Kirill Bobyrev via cfe-commits]

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

In https://reviews.llvm.org/D24349#537350, @Eugene.Zelenko wrote:

> Probably check should have options to extend list of containers and also to 
> assume all classes with integer type size() const and bool empty() const as 
> containers. It may be not trivial to find out all custom containers and last 
> option will be helpful to assemble such list.


I was thinking about

In https://reviews.llvm.org/D24349#537552, @aaron.ballman wrote:

> In https://reviews.llvm.org/D24349#537549, @Eugene.Zelenko wrote:
>
> > Should we also check for absence of parameters in size() and empty() as 
> > well as const?
>
>
> I think that would be reasonable.


I do not agree about the const part. It should be encouraged to use const for 
these, but I do not think they should be a requirement.


Repository:
  rL LLVM

https://reviews.llvm.org/D24349



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

Re: [PATCH] D24065: [libc++] Use _LIBCPP_TYPE_VIS_ONLY with enum class

[Shoaib Meenai via cfe-commits]

smeenai added a comment.

Friendly ping :)


https://reviews.llvm.org/D24065



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

Re: [PATCH] D24289: Add warning when assigning enums to bitfields without an explicit unsigned underlying type

[Nico Weber via cfe-commits]

thakis added inline comments.


Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2962
@@ -2959,1 +2961,3 @@
+  "the enum %0 an unsigned underlying type">,
+  InGroup>, DefaultIgnore;
 def warn_attribute_packed_for_bitfield : Warning<

Hm, isn't the more safe default to have it on by default? A "what you're doing 
isn't portable" warning seems useful, and people who don't care can easily turn 
it off. And if it's not off by default, it's hard for people who do care to 
turn it on (they'll have to know about the warning).

Maybe it should be off-by-default but part of -Wall?


https://reviews.llvm.org/D24289



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

Re: [PATCH] D24349: [clang-tidy] Extend readability-container-size-empty to arbitrary class with size() and empty()

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D24349#537549, @Eugene.Zelenko wrote:

> Should we also check for absence of parameters in size() and empty() as well 
> as const?


I think that would be reasonable.


https://reviews.llvm.org/D24349



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

Re: [PATCH] D24349: [clang-tidy] Extend readability-container-size-empty to arbitrary class with size() and empty()

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko added a comment.

Should we also check for absence of parameters in size() and empty() as well as 
const?


https://reviews.llvm.org/D24349



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

Re: [PATCH] D21698: [OpenCL] Allow disabling types and declarations associated with extensions

[Yaxun Liu via cfe-commits]

yaxunl updated this revision to Diff 70738.
yaxunl marked 5 inline comments as done.
yaxunl added a comment.

Revised by Anastasia's comments.


https://reviews.llvm.org/D21698

Files:
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/OpenCLImageTypes.def
  include/clang/Basic/OpenCLOptions.h
  include/clang/Sema/Overload.h
  include/clang/Sema/Sema.h
  include/clang/Serialization/ASTReader.h
  lib/Basic/Targets.cpp
  lib/Frontend/InitPreprocessor.cpp
  lib/Headers/opencl-c.h
  lib/Parse/ParsePragma.cpp
  lib/Parse/Parser.cpp
  lib/Sema/DeclSpec.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaCast.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaType.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  test/CodeGenOpenCL/extension-begin.cl
  test/Parser/opencl-atomics-cl20.cl
  test/Parser/opencl-pragma.cl
  test/SemaOpenCL/extension-begin.cl

Index: test/SemaOpenCL/extension-begin.cl
===
--- /dev/null
+++ test/SemaOpenCL/extension-begin.cl
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only
+
+#pragma OPENCL EXTENSION all : begin // expected-warning {{expected 'disable' - ignoring}}
+#pragma OPENCL EXTENSION all : end // expected-warning {{expected 'disable' - ignoring}}
+
+#pragma OPENCL EXTENSION my_ext : begin 
+
+struct A {
+  int a;
+};
+
+void f(void);
+
+__attribute__((overloadable)) void g(long x);
+
+#pragma OPENCL EXTENSION my_ext : end
+#pragma OPENCL EXTENSION my_ext : end // expected-warning {{OpenCL extension end directive mismatches begin directive - ignoring}}
+
+__attribute__((overloadable)) void g(void);
+
+#pragma OPENCL EXTENSION my_ext : enable
+void test_f1(void) {
+  struct A test_A1;
+  f();
+  g(0);
+}
+
+#pragma OPENCL EXTENSION my_ext : disable 
+void test_f2(void) {
+  struct A test_A2; // expected-error {{use of type 'struct A' requires my_ext extension to be enabled}}
+  f(); // expected-error {{use of declaration requires my_ext extension to be enabled}}
+  g(0); // expected-error {{no matching function for call to 'g'}}
+// expected-note@-19 {{candidate disabled due to OpenCL extension}}
+// expected-note@-15 {{candidate function not viable: requires 0 arguments, but 1 was provided}}
+}
Index: test/Parser/opencl-pragma.cl
===
--- test/Parser/opencl-pragma.cl
+++ test/Parser/opencl-pragma.cl
@@ -5,9 +5,9 @@
 #pragma OPENCL EXTENSION cl_no_such_extension : disable /* expected-warning {{unknown OpenCL extension 'cl_no_such_extension' - ignoring}} */
 
 #pragma OPENCL EXTENSION all : disable
-#pragma OPENCL EXTENSION all : enable /* expected-warning {{unknown OpenCL extension 'all' - ignoring}} */
+#pragma OPENCL EXTENSION all : enable /* expected-warning {{expected 'disable' - ignoring}} */
 
-#pragma OPENCL EXTENSION cl_khr_fp64 : on /* expected-warning {{expected 'enable' or 'disable' - ignoring}} */
+#pragma OPENCL EXTENSION cl_khr_fp64 : on /* expected-warning {{expected 'enable', 'disable', 'begin' or 'end' - ignoring}} */
 
 #pragma OPENCL FP_CONTRACT ON
 #pragma OPENCL FP_CONTRACT OFF
Index: test/Parser/opencl-atomics-cl20.cl
===
--- test/Parser/opencl-atomics-cl20.cl
+++ test/Parser/opencl-atomics-cl20.cl
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only
 // RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -fsyntax-only -cl-std=CL2.0 -DCL20
+// RUN: %clang_cc1 %s -triple spir64-unknown-unknown -verify -fsyntax-only -cl-std=CL2.0 -DCL20
 // RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -fsyntax-only -cl-std=CL2.0 -DCL20 -DEXT -Wpedantic-core-features
 
 #ifdef EXT
@@ -47,14 +48,16 @@
 // expected-error@-28 {{use of type 'atomic_ulong' (aka '_Atomic(unsigned long)') requires cl_khr_int64_extended_atomics extension to be enabled}}
 // expected-error@-27 {{use of type 'atomic_double' (aka '_Atomic(double)') requires cl_khr_int64_base_atomics extension to be enabled}}
 // expected-error@-28 {{use of type 'atomic_double' (aka '_Atomic(double)') requires cl_khr_int64_extended_atomics extension to be enabled}}
-// expected-error-re@-27 {{use of type 'atomic_intptr_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_base_atomics extension to be enabled}}
-// expected-error-re@-28 {{use of type 'atomic_intptr_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_extended_atomics extension to be enabled}}
-// expected-error-re@-28 {{use of type 'atomic_uintptr_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_base_atomics extension to be enabled}}
-// expected-error-re@-29 {{use of type 'atomic_uintptr_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_extended_atomics extension to be enabled}}
-// expected-error-re@-29 {{use of type 'atomic_size_t' 

Re: [PATCH] D24349: [clang-tidy] Extend readability-container-size-empty to arbitrary class with size() and empty()

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D24349#537350, @Eugene.Zelenko wrote:

> Probably check should have options to extend list of containers and also to 
> assume all classes with integer type size() const and bool empty() const as 
> containers. It may be not trivial to find out all custom containers and last 
> option will be helpful to assemble such list.


I think this should actually have two options: one is a list of default-checked 
containers (default is the list of STL containers we previously had), and the 
other is a Boolean option for guessing at what might be a container based on 
function signature (default is true). This gives people a way to silence the 
diagnostic while still being useful. I would document what function signatures 
we use as our heuristic, but note that the function signatures we care about 
may change (for instance, we may decide that const-qualification is 
unimportant, or that we want to require begin()/end() functions, etc). I think 
` size() const` and `bool empty() const` are a 
reasonable heuristic for a first pass.



Comment at: clang-tidy/readability/ContainerSizeEmptyCheck.cpp:33
@@ +32,3 @@
+  const auto validContainer = namedDecl(
+  has(functionDecl(
+  isPublic(), hasName("size"), returns(isInteger()),

omtcyfz wrote:
> Thank you!
> 
> Blacklisted these types.
> 
> Actually, I believe if someone has `bool size()` in their codebase there's 
> still a problem...
> Blacklisted these types.

I'm still not certain this is correct. For instance, if someone wants to do 
`uint8_t size() const;`, we shouldn't punish them because `uint8_t` happens to 
be `unsigned char` for their platform. But you are correct that we don't want 
this to work with char32_t, for instance.

I think the best approach is to make a local matcher for the type predicate. We 
want isInteger() unless it's char (that's not explicitly qualified with signed 
or unsigned), bool, wchar_t, char16_t, char32_t, or an enumeration type.

> Actually, I believe if someone has bool size() in their codebase there's 
> still a problem...

Agreed, though not as part of this check. ;-)


https://reviews.llvm.org/D24349



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

Re: [PATCH] D24339: [clang-tidy] Add check 'readability-redundant-member-init'

[Malcolm Parsons via cfe-commits]

malcolm.parsons added a comment.

How do I add FixIt hints?
They should be simple removals, but how do I decide whether to remove the 
following comma, preceding comma or preceding colon?



Comment at: clang-tidy/readability/RedundantMemberInitCheck.cpp:33
@@ +32,3 @@
+  const auto *Init = Result.Nodes.getNodeAs("init");
+  const auto *Construct = 
Result.Nodes.getNodeAs("construct");
+  const auto arguments = Construct->arguments();

sbenza wrote:
> These construct expressions might actually have side effects that you would 
> not get if you omit them.
> For example, aggregates will be value initialized. If you remove it you 
> change the behavior.
> 
> The tests should include defaulted vs non-defaulted default constructor, 
> user-defined/user-provided/implicit default constructor, aggregates with and 
> without trivially constructible members, etc.
Yes, more tests needed.


Comment at: clang-tidy/readability/RedundantMemberInitCheck.cpp:39
@@ +38,3 @@
+
+  if (std::find_if(begin(arguments), end(arguments), [](const Expr *expr) {
+return !expr->isDefaultArgument();

sbenza wrote:
> You want `std::none_of` instead of `std::find_if`.
> (or use `std::any_of` and don't negate the expression in the lambda)
Maybe I only need to check the first argument - if that's defaulted then they 
all are.


Comment at: docs/clang-tidy/checks/readability-redundant-member-init.rst:6
@@ +5,3 @@
+
+Finds unnecessary member initializations.  
 
+   
 

sbenza wrote:
> Explain when they are unnecessary.
"Finds member initializations that are unnecessary because the same default 
constructor would be called if they were not present"


Repository:
  rL LLVM

https://reviews.llvm.org/D24339



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

Re: [PATCH] D24048: [Driver] [Darwin] Add sanitizer libraries even if -nodefaultlibs is passed

[Filipe Cabecinhas via cfe-commits]

It seems some people on this thread (I'm sorry if everyone is taking this
into account, but it seemed to me that this was going unnoticed, I want to
make sure everyone is on the same page) is only talking about linking (or
not) the sanitizer runtimes. But that's not the only thing.

Depending on the OS, -fsanitize will also link in the sanitizer's
dependencies, which might include libdl, libc++, and others. That was the
reason why the patch was reverted last time we tried to change this flag's
behavior: a sanitized libc++ needs the sanitizer libs, but not libc++...
And it might need other dependencies for the sanitizers.

Maybe have something like:
-fsanitize=blah links all libs
-fsanitize=blah -nodefaultlibs links nothing
And then have two flags for explicitly linking either the sanitizer flag or
the dependencies? Libc++ would still have the problem of wanting some
dependencies but not all, though :-(

Thank you,

 Filipe

On Thursday, 8 September 2016, Anna Zaks  wrote:

> zaks.anna added a comment.
>
> > I don't see the point of adding another flag to control this when we
> already have a perfectly good set of
>
> >  flags that already do the right thing -- that takes us three levels
> deep in flags overriding the behavior of
>
> >  other flags, and I don't see how it actually helps our users in any way.
>
>
> Going with silently linking the sanitizer runtimes when -nostdlib is
> passed will cause regressions in user experience. They will start getting
> inexplicable run time failures instead of build failures. Which solution do
> you propose to fix this problem?
>
> This specific situation comes up often on internal mailing lists, so we
> should not go for a solution that regresses the behavior on Darwin.
>
>
> https://reviews.llvm.org/D24048
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24339: [clang-tidy] Add check 'readability-redundant-member-init'

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


Comment at: clang-tidy/readability/RedundantMemberInitCheck.cpp:34
@@ +33,3 @@
+  const auto *Construct = 
Result.Nodes.getNodeAs("construct");
+  const auto arguments = Construct->arguments();
+

sbenza wrote:
> Prazek wrote:
> > Arguments (upper case)
> Arguments variable name. (should start with upper case)
Please do not use `auto` here, since the type is not spelled out in the 
initializer.


Comment at: clang-tidy/readability/RedundantMemberInitCheck.h:19
@@ +18,3 @@
+
+/// Find unnecessary member initializers
+///

What makes one unnecessary? Also, missing a full stop at the end of the 
sentence.


Repository:
  rL LLVM

https://reviews.llvm.org/D24339



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

Fixing some byte compilation warnings in utils/clang-completion-mode.el

[Philipp Stephani via cfe-commits]

Hi,

the following patch fixes a couple of byte compilation warnings in the
Emacs library utils/clang-completion-mode.el.

Thanks,
Philipp
-- 

Google Germany GmbH
Erika-Mann-Straße 33
80636 München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle

Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind,
leiten Sie diese bitte nicht weiter, informieren Sie den Absender und
löschen Sie die E-Mail und alle Anhänge. Vielen Dank.

This e-mail is confidential. If you are not the right addressee please do
not forward it, please inform the sender, and please erase this e-mail
including any attachments. Thanks.
Index: utils/clang-completion-mode.el
===
--- utils/clang-completion-mode.el	(revision 280970)
+++ utils/clang-completion-mode.el	(working copy)
@@ -64,15 +64,15 @@
   :group 'clang-completion-mode)
 
 ;;; The prefix header to use with Clang code completion. 
-(setq clang-completion-prefix-header "")
+(defvar clang-completion-prefix-header "")
 
 ;;; The substring we will use to filter completion results
-(setq clang-completion-substring "")
+(defvar clang-completion-substring "")
 
 ;;; The current completion buffer
-(setq clang-completion-buffer nil)
+(defvar clang-completion-buffer nil)
 
-(setq clang-result-string "")
+(defvar clang-result-string "")
 
 ;;; Compute the current line in the buffer	
 (defun current-line ()
@@ -199,7 +199,7 @@
 ;; for the currently-active code completion.
 (defun clang-backspace ()
   (interactive)
-  (delete-backward-char 1)
+  (delete-char -1)
   (clang-update-filter))
 
 ;; Invoked when the user types the delete key to update the filter
@@ -206,7 +206,7 @@
 ;; for the currently-active code completion.
 (defun clang-delete ()
   (interactive)
-  (delete-backward-char 1)
+  (delete-char -1)
   (clang-update-filter))
 
 ;; Set up the keymap for the Clang minor mode.
@@ -246,4 +246,3 @@
   nil
   " Clang"
   clang-completion-mode-map)
-
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Fix for https://llvm.org/bugs/show_bug.cgi?id=29073

[Philipp Stephani via cfe-commits]

Hi,

the attached patch fixes https://llvm.org/bugs/show_bug.cgi?id=29073.

Thanks,
Philipp
-- 

Google Germany GmbH
Erika-Mann-Straße 33
80636 München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle

Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind,
leiten Sie diese bitte nicht weiter, informieren Sie den Absender und
löschen Sie die E-Mail und alle Anhänge. Vielen Dank.

This e-mail is confidential. If you are not the right addressee please do
not forward it, please inform the sender, and please erase this e-mail
including any attachments. Thanks.
Index: tools/clang-format/clang-format.el
===
--- tools/clang-format/clang-format.el	(revision 280970)
+++ tools/clang-format/clang-format.el	(working copy)
@@ -122,7 +122,7 @@
 (let (status stderr operations)
   (setq status
 (call-process-region
- (point-min) (point-max) clang-format-executable
+ nil nil clang-format-executable
  nil `(,temp-buffer ,temp-file) nil
 
  "-output-replacements-xml"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21698: [OpenCL] Allow disabling types and declarations associated with extensions

[Yaxun Liu via cfe-commits]

yaxunl marked 8 inline comments as done.
yaxunl added a comment.

F2413912: ___10_Fiji.i 

F2413913: callgrind.out.20440 

Uploaded the kernel used for profiling and the calgrind data. This kernel is 
dumped from the open source ray tracer Blender Cycles. It is a large kernel 
with lots of declarations.



Comment at: include/clang/Basic/DiagnosticParseKinds.td:957
@@ +956,3 @@
+  "expected %select{'enable', 'disable', 'begin' or 'end'|'disable'}0 - 
ignoring">, InGroup;
+def warn_pragma_begin_end_mismatch : Warning<
+  "OpenCL extension end directive mismatches begin directive - ignoring">, 
InGroup;

Anastasia wrote:
> Could you please add a test for this diagnostic too?
added


Comment at: include/clang/Basic/DiagnosticSemaKinds.td:7998
@@ -7995,1 +7997,3 @@
   InGroup;
+ def err_decl_requires_extension : Error<
+  "use of declaration requires %0 extension to be enabled">;

Anastasia wrote:
> Could we unify with err_type_requires_extension?
these two error msgs have different format.

  def err_type_requires_extension : Error<
   "use of type %0 requires %1 extension to be enabled">;

err_type_requires_extension has an extra type name argument.


Comment at: lib/Headers/opencl-c.h:15484
@@ +15483,3 @@
+#ifdef cl_khr_gl_msaa_sharing
+#pragma OPENCL EXTENSION cl_khr_gl_msaa_sharing : enable
+#endif //cl_khr_gl_msaa_sharing

Anastasia wrote:
> Does this change belong here?
With this feature, use of image types associated with an extension requires 
enabling the extension. That's why this change.


Comment at: lib/Parse/ParsePragma.cpp:461
@@ -459,1 +460,3 @@
+  };
+  typedef std::pair OpenCLExtData;
 }

Anastasia wrote:
> Could we use PointerIntPair still but with 2 bits?
Using PointerIntPair with 2 bits requires IdentifierInfo to be aligned at 4 
bytes, however this is not true. IdentifierInfo has no explicit alignment 
specifier, therefore it can only hold 1 bit in PointerIntPair.


Comment at: lib/Sema/Sema.cpp:1558
@@ +1557,3 @@
+  if (Exts.empty())
+return;
+  auto CanT = T.getCanonicalType().getTypePtr();

Anastasia wrote:
> Would this not be an error?
This function can be called grammatically to set extensions for a group of 
image types, e.g., the extensions associated with image types. It is more 
convenient to allow empty string here. If empty string is not allowed, it can 
be diagnosed before calling this function.


Comment at: lib/Sema/SemaDecl.cpp:4797
@@ +4796,3 @@
+  if (getLangOpts().OpenCL)
+setCurrentOpenCLExtensionForDecl(Dcl);
+

Anastasia wrote:
> I am a  bit concerned about the compilation speed since we will be performing 
> expensive container lookups for each declaration here.
> 
> Have you done any profiling in this direction?
Profiling result shows cost of checking disabled types and declarations can be 
ignored for typical OpenCL kernels.


Comment at: lib/Serialization/ASTReader.cpp:6937
@@ -6936,3 +6942,1 @@
 
-  // FIXME: What happens if these are changed by a module import?
-  if (!OpenCLExtensions.empty()) {

Anastasia wrote:
> Is the comment no longer relevant?
This change will initialize Sema with OpenCLExtensions imported from 
module/pch, so the original comment no longer applies.


https://reviews.llvm.org/D21698



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

Re: [PATCH] D24339: [clang-tidy] Add check 'readability-redundant-member-init'

[Samuel Benzaquen via cfe-commits]

sbenza added inline comments.


Comment at: clang-tidy/readability/RedundantMemberInitCheck.cpp:33
@@ +32,3 @@
+  const auto *Init = Result.Nodes.getNodeAs("init");
+  const auto *Construct = 
Result.Nodes.getNodeAs("construct");
+  const auto arguments = Construct->arguments();

These construct expressions might actually have side effects that you would not 
get if you omit them.
For example, aggregates will be value initialized. If you remove it you change 
the behavior.

The tests should include defaulted vs non-defaulted default constructor, 
user-defined/user-provided/implicit default constructor, aggregates with and 
without trivially constructible members, etc.


Comment at: clang-tidy/readability/RedundantMemberInitCheck.cpp:34
@@ +33,3 @@
+  const auto *Construct = 
Result.Nodes.getNodeAs("construct");
+  const auto arguments = Construct->arguments();
+

Prazek wrote:
> Arguments (upper case)
Arguments variable name. (should start with upper case)


Comment at: clang-tidy/readability/RedundantMemberInitCheck.cpp:36
@@ +35,3 @@
+
+  using std::begin;
+  using std::end;

There's no need for these using declarations.
Just do arguments.begin()/arguments.end().
They are using in generic code. This is not generic code.


Comment at: clang-tidy/readability/RedundantMemberInitCheck.cpp:39
@@ +38,3 @@
+
+  if (std::find_if(begin(arguments), end(arguments), [](const Expr *expr) {
+return !expr->isDefaultArgument();

You want `std::none_of` instead of `std::find_if`.
(or use `std::any_of` and don't negate the expression in the lambda)


Comment at: docs/clang-tidy/checks/readability-redundant-member-init.rst:6
@@ +5,3 @@
+
+Finds unnecessary member initializations.  
 
+   
 

Explain when they are unnecessary.


Repository:
  rL LLVM

https://reviews.llvm.org/D24339



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

Re: [PATCH] D22057: Prevent devirtualization of calls to un-instantiated functions.

[Arthur O'Dwyer via cfe-commits]

Quuxplusone added inline comments.


Comment at: test/CodeGen/no-devirt.cpp:16
@@ +15,3 @@
+   if (a > 6) return data[a] ;  // Should not devirtualize
+   if (a > 4) return data.func1(a); // Should devirtualize
+   return data.func2(a);// Should devirtualize

Sunil_Srivastava wrote:
> Quuxplusone wrote:
> > This is a really dumb question from the peanut gallery, but, could you 
> > explain why these two cases (lines 15 and 16) should differ?  It really 
> > seems like both calls should be able to be devirtualized, because the 
> > compiler statically knows what they should call.
> > 
> > I think you mention the difference between lines 15 and 16 here:
> > 
> > > except, for some reason, when it is an operator and used with an operator 
> > > syntax
> > 
> > but you don't explain *why* the difference is desirable. Shouldn't we just 
> > fix that difference, then?
> > 
> > Is your first fix (
> > 
> > > The first fix will be in the front end to force the instantiation on 
> > > virtual calls that are potentially devirtualizable.
> > 
> > ) basically "fix the difference between lines 15 and 16", or is it talking 
> > about something else entirely?
> AFAICS, The two cases (line 15 and 16) should not differ.
> 
> The first fix will "fix the difference between line 15 and 16". I have the 
> change for that ready, but once we do that, there will be no way (known to 
> me) of testing the second "fix". Hence the second "fix" is being proposed for 
> commit before the first.
Okay, makes sense to me now. (As long as the comment on line 15 gets updated in 
the "first fix". I believe you'll be touching this test case again anyway in 
the "first fix", because the expected output will change.)
Question answered. :)


https://reviews.llvm.org/D22057



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

Re: [PATCH] D24339: [clang-tidy] Add check 'readability-redundant-member-init'

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko added inline comments.


Comment at: clang-tidy/readability/RedundantMemberInitCheck.cpp:36
@@ +35,3 @@
+
+  using std::begin;
+  using std::end;

begin() and end() are not used extensively. Why not to use std::?


Repository:
  rL LLVM

https://reviews.llvm.org/D24339



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

Re: [PATCH] D24339: [clang-tidy] Add check 'readability-redundant-member-init'

[Piotr Padlewski via cfe-commits]

Prazek added inline comments.


Comment at: clang-tidy/readability/RedundantMemberInitCheck.cpp:34
@@ +33,3 @@
+  const auto *Construct = 
Result.Nodes.getNodeAs("construct");
+  const auto arguments = Construct->arguments();
+

Arguments (upper case)


Repository:
  rL LLVM

https://reviews.llvm.org/D24339



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

Re: [PATCH] D24311: Implement MS _rot intrinsics

[David Majnemer via cfe-commits]

majnemer added inline comments.


Comment at: lib/AST/ExprConstant.cpp:7024-7050
@@ -7023,1 +7023,29 @@
 
+  case Builtin::BI_rotl8:
+  case Builtin::BI_rotl16:
+  case Builtin::BI_rotl:
+  case Builtin::BI_lrotl:
+  case Builtin::BI_rotl64: {
+APSInt Val, Shift;
+if (!EvaluateInteger(E->getArg(0), Val, Info) ||
+!EvaluateInteger(E->getArg(1), Shift, Info))
+  return false;
+
+APSInt BitWidth(llvm::APInt(32, Val.getBitWidth()));
+return Success(Val.rotl(Shift % BitWidth), E);
+  }
+
+  case Builtin::BI_rotr8:
+  case Builtin::BI_rotr16:
+  case Builtin::BI_rotr:
+  case Builtin::BI_lrotr:
+  case Builtin::BI_rotr64: {
+APSInt Val, Shift;
+if (!EvaluateInteger(E->getArg(0), Val, Info) ||
+!EvaluateInteger(E->getArg(1), Shift, Info))
+  return false;
+
+APSInt BitWidth(llvm::APInt(32, Val.getBitWidth()));
+return Success(Val.rotr(Shift % BitWidth), E);
+  }
+

agutowski wrote:
> majnemer wrote:
> > Any reason why we need this?
> > 
> > Given:
> >   #include 
> > 
> >   constexpr int x = _rotl8(1, 2);
> > 
> > MSVC 2015 reports:
> >   error C2131: expression did not evaluate to a constant
> Hm, I don't know. Is there any reason why we shouldn't do this? I mean, I 
> just had the feeling that if we can evaluate something during compilation 
> time, we should to it.
The best reason I can think of is that it allows people to use a 
Microsoft-specific intrinsic in ways which won't compile with Microsoft's 
compiler.


https://reviews.llvm.org/D24311



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

Re: [PATCH] D24311: Implement MS _rot intrinsics

[Albert Gutowski via cfe-commits]

agutowski added inline comments.


Comment at: lib/AST/ExprConstant.cpp:7024-7050
@@ -7023,1 +7023,29 @@
 
+  case Builtin::BI_rotl8:
+  case Builtin::BI_rotl16:
+  case Builtin::BI_rotl:
+  case Builtin::BI_lrotl:
+  case Builtin::BI_rotl64: {
+APSInt Val, Shift;
+if (!EvaluateInteger(E->getArg(0), Val, Info) ||
+!EvaluateInteger(E->getArg(1), Shift, Info))
+  return false;
+
+APSInt BitWidth(llvm::APInt(32, Val.getBitWidth()));
+return Success(Val.rotl(Shift % BitWidth), E);
+  }
+
+  case Builtin::BI_rotr8:
+  case Builtin::BI_rotr16:
+  case Builtin::BI_rotr:
+  case Builtin::BI_lrotr:
+  case Builtin::BI_rotr64: {
+APSInt Val, Shift;
+if (!EvaluateInteger(E->getArg(0), Val, Info) ||
+!EvaluateInteger(E->getArg(1), Shift, Info))
+  return false;
+
+APSInt BitWidth(llvm::APInt(32, Val.getBitWidth()));
+return Success(Val.rotr(Shift % BitWidth), E);
+  }
+

majnemer wrote:
> Any reason why we need this?
> 
> Given:
>   #include 
> 
>   constexpr int x = _rotl8(1, 2);
> 
> MSVC 2015 reports:
>   error C2131: expression did not evaluate to a constant
Hm, I don't know. Is there any reason why we shouldn't do this? I mean, I just 
had the feeling that if we can evaluate something during compilation time, we 
should to it.


https://reviews.llvm.org/D24311



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

Re: [PATCH] D21698: [OpenCL] Allow disabling types and declarations associated with extensions

[Anastasia Stulova via cfe-commits]

Anastasia added a comment.

In https://reviews.llvm.org/D21698#537156, @yaxunl wrote:

> I did profiling with valgrind for the cost of checking disabled types and 
> declarations. For a typical program, time spent in checking disabled types 
> and declarations are less than 0.1%, so this cost can be ignored.


Could you please provide the profiled code example here please? As far as I 
understand this lookup will run for all the declarations and function calls. So 
we should make sure to profile various programs to see what the impact is with 
different use cases.


https://reviews.llvm.org/D21698



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

Re: [PATCH] D24048: [Driver] [Darwin] Add sanitizer libraries even if -nodefaultlibs is passed

[Anna Zaks via cfe-commits]

zaks.anna added a comment.

> I don't see the point of adding another flag to control this when we already 
> have a perfectly good set of 

>  flags that already do the right thing -- that takes us three levels deep in 
> flags overriding the behavior of 

>  other flags, and I don't see how it actually helps our users in any way.


Going with silently linking the sanitizer runtimes when -nostdlib is passed 
will cause regressions in user experience. They will start getting inexplicable 
run time failures instead of build failures. Which solution do you propose to 
fix this problem?

This specific situation comes up often on internal mailing lists, so we should 
not go for a solution that regresses the behavior on Darwin.


https://reviews.llvm.org/D24048



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

Re: [PATCH] D24330: Add some MS aliases for existing intrinsics

[Richard Smith via cfe-commits]

rsmith added inline comments.


Comment at: include/clang/Basic/BuiltinsX86.def:304
@@ -303,2 +303,3 @@
 TARGET_BUILTIN(__builtin_ia32_ldmxcsr, "vUi", "", "sse")
+TARGET_BUILTIN(_mm_setcsr, "vUi", "", "sse")
 TARGET_BUILTIN(__builtin_ia32_stmxcsr, "Ui", "", "sse")

majnemer wrote:
> rnk wrote:
> > Part of the idea behind the Intel *mmintrin.h headers is that they define 
> > symbols outside of the implementor's namespace. Users should be able to 
> > write code that uses these _mm_* names if they don't include those headers. 
> > Having this builtin always be available on x86 makes that impossible.
> > 
> > That said, I can see that winnt.h uses these directly, rather than 
> > including xmmintrin.h, so we need them to be builtins in MSVC mode, and 
> > it's annoying to have them sometimes be builtins and sometimes be functions.
> We could have the header file use a pragma which turns the _mm intrinsics on 
> along the same lines as `#pragma intrinsic`.  For MSVC compat, we could treat 
> them as-if they were enabled by some similar mechanism.
Names starting with an underscore are reserved to the implementation for use in 
the global namespace. Also, we declare these lazily, on-demand, only if name 
lookup would otherwise fail, so (outside of odd SFINAE cases in C++) this 
shouldn't affect the meaning of any valid programs. While I find it distasteful 
to model these as builtins, the only significant problem I see here is that 
we'll accept non-portable code that forgets to include the right header if we 
do so.

Perhaps we could model this somewhat similarly to `LIBBUILTIN`, so that we 
allow use of the builtin without a declaration, but produce a diagnostic about 
the missing `#include` when we do so?


https://reviews.llvm.org/D24330



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

Re: [clang-tools-extra] r280839 - Resubmit "Add a test for clang-tidy using the clang-cl driver."

[Ahmed Bougacha via cfe-commits]

On Thu, Sep 8, 2016 at 9:52 AM, Zachary Turner  wrote:
> I don't have access to an OSX machine to test on, but if you do, is there
> any way you could test this command line manually using a path that starts
> with /U and see if you can reproduce it?

(Did you mean to include a command line?)
I tried both the test and a trivial clang-check command line with /U
and non-/U paths (with -###;  I couldn't find a clang-cl equivalent to '-v'):

  clang-check "/Volumes/.../clang-cl-driver.cpp" -- --driver-mode=cl -###
  clang-check "/Users/.../clang-cl-driver.cpp" -- --driver-mode=cl -###

and I do get an error for the /U path:

  warning: argument unused during compilation: '-U sers/.../clang-cl-driver.cpp'
  error: unable to handle compilation, expected exactly one compiler job in ''
  Error while processing /Users/.../clang-cl-driver.cpp.

In any case, I reverted the test in r280975, let's continue this
discussion in PR30328.  Let me know if you have any ideas;  I'll
experiment locally later.

-Ahmed

> On Thu, Sep 8, 2016 at 9:45 AM Ahmed Bougacha 
> wrote:
>>
>> On Thu, Sep 8, 2016 at 9:38 AM, Zachary Turner  wrote:
>> > What happens if you put the %s in quotes?
>>
>> Same error
>>
>> -Ahmed
>>
>> >
>> > On Thu, Sep 8, 2016 at 9:32 AM Ahmed Bougacha 
>> > wrote:
>> >>
>> >> Hi Zachary,
>> >>
>> >> This test has been failing on some OS X machines, e.g.:
>> >>
>> >>
>> >> http://lab.llvm.org:8080/green/job/clang-stage1-configure-RA_check/21470/consoleFull#19036563798254eaf0-7326-4999-85b0-388101f2d404
>> >>
>> >> My hypothesis is that the %s path ('/Users/...')  is somehow
>> >> interpreted as a '/U' by the clang-cl driver, causing the cryptic
>> >> 'Error while processing /Users/.../clang-cl-driver.cpp' error  (as the
>> >> test succeeds when run from other paths: /tmp/, /Volumes/).
>> >> Does that make sense?  Any suggestions on how to fix?
>> >>
>> >> Thanks!
>> >> -Ahmed
>> >>
>> >>
>> >> On Wed, Sep 7, 2016 at 11:28 AM, Zachary Turner via cfe-commits
>> >>  wrote:
>> >> > Author: zturner
>> >> > Date: Wed Sep  7 13:28:42 2016
>> >> > New Revision: 280839
>> >> >
>> >> > URL: http://llvm.org/viewvc/llvm-project?rev=280839=rev
>> >> > Log:
>> >> > Resubmit "Add a test for clang-tidy using the clang-cl driver."
>> >> >
>> >> > This was originally reverted because the patch on the clang
>> >> > tooling side was reverted.  That patch is being resubmitted,
>> >> > so this patch is resubmitted as well.
>> >> >
>> >> > Added:
>> >> > clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp
>> >> >
>> >> > Added: clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp
>> >> > URL:
>> >> >
>> >> > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp?rev=280839=auto
>> >> >
>> >> >
>> >> > ==
>> >> > --- clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp
>> >> > (added)
>> >> > +++ clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp Wed
>> >> > Sep
>> >> > 7 13:28:42 2016
>> >> > @@ -0,0 +1,17 @@
>> >> > +// RUN: clang-tidy -checks=-*,modernize-use-nullptr %s --
>> >> > --driver-mode=cl /DTEST1 /DFOO=foo /DBAR=bar | FileCheck
>> >> > -implicit-check-not="{{warning|error}}:" %s
>> >> > +int *a = 0;
>> >> > +// CHECK: :[[@LINE-1]]:10: warning: use nullptr
>> >> > +#ifdef TEST1
>> >> > +int *b = 0;
>> >> > +// CHECK: :[[@LINE-1]]:10: warning: use nullptr
>> >> > +#endif
>> >> > +#define foo 1
>> >> > +#define bar 1
>> >> > +#if FOO
>> >> > +int *c = 0;
>> >> > +// CHECK: :[[@LINE-1]]:10: warning: use nullptr
>> >> > +#endif
>> >> > +#if BAR
>> >> > +int *d = 0;
>> >> > +// CHECK: :[[@LINE-1]]:10: warning: use nullptr
>> >> > +#endif
>> >> >
>> >> >
>> >> > ___
>> >> > cfe-commits mailing list
>> >> > cfe-commits@lists.llvm.org
>> >> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r280975 - Revert "Resubmit "Add a test for clang-tidy using the clang-cl driver.""

[Ahmed Bougacha via cfe-commits]

Author: ab
Date: Thu Sep  8 13:02:14 2016
New Revision: 280975

URL: http://llvm.org/viewvc/llvm-project?rev=280975=rev
Log:
Revert "Resubmit "Add a test for clang-tidy using the clang-cl driver.""

This reverts commit r280839.
It's problematic on OS X, where the '/Users/...' paths are interpreted
as '/U' options.

Investigation ongoing in http://llvm.org/PR30328.

Removed:
clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp

Removed: clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp?rev=280974=auto
==
--- clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp (removed)
@@ -1,17 +0,0 @@
-// RUN: clang-tidy -checks=-*,modernize-use-nullptr %s -- --driver-mode=cl 
/DTEST1 /DFOO=foo /DBAR=bar | FileCheck 
-implicit-check-not="{{warning|error}}:" %s
-int *a = 0;
-// CHECK: :[[@LINE-1]]:10: warning: use nullptr
-#ifdef TEST1
-int *b = 0;
-// CHECK: :[[@LINE-1]]:10: warning: use nullptr
-#endif
-#define foo 1
-#define bar 1
-#if FOO
-int *c = 0;
-// CHECK: :[[@LINE-1]]:10: warning: use nullptr
-#endif
-#if BAR
-int *d = 0;
-// CHECK: :[[@LINE-1]]:10: warning: use nullptr
-#endif


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

Re: [PATCH] D24048: [Driver] [Darwin] Add sanitizer libraries even if -nodefaultlibs is passed

[Richard Smith via cfe-commits]

rsmith added a comment.

In https://reviews.llvm.org/D24048#537257, @beanz wrote:

> @kubabrecka, I can understand where you're coming from about the option 
> starting with `-fsanitize`, but I disagree for two reasons. First, I think 
> that it is more important for the option to be concise and clear about what 
> it means, and `-flink-sanitizer-runtimes` seems pretty clear to me. Second, 
> the existing `-fsanitize` options all refer to the compiler inserted runtime 
> checks, where as this refers to the driver behavior, so I think there is a 
> difference between the intention of the flags that justifies them not 
> conforming to the same format.


Your claim about the existing `-fsanitize` options seems off the mark -- at 
least `-fsanitize=` and `fsanitize-link-c++-runtime` already affect the driver 
linking behaviour, not just the compiler-inserted runtime checks. And more 
generally, we're trying to group our driver options based on prefixes, so 
`-flink-sanitizer-runtimes` is not appropriate on that basis.

> I also have a small problem with @rsmith's point about `-fsanitize` being an 
> explicit request when linking. I get very nervous about the idea of a flag 
> having different meaning when compiling from when linking. While I understand 
> that it doesn't make sense for `-fsanitize` options to not imply linking the 
> runtimes, I think there is a good argument that they're not an explicit 
> request for the runtime libraries.


I think it depends a lot on how you set up your build system. If you have one 
set of flags for compilation steps and one set of flags for link steps, then 
adding `-fsanitize=` to the link flags is pretty clearly requesting that you 
link against the sanitizer runtimes. This is, in fact, the documented meaning 
of passing `-fsanitize=` when linking 
(http://clang.llvm.org/docs/UsersManual.html#controlling-code-generation).

If, on the other hand, you have a common set of flags passed to both 
compilations and links, then perhaps you could argue that you're not explicitly 
opting in. However, as there are a large number of Clang compilation flags 
(`-f` flags in particular) that will cause the link to fail if they're passed 
to the driver when linking, someone will have needed to choose to add the flags 
to the "both compile and link" flag set to get into this state (rather than 
adding them just to the "compile" flag set, which should be the default 
choice), and again this seems like an explicit statement of intent.

I don't see the point of adding another flag to control this when we already 
have a perfectly good set of flags that already do the right thing -- that 
takes us three levels deep in flags overriding the behavior of other flags, and 
I don't see how it actually helps our users in any way.


https://reviews.llvm.org/D24048



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

Re: [PATCH] D24339: [clang-tidy] Add check 'readability-redundant-member-init'

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko added a comment.

I think will be good idea to add cases when member is initialized in 
declaration and constructor, with same and different values.



Comment at: docs/clang-tidy/checks/readability-redundant-member-init.rst:4
@@ +3,3 @@
+readability-redundant-member-init
+==
+

This line length should be same as header above.


Comment at: docs/clang-tidy/checks/readability-redundant-member-init.rst:13
@@ +12,3 @@
+  // Explicitly initializing the member s is unnecessary.  

+  class Foo {
+public:

Please run Clang-format over example. Will be good idea to add empty line 
before private.


Comment at: test/clang-tidy/readability-redundant-member-init.cpp:33
@@ +32,3 @@
+
+
+struct NF1 {

Please remove extra empty-line.


Repository:
  rL LLVM

https://reviews.llvm.org/D24339



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

Re: [PATCH] D24349: [clang-tidy] Extend readability-container-size-empty to arbitrary class with size() and empty()

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko added a comment.

Probably check should have options to extend list of containers and also to 
assume all classes with integer type size() const and bool empty() const as 
containers. It may be not trivial to find out all custom containers and last 
option will be helpful to assemble such list.


https://reviews.llvm.org/D24349



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

Re: [PATCH] D24330: Add some MS aliases for existing intrinsics

[David Majnemer via cfe-commits]

majnemer added inline comments.


Comment at: include/clang/Basic/BuiltinsX86.def:304
@@ -303,2 +303,3 @@
 TARGET_BUILTIN(__builtin_ia32_ldmxcsr, "vUi", "", "sse")
+TARGET_BUILTIN(_mm_setcsr, "vUi", "", "sse")
 TARGET_BUILTIN(__builtin_ia32_stmxcsr, "Ui", "", "sse")

rnk wrote:
> Part of the idea behind the Intel *mmintrin.h headers is that they define 
> symbols outside of the implementor's namespace. Users should be able to write 
> code that uses these _mm_* names if they don't include those headers. Having 
> this builtin always be available on x86 makes that impossible.
> 
> That said, I can see that winnt.h uses these directly, rather than including 
> xmmintrin.h, so we need them to be builtins in MSVC mode, and it's annoying 
> to have them sometimes be builtins and sometimes be functions.
We could have the header file use a pragma which turns the _mm intrinsics on 
along the same lines as `#pragma intrinsic`.  For MSVC compat, we could treat 
them as-if they were enabled by some similar mechanism.


https://reviews.llvm.org/D24330



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

Re: [PATCH] D24330: Add some MS aliases for existing intrinsics

[Reid Kleckner via cfe-commits]

rnk added a comment.

+Richard for ideas on how to navigate the intel intrinsics



Comment at: include/clang/Basic/BuiltinsX86.def:304
@@ -303,2 +303,3 @@
 TARGET_BUILTIN(__builtin_ia32_ldmxcsr, "vUi", "", "sse")
+TARGET_BUILTIN(_mm_setcsr, "vUi", "", "sse")
 TARGET_BUILTIN(__builtin_ia32_stmxcsr, "Ui", "", "sse")

Part of the idea behind the Intel *mmintrin.h headers is that they define 
symbols outside of the implementor's namespace. Users should be able to write 
code that uses these _mm_* names if they don't include those headers. Having 
this builtin always be available on x86 makes that impossible.

That said, I can see that winnt.h uses these directly, rather than including 
xmmintrin.h, so we need them to be builtins in MSVC mode, and it's annoying to 
have them sometimes be builtins and sometimes be functions.


https://reviews.llvm.org/D24330



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

r280968 - Revert "[XRay] ARM 32-bit no-Thumb support in Clang"

[Renato Golin via cfe-commits]

Author: rengolin
Date: Thu Sep  8 12:12:32 2016
New Revision: 280968

URL: http://llvm.org/viewvc/llvm-project?rev=280968=rev
Log:
Revert "[XRay] ARM 32-bit no-Thumb support in Clang"

This reverts commit r280889, as the original LLVM commits broke the thumb
buildbots.

Removed:
cfe/trunk/test/CodeGen/xray-attributes-supported-arm.cpp

Removed: cfe/trunk/test/CodeGen/xray-attributes-supported-arm.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/xray-attributes-supported-arm.cpp?rev=280967=auto
==
--- cfe/trunk/test/CodeGen/xray-attributes-supported-arm.cpp (original)
+++ cfe/trunk/test/CodeGen/xray-attributes-supported-arm.cpp (removed)
@@ -1,13 +0,0 @@
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - 
-triple arm-unknown-linux-gnu | FileCheck %s
-
-// Make sure that the LLVM attribute for XRay-annotated functions do show up.
-[[clang::xray_always_instrument]] void foo() {
-// CHECK: define void @_Z3foov() #0
-};
-
-[[clang::xray_never_instrument]] void bar() {
-// CHECK: define void @_Z3barv() #1
-};
-
-// CHECK: #0 = {{.*}}"function-instrument"="xray-always"
-// CHECK: #1 = {{.*}}"function-instrument"="xray-never"


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

Re: [PATCH] D24349: [clang-tidy] Extend readability-container-size-empty to arbitrary class with size() and empty()

[Kirill Bobyrev via cfe-commits]

omtcyfz marked an inline comment as done.


Comment at: clang-tidy/readability/ContainerSizeEmptyCheck.cpp:33
@@ +32,3 @@
+  const auto validContainer = namedDecl(
+  has(functionDecl(
+  isPublic(), hasName("size"), returns(isInteger()),

Thank you!

Blacklisted these types.

Actually, I believe if someone has `bool size()` in their codebase there's 
still a problem...


https://reviews.llvm.org/D24349



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

Re: [PATCH] D24349: [clang-tidy] Extend readability-container-size-empty to arbitrary class with size() and empty()

[Kirill Bobyrev via cfe-commits]

omtcyfz updated this revision to Diff 70717.
omtcyfz added a comment.

Blacklist `enum` and `bool` return types for `size()`.


https://reviews.llvm.org/D24349

Files:
  clang-tidy/readability/ContainerSizeEmptyCheck.cpp
  test/clang-tidy/readability-container-size-empty.cpp

Index: test/clang-tidy/readability-container-size-empty.cpp
===
--- test/clang-tidy/readability-container-size-empty.cpp
+++ test/clang-tidy/readability-container-size-empty.cpp
@@ -24,9 +24,34 @@
 };
 }
 
-
 }
 
+template 
+class TemplatedContainer {
+public:
+  int size();
+  bool empty();
+};
+
+template 
+class PrivateEmpty {
+public:
+  int size();
+private:
+  bool empty();
+};
+
+struct BoolSize {
+  bool size();
+  bool empty();
+};
+
+struct EnumSize {
+  enum E { one };
+  enum E size() const;
+  bool empty();
+};
+
 int main() {
   std::set intSet;
   std::string str;
@@ -127,6 +152,110 @@
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
   // CHECK-FIXES: {{^  }}if (vect4.empty()){{$}}
+
+  TemplatedContainer templated_container;
+  if (templated_container.size() == 0)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (templated_container.empty()){{$}}
+  if (templated_container.size() != 0)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+  if (0 == templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (templated_container.empty()){{$}}
+  if (0 != templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+  if (templated_container.size() > 0)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+  if (0 < templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:11: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+  if (templated_container.size() < 1)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (templated_container.empty()){{$}}
+  if (1 > templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:11: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (templated_container.empty()){{$}}
+  if (templated_container.size() >= 1)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+  if (1 <= templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+  if (templated_container.size() > 1) // no warning
+;
+  if (1 < templated_container.size()) // no warning
+;
+  if (templated_container.size() <= 1) // no warning
+;
+  if (1 >= templated_container.size()) // no warning
+;
+  if (!templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (templated_container.empty()){{$}}
+  if (templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+
+  if (templated_container.empty())
+;
+
+  // No warnings expected.
+  PrivateEmpty private_empty;
+  if (private_empty.size() == 0)
+;
+  if (private_empty.size() != 0)
+;
+  if (0 == private_empty.size())
+;
+  if (0 != private_empty.size())
+;
+  if (private_empty.size() > 0)
+;
+  if (0 < private_empty.size())
+;
+  if (private_empty.size() < 1)
+;
+  if (1 > private_empty.size())
+;
+  if (private_empty.size() >= 1)
+;
+  if (1 <= private_empty.size())
+;
+  if (private_empty.size() > 1)
+;
+  if (1 < private_empty.size())
+;
+  if (private_empty.size() <= 1)
+;
+  if (1 >= private_empty.size())
+;
+  if (!private_empty.size())
+;
+  if (private_empty.size())
+;
+
+  // Types with weird size() return type.
+  BoolSize bs;
+  if (bs.size() == 0)
+;
+  EnumSize es;
+  if (es.size() == 0)
+;
 }
 
 #define CHECKSIZE(x) if (x.size())
@@ -142,6 +271,16 @@
   CHECKSIZE(v);
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: the 'empty' method should be used
   // CHECK-MESSAGES: CHECKSIZE(v);
+
+  TemplatedContainer templated_container;
+  if (templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+  // 

Re: [clang-tools-extra] r280839 - Resubmit "Add a test for clang-tidy using the clang-cl driver."

[Zachary Turner via cfe-commits]

I don't have access to an OSX machine to test on, but if you do, is there
any way you could test this command line manually using a path that starts
with /U and see if you can reproduce it?

On Thu, Sep 8, 2016 at 9:45 AM Ahmed Bougacha 
wrote:

> On Thu, Sep 8, 2016 at 9:38 AM, Zachary Turner  wrote:
> > What happens if you put the %s in quotes?
>
> Same error
>
> -Ahmed
>
> >
> > On Thu, Sep 8, 2016 at 9:32 AM Ahmed Bougacha 
> > wrote:
> >>
> >> Hi Zachary,
> >>
> >> This test has been failing on some OS X machines, e.g.:
> >>
> >>
> http://lab.llvm.org:8080/green/job/clang-stage1-configure-RA_check/21470/consoleFull#19036563798254eaf0-7326-4999-85b0-388101f2d404
> >>
> >> My hypothesis is that the %s path ('/Users/...')  is somehow
> >> interpreted as a '/U' by the clang-cl driver, causing the cryptic
> >> 'Error while processing /Users/.../clang-cl-driver.cpp' error  (as the
> >> test succeeds when run from other paths: /tmp/, /Volumes/).
> >> Does that make sense?  Any suggestions on how to fix?
> >>
> >> Thanks!
> >> -Ahmed
> >>
> >>
> >> On Wed, Sep 7, 2016 at 11:28 AM, Zachary Turner via cfe-commits
> >>  wrote:
> >> > Author: zturner
> >> > Date: Wed Sep  7 13:28:42 2016
> >> > New Revision: 280839
> >> >
> >> > URL: http://llvm.org/viewvc/llvm-project?rev=280839=rev
> >> > Log:
> >> > Resubmit "Add a test for clang-tidy using the clang-cl driver."
> >> >
> >> > This was originally reverted because the patch on the clang
> >> > tooling side was reverted.  That patch is being resubmitted,
> >> > so this patch is resubmitted as well.
> >> >
> >> > Added:
> >> > clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp
> >> >
> >> > Added: clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp
> >> > URL:
> >> >
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp?rev=280839=auto
> >> >
> >> >
> ==
> >> > --- clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp
> (added)
> >> > +++ clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp Wed
> Sep
> >> > 7 13:28:42 2016
> >> > @@ -0,0 +1,17 @@
> >> > +// RUN: clang-tidy -checks=-*,modernize-use-nullptr %s --
> >> > --driver-mode=cl /DTEST1 /DFOO=foo /DBAR=bar | FileCheck
> >> > -implicit-check-not="{{warning|error}}:" %s
> >> > +int *a = 0;
> >> > +// CHECK: :[[@LINE-1]]:10: warning: use nullptr
> >> > +#ifdef TEST1
> >> > +int *b = 0;
> >> > +// CHECK: :[[@LINE-1]]:10: warning: use nullptr
> >> > +#endif
> >> > +#define foo 1
> >> > +#define bar 1
> >> > +#if FOO
> >> > +int *c = 0;
> >> > +// CHECK: :[[@LINE-1]]:10: warning: use nullptr
> >> > +#endif
> >> > +#if BAR
> >> > +int *d = 0;
> >> > +// CHECK: :[[@LINE-1]]:10: warning: use nullptr
> >> > +#endif
> >> >
> >> >
> >> > ___
> >> > cfe-commits mailing list
> >> > cfe-commits@lists.llvm.org
> >> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libclc] r280961 - Replace nextafter implementation

[Matt Arsenault via cfe-commits]

Author: arsenm
Date: Thu Sep  8 11:37:56 2016
New Revision: 280961

URL: http://llvm.org/viewvc/llvm-project?rev=280961=rev
Log:
Replace nextafter implementation

This one passes conformance.

Modified:
libclc/trunk/amdgpu/lib/math/nextafter.cl
libclc/trunk/generic/lib/math/clc_nextafter.cl

Modified: libclc/trunk/amdgpu/lib/math/nextafter.cl
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgpu/lib/math/nextafter.cl?rev=280961=280960=280961=diff
==
--- libclc/trunk/amdgpu/lib/math/nextafter.cl (original)
+++ libclc/trunk/amdgpu/lib/math/nextafter.cl Thu Sep  8 11:37:56 2016
@@ -2,3 +2,8 @@
 #include "../lib/clcmacro.h"
 
 _CLC_DEFINE_BINARY_BUILTIN(float, nextafter, __clc_nextafter, float, float)
+
+#ifdef cl_khr_fp64
+#pragma OPENCL EXTENSION cl_khr_fp64 : enable
+_CLC_DEFINE_BINARY_BUILTIN(double, nextafter, __clc_nextafter, double, double)
+#endif

Modified: libclc/trunk/generic/lib/math/clc_nextafter.cl
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/generic/lib/math/clc_nextafter.cl?rev=280961=280960=280961=diff
==
--- libclc/trunk/generic/lib/math/clc_nextafter.cl (original)
+++ libclc/trunk/generic/lib/math/clc_nextafter.cl Thu Sep  8 11:37:56 2016
@@ -1,43 +1,39 @@
 #include 
 #include "../clcmacro.h"
 
-// This file provides OpenCL C implementations of nextafter for targets that
-// don't support the clang builtin.
+// This file provides OpenCL C implementations of nextafter for
+// targets that don't support the clang builtin.
 
-#define FLT_NAN 0.0f/0.0f
+#define AS_TYPE(x) as_##x
 
-#define NEXTAFTER(FLOAT_TYPE, UINT_TYPE, NAN, ZERO, NEXTAFTER_ZERO) \
+#define NEXTAFTER(FLOAT_TYPE, UINT_TYPE, INT_TYPE)  \
 _CLC_OVERLOAD _CLC_DEF FLOAT_TYPE __clc_nextafter(FLOAT_TYPE x, FLOAT_TYPE y) 
{ \
-  union { \
-FLOAT_TYPE f; \
-UINT_TYPE i;  \
-  } next; \
-  if (isnan(x) || isnan(y)) { \
-return NAN;   \
-  }   \
-  if (x == y) {   \
-return y; \
-  }   \
-  next.f = x; \
-  if (x < y) {\
-next.i++; \
-  } else {\
-if (next.f == ZERO) { \
-next.i = NEXTAFTER_ZERO;  \
-} else {  \
-  next.i--;   \
-} \
-  }   \
-  return next.f;  \
+  const UINT_TYPE sign_bit\
+   = (UINT_TYPE)1 << (sizeof(INT_TYPE) * 8 - 1);  \
+  const UINT_TYPE sign_bit_mask = sign_bit - 1;   \
+  INT_TYPE ix = AS_TYPE(INT_TYPE)(x); \
+  INT_TYPE ax = ix & sign_bit_mask;   \
+  INT_TYPE mx = sign_bit - ix;\
+  mx = ix < 0 ? mx : ix;  \
+  INT_TYPE iy = AS_TYPE(INT_TYPE)(y); \
+  INT_TYPE ay = iy & sign_bit_mask;   \
+  INT_TYPE my = sign_bit - iy;\
+  my = iy < 0 ? my : iy;  \
+  INT_TYPE t = mx + (mx < my ? 1 : -1);   \
+  INT_TYPE r = sign_bit - t;  \
+  r = t < 0 ? r : t;  \
+  r = isnan(x) ? ix : r;  \
+  r = isnan(y) ? iy : r;  \
+  r = ((ax | ay) == 0 | ix == iy) ? iy : r;   \
+  return AS_TYPE(FLOAT_TYPE)(r);  \
 }
 
-NEXTAFTER(float, uint, FLT_NAN, 0.0f, 0x8001)
+NEXTAFTER(float, uint, int)
 _CLC_BINARY_VECTORIZE(_CLC_OVERLOAD _CLC_DEF, float, __clc_nextafter, float, 
float)
 
 #ifdef cl_khr_fp64
 #pragma OPENCL EXTENSION cl_khr_fp64 : enable
-#define DBL_NAN 0.0/0.0
 
-NEXTAFTER(double, ulong, DBL_NAN, 0.0, 0x8001)
+NEXTAFTER(double, ulong, long)
 _CLC_BINARY_VECTORIZE(_CLC_OVERLOAD _CLC_DEF, double, __clc_nextafter, double, 
double)
 #endif


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

Re: [clang-tools-extra] r280839 - Resubmit "Add a test for clang-tidy using the clang-cl driver."

[Ahmed Bougacha via cfe-commits]

On Thu, Sep 8, 2016 at 9:38 AM, Zachary Turner  wrote:
> What happens if you put the %s in quotes?

Same error

-Ahmed

>
> On Thu, Sep 8, 2016 at 9:32 AM Ahmed Bougacha 
> wrote:
>>
>> Hi Zachary,
>>
>> This test has been failing on some OS X machines, e.g.:
>>
>> http://lab.llvm.org:8080/green/job/clang-stage1-configure-RA_check/21470/consoleFull#19036563798254eaf0-7326-4999-85b0-388101f2d404
>>
>> My hypothesis is that the %s path ('/Users/...')  is somehow
>> interpreted as a '/U' by the clang-cl driver, causing the cryptic
>> 'Error while processing /Users/.../clang-cl-driver.cpp' error  (as the
>> test succeeds when run from other paths: /tmp/, /Volumes/).
>> Does that make sense?  Any suggestions on how to fix?
>>
>> Thanks!
>> -Ahmed
>>
>>
>> On Wed, Sep 7, 2016 at 11:28 AM, Zachary Turner via cfe-commits
>>  wrote:
>> > Author: zturner
>> > Date: Wed Sep  7 13:28:42 2016
>> > New Revision: 280839
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=280839=rev
>> > Log:
>> > Resubmit "Add a test for clang-tidy using the clang-cl driver."
>> >
>> > This was originally reverted because the patch on the clang
>> > tooling side was reverted.  That patch is being resubmitted,
>> > so this patch is resubmitted as well.
>> >
>> > Added:
>> > clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp
>> >
>> > Added: clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp?rev=280839=auto
>> >
>> > ==
>> > --- clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp (added)
>> > +++ clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp Wed Sep
>> > 7 13:28:42 2016
>> > @@ -0,0 +1,17 @@
>> > +// RUN: clang-tidy -checks=-*,modernize-use-nullptr %s --
>> > --driver-mode=cl /DTEST1 /DFOO=foo /DBAR=bar | FileCheck
>> > -implicit-check-not="{{warning|error}}:" %s
>> > +int *a = 0;
>> > +// CHECK: :[[@LINE-1]]:10: warning: use nullptr
>> > +#ifdef TEST1
>> > +int *b = 0;
>> > +// CHECK: :[[@LINE-1]]:10: warning: use nullptr
>> > +#endif
>> > +#define foo 1
>> > +#define bar 1
>> > +#if FOO
>> > +int *c = 0;
>> > +// CHECK: :[[@LINE-1]]:10: warning: use nullptr
>> > +#endif
>> > +#if BAR
>> > +int *d = 0;
>> > +// CHECK: :[[@LINE-1]]:10: warning: use nullptr
>> > +#endif
>> >
>> >
>> > ___
>> > cfe-commits mailing list
>> > cfe-commits@lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21968: [libcxx] Externally threaded libc++ variant - Take 2

[Asiri Rathnayake via cfe-commits]

rmaprath added a comment.

In https://reviews.llvm.org/D21968#534462, @EricWF wrote:

> This looks great.  Two comments:
>
> 1. The declarations should be used in both the inline and external pthread 
> implementation. They also need visibility declarations.
> 2. Why can't we use the inline implementation to implement 
> `external_threads.cpp`?
>
>   I took a stab at it here 
> .


Yup, that looks better.

I will incorporate this change and also address the remaining comments from 
@compnerd before I commit.

Thanks all!

/ Asiri


https://reviews.llvm.org/D21968



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

Re: [clang-tools-extra] r280839 - Resubmit "Add a test for clang-tidy using the clang-cl driver."

[Zachary Turner via cfe-commits]

What happens if you put the %s in quotes?

On Thu, Sep 8, 2016 at 9:32 AM Ahmed Bougacha 
wrote:

> Hi Zachary,
>
> This test has been failing on some OS X machines, e.g.:
>
> http://lab.llvm.org:8080/green/job/clang-stage1-configure-RA_check/21470/consoleFull#19036563798254eaf0-7326-4999-85b0-388101f2d404
>
> My hypothesis is that the %s path ('/Users/...')  is somehow
> interpreted as a '/U' by the clang-cl driver, causing the cryptic
> 'Error while processing /Users/.../clang-cl-driver.cpp' error  (as the
> test succeeds when run from other paths: /tmp/, /Volumes/).
> Does that make sense?  Any suggestions on how to fix?
>
> Thanks!
> -Ahmed
>
>
> On Wed, Sep 7, 2016 at 11:28 AM, Zachary Turner via cfe-commits
>  wrote:
> > Author: zturner
> > Date: Wed Sep  7 13:28:42 2016
> > New Revision: 280839
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=280839=rev
> > Log:
> > Resubmit "Add a test for clang-tidy using the clang-cl driver."
> >
> > This was originally reverted because the patch on the clang
> > tooling side was reverted.  That patch is being resubmitted,
> > so this patch is resubmitted as well.
> >
> > Added:
> > clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp
> >
> > Added: clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp?rev=280839=auto
> >
> ==
> > --- clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp (added)
> > +++ clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp Wed Sep
> 7 13:28:42 2016
> > @@ -0,0 +1,17 @@
> > +// RUN: clang-tidy -checks=-*,modernize-use-nullptr %s --
> --driver-mode=cl /DTEST1 /DFOO=foo /DBAR=bar | FileCheck
> -implicit-check-not="{{warning|error}}:" %s
> > +int *a = 0;
> > +// CHECK: :[[@LINE-1]]:10: warning: use nullptr
> > +#ifdef TEST1
> > +int *b = 0;
> > +// CHECK: :[[@LINE-1]]:10: warning: use nullptr
> > +#endif
> > +#define foo 1
> > +#define bar 1
> > +#if FOO
> > +int *c = 0;
> > +// CHECK: :[[@LINE-1]]:10: warning: use nullptr
> > +#endif
> > +#if BAR
> > +int *d = 0;
> > +// CHECK: :[[@LINE-1]]:10: warning: use nullptr
> > +#endif
> >
> >
> > ___
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [clang-tools-extra] r280839 - Resubmit "Add a test for clang-tidy using the clang-cl driver."

[Ahmed Bougacha via cfe-commits]

Hi Zachary,

This test has been failing on some OS X machines, e.g.:
  
http://lab.llvm.org:8080/green/job/clang-stage1-configure-RA_check/21470/consoleFull#19036563798254eaf0-7326-4999-85b0-388101f2d404

My hypothesis is that the %s path ('/Users/...')  is somehow
interpreted as a '/U' by the clang-cl driver, causing the cryptic
'Error while processing /Users/.../clang-cl-driver.cpp' error  (as the
test succeeds when run from other paths: /tmp/, /Volumes/).
Does that make sense?  Any suggestions on how to fix?

Thanks!
-Ahmed


On Wed, Sep 7, 2016 at 11:28 AM, Zachary Turner via cfe-commits
 wrote:
> Author: zturner
> Date: Wed Sep  7 13:28:42 2016
> New Revision: 280839
>
> URL: http://llvm.org/viewvc/llvm-project?rev=280839=rev
> Log:
> Resubmit "Add a test for clang-tidy using the clang-cl driver."
>
> This was originally reverted because the patch on the clang
> tooling side was reverted.  That patch is being resubmitted,
> so this patch is resubmitted as well.
>
> Added:
> clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp
>
> Added: clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp?rev=280839=auto
> ==
> --- clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp (added)
> +++ clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp Wed Sep  7 
> 13:28:42 2016
> @@ -0,0 +1,17 @@
> +// RUN: clang-tidy -checks=-*,modernize-use-nullptr %s -- --driver-mode=cl 
> /DTEST1 /DFOO=foo /DBAR=bar | FileCheck 
> -implicit-check-not="{{warning|error}}:" %s
> +int *a = 0;
> +// CHECK: :[[@LINE-1]]:10: warning: use nullptr
> +#ifdef TEST1
> +int *b = 0;
> +// CHECK: :[[@LINE-1]]:10: warning: use nullptr
> +#endif
> +#define foo 1
> +#define bar 1
> +#if FOO
> +int *c = 0;
> +// CHECK: :[[@LINE-1]]:10: warning: use nullptr
> +#endif
> +#if BAR
> +int *d = 0;
> +// CHECK: :[[@LINE-1]]:10: warning: use nullptr
> +#endif
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24048: [Driver] [Darwin] Add sanitizer libraries even if -nodefaultlibs is passed

[Chris Bieneman via cfe-commits]

beanz added a comment.

There are basically two solutions being debated. Either make `-fsanitize=...` 
bypass `-nostdlib` `-nodefaultlibs` or support a flag that explicitly bypasses 
it.

Personally I think the flag is probably the better way to go because it is more 
explicit, and has less implied behavior. If we go with this solution I will add 
a check to libcxx to detect if the compiler supports the flag and add it to 
sanitizer builds before committing the support to clang. This should prevent 
any breakages.

@kubabrecka, I can understand where you're coming from about the option 
starting with `-fsanitize`, but I disagree for two reasons. First, I think that 
it is more important for the option to be concise and clear about what it 
means, and `-flink-sanitizer-runtimes` seems pretty clear to me. Second, the 
existing `-fsanitize` options all refer to the compiler inserted runtime 
checks, where as this refers to the driver behavior, so I think there is a 
difference between the intention of the flags that justifies them not 
conforming to the same format.

I also have a small problem with @rsmith's point about `-fsanitize` being an 
explicit request when linking. I get very nervous about the idea of a flag 
having different meaning when compiling from when linking. While I understand 
that it doesn't make sense for `-fsanitize` options to not imply linking the 
runtimes, I think there is a good argument that they're not an explicit request 
for the runtime libraries.

Either way I don't really care which set of patches we go with. At this point 
the current diff has a functional `-flink-sanitizer-runtimes` flag supported 
across all drivers, and the original patch modified the Darwin behavior to 
match FreeBSD and GNUTools.

Thoughts anyone?


https://reviews.llvm.org/D24048



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

Re: [PATCH] D24289: Add warning when assigning enums to bitfields without an explicit unsigned underlying type

[Reid Kleckner via cfe-commits]

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

lgtm



Comment at: lib/Sema/SemaChecking.cpp:7841
@@ +7840,3 @@
+if (S.getLangOpts().CPlusPlus11 &&
+!BitfieldEnumDecl->getIntegerTypeSourceInfo() &&
+BitfieldEnumDecl->getNumPositiveBits() > 0 &&

Got it.


https://reviews.llvm.org/D24289



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

Re: [PATCH] D24349: [clang-tidy] Extend readability-container-size-empty to arbitrary class with size() and empty()

[Aaron Ballman via cfe-commits]

aaron.ballman added a subscriber: aaron.ballman.
aaron.ballman added a reviewer: aaron.ballman.


Comment at: clang-tidy/readability/ContainerSizeEmptyCheck.cpp:33
@@ +32,3 @@
+  const auto validContainer = namedDecl(
+  has(functionDecl(isPublic(), hasName("size"), returns(isInteger(,
+  has(functionDecl(isPublic(), hasName("empty"), returns(booleanType();

While it's not entirely unreasonable, it makes me uneasy to assume that 
anything with a size() and empty() member function must be a container. Is 
there a way to silence false positives aside from disabling the check entirely?

Perhaps this should instead be a user-configurable list of containers that's 
prepopulated with STL container names?

Also note: `returns(isInteger())` seems a bit permissive. Consider:
```
bool size() const;

enum E { one };
enum E size() const;
```
These both will qualify for that matcher, but don't seem like particularly 
valid indications that the object is a container.


https://reviews.llvm.org/D24349



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

Re: [PATCH] D24048: [Driver] [Darwin] Add sanitizer libraries even if -nodefaultlibs is passed

[Chris Bieneman via cfe-commits]

beanz updated this revision to Diff 70709.
beanz added a comment.

- Updated with FreeBSD and GNUTools support for -flink-sanitizer-runtimes


https://reviews.llvm.org/D24048

Files:
  include/clang/Driver/Options.td
  lib/Driver/ToolChains.cpp
  lib/Driver/ToolChains.h
  lib/Driver/Tools.cpp
  test/Driver/nodefaultlib.c
  test/Driver/nostdlib.c

Index: test/Driver/nostdlib.c
===
--- test/Driver/nostdlib.c
+++ test/Driver/nostdlib.c
@@ -29,3 +29,17 @@
 // CHECK-LINUX-NOSTDLIB: "{{(.*[^.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
 // CHECK-LINUX-NOSTDLIB-NOT: "{{.*}}/Inputs/resource_dir{{/|}}lib{{/|}}linux{{/|}}libclang_rt.builtins-i686.a"
 // CHECK-MSVC-NOSTDLIB: warning: argument unused during compilation: '--rtlib=compiler-rt'
+
+// RUN: %clang -target i686-apple-darwin -nostdlib -fsanitize=address -### %s 2>&1 | FileCheck -check-prefix=DARWIN_SAN_LIB1 %s
+// DARWIN_SAN_LIB1-NOT: libclang_rt.asan_osx_dynamic.dylib
+
+// RUN: %clang -target i686-apple-darwin -nostdlib -fsanitize=address -flink-sanitizer-runtimes -### %s 2>&1 | FileCheck -check-prefix=DARWIN_SAN_LIB2 %s
+// DARWIN_SAN_LIB2: libclang_rt.asan_osx_dynamic.dylib
+
+// RUN: %clang -target i686-pc-linux-gnu -nostdlib -fsanitize=address -### %s 2>&1 | FileCheck -check-prefix=NIX_SAN_LIB1 %s
+// RUN: %clang -target i686-unknown-freebsd -nostdlib -fsanitize=address -### %s 2>&1 | FileCheck -check-prefix=NIX_SAN_LIB1 %s
+// NIX_SAN_LIB1-NOT: libclang_rt.asan-i686.a
+
+// RUN: %clang -target i686-pc-linux-gnu -nostdlib -fsanitize=address -flink-sanitizer-runtimes -### %s 2>&1 | FileCheck -check-prefix=NIX_SAN_LIB2 %s
+// RUN: %clang -target i686-unknown-freebsd -nostdlib -fsanitize=address -flink-sanitizer-runtimes -### %s 2>&1 | FileCheck -check-prefix=NIX_SAN_LIB2 %s
+// NIX_SAN_LIB2: libclang_rt.asan-i686.a
Index: test/Driver/nodefaultlib.c
===
--- test/Driver/nodefaultlib.c
+++ test/Driver/nodefaultlib.c
@@ -8,3 +8,18 @@
 // RUN: %clang -target i686-pc-linux-gnu -stdlib=libc++ -nodefaultlibs -lstdc++ -### %s 2>&1 | FileCheck -check-prefix=TEST2 %s
 // TEST2-NOT: "-lc++"
 // TEST2: "-lstdc++"
+
+// RUN: %clang -target i686-apple-darwin -nodefaultlibs -fsanitize=address -### %s 2>&1 | FileCheck -check-prefix=DARWIN_SAN_LIB1 %s
+// DARWIN_SAN_LIB1-NOT: libclang_rt.asan_osx_dynamic.dylib
+
+// RUN: %clang -target i686-apple-darwin -nodefaultlibs -fsanitize=address -flink-sanitizer-runtimes -### %s 2>&1 | FileCheck -check-prefix=DARWIN_SAN_LIB2 %s
+// DARWIN_SAN_LIB2: libclang_rt.asan_osx_dynamic.dylib
+
+
+// RUN: %clang -target i686-pc-linux-gnu -nodefaultlibs -fsanitize=address -### %s 2>&1 | FileCheck -check-prefix=NIX_SAN_LIB1 %s
+// RUN: %clang -target i686-unknown-freebsd -nodefaultlibs -fsanitize=address -### %s 2>&1 | FileCheck -check-prefix=NIX_SAN_LIB1 %s
+// NIX_SAN_LIB1-NOT: libclang_rt.asan-i686.a
+
+// RUN: %clang -target i686-pc-linux-gnu -nodefaultlibs -fsanitize=address -flink-sanitizer-runtimes -### %s 2>&1 | FileCheck -check-prefix=NIX_SAN_LIB2 %s
+// RUN: %clang -target i686-unknown-freebsd -nodefaultlibs -fsanitize=address -flink-sanitizer-runtimes -### %s 2>&1 | FileCheck -check-prefix=NIX_SAN_LIB2 %s
+// NIX_SAN_LIB2: libclang_rt.asan-i686.a
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -3157,6 +3157,12 @@
 // C runtime, etc). Returns true if sanitizer system deps need to be linked in.
 static bool addSanitizerRuntimes(const ToolChain , const ArgList ,
  ArgStringList ) {
+  // If -nostdlib or -nodefaultlibs is passed and -flink-sanitizer-runtimes is
+  // not, we can just bail out of this early.
+  if (Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs) &&
+  !Args.hasArg(options::OPT_flink_sanitizer_runtimes))
+return false;
+
   SmallVector SharedRuntimes, StaticRuntimes,
   NonWholeStaticRuntimes, HelperStaticRuntimes, RequiredSymbols;
   collectSanitizerRuntimes(TC, Args, SharedRuntimes, StaticRuntimes,
@@ -8002,7 +8008,10 @@
   // SafeStack requires its own runtime libraries
   // These libraries should be linked first, to make sure the
   // __safestack_init constructor executes before everything else
-  if (getToolChain().getSanitizerArgs().needsSafeStackRt()) {
+  auto linkSanitizers =
+  !Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles) ||
+  Args.hasArg(options::OPT_flink_sanitizer_runtimes);
+  if (linkSanitizers && getToolChain().getSanitizerArgs().needsSafeStackRt()) {
 getMachOToolChain().AddLinkRuntimeLib(Args, CmdArgs,
   "libclang_rt.safestack_osx.a",
   /*AlwaysLink=*/true);
@@ -8061,6 +8070,8 @@
 
 // Let the tool chain choose which runtime library to link.
 

[PATCH] D24349: [clang-tidy] Extend readability-container-size-empty to arbitrary class with size() and empty()

[Kirill Bobyrev via cfe-commits]

omtcyfz created this revision.
omtcyfz added reviewers: alexfh, Eugene.Zelenko, klimek.
omtcyfz added a subscriber: cfe-commits.

Implementing [[ https://llvm.org/bugs/show_bug.cgi?id=26823 | feature request 
]].

This patch extends readability-container-size-empty check allowing it to 
produce warnings not only for stl containers.

https://reviews.llvm.org/D24349

Files:
  clang-tidy/readability/ContainerSizeEmptyCheck.cpp
  test/clang-tidy/readability-container-size-empty.cpp

Index: test/clang-tidy/readability-container-size-empty.cpp
===
--- test/clang-tidy/readability-container-size-empty.cpp
+++ test/clang-tidy/readability-container-size-empty.cpp
@@ -24,9 +24,23 @@
 };
 }
 
-
 }
 
+template 
+class TemplatedContainer {
+public:
+  int size();
+  bool empty();
+};
+
+template 
+class PrivateEmpty {
+public:
+  int size();
+private:
+  bool empty();
+};
+
 int main() {
   std::set intSet;
   std::string str;
@@ -127,6 +141,102 @@
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
   // CHECK-FIXES: {{^  }}if (vect4.empty()){{$}}
+
+  TemplatedContainer templated_container;
+  if (templated_container.size() == 0)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (templated_container.empty()){{$}}
+  if (templated_container.size() != 0)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+  if (0 == templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (templated_container.empty()){{$}}
+  if (0 != templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+  if (templated_container.size() > 0)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+  if (0 < templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:11: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+  if (templated_container.size() < 1)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (templated_container.empty()){{$}}
+  if (1 > templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:11: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (templated_container.empty()){{$}}
+  if (templated_container.size() >= 1)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+  if (1 <= templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+  if (templated_container.size() > 1) // no warning
+;
+  if (1 < templated_container.size()) // no warning
+;
+  if (templated_container.size() <= 1) // no warning
+;
+  if (1 >= templated_container.size()) // no warning
+;
+  if (!templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (templated_container.empty()){{$}}
+  if (templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+
+  if (templated_container.empty())
+;
+
+  // No warnings expected.
+  PrivateEmpty private_empty;
+  if (private_empty.size() == 0)
+;
+  if (private_empty.size() != 0)
+;
+  if (0 == private_empty.size())
+;
+  if (0 != private_empty.size())
+;
+  if (private_empty.size() > 0)
+;
+  if (0 < private_empty.size())
+;
+  if (private_empty.size() < 1)
+;
+  if (1 > private_empty.size())
+;
+  if (private_empty.size() >= 1)
+;
+  if (1 <= private_empty.size())
+;
+  if (private_empty.size() > 1)
+;
+  if (1 < private_empty.size())
+;
+  if (private_empty.size() <= 1)
+;
+  if (1 >= private_empty.size())
+;
+  if (!private_empty.size())
+;
+  if (private_empty.size())
+;
 }
 
 #define CHECKSIZE(x) if (x.size())
@@ -142,6 +252,16 @@
   CHECKSIZE(v);
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: the 'empty' method should be used
   // CHECK-MESSAGES: CHECKSIZE(v);
+
+  TemplatedContainer templated_container;
+  if (templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+  // CHECK-FIXES-NEXT: ;
+  CHECKSIZE(templated_container);
+  // 

Re: [PATCH] D15075: No error for conflict between inputs\outputs and clobber list

[Ziv Izhar via cfe-commits]

zizhar updated this revision to Diff 70660.
zizhar added a comment.

Hi,
I have moved the relevant code to TargetInfo as previously discussed, and 
created a default implementation and the x86 implementation.
Regarding the intrinsics, I believe it should be a separate fix, as it is not 
related to the clobber conflict this review is about.
Do you want me to open a bugzilla on the intrinsics?


https://reviews.llvm.org/D15075

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/TargetInfo.h
  lib/Basic/TargetInfo.cpp
  lib/Basic/Targets.cpp
  lib/Headers/intrin.h
  lib/Sema/SemaStmtAsm.cpp
  test/Sema/asm.c

Index: test/Sema/asm.c
===
--- test/Sema/asm.c
+++ test/Sema/asm.c
@@ -28,6 +28,16 @@
   asm ("nop" : : : "204"); // expected-error {{unknown register name '204' in asm}}
   asm ("nop" : : : "-1"); // expected-error {{unknown register name '-1' in asm}}
   asm ("nop" : : : "+1"); // expected-error {{unknown register name '+1' in asm}}
+  register void *clobber_conflict asm ("%rcx");
+  register void *no_clobber_conflict asm ("%rax");
+  int a,b,c;
+  asm ("nop" : "=r" (no_clobber_conflict) : "r" (clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}}
+  asm ("nop" : "=r" (clobber_conflict) : "r" (no_clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}}
+  asm ("nop" : "=r" (clobber_conflict) : "r" (clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}}
+  asm ("nop" : "=c" (a) : "r" (no_clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}}
+  asm ("nop" : "=r" (no_clobber_conflict) : "c" (c) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}}
+  asm ("nop" : "=r" (clobber_conflict) : "c" (c) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}}
+  asm ("nop" : "=a" (a) : "b" (b) : "%rcx", "%rbx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} 
 }
 
 // rdar://6094010
Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -22,6 +22,7 @@
 #include "clang/Sema/ScopeInfo.h"
 #include "clang/Sema/SemaInternal.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringSet.h"
 #include "llvm/MC/MCParser/MCAsmParser.h"
 using namespace clang;
 using namespace sema;
@@ -137,6 +138,55 @@
   return false;
 }
 
+// Extracting the register name from the Expression value,
+// if there is no register name to extract, returns ""
+StringRef ExtractRegisterName(const Expr *Expression, const TargetInfo ) {
+  while (const ImplicitCastExpr *P = dyn_cast(Expression)) {
+Expression = P->getSubExpr();
+  }
+  if (const DeclRefExpr *AsmDeclRef = dyn_cast(Expression)) {
+// Handle cases where the expression is a variable
+const VarDecl *Variable = dyn_cast(AsmDeclRef->getDecl());
+if (Variable && Variable->getStorageClass() == SC_Register) {
+  if (AsmLabelAttr *Attr = Variable->getAttr())
+return Target.isValidGCCRegisterName(Attr->getLabel())
+? Target.getNormalizedGCCRegisterName(Attr->getLabel(), true)
+: "";
+}
+  }
+  return "";
+}
+
+// Checks if there is a conflict between the input and output lists with the
+// clobbers list
+bool HasClobberConflict(MultiExprArg Exprs, StringLiteral **Constraints,
+  StringLiteral **Clobbers, int NumClobbers,
+  const TargetInfo , ASTContext ) {
+  llvm::StringSet<> InOutVars;
+  // Collect all the input and output registers from the extended asm statement
+  // in order to check for conflicts with the clobber list
+  for (int i = 0; i < Exprs.size(); ++i) {
+StringRef Constraint = Constraints[i]->getString();
+StringRef InOutReg = Target.getConstraintRegister(
+  Constraint, ExtractRegisterName(Exprs[i], Target));
+if (InOutReg != "")
+  InOutVars.insert(InOutReg);
+  }
+  // Check for each item in the clobber list if it conflicts with the input
+  // or output
+  for (int i = 0; i < NumClobbers; ++i) {
+StringRef Clobber = Clobbers[i]->getString();
+// We only check registers, therefore we don't check cc and memory clobbers
+if (Clobber == "cc" || Clobber == "memory")
+  continue;
+Clobber = Target.getNormalizedGCCRegisterName(Clobber, true);
+// Go over the output's registers we collected
+if (InOutVars.find(Clobber) != InOutVars.end())
+  return true;
+  }
+  return false;
+}
+
 StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple,
  bool IsVolatile, unsigned NumOutputs,
  

Re: [PATCH] D21698: [OpenCL] Allow disabling types and declarations associated with extensions

[Yaxun Liu via cfe-commits]

yaxunl added a comment.

I did profiling with valgrind for the cost of checking disabled types and 
declarations. For a typical program, time spent in checking disabled types and 
declarations are less than 0.1%, so this cost can be ignored.


https://reviews.llvm.org/D21698



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

[libcxx] r280944 - Added 'inline' attribute to basic_string's destructor

[Aditya Kumar via cfe-commits]

Author: hiraditya
Date: Thu Sep  8 09:31:44 2016
New Revision: 280944

URL: http://llvm.org/viewvc/llvm-project?rev=280944=rev
Log:
Added 'inline' attribute to basic_string's destructor

Author: laxmansole

Reviewers: howard.hinnant
   mclow.lists
Subscribers: EricWF, flyingforyou, evandro

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

Currently basic_string's destructor is not getting inlined. So adding 'inline' 
attribute to ~basic_string().
Worked in collaboration with Aditya Kumar.

Modified:
libcxx/trunk/include/string

Modified: libcxx/trunk/include/string
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/string?rev=280944=280943=280944=diff
==
--- libcxx/trunk/include/string (original)
+++ libcxx/trunk/include/string Thu Sep  8 09:31:44 2016
@@ -1798,6 +1798,7 @@ basic_string<_CharT, _Traits, _Allocator
 #endif  // _LIBCPP_HAS_NO_GENERALIZED_INITIALIZERS
 
 template 
+inline _LIBCPP_INLINE_VISIBILITY
 basic_string<_CharT, _Traits, _Allocator>::~basic_string()
 {
 #if _LIBCPP_DEBUG_LEVEL >= 2


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

Re: [PATCH] D24183: A clang tool for changing surrouding namespaces of class/function definitions.

[Kirill Bobyrev via cfe-commits]

omtcyfz added a comment.

Also +R Alex if he has some time to take a look at the code.


https://reviews.llvm.org/D24183



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

Re: [PATCH] D22130: Link static PIE programs against rcrt0.o on OpenBSD

[Ed Maste via cfe-commits]

emaste added a comment.

Seems fine to me, but I'm not particularly knowledgeable about OpenBSD's 
toolchain.


https://reviews.llvm.org/D22130



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

Re: [PATCH] D24183: A clang tool for changing surrouding namespaces of class/function definitions.

[Kirill Bobyrev via cfe-commits]

omtcyfz added a comment.

A round of mostly stylistic comments.



Comment at: change-namespace/ChangeNamespace.cpp:85
@@ +84,3 @@
+
+SourceLocation getStartOfNextLine(SourceLocation Loc, const SourceManager ,
+  const LangOptions ) {

Wouldn't it be better of in some common interface?

A couple of useful cases for other refactorings come to my mind. I.e. 
`getStartOfPreviousLine` would be nice to have there, too.

For this patch, though, I think `FIXME` would be enough.


Comment at: change-namespace/ChangeNamespace.cpp:111
@@ +110,3 @@
+
+// Returns a new replacement that is `R` with range that refers to
+// code after `Replaces` being applied.

A little confusing wording "a new replacement that is `R`". Here and later.


Comment at: change-namespace/ChangeNamespace.cpp:124
@@ +123,3 @@
+// Adds a replacement `R` into `Replaces` or merges it into `Replaces` by
+// applying all existing Replaces first if there is conflict.
+void addOrMergeReplacement(const tooling::Replacement ,

(probably not very much related to the patch and more out of curiosity)

If there is a conflict, why does it get resolved by applying all existing 
`Replaces` first?


Comment at: change-namespace/ChangeNamespace.cpp:139
@@ +138,3 @@
+  if (!Start.isValid() || !End.isValid()) {
+llvm::errs() << "start or end location were invalid\n";
+return tooling::Replacement();

Alex suggested using diagnostics instead of dumping stuff to `llvm::errs()` in 
D24224 and I think it looks way better. Actually, you can output locations with 
that, which makes error message more informative. It might be also easier to 
track down bugs with that info, rather than with raw error messages.

That's just a nit, probably `FIXME` is enough.

Same comment applies to the other `llvm::errs()` usages in this function.


Comment at: change-namespace/ChangeNamespace.cpp:179
@@ +178,3 @@
+return DeclName;
+  auto UnqualifiedName = DeclNameSplitted.back();
+  while (true) {

Shouldn't this one be `const`?


Comment at: change-namespace/ChangeNamespace.cpp:184
@@ +183,3 @@
+  return DeclName;
+auto Prefix = NsName.substr(0, Pos - 1);
+if (DeclName.startswith(Prefix))

Same for `Prefix` and `Pos`.


Comment at: change-namespace/ChangeNamespace.cpp:199
@@ +198,3 @@
+Code = ("namespace " + NsSplitted.back() + " {\n" + Code +
+"} // namespace " + NsSplitted.back() + "\n")
+   .str();

Probably `"} // end namespace "` here, at least that's what [[ 
http://llvm.org/docs/CodingStandards.html#namespace-indentation | LLVM Coding 
Standards ]] suggest.


Comment at: change-namespace/ChangeNamespace.cpp:231
@@ +230,3 @@
+
+// FIXME(ioeric): handle the following symbols:
+//   - Types in `UsingShadowDecl` (e.g. `using a::b::c;`) which are not matched

`FIXME` without handle here?


Comment at: change-namespace/ChangeNamespace.cpp:307
@@ +306,3 @@
+  // retrieving offset and length from it.
+  auto R = createReplacement(Start, End, "", *Result.SourceManager);
+  MoveNamespace MoveNs;

`const` here, too?


Comment at: change-namespace/ChangeNamespace.cpp:335
@@ +334,3 @@
+// into the old namespace after moving code from the old namespace to the new
+// namespace.
+void ChangeNamespaceTool::moveClassForwardDeclaration(

Slightly strange wording. I recall an offline discussion where you told about 
it, so I can understand that, but probably an example would be cool to have.


Comment at: change-namespace/ChangeNamespace.cpp:347
@@ +346,3 @@
+  // Delete the forward declaration from the code to be moved.
+  auto Deletion = createReplacement(Start, End, "", *Result.SourceManager);
+  addOrMergeReplacement(Deletion, [Deletion.getFilePath()]);

`const` here, too?


Comment at: change-namespace/ChangeNamespace.cpp:362
@@ +361,3 @@
+  assert(!NsDecl->decls_empty());
+  auto Insertion = createInsertion(NsDecl->decls_begin()->getLocStart(), Code,
+   *Result.SourceManager);

`const` here, too?


Comment at: change-namespace/ChangeNamespace.cpp:381
@@ +380,3 @@
+  llvm::StringRef Postfix = OldNs;
+  bool Consumed = Postfix.consume_front(OldNamespace);
+  assert(Consumed);

Also, if you only use this for an assert, why not just pass 
`Postfix.consume_front(OldNamespace)` to `assert`?


Comment at: change-namespace/ChangeNamespace.cpp:382
@@ +381,3 @@
+  bool Consumed = Postfix.consume_front(OldNamespace);
+  assert(Consumed);
+  (void)Consumed;

`assert`s are even more cool when they contain informative message, just like 
couple 

Re: [PATCH] D24224: [clang-rename] Merge rename-{ at | all } and optimise USRFindingAction.

[Kirill Bobyrev via cfe-commits]

omtcyfz updated this revision to Diff 70687.
omtcyfz marked 4 inline comments as done.
omtcyfz added a comment.

Address another round of comments.


https://reviews.llvm.org/D24224

Files:
  clang-rename/USRFindingAction.cpp
  clang-rename/USRFindingAction.h
  clang-rename/tool/ClangRename.cpp
  docs/clang-rename.rst
  test/clang-rename/ClassFindByName.cpp
  test/clang-rename/ClassTestMulti.cpp
  test/clang-rename/ClassTestMultiByName.cpp
  test/clang-rename/ClassTestMultiByNameYAML.cpp
  test/clang-rename/FunctionWithClassFindByName.cpp
  test/clang-rename/Inputs/ClassTestMultiByNameYAMLRenameAll.yaml
  test/clang-rename/Inputs/ClassTestMultiByNameYAMLRenameAt.yaml
  test/clang-rename/InvalidOldName.cpp
  test/clang-rename/NoNewName.cpp

Index: test/clang-rename/NoNewName.cpp
===
--- test/clang-rename/NoNewName.cpp
+++ test/clang-rename/NoNewName.cpp
@@ -1,4 +1,4 @@
 // Check for an error while -new-name argument has not been passed to
 // clang-rename.
 // RUN: not clang-rename -offset=133 %s 2>&1 | FileCheck %s
-// CHECK: clang-rename: for the -new-name option: must be specified
+// CHECK: clang-rename: -new-name must be specified.
Index: test/clang-rename/InvalidOldName.cpp
===
--- test/clang-rename/InvalidOldName.cpp
+++ /dev/null
@@ -1,2 +0,0 @@
-// RUN: not clang-rename rename-all -new-name=Foo -old-name=Bar %s -- 2>&1 | FileCheck %s
-// CHECK: clang-rename: could not find symbol Bar.
Index: test/clang-rename/Inputs/ClassTestMultiByNameYAMLRenameAt.yaml
===
--- test/clang-rename/Inputs/ClassTestMultiByNameYAMLRenameAt.yaml
+++ /dev/null
@@ -1,6 +0,0 @@

-- Offset: 6
-  NewName:Bar1
-- Offset: 44
-  NewName:Bar2
-...
Index: test/clang-rename/Inputs/ClassTestMultiByNameYAMLRenameAll.yaml
===
--- test/clang-rename/Inputs/ClassTestMultiByNameYAMLRenameAll.yaml
+++ /dev/null
@@ -1,6 +0,0 @@

-- OldName:Foo1
-  NewName:Bar1
-- OldName:Foo2
-  NewName:Bar2
-...
Index: test/clang-rename/FunctionWithClassFindByName.cpp
===
--- test/clang-rename/FunctionWithClassFindByName.cpp
+++ test/clang-rename/FunctionWithClassFindByName.cpp
@@ -9,4 +9,4 @@
   return 0;
 }
 
-// RUN: clang-rename rename-all -old-name=Foo -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// RUN: clang-rename -qualified-name=Foo -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
Index: test/clang-rename/ClassTestMultiByNameYAML.cpp
===
--- test/clang-rename/ClassTestMultiByNameYAML.cpp
+++ /dev/null
@@ -1,10 +0,0 @@
-class Foo1 { // CHECK: class Bar1
-};
-
-class Foo2 { // CHECK: class Bar2
-};
-
-// Test 1.
-// RUN: clang-rename rename-all -input %S/Inputs/ClassTestMultiByNameYAMLRenameAll.yaml %s -- | sed 's,//.*,,' | FileCheck %s
-// Test 2.
-// RUN: clang-rename rename-all -input %S/Inputs/ClassTestMultiByNameYAMLRenameAt.yaml %s -- | sed 's,//.*,,' | FileCheck %s
Index: test/clang-rename/ClassTestMultiByName.cpp
===
--- test/clang-rename/ClassTestMultiByName.cpp
+++ test/clang-rename/ClassTestMultiByName.cpp
@@ -5,4 +5,4 @@
 };
 
 // Test 1.
-// RUN: clang-rename rename-all -old-name=Foo1 -new-name=Bar1 -old-name=Foo2 -new-name=Bar2 %s -- | sed 's,//.*,,' | FileCheck %s
+// RUN: clang-rename -qualified-name=Foo1 -new-name=Bar1 -qualified-name=Foo2 -new-name=Bar2 %s -- | sed 's,//.*,,' | FileCheck %s
Index: test/clang-rename/ClassTestMulti.cpp
===
--- test/clang-rename/ClassTestMulti.cpp
+++ test/clang-rename/ClassTestMulti.cpp
@@ -5,7 +5,7 @@
 };
 
 // Test 1.
-// RUN: clang-rename rename-all -offset=6 -new-name=Bar1 -offset=76 -new-name=Bar2 %s -- | sed 's,//.*,,' | FileCheck %s
+// RUN: clang-rename -offset=6 -new-name=Bar1 -offset=76 -new-name=Bar2 %s -- | sed 's,//.*,,' | FileCheck %s
 
 // To find offsets after modifying the file, use:
 //   grep -Ubo 'Foo.*' 
Index: test/clang-rename/ClassFindByName.cpp
===
--- test/clang-rename/ClassFindByName.cpp
+++ test/clang-rename/ClassFindByName.cpp
@@ -7,4 +7,4 @@
 }
 
 // Test 1.
-// RUN: clang-rename rename-all -old-name=Foo -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// RUN: clang-rename -qualified-name=Foo -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
Index: docs/clang-rename.rst
===
--- docs/clang-rename.rst
+++ docs/clang-rename.rst
@@ -52,21 +52,29 @@
 Although a command line interface exists, it is highly recommended to use the
 text editor interface instead 

Re: [PATCH] D24192: [clang-refactor] introducing clang-refactor

[Marek Kurdej via cfe-commits]

curdeius added inline comments.


Comment at: clang-refactor/driver/ModuleManager.h:14-20
@@ +13,9 @@
+#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/StringRef.h"
+
+#include 
+#include 
+#include 
+
+#include "core/RefactoringModule.h"
+

I thought that idea behind sorting includes using clang-format is to avoid 
handling groups and order manually.
I don't think that there is any policy prohibiting separating includes into 
groups, but AFAIK, there is one that says that STL includes should be the last 
(you should include in order from the most specific to the most generic, i.e. 
subproject, clang, llvm, STL).


https://reviews.llvm.org/D24192



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

Re: [PATCH] D24224: [clang-rename] Merge rename-{ at | all } and optimise USRFindingAction.

[Alexander Kornienko via cfe-commits]

alexfh requested changes to this revision.
This revision now requires changes to proceed.


Comment at: clang-rename/USRFindingAction.cpp:157
@@ +156,3 @@
+if (!Point.isValid()) {
+  ErrorOccurred = true;
+  return false;

Should we emit a diagnostic in this case as well?


Comment at: clang-rename/USRFindingAction.cpp:171
@@ +170,3 @@
+DiagnosticsEngine::Error, "clang-rename could not find symbol "
+  "in %0 at %1:%2 (offset %3).");
+Engine.Report(CouldNotFindSymbolAt)

nit: no trailing period needed


Comment at: clang-rename/USRFindingAction.cpp:173
@@ +172,3 @@
+Engine.Report(CouldNotFindSymbolAt)
+<< SourceMgr.getFilename(Point) << FullLoc.getSpellingLineNumber()
+<< FullLoc.getSpellingColumnNumber() << SymbolOffset;

How about reporting diagnostics at this specific location?


Comment at: clang-rename/USRFindingAction.h:40
@@ -36,1 +39,3 @@
+  ArrayRef getUSRList() { return USRList; }
+  bool hasErrorOccurred() { return ErrorOccurred; }
 

I'm not a native speaker, but to me `has` here reads as "possesses", not as a 
marker of past perfect tense. Maybe just `errorOccurred()`?


https://reviews.llvm.org/D24224



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

Re: [PATCH] D24192: [clang-refactor] introducing clang-refactor

[Kirill Bobyrev via cfe-commits]

omtcyfz added inline comments.


Comment at: clang-refactor/driver/ModuleManager.cpp:22-24
@@ +21,5 @@
+int ModuleManager::Dispatch(StringRef Command, int argc, const char **argv) {
+  if (CommandToModuleID.find(Command) != CommandToModuleID.end()) {
+return RegisteredModules[CommandToModuleID[Command]]->run(argc, argv);
+  }
+  return 1;

curdeius wrote:
> Using `find` and then `operator[]` makes the search twice. IMO, it would be 
> better to avoid that by:
> 
> ```
>   auto it = CommandToModuleID.find(Command);
>   if (it != CommandToModuleID.end()) {
> return RegisteredModules[*it]->run(argc, argv);
>   }
> ```
The search is O(1) anyway, but the code itself probably becomes more 
reasonable, thanks.


Comment at: clang-refactor/driver/ModuleManager.h:13-19
@@ +12,9 @@
+
+#include "llvm/ADT/StringRef.h"
+
+#include 
+#include 
+#include 
+
+#include "core/RefactoringModule.h"
+

curdeius wrote:
> clang-format includes (and make it a single block)?
I like to separate includes into three groups (LLVM includes, STL includes, 
subproject includes), which I think is not very bad. Haven't seen any policy 
prohibiting it. Feel free to elaborate, though.

Sorted the includes within the second group.


Comment at: clang-refactor/modules/core/RefactoringModule.h:148
@@ +147,3 @@
+  // Panic: if there are conflicting replacements.
+  virtual int applyReplacementsOrOutputDiagnostics() = 0;
+

alexfh wrote:
> This should be possible to implement in a refactoring-independent way in 
> clang-refactor itself. Or do you see any issues with this?
Nope, totally agree.


https://reviews.llvm.org/D24192



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

Re: [PATCH] D24192: [clang-refactor] introducing clang-refactor

[Kirill Bobyrev via cfe-commits]

omtcyfz updated this revision to Diff 70682.
omtcyfz marked 8 inline comments as done.
omtcyfz added a comment.
Herald added a subscriber: beanz.

Addressing few comments.

Major improvements on the way.


https://reviews.llvm.org/D24192

Files:
  CMakeLists.txt
  clang-refactor/CMakeLists.txt
  clang-refactor/driver/CMakeLists.txt
  clang-refactor/driver/Driver.cpp
  clang-refactor/driver/ModuleManager.cpp
  clang-refactor/driver/ModuleManager.h
  clang-refactor/modules/CMakeLists.txt
  clang-refactor/modules/core/RefactoringModule.h

Index: clang-refactor/modules/core/RefactoringModule.h
===
--- /dev/null
+++ clang-refactor/modules/core/RefactoringModule.h
@@ -0,0 +1,169 @@
+//===--- RefactoringModule.h - clang-refactor ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_REFACTOR_REFACTORING_MODULE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_REFACTOR_REFACTORING_MODULE_H
+
+#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Refactoring.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Signals.h"
+
+#include 
+
+namespace clang {
+namespace refactor {
+
+class RefactoringModule {
+public:
+  RefactoringModule(const std::string ,
+const std::string )
+  : ModuleName(ModuleName), ModuleMetaDescription(ModuleMetaDescription) {}
+
+  // Refactoring consists of 3.5 stages:
+  //
+  // 0. Check arguments for validity.
+  //
+  // 1. Extract information needed for the refactoring upon tool invocation.
+  //
+  // 2. Handle translation unit and identify whether the refactoring can be
+  // applied. If it can, store the replacements. Panic otherwise.
+  //
+  // 3. Merge duplicate replacements, panic if there are conflicting ones.
+  // Process translation unit and apply refactoring.
+  //
+  // Few examples of how each of these stages would look like for future
+  // modules "rename" and "inline".
+  //
+  // Rename
+  // ==
+  //
+  // 0. Check arguments for validity.
+  //
+  // 1. Rename operates with USRs, so the first stage would be finding the
+  // symbol, which will be renamed and storing its USR.
+  //
+  // 2. Check whether renaming will introduce any name conflicts. If it won't
+  // find each occurrence of the symbol in translation unit using USR and store
+  // replacements.
+  //
+  // 3. Apply replacements or output diagnostics. Handle duplicates and panic if
+  // there are conflicts.
+  //
+  // Inline
+  // ==
+  //
+  // 0. Check arguments for validity.
+  //
+  // 1. Find function, identify what the possible issues might be.
+  //
+  // 2. Check whether inlining will introduce any issues, e.g. there is a
+  // pointer passed somewhere to the inlined function and after inlining it the
+  // code will no longer compile. If it won't find function calls, add needed
+  // temporary variables and replace the call with function body.
+  //
+  // 3. Apply replacements. Handle duplicates and panic if there are conflicts.
+  //
+  // Summary
+  // ===
+  //
+  // As one can easily see, step 1 should be performed upon module invocation
+  // and it needs to process translation unit, from which the passed 
+  // comes from.
+  //
+  // With appropriate facilities step 2 can be parallelized to process multiple
+  // translation units of the project independently. If none of them have any
+  // issues with applying this refactoring replacements are stored and queued
+  // for later.
+  //
+  // Step 3 can be parallelized even more easily. It basically consists of text
+  // replacements.
+  //
+  int run(int argc, const char **argv) {
+// Register options.
+llvm::cl::OptionCategory RefactoringModuleOptions(ModuleName.c_str(),
+  ModuleMetaDescription.c_str());
+registerCustomOptions(RefactoringModuleOptions);
+registerCustomOptions(RefactoringModuleOptions);
+// Parse options and get compilations.
+llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
+tooling::CommonOptionsParser OptionsParser(argc, argv,
+   RefactoringModuleOptions);
+tooling::RefactoringTool Tool(OptionsParser.getCompilations(),
+  OptionsParser.getSourcePathList());
+
+// Actual routine.
+checkArguments();
+extractRefactoringInformation();
+handleTranslationUnit();
+applyReplacementsOrOutputDiagnostics();
+return 0;
+  }
+
+  // Routine for registering common modules options.
+  void registerCommonOptions(llvm::cl::OptionCategory ) {
+  

r280921 - Moved unreachable to appease msvc, gcc and clang

[Simon Pilgrim via cfe-commits]

Author: rksimon
Date: Thu Sep  8 06:03:41 2016
New Revision: 280921

URL: http://llvm.org/viewvc/llvm-project?rev=280921=rev
Log:
Moved unreachable to appease msvc, gcc and clang

Modified:
cfe/trunk/lib/CodeGen/CGVTables.cpp

Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.cpp?rev=280921=280920=280921=diff
==
--- cfe/trunk/lib/CodeGen/CGVTables.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGVTables.cpp Thu Sep  8 06:03:41 2016
@@ -529,9 +529,6 @@ llvm::Constant *CodeGenVTables::CreateVT
   };
 
   switch (Component.getKind()) {
-  default:
-llvm_unreachable("Unexpected vtable component kind");
-
   case VTableComponent::CK_VCallOffset:
 return OffsetConstant(Component.getVCallOffset());
 
@@ -619,6 +616,8 @@ llvm::Constant *CodeGenVTables::CreateVT
   case VTableComponent::CK_UnusedFunctionPointer:
 return llvm::ConstantExpr::getNullValue(CGM.Int8PtrTy);
   }
+
+  llvm_unreachable("Unexpected vtable component kind");
 }
 
 llvm::Constant *


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

Re: [PATCH] D24224: [clang-rename] Merge rename-{ at | all } and optimise USRFindingAction.

[Kirill Bobyrev via cfe-commits]

omtcyfz updated this revision to Diff 70673.
omtcyfz marked 2 inline comments as done.
omtcyfz added a comment.

Address comments.


https://reviews.llvm.org/D24224

Files:
  clang-rename/USRFindingAction.cpp
  clang-rename/USRFindingAction.h
  clang-rename/tool/ClangRename.cpp
  docs/clang-rename.rst
  test/clang-rename/ClassFindByName.cpp
  test/clang-rename/ClassTestMulti.cpp
  test/clang-rename/ClassTestMultiByName.cpp
  test/clang-rename/ClassTestMultiByNameYAML.cpp
  test/clang-rename/FunctionWithClassFindByName.cpp
  test/clang-rename/Inputs/ClassTestMultiByNameYAMLRenameAll.yaml
  test/clang-rename/Inputs/ClassTestMultiByNameYAMLRenameAt.yaml
  test/clang-rename/InvalidOldName.cpp
  test/clang-rename/NoNewName.cpp

Index: test/clang-rename/NoNewName.cpp
===
--- test/clang-rename/NoNewName.cpp
+++ test/clang-rename/NoNewName.cpp
@@ -1,4 +1,4 @@
 // Check for an error while -new-name argument has not been passed to
 // clang-rename.
 // RUN: not clang-rename -offset=133 %s 2>&1 | FileCheck %s
-// CHECK: clang-rename: for the -new-name option: must be specified
+// CHECK: clang-rename: -new-name must be specified.
Index: test/clang-rename/InvalidOldName.cpp
===
--- test/clang-rename/InvalidOldName.cpp
+++ /dev/null
@@ -1,2 +0,0 @@
-// RUN: not clang-rename rename-all -new-name=Foo -old-name=Bar %s -- 2>&1 | FileCheck %s
-// CHECK: clang-rename: could not find symbol Bar.
Index: test/clang-rename/Inputs/ClassTestMultiByNameYAMLRenameAt.yaml
===
--- test/clang-rename/Inputs/ClassTestMultiByNameYAMLRenameAt.yaml
+++ /dev/null
@@ -1,6 +0,0 @@

-- Offset: 6
-  NewName:Bar1
-- Offset: 44
-  NewName:Bar2
-...
Index: test/clang-rename/Inputs/ClassTestMultiByNameYAMLRenameAll.yaml
===
--- test/clang-rename/Inputs/ClassTestMultiByNameYAMLRenameAll.yaml
+++ /dev/null
@@ -1,6 +0,0 @@

-- OldName:Foo1
-  NewName:Bar1
-- OldName:Foo2
-  NewName:Bar2
-...
Index: test/clang-rename/FunctionWithClassFindByName.cpp
===
--- test/clang-rename/FunctionWithClassFindByName.cpp
+++ test/clang-rename/FunctionWithClassFindByName.cpp
@@ -9,4 +9,4 @@
   return 0;
 }
 
-// RUN: clang-rename rename-all -old-name=Foo -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// RUN: clang-rename -qualified-name=Foo -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
Index: test/clang-rename/ClassTestMultiByNameYAML.cpp
===
--- test/clang-rename/ClassTestMultiByNameYAML.cpp
+++ /dev/null
@@ -1,10 +0,0 @@
-class Foo1 { // CHECK: class Bar1
-};
-
-class Foo2 { // CHECK: class Bar2
-};
-
-// Test 1.
-// RUN: clang-rename rename-all -input %S/Inputs/ClassTestMultiByNameYAMLRenameAll.yaml %s -- | sed 's,//.*,,' | FileCheck %s
-// Test 2.
-// RUN: clang-rename rename-all -input %S/Inputs/ClassTestMultiByNameYAMLRenameAt.yaml %s -- | sed 's,//.*,,' | FileCheck %s
Index: test/clang-rename/ClassTestMultiByName.cpp
===
--- test/clang-rename/ClassTestMultiByName.cpp
+++ test/clang-rename/ClassTestMultiByName.cpp
@@ -5,4 +5,4 @@
 };
 
 // Test 1.
-// RUN: clang-rename rename-all -old-name=Foo1 -new-name=Bar1 -old-name=Foo2 -new-name=Bar2 %s -- | sed 's,//.*,,' | FileCheck %s
+// RUN: clang-rename -qualified-name=Foo1 -new-name=Bar1 -qualified-name=Foo2 -new-name=Bar2 %s -- | sed 's,//.*,,' | FileCheck %s
Index: test/clang-rename/ClassTestMulti.cpp
===
--- test/clang-rename/ClassTestMulti.cpp
+++ test/clang-rename/ClassTestMulti.cpp
@@ -5,7 +5,7 @@
 };
 
 // Test 1.
-// RUN: clang-rename rename-all -offset=6 -new-name=Bar1 -offset=76 -new-name=Bar2 %s -- | sed 's,//.*,,' | FileCheck %s
+// RUN: clang-rename -offset=6 -new-name=Bar1 -offset=76 -new-name=Bar2 %s -- | sed 's,//.*,,' | FileCheck %s
 
 // To find offsets after modifying the file, use:
 //   grep -Ubo 'Foo.*' 
Index: test/clang-rename/ClassFindByName.cpp
===
--- test/clang-rename/ClassFindByName.cpp
+++ test/clang-rename/ClassFindByName.cpp
@@ -7,4 +7,4 @@
 }
 
 // Test 1.
-// RUN: clang-rename rename-all -old-name=Foo -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// RUN: clang-rename -qualified-name=Foo -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
Index: docs/clang-rename.rst
===
--- docs/clang-rename.rst
+++ docs/clang-rename.rst
@@ -52,21 +52,29 @@
 Although a command line interface exists, it is highly recommended to use the
 text editor interface instead for better 

r280917 - Fixed a 'not all control paths return a value' warning on MSVC builds

[Simon Pilgrim via cfe-commits]

Author: rksimon
Date: Thu Sep  8 04:59:58 2016
New Revision: 280917

URL: http://llvm.org/viewvc/llvm-project?rev=280917=rev
Log:
Fixed a 'not all control paths return a value' warning on MSVC builds

Modified:
cfe/trunk/lib/CodeGen/CGVTables.cpp

Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.cpp?rev=280917=280916=280917=diff
==
--- cfe/trunk/lib/CodeGen/CGVTables.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGVTables.cpp Thu Sep  8 04:59:58 2016
@@ -29,7 +29,7 @@ using namespace CodeGen;
 CodeGenVTables::CodeGenVTables(CodeGenModule )
 : CGM(CGM), VTContext(CGM.getContext().getVTableContext()) {}
 
-llvm::Constant *CodeGenModule::GetAddrOfThunk(GlobalDecl GD, 
+llvm::Constant *CodeGenModule::GetAddrOfThunk(GlobalDecl GD,
   const ThunkInfo ) {
   const CXXMethodDecl *MD = cast(GD.getDecl());
 
@@ -93,7 +93,7 @@ static RValue PerformReturnAdjustment(Co
 AdjustNull = CGF.createBasicBlock("adjust.null");
 AdjustNotNull = CGF.createBasicBlock("adjust.notnull");
 AdjustEnd = CGF.createBasicBlock("adjust.end");
-  
+
 llvm::Value *IsNull = CGF.Builder.CreateIsNull(ReturnValue);
 CGF.Builder.CreateCondBr(IsNull, AdjustNull, AdjustNotNull);
 CGF.EmitBlock(AdjustNotNull);
@@ -110,14 +110,14 @@ static RValue PerformReturnAdjustment(Co
 CGF.EmitBlock(AdjustNull);
 CGF.Builder.CreateBr(AdjustEnd);
 CGF.EmitBlock(AdjustEnd);
-  
+
 llvm::PHINode *PHI = CGF.Builder.CreatePHI(ReturnValue->getType(), 2);
 PHI->addIncoming(ReturnValue, AdjustNotNull);
-PHI->addIncoming(llvm::Constant::getNullValue(ReturnValue->getType()), 
+PHI->addIncoming(llvm::Constant::getNullValue(ReturnValue->getType()),
  AdjustNull);
 ReturnValue = PHI;
   }
-  
+
   return RValue::get(ReturnValue);
 }
 
@@ -314,7 +314,7 @@ void CodeGenFunction::EmitCallAndReturnF
   CurFnInfo->getReturnInfo().getKind() == ABIArgInfo::Indirect &&
   !hasScalarEvaluationKind(CurFnInfo->getReturnType()))
 Slot = ReturnValueSlot(ReturnValue, ResultType.isVolatileQualified());
-  
+
   // Now emit our call.
   llvm::Instruction *CallOrInvoke;
   RValue RV = EmitCall(*CurFnInfo, Callee, Slot, CallArgs, MD, );
@@ -433,14 +433,14 @@ void CodeGenVTables::emitThunk(GlobalDec
 // Remove the name from the old thunk function and get a new thunk.
 OldThunkFn->setName(StringRef());
 Entry = cast(CGM.GetAddrOfThunk(GD, Thunk));
-
+
 // If needed, replace the old thunk with a bitcast.
 if (!OldThunkFn->use_empty()) {
   llvm::Constant *NewPtrForOldDecl =
 llvm::ConstantExpr::getBitCast(Entry, OldThunkFn->getType());
   OldThunkFn->replaceAllUsesWith(NewPtrForOldDecl);
 }
-
+
 // Remove the old thunk.
 OldThunkFn->eraseFromParent();
   }
@@ -500,7 +500,7 @@ void CodeGenVTables::maybeEmitThunkForVT
 
 void CodeGenVTables::EmitThunks(GlobalDecl GD)
 {
-  const CXXMethodDecl *MD = 
+  const CXXMethodDecl *MD =
 cast(GD.getDecl())->getCanonicalDecl();
 
   // We don't need to generate thunks for the base destructor.
@@ -529,6 +529,9 @@ llvm::Constant *CodeGenVTables::CreateVT
   };
 
   switch (Component.getKind()) {
+  default:
+llvm_unreachable("Unexpected vtable component kind");
+
   case VTableComponent::CK_VCallOffset:
 return OffsetConstant(Component.getVCallOffset());
 
@@ -636,9 +639,9 @@ CodeGenVTables::CreateVTableInitializer(
 }
 
 llvm::GlobalVariable *
-CodeGenVTables::GenerateConstructionVTable(const CXXRecordDecl *RD, 
-  const BaseSubobject , 
-  bool BaseIsVirtual, 
+CodeGenVTables::GenerateConstructionVTable(const CXXRecordDecl *RD,
+  const BaseSubobject ,
+  bool BaseIsVirtual,
llvm::GlobalVariable::LinkageTypes Linkage,
   VTableAddressPointsMapTy& AddressPoints) 
{
   if (CGDebugInfo *DI = CGM.getModuleDebugInfo())
@@ -671,7 +674,7 @@ CodeGenVTables::GenerateConstructionVTab
 Linkage = llvm::GlobalVariable::InternalLinkage;
 
   // Create the variable that will hold the construction vtable.
-  llvm::GlobalVariable *VTable = 
+  llvm::GlobalVariable *VTable =
 CGM.CreateOrReplaceCXXRuntimeVariable(Name, ArrayType, Linkage);
   CGM.setGlobalVisibility(VTable, RD);
 
@@ -684,7 +687,7 @@ CodeGenVTables::GenerateConstructionVTab
   // Create and set the initializer.
   llvm::Constant *Init = CreateVTableInitializer(*VTLayout, RTTI);
   VTable->setInitializer(Init);
-  
+
   CGM.EmitVTableTypeMetadata(VTable, *VTLayout.get());
 
   return VTable;
@@ -699,7 +702,7 @@ static bool shouldEmitAvailableExternall
 /// Compute the required linkage of the vtable for the given class.
 ///
 /// Note that we 

Re: [PATCH] D24307: calculate extent size for memory regions allocated by C++ new expression

[Artem Dergachev via cfe-commits]

NoQ added a comment.

Thanks for the patch! Not sure why, but i always have warm feelings for the 
`MallocChecker` and wish it to improve.

Added minor style comments, joined the "where to put the code" debate.



Comment at: lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:69
@@ -68,2 +68,3 @@
 
 static SVal computeExtentBegin(SValBuilder ,
+   const MemRegion *region, ProgramStateRef state) 
{

For ease of use, you no longer need to pass `svalBuilder` here - you can take 
it from the `state->getStateManager()`.


Comment at: lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:81
@@ +80,3 @@
+  // If we known the symbolic region extent size
+  //(e.g. allocated by malloc or new)
+  // we can assume that the region starts at 0.

Whitespace slightly off.


Comment at: lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:83
@@ -78,1 +82,3 @@
+  // we can assume that the region starts at 0.
+  if (!state->isNull(extentVal).isConstrained()) {
 return UnknownVal();

Perhaps you could consider the memory space of the `region`, it would look a 
bit less hacky to me.

In my dreams, i wish heap regions were no longer symbolic regions, and this 
hack would go away then.

Also, i recall there is a bug in `isNull()`: in the `ConstraintManager` class 
(this time i actually mean //the abstract base class// of 
`RangeConstraintManager`) this function boils down to `assume()`, but in 
`RangeConstraintManager` it is overridden to do a direct lookup into the 
constraint map; which means that in fact this function does not simplify 
symbolic expressions before answering. This code is probably unaffected because 
extents are always either concrete or atomic symbols, but i think i'd make a 
patch for that.


Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:995
@@ -983,2 +994,3 @@
: AF_CXXNew);
+  State=addExtentSize(C,NE,State);
   State = ProcessZeroAllocation(C, NE, 0, State);

This code could use a bit more spaces around "=" and ",".


Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1003
@@ +1002,3 @@
+//
+ProgramStateRef MallocChecker::addExtentSize(CheckerContext ,
+ const CXXNewExpr *NE,

dkrupp wrote:
> xazax.hun wrote:
> > zaks.anna wrote:
> > > I am not sure this code belongs to the malloc checker since it only 
> > > supports the array bounds checker. Is there a reason it's not part of 
> > > that checker?
> > I think it is part of the malloc checker because it already does something 
> > very very similar to malloc, see the MallocMemAux function. So in fact, for 
> > the array bounds checker to work properly, the malloc checker should be 
> > turned on.
> Extent size is used by ArrayBoundChecker, ArrayBoundCheckerV2 and 
> CStringChecker checkers currently. New expression in case of simple 
> allocations (0 allocation) was already handled in Malloc checker , that's why 
> I implemented it there. But I agree it feels odd that one has to switch on 
> unix.Malloc checker to get the size of new allocated heap regions. Should I 
> move this to ArrayBoundChecker or ArrayBoundCheckerV2?
1. Well, in my dreams, this code should be in the silent operator-new modelling 
checker, which would be separate from the stateless operator-new bug-finding 
checker. Then all the checkers that rely on extent would automatically load the 
modelling checker as a dependency.

2. Yeah, i think many checkers may consider extents, not just array bound; 
other checkers may appear in the future. This info is very useful, which is why 
we have the whole symbol class just for that (however, checker communication 
through constraints on this symbol class wasn't IMHO a good idea, because it's 
harder for the constraint manager to deal with symbols and constraints rather 
than with concrete values).

//Just wanted to speak out, thoughts below might perhaps be more on-topic//

3. The MallocChecker is not just unix.Malloc, but also cplusplus.NewDelete, 
etc., which makes a lot more sense to leave it here.

4. Consider another part of MallocChecker's job - modeling the memory spaces 
for symbolic regions (heap vs. unknown). For malloc(), this is done in the 
checker. For operator-new, it is done in the ExprEngine(!), because this is 
part of the language rather than a library function. Perhaps ExprEngine would 
be the proper place for that code as well.


Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1010
@@ +1009,3 @@
+  SVal Size;
+  const LocationContext *LCtx = C.getPredecessor()->getLocationContext();
+  const MemRegion *Region;

`C.getLocationContext()` is a bit shorter.


Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1017
@@ +1016,3 @@
+ 

Re: [PATCH] D24333: [CleanupInfo] Use cleanupsHaveSideEffects instead of exprNeedsCleanups in assertions

[Tim Shen via cfe-commits]

timshen updated this revision to Diff 70667.
timshen added a comment.

Update the test file name and remove unused code.


https://reviews.llvm.org/D24333

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/pr30306.cpp

Index: clang/test/SemaCXX/pr30306.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/pr30306.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+int f(int const ) { return p; }
+
+template 
+void g(T) { int a[f(3)]; } // expected-no-diagnostics
+
+int main() {
+  g(2);
+  return 0;
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5746,7 +5746,7 @@
 
   unsigned FirstCleanup = ExprEvalContexts.back().NumCleanupObjects;
   assert(ExprCleanupObjects.size() >= FirstCleanup);
-  assert(Cleanup.exprNeedsCleanups() ||
+  assert(Cleanup.cleanupsHaveSideEffects() ||
  ExprCleanupObjects.size() == FirstCleanup);
   if (!Cleanup.exprNeedsCleanups())
 return SubExpr;
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11736,7 +11736,7 @@
 
   if (hasAnyUnrecoverableErrorsInThisFunction())
 DiscardCleanupsInEvaluationContext();
-  assert(!Cleanup.exprNeedsCleanups() &&
+  assert(!Cleanup.cleanupsHaveSideEffects() &&
  "cleanups within StmtExpr not correctly bound!");
   PopExpressionEvaluationContext();
 
@@ -12204,7 +12204,7 @@
   // Leave the expression-evaluation context.
   if (hasAnyUnrecoverableErrorsInThisFunction())
 DiscardCleanupsInEvaluationContext();
-  assert(!Cleanup.exprNeedsCleanups() &&
+  assert(!Cleanup.cleanupsHaveSideEffects() &&
  "cleanups within block not correctly bound!");
   PopExpressionEvaluationContext();
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -11830,7 +11830,8 @@
 assert(ExprCleanupObjects.size() ==
ExprEvalContexts.back().NumCleanupObjects &&
"Leftover temporaries in function");
-assert(!Cleanup.exprNeedsCleanups() && "Unaccounted cleanups in function");
+assert(!Cleanup.cleanupsHaveSideEffects() &&
+   "Unaccounted cleanups in function");
 assert(MaybeODRUseExprs.empty() &&
"Leftover expressions for odr-use checking");
   }


Index: clang/test/SemaCXX/pr30306.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/pr30306.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+int f(int const ) { return p; }
+
+template 
+void g(T) { int a[f(3)]; } // expected-no-diagnostics
+
+int main() {
+  g(2);
+  return 0;
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5746,7 +5746,7 @@
 
   unsigned FirstCleanup = ExprEvalContexts.back().NumCleanupObjects;
   assert(ExprCleanupObjects.size() >= FirstCleanup);
-  assert(Cleanup.exprNeedsCleanups() ||
+  assert(Cleanup.cleanupsHaveSideEffects() ||
  ExprCleanupObjects.size() == FirstCleanup);
   if (!Cleanup.exprNeedsCleanups())
 return SubExpr;
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11736,7 +11736,7 @@
 
   if (hasAnyUnrecoverableErrorsInThisFunction())
 DiscardCleanupsInEvaluationContext();
-  assert(!Cleanup.exprNeedsCleanups() &&
+  assert(!Cleanup.cleanupsHaveSideEffects() &&
  "cleanups within StmtExpr not correctly bound!");
   PopExpressionEvaluationContext();
 
@@ -12204,7 +12204,7 @@
   // Leave the expression-evaluation context.
   if (hasAnyUnrecoverableErrorsInThisFunction())
 DiscardCleanupsInEvaluationContext();
-  assert(!Cleanup.exprNeedsCleanups() &&
+  assert(!Cleanup.cleanupsHaveSideEffects() &&
  "cleanups within block not correctly bound!");
   PopExpressionEvaluationContext();
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -11830,7 +11830,8 @@
 assert(ExprCleanupObjects.size() ==
ExprEvalContexts.back().NumCleanupObjects &&
"Leftover temporaries in function");
-assert(!Cleanup.exprNeedsCleanups() && "Unaccounted cleanups in function");
+assert(!Cleanup.cleanupsHaveSideEffects() &&
+   "Unaccounted cleanups in function");
 assert(MaybeODRUseExprs.empty() &&
"Leftover expressions for odr-use checking");
   }

[PATCH] D24333: [CleanupInfo] Use cleanupsHaveSideEffects instead of exprNeedsCleanups in assertions

[Tim Shen via cfe-commits]

timshen created this revision.
timshen added a reviewer: rsmith.
timshen added a subscriber: cfe-commits.

Before r272296, the assertion was !ExprNeedsCleanups, which means that there is 
no cleanups (with dtor calls). It should still check so after r272296. This 
fixes pr30306.

https://reviews.llvm.org/D24333

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/cleanup-side-effects-assert.cpp

Index: clang/test/SemaCXX/cleanup-side-effects-assert.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cleanup-side-effects-assert.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+int f(int const ) { return p; }
+
+struct A {
+  ~A();
+};
+
+template 
+void g(T) { int a[f(3)]; } // expected-no-diagnostics
+
+int main() {
+  g(2);
+  return 0;
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5746,7 +5746,7 @@
 
   unsigned FirstCleanup = ExprEvalContexts.back().NumCleanupObjects;
   assert(ExprCleanupObjects.size() >= FirstCleanup);
-  assert(Cleanup.exprNeedsCleanups() ||
+  assert(Cleanup.cleanupsHaveSideEffects() ||
  ExprCleanupObjects.size() == FirstCleanup);
   if (!Cleanup.exprNeedsCleanups())
 return SubExpr;
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11736,7 +11736,7 @@
 
   if (hasAnyUnrecoverableErrorsInThisFunction())
 DiscardCleanupsInEvaluationContext();
-  assert(!Cleanup.exprNeedsCleanups() &&
+  assert(!Cleanup.cleanupsHaveSideEffects() &&
  "cleanups within StmtExpr not correctly bound!");
   PopExpressionEvaluationContext();
 
@@ -12204,7 +12204,7 @@
   // Leave the expression-evaluation context.
   if (hasAnyUnrecoverableErrorsInThisFunction())
 DiscardCleanupsInEvaluationContext();
-  assert(!Cleanup.exprNeedsCleanups() &&
+  assert(!Cleanup.cleanupsHaveSideEffects() &&
  "cleanups within block not correctly bound!");
   PopExpressionEvaluationContext();
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -11830,7 +11830,8 @@
 assert(ExprCleanupObjects.size() ==
ExprEvalContexts.back().NumCleanupObjects &&
"Leftover temporaries in function");
-assert(!Cleanup.exprNeedsCleanups() && "Unaccounted cleanups in function");
+assert(!Cleanup.cleanupsHaveSideEffects() &&
+   "Unaccounted cleanups in function");
 assert(MaybeODRUseExprs.empty() &&
"Leftover expressions for odr-use checking");
   }


Index: clang/test/SemaCXX/cleanup-side-effects-assert.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cleanup-side-effects-assert.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+int f(int const ) { return p; }
+
+struct A {
+  ~A();
+};
+
+template 
+void g(T) { int a[f(3)]; } // expected-no-diagnostics
+
+int main() {
+  g(2);
+  return 0;
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5746,7 +5746,7 @@
 
   unsigned FirstCleanup = ExprEvalContexts.back().NumCleanupObjects;
   assert(ExprCleanupObjects.size() >= FirstCleanup);
-  assert(Cleanup.exprNeedsCleanups() ||
+  assert(Cleanup.cleanupsHaveSideEffects() ||
  ExprCleanupObjects.size() == FirstCleanup);
   if (!Cleanup.exprNeedsCleanups())
 return SubExpr;
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11736,7 +11736,7 @@
 
   if (hasAnyUnrecoverableErrorsInThisFunction())
 DiscardCleanupsInEvaluationContext();
-  assert(!Cleanup.exprNeedsCleanups() &&
+  assert(!Cleanup.cleanupsHaveSideEffects() &&
  "cleanups within StmtExpr not correctly bound!");
   PopExpressionEvaluationContext();
 
@@ -12204,7 +12204,7 @@
   // Leave the expression-evaluation context.
   if (hasAnyUnrecoverableErrorsInThisFunction())
 DiscardCleanupsInEvaluationContext();
-  assert(!Cleanup.exprNeedsCleanups() &&
+  assert(!Cleanup.cleanupsHaveSideEffects() &&
  "cleanups within block not correctly bound!");
   PopExpressionEvaluationContext();
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -11830,7 +11830,8 @@
 assert(ExprCleanupObjects.size() ==
ExprEvalContexts.back().NumCleanupObjects &&
"Leftover temporaries in function");
-