[PATCH] D53860: [SemaCXX] Don't check base's dtor is accessible

[Brian Gesiak via Phabricator via cfe-commits]

modocache added a comment.

Here's a compiler explorer link demonstrating that GCC 8.2 and Clang 7.0.0 
compile the example code, but Clang trunk emits an error: 
https://godbolt.org/z/l3baI_

I realize the proposed "fix" here isn't likely to be exactly correct, but I 
thought it could be a good starting point. I'd also like to confirm that my 
example is, in fact, valid C++ code.

Any and all reviews appreciated!


Repository:
  rC Clang

https://reviews.llvm.org/D53860



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

[PATCH] D53860: [SemaCXX] Don't check base's dtor is accessible

[Brian Gesiak via Phabricator via cfe-commits]

modocache created this revision.
modocache added reviewers: rsmith, rjmccall, doug.gregor, ahatanak.

In https://reviews.llvm.org/D45898?id=157373, @rsmith advises to move a
`checkMemberDestructor` into `InitListChecker::CheckStructUnionTypes`,
to check base classes for an accessible destructor when performing
aggregate initialization. However, in so doing, the following C++ code
now fails to compile:

  class Base { protected: ~Base() = default; };
  struct Derived : Base {};
  void foo() { Derived d = {}; }

GCC 8.2 and Clang 7.0.0 both treat this C++ code as valid, whereas
Clang trunk emits the following error diagnostic:

  :3:27: error: temporary of type 'Base' has protected destructor
  void foo() { Derived d = {}; }
^
  :1:25: note: declared protected here
  class Base { protected: ~Base() = default; };
  ^

I think Clang trunk is wrong about the validity of this C++ code: my
understanding of the C++17 standard is that the `Base` destructor need
only be accessible from within the context of `Derived`, and not at the
context in which `Derived` is aggregate initialized within the `foo`
function.

Add a test for the case above, and modify the destructor access check
within `InitListChecker::CheckStructUnionTypes` to check only that the
derived class has an accessible destructor.

Test Plan: `check-clang`


Repository:
  rC Clang

https://reviews.llvm.org/D53860

Files:
  lib/Sema/SemaInit.cpp
  test/SemaCXX/aggregate-initialization.cpp


Index: test/SemaCXX/aggregate-initialization.cpp
===
--- test/SemaCXX/aggregate-initialization.cpp
+++ test/SemaCXX/aggregate-initialization.cpp
@@ -219,7 +219,7 @@
  struct S0 { int f; ~S0() = delete; }; // expected-note {{'~S0' has been 
explicitly marked deleted here}}
 
 // Check destructor of base class.
-struct S3 : S0 {};
+struct S3 : S0 {}; // expected-note {{destructor of 'S3' is implicitly 
deleted because base class 'ElementDestructor::BaseDestructor::S0' has a 
deleted destructor}}
 
 void test3() {
   S3 s3 = {1}; // expected-error {{attempt to use a deleted function}}
@@ -233,4 +233,10 @@
   struct B { B(); A a; };
   struct C { B b; };
   C c = { B() };
+
+  // Zylong's destructor doesn't have to be accessible from the context of
+  // Yerbl's initialization.
+  struct Zylong { protected: ~Zylong(); };
+  struct Yerbl : Zylong {};
+  Yerbl y = {};
 }
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -1954,7 +1954,7 @@
 }
 
 if (!VerifyOnly)
-  if (hasAccessibleDestructor(Base.getType(), InitLoc, SemaRef)) {
+  if (hasAccessibleDestructor(DeclType, InitLoc, SemaRef)) {
 hadError = true;
 return;
   }


Index: test/SemaCXX/aggregate-initialization.cpp
===
--- test/SemaCXX/aggregate-initialization.cpp
+++ test/SemaCXX/aggregate-initialization.cpp
@@ -219,7 +219,7 @@
  struct S0 { int f; ~S0() = delete; }; // expected-note {{'~S0' has been explicitly marked deleted here}}
 
 // Check destructor of base class.
-struct S3 : S0 {};
+struct S3 : S0 {}; // expected-note {{destructor of 'S3' is implicitly deleted because base class 'ElementDestructor::BaseDestructor::S0' has a deleted destructor}}
 
 void test3() {
   S3 s3 = {1}; // expected-error {{attempt to use a deleted function}}
@@ -233,4 +233,10 @@
   struct B { B(); A a; };
   struct C { B b; };
   C c = { B() };
+
+  // Zylong's destructor doesn't have to be accessible from the context of
+  // Yerbl's initialization.
+  struct Zylong { protected: ~Zylong(); };
+  struct Yerbl : Zylong {};
+  Yerbl y = {};
 }
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -1954,7 +1954,7 @@
 }
 
 if (!VerifyOnly)
-  if (hasAccessibleDestructor(Base.getType(), InitLoc, SemaRef)) {
+  if (hasAccessibleDestructor(DeclType, InitLoc, SemaRef)) {
 hadError = true;
 return;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53787: [Sema] Use proper visibility for global new and delete declarations

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

In https://reviews.llvm.org/D53787#1279899, @rsmith wrote:

> Replacing the global new and delete is supposed to be a whole-program 
> operation (you only get one global allocator). Otherwise you couldn't 
> allocate memory in one DSO and deallocate it in another. (And nor could 
> inline functions etc.)
>
> If you really want to do this weird thing, and you understand what you're 
> getting yourself into, I don't see a problem with having a dedicated flag for 
> it, but don't break all existing users of -fvisibility=.


That sounds reasonable, do you have any suggestion for the name of such flag?


Repository:
  rC Clang

https://reviews.llvm.org/D53787



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

[PATCH] D53856: [analyzer] Put llvm.Conventions back in alpha

[Henry Wong via Phabricator via cfe-commits]

MTC added a comment.

In addition, `clang/lib/StaticAnalyzer/README.txt`  and other related docs in 
`clang/lib/docs/analyzer` are also out of date.




Comment at: www/analyzer/alpha_checks.html:570
+
+  A StringRef should not be bound to a temporary std::string
+  whose lifetime is shorter than the StringRef's.

Like `StringRef`, `ArrayRef` may have similar issues. After this patch landed, 
maybe we can enhance this checker step by step. By that time, we may need the 
ideas of the heavy users of the LLVM library.


https://reviews.llvm.org/D53856



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

[PATCH] D53787: [Sema] Use proper visibility for global new and delete declarations

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

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

Replacing the global new and delete is supposed to be a whole-program operation 
(you only get one global allocator). Otherwise you couldn't allocate memory in 
one DSO and deallocate it in another. (And nor could inline functions etc.)

If you really want to do this weird thing, and you understand what you're 
getting yourself into, I don't see a problem with having a dedicated flag for 
it, but don't break all existing users of -fvisibility=.


Repository:
  rC Clang

https://reviews.llvm.org/D53787



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

[PATCH] D53837: [clang] Move two utility functions into SourceManager

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

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

Thanks, LG. I bet there's a bunch more places we could change to call these two.


Repository:
  rC Clang

https://reviews.llvm.org/D53837



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

[PATCH] D53856: [analyzer] Put llvm.Conventions back in alpha

[Artem Dergachev via Phabricator via cfe-commits]

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

I've actually never noticed this checker until you started updating the website 
:)

This might be also covered by @rnkovacs's string buffer escape checker - either 
already or eventually, it'll become just yet another string view API that the 
checker supports.




Comment at: test/Analysis/llvm-conventions.cpp:8
+
+using size_t = unsigned long;
+using size_type = size_t;

I think we should try to re-use (and possibly extend) 
`test/Analysis/inputs/system-header-simulator-cxx.h`.


https://reviews.llvm.org/D53856



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

r345562 - PR23833, DR2140: an lvalue-to-rvalue conversion on a glvalue of type

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Oct 29 19:02:49 2018
New Revision: 345562

URL: http://llvm.org/viewvc/llvm-project?rev=345562=rev
Log:
PR23833, DR2140: an lvalue-to-rvalue conversion on a glvalue of type
nullptr_t does not access memory.

We now reuse CK_NullToPointer to represent a conversion from a glvalue
of type nullptr_t to a prvalue of nullptr_t where necessary.

Modified:
cfe/trunk/lib/AST/Expr.cpp
cfe/trunk/lib/CodeGen/CGExprAgg.cpp
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
cfe/trunk/test/Analysis/nullptr.cpp
cfe/trunk/test/CXX/drs/dr21xx.cpp
cfe/trunk/test/CodeGenCXX/nullptr.cpp
cfe/trunk/www/cxx_dr_status.html

Modified: cfe/trunk/lib/AST/Expr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=345562=345561=345562=diff
==
--- cfe/trunk/lib/AST/Expr.cpp (original)
+++ cfe/trunk/lib/AST/Expr.cpp Mon Oct 29 19:02:49 2018
@@ -1816,6 +1816,11 @@ ImplicitCastExpr *ImplicitCastExpr::Crea
   void *Buffer =
   C.Allocate(totalSizeToAlloc(
   PathSize ? 1 : 0, PathSize));
+  // Per C++ [conv.lval]p3, lvalue-to-rvalue conversions on class and
+  // std::nullptr_t have special semantics not captured by CK_LValueToRValue.
+  assert((Kind != CK_LValueToRValue ||
+  !(T->isNullPtrType() || T->getAsCXXRecordDecl())) &&
+ "invalid type for lvalue-to-rvalue conversion");
   ImplicitCastExpr *E =
 new (Buffer) ImplicitCastExpr(T, Kind, Operand, PathSize, VK);
   if (PathSize)

Modified: cfe/trunk/lib/CodeGen/CGExprAgg.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprAgg.cpp?rev=345562=345561=345562=diff
==
--- cfe/trunk/lib/CodeGen/CGExprAgg.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprAgg.cpp Mon Oct 29 19:02:49 2018
@@ -1300,7 +1300,8 @@ static bool isSimpleZero(const Expr *E,
   // (int*)0 - Null pointer expressions.
   if (const CastExpr *ICE = dyn_cast(E))
 return ICE->getCastKind() == CK_NullToPointer &&
-CGF.getTypes().isPointerZeroInitializable(E->getType());
+   CGF.getTypes().isPointerZeroInitializable(E->getType()) &&
+   !E->HasSideEffects(CGF.getContext());
   // '\0'
   if (const CharacterLiteral *CL = dyn_cast(E))
 return CL->getValue() == 0;

Modified: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprScalar.cpp?rev=345562=345561=345562=diff
==
--- cfe/trunk/lib/CodeGen/CGExprScalar.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp Mon Oct 29 19:02:49 2018
@@ -1895,14 +1895,14 @@ Value *ScalarExprEmitter::VisitCastExpr(
 
   case CK_NullToPointer:
 if (MustVisitNullValue(E))
-  (void) Visit(E);
+  CGF.EmitIgnoredExpr(E);
 
 return CGF.CGM.getNullPointer(cast(ConvertType(DestTy)),
   DestTy);
 
   case CK_NullToMemberPointer: {
 if (MustVisitNullValue(E))
-  (void) Visit(E);
+  CGF.EmitIgnoredExpr(E);
 
 const MemberPointerType *MPT = CE->getType()->getAs();
 return CGF.CGM.getCXXABI().EmitNullMemberPointer(MPT);

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=345562=345561=345562=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon Oct 29 19:02:49 2018
@@ -623,8 +623,11 @@ ExprResult Sema::DefaultLvalueConversion
   if (E->getType().getObjCLifetime() == Qualifiers::OCL_Weak)
 Cleanup.setExprNeedsCleanups(true);
 
-  ExprResult Res = ImplicitCastExpr::Create(Context, T, CK_LValueToRValue, E,
-nullptr, VK_RValue);
+  // C++ [conv.lval]p3:
+  //   If T is cv std::nullptr_t, the result is a null pointer constant.
+  CastKind CK = T->isNullPtrType() ? CK_NullToPointer : CK_LValueToRValue;
+  ExprResult Res =
+  ImplicitCastExpr::Create(Context, T, CK, E, nullptr, VK_RValue);
 
   // C11 6.3.2.1p2:
   //   ... if the lvalue has atomic type, the value has the non-atomic version

Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=345562=345561=345562=diff
==
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Mon Oct 29 19:02:49 2018
@@ -7644,9 +7644,13 @@ InitializationSequence::Perform(Sema ,
 
 case SK_LValueToRValue: {
   assert(CurInit.get()->isGLValue() && "cannot load from a prvalue");
-  CurInit = ImplicitCastExpr::Create(S.Context, Step->Type,
-  

[PATCH] D53856: [analyzer] Put llvm.Conventions back in alpha

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/LLVMConventionsChecker.cpp:193-195
 static bool AllocatesMemory(QualType T) {
   return IsStdVector(T) || IsStdString(T) || IsSmallVector(T);
 }

The `StringRef`part of this checker doesn't do a thing for me at all (even 
after a good amount of trying to make it work), but this part of the checker 
suffers a lot too.

I'm sure we can some up with better ways for this function (and every single 
one in this file to be honest) now that almost a decade has passed.


https://reviews.llvm.org/D53856



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

[PATCH] D53069: [analyzer][www] Update avaible_checks.html

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus updated this revision to Diff 171624.
Szelethus added a comment.

Excuse my informality, but `llvm.Conventions` fell flat on its face in my eyes 
(details: https://reviews.llvm.org/D53856), so I'm no longer insisting on 
including it on this page.


https://reviews.llvm.org/D53069

Files:
  www/analyzer/available_checks.html

Index: www/analyzer/available_checks.html
===
--- www/analyzer/available_checks.html
+++ www/analyzer/available_checks.html
@@ -43,6 +43,7 @@
 OS X Checkers perform Objective-C-specific checks and check the use of Apple's SDKs (OS X and iOS)
 Security Checkers check for insecure API usage and perform checks based on the CERT Secure Coding Standards
 Unix Checkers check the use of Unix and POSIX APIs
+Variable Argument Checkers
 
 
 
@@ -369,6 +370,25 @@
 Name, DescriptionExample
 
 
+
+
+
+cplusplus.InnerPointer
+(C++)
+Check for inner pointers of C++ containers used after re/deallocation.
+
+
+
+void log(const char *str);
+
+void test(int value) {
+  const char *msg = std::to_string(value).c_str();
+  // msg points to the buffer of a temporary that is now destroyed
+  log(msg);  // warn: inner pointer of container used after re/deallocation
+}
+
+
+
 
 cplusplus.NewDelete
 (C++)
@@ -435,6 +455,7 @@
 } // warn
 
 
+
 
 
 
@@ -458,6 +479,7 @@
 
 
 
+
 
 Nullability Checkers
 
@@ -535,6 +557,21 @@
 }
 
 
+
+
+nullability.NullableReturnedFromNonnull
+(ObjC)
+Warns when a nullable pointer is returned from a function that has _Nonnull return type.
+
+
+typedef struct Dummy { int val; } Dummy;
+
+Dummy *_Nonnull test(Dummy *_Nullable a) {
+  Dummy *p = a;
+  return p; // warn
+}
+
+
 
 
 
@@ -610,6 +647,95 @@
 [alarmStateLabel setText:alarmText];
 
 
+
+
+optin.performance.GCDAntipattern
+(ObjC)
+This checker finds a common performance anti-pattern in a code that uses Grand
+Central dispatch. The anti-pattern involves emulating a synchronous call from an
+asynchronous API using semaphores, as in the snippet below, where the
+requestCurrentTaskName function makes an XPC call and then uses the
+semaphore to block until the XPC call returns (example 1.).
+Usage of such a pattern in production code running on the main thread is
+discouraged, as the main queue gets blocked waiting for the background queue,
+which could be running at a lower priority, and unnecessary threads are spawned
+in the process.
+In order to avoid the anti-pattern, the available alternatives are:
+
+  Use the synchronous version of the API, if available (as seen on example
+  2.)
+  Alternatively, the API can be used in the asynchronous way.
+
+
+
+
+// Example 1.
++ (NSString *)requestCurrentTaskName {
+__block NSString *taskName = nil;
+dispatch_semaphore_t sema = dispatch_semaphore_create(0);
+NSXPCConnection *connection = [[NSXPCConnection alloc] initWithServiceName:@"MyConnection"];
+id remoteObjectProxy = connection.remoteObjectProxy;
+[remoteObjectProxy requestCurrentTaskName:^(NSString *task) {
+taskName = task;
+dispatch_semaphore_signal(sema);
+}];
+dispatch_semaphore_wait(sema, DISPATCH_TIME_FOREVER);
+return taskName;
+}
+
+
+// Example 2.
++ (NSString *)requestCurrentTaskName {
+__block NSString *taskName = nil;
+NSXPCConnection *connection = [[NSXPCConnection alloc] initWithServiceName:@"MyConnection"];
+id remoteObjectProxy = [connection synchronousRemoteObjectProxyWithErrorHandler:^(NSError *error) {
+  NSLog(@"Error = %@", error);
+
+}];
+[remoteObjectProxy requestCurrentTaskName:^(NSString *task) {
+taskName = task;
+}];
+return taskName;
+}
+
+
+
+
+optin.performance.Padding
+(C)
+Check for excessively padded structs.
+
+
+
+class PaddedA { // warn: excessive padding
+  char c1;
+  int i;
+  char c2;
+};
+
+
+
+
+optin.portability.UnixAPI
+(C)
+Finds implementation-defined behavior in UNIX/Posix functions.
+
+calloc
+malloc
+realloc
+reallocf
+alloca, __builtin_alloca
+__builtin_alloca_with_align
+valloc
+
+
+
+void *f(int n) {
+  return malloc(n * 0 * sizeof(int)); // warn: Call to 'malloc' has an
+  //   allocation size of 0 bytes
+}
+
+
 
 
 
@@ -649,6 +775,9 @@
 
 
 
+
+
+
 
 osx.SecKeychainAPI
 (C)
@@ -732,7 +861,8 @@
 
 osx.cocoa.AtSync
 (ObjC)
-Check for nil pointers used as mutexes for @synchronized.
+Check for nil pointers used as mutexes for @synchronized.
+
 
 
 void test(id x) {
@@ -748,6 +878,38 @@
 
 
 
+
+osx.cocoa.AutoreleaseWrite
+(ObjC)
+Under ARC, function parameters which are pointers to pointers (e.g.
+NSError **) are __autoreleasing. Writing to such
+parameters inside autoreleasing pools might crash whenever the parameter
+outlives the pool. Detecting such crashes may be difficult, as usage of
+autorelease pool is usually hidden inside the called functions implementation.
+
+
+
+BOOL writeToErrorWithIterator(NSError *__autoreleasing* error, NSArray *a) { [a 

[PATCH] D53856: [analyzer] Put llvm.Conventions back in alpha

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus updated this revision to Diff 171623.
Szelethus added a comment.

Added an entry to the website.


https://reviews.llvm.org/D53856

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/LLVMConventionsChecker.cpp
  test/Analysis/llvm-conventions.cpp
  www/analyzer/alpha_checks.html

Index: www/analyzer/alpha_checks.html
===
--- www/analyzer/alpha_checks.html
+++ www/analyzer/alpha_checks.html
@@ -27,6 +27,7 @@
 Clone Alpha Checkers
 Core Alpha Checkers
 C++ Alpha Checkers
+LLVM Checkers
 Variable Argument Alpha Checkers
 Dead Code Alpha Checkers
 OS X Alpha Checkers
@@ -554,6 +555,31 @@
 
 
 
+
+LLVM Checkers
+
+
+Name, DescriptionExample
+
+
+
+alpha.llvm.Conventions
+(C)
+Check code for LLVM codebase conventions:
+
+  A StringRef should not be bound to a temporary std::string
+  whose lifetime is shorter than the StringRef's.
+  Clang AST nodes should not have fields that can allocate memory.
+
+
+
+
+
+
+
+
+
+
 
 OS X Alpha Checkers
 
Index: test/Analysis/llvm-conventions.cpp
===
--- /dev/null
+++ test/Analysis/llvm-conventions.cpp
@@ -0,0 +1,271 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.llvm.Conventions \
+// RUN:   -std=c++14 -verify  %s
+
+//===--===//
+// Forward declarations for StringRef tests.
+//===--===//
+
+using size_t = unsigned long;
+using size_type = size_t;
+
+namespace std {
+
+using nullptr_t = decltype(nullptr);
+
+template 
+struct pair { T1 first; T2 second; };
+
+template
+struct enable_if {};
+
+template
+struct enable_if { typedef T type; };
+
+template
+struct is_same { const static bool value; };
+
+template 
+struct numeric_limits { const static bool is_signed; };
+
+template 
+class basic_string {
+public:
+  basic_string();
+  basic_string(const CharT *s);
+
+  ~basic_string();
+  void clear();
+
+  basic_string =(const basic_string );
+  basic_string +=(const basic_string );
+
+  const CharT *c_str() const;
+  const CharT *data() const;
+  CharT *data();
+
+  basic_string (size_type count, CharT ch);
+  basic_string (size_type count, CharT ch);
+  basic_string (size_type index, size_type count);
+  basic_string (size_type index, size_type count, CharT ch);
+  basic_string (size_type pos, size_type count, const basic_string );
+  void pop_back();
+  void push_back(CharT ch);
+  void reserve(size_type new_cap);
+  void resize(size_type count);
+  void shrink_to_fit();
+  void swap(basic_string );
+};
+
+typedef basic_string string;
+typedef basic_string wstring;
+typedef basic_string u16string;
+typedef basic_string u32string;
+
+} // end of namespace std
+
+namespace llvm {
+
+template 
+struct iterator_range;
+
+template 
+struct function_ref;
+
+struct hash_code;
+
+template 
+struct SmallVectorImpl;
+
+struct APInt;
+
+class StringRef {
+public:
+  static const size_t npos = ~size_t(0);
+  using iterator = const char *;
+  using const_iterator = const char *;
+  using size_type = size_t;
+
+  /*implicit*/ StringRef() = default;
+  StringRef(std::nullptr_t) = delete;
+  /*implicit*/ StringRef(const char *Str);
+  /*implicit*/ constexpr StringRef(const char *data, size_t length);
+  /*implicit*/ StringRef(const std::string );
+
+  static StringRef withNullAsEmpty(const char *data);
+  iterator begin() const;
+  iterator end() const;
+  const unsigned char *bytes_begin() const;
+  const unsigned char *bytes_end() const;
+  iterator_range bytes() const;
+  const char *data() const;
+  bool empty() const;
+  size_t size() const;
+  char front() const;
+  char back() const;
+  template 
+  StringRef copy(Allocator ) const;
+  bool equals(StringRef RHS) const;
+  bool equals_lower(StringRef RHS) const;
+  int compare(StringRef RHS) const;
+  int compare_lower(StringRef RHS) const;
+  int compare_numeric(StringRef RHS) const;
+  unsigned edit_distance(StringRef Other, bool AllowReplacements = true,
+ unsigned MaxEditDistance = 0) const;
+  std::string str() const;
+  char operator[](size_t Index) const;
+  template 
+  typename std::enable_if::value,
+  StringRef>::type &
+  operator=(T &) = delete;
+  operator std::string() const;
+  bool startswith(StringRef Prefix) const;
+  bool startswith_lower(StringRef Prefix) const;
+  bool endswith(StringRef Suffix) const;
+  bool endswith_lower(StringRef Suffix) const;
+  size_t find(char C, size_t From = 0) const;
+  size_t find_lower(char C, size_t From = 0) const;
+  size_t find_if(function_ref F, size_t From = 0) const;
+  size_t find_if_not(function_ref F, size_t From = 0) const;
+  size_t find(StringRef Str, size_t From = 0) const;
+  size_t find_lower(StringRef Str, size_t From = 0) const;
+  size_t rfind(char C, size_t From = npos) const;
+  size_t 

[PATCH] D53856: [analyzer] Put llvm.Conventions back in alpha

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus created this revision.
Szelethus added reviewers: george.karpenkov, NoQ, xazax.hun, MTC.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, rnkovacs, szepet, whisperity.

Interestingly, this many year old (when I last looked I remember 2010ish) 
checker was committed without any tests, so I thought I'd implement them, but I 
was shocked to see how I barely managed to get it working. The code is severely 
outdated, I'm not even sure it has ever been used, so I'd propose to move it 
back into alpha, and possibly even remove it.


Repository:
  rC Clang

https://reviews.llvm.org/D53856

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/LLVMConventionsChecker.cpp
  test/Analysis/llvm-conventions.cpp

Index: test/Analysis/llvm-conventions.cpp
===
--- /dev/null
+++ test/Analysis/llvm-conventions.cpp
@@ -0,0 +1,271 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.llvm.Conventions \
+// RUN:   -std=c++14 -verify  %s
+
+//===--===//
+// Forward declarations for StringRef tests.
+//===--===//
+
+using size_t = unsigned long;
+using size_type = size_t;
+
+namespace std {
+
+using nullptr_t = decltype(nullptr);
+
+template 
+struct pair { T1 first; T2 second; };
+
+template
+struct enable_if {};
+
+template
+struct enable_if { typedef T type; };
+
+template
+struct is_same { const static bool value; };
+
+template 
+struct numeric_limits { const static bool is_signed; };
+
+template 
+class basic_string {
+public:
+  basic_string();
+  basic_string(const CharT *s);
+
+  ~basic_string();
+  void clear();
+
+  basic_string =(const basic_string );
+  basic_string +=(const basic_string );
+
+  const CharT *c_str() const;
+  const CharT *data() const;
+  CharT *data();
+
+  basic_string (size_type count, CharT ch);
+  basic_string (size_type count, CharT ch);
+  basic_string (size_type index, size_type count);
+  basic_string (size_type index, size_type count, CharT ch);
+  basic_string (size_type pos, size_type count, const basic_string );
+  void pop_back();
+  void push_back(CharT ch);
+  void reserve(size_type new_cap);
+  void resize(size_type count);
+  void shrink_to_fit();
+  void swap(basic_string );
+};
+
+typedef basic_string string;
+typedef basic_string wstring;
+typedef basic_string u16string;
+typedef basic_string u32string;
+
+} // end of namespace std
+
+namespace llvm {
+
+template 
+struct iterator_range;
+
+template 
+struct function_ref;
+
+struct hash_code;
+
+template 
+struct SmallVectorImpl;
+
+struct APInt;
+
+class StringRef {
+public:
+  static const size_t npos = ~size_t(0);
+  using iterator = const char *;
+  using const_iterator = const char *;
+  using size_type = size_t;
+
+  /*implicit*/ StringRef() = default;
+  StringRef(std::nullptr_t) = delete;
+  /*implicit*/ StringRef(const char *Str);
+  /*implicit*/ constexpr StringRef(const char *data, size_t length);
+  /*implicit*/ StringRef(const std::string );
+
+  static StringRef withNullAsEmpty(const char *data);
+  iterator begin() const;
+  iterator end() const;
+  const unsigned char *bytes_begin() const;
+  const unsigned char *bytes_end() const;
+  iterator_range bytes() const;
+  const char *data() const;
+  bool empty() const;
+  size_t size() const;
+  char front() const;
+  char back() const;
+  template 
+  StringRef copy(Allocator ) const;
+  bool equals(StringRef RHS) const;
+  bool equals_lower(StringRef RHS) const;
+  int compare(StringRef RHS) const;
+  int compare_lower(StringRef RHS) const;
+  int compare_numeric(StringRef RHS) const;
+  unsigned edit_distance(StringRef Other, bool AllowReplacements = true,
+ unsigned MaxEditDistance = 0) const;
+  std::string str() const;
+  char operator[](size_t Index) const;
+  template 
+  typename std::enable_if::value,
+  StringRef>::type &
+  operator=(T &) = delete;
+  operator std::string() const;
+  bool startswith(StringRef Prefix) const;
+  bool startswith_lower(StringRef Prefix) const;
+  bool endswith(StringRef Suffix) const;
+  bool endswith_lower(StringRef Suffix) const;
+  size_t find(char C, size_t From = 0) const;
+  size_t find_lower(char C, size_t From = 0) const;
+  size_t find_if(function_ref F, size_t From = 0) const;
+  size_t find_if_not(function_ref F, size_t From = 0) const;
+  size_t find(StringRef Str, size_t From = 0) const;
+  size_t find_lower(StringRef Str, size_t From = 0) const;
+  size_t rfind(char C, size_t From = npos) const;
+  size_t rfind_lower(char C, size_t From = npos) const;
+  size_t rfind(StringRef Str) const;
+  size_t rfind_lower(StringRef Str) const;
+  size_t find_first_of(char C, size_t From = 0) const;
+  size_t find_first_of(StringRef Chars, size_t From = 0) const;
+  size_t 

[PATCH] D53206: Allow padding checker to traverse simple class hierarchies

[Alexander Shaposhnikov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC345558: [analyzer] Allow padding checker to traverse simple 
class hierarchies (authored by alexshap, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D53206

Files:
  lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
  test/Analysis/padding_inherit.cpp

Index: test/Analysis/padding_inherit.cpp
===
--- test/Analysis/padding_inherit.cpp
+++ test/Analysis/padding_inherit.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_analyze_cc1 -std=c++14 -analyzer-checker=optin.performance -analyzer-config optin.performance.Padding:AllowedPad=20 -verify %s
+
+// A class that has no fields and one base class should visit that base class
+// instead. Note that despite having excess padding of 2, this is flagged
+// because of its usage in an array of 100 elements below (`ais').
+// TODO: Add a note to the bug report with BugReport::addNote() to mention the
+// variable using the class and also mention what class is inherting from what.
+// expected-warning@+1{{Excessive padding in 'struct FakeIntSandwich'}}
+struct FakeIntSandwich {
+  char c1;
+  int i;
+  char c2;
+};
+
+struct AnotherIntSandwich : FakeIntSandwich { // no-warning
+};
+
+// But we don't yet support multiple base classes.
+struct IntSandwich {};
+struct TooManyBaseClasses : FakeIntSandwich, IntSandwich { // no-warning
+};
+
+AnotherIntSandwich ais[100];
+
+struct Empty {};
+struct DoubleEmpty : Empty { // no-warning
+Empty e;
+};
Index: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
+++ lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
@@ -75,6 +75,20 @@
 if (shouldSkipDecl(RD))
   return;
 
+// TODO: Figure out why we are going through declarations and not only
+// definitions.
+if (!(RD = RD->getDefinition()))
+  return;
+
+// This is the simplest correct case: a class with no fields and one base
+// class. Other cases are more complicated because of how the base classes
+// & fields might interact, so we don't bother dealing with them.
+// TODO: Support other combinations of base classes and fields.
+if (auto *CXXRD = dyn_cast(RD))
+  if (CXXRD->field_empty() && CXXRD->getNumBases() == 1)
+return visitRecord(CXXRD->bases().begin()->getType()->getAsRecordDecl(),
+   PadMultiplier);
+
 auto  = RD->getASTContext();
 const ASTRecordLayout  = ASTContext.getASTRecordLayout(RD);
 assert(llvm::isPowerOf2_64(RL.getAlignment().getQuantity()));
@@ -112,12 +126,15 @@
 if (RT == nullptr)
   return;
 
-// TODO: Recurse into the fields and base classes to see if any
-// of those have excess padding.
+// TODO: Recurse into the fields to see if they have excess padding.
 visitRecord(RT->getDecl(), Elts);
   }
 
   bool shouldSkipDecl(const RecordDecl *RD) const {
+// TODO: Figure out why we are going through declarations and not only
+// definitions.
+if (!(RD = RD->getDefinition()))
+  return true;
 auto Location = RD->getLocation();
 // If the construct doesn't have a source file, then it's not something
 // we want to diagnose.
@@ -132,13 +149,14 @@
 // Not going to attempt to optimize unions.
 if (RD->isUnion())
   return true;
-// How do you reorder fields if you haven't got any?
-if (RD->field_empty())
-  return true;
 if (auto *CXXRD = dyn_cast(RD)) {
   // Tail padding with base classes ends up being very complicated.
-  // We will skip objects with base classes for now.
-  if (CXXRD->getNumBases() != 0)
+  // We will skip objects with base classes for now, unless they do not
+  // have fields.
+  // TODO: Handle more base class scenarios.
+  if (!CXXRD->field_empty() && CXXRD->getNumBases() != 0)
+return true;
+  if (CXXRD->field_empty() && CXXRD->getNumBases() != 1)
 return true;
   // Virtual bases are complicated, skipping those for now.
   if (CXXRD->getNumVBases() != 0)
@@ -150,6 +168,10 @@
   if (CXXRD->getTypeForDecl()->isInstantiationDependentType())
 return true;
 }
+// How do you reorder fields if you haven't got any?
+else if (RD->field_empty())
+  return true;
+
 auto IsTrickyField = [](const FieldDecl *FD) -> bool {
   // Bitfield layout is hard.
   if (FD->isBitField())
@@ -323,7 +345,7 @@
 BR->emitReport(std::move(Report));
   }
 };
-}
+} // namespace
 
 void ento::registerPaddingChecker(CheckerManager ) {
   Mgr.registerChecker();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r345558 - [analyzer] Allow padding checker to traverse simple class hierarchies

[Alexander Shaposhnikov via cfe-commits]

Author: alexshap
Date: Mon Oct 29 18:20:37 2018
New Revision: 345558

URL: http://llvm.org/viewvc/llvm-project?rev=345558=rev
Log:
[analyzer] Allow padding checker to traverse simple class hierarchies

The existing padding checker skips classes that have any base classes. 
This patch allows the checker to traverse very simple cases: 
classes that have no fields and have exactly one base class. 
This is important mostly in the case of array declarations.

Patch by Max Bernstein!

Test plan: make check-all

Differential revision: https://reviews.llvm.org/D53206

Added:
cfe/trunk/test/Analysis/padding_inherit.cpp
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp?rev=345558=345557=345558=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp Mon Oct 29 
18:20:37 2018
@@ -75,6 +75,20 @@ public:
 if (shouldSkipDecl(RD))
   return;
 
+// TODO: Figure out why we are going through declarations and not only
+// definitions.
+if (!(RD = RD->getDefinition()))
+  return;
+
+// This is the simplest correct case: a class with no fields and one base
+// class. Other cases are more complicated because of how the base classes
+// & fields might interact, so we don't bother dealing with them.
+// TODO: Support other combinations of base classes and fields.
+if (auto *CXXRD = dyn_cast(RD))
+  if (CXXRD->field_empty() && CXXRD->getNumBases() == 1)
+return 
visitRecord(CXXRD->bases().begin()->getType()->getAsRecordDecl(),
+   PadMultiplier);
+
 auto  = RD->getASTContext();
 const ASTRecordLayout  = ASTContext.getASTRecordLayout(RD);
 assert(llvm::isPowerOf2_64(RL.getAlignment().getQuantity()));
@@ -112,12 +126,15 @@ public:
 if (RT == nullptr)
   return;
 
-// TODO: Recurse into the fields and base classes to see if any
-// of those have excess padding.
+// TODO: Recurse into the fields to see if they have excess padding.
 visitRecord(RT->getDecl(), Elts);
   }
 
   bool shouldSkipDecl(const RecordDecl *RD) const {
+// TODO: Figure out why we are going through declarations and not only
+// definitions.
+if (!(RD = RD->getDefinition()))
+  return true;
 auto Location = RD->getLocation();
 // If the construct doesn't have a source file, then it's not something
 // we want to diagnose.
@@ -132,13 +149,14 @@ public:
 // Not going to attempt to optimize unions.
 if (RD->isUnion())
   return true;
-// How do you reorder fields if you haven't got any?
-if (RD->field_empty())
-  return true;
 if (auto *CXXRD = dyn_cast(RD)) {
   // Tail padding with base classes ends up being very complicated.
-  // We will skip objects with base classes for now.
-  if (CXXRD->getNumBases() != 0)
+  // We will skip objects with base classes for now, unless they do not
+  // have fields.
+  // TODO: Handle more base class scenarios.
+  if (!CXXRD->field_empty() && CXXRD->getNumBases() != 0)
+return true;
+  if (CXXRD->field_empty() && CXXRD->getNumBases() != 1)
 return true;
   // Virtual bases are complicated, skipping those for now.
   if (CXXRD->getNumVBases() != 0)
@@ -150,6 +168,10 @@ public:
   if (CXXRD->getTypeForDecl()->isInstantiationDependentType())
 return true;
 }
+// How do you reorder fields if you haven't got any?
+else if (RD->field_empty())
+  return true;
+
 auto IsTrickyField = [](const FieldDecl *FD) -> bool {
   // Bitfield layout is hard.
   if (FD->isBitField())
@@ -323,7 +345,7 @@ public:
 BR->emitReport(std::move(Report));
   }
 };
-}
+} // namespace
 
 void ento::registerPaddingChecker(CheckerManager ) {
   Mgr.registerChecker();

Added: cfe/trunk/test/Analysis/padding_inherit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/padding_inherit.cpp?rev=345558=auto
==
--- cfe/trunk/test/Analysis/padding_inherit.cpp (added)
+++ cfe/trunk/test/Analysis/padding_inherit.cpp Mon Oct 29 18:20:37 2018
@@ -0,0 +1,28 @@
+// RUN: %clang_analyze_cc1 -std=c++14 -analyzer-checker=optin.performance 
-analyzer-config optin.performance.Padding:AllowedPad=20 -verify %s
+
+// A class that has no fields and one base class should visit that base class
+// instead. Note that despite having excess padding of 2, this is flagged
+// because of its usage in an array of 100 elements below (`ais').
+// TODO: Add a note to the bug report with BugReport::addNote() to mention the
+// variable using the class and also mention what class is inherting 

[PATCH] D53854: [Driver] Use -push-/-pop-state and -as-needed for libc++ on Fuchsia

[Petr Hosek via Phabricator via cfe-commits]

phosek created this revision.
phosek added a reviewer: mcgrathr.
Herald added a reviewer: EricWF.
Herald added a subscriber: cfe-commits.

This avoids introducing unnecessary DT_NEEDED entries when using
C++ driver for linking C code or C++ code that doesn't use C++
standard library.


Repository:
  rC Clang

https://reviews.llvm.org/D53854

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/test/Driver/fuchsia.cpp


Index: clang/test/Driver/fuchsia.cpp
===
--- clang/test/Driver/fuchsia.cpp
+++ clang/test/Driver/fuchsia.cpp
@@ -15,7 +15,11 @@
 // CHECK-NOT: crti.o
 // CHECK-NOT: crtbegin.o
 // CHECK: "-L[[SYSROOT]]{{/|}}lib"
-// CHECK: "-lc++" "-lm"
+// CHECK: "-push-state"
+// CHECK: "-as-needed"
+// CHECK: "-lc++"
+// CHECK: "-pop-state"
+// CHECK: "-lm"
 // CHECK: "{{.*[/\\]}}libclang_rt.builtins-x86_64.a"
 // CHECK: "-lc"
 // CHECK-NOT: crtend.o
@@ -29,8 +33,10 @@
 // RUN: %clangxx %s -### --target=x86_64-unknown-fuchsia -static-libstdc++ \
 // RUN: -fuse-ld=lld 2>&1 \
 // RUN: | FileCheck %s -check-prefix=CHECK-STATIC
-// CHECK-STATIC: "-Bstatic"
+// CHECK-STATIC: "-push-state"
+// CHECK-STATIC: "-as-needed"
+// CHECK-STATIC: "-static"
 // CHECK-STATIC: "-lc++"
-// CHECK-STATIC: "-Bdynamic"
+// CHECK-STATIC: "-pop-state"
 // CHECK-STATIC: "-lm"
 // CHECK-STATIC: "-lc"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -122,11 +122,12 @@
   if (ToolChain.ShouldLinkCXXStdlib(Args)) {
 bool OnlyLibstdcxxStatic = Args.hasArg(options::OPT_static_libstdcxx) 
&&
!Args.hasArg(options::OPT_static);
+CmdArgs.push_back("-push-state");
+CmdArgs.push_back("-as-needed");
 if (OnlyLibstdcxxStatic)
-  CmdArgs.push_back("-Bstatic");
+  CmdArgs.push_back("-static");
 ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
-if (OnlyLibstdcxxStatic)
-  CmdArgs.push_back("-Bdynamic");
+CmdArgs.push_back("-pop-state");
   }
   CmdArgs.push_back("-lm");
 }


Index: clang/test/Driver/fuchsia.cpp
===
--- clang/test/Driver/fuchsia.cpp
+++ clang/test/Driver/fuchsia.cpp
@@ -15,7 +15,11 @@
 // CHECK-NOT: crti.o
 // CHECK-NOT: crtbegin.o
 // CHECK: "-L[[SYSROOT]]{{/|}}lib"
-// CHECK: "-lc++" "-lm"
+// CHECK: "-push-state"
+// CHECK: "-as-needed"
+// CHECK: "-lc++"
+// CHECK: "-pop-state"
+// CHECK: "-lm"
 // CHECK: "{{.*[/\\]}}libclang_rt.builtins-x86_64.a"
 // CHECK: "-lc"
 // CHECK-NOT: crtend.o
@@ -29,8 +33,10 @@
 // RUN: %clangxx %s -### --target=x86_64-unknown-fuchsia -static-libstdc++ \
 // RUN: -fuse-ld=lld 2>&1 \
 // RUN: | FileCheck %s -check-prefix=CHECK-STATIC
-// CHECK-STATIC: "-Bstatic"
+// CHECK-STATIC: "-push-state"
+// CHECK-STATIC: "-as-needed"
+// CHECK-STATIC: "-static"
 // CHECK-STATIC: "-lc++"
-// CHECK-STATIC: "-Bdynamic"
+// CHECK-STATIC: "-pop-state"
 // CHECK-STATIC: "-lm"
 // CHECK-STATIC: "-lc"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -122,11 +122,12 @@
   if (ToolChain.ShouldLinkCXXStdlib(Args)) {
 bool OnlyLibstdcxxStatic = Args.hasArg(options::OPT_static_libstdcxx) &&
!Args.hasArg(options::OPT_static);
+CmdArgs.push_back("-push-state");
+CmdArgs.push_back("-as-needed");
 if (OnlyLibstdcxxStatic)
-  CmdArgs.push_back("-Bstatic");
+  CmdArgs.push_back("-static");
 ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
-if (OnlyLibstdcxxStatic)
-  CmdArgs.push_back("-Bdynamic");
+CmdArgs.push_back("-pop-state");
   }
   CmdArgs.push_back("-lm");
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53787: [Sema] Use proper visibility for global new and delete declarations

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

This seems pretty inconsistent with how -fvisibility=hidden behaves with 
normal, user written declarations. Consider this code:

  void foo();
  void bar() { foo(); }

We get this IR:

  $ clang -S -emit-llvm --target=x86_64-linux t.c -o - -fvisibility=hidden
  ...
  define hidden void @bar() #0 {
  entry:
call void @foo()
ret void
  }
  declare dso_local void @foo() #1

Basically, declarations are never marked hidden, only definitions are. This is 
because system headers are not explicitly marked with default visibility 
annotations, and you still want to be able to call libc from code compiled with 
-fvisibility=hidden.

I'm... kind of surprised we have code to explicitly mark these declarations 
with default visibility attributes.


Repository:
  rC Clang

https://reviews.llvm.org/D53787



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

[PATCH] D53850: Declares __cpu_model as hidden symbol

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: lib/CodeGen/CGBuiltin.cpp:9013
   Features, llvm::ConstantInt::get(Int32Ty, FeaturesMask));
   return Builder.CreateICmpNE(Bitset, llvm::ConstantInt::get(Int32Ty, 0));
 }

This code looks to be out of date. It's missing the changes from r344832 that 
added another runtime variable that presumably has the same issue.


https://reviews.llvm.org/D53850



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

[PATCH] D53850: Declares __cpu_model as hidden symbol

[Pirama Arumuga Nainar via Phabricator via cfe-commits]

pirama edited reviewers, added: echristo, craig.topper; removed: pirama.
pirama added subscribers: srhines, pirama, cfe-commits.
pirama added a comment.

Adding reviewers suggested by 'arc cover'.


https://reviews.llvm.org/D53850



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

[PATCH] D53847: [C++2a] P0634r3: Down with typename!

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

rsmith added inline comments.



Comment at: test/SemaCXX/unknown-type-name.cpp:1
-// RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -Wc++2a-compat -fsyntax-only -verify %s
+// RUN: %clang_cc1 -Wc++2a-compat -fsyntax-only -verify -std=c++98 %s

rsmith wrote:
> Several of the changes in this file look wrong to me.
Specifics below.



Comment at: test/SemaCXX/unknown-type-name.cpp:50
 template
-void f(T::type) { } // expected-error{{missing 'typename'}}
+void f(T::type) { } // expected-warning {{implicit 'typename' is a C++2a 
extension}}
 

This is wrong.

```
template
X f(T::type);
```

declares a variable template. This would be valid if the name `f` were a 
//qualified-id//, and lookup for `f` found a function template, though.

(Same for the next 7 cases.)



Comment at: test/SemaCXX/unknown-type-name.cpp:95-96
 
-template int h(T::type, int); // expected-error{{missing 
'typename'}}
-template int h(T::type x, char); // expected-error{{missing 
'typename'}}
+template int h(T::type, int); // expected-warning {{implicit 
'typename' is a C++2a extension}}
+template int h(T::type x, char); // expected-warning {{implicit 
'typename' is a C++2a extension}}
 

No implicit `typename` for these two (but the previous two are fine).



Comment at: test/SemaCXX/unknown-type-name.cpp:102-103
 #endif
-template int junk2(T::junk) throw(); // expected-error{{missing 
'typename'}}
-template int junk3(T::junk) = delete; // expected-error{{missing 
'typename'}}
+template int junk2(T::junk) throw(); // expected-warning 
{{implicit 'typename' is a C++2a extension}}
+template int junk3(T::junk) = delete; // expected-warning 
{{implicit 'typename' is a C++2a extension}}
 #if __cplusplus <= 199711L

These two are incorrect.



Comment at: test/SemaCXX/unknown-type-name.cpp:108
 
-template int junk4(T::junk j); // expected-error{{missing 
'typename'}}
+template int junk4(T::junk j); // expected-warning {{implicit 
'typename' is a C++2a extension}}
 

This one is incorrect.



Comment at: test/SemaCXX/unknown-type-name.cpp:121-122
 template
-A::g() { } // expected-error{{requires a type specifier}}
+A::g() { } // expected-error{{expected unqualified-id}}
+// expected-warning@-1{{implicit 'typename' is a C++2a extension}}

This is a diagnostic quality regression. Perhaps that's an inevitable 
consequence of P0634, but we should at least try to do better.


Repository:
  rC Clang

https://reviews.llvm.org/D53847



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

[PATCH] D53847: [C++2a] P0634r3: Down with typename!

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

rsmith added inline comments.



Comment at: lib/Parse/ParseDecl.cpp:3105
+  isClassTemplateDeductionContext(DSContext),
+  /*AllowImplicitTypename=*/true);
 

This looks overly broad: shouldn't this depend on the DSContext?



Comment at: lib/Parse/Parser.cpp:1778
+/*IsClassTemplateDeductionContext*/true,
+/*AllowImplicitTypename*/false)) {
   SourceLocation BeginLoc = Tok.getLocation();

Don't we need to sometimes allow an implicit typename here for correct 
disambiguation?



Comment at: test/CXX/temp/temp.res/p5.cpp:1
+// RUN: %clang_cc1 -std=c++2a -pedantic -verify %s
+

Negative tests are as important as positive tests; please add as many cases as 
you can think of where this should *not* apply, to make sure it doesn't. Pay 
particular attention to grammar ambiguities that typename resolves.



Comment at: test/CXX/temp/temp.res/p5.cpp:15
+  // it is a qualified name in a type-id-only context (see below), or
+  // [it's smallest enclosing [/new/defining/]-type-id is]:
+  // - a new-type-id

it's -> its



Comment at: test/FixIt/fixit.cpp:214
-  expected-error {{function definition declared 'typedef'}} \
-  expected-error {{missing 'typename' prior to dependent}}
-  return Mystery::get();

Please retain this, and add a FixItHint to the new extension warning so that it 
still works.



Comment at: test/SemaCXX/PR11358.cpp:15
 void test() {
-  Container::iterator i = c.begin(); // expected-error{{missing 
'typename'}}
+  Container::iterator i = c.begin(); // expected-warning{{implicit 
'typename' is a C++2a extension}}
 }

This is a bug. There's no implicit typename in this context. (Likewise later in 
this file.)



Comment at: test/SemaCXX/unknown-type-name.cpp:1
-// RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -Wc++2a-compat -fsyntax-only -verify %s
+// RUN: %clang_cc1 -Wc++2a-compat -fsyntax-only -verify -std=c++98 %s

Several of the changes in this file look wrong to me.


Repository:
  rC Clang

https://reviews.llvm.org/D53847



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

[PATCH] D53847: [C++2a] P0634r3: Down with typename!

[Nicolas Lesser via Phabricator via cfe-commits]

Rakete updated this revision to Diff 171612.
Rakete added a comment.

Fix easy errors in tests that I missed by adding explicit template 
instantations.


Repository:
  rC Clang

https://reviews.llvm.org/D53847

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Parse/Parser.cpp
  lib/Sema/SemaDecl.cpp
  test/CXX/drs/dr1xx.cpp
  test/CXX/drs/dr2xx.cpp
  test/CXX/drs/dr4xx.cpp
  test/CXX/drs/dr5xx.cpp
  test/CXX/temp/temp.res/p5.cpp
  test/CXX/temp/temp.res/temp.dep/temp.dep.type/p1.cpp
  test/FixIt/fixit.cpp
  test/SemaCXX/MicrosoftCompatibility.cpp
  test/SemaCXX/MicrosoftExtensions.cpp
  test/SemaCXX/MicrosoftSuper.cpp
  test/SemaCXX/PR11358.cpp
  test/SemaCXX/unknown-type-name.cpp

Index: test/SemaCXX/unknown-type-name.cpp
===
--- test/SemaCXX/unknown-type-name.cpp
+++ test/SemaCXX/unknown-type-name.cpp
@@ -36,39 +36,39 @@
 
   static int n;
   static type m;
-  static int h(T::type, int); // expected-error{{missing 'typename'}}
-  static int h(T::type x, char); // expected-error{{missing 'typename'}}
+  static int h(T::type, int); // expected-warning {{implicit 'typename' is a C++2a extension}}
+  static int h(T::type x, char); // expected-warning {{implicit 'typename' is a C++2a extension}}
 };
 
 template
-A::type g(T t) { return t; } // expected-error{{missing 'typename'}}
+A::type g(T t) { return t; } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-A::type A::f() { return type(); } // expected-error{{missing 'typename'}}
+A::type A::f() { return type(); } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void f(T::type) { } // expected-error{{missing 'typename'}}
+void f(T::type) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void g(T::type x) { } // expected-error{{missing 'typename'}}
+void g(T::type x) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void f(T::type, int) { } // expected-error{{missing 'typename'}}
+void f(T::type, int) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void f(T::type x, char) { } // expected-error{{missing 'typename'}}
+void f(T::type x, char) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void f(int, T::type) { } // expected-error{{missing 'typename'}}
+void f(int, T::type) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void f(char, T::type x) { } // expected-error{{missing 'typename'}}
+void f(char, T::type x) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void f(int, T::type, int) { } // expected-error{{missing 'typename'}}
+void f(int, T::type, int) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void f(int, T::type x, char) { } // expected-error{{missing 'typename'}}
+void f(int, T::type x, char) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 int *p;
 
@@ -86,26 +86,26 @@
 
 template int A::n(T::value); // ok
 template
-A::type // expected-error{{missing 'typename'}}
+A::type // expected-warning {{implicit 'typename' is a C++2a extension}}
 A::m(T::value, 0); // ok
 
-template int A::h(T::type, int) {} // expected-error{{missing 'typename'}}
-template int A::h(T::type x, char) {} // expected-error{{missing 'typename'}}
+template int A::h(T::type, int) {} // expected-warning {{implicit 'typename' is a C++2a extension}}
+template int A::h(T::type x, char) {} // expected-warning {{implicit 'typename' is a C++2a extension}}
 
-template int h(T::type, int); // expected-error{{missing 'typename'}}
-template int h(T::type x, char); // expected-error{{missing 'typename'}}
+template int h(T::type, int); // expected-warning {{implicit 'typename' is a C++2a extension}}
+template int h(T::type x, char); // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template int junk1(T::junk);
 #if __cplusplus <= 201103L
 // expected-warning@-2 {{variable templates are a C++14 extension}}
 #endif
-template int junk2(T::junk) throw(); // expected-error{{missing 'typename'}}
-template int junk3(T::junk) = delete; // expected-error{{missing 'typename'}}
+template int junk2(T::junk) throw(); // expected-warning {{implicit 'typename' is a C++2a extension}}
+template int junk3(T::junk) = delete; // expected-warning {{implicit 'typename' is a C++2a extension}}
 #if __cplusplus <= 199711L
 //expected-warning@-2 {{deleted function definitions are a C++11 extension}}
 #endif
 
-template int junk4(T::junk j); // expected-error{{missing 'typename'}}
+template int junk4(T::junk j); // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 // FIXME: We can tell this was intended to be a function because it does not
 //have a dependent nested name specifier.
@@ -118,4 +118,5 @@
 // 

[PATCH] D53296: [analyzer] New flag to print all -analyzer-config options

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus marked 2 inline comments as done.
Szelethus added a comment.

Thanks! :)




Comment at: lib/FrontendTool/ExecuteCompilerInvocation.cpp:255
+ento::printAnalyzerConfigList(llvm::outs());
+return true;
+  }

george.karpenkov wrote:
> Hm, should we return here?
> If there are errors, which are only printed in the next part, should they 
> still be printed?
I'd prefer to return as every other option. I think the user, if 
`-analyzer-config-help` was supplied, is only interested in a config option 
list.


https://reviews.llvm.org/D53296



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

[PATCH] D53296: [analyzer] New flag to print all -analyzer-config options

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus updated this revision to Diff 171611.
Szelethus added a comment.

Hardcoded the first entry as suggested by @george.karpenkov.


https://reviews.llvm.org/D53296

Files:
  include/clang/Driver/CC1Options.td
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Frontend/FrontendActions.h
  lib/Frontend/CompilerInvocation.cpp
  lib/FrontendTool/ExecuteCompilerInvocation.cpp
  lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
  test/Analysis/analyzer-list-configs.c

Index: test/Analysis/analyzer-list-configs.c
===
--- /dev/null
+++ test/Analysis/analyzer-list-configs.c
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -analyzer-config-help 2>&1 | FileCheck %s
+// CHECK: OVERVIEW: Clang Static Analyzer -analyzer-config Option List
+//
+// CHECK-NEXT: USAGE: clang -cc1 [CLANG_OPTIONS] -analyzer-config 
+//
+// CHCEK-NEXT:  clang -cc1 [CLANG_OPTIONS] -analyzer-config OPTION1=VALUE, -analyzer-config OPTION2=VALUE, ...
+//
+// CHECK-NEXT:  clang [CLANG_OPTIONS] -Xclang -analyzer-config -Xclang
+//
+// CHECK-NEXT:  clang [CLANG_OPTIONS] -Xclang -analyzer-config -Xclang OPTION1=VALUE, -Xclang -analyzer-config -Xclang OPTION2=VALUE, ...
+//
+//
+// CHECK-NEXT: OPTIONS:
+//
+// CHECK-NEXT: aggressive-binary-operation-simplification
+// CHECK-NEXT:   (bool) Whether SValBuilder should rearrange
+// CHECK-NEXT:   comparisons and additive operations of symbolic
+// CHECK-NEXT:   expressions which consist of a sum of a
+// CHECK-NEXT:   symbol and a concrete integer into the format
+// CHECK-NEXT:   where symbols are on the left-hand side
+// CHECK-NEXT:   and the integer is on the right. This is
+// CHECK-NEXT:   only done if both symbols and both concrete
+// CHECK-NEXT:   integers are signed, greater than or equal
+// CHECK-NEXT:   to the quarter of the minimum value of the
+// CHECK-NEXT:   type and less than or equal to the quarter
+// CHECK-NEXT:   of the maximum value of that type. A + n
+// CHECK-NEXT:B + m becomes A - B  m - n, where
+// CHECK-NEXT:   A and B symbolic, n and m are integers.
+// CHECK-NEXT:is any of '==', '!=', '<', '<=', '>',
+// CHECK-NEXT:   '>=', '+' or '-'. The rearrangement also
+// CHECK-NEXT:   happens with '-' instead of '+' on either
+// CHECK-NEXT:   or both side and also if any or both integers
+// CHECK-NEXT:   are missing. (default: false)
Index: lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
===
--- lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
+++ lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
@@ -22,6 +22,7 @@
 #include "clang/StaticAnalyzer/Frontend/FrontendActions.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/DynamicLibrary.h"
+#include "llvm/Support/FormattedStream.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
@@ -157,3 +158,74 @@
   SmallVector checkerOpts = getCheckerOptList(opts);
   ClangCheckerRegistry(plugins).printList(out, checkerOpts);
 }
+
+void ento::printAnalyzerConfigList(raw_ostream ) {
+  out << "OVERVIEW: Clang Static Analyzer -analyzer-config Option List\n\n";
+  out << "USAGE: clang -cc1 [CLANG_OPTIONS] -analyzer-config "
+"\n\n";
+  out << "   clang -cc1 [CLANG_OPTIONS] -analyzer-config OPTION1=VALUE, "
+  "-analyzer-config OPTION2=VALUE, ...\n\n";
+  out << "   clang [CLANG_OPTIONS] -Xclang -analyzer-config -Xclang"
+"\n\n";
+  out << "   clang [CLANG_OPTIONS] -Xclang -analyzer-config -Xclang "
+  "OPTION1=VALUE, -Xclang -analyzer-config -Xclang "
+  "OPTION2=VALUE, ...\n\n";
+  out << "OPTIONS:\n\n";
+
+  using OptionAndDescriptionTy = std::pair;
+  OptionAndDescriptionTy PrintableOptions[] = {
+#define ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, DEFAULT_VAL)\
+{  \
+  CMDFLAG, \
+  llvm::Twine(llvm::Twine() + "(" +\
+  (#TYPE == "StringRef" ? "string" : #TYPE ) + ") " DESC   \
+  " (default: " #DEFAULT_VAL ")").str()\
+},
+
+#define ANALYZER_OPTION_DEPENDS_ON_USER_MODE(TYPE, NAME, CMDFLAG, DESC,\
+ SHALLOW_VAL, DEEP_VAL)\
+{  \
+  CMDFLAG, 

[PATCH] D53847: [C++2a] P0634r3: Down with typename!

[Nicolas Lesser via Phabricator via cfe-commits]

Rakete updated this revision to Diff 171609.
Rakete added a comment.

Remove unneeded -Wc++2a-compat flag in tests.


Repository:
  rC Clang

https://reviews.llvm.org/D53847

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Parse/Parser.cpp
  lib/Sema/SemaDecl.cpp
  test/CXX/drs/dr1xx.cpp
  test/CXX/drs/dr2xx.cpp
  test/CXX/drs/dr4xx.cpp
  test/CXX/drs/dr5xx.cpp
  test/CXX/temp/temp.res/p5.cpp
  test/CXX/temp/temp.res/temp.dep/temp.dep.type/p1.cpp
  test/FixIt/fixit.cpp
  test/SemaCXX/MicrosoftCompatibility.cpp
  test/SemaCXX/MicrosoftExtensions.cpp
  test/SemaCXX/MicrosoftSuper.cpp
  test/SemaCXX/PR11358.cpp
  test/SemaCXX/unknown-type-name.cpp

Index: test/SemaCXX/unknown-type-name.cpp
===
--- test/SemaCXX/unknown-type-name.cpp
+++ test/SemaCXX/unknown-type-name.cpp
@@ -36,39 +36,39 @@
 
   static int n;
   static type m;
-  static int h(T::type, int); // expected-error{{missing 'typename'}}
-  static int h(T::type x, char); // expected-error{{missing 'typename'}}
+  static int h(T::type, int); // expected-warning {{implicit 'typename' is a C++2a extension}}
+  static int h(T::type x, char); // expected-warning {{implicit 'typename' is a C++2a extension}}
 };
 
 template
-A::type g(T t) { return t; } // expected-error{{missing 'typename'}}
+A::type g(T t) { return t; } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-A::type A::f() { return type(); } // expected-error{{missing 'typename'}}
+A::type A::f() { return type(); } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void f(T::type) { } // expected-error{{missing 'typename'}}
+void f(T::type) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void g(T::type x) { } // expected-error{{missing 'typename'}}
+void g(T::type x) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void f(T::type, int) { } // expected-error{{missing 'typename'}}
+void f(T::type, int) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void f(T::type x, char) { } // expected-error{{missing 'typename'}}
+void f(T::type x, char) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void f(int, T::type) { } // expected-error{{missing 'typename'}}
+void f(int, T::type) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void f(char, T::type x) { } // expected-error{{missing 'typename'}}
+void f(char, T::type x) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void f(int, T::type, int) { } // expected-error{{missing 'typename'}}
+void f(int, T::type, int) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void f(int, T::type x, char) { } // expected-error{{missing 'typename'}}
+void f(int, T::type x, char) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 int *p;
 
@@ -86,26 +86,26 @@
 
 template int A::n(T::value); // ok
 template
-A::type // expected-error{{missing 'typename'}}
+A::type // expected-warning {{implicit 'typename' is a C++2a extension}}
 A::m(T::value, 0); // ok
 
-template int A::h(T::type, int) {} // expected-error{{missing 'typename'}}
-template int A::h(T::type x, char) {} // expected-error{{missing 'typename'}}
+template int A::h(T::type, int) {} // expected-warning {{implicit 'typename' is a C++2a extension}}
+template int A::h(T::type x, char) {} // expected-warning {{implicit 'typename' is a C++2a extension}}
 
-template int h(T::type, int); // expected-error{{missing 'typename'}}
-template int h(T::type x, char); // expected-error{{missing 'typename'}}
+template int h(T::type, int); // expected-warning {{implicit 'typename' is a C++2a extension}}
+template int h(T::type x, char); // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template int junk1(T::junk);
 #if __cplusplus <= 201103L
 // expected-warning@-2 {{variable templates are a C++14 extension}}
 #endif
-template int junk2(T::junk) throw(); // expected-error{{missing 'typename'}}
-template int junk3(T::junk) = delete; // expected-error{{missing 'typename'}}
+template int junk2(T::junk) throw(); // expected-warning {{implicit 'typename' is a C++2a extension}}
+template int junk3(T::junk) = delete; // expected-warning {{implicit 'typename' is a C++2a extension}}
 #if __cplusplus <= 199711L
 //expected-warning@-2 {{deleted function definitions are a C++11 extension}}
 #endif
 
-template int junk4(T::junk j); // expected-error{{missing 'typename'}}
+template int junk4(T::junk j); // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 // FIXME: We can tell this was intended to be a function because it does not
 //have a dependent nested name specifier.
@@ -118,4 +118,5 @@
 // FIXME: We know which type specifier 

[PATCH] D53847: [C++2a] P0634r3: Down with typename!

[Nicolas Lesser via Phabricator via cfe-commits]

Rakete added inline comments.



Comment at: test/CXX/temp/temp.res/p5.cpp:71
+
+// FIXME: This is ok.
+template 

I think the below is well-formed according to the quote above, but I'm not sure 
I understand it correctly.


Repository:
  rC Clang

https://reviews.llvm.org/D53847



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

[PATCH] D53847: [C++2a] P0634r3: Down with typename!

[Nicolas Lesser via Phabricator via cfe-commits]

Rakete created this revision.
Rakete added a reviewer: rsmith.

This patch implements P0634r3 that removes the need for 'typename' in certain 
contexts.

For example,

  template 
  using foo = T::type; // ok

This is also allowed in previous language versions as an extension, because I 
think it's pretty useful. :)


Repository:
  rC Clang

https://reviews.llvm.org/D53847

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Parse/Parser.cpp
  lib/Sema/SemaDecl.cpp
  test/CXX/drs/dr1xx.cpp
  test/CXX/drs/dr2xx.cpp
  test/CXX/drs/dr4xx.cpp
  test/CXX/drs/dr5xx.cpp
  test/CXX/temp/temp.res/p5.cpp
  test/CXX/temp/temp.res/temp.dep/temp.dep.type/p1.cpp
  test/FixIt/fixit.cpp
  test/SemaCXX/MicrosoftCompatibility.cpp
  test/SemaCXX/MicrosoftExtensions.cpp
  test/SemaCXX/MicrosoftSuper.cpp
  test/SemaCXX/PR11358.cpp
  test/SemaCXX/unknown-type-name.cpp

Index: test/SemaCXX/unknown-type-name.cpp
===
--- test/SemaCXX/unknown-type-name.cpp
+++ test/SemaCXX/unknown-type-name.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -Wc++2a-compat -fsyntax-only -verify %s
+// RUN: %clang_cc1 -Wc++2a-compat -fsyntax-only -verify -std=c++98 %s
+// RUN: %clang_cc1 -Wc++2a-compat -fsyntax-only -verify -std=c++11 %s
 
 // PR3990
 namespace N {
@@ -36,39 +36,39 @@
 
   static int n;
   static type m;
-  static int h(T::type, int); // expected-error{{missing 'typename'}}
-  static int h(T::type x, char); // expected-error{{missing 'typename'}}
+  static int h(T::type, int); // expected-warning {{implicit 'typename' is a C++2a extension}}
+  static int h(T::type x, char); // expected-warning {{implicit 'typename' is a C++2a extension}}
 };
 
 template
-A::type g(T t) { return t; } // expected-error{{missing 'typename'}}
+A::type g(T t) { return t; } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-A::type A::f() { return type(); } // expected-error{{missing 'typename'}}
+A::type A::f() { return type(); } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void f(T::type) { } // expected-error{{missing 'typename'}}
+void f(T::type) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void g(T::type x) { } // expected-error{{missing 'typename'}}
+void g(T::type x) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void f(T::type, int) { } // expected-error{{missing 'typename'}}
+void f(T::type, int) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void f(T::type x, char) { } // expected-error{{missing 'typename'}}
+void f(T::type x, char) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void f(int, T::type) { } // expected-error{{missing 'typename'}}
+void f(int, T::type) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void f(char, T::type x) { } // expected-error{{missing 'typename'}}
+void f(char, T::type x) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void f(int, T::type, int) { } // expected-error{{missing 'typename'}}
+void f(int, T::type, int) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void f(int, T::type x, char) { } // expected-error{{missing 'typename'}}
+void f(int, T::type x, char) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 int *p;
 
@@ -86,26 +86,26 @@
 
 template int A::n(T::value); // ok
 template
-A::type // expected-error{{missing 'typename'}}
+A::type // expected-warning {{implicit 'typename' is a C++2a extension}}
 A::m(T::value, 0); // ok
 
-template int A::h(T::type, int) {} // expected-error{{missing 'typename'}}
-template int A::h(T::type x, char) {} // expected-error{{missing 'typename'}}
+template int A::h(T::type, int) {} // expected-warning {{implicit 'typename' is a C++2a extension}}
+template int A::h(T::type x, char) {} // expected-warning {{implicit 'typename' is a C++2a extension}}
 
-template int h(T::type, int); // expected-error{{missing 'typename'}}
-template int h(T::type x, char); // expected-error{{missing 'typename'}}
+template int h(T::type, int); // expected-warning {{implicit 'typename' is a C++2a extension}}
+template int h(T::type x, char); // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template int junk1(T::junk);
 #if __cplusplus <= 201103L
 // expected-warning@-2 {{variable templates are a C++14 extension}}
 #endif
-template int junk2(T::junk) throw(); // expected-error{{missing 'typename'}}
-template int junk3(T::junk) = delete; // expected-error{{missing 'typename'}}
+template int junk2(T::junk) throw(); // expected-warning {{implicit 'typename' is 

[PATCH] D53839: [CMake][Fuchsia] Drop the LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT

[Petr Hosek via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL345552: [CMake][Fuchsia] Drop the 
LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT (authored by phosek, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D53839?vs=171586=171604#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D53839

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


Index: cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
===
--- cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
+++ cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
@@ -58,8 +58,6 @@
   set(RUNTIMES_${target}-linux-gnu_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
   set(RUNTIMES_${target}-linux-gnu_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON 
CACHE BOOL "")
   set(RUNTIMES_${target}-linux-gnu_LIBCXX_ABI_VERSION 2 CACHE STRING "")
-  # TODO: this is a workaround for PR39053 which was uncovered by D50652.
-  set(RUNTIMES_${target}-linux-gnu_LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT 
ON CACHE BOOL "")
   set(RUNTIMES_${target}-linux-gnu_SANITIZER_CXX_ABI "libc++" CACHE STRING 
"")
   set(RUNTIMES_${target}-linux-gnu_SANITIZER_CXX_ABI_INTREE ON CACHE BOOL 
"")
   set(RUNTIMES_${target}-linux-gnu_COMPILER_RT_USE_BUILTINS_LIBRARY ON 
CACHE BOOL "")
@@ -113,8 +111,6 @@
 set(RUNTIMES_${target}-fuchsia_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE 
BOOL "")
 
set(RUNTIMES_${target}-fuchsia_LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY OFF 
CACHE BOOL "")
 set(RUNTIMES_${target}-fuchsia_LIBCXX_ABI_VERSION 2 CACHE STRING "")
-# TODO: this is a workaround for PR39053 which was uncovered by D50652.
-set(RUNTIMES_${target}-fuchsia_LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT ON 
CACHE BOOL "")
   endforeach()
 
   set(LLVM_RUNTIME_SANITIZERS "Address" CACHE STRING "")


Index: cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
===
--- cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
+++ cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
@@ -58,8 +58,6 @@
   set(RUNTIMES_${target}-linux-gnu_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
   set(RUNTIMES_${target}-linux-gnu_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
   set(RUNTIMES_${target}-linux-gnu_LIBCXX_ABI_VERSION 2 CACHE STRING "")
-  # TODO: this is a workaround for PR39053 which was uncovered by D50652.
-  set(RUNTIMES_${target}-linux-gnu_LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT ON CACHE BOOL "")
   set(RUNTIMES_${target}-linux-gnu_SANITIZER_CXX_ABI "libc++" CACHE STRING "")
   set(RUNTIMES_${target}-linux-gnu_SANITIZER_CXX_ABI_INTREE ON CACHE BOOL "")
   set(RUNTIMES_${target}-linux-gnu_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
@@ -113,8 +111,6 @@
 set(RUNTIMES_${target}-fuchsia_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
 set(RUNTIMES_${target}-fuchsia_LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY OFF CACHE BOOL "")
 set(RUNTIMES_${target}-fuchsia_LIBCXX_ABI_VERSION 2 CACHE STRING "")
-# TODO: this is a workaround for PR39053 which was uncovered by D50652.
-set(RUNTIMES_${target}-fuchsia_LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT ON CACHE BOOL "")
   endforeach()
 
   set(LLVM_RUNTIME_SANITIZERS "Address" CACHE STRING "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r345552 - [CMake][Fuchsia] Drop the LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT

[Petr Hosek via cfe-commits]

Author: phosek
Date: Mon Oct 29 16:10:49 2018
New Revision: 345552

URL: http://llvm.org/viewvc/llvm-project?rev=345552=rev
Log:
[CMake][Fuchsia] Drop the LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT

Now that libc++ uses __exclude_from_explicit_instantiation__ attribute,
this is no longer needed.

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

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

Modified: cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/caches/Fuchsia-stage2.cmake?rev=345552=345551=345552=diff
==
--- cfe/trunk/cmake/caches/Fuchsia-stage2.cmake (original)
+++ cfe/trunk/cmake/caches/Fuchsia-stage2.cmake Mon Oct 29 16:10:49 2018
@@ -58,8 +58,6 @@ elseif(UNIX)
   set(RUNTIMES_${target}-linux-gnu_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
   set(RUNTIMES_${target}-linux-gnu_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON 
CACHE BOOL "")
   set(RUNTIMES_${target}-linux-gnu_LIBCXX_ABI_VERSION 2 CACHE STRING "")
-  # TODO: this is a workaround for PR39053 which was uncovered by D50652.
-  set(RUNTIMES_${target}-linux-gnu_LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT 
ON CACHE BOOL "")
   set(RUNTIMES_${target}-linux-gnu_SANITIZER_CXX_ABI "libc++" CACHE STRING 
"")
   set(RUNTIMES_${target}-linux-gnu_SANITIZER_CXX_ABI_INTREE ON CACHE BOOL 
"")
   set(RUNTIMES_${target}-linux-gnu_COMPILER_RT_USE_BUILTINS_LIBRARY ON 
CACHE BOOL "")
@@ -113,8 +111,6 @@ if(FUCHSIA_SDK)
 set(RUNTIMES_${target}-fuchsia_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE 
BOOL "")
 
set(RUNTIMES_${target}-fuchsia_LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY OFF 
CACHE BOOL "")
 set(RUNTIMES_${target}-fuchsia_LIBCXX_ABI_VERSION 2 CACHE STRING "")
-# TODO: this is a workaround for PR39053 which was uncovered by D50652.
-set(RUNTIMES_${target}-fuchsia_LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT ON 
CACHE BOOL "")
   endforeach()
 
   set(LLVM_RUNTIME_SANITIZERS "Address" CACHE STRING "")


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

[PATCH] D50269: [compiler-rt][builtins] Don't #include CoreFoundation in os_version_check.c

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL345551: [compiler-rt][builtins] Dont #include 
CoreFoundation in os_version_check.c (authored by epilk, committed by ).
Herald added a subscriber: delcypher.

Changed prior to commit:
  https://reviews.llvm.org/D50269?vs=159084=171603#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D50269

Files:
  compiler-rt/trunk/lib/builtins/os_version_check.c


Index: compiler-rt/trunk/lib/builtins/os_version_check.c
===
--- compiler-rt/trunk/lib/builtins/os_version_check.c
+++ compiler-rt/trunk/lib/builtins/os_version_check.c
@@ -15,7 +15,6 @@
 
 #ifdef __APPLE__
 
-#include 
 #include 
 #include 
 #include 
@@ -28,6 +27,33 @@
 static int32_t GlobalMajor, GlobalMinor, GlobalSubminor;
 static dispatch_once_t DispatchOnceCounter;
 
+/* We can't include  directly from here, so
+ * just forward declare everything that we need from it. */
+
+typedef const void *CFDataRef, *CFAllocatorRef, *CFPropertyListRef,
+   *CFStringRef, *CFDictionaryRef, *CFTypeRef, *CFErrorRef;
+
+#if __LLP64__
+typedef unsigned long long CFTypeID;
+typedef unsigned long long CFOptionFlags;
+typedef signed long long CFIndex;
+#else
+typedef unsigned long CFTypeID;
+typedef unsigned long CFOptionFlags;
+typedef signed long CFIndex;
+#endif
+
+typedef unsigned char UInt8;
+typedef _Bool Boolean;
+typedef CFIndex CFPropertyListFormat;
+typedef uint32_t CFStringEncoding;
+
+/* kCFStringEncodingASCII analog. */
+#define CF_STRING_ENCODING_ASCII 0x0600
+/* kCFStringEncodingUTF8 analog. */
+#define CF_STRING_ENCODING_UTF8 0x08000100
+#define CF_PROPERTY_LIST_IMMUTABLE 0
+
 typedef CFDataRef (*CFDataCreateWithBytesNoCopyFuncTy)(CFAllocatorRef,
const UInt8 *, CFIndex,
CFAllocatorRef);
@@ -55,8 +81,7 @@
   const void *NullAllocator = dlsym(RTLD_DEFAULT, "kCFAllocatorNull");
   if (!NullAllocator)
 return;
-  const CFAllocatorRef kCFAllocatorNull =
-  *(const CFAllocatorRef *)NullAllocator;
+  const CFAllocatorRef AllocatorNull = *(const CFAllocatorRef *)NullAllocator;
   CFDataCreateWithBytesNoCopyFuncTy CFDataCreateWithBytesNoCopyFunc =
   (CFDataCreateWithBytesNoCopyFuncTy)dlsym(RTLD_DEFAULT,
"CFDataCreateWithBytesNoCopy");
@@ -140,21 +165,21 @@
   /* Get the file buffer into CF's format. We pass in a null allocator here *
* because we free PListBuf ourselves */
   FileContentsRef = (*CFDataCreateWithBytesNoCopyFunc)(
-  NULL, PListBuf, (CFIndex)NumRead, kCFAllocatorNull);
+  NULL, PListBuf, (CFIndex)NumRead, AllocatorNull);
   if (!FileContentsRef)
 goto Fail;
 
   if (CFPropertyListCreateWithDataFunc)
 PListRef = (*CFPropertyListCreateWithDataFunc)(
-NULL, FileContentsRef, kCFPropertyListImmutable, NULL, NULL);
+NULL, FileContentsRef, CF_PROPERTY_LIST_IMMUTABLE, NULL, NULL);
   else
 PListRef = (*CFPropertyListCreateFromXMLDataFunc)(
-NULL, FileContentsRef, kCFPropertyListImmutable, NULL);
+NULL, FileContentsRef, CF_PROPERTY_LIST_IMMUTABLE, NULL);
   if (!PListRef)
 goto Fail;
 
   CFStringRef ProductVersion = (*CFStringCreateWithCStringNoCopyFunc)(
-  NULL, "ProductVersion", kCFStringEncodingASCII, kCFAllocatorNull);
+  NULL, "ProductVersion", CF_STRING_ENCODING_ASCII, AllocatorNull);
   if (!ProductVersion)
 goto Fail;
   CFTypeRef OpaqueValue = (*CFDictionaryGetValueFunc)(PListRef, 
ProductVersion);
@@ -165,7 +190,7 @@
 
   char VersionStr[32];
   if (!(*CFStringGetCStringFunc)((CFStringRef)OpaqueValue, VersionStr,
- sizeof(VersionStr), kCFStringEncodingUTF8))
+ sizeof(VersionStr), CF_STRING_ENCODING_UTF8))
 goto Fail;
   sscanf(VersionStr, "%d.%d.%d", , , );
 


Index: compiler-rt/trunk/lib/builtins/os_version_check.c
===
--- compiler-rt/trunk/lib/builtins/os_version_check.c
+++ compiler-rt/trunk/lib/builtins/os_version_check.c
@@ -15,7 +15,6 @@
 
 #ifdef __APPLE__
 
-#include 
 #include 
 #include 
 #include 
@@ -28,6 +27,33 @@
 static int32_t GlobalMajor, GlobalMinor, GlobalSubminor;
 static dispatch_once_t DispatchOnceCounter;
 
+/* We can't include  directly from here, so
+ * just forward declare everything that we need from it. */
+
+typedef const void *CFDataRef, *CFAllocatorRef, *CFPropertyListRef,
+   *CFStringRef, *CFDictionaryRef, *CFTypeRef, *CFErrorRef;
+
+#if __LLP64__
+typedef unsigned long long CFTypeID;
+typedef unsigned long long CFOptionFlags;
+typedef signed long long CFIndex;
+#else
+typedef unsigned long CFTypeID;
+typedef unsigned long CFOptionFlags;
+typedef signed long CFIndex;
+#endif
+
+typedef unsigned char UInt8;
+typedef _Bool Boolean;
+typedef CFIndex 

[PATCH] D53839: [CMake][Fuchsia] Drop the LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT

[Louis Dionne via Phabricator via cfe-commits]

ldionne accepted this revision.
ldionne added a comment.

Please LMK if you see nice consequences of this. I'm trying to gather "real 
stories" where this helped.


Repository:
  rC Clang

https://reviews.llvm.org/D53839



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

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

[Russell McClellan via Phabricator via cfe-commits]

russellmcc added a comment.

Bump!  Thanks for your time


https://reviews.llvm.org/D40988



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

[PATCH] D52998: [benchmark] Disable exceptions in Microsoft STL

[Elizabeth Andrews via Phabricator via cfe-commits]

eandrews added a comment.

In https://reviews.llvm.org/D52998#1259602, @lebedev.ri wrote:

> In https://reviews.llvm.org/D52998#1258962, @eandrews wrote:
>
> > Yes. I understand. At the moment, exception handling flags are generated 
> > based on `BENCHMARK_ENABLE_EXCEPTIONS`  in `utils/benchmark/CMakeLists.txt` 
> > .  However, `_HAS_EXCEPTIONS` is not defined based on this (code below). 
> > The warnings are a result of this mismatch.
> >
> >   if (NOT BENCHMARK_ENABLE_EXCEPTIONS)
> >   add_cxx_compiler_flag(-EHs-)
> >   add_cxx_compiler_flag(-EHa-)
> > endif()
> >
> >
> > I think it makes most sense to add definition for `_HAS_EXCEPTIONS=0 `here 
> > as opposed to modifying `llvm/CMakeLists.txt`.
>
>
> Then i'd recommend/suggest to submit this upstream 
>  first.
>
> > Please correct me if I'm wrong. I'm not too familiar with CMake. @kbobyrev 
> > Please let me know what you think as well since you had suggested 
> > `llvm/CMakeLists.txt`.


@kbobyrev Is this alright with you?


https://reviews.llvm.org/D52998



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

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added a comment.

In https://reviews.llvm.org/D33672#1279520, @ZaMaZaN4iK wrote:

> Thank you for the roadmap! Honestly I am not so familiar with Clang AST and 
> Clang Static Analyzer details (all my experience is writing some simple 
> checkers for clang-tidy and watching some videos about Clang/Clang Static 
> Analyzers), so I even don't understand of your words :) So don't rely on 
> quick fixes from my side. But I think I can write something useful after 
> couple of tries (slowly, but I can). Thank you for the help!


Always happy to assist here or on cfe-dev 
, feel free to send a mail! If 
you haven't seen it yet, the Static Analyzer Developer Guide 
 may help you expand 
your current knowledge greatly, especially the links on the bottom.


https://reviews.llvm.org/D33672



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

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK added a comment.

In https://reviews.llvm.org/D33672#1279305, @NoQ wrote:

> Thanks! I like where this is going. Let's land the patch and continue 
> developing it incrementally in trunk.
>
> The next steps for this checker are, in my opinion:
>
> - Do the visitor thingy that i requested in inline-311373 
> . I think it's a necessary 
> thing to do, but don't jump into implementing it right away: i already have 
> some code for this that i want to share.
> - Play nicely with typedefs. For now i believe the checker ignores them 
> because you can't cast `TypedefType` to `EnumType`. Once this is done, it 
> will be worth it to include the name of the enum in the warning message.
> - Optimize the code using `assumeInclusiveRange`. Because `assume` is an 
> expensive operation, i'd like to avoid doing it O(n) times for contiguous 
> enums in which just 2 `assume`s are enough (or, even better, as single 
> `assumeInclusiveRange`).
> - See how this checker performs on real code, fix crashes and false positives 
> if any.


Thank you for the roadmap! Honestly I am not so familiar with Clang AST and 
Clang Static Analyzer details (all my experience is writing some simple 
checkers for clang-tidy and watching some videos about Clang/Clang Static 
Analyzers), so I even don't understand of your words :) So don't rely on quick 
fixes from my side. But I think I can write something useful after couple of 
tries (slowly, but I can). Thank you for the help!


https://reviews.llvm.org/D33672



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

[PATCH] D53839: [CMake][Fuchsia] Drop the LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT

[Petr Hosek via Phabricator via cfe-commits]

phosek created this revision.
phosek added reviewers: jakehehrlich, mcgrathr.
Herald added subscribers: cfe-commits, mgorny.

Now that libc++ uses __exclude_from_explicit_instantiation__ attribute,
this is no longer needed.


Repository:
  rC Clang

https://reviews.llvm.org/D53839

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -58,8 +58,6 @@
   set(RUNTIMES_${target}-linux-gnu_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
   set(RUNTIMES_${target}-linux-gnu_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON 
CACHE BOOL "")
   set(RUNTIMES_${target}-linux-gnu_LIBCXX_ABI_VERSION 2 CACHE STRING "")
-  # TODO: this is a workaround for PR39053 which was uncovered by D50652.
-  set(RUNTIMES_${target}-linux-gnu_LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT 
ON CACHE BOOL "")
   set(RUNTIMES_${target}-linux-gnu_SANITIZER_CXX_ABI "libc++" CACHE STRING 
"")
   set(RUNTIMES_${target}-linux-gnu_SANITIZER_CXX_ABI_INTREE ON CACHE BOOL 
"")
   set(RUNTIMES_${target}-linux-gnu_COMPILER_RT_USE_BUILTINS_LIBRARY ON 
CACHE BOOL "")
@@ -113,8 +111,6 @@
 set(RUNTIMES_${target}-fuchsia_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE 
BOOL "")
 
set(RUNTIMES_${target}-fuchsia_LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY OFF 
CACHE BOOL "")
 set(RUNTIMES_${target}-fuchsia_LIBCXX_ABI_VERSION 2 CACHE STRING "")
-# TODO: this is a workaround for PR39053 which was uncovered by D50652.
-set(RUNTIMES_${target}-fuchsia_LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT ON 
CACHE BOOL "")
   endforeach()
 
   set(LLVM_RUNTIME_SANITIZERS "Address" CACHE STRING "")


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -58,8 +58,6 @@
   set(RUNTIMES_${target}-linux-gnu_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
   set(RUNTIMES_${target}-linux-gnu_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
   set(RUNTIMES_${target}-linux-gnu_LIBCXX_ABI_VERSION 2 CACHE STRING "")
-  # TODO: this is a workaround for PR39053 which was uncovered by D50652.
-  set(RUNTIMES_${target}-linux-gnu_LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT ON CACHE BOOL "")
   set(RUNTIMES_${target}-linux-gnu_SANITIZER_CXX_ABI "libc++" CACHE STRING "")
   set(RUNTIMES_${target}-linux-gnu_SANITIZER_CXX_ABI_INTREE ON CACHE BOOL "")
   set(RUNTIMES_${target}-linux-gnu_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
@@ -113,8 +111,6 @@
 set(RUNTIMES_${target}-fuchsia_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
 set(RUNTIMES_${target}-fuchsia_LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY OFF CACHE BOOL "")
 set(RUNTIMES_${target}-fuchsia_LIBCXX_ABI_VERSION 2 CACHE STRING "")
-# TODO: this is a workaround for PR39053 which was uncovered by D50652.
-set(RUNTIMES_${target}-fuchsia_LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT ON CACHE BOOL "")
   endforeach()
 
   set(LLVM_RUNTIME_SANITIZERS "Address" CACHE STRING "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53817: [clang-tidy] cppcoreguidelines-macro-usage: print macro names

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 171583.
lebedev.ri added a comment.

Add an opt-in option to diagnose such command-line-based macros.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53817

Files:
  clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
  clang-tidy/cppcoreguidelines/MacroUsageCheck.h
  docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
  test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp
  test/clang-tidy/cppcoreguidelines-macro-usage-command-line-macros.cpp
  test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
  test/clang-tidy/cppcoreguidelines-macro-usage.cpp

Index: test/clang-tidy/cppcoreguidelines-macro-usage.cpp
===
--- test/clang-tidy/cppcoreguidelines-macro-usage.cpp
+++ test/clang-tidy/cppcoreguidelines-macro-usage.cpp
@@ -4,15 +4,15 @@
 #define INCLUDE_GUARD
 
 #define PROBLEMATIC_CONSTANT 0
-// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro used to declare a constant; consider using a 'constexpr' constant
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro 'PROBLEMATIC_CONSTANT' used to declare a constant; consider using a 'constexpr' constant
 
 #define PROBLEMATIC_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
-// CHECK-MESSAGES: [[@LINE-1]]:9: warning: function-like macro used; consider a 'constexpr' template function
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: function-like macro 'PROBLEMATIC_FUNCTION' used; consider a 'constexpr' template function
 
 #define PROBLEMATIC_VARIADIC(...) (__VA_ARGS__)
-// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro 'PROBLEMATIC_VARIADIC' used; consider using a 'constexpr' variadic template function
 
 #define PROBLEMATIC_VARIADIC2(x, ...) (__VA_ARGS__)
-// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro 'PROBLEMATIC_VARIADIC2' used; consider using a 'constexpr' variadic template function
 
 #endif
Index: test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
===
--- test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
+++ test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
@@ -6,16 +6,16 @@
 #define INCLUDE_GUARD
 
 #define PROBLEMATIC_CONSTANT 0
-// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro used to declare a constant; consider using a 'constexpr' constant
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro 'PROBLEMATIC_CONSTANT' used to declare a constant; consider using a 'constexpr' constant
 
 #define PROBLEMATIC_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
-// CHECK-MESSAGES: [[@LINE-1]]:9: warning: function-like macro used; consider a 'constexpr' template function
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: function-like macro 'PROBLEMATIC_FUNCTION' used; consider a 'constexpr' template function
 
 #define PROBLEMATIC_VARIADIC(...) (__VA_ARGS__)
-// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro 'PROBLEMATIC_VARIADIC' used; consider using a 'constexpr' variadic template function
 
 #define PROBLEMATIC_VARIADIC2(x, ...) (__VA_ARGS__)
-// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro 'PROBLEMATIC_VARIADIC2' used; consider using a 'constexpr' variadic template function
 
 #define DEBUG_CONSTANT 0
 #define DEBUG_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
Index: test/clang-tidy/cppcoreguidelines-macro-usage-command-line-macros.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-macro-usage-command-line-macros.cpp
@@ -0,0 +1,8 @@
+// RUN: %check_clang_tidy -check-suffixes=NORMAL %s cppcoreguidelines-macro-usage %t -- -- -D_ZZZ_IM_A_MACRO
+// RUN: %check_clang_tidy -check-suffixes=NORMAL %s cppcoreguidelines-macro-usage %t -- -config='{CheckOptions: [{key: cppcoreguidelines-macro-usage.IgnoreLocationless, value: 1}]}' -- -D_ZZZ_IM_A_MACRO
+// RUN: %check_clang_tidy -check-suffixes=NORMAL,CL %s cppcoreguidelines-macro-usage %t -- -config='{CheckOptions: [{key: cppcoreguidelines-macro-usage.IgnoreLocationless, value: 0}]}' -- -D_ZZZ_IM_A_MACRO
+
+// CHECK-MESSAGES-CL: warning: macro '_ZZZ_IM_A_MACRO' used to declare a constant; consider using a 'constexpr' constant
+
+#define PROBLEMATIC_CONSTANT 0
+// CHECK-MESSAGES-NORMAL: [[@LINE-1]]:9: warning: macro 'PROBLEMATIC_CONSTANT' used to declare a constant; consider using a 'constexpr' constant
Index: test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp
===
--- 

[PATCH] D53837: [clang] Move two utility functions into SourceManager

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri created this revision.
lebedev.ri added reviewers: rsmith, aaron.ballman.
lebedev.ri added a project: clang.
Herald added subscribers: kbarton, nemanjai.

So we can keep that not-so-great logic in one place.


Repository:
  rC Clang

https://reviews.llvm.org/D53837

Files:
  include/clang/Basic/SourceManager.h
  lib/CodeGen/MacroPPCallbacks.cpp


Index: lib/CodeGen/MacroPPCallbacks.cpp
===
--- lib/CodeGen/MacroPPCallbacks.cpp
+++ lib/CodeGen/MacroPPCallbacks.cpp
@@ -88,16 +88,6 @@
   return SourceLocation();
 }
 
-static bool isBuiltinFile(SourceManager , SourceLocation Loc) {
-  StringRef Filename(SM.getPresumedLoc(Loc).getFilename());
-  return Filename.equals("");
-}
-
-static bool isCommandLineFile(SourceManager , SourceLocation Loc) {
-  StringRef Filename(SM.getPresumedLoc(Loc).getFilename());
-  return Filename.equals("");
-}
-
 void MacroPPCallbacks::updateStatusToNextScope() {
   switch (Status) {
   case NoScope:
@@ -127,7 +117,7 @@
 updateStatusToNextScope();
 return;
   case BuiltinScope:
-if (isCommandLineFile(PP.getSourceManager(), Loc))
+if (PP.getSourceManager().isWrittenInCommandLineFile(Loc))
   return;
 updateStatusToNextScope();
 LLVM_FALLTHROUGH;
@@ -147,7 +137,7 @@
   default:
 llvm_unreachable("Do not expect to exit a file from current scope");
   case BuiltinScope:
-if (!isBuiltinFile(PP.getSourceManager(), Loc))
+if (!PP.getSourceManager().isWrittenInBuiltinFile(Loc))
   // Skip next scope and change status to MainFileScope.
   Status = MainFileScope;
 return;
Index: include/clang/Basic/SourceManager.h
===
--- include/clang/Basic/SourceManager.h
+++ include/clang/Basic/SourceManager.h
@@ -1428,6 +1428,18 @@
 return getFileID(Loc) == getMainFileID();
   }
 
+  /// Returns whether \p Loc is located in a  file.
+  bool isWrittenInBuiltinFile(SourceLocation Loc) const {
+StringRef Filename(getPresumedLoc(Loc).getFilename());
+return Filename.equals("");
+  }
+
+  /// Returns whether \p Loc is located in a  file.
+  bool isWrittenInCommandLineFile(SourceLocation Loc) const {
+StringRef Filename(getPresumedLoc(Loc).getFilename());
+return Filename.equals("");
+  }
+
   /// Returns if a SourceLocation is in a system header.
   bool isInSystemHeader(SourceLocation Loc) const {
 return isSystem(getFileCharacteristic(Loc));


Index: lib/CodeGen/MacroPPCallbacks.cpp
===
--- lib/CodeGen/MacroPPCallbacks.cpp
+++ lib/CodeGen/MacroPPCallbacks.cpp
@@ -88,16 +88,6 @@
   return SourceLocation();
 }
 
-static bool isBuiltinFile(SourceManager , SourceLocation Loc) {
-  StringRef Filename(SM.getPresumedLoc(Loc).getFilename());
-  return Filename.equals("");
-}
-
-static bool isCommandLineFile(SourceManager , SourceLocation Loc) {
-  StringRef Filename(SM.getPresumedLoc(Loc).getFilename());
-  return Filename.equals("");
-}
-
 void MacroPPCallbacks::updateStatusToNextScope() {
   switch (Status) {
   case NoScope:
@@ -127,7 +117,7 @@
 updateStatusToNextScope();
 return;
   case BuiltinScope:
-if (isCommandLineFile(PP.getSourceManager(), Loc))
+if (PP.getSourceManager().isWrittenInCommandLineFile(Loc))
   return;
 updateStatusToNextScope();
 LLVM_FALLTHROUGH;
@@ -147,7 +137,7 @@
   default:
 llvm_unreachable("Do not expect to exit a file from current scope");
   case BuiltinScope:
-if (!isBuiltinFile(PP.getSourceManager(), Loc))
+if (!PP.getSourceManager().isWrittenInBuiltinFile(Loc))
   // Skip next scope and change status to MainFileScope.
   Status = MainFileScope;
 return;
Index: include/clang/Basic/SourceManager.h
===
--- include/clang/Basic/SourceManager.h
+++ include/clang/Basic/SourceManager.h
@@ -1428,6 +1428,18 @@
 return getFileID(Loc) == getMainFileID();
   }
 
+  /// Returns whether \p Loc is located in a  file.
+  bool isWrittenInBuiltinFile(SourceLocation Loc) const {
+StringRef Filename(getPresumedLoc(Loc).getFilename());
+return Filename.equals("");
+  }
+
+  /// Returns whether \p Loc is located in a  file.
+  bool isWrittenInCommandLineFile(SourceLocation Loc) const {
+StringRef Filename(getPresumedLoc(Loc).getFilename());
+return Filename.equals("");
+  }
+
   /// Returns if a SourceLocation is in a system header.
   bool isInSystemHeader(SourceLocation Loc) const {
 return isSystem(getFileCharacteristic(Loc));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53814: Allow the analyzer to output to a SARIF file

[George Karpenkov via Phabricator via cfe-commits]

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

I don't think a new PathGenerationScheme is needed, unless you plan changes to 
BugReporter.cpp.

The code is fine otherwise, but could we try to use `diff` for testing?




Comment at: Analysis/diagnostics/sarif-diagnostics-taint-test.c:18
+// Test the basics for sanity.
+// CHECK: sarifLog['version'].startswith("2.0.0")
+// CHECK: sarifLog['runs'][0]['tool']['fullName'] == "clang static analyzer"

aaron.ballman wrote:
> george.karpenkov wrote:
> > Would it make more sense to just use `diff` + json pretty-formatter to 
> > write a test?
> > With this test I can't even quite figure out how the output should look 
> > like.
> I'm not super comfortable with that approach, but perhaps I'm thinking of 
> something different than what you're proposing. The reason I went with this 
> approach is because diff would be fragile (depends heavily on field ordering, 
> which the JSON support library doesn't give control over) and the physical 
> layout of the file isn't what needs to be tested anyway. SARIF has a fair 
> amount of optional data that can be provided as well, so using a purely 
> textual diff worried me that exporting additional optional data in the future 
> would require extensive unrelated changes to all SARIF diffs in the test 
> directory.
> 
> The goal for this test was to demonstrate that we can validate that the 
> interesting bits of information are present in the output without worrying 
> about the details.
> 
> Also, the python approach allows us to express relationships between data 
> items more easily than a textual diff tool would. I've not used that here, 
> but I could imagine a test where we want to check that each code location has 
> a corresponding file entry in another list.
Using a sample file + diff would have an advantage of being easier to read 
(just glance at the "Inputs/blah.serif" to see a sample output), and consistent 
with how we already do checking for plists.

> depends heavily on field ordering

Is it an issue in practice though? I would assume that JSON support library 
would not switch field ordering too often (and even if it does, we can have a 
python wrapper testing that)

> SARIF has a fair amount of optional data

Would diff `--ignore-matching-lines` be enough for those?



Comment at: StaticAnalyzer/Core/SarifDiagnostics.cpp:69
+return std::string(, 1);
+  return "%" + llvm::toHex(StringRef(, 1));
+}

+1, I would use this in other consumers.



Comment at: clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h:128
+/// Used for SARIF output.
+Sarif,
   };

Do you actually need a new generation scheme here?
I'm pretty sure that using "Minimal" would give you the same effect.


https://reviews.llvm.org/D53814



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

[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished

[Aleksei Sidorin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC345545: [ASTImporter] Reorder fields after structure import 
is finished (authored by a.sidorin, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D44100?vs=160477=171578#toc

Repository:
  rC Clang

https://reviews.llvm.org/D44100

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp


Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -1658,13 +1658,53 @@
 auto ToDCOrErr = Importer.ImportContext(FromDC);
 return ToDCOrErr.takeError();
   }
-  llvm::SmallVector ImportedDecls;
+
+  const auto *FromRD = dyn_cast(FromDC);
   for (auto *From : FromDC->decls()) {
 ExpectedDecl ImportedOrErr = import(From);
-if (!ImportedOrErr)
+if (!ImportedOrErr) {
+  // For RecordDecls, failed import of a field will break the layout of the
+  // structure. Handle it as an error.
+  if (FromRD)
+return ImportedOrErr.takeError();
   // Ignore the error, continue with next Decl.
   // FIXME: Handle this case somehow better.
-  consumeError(ImportedOrErr.takeError());
+  else
+consumeError(ImportedOrErr.takeError());
+}
+  }
+
+  // Reorder declarations in RecordDecls because they may have another
+  // order. Keeping field order is vitable because it determines structure
+  // layout.
+  // FIXME: This is an ugly fix. Unfortunately, I cannot come with better
+  // solution for this issue. We cannot defer expression import here because
+  // type import can depend on them.
+  if (!FromRD)
+return Error::success();
+
+  auto ImportedDC = import(cast(FromDC));
+  assert(ImportedDC);
+  auto *ToRD = cast(*ImportedDC);
+
+  for (auto *D : FromRD->decls()) {
+if (isa(D) || isa(D)) {
+  Decl *ToD = Importer.GetAlreadyImportedOrNull(D);
+  assert(ToRD == ToD->getDeclContext() && ToRD->containsDecl(ToD));
+  ToRD->removeDecl(ToD);
+}
+  }
+
+  assert(ToRD->field_empty());
+
+  for (auto *D : FromRD->decls()) {
+if (isa(D) || isa(D)) {
+  Decl *ToD = Importer.GetAlreadyImportedOrNull(D);
+  assert(ToRD == ToD->getDeclContext());
+  assert(ToRD == ToD->getLexicalDeclContext());
+  assert(!ToRD->containsDecl(ToD));
+  ToRD->addDeclInternal(ToD);
+}
   }
 
   return Error::success();
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1457,7 +1457,7 @@
 }
 
 TEST_P(ASTImporterTestBase,
-   DISABLED_CXXRecordDeclFieldOrderShouldNotDependOnImportOrder) {
+   CXXRecordDeclFieldOrderShouldNotDependOnImportOrder) {
   Decl *From, *To;
   std::tie(From, To) = getImportedDecl(
   // The original recursive algorithm of ASTImporter first imports 'c' then
@@ -3767,5 +3767,16 @@
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportVariables,
 DefaultTestValuesForRunOptions, );
 
+TEST_P(ImportDecl, ImportFieldOrder) {
+  MatchVerifier Verifier;
+  testImport("struct declToImport {"
+ "  int b = a + 2;"
+ "  int a = 5;"
+ "};",
+ Lang_CXX11, "", Lang_CXX11, Verifier,
+ recordDecl(hasFieldOrder({"b", "a"})));
+}
+
+
 } // end namespace ast_matchers
 } // end namespace clang


Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -1658,13 +1658,53 @@
 auto ToDCOrErr = Importer.ImportContext(FromDC);
 return ToDCOrErr.takeError();
   }
-  llvm::SmallVector ImportedDecls;
+
+  const auto *FromRD = dyn_cast(FromDC);
   for (auto *From : FromDC->decls()) {
 ExpectedDecl ImportedOrErr = import(From);
-if (!ImportedOrErr)
+if (!ImportedOrErr) {
+  // For RecordDecls, failed import of a field will break the layout of the
+  // structure. Handle it as an error.
+  if (FromRD)
+return ImportedOrErr.takeError();
   // Ignore the error, continue with next Decl.
   // FIXME: Handle this case somehow better.
-  consumeError(ImportedOrErr.takeError());
+  else
+consumeError(ImportedOrErr.takeError());
+}
+  }
+
+  // Reorder declarations in RecordDecls because they may have another
+  // order. Keeping field order is vitable because it determines structure
+  // layout.
+  // FIXME: This is an ugly fix. Unfortunately, I cannot come with better
+  // solution for this issue. We cannot defer expression import here because
+  // type import can depend on them.
+  if (!FromRD)
+return Error::success();
+
+  auto ImportedDC = import(cast(FromDC));
+  assert(ImportedDC);
+  auto *ToRD = cast(*ImportedDC);
+
+  for (auto *D : FromRD->decls()) {
+if (isa(D) || isa(D)) {
+  Decl *ToD = 

r345545 - [ASTImporter] Reorder fields after structure import is finished

[Aleksei Sidorin via cfe-commits]

Author: a.sidorin
Date: Mon Oct 29 14:46:18 2018
New Revision: 345545

URL: http://llvm.org/viewvc/llvm-project?rev=345545=rev
Log:
[ASTImporter] Reorder fields after structure import is finished

There are multiple reasons why field structures can be imported
in wrong order. The simplest is the ability of field initializers
and method bodies to refer fields not in order they are listed in.
Unfortunately, there is no clean solution for that currently
so I'm leaving a FIXME.

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


Modified:
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/unittests/AST/ASTImporterTest.cpp

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=345545=345544=345545=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Mon Oct 29 14:46:18 2018
@@ -1658,13 +1658,53 @@ ASTNodeImporter::ImportDeclContext(DeclC
 auto ToDCOrErr = Importer.ImportContext(FromDC);
 return ToDCOrErr.takeError();
   }
-  llvm::SmallVector ImportedDecls;
+
+  const auto *FromRD = dyn_cast(FromDC);
   for (auto *From : FromDC->decls()) {
 ExpectedDecl ImportedOrErr = import(From);
-if (!ImportedOrErr)
+if (!ImportedOrErr) {
+  // For RecordDecls, failed import of a field will break the layout of the
+  // structure. Handle it as an error.
+  if (FromRD)
+return ImportedOrErr.takeError();
   // Ignore the error, continue with next Decl.
   // FIXME: Handle this case somehow better.
-  consumeError(ImportedOrErr.takeError());
+  else
+consumeError(ImportedOrErr.takeError());
+}
+  }
+
+  // Reorder declarations in RecordDecls because they may have another
+  // order. Keeping field order is vitable because it determines structure
+  // layout.
+  // FIXME: This is an ugly fix. Unfortunately, I cannot come with better
+  // solution for this issue. We cannot defer expression import here because
+  // type import can depend on them.
+  if (!FromRD)
+return Error::success();
+
+  auto ImportedDC = import(cast(FromDC));
+  assert(ImportedDC);
+  auto *ToRD = cast(*ImportedDC);
+
+  for (auto *D : FromRD->decls()) {
+if (isa(D) || isa(D)) {
+  Decl *ToD = Importer.GetAlreadyImportedOrNull(D);
+  assert(ToRD == ToD->getDeclContext() && ToRD->containsDecl(ToD));
+  ToRD->removeDecl(ToD);
+}
+  }
+
+  assert(ToRD->field_empty());
+
+  for (auto *D : FromRD->decls()) {
+if (isa(D) || isa(D)) {
+  Decl *ToD = Importer.GetAlreadyImportedOrNull(D);
+  assert(ToRD == ToD->getDeclContext());
+  assert(ToRD == ToD->getLexicalDeclContext());
+  assert(!ToRD->containsDecl(ToD));
+  ToRD->addDeclInternal(ToD);
+}
   }
 
   return Error::success();

Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=345545=345544=345545=diff
==
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Mon Oct 29 14:46:18 2018
@@ -1457,7 +1457,7 @@ TEST_P(ASTImporterTestBase, CXXRecordDec
 }
 
 TEST_P(ASTImporterTestBase,
-   DISABLED_CXXRecordDeclFieldOrderShouldNotDependOnImportOrder) {
+   CXXRecordDeclFieldOrderShouldNotDependOnImportOrder) {
   Decl *From, *To;
   std::tie(From, To) = getImportedDecl(
   // The original recursive algorithm of ASTImporter first imports 'c' then
@@ -3767,5 +3767,16 @@ INSTANTIATE_TEST_CASE_P(ParameterizedTes
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportVariables,
 DefaultTestValuesForRunOptions, );
 
+TEST_P(ImportDecl, ImportFieldOrder) {
+  MatchVerifier Verifier;
+  testImport("struct declToImport {"
+ "  int b = a + 2;"
+ "  int a = 5;"
+ "};",
+ Lang_CXX11, "", Lang_CXX11, Verifier,
+ recordDecl(hasFieldOrder({"b", "a"})));
+}
+
+
 } // end namespace ast_matchers
 } // end namespace clang


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

[PATCH] D50269: [compiler-rt][builtins] Don't #include CoreFoundation in os_version_check.c

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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

This LGTM with a couple of nitpicks.




Comment at: compiler-rt/lib/builtins/os_version_check.c:183
   CFStringRef ProductVersion = (*CFStringCreateWithCStringNoCopyFunc)(
-  NULL, "ProductVersion", kCFStringEncodingASCII, kCFAllocatorNull);
+  NULL, "ProductVersion", CF_STRING_ENCODING_ASCII, kCFAllocatorNull);
   if (!ProductVersion)

It wasn't obvious that `kCFAllocatorNull` had been `dlsym`'ed in.  Is there a 
way of naming this that would make it more clear you weren't getting this from 
an `#include`?  Or does it not matter?



Comment at: compiler-rt/lib/builtins/os_version_check.c:211-214
+  if (Major < GlobalMajor) return 1;
+  if (Major > GlobalMajor) return 0;
+  if (Minor < GlobalMinor) return 1;
+  if (Minor > GlobalMinor) return 0;

If you're going to do this, please do it in a separate NFC commit since it 
appears to be whitespace only.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D50269



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

Re: r345109 - Debug Info (-gmodules): emit full types for non-anchored template specializations

[David Blaikie via cfe-commits]

Fair enough - pity we couldn't readily have a single implementation or at
least semantics for modular debug info between implicit and explicit modes
(I mean, my fault in part for building a separate/new system when I did
modular codegen anyway) but hopefully we'll move to explicit modules across
the board in the future anyway & standardize there.

Thanks for the context!

On Mon, Oct 29, 2018 at 12:32 PM Adrian Prantl  wrote:

>
>
> > On Oct 29, 2018, at 11:26 AM, David Blaikie  wrote:
> >
> > Is this a workaround for now with the intent to fix this to allow such
> implicit specializations to have their debug info modularized? I believe
> this does work correctly in modular debug info with expliict modules, would
> probably be sort of nice to have these things be consistent/similar?
>
> It started as a workaround, but I reached the conclusion that it's not
> worthwhile pursuing a more space-efficient solution. Note that all this
> patch does is emit plain old non-modular debug info for non-explicit
> template specializations, so it is definitely safe & conservative. This
> increases the size of the clang module cache in a build of clang by 4MiB
> out of 1GiB total.
>
> As you can read in my thread with Richard, it isn't possible in Clang to
> determine the clang module that contains the complete definition of a
> template specialization inside of a namespace for indirectly imported
> modules (such as in my testcase). That means that a consumer would have to
> look in every Clang module for complete types; not just in the transitive
> closure of imports of the .pcm that has the forward declaration. This
> lookup is expensive and difficult to implement in LLDB, so I decided not to
> pursue this further.
>
> -- adrian
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:36
+  const ProgramStateRef PS;
+  SValBuilder 
+

NoQ wrote:
> ZaMaZaN4iK wrote:
> > Szelethus wrote:
> > > You can acquire `SValBuilder` from `ProgramState`:
> > > `PS->getStateManager()->getSvalBuilder()`
> > Is there any difference? Is it critical to get `SValBuilder` from 
> > `CheckerContext' ?
> There's only one instance of `SValBuilder` in existence at any particular 
> moment of time. The same applies to `BasicValueFactory`, `SymbolManager`, 
> `MemRegionManager`, `ConstraintManager`, `StoreManager`, 
> `ProgramStateManager`, ...
> 
> All these objects live within `ExprEngine` and have the same lifetime.
> 
> `ExprEngine`, together with all these objects, is created from scratch for 
> every analysis of a top-level function.
> 
> AST entities, such ast `ASTContex`, on the contrary, live much longer - only 
> one is created per clang process. That is, until somebody takes `ASTImporter` 
> and tries to frankenstein multiple ASTs into one :)
Then I guess no, it's not critical. ^-^


https://reviews.llvm.org/D33672



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

[PATCH] D53833: [Driver] Include missing touch files for sanitized library paths

[Petr Hosek via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL345540: [Driver] Include missing touch files for sanitized 
library paths (authored by phosek, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D53833?vs=171570=171571#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D53833

Files:
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/aarch64-fuchsia/lib/asan/.keep
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/x86_64-fuchsia/lib/asan/.keep




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

r345540 - [Driver] Include missing touch files for sanitized library paths

[Petr Hosek via cfe-commits]

Author: phosek
Date: Mon Oct 29 14:04:12 2018
New Revision: 345540

URL: http://llvm.org/viewvc/llvm-project?rev=345540=rev
Log:
[Driver] Include missing touch files for sanitized library paths

These were forgotten in r345537 causing test failures on Clang builders.

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

Added:

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/aarch64-fuchsia/lib/asan/

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/aarch64-fuchsia/lib/asan/.keep

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/x86_64-fuchsia/lib/asan/

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/x86_64-fuchsia/lib/asan/.keep

Added: 
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/aarch64-fuchsia/lib/asan/.keep
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/aarch64-fuchsia/lib/asan/.keep?rev=345540=auto
==
(empty)

Added: 
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/x86_64-fuchsia/lib/asan/.keep
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/x86_64-fuchsia/lib/asan/.keep?rev=345540=auto
==
(empty)


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

[PATCH] D53833: [Driver] Include missing touch files for sanitized library paths

[Petr Hosek via Phabricator via cfe-commits]

phosek created this revision.
phosek added reviewers: jakehehrlich, juliehockett.
Herald added subscribers: cfe-commits, javed.absar, kubamracek.

These were forgotten in r345537 causing test failures on Clang builders.


Repository:
  rC Clang

https://reviews.llvm.org/D53833

Files:
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/aarch64-fuchsia/lib/asan/.keep
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/x86_64-fuchsia/lib/asan/.keep




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

[PATCH] D53206: Allow padding checker to traverse simple class hierarchies

[Max Bernstein via Phabricator via cfe-commits]

tekknolagi updated this revision to Diff 171568.
tekknolagi added a comment.

Skip non-definitions in `VisitRecord`


Repository:
  rC Clang

https://reviews.llvm.org/D53206

Files:
  lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
  test/Analysis/padding_inherit.cpp

Index: test/Analysis/padding_inherit.cpp
===
--- /dev/null
+++ test/Analysis/padding_inherit.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_analyze_cc1 -std=c++14 -analyzer-checker=optin.performance -analyzer-config optin.performance.Padding:AllowedPad=20 -verify %s
+
+// A class that has no fields and one base class should visit that base class
+// instead. Note that despite having excess padding of 2, this is flagged
+// because of its usage in an array of 100 elements below (`ais').
+// TODO: Add a note to the bug report with BugReport::addNote() to mention the
+// variable using the class and also mention what class is inherting from what.
+// expected-warning@+1{{Excessive padding in 'struct FakeIntSandwich'}}
+struct FakeIntSandwich {
+  char c1;
+  int i;
+  char c2;
+};
+
+struct AnotherIntSandwich : FakeIntSandwich { // no-warning
+};
+
+// But we don't yet support multiple base classes.
+struct IntSandwich {};
+struct TooManyBaseClasses : FakeIntSandwich, IntSandwich { // no-warning
+};
+
+AnotherIntSandwich ais[100];
+
+struct Empty {};
+struct DoubleEmpty : Empty { // no-warning
+Empty e;
+};
Index: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
+++ lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
@@ -75,6 +75,20 @@
 if (shouldSkipDecl(RD))
   return;
 
+// TODO: Figure out why we are going through declarations and not only
+// definitions.
+if (!(RD = RD->getDefinition()))
+  return;
+
+// This is the simplest correct case: a class with no fields and one base
+// class. Other cases are more complicated because of how the base classes
+// & fields might interact, so we don't bother dealing with them.
+// TODO: Support other combinations of base classes and fields.
+if (auto *CXXRD = dyn_cast(RD))
+  if (CXXRD->field_empty() && CXXRD->getNumBases() == 1)
+return visitRecord(CXXRD->bases().begin()->getType()->getAsRecordDecl(),
+   PadMultiplier);
+
 auto  = RD->getASTContext();
 const ASTRecordLayout  = ASTContext.getASTRecordLayout(RD);
 assert(llvm::isPowerOf2_64(RL.getAlignment().getQuantity()));
@@ -112,12 +126,15 @@
 if (RT == nullptr)
   return;
 
-// TODO: Recurse into the fields and base classes to see if any
-// of those have excess padding.
+// TODO: Recurse into the fields to see if they have excess padding.
 visitRecord(RT->getDecl(), Elts);
   }
 
   bool shouldSkipDecl(const RecordDecl *RD) const {
+// TODO: Figure out why we are going through declarations and not only
+// definitions.
+if (!(RD = RD->getDefinition()))
+  return true;
 auto Location = RD->getLocation();
 // If the construct doesn't have a source file, then it's not something
 // we want to diagnose.
@@ -132,13 +149,14 @@
 // Not going to attempt to optimize unions.
 if (RD->isUnion())
   return true;
-// How do you reorder fields if you haven't got any?
-if (RD->field_empty())
-  return true;
 if (auto *CXXRD = dyn_cast(RD)) {
   // Tail padding with base classes ends up being very complicated.
-  // We will skip objects with base classes for now.
-  if (CXXRD->getNumBases() != 0)
+  // We will skip objects with base classes for now, unless they do not
+  // have fields.
+  // TODO: Handle more base class scenarios.
+  if (!CXXRD->field_empty() && CXXRD->getNumBases() != 0)
+return true;
+  if (CXXRD->field_empty() && CXXRD->getNumBases() != 1)
 return true;
   // Virtual bases are complicated, skipping those for now.
   if (CXXRD->getNumVBases() != 0)
@@ -150,6 +168,10 @@
   if (CXXRD->getTypeForDecl()->isInstantiationDependentType())
 return true;
 }
+// How do you reorder fields if you haven't got any?
+else if (RD->field_empty())
+  return true;
+
 auto IsTrickyField = [](const FieldDecl *FD) -> bool {
   // Bitfield layout is hard.
   if (FD->isBitField())
@@ -323,7 +345,7 @@
 BR->emitReport(std::move(Report));
   }
 };
-}
+} // namespace
 
 void ento::registerPaddingChecker(CheckerManager ) {
   Mgr.registerChecker();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53830: [clang-tidy]: Abseil: new check 'abseil-upgrade-duration-conversions'

[Alex Strelnikov via Phabricator via cfe-commits]

astrelni updated this revision to Diff 171566.
astrelni added a comment.

Reply to comments: add check for language being c++, explicitly name type in 
declaration, alphabetize release notes.


https://reviews.llvm.org/D53830

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp
  clang-tidy/abseil/UpgradeDurationConversionsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-upgrade-duration-conversions.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-upgrade-duration-conversions.cpp

Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -10,8 +10,9 @@
abseil-no-internal-dependencies
abseil-no-namespace
abseil-redundant-strcat-calls
+   abseil-str-cat-append
abseil-string-find-startswith
-   abseil-str-cat-append
+   abseil-upgrade-duration-conversions
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
@@ -151,6 +152,7 @@
hicpp-special-member-functions (redirects to cppcoreguidelines-special-member-functions) 
hicpp-static-assert (redirects to misc-static-assert) 
hicpp-undelegated-constructor (redirects to bugprone-undelegated-constructor) 
+   hicpp-uppercase-literal-suffix (redirects to readability-uppercase-literal-suffix) 
hicpp-use-auto (redirects to modernize-use-auto) 
hicpp-use-emplace (redirects to modernize-use-emplace) 
hicpp-use-equals-default (redirects to modernize-use-equals-default) 
@@ -159,7 +161,6 @@
hicpp-use-nullptr (redirects to modernize-use-nullptr) 
hicpp-use-override (redirects to modernize-use-override) 
hicpp-vararg (redirects to cppcoreguidelines-pro-type-vararg) 
-   hicpp-uppercase-literal-suffix (redirects to readability-uppercase-literal-suffix) 
llvm-header-guard
llvm-include-order
llvm-namespace-comment
Index: docs/clang-tidy/checks/abseil-upgrade-duration-conversions.rst
===
--- docs/clang-tidy/checks/abseil-upgrade-duration-conversions.rst
+++ docs/clang-tidy/checks/abseil-upgrade-duration-conversions.rst
@@ -0,0 +1,43 @@
+.. title:: clang-tidy - abseil-upgrade-duration-conversions
+
+abseil-upgrade-duration-conversions
+===
+
+Finds calls to ``absl::Duration`` arithmetic operators and factories whose
+argument needs an explicit cast to continue compiling after upcoming API
+changes.
+
+The operators ``*=``, ``/=``, ``*``, and ``/`` for ``absl::Duration`` currently
+accept an argument of class type that is convertible to an arithmetic type. Such
+a call currently converts the value to an ``int64_t``, even in a case such as
+``std::atomic`` that would result in lossy conversion.
+
+Additionally, the ``absl::Duration`` factory functions (``absl::Hours``,
+``absl::Minutes``, etc) currently accept an ``int64_t`` or a floating point
+type. Similar to the arithmetic operators, calls with an argument of class type
+that is convertible to an arithmetic type go through the ``int64_t`` path.
+
+The operators and factories will be updated to only accept arithmetic types to
+prevent unintended behavior. Passing an argument of class type will result in
+compile error, even if the type is implicitly convertible to an arithmetic type.
+
+Here are example fixes created by this check:
+
+.. code-block:: c++
+
+  std::atomic a;
+  absl::Duration d = absl::Milliseconds(a);
+  d *= a;
+
+becomes
+
+.. code-block:: c++
+
+  std::atomic a;
+  absl::Duration d = absl::Milliseconds(static_cast(a));
+  d *= static_cast(a);
+
+Note that this check always adds a cast to ``int64_t`` in order to preserve the
+current behavior of user code. It is possible that this uncovers unintended
+behavior due to types implicitly convertible to a floating point type.
+
Index: test/clang-tidy/abseil-upgrade-duration-conversions.cpp
===
--- test/clang-tidy/abseil-upgrade-duration-conversions.cpp
+++ test/clang-tidy/abseil-upgrade-duration-conversions.cpp
@@ -0,0 +1,357 @@
+// RUN: %check_clang_tidy %s abseil-upgrade-duration-conversions %t
+
+using int64_t = long long;
+
+// Partial implementation of relevant APIs from
+// https://github.com/abseil/abseil-cpp/blob/master/absl/time/time.h
+namespace absl {
+
+class Duration {
+public:
+  Duration *=(int64_t r);
+  Duration *=(float r);
+  Duration *=(double r);
+  template  Duration *=(T r);
+
+  Duration /=(int64_t r);
+  Duration /=(float r);
+  Duration /=(double r);
+  template  Duration /=(T r);
+};
+
+template  Duration operator*(Duration lhs, T rhs);
+template  Duration operator*(T lhs, Duration rhs);
+template  Duration operator/(Duration lhs, T rhs);
+
+constexpr Duration Nanoseconds(int64_t n);
+constexpr Duration Microseconds(int64_t n);
+constexpr Duration 

r345537 - [Driver] Support sanitized libraries on Fuchsia

[Petr Hosek via cfe-commits]

Author: phosek
Date: Mon Oct 29 13:37:52 2018
New Revision: 345537

URL: http://llvm.org/viewvc/llvm-project?rev=345537=rev
Log:
[Driver] Support sanitized libraries on Fuchsia

When using sanitizers, add //lib/
to the list of library paths to support using sanitized version of
runtime libraries if available.

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

Modified:
cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
cfe/trunk/lib/Driver/ToolChains/CommonArgs.h
cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp
cfe/trunk/test/Driver/fuchsia.c

Modified: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp?rev=345537=345536=345537=diff
==
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp Mon Oct 29 13:37:52 2018
@@ -560,6 +560,40 @@ static bool addSanitizerDynamicList(cons
   return false;
 }
 
+static void addSanitizerLibPath(const ToolChain , const ArgList ,
+ArgStringList , StringRef Name) {
+  for (const auto  : TC.getLibraryPaths()) {
+if (!LibPath.empty()) {
+  SmallString<128> P(LibPath);
+  llvm::sys::path::append(P, Name);
+  if (TC.getVFS().exists(P))
+CmdArgs.push_back(Args.MakeArgString(StringRef("-L") + P));
+}
+  }
+}
+
+void tools::addSanitizerPathLibArgs(const ToolChain , const ArgList ,
+ArgStringList ) {
+  const SanitizerArgs  = TC.getSanitizerArgs();
+  if (SanArgs.needsAsanRt()) {
+addSanitizerLibPath(TC, Args, CmdArgs, "asan");
+  }
+  if (SanArgs.needsHwasanRt()) {
+addSanitizerLibPath(TC, Args, CmdArgs, "hwasan");
+  }
+  if (SanArgs.needsLsanRt()) {
+addSanitizerLibPath(TC, Args, CmdArgs, "lsan");
+  }
+  if (SanArgs.needsMsanRt()) {
+addSanitizerLibPath(TC, Args, CmdArgs, "msan");
+  }
+  if (SanArgs.needsTsanRt()) {
+addSanitizerLibPath(TC, Args, CmdArgs, "tsan");
+  }
+}
+
+
+
 void tools::linkSanitizerRuntimeDeps(const ToolChain ,
  ArgStringList ) {
   // Force linking against the system libraries sanitizers depends on

Modified: cfe/trunk/lib/Driver/ToolChains/CommonArgs.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/CommonArgs.h?rev=345537=345536=345537=diff
==
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.h (original)
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.h Mon Oct 29 13:37:52 2018
@@ -32,6 +32,10 @@ void claimNoWarnArgs(const llvm::opt::Ar
 bool addSanitizerRuntimes(const ToolChain , const llvm::opt::ArgList ,
   llvm::opt::ArgStringList );
 
+void addSanitizerPathLibArgs(const ToolChain ,
+ const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList );
+
 void linkSanitizerRuntimeDeps(const ToolChain ,
   llvm::opt::ArgStringList );
 

Modified: cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp?rev=345537=345536=345537=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp Mon Oct 29 13:37:52 2018
@@ -76,10 +76,11 @@ void fuchsia::Linker::ConstructJob(Compi
   else if (Args.hasArg(options::OPT_shared))
 CmdArgs.push_back("-shared");
 
+  const SanitizerArgs  = ToolChain.getSanitizerArgs();
+
   if (!Args.hasArg(options::OPT_shared)) {
 std::string Dyld = D.DyldPrefix;
-if (ToolChain.getSanitizerArgs().needsAsanRt() &&
-ToolChain.getSanitizerArgs().needsSharedRt())
+if (SanArgs.needsAsanRt() && SanArgs.needsSharedRt())
   Dyld += "asan/";
 Dyld += "ld.so.1";
 CmdArgs.push_back("-dynamic-linker");
@@ -98,6 +99,8 @@ void fuchsia::Linker::ConstructJob(Compi
   Args.AddAllArgs(CmdArgs, options::OPT_L);
   Args.AddAllArgs(CmdArgs, options::OPT_u);
 
+  addSanitizerPathLibArgs(ToolChain, Args, CmdArgs);
+
   ToolChain.AddFilePathLibArgs(Args, CmdArgs);
 
   if (D.isUsingLTO()) {

Modified: cfe/trunk/test/Driver/fuchsia.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fuchsia.c?rev=345537=345536=345537=diff
==
--- cfe/trunk/test/Driver/fuchsia.c (original)
+++ cfe/trunk/test/Driver/fuchsia.c Mon Oct 29 13:37:52 2018
@@ -66,22 +66,28 @@
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN: -fuse-ld=lld \
 // RUN: | FileCheck %s -check-prefix=CHECK-ASAN-X86
+// CHECK-ASAN-X86: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK-ASAN-X86: "-fsanitize=address"
 // CHECK-ASAN-X86: 

[PATCH] D53487: [Driver] Support sanitized libraries on Fuchsia

[Petr Hosek via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC345537: [Driver] Support sanitized libraries on Fuchsia 
(authored by phosek, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D53487?vs=170805=171564#toc

Repository:
  rC Clang

https://reviews.llvm.org/D53487

Files:
  lib/Driver/ToolChains/CommonArgs.cpp
  lib/Driver/ToolChains/CommonArgs.h
  lib/Driver/ToolChains/Fuchsia.cpp
  test/Driver/fuchsia.c

Index: lib/Driver/ToolChains/CommonArgs.h
===
--- lib/Driver/ToolChains/CommonArgs.h
+++ lib/Driver/ToolChains/CommonArgs.h
@@ -32,6 +32,10 @@
 bool addSanitizerRuntimes(const ToolChain , const llvm::opt::ArgList ,
   llvm::opt::ArgStringList );
 
+void addSanitizerPathLibArgs(const ToolChain ,
+ const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList );
+
 void linkSanitizerRuntimeDeps(const ToolChain ,
   llvm::opt::ArgStringList );
 
Index: lib/Driver/ToolChains/Fuchsia.cpp
===
--- lib/Driver/ToolChains/Fuchsia.cpp
+++ lib/Driver/ToolChains/Fuchsia.cpp
@@ -76,10 +76,11 @@
   else if (Args.hasArg(options::OPT_shared))
 CmdArgs.push_back("-shared");
 
+  const SanitizerArgs  = ToolChain.getSanitizerArgs();
+
   if (!Args.hasArg(options::OPT_shared)) {
 std::string Dyld = D.DyldPrefix;
-if (ToolChain.getSanitizerArgs().needsAsanRt() &&
-ToolChain.getSanitizerArgs().needsSharedRt())
+if (SanArgs.needsAsanRt() && SanArgs.needsSharedRt())
   Dyld += "asan/";
 Dyld += "ld.so.1";
 CmdArgs.push_back("-dynamic-linker");
@@ -98,6 +99,8 @@
   Args.AddAllArgs(CmdArgs, options::OPT_L);
   Args.AddAllArgs(CmdArgs, options::OPT_u);
 
+  addSanitizerPathLibArgs(ToolChain, Args, CmdArgs);
+
   ToolChain.AddFilePathLibArgs(Args, CmdArgs);
 
   if (D.isUsingLTO()) {
Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -560,6 +560,40 @@
   return false;
 }
 
+static void addSanitizerLibPath(const ToolChain , const ArgList ,
+ArgStringList , StringRef Name) {
+  for (const auto  : TC.getLibraryPaths()) {
+if (!LibPath.empty()) {
+  SmallString<128> P(LibPath);
+  llvm::sys::path::append(P, Name);
+  if (TC.getVFS().exists(P))
+CmdArgs.push_back(Args.MakeArgString(StringRef("-L") + P));
+}
+  }
+}
+
+void tools::addSanitizerPathLibArgs(const ToolChain , const ArgList ,
+ArgStringList ) {
+  const SanitizerArgs  = TC.getSanitizerArgs();
+  if (SanArgs.needsAsanRt()) {
+addSanitizerLibPath(TC, Args, CmdArgs, "asan");
+  }
+  if (SanArgs.needsHwasanRt()) {
+addSanitizerLibPath(TC, Args, CmdArgs, "hwasan");
+  }
+  if (SanArgs.needsLsanRt()) {
+addSanitizerLibPath(TC, Args, CmdArgs, "lsan");
+  }
+  if (SanArgs.needsMsanRt()) {
+addSanitizerLibPath(TC, Args, CmdArgs, "msan");
+  }
+  if (SanArgs.needsTsanRt()) {
+addSanitizerLibPath(TC, Args, CmdArgs, "tsan");
+  }
+}
+
+
+
 void tools::linkSanitizerRuntimeDeps(const ToolChain ,
  ArgStringList ) {
   // Force linking against the system libraries sanitizers depends on
Index: test/Driver/fuchsia.c
===
--- test/Driver/fuchsia.c
+++ test/Driver/fuchsia.c
@@ -66,22 +66,28 @@
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN: -fuse-ld=lld \
 // RUN: | FileCheck %s -check-prefix=CHECK-ASAN-X86
+// CHECK-ASAN-X86: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK-ASAN-X86: "-fsanitize=address"
 // CHECK-ASAN-X86: "-fsanitize-address-globals-dead-stripping"
 // CHECK-ASAN-X86: "-dynamic-linker" "asan/ld.so.1"
-// CHECK-ASAN-X86: "{{.*[/\\]}}libclang_rt.asan.so"
-// CHECK-ASAN-X86: "{{.*[/\\]}}libclang_rt.asan-preinit.a"
+// CHECK-ASAN-X86: "-L[[RESOURCE_DIR]]{{/|}}x86_64-fuchsia{{/|}}lib{{/|}}asan"
+// CHECK-ASAN-X86: "-L[[RESOURCE_DIR]]{{/|}}x86_64-fuchsia{{/|}}lib"
+// CHECK-ASAN-X86: "[[RESOURCE_DIR]]{{/|}}x86_64-fuchsia{{/|}}lib{{/|}}libclang_rt.asan.so"
+// CHECK-ASAN-X86: "[[RESOURCE_DIR]]{{/|}}x86_64-fuchsia{{/|}}lib{{/|}}libclang_rt.asan-preinit.a"
 
 // RUN: %clang %s -### --target=aarch64-fuchsia \
 // RUN: -fsanitize=address 2>&1 \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN: -fuse-ld=lld \
 // RUN: | FileCheck %s -check-prefix=CHECK-ASAN-AARCH64
+// CHECK-ASAN-AARCH64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK-ASAN-AARCH64: "-fsanitize=address"
 // CHECK-ASAN-AARCH64: "-fsanitize-address-globals-dead-stripping"
 // CHECK-ASAN-AARCH64: "-dynamic-linker" 

r345536 - In swiftcall, don't merge FP/vector types within a chunk.

[John McCall via cfe-commits]

Author: rjmccall
Date: Mon Oct 29 13:32:36 2018
New Revision: 345536

URL: http://llvm.org/viewvc/llvm-project?rev=345536=rev
Log:
In swiftcall, don't merge FP/vector types within a chunk.

Modified:
cfe/trunk/include/clang/CodeGen/SwiftCallingConv.h
cfe/trunk/lib/CodeGen/SwiftCallingConv.cpp
cfe/trunk/test/CodeGen/64bit-swiftcall.c
cfe/trunk/test/CodeGen/windows-swiftcall.c

Modified: cfe/trunk/include/clang/CodeGen/SwiftCallingConv.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/CodeGen/SwiftCallingConv.h?rev=345536=345535=345536=diff
==
--- cfe/trunk/include/clang/CodeGen/SwiftCallingConv.h (original)
+++ cfe/trunk/include/clang/CodeGen/SwiftCallingConv.h Mon Oct 29 13:32:36 2018
@@ -114,6 +114,9 @@ private:
   void addLegalTypedData(llvm::Type *type, CharUnits begin, CharUnits end);
   void addEntry(llvm::Type *type, CharUnits begin, CharUnits end);
   void splitVectorEntry(unsigned index);
+  static bool shouldMergeEntries(const StorageEntry ,
+ const StorageEntry ,
+ CharUnits chunkSize);
 };
 
 /// Should an aggregate which expands to the given type sequence

Modified: cfe/trunk/lib/CodeGen/SwiftCallingConv.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/SwiftCallingConv.cpp?rev=345536=345535=345536=diff
==
--- cfe/trunk/lib/CodeGen/SwiftCallingConv.cpp (original)
+++ cfe/trunk/lib/CodeGen/SwiftCallingConv.cpp Mon Oct 29 13:32:36 2018
@@ -415,6 +415,40 @@ static bool areBytesInSameUnit(CharUnits
   == getOffsetAtStartOfUnit(second, chunkSize);
 }
 
+static bool isMergeableEntryType(llvm::Type *type) {
+  // Opaquely-typed memory is always mergeable.
+  if (type == nullptr) return true;
+
+  // Pointers and integers are always mergeable.  In theory we should not
+  // merge pointers, but (1) it doesn't currently matter in practice because
+  // the chunk size is never greater than the size of a pointer and (2)
+  // Swift IRGen uses integer types for a lot of things that are "really"
+  // just storing pointers (like Optional).  If we ever have a
+  // target that would otherwise combine pointers, we should put some effort
+  // into fixing those cases in Swift IRGen and then call out pointer types
+  // here.
+
+  // Floating-point and vector types should never be merged.
+  // Most such types are too large and highly-aligned to ever trigger merging
+  // in practice, but it's important for the rule to cover at least 'half'
+  // and 'float', as well as things like small vectors of 'i1' or 'i8'.
+  return (!type->isFloatingPointTy() && !type->isVectorTy());
+}
+
+bool SwiftAggLowering::shouldMergeEntries(const StorageEntry ,
+  const StorageEntry ,
+  CharUnits chunkSize) {
+  // Only merge entries that overlap the same chunk.  We test this first
+  // despite being a bit more expensive because this is the condition that
+  // tends to prevent merging.
+  if (!areBytesInSameUnit(first.End - CharUnits::One(), second.Begin,
+  chunkSize))
+return false;
+
+  return (isMergeableEntryType(first.Type) &&
+  isMergeableEntryType(second.Type));
+}
+
 void SwiftAggLowering::finish() {
   if (Entries.empty()) {
 Finished = true;
@@ -425,12 +459,12 @@ void SwiftAggLowering::finish() {
   // which is generally the size of a pointer.
   const CharUnits chunkSize = getMaximumVoluntaryIntegerSize(CGM);
 
-  // First pass: if two entries share a chunk, make them both opaque
+  // First pass: if two entries should be merged, make them both opaque
   // and stretch one to meet the next.
+  // Also, remember if there are any opaque entries.
   bool hasOpaqueEntries = (Entries[0].Type == nullptr);
   for (size_t i = 1, e = Entries.size(); i != e; ++i) {
-if (areBytesInSameUnit(Entries[i - 1].End - CharUnits::One(),
-   Entries[i].Begin, chunkSize)) {
+if (shouldMergeEntries(Entries[i - 1], Entries[i], chunkSize)) {
   Entries[i - 1].Type = nullptr;
   Entries[i].Type = nullptr;
   Entries[i - 1].End = Entries[i].Begin;

Modified: cfe/trunk/test/CodeGen/64bit-swiftcall.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/64bit-swiftcall.c?rev=345536=345535=345536=diff
==
--- cfe/trunk/test/CodeGen/64bit-swiftcall.c (original)
+++ cfe/trunk/test/CodeGen/64bit-swiftcall.c Mon Oct 29 13:32:36 2018
@@ -10,7 +10,7 @@
 #define ERROR __attribute__((swift_error_result))
 #define CONTEXT __attribute__((swift_context))
 
-// CHECK: [[STRUCT2_RESULT:@.*]] = private {{.*}} constant 
[[STRUCT2_TYPE:%.*]] { i32 0, i8 0, i8 undef, i8 0, float 0.00e+00, float 
0.00e+00 }
+// CHECK: [[STRUCT2_RESULT:@.*]] = 

[PATCH] D53830: [clang-tidy]: Abseil: new check 'abseil-upgrade-duration-conversions'

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:22
+ast_matchers::MatchFinder *Finder) {
+  // For the arithmetic calls, we match only the uses of the templated 
operators
+  // where the template parameter is not a built-in type. This means the

Please add check for C++. See other Abseil checks code as example.



Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:136
+
+  auto SourceRange = Lexer::makeFileCharRange(
+  CharSourceRange::getTokenRange(ArgExpr->getSourceRange()),

Please don't use auto, since return type is not obvious.



Comment at: docs/ReleaseNotes.rst:70
 
+- New :doc:`abseil-upgrade-duration-conversions
+  ` check.

Please sort new checks list alphabetically.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53830



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

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.

Thanks! I like where this is going. Let's land the patch and continue 
developing it incrementally in trunk.

The next steps for this checker are, in my opinion:

- Do the visitor thingy that i requested in inline-311373 
. I think it's a necessary thing 
to do, but don't jump into implementing it right away: i already have some code 
for this that i want to share.
- Play nicely with typedefs. For now i believe the checker ignores them because 
you can't cast `TypedefType` to `EnumType`. Once this is done, it will be worth 
it to include the name of the enum in the warning message.
- Optimize the code using `assumeInclusiveRange`. Because `assume` is an 
expensive operation, i'd like to avoid doing it O(n) times for contiguous enums 
in which just 2 `assume`s are enough (or, even better, as single 
`assumeInclusiveRange`).
- See how this checker performs on real code, fix crashes and false positives 
if any.




Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:36
+  const ProgramStateRef PS;
+  SValBuilder 
+

ZaMaZaN4iK wrote:
> Szelethus wrote:
> > You can acquire `SValBuilder` from `ProgramState`:
> > `PS->getStateManager()->getSvalBuilder()`
> Is there any difference? Is it critical to get `SValBuilder` from 
> `CheckerContext' ?
There's only one instance of `SValBuilder` in existence at any particular 
moment of time. The same applies to `BasicValueFactory`, `SymbolManager`, 
`MemRegionManager`, `ConstraintManager`, `StoreManager`, `ProgramStateManager`, 
...

All these objects live within `ExprEngine` and have the same lifetime.

`ExprEngine`, together with all these objects, is created from scratch for 
every analysis of a top-level function.

AST entities, such ast `ASTContex`, on the contrary, live much longer - only 
one is created per clang process. That is, until somebody takes `ASTImporter` 
and tries to frankenstein multiple ASTs into one :)



Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:84-86
+  new BuiltinBug(this, "enum cast out of range",
+ "the value provided to the cast expression is not in "
+ "the valid range of values for the enum"));

> diagnostics should not be full sentences or grammatically correct, so drop 
> the capitalization and full stop

No, in fact, Static Analyzer diagnostics are traditionally capitalized, unlike 
other warnings, so i'm afraid this will need to be changed back >.< Still no 
fullstop though.


https://reviews.llvm.org/D33672



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

[PATCH] D53830: [clang-tidy]: Abseil: new check 'upgrade-duration-conversions'

[Alex Strelnikov via Phabricator via cfe-commits]

astrelni created this revision.
astrelni added reviewers: ahedberg, hokein.
astrelni added a project: clang-tools-extra.
Herald added subscribers: jfb, xazax.hun, mgorny.

Introduce a new check to upgrade user code based on upcoming API breaking 
changes to absl::Duration.

The check finds calls to arithmetic operators and factory functions for 
absl::Duration that rely on an implicit user defined conversion to int64_t. 
These cases will no longer compile after proposed changes are released. 
Suggested fixes explicitly cast the argument int64_t.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53830

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp
  clang-tidy/abseil/UpgradeDurationConversionsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-upgrade-duration-conversions.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-upgrade-duration-conversions.cpp

Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -10,8 +10,9 @@
abseil-no-internal-dependencies
abseil-no-namespace
abseil-redundant-strcat-calls
+   abseil-str-cat-append
abseil-string-find-startswith
-   abseil-str-cat-append
+   abseil-upgrade-duration-conversions
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
@@ -151,6 +152,7 @@
hicpp-special-member-functions (redirects to cppcoreguidelines-special-member-functions) 
hicpp-static-assert (redirects to misc-static-assert) 
hicpp-undelegated-constructor (redirects to bugprone-undelegated-constructor) 
+   hicpp-uppercase-literal-suffix (redirects to readability-uppercase-literal-suffix) 
hicpp-use-auto (redirects to modernize-use-auto) 
hicpp-use-emplace (redirects to modernize-use-emplace) 
hicpp-use-equals-default (redirects to modernize-use-equals-default) 
@@ -159,7 +161,6 @@
hicpp-use-nullptr (redirects to modernize-use-nullptr) 
hicpp-use-override (redirects to modernize-use-override) 
hicpp-vararg (redirects to cppcoreguidelines-pro-type-vararg) 
-   hicpp-uppercase-literal-suffix (redirects to readability-uppercase-literal-suffix) 
llvm-header-guard
llvm-include-order
llvm-namespace-comment
Index: docs/clang-tidy/checks/abseil-upgrade-duration-conversions.rst
===
--- docs/clang-tidy/checks/abseil-upgrade-duration-conversions.rst
+++ docs/clang-tidy/checks/abseil-upgrade-duration-conversions.rst
@@ -0,0 +1,43 @@
+.. title:: clang-tidy - abseil-upgrade-duration-conversions
+
+abseil-upgrade-duration-conversions
+===
+
+Finds calls to ``absl::Duration`` arithmetic operators and factories whose
+argument needs an explicit cast to continue compiling after upcoming API
+changes.
+
+The operators ``*=``, ``/=``, ``*``, and ``/`` for ``absl::Duration`` currently
+accept an argument of class type that is convertible to an arithmetic type. Such
+a call currently converts the value to an ``int64_t``, even in a case such as
+``std::atomic`` that would result in lossy conversion.
+
+Additionally, the ``absl::Duration`` factory functions (``absl::Hours``,
+``absl::Minutes``, etc) currently accept an ``int64_t`` or a floating point
+type. Similar to the arithmetic operators, calls with an argument of class type
+that is convertible to an arithmetic type go through the ``int64_t`` path.
+
+The operators and factories will be updated to only accept arithmetic types to
+prevent unintended behavior. Passing an argument of class type will result in
+compile error, even if the type is implicitly convertible to an arithmetic type.
+
+Here are example fixes created by this check:
+
+.. code-block:: c++
+
+  std::atomic a;
+  absl::Duration d = absl::Milliseconds(a);
+  d *= a;
+
+becomes
+
+.. code-block:: c++
+
+  std::atomic a;
+  absl::Duration d = absl::Milliseconds(static_cast(a));
+  d *= static_cast(a);
+
+Note that this check always adds a cast to ``int64_t`` in order to preserve the
+current behavior of user code. It is possible that this uncovers unintended
+behavior due to types implicitly convertible to a floating point type.
+
Index: test/clang-tidy/abseil-upgrade-duration-conversions.cpp
===
--- test/clang-tidy/abseil-upgrade-duration-conversions.cpp
+++ test/clang-tidy/abseil-upgrade-duration-conversions.cpp
@@ -0,0 +1,357 @@
+// RUN: %check_clang_tidy %s abseil-upgrade-duration-conversions %t
+
+using int64_t = long long;
+
+// Partial implementation of relevant APIs from
+// https://github.com/abseil/abseil-cpp/blob/master/absl/time/time.h
+namespace absl {
+
+class Duration {
+public:
+  Duration *=(int64_t r);
+  Duration *=(float r);
+  Duration *=(double r);
+  template  Duration *=(T 

[PATCH] D53524: [ThinLTO] Enable LTOUnit only when it is needed

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

In https://reviews.llvm.org/D53524#1279288, @tejohnson wrote:

> In https://reviews.llvm.org/D53524#1276038, @pcc wrote:
>
> > In https://reviews.llvm.org/D53524#1274505, @tejohnson wrote:
> >
> > > In https://reviews.llvm.org/D53524#1271387, @tejohnson wrote:
> > >
> > > > In https://reviews.llvm.org/D53524#1271357, @pcc wrote:
> > > >
> > > > > The reason why LTO unit is always enabled is so that you can link 
> > > > > translation units compiled with `-fsanitize=cfi` and/or 
> > > > > `-fwhole-program-vtables` against translation units compiled without 
> > > > > CFI/WPD. With this change we will see miscompiles in the translation 
> > > > > units compiled with CFI/WPD if they use vtables in the translation 
> > > > > units compiled without CFI/WPD. If we really need this option I think 
> > > > > it should be an opt out.
> > > >
> > > >
> > > > Is there an important use case for support thing mixing and matching? 
> > > > The issue is that it comes at a cost to all ThinLTO compiles for codes 
> > > > with vtables by requiring them all to process IR during the thin link.
> > >
> > >
> > > Ping on the question of why this mode needs to be default. If it was a 
> > > matter of a few percent overhead that would be one thing, but we're 
> > > talking a *huge* overhead (as noted off-patch for my app I'm seeing >20x 
> > > thin link time currently, and with improvements to the hashing to always 
> > > get successful splitting we could potentially get down to closer to 2x - 
> > > still a big overhead). This kind of overhead should be opt-in. The 
> > > average ThinLTO user is not going to realize they are paying a big 
> > > overhead because CFI is always pre-enabled.
> >
> >
> > Well, the intent was always that the overhead would be minimal, which is 
> > why things are set up the way that they are. But it doesn't sound like 
> > anyone is going to have the time to fully address the performance problems 
> > that you've seen any time soon, so maybe it would be fine to introduce the 
> > -flto-unit flag. I guess we can always change the flag so that it has no 
> > effect if/when the performance problem is addressed.
>
>
> Just to clarify, since there is already a -flto-unit flag: it is currently a 
> cc1 flag, did you want it made into a driver option as well?


Yes, that's what I had in mind.

 Can we detect that TUs compiled with -flto-unit are being mixed with those 
 not built without -flto-unit at the thin link time and issue an error?
>>> 
>>> This would be doable pretty easily. E.g. add a flag at the index level that 
>>> the module would have been split but wasn't. Users who get the error and 
>>> want to support always-enabled CFI could opt in via -flto-unit.
>> 
>> Yes. I don't think we should make a change like this unless there is 
>> something like that in place, though. The documentation (LTOVisibility.rst) 
>> needs to be updated too.
> 
> Ok, let me work on that now and we can get that in before this one.




Repository:
  rC Clang

https://reviews.llvm.org/D53524



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

[PATCH] D53524: [ThinLTO] Enable LTOUnit only when it is needed

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In https://reviews.llvm.org/D53524#1276038, @pcc wrote:

> In https://reviews.llvm.org/D53524#1274505, @tejohnson wrote:
>
> > In https://reviews.llvm.org/D53524#1271387, @tejohnson wrote:
> >
> > > In https://reviews.llvm.org/D53524#1271357, @pcc wrote:
> > >
> > > > The reason why LTO unit is always enabled is so that you can link 
> > > > translation units compiled with `-fsanitize=cfi` and/or 
> > > > `-fwhole-program-vtables` against translation units compiled without 
> > > > CFI/WPD. With this change we will see miscompiles in the translation 
> > > > units compiled with CFI/WPD if they use vtables in the translation 
> > > > units compiled without CFI/WPD. If we really need this option I think 
> > > > it should be an opt out.
> > >
> > >
> > > Is there an important use case for support thing mixing and matching? The 
> > > issue is that it comes at a cost to all ThinLTO compiles for codes with 
> > > vtables by requiring them all to process IR during the thin link.
> >
> >
> > Ping on the question of why this mode needs to be default. If it was a 
> > matter of a few percent overhead that would be one thing, but we're talking 
> > a *huge* overhead (as noted off-patch for my app I'm seeing >20x thin link 
> > time currently, and with improvements to the hashing to always get 
> > successful splitting we could potentially get down to closer to 2x - still 
> > a big overhead). This kind of overhead should be opt-in. The average 
> > ThinLTO user is not going to realize they are paying a big overhead because 
> > CFI is always pre-enabled.
>
>
> Well, the intent was always that the overhead would be minimal, which is why 
> things are set up the way that they are. But it doesn't sound like anyone is 
> going to have the time to fully address the performance problems that you've 
> seen any time soon, so maybe it would be fine to introduce the -flto-unit 
> flag. I guess we can always change the flag so that it has no effect if/when 
> the performance problem is addressed.


Just to clarify, since there is already a -flto-unit flag: it is currently a 
cc1 flag, did you want it made into a driver option as well?

> 
> 
>>> Can we detect that TUs compiled with -flto-unit are being mixed with those 
>>> not built without -flto-unit at the thin link time and issue an error?
>> 
>> This would be doable pretty easily. E.g. add a flag at the index level that 
>> the module would have been split but wasn't. Users who get the error and 
>> want to support always-enabled CFI could opt in via -flto-unit.
> 
> Yes. I don't think we should make a change like this unless there is 
> something like that in place, though. The documentation (LTOVisibility.rst) 
> needs to be updated too.

Ok, let me work on that now and we can get that in before this one.


Repository:
  rC Clang

https://reviews.llvm.org/D53524



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

[PATCH] D53810: [analyzer][NFC] Refactor PlistDiagnostics to use a class instead of passing 9 parameters around

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus closed this revision.
Szelethus added a comment.

Commited in https://reviews.llvm.org/rC345531. Oops.


https://reviews.llvm.org/D53810



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

r345531 - [analyzer][NFC] Refactor PlistDiagnostics to use a class instead of passing 9 parameters around

[Kristof Umann via cfe-commits]

Author: szelethus
Date: Mon Oct 29 13:06:30 2018
New Revision: 345531

URL: http://llvm.org/viewvc/llvm-project?rev=345531=rev
Log:
[analyzer][NFC] Refactor PlistDiagnostics to use a class instead of passing 9 
parameters around

This has been a long time coming. Note the usage of AnalyzerOptions: I'll need
it for D52742, and added it in rC343620. The main motivation for this was that
I'll need to add yet another parameter to every single function, and some
functions would reach their 10th parameter with that change.

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp?rev=345531=345530=345531=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp Mon Oct 29 13:06:30 
2018
@@ -24,10 +24,16 @@
 #include "llvm/ADT/Statistic.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Casting.h"
+
 using namespace clang;
 using namespace ento;
 using namespace markup;
 
+//===--===//
+// Declarations of helper classes and functions for emitting bug reports in
+// plist format.
+//===--===//
+
 namespace {
   class PlistDiagnostics : public PathDiagnosticConsumer {
 const std::string OutputFile;
@@ -59,34 +65,91 @@ namespace {
   };
 } // end anonymous namespace
 
-PlistDiagnostics::PlistDiagnostics(AnalyzerOptions ,
-   const std::string& output,
-   const Preprocessor ,
-   bool supportsMultipleFiles)
-  : OutputFile(output), PP(PP), AnOpts(AnalyzerOpts),
-SupportsCrossFileDiagnostics(supportsMultipleFiles) {}
+namespace {
 
-void ento::createPlistDiagnosticConsumer(AnalyzerOptions ,
- PathDiagnosticConsumers ,
- const std::string& s,
- const Preprocessor ) {
-  C.push_back(new PlistDiagnostics(AnalyzerOpts, s, PP,
-   /*supportsMultipleFiles*/ false));
-}
+/// A helper class for emitting a single report.
+class PlistPrinter {
+  const FIDMap& FM;
+  AnalyzerOptions 
+  const Preprocessor 
+
+public:
+  PlistPrinter(const FIDMap& FM, AnalyzerOptions ,
+   const Preprocessor )
+: FM(FM), AnOpts(AnOpts), PP(PP) {
+  }
 
-void ento::createPlistMultiFileDiagnosticConsumer(AnalyzerOptions 
,
-  PathDiagnosticConsumers ,
-  const std::string ,
-  const Preprocessor ) {
-  C.push_back(new PlistDiagnostics(AnalyzerOpts, s, PP,
-   /*supportsMultipleFiles*/ true));
-}
+  void ReportDiag(raw_ostream , const PathDiagnosticPiece& P) {
+ReportPiece(o, P, /*indent*/ 4, /*depth*/ 0, /*includeControlFlow*/ true);
+
+// Don't emit a warning about an unused private field.
+(void)AnOpts;
+  }
+
+private:
+  void ReportPiece(raw_ostream , const PathDiagnosticPiece ,
+   unsigned indent, unsigned depth, bool includeControlFlow,
+   bool isKeyEvent = false) {
+switch (P.getKind()) {
+  case PathDiagnosticPiece::ControlFlow:
+if (includeControlFlow)
+  ReportControlFlow(o, cast(P), 
indent);
+break;
+  case PathDiagnosticPiece::Call:
+ReportCall(o, cast(P), indent,
+   depth);
+break;
+  case PathDiagnosticPiece::Event:
+ReportEvent(o, cast(P), indent, depth,
+isKeyEvent);
+break;
+  case PathDiagnosticPiece::Macro:
+ReportMacro(o, cast(P), indent, depth);
+break;
+  case PathDiagnosticPiece::Note:
+ReportNote(o, cast(P), indent);
+break;
+}
+  }
+
+  void EmitRanges(raw_ostream , const ArrayRef Ranges,
+  unsigned indent);
+  void EmitMessage(raw_ostream , StringRef Message, unsigned indent);
+
+  void ReportControlFlow(raw_ostream ,
+ const PathDiagnosticControlFlowPiece& P,
+ unsigned indent);
+  void ReportEvent(raw_ostream , const PathDiagnosticEventPiece& P,
+   unsigned indent, unsigned depth, bool isKeyEvent = false);
+  void ReportCall(raw_ostream , const PathDiagnosticCallPiece ,
+  unsigned indent, unsigned depth);
+  void ReportMacro(raw_ostream , const PathDiagnosticMacroPiece& P,
+   unsigned indent, unsigned depth);
+  void ReportNote(raw_ostream , const PathDiagnosticNotePiece& P,
+  

[PATCH] D53514: os_log: make buffer size an integer constant expression.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added subscribers: vsk, efriedma.
efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D53514



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

[PATCH] D53827: [OpenMP] Fix condition.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC345527: [OpenMP] Fix condition. (authored by gbercea, 
committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D53827?vs=171558=171559#toc

Repository:
  rC Clang

https://reviews.llvm.org/D53827

Files:
  lib/Sema/SemaOpenMP.cpp
  test/OpenMP/distribute_parallel_for_codegen.cpp
  test/OpenMP/distribute_parallel_for_simd_codegen.cpp
  test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp


Index: test/OpenMP/distribute_parallel_for_codegen.cpp
===
--- test/OpenMP/distribute_parallel_for_codegen.cpp
+++ test/OpenMP/distribute_parallel_for_codegen.cpp
@@ -447,7 +447,7 @@
   // LAMBDA-DAG: [[OMP_IV_VAL_1:%.+]] = load {{.+}} [[OMP_IV]],
   // LAMBDA-DAG: [[OMP_UB_VAL_3:%.+]] = load {{.+}}
   // LAMBDA-DAG: [[OMP_UB_VAL_3_PLUS_ONE:%.+]] = add {{.+}} 
[[OMP_UB_VAL_3]], 1
-  // LAMBDA: [[CMP_IV_UB:%.+]] = icmp sle {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
+  // LAMBDA: [[CMP_IV_UB:%.+]] = icmp slt {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
   // LAMBDA: br {{.+}} [[CMP_IV_UB]], label %[[DIST_INNER_LOOP_BODY:.+]], 
label %[[DIST_INNER_LOOP_END:.+]]
 
   // check that PrevLB and PrevUB are passed to the 'for'
@@ -1210,7 +1210,7 @@
 // CHECK-DAG: [[OMP_IV_VAL_1:%.+]] = load {{.+}} [[OMP_IV]],
 // CHECK-DAG: [[OMP_UB_VAL_3:%.+]] = load {{.+}}
 // CHECK-DAG: [[OMP_UB_VAL_3_PLUS_ONE:%.+]] = add {{.+}} [[OMP_UB_VAL_3]], 
1
-// CHECK: [[CMP_IV_UB:%.+]] = icmp sle {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
+// CHECK: [[CMP_IV_UB:%.+]] = icmp slt {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
 // CHECK: br {{.+}} [[CMP_IV_UB]], label %[[DIST_INNER_LOOP_BODY:.+]], 
label %[[DIST_INNER_LOOP_END:.+]]
 
 // check that PrevLB and PrevUB are passed to the 'for'
@@ -1938,7 +1938,7 @@
 // CHECK-DAG: [[OMP_IV_VAL_1:%.+]] = load {{.+}} [[OMP_IV]],
 // CHECK-DAG: [[OMP_UB_VAL_3:%.+]] = load {{.+}}
 // CHECK-DAG: [[OMP_UB_VAL_3_PLUS_ONE:%.+]] = add {{.+}} [[OMP_UB_VAL_3]], 1
-// CHECK: [[CMP_IV_UB:%.+]] = icmp sle {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
+// CHECK: [[CMP_IV_UB:%.+]] = icmp slt {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
 // CHECK: br {{.+}} [[CMP_IV_UB]], label %[[DIST_INNER_LOOP_BODY:.+]], label 
%[[DIST_INNER_LOOP_END:.+]]
 
 // check that PrevLB and PrevUB are passed to the 'for'
Index: test/OpenMP/distribute_parallel_for_simd_codegen.cpp
===
--- test/OpenMP/distribute_parallel_for_simd_codegen.cpp
+++ test/OpenMP/distribute_parallel_for_simd_codegen.cpp
@@ -446,7 +446,7 @@
   // LAMBDA-DAG: [[OMP_IV_VAL_1:%.+]] = load {{.+}} [[OMP_IV]],
   // LAMBDA-DAG: [[OMP_UB_VAL_3:%.+]] = load {{.+}}
   // LAMBDA-DAG: [[OMP_UB_VAL_3_PLUS_ONE:%.+]] = add {{.+}} 
[[OMP_UB_VAL_3]], 1
-  // LAMBDA: [[CMP_IV_UB:%.+]] = icmp sle {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
+  // LAMBDA: [[CMP_IV_UB:%.+]] = icmp slt {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
   // LAMBDA: br {{.+}} [[CMP_IV_UB]], label %[[DIST_INNER_LOOP_BODY:.+]], 
label %[[DIST_INNER_LOOP_END:.+]]
 
   // check that PrevLB and PrevUB are passed to the 'for'
@@ -1209,7 +1209,7 @@
 // CHECK-DAG: [[OMP_IV_VAL_1:%.+]] = load {{.+}} [[OMP_IV]],
 // CHECK-DAG: [[OMP_UB_VAL_3:%.+]] = load {{.+}}
 // CHECK-DAG: [[OMP_UB_VAL_3_PLUS_ONE:%.+]] = add {{.+}} [[OMP_UB_VAL_3]], 
1
-// CHECK: [[CMP_IV_UB:%.+]] = icmp sle {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
+// CHECK: [[CMP_IV_UB:%.+]] = icmp slt {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
 // CHECK: br {{.+}} [[CMP_IV_UB]], label %[[DIST_INNER_LOOP_BODY:.+]], 
label %[[DIST_INNER_LOOP_END:.+]]
 
 // check that PrevLB and PrevUB are passed to the 'for'
@@ -1937,7 +1937,7 @@
 // CHECK-DAG: [[OMP_IV_VAL_1:%.+]] = load {{.+}} [[OMP_IV]],
 // CHECK-DAG: [[OMP_UB_VAL_3:%.+]] = load {{.+}}
 // CHECK-DAG: [[OMP_UB_VAL_3_PLUS_ONE:%.+]] = add {{.+}} [[OMP_UB_VAL_3]], 1
-// CHECK: [[CMP_IV_UB:%.+]] = icmp sle {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
+// CHECK: [[CMP_IV_UB:%.+]] = icmp slt {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
 // CHECK: br {{.+}} [[CMP_IV_UB]], label %[[DIST_INNER_LOOP_BODY:.+]], label 
%[[DIST_INNER_LOOP_END:.+]]
 
 // check that PrevLB and PrevUB are passed to the 'for'
Index: test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
===
--- test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
+++ test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
@@ -149,7 +149,7 @@
 
 // check exit condition
 // CHECK-DAG: [[OMP_IV_VAL_1:%.+]] = load {{.+}} [[OMP_IV]],
-// CHECK: [[CMP_IV_UB:%.+]] = icmp sle {{.+}} [[OMP_IV_VAL_1]], 100
+// CHECK: [[CMP_IV_UB:%.+]] = 

r345527 - [OpenMP] Fix condition.

[Gheorghe-Teodor Bercea via cfe-commits]

Author: gbercea
Date: Mon Oct 29 12:44:25 2018
New Revision: 345527

URL: http://llvm.org/viewvc/llvm-project?rev=345527=rev
Log:
[OpenMP] Fix condition.

Summary: Iteration variable must be strictly less than the number of 
iterations. This fixes a bug introduced by previous patch D53448.

Reviewers: ABataev, caomhin

Reviewed By: ABataev

Subscribers: guansong, cfe-commits

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

Modified:
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/distribute_parallel_for_codegen.cpp
cfe/trunk/test/OpenMP/distribute_parallel_for_simd_codegen.cpp
cfe/trunk/test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=345527=345526=345527=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Mon Oct 29 12:44:25 2018
@@ -5299,7 +5299,8 @@ checkOpenMPLoop(OpenMPDirectiveKind DKin
   ExprResult CombDistCond;
   if (isOpenMPLoopBoundSharingDirective(DKind)) {
 CombDistCond =
-SemaRef.BuildBinOp(CurScope, CondLoc, BO_LE, IV.get(), 
NumIterations.get());
+SemaRef.BuildBinOp(
+CurScope, CondLoc, BO_LT, IV.get(), NumIterations.get());
   }
 
   ExprResult CombCond;

Modified: cfe/trunk/test/OpenMP/distribute_parallel_for_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/distribute_parallel_for_codegen.cpp?rev=345527=345526=345527=diff
==
--- cfe/trunk/test/OpenMP/distribute_parallel_for_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/distribute_parallel_for_codegen.cpp Mon Oct 29 
12:44:25 2018
@@ -447,7 +447,7 @@ int main() {
   // LAMBDA-DAG: [[OMP_IV_VAL_1:%.+]] = load {{.+}} [[OMP_IV]],
   // LAMBDA-DAG: [[OMP_UB_VAL_3:%.+]] = load {{.+}}
   // LAMBDA-DAG: [[OMP_UB_VAL_3_PLUS_ONE:%.+]] = add {{.+}} 
[[OMP_UB_VAL_3]], 1
-  // LAMBDA: [[CMP_IV_UB:%.+]] = icmp sle {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
+  // LAMBDA: [[CMP_IV_UB:%.+]] = icmp slt {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
   // LAMBDA: br {{.+}} [[CMP_IV_UB]], label %[[DIST_INNER_LOOP_BODY:.+]], 
label %[[DIST_INNER_LOOP_END:.+]]
 
   // check that PrevLB and PrevUB are passed to the 'for'
@@ -1210,7 +1210,7 @@ int main() {
 // CHECK-DAG: [[OMP_IV_VAL_1:%.+]] = load {{.+}} [[OMP_IV]],
 // CHECK-DAG: [[OMP_UB_VAL_3:%.+]] = load {{.+}}
 // CHECK-DAG: [[OMP_UB_VAL_3_PLUS_ONE:%.+]] = add {{.+}} [[OMP_UB_VAL_3]], 
1
-// CHECK: [[CMP_IV_UB:%.+]] = icmp sle {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
+// CHECK: [[CMP_IV_UB:%.+]] = icmp slt {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
 // CHECK: br {{.+}} [[CMP_IV_UB]], label %[[DIST_INNER_LOOP_BODY:.+]], 
label %[[DIST_INNER_LOOP_END:.+]]
 
 // check that PrevLB and PrevUB are passed to the 'for'
@@ -1938,7 +1938,7 @@ int main() {
 // CHECK-DAG: [[OMP_IV_VAL_1:%.+]] = load {{.+}} [[OMP_IV]],
 // CHECK-DAG: [[OMP_UB_VAL_3:%.+]] = load {{.+}}
 // CHECK-DAG: [[OMP_UB_VAL_3_PLUS_ONE:%.+]] = add {{.+}} [[OMP_UB_VAL_3]], 1
-// CHECK: [[CMP_IV_UB:%.+]] = icmp sle {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
+// CHECK: [[CMP_IV_UB:%.+]] = icmp slt {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
 // CHECK: br {{.+}} [[CMP_IV_UB]], label %[[DIST_INNER_LOOP_BODY:.+]], label 
%[[DIST_INNER_LOOP_END:.+]]
 
 // check that PrevLB and PrevUB are passed to the 'for'

Modified: cfe/trunk/test/OpenMP/distribute_parallel_for_simd_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/distribute_parallel_for_simd_codegen.cpp?rev=345527=345526=345527=diff
==
--- cfe/trunk/test/OpenMP/distribute_parallel_for_simd_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/distribute_parallel_for_simd_codegen.cpp Mon Oct 29 
12:44:25 2018
@@ -446,7 +446,7 @@ int main() {
   // LAMBDA-DAG: [[OMP_IV_VAL_1:%.+]] = load {{.+}} [[OMP_IV]],
   // LAMBDA-DAG: [[OMP_UB_VAL_3:%.+]] = load {{.+}}
   // LAMBDA-DAG: [[OMP_UB_VAL_3_PLUS_ONE:%.+]] = add {{.+}} 
[[OMP_UB_VAL_3]], 1
-  // LAMBDA: [[CMP_IV_UB:%.+]] = icmp sle {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
+  // LAMBDA: [[CMP_IV_UB:%.+]] = icmp slt {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
   // LAMBDA: br {{.+}} [[CMP_IV_UB]], label %[[DIST_INNER_LOOP_BODY:.+]], 
label %[[DIST_INNER_LOOP_END:.+]]
 
   // check that PrevLB and PrevUB are passed to the 'for'
@@ -1209,7 +1209,7 @@ int main() {
 // CHECK-DAG: [[OMP_IV_VAL_1:%.+]] = load {{.+}} [[OMP_IV]],
 // CHECK-DAG: [[OMP_UB_VAL_3:%.+]] = load {{.+}}
 // CHECK-DAG: [[OMP_UB_VAL_3_PLUS_ONE:%.+]] = add {{.+}} [[OMP_UB_VAL_3]], 
1
-// CHECK: 

[PATCH] D53827: [OpenMP] Fix condition.

[Alexey Bataev via Phabricator via cfe-commits]

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

Rename, the patch, it is not an NFC.


Repository:
  rC Clang

https://reviews.llvm.org/D53827



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

[PATCH] D53827: [OpenMP][NFC] Fix condition.

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea updated this revision to Diff 171558.
gtbercea added a comment.

  Add tests.


Repository:
  rC Clang

https://reviews.llvm.org/D53827

Files:
  lib/Sema/SemaOpenMP.cpp
  test/OpenMP/distribute_parallel_for_codegen.cpp
  test/OpenMP/distribute_parallel_for_simd_codegen.cpp
  test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp


Index: test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
===
--- test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
+++ test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
@@ -149,7 +149,7 @@
 
 // check exit condition
 // CHECK-DAG: [[OMP_IV_VAL_1:%.+]] = load {{.+}} [[OMP_IV]],
-// CHECK: [[CMP_IV_UB:%.+]] = icmp sle {{.+}} [[OMP_IV_VAL_1]], 100
+// CHECK: [[CMP_IV_UB:%.+]] = icmp slt {{.+}} [[OMP_IV_VAL_1]], 100
 // CHECK: br {{.+}} [[CMP_IV_UB]], label %[[DIST_INNER_LOOP_BODY:.+]], label 
%[[DIST_INNER_LOOP_END:.+]]
 
 // check that PrevLB and PrevUB are passed to the 'for'
Index: test/OpenMP/distribute_parallel_for_simd_codegen.cpp
===
--- test/OpenMP/distribute_parallel_for_simd_codegen.cpp
+++ test/OpenMP/distribute_parallel_for_simd_codegen.cpp
@@ -446,7 +446,7 @@
   // LAMBDA-DAG: [[OMP_IV_VAL_1:%.+]] = load {{.+}} [[OMP_IV]],
   // LAMBDA-DAG: [[OMP_UB_VAL_3:%.+]] = load {{.+}}
   // LAMBDA-DAG: [[OMP_UB_VAL_3_PLUS_ONE:%.+]] = add {{.+}} 
[[OMP_UB_VAL_3]], 1
-  // LAMBDA: [[CMP_IV_UB:%.+]] = icmp sle {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
+  // LAMBDA: [[CMP_IV_UB:%.+]] = icmp slt {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
   // LAMBDA: br {{.+}} [[CMP_IV_UB]], label %[[DIST_INNER_LOOP_BODY:.+]], 
label %[[DIST_INNER_LOOP_END:.+]]
 
   // check that PrevLB and PrevUB are passed to the 'for'
@@ -1209,7 +1209,7 @@
 // CHECK-DAG: [[OMP_IV_VAL_1:%.+]] = load {{.+}} [[OMP_IV]],
 // CHECK-DAG: [[OMP_UB_VAL_3:%.+]] = load {{.+}}
 // CHECK-DAG: [[OMP_UB_VAL_3_PLUS_ONE:%.+]] = add {{.+}} [[OMP_UB_VAL_3]], 
1
-// CHECK: [[CMP_IV_UB:%.+]] = icmp sle {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
+// CHECK: [[CMP_IV_UB:%.+]] = icmp slt {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
 // CHECK: br {{.+}} [[CMP_IV_UB]], label %[[DIST_INNER_LOOP_BODY:.+]], 
label %[[DIST_INNER_LOOP_END:.+]]
 
 // check that PrevLB and PrevUB are passed to the 'for'
@@ -1937,7 +1937,7 @@
 // CHECK-DAG: [[OMP_IV_VAL_1:%.+]] = load {{.+}} [[OMP_IV]],
 // CHECK-DAG: [[OMP_UB_VAL_3:%.+]] = load {{.+}}
 // CHECK-DAG: [[OMP_UB_VAL_3_PLUS_ONE:%.+]] = add {{.+}} [[OMP_UB_VAL_3]], 1
-// CHECK: [[CMP_IV_UB:%.+]] = icmp sle {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
+// CHECK: [[CMP_IV_UB:%.+]] = icmp slt {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
 // CHECK: br {{.+}} [[CMP_IV_UB]], label %[[DIST_INNER_LOOP_BODY:.+]], label 
%[[DIST_INNER_LOOP_END:.+]]
 
 // check that PrevLB and PrevUB are passed to the 'for'
Index: test/OpenMP/distribute_parallel_for_codegen.cpp
===
--- test/OpenMP/distribute_parallel_for_codegen.cpp
+++ test/OpenMP/distribute_parallel_for_codegen.cpp
@@ -447,7 +447,7 @@
   // LAMBDA-DAG: [[OMP_IV_VAL_1:%.+]] = load {{.+}} [[OMP_IV]],
   // LAMBDA-DAG: [[OMP_UB_VAL_3:%.+]] = load {{.+}}
   // LAMBDA-DAG: [[OMP_UB_VAL_3_PLUS_ONE:%.+]] = add {{.+}} 
[[OMP_UB_VAL_3]], 1
-  // LAMBDA: [[CMP_IV_UB:%.+]] = icmp sle {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
+  // LAMBDA: [[CMP_IV_UB:%.+]] = icmp slt {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
   // LAMBDA: br {{.+}} [[CMP_IV_UB]], label %[[DIST_INNER_LOOP_BODY:.+]], 
label %[[DIST_INNER_LOOP_END:.+]]
 
   // check that PrevLB and PrevUB are passed to the 'for'
@@ -1210,7 +1210,7 @@
 // CHECK-DAG: [[OMP_IV_VAL_1:%.+]] = load {{.+}} [[OMP_IV]],
 // CHECK-DAG: [[OMP_UB_VAL_3:%.+]] = load {{.+}}
 // CHECK-DAG: [[OMP_UB_VAL_3_PLUS_ONE:%.+]] = add {{.+}} [[OMP_UB_VAL_3]], 
1
-// CHECK: [[CMP_IV_UB:%.+]] = icmp sle {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
+// CHECK: [[CMP_IV_UB:%.+]] = icmp slt {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
 // CHECK: br {{.+}} [[CMP_IV_UB]], label %[[DIST_INNER_LOOP_BODY:.+]], 
label %[[DIST_INNER_LOOP_END:.+]]
 
 // check that PrevLB and PrevUB are passed to the 'for'
@@ -1938,7 +1938,7 @@
 // CHECK-DAG: [[OMP_IV_VAL_1:%.+]] = load {{.+}} [[OMP_IV]],
 // CHECK-DAG: [[OMP_UB_VAL_3:%.+]] = load {{.+}}
 // CHECK-DAG: [[OMP_UB_VAL_3_PLUS_ONE:%.+]] = add {{.+}} [[OMP_UB_VAL_3]], 1
-// CHECK: [[CMP_IV_UB:%.+]] = icmp sle {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
+// CHECK: [[CMP_IV_UB:%.+]] = icmp slt {{.+}} [[OMP_IV_VAL_1]], 
[[OMP_UB_VAL_3_PLUS_ONE]]
 // CHECK: br {{.+}} [[CMP_IV_UB]], label %[[DIST_INNER_LOOP_BODY:.+]], label 
%[[DIST_INNER_LOOP_END:.+]]
 
 // check 

[PATCH] D53810: [analyzer][NFC] Refactor PlistDiagnostics to use a class instead of passing 9 parameters around

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:82
 
-static void EmitRanges(raw_ostream ,
-   const ArrayRef Ranges,
-   const FIDMap& FM,
-   const Preprocessor ,
-   unsigned indent) {
+  void ReportDiag(raw_ostream , const PathDiagnosticPiece& P) {
+ReportPiece(o, P, /*indent*/ 4, /*depth*/ 0, /*includeControlFlow*/ true);

Szelethus wrote:
> NoQ wrote:
> > Szelethus wrote:
> > > xazax.hun wrote:
> > > > The `raw_ostream` seems to be a common argument. MAybe it would be 
> > > > worth to make that a member too?
> > > I did that, and then later changed it back for debugging purposes, if 
> > > anyone needs to print note pieces to errs or something.
> > I guess both problems could be solved by doing a private method 
> > `raw_ostream` while also wrapping it into a public method with a default 
> > `raw_ostream`.
> I think that would add unnecessary complication for very little value. I'd 
> rather commit as is.
Sure np, just thinking aloud.


https://reviews.llvm.org/D53810



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

[PATCH] D53810: [analyzer][NFC] Refactor PlistDiagnostics to use a class instead of passing 9 parameters around

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added inline comments.



Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:82
 
-static void EmitRanges(raw_ostream ,
-   const ArrayRef Ranges,
-   const FIDMap& FM,
-   const Preprocessor ,
-   unsigned indent) {
+  void ReportDiag(raw_ostream , const PathDiagnosticPiece& P) {
+ReportPiece(o, P, /*indent*/ 4, /*depth*/ 0, /*includeControlFlow*/ true);

NoQ wrote:
> Szelethus wrote:
> > xazax.hun wrote:
> > > The `raw_ostream` seems to be a common argument. MAybe it would be worth 
> > > to make that a member too?
> > I did that, and then later changed it back for debugging purposes, if 
> > anyone needs to print note pieces to errs or something.
> I guess both problems could be solved by doing a private method `raw_ostream` 
> while also wrapping it into a public method with a default `raw_ostream`.
I think that would add unnecessary complication for very little value. I'd 
rather commit as is.


https://reviews.llvm.org/D53810



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

[PATCH] D53827: [OpenMP][NFC] Fix condition.

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea created this revision.
Herald added subscribers: cfe-commits, guansong.
gtbercea added reviewers: ABataev, caomhin.

Iteration variable must be strictly less than the number of iterations.


Repository:
  rC Clang

https://reviews.llvm.org/D53827

Files:
  lib/Sema/SemaOpenMP.cpp


Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -5299,7 +5299,8 @@
   ExprResult CombDistCond;
   if (isOpenMPLoopBoundSharingDirective(DKind)) {
 CombDistCond =
-SemaRef.BuildBinOp(CurScope, CondLoc, BO_LE, IV.get(), 
NumIterations.get());
+SemaRef.BuildBinOp(
+CurScope, CondLoc, BO_LT, IV.get(), NumIterations.get());
   }
 
   ExprResult CombCond;


Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -5299,7 +5299,8 @@
   ExprResult CombDistCond;
   if (isOpenMPLoopBoundSharingDirective(DKind)) {
 CombDistCond =
-SemaRef.BuildBinOp(CurScope, CondLoc, BO_LE, IV.get(), NumIterations.get());
+SemaRef.BuildBinOp(
+CurScope, CondLoc, BO_LT, IV.get(), NumIterations.get());
   }
 
   ExprResult CombCond;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52610: [Esan] Port cache frag to FreeBSD

[David CARLIER via Phabricator via cfe-commits]

devnexen added a comment.

Things might differ between NetBSD and FreeBSD about the feasibility. So maybe 
for the former it is more reachable (only would need to set specific 
application mappings maybe ?).

- So once the non writable addresses are created, it processes the working sets 
to make it world aligned => One of the points of failure.
- The segfault handler does not work or can't be instrumented because not 
initialised yet at this stage.

The caching frag works because there is no shadow mapping (yet?).


Repository:
  rC Clang

https://reviews.llvm.org/D52610



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

Re: r345109 - Debug Info (-gmodules): emit full types for non-anchored template specializations

[Adrian Prantl via cfe-commits]

> On Oct 29, 2018, at 11:26 AM, David Blaikie  wrote:
> 
> Is this a workaround for now with the intent to fix this to allow such 
> implicit specializations to have their debug info modularized? I believe this 
> does work correctly in modular debug info with expliict modules, would 
> probably be sort of nice to have these things be consistent/similar?

It started as a workaround, but I reached the conclusion that it's not 
worthwhile pursuing a more space-efficient solution. Note that all this patch 
does is emit plain old non-modular debug info for non-explicit template 
specializations, so it is definitely safe & conservative. This increases the 
size of the clang module cache in a build of clang by 4MiB out of 1GiB total.

As you can read in my thread with Richard, it isn't possible in Clang to 
determine the clang module that contains the complete definition of a template 
specialization inside of a namespace for indirectly imported modules (such as 
in my testcase). That means that a consumer would have to look in every Clang 
module for complete types; not just in the transitive closure of imports of the 
.pcm that has the forward declaration. This lookup is expensive and difficult 
to implement in LLDB, so I decided not to pursue this further.

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

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

This is usually not done in the LLVM codebase, as its not a
pointer/reference.

Am 29.10.18 um 20:29 schrieb Alexander Zaitsev via Phabricator:

> ZaMaZaN4iK added inline comments.
> 
> 
>  Comment at: clang-tidy/readability/IsolateDeclarationCheck.cpp:119
>  +   const LangOptions ) {
>  +  std::size_t DeclCount = std::distance(DS->decl_begin(), DS->decl_end());
>  +  if (DeclCount < 2)
> 
>  
> 
> Mark `DeclCount` as const
> 
> Repository:
> 
>   rCTE Clang Tools Extra
> 
> https://reviews.llvm.org/D51949


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK added inline comments.



Comment at: clang-tidy/readability/IsolateDeclarationCheck.cpp:119
+   const LangOptions ) {
+  std::size_t DeclCount = std::distance(DS->decl_begin(), DS->decl_end());
+  if (DeclCount < 2)

Mark `DeclCount` as const


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D53457: clang-cl: Add "/clang:" pass-through arg support.

[Neeraj K. Singh via Phabricator via cfe-commits]

neerajksingh added a comment.

In https://reviews.llvm.org/D53457#1278579, @hans wrote:

> The `-Xclang` option has the same issue, and I think `/clang:` should work 
> similarly, i.e. `/clang:-MF /clang:`. It's not pretty, but at least 
> it's consistent. Yes, that means processing consecutive `/Xclang:` options 
> together, but hopefully that's doable.


It looks like the handling for -Xclang is a lot simpler (in 
`Clang::ConstructJob`).  There the Xclang options are all gathered and 
forwarded at a particular spot in the command line for cc1.  They aren't 
interleaved with other options, and it wouldn't really make sense to do so 
anyway since it doesn't really look like cc1 arguments are constructed from 
driver arguments in any particular order.

The llvm/lib/Option/OptTable.cpp code is not really setup to easily interleave 
arguments like would be required for your request.  Can you see a way to 
accomplish what you want without a deep refactoring of OptTable::ParseArgs and 
the InputArgList class?


https://reviews.llvm.org/D53457



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

[PATCH] D53810: [analyzer][NFC] Refactor PlistDiagnostics to use a class instead of passing 9 parameters around

[Artem Dergachev via Phabricator via cfe-commits]

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

Wow thanks! Great point.




Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:82
 
-static void EmitRanges(raw_ostream ,
-   const ArrayRef Ranges,
-   const FIDMap& FM,
-   const Preprocessor ,
-   unsigned indent) {
+  void ReportDiag(raw_ostream , const PathDiagnosticPiece& P) {
+ReportPiece(o, P, /*indent*/ 4, /*depth*/ 0, /*includeControlFlow*/ true);

Szelethus wrote:
> xazax.hun wrote:
> > The `raw_ostream` seems to be a common argument. MAybe it would be worth to 
> > make that a member too?
> I did that, and then later changed it back for debugging purposes, if anyone 
> needs to print note pieces to errs or something.
I guess both problems could be solved by doing a private method `raw_ostream` 
while also wrapping it into a public method with a default `raw_ostream`.


https://reviews.llvm.org/D53810



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

[PATCH] D52610: [Esan] Port cache frag to FreeBSD

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

In https://reviews.llvm.org/D52610#1279176, @devnexen wrote:

> - FreeBSD does not have real Linux's clone equivalent.
> - Hangs or crashes during the final report (the shadow mapping is similar as 
> Linux's though).


NetBSD has a clone compatibility syscall... but please try to explain what is 
the algorithm to map to a BSD system.

There must but a way to port it to BSD.


Repository:
  rC Clang

https://reviews.llvm.org/D52610



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

[PATCH] D52610: [Esan] Port cache frag to FreeBSD

[David CARLIER via Phabricator via cfe-commits]

devnexen added a comment.

- FreeBSD does not have real Linux's clone equivalent.
- Hangs or crashes during the final report (the shadow mapping is similar as 
Linux's though).


Repository:
  rC Clang

https://reviews.llvm.org/D52610



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

[PATCH] D52857: [clang-query] Add non-exclusive output API

[Stephen Kelly via Phabricator via cfe-commits]

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



Comment at: clang-query/QueryParser.cpp:285
+if (VarStr.empty())
+  return new InvalidQuery("expected variable name");
+if (Var == PQV_Invalid)

aaron.ballman wrote:
> This seems incorrect to me; we expect the command identifier `output` here, 
> don't we?
Actually this is the case where the user writes `enable` with no more output.

I match `set` command behavior and output here.

I added a test for this, similar to the existing equivalent `set` test.


Repository:
  rL LLVM

https://reviews.llvm.org/D52857



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

Re: r343653 - OpenCL: Mark printf format string argument

[Anastasia Stulova via cfe-commits]

It seems like it caused a bug:

https://bugs.llvm.org/show_bug.cgi?id=39486



From: cfe-commits  on behalf of Matt 
Arsenault via cfe-commits 
Sent: 03 October 2018 03:01
To: cfe-commits@lists.llvm.org
Subject: r343653 - OpenCL: Mark printf format string argument

Author: arsenm
Date: Tue Oct  2 19:01:19 2018
New Revision: 343653

URL: http://llvm.org/viewvc/llvm-project?rev=343653=rev
Log:
OpenCL: Mark printf format string argument

Fixes not warning on format string errors.

Added:
cfe/trunk/test/SemaOpenCL/printf-format-string-warnings.cl
Modified:
cfe/trunk/lib/Headers/opencl-c.h

Modified: cfe/trunk/lib/Headers/opencl-c.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/opencl-c.h?rev=343653=343652=343653=diff
==
--- cfe/trunk/lib/Headers/opencl-c.h (original)
+++ cfe/trunk/lib/Headers/opencl-c.h Tue Oct  2 19:01:19 2018
@@ -14462,7 +14462,7 @@ half16 __ovld __cnfn shuffle2(half16 x,
 #if __OPENCL_C_VERSION__ >= CL_VERSION_1_2
 // OpenCL v1.2 s6.12.13, v2.0 s6.13.13 - printf

-int printf(__constant const char* st, ...);
+int printf(__constant const char* st, ...) __attribute__((format(printf, 1, 
2)));
 #endif

 // OpenCL v1.1 s6.11.3, v1.2 s6.12.14, v2.0 s6.13.14 - Image Read and Write 
Functions

Added: cfe/trunk/test/SemaOpenCL/printf-format-string-warnings.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaOpenCL/printf-format-string-warnings.cl?rev=343653=auto
==
--- cfe/trunk/test/SemaOpenCL/printf-format-string-warnings.cl (added)
+++ cfe/trunk/test/SemaOpenCL/printf-format-string-warnings.cl Tue Oct  2 
19:01:19 2018
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0 
-finclude-default-header
+
+// Make sure warnings are produced based on printf format strings.
+
+
+kernel void format_string_warnings(__constant char* arg) {
+
+  printf("%d", arg); // expected-warning {{format specifies type 'int' but the 
argument has type '__constant char *'}}
+
+  printf("not enough arguments %d %d", 4); // expected-warning {{more '%' 
conversions than data arguments}}
+
+  printf("too many arguments", 4); // expected-warning {{data argument not 
used by format string}}
+}


___
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

[PATCH] D52610: [Esan] Port cache frag to FreeBSD

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

In https://reviews.llvm.org/D52610#1278743, @devnexen wrote:

> ping working-set on FreeBSD does not seem doable.


Rationale? I don't feel convinced.


Repository:
  rC Clang

https://reviews.llvm.org/D52610



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

[PATCH] D52857: [clang-query] Add non-exclusive output API

[Stephen Kelly via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL345522: [clang-query] Add non-exclusive output API (authored 
by steveire, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52857?vs=171046=171549#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52857

Files:
  clang-tools-extra/trunk/clang-query/Query.cpp
  clang-tools-extra/trunk/clang-query/Query.h
  clang-tools-extra/trunk/clang-query/QueryParser.cpp
  clang-tools-extra/trunk/clang-query/QueryParser.h
  clang-tools-extra/trunk/unittests/clang-query/QueryEngineTest.cpp
  clang-tools-extra/trunk/unittests/clang-query/QueryParserTest.cpp

Index: clang-tools-extra/trunk/clang-query/Query.h
===
--- clang-tools-extra/trunk/clang-query/Query.h
+++ clang-tools-extra/trunk/clang-query/Query.h
@@ -29,6 +29,8 @@
   QK_Match,
   QK_SetBool,
   QK_SetOutputKind,
+  QK_EnableOutputKind,
+  QK_DisableOutputKind,
   QK_Quit
 };
 
@@ -151,6 +153,36 @@
   bool QuerySession::*Var;
 };
 
+// Implements the non-exclusive 'set output dump|diag|print' options.
+struct SetNonExclusiveOutputQuery : Query {
+  SetNonExclusiveOutputQuery(QueryKind Kind, bool QuerySession::*Var,
+ bool Value)
+  : Query(Kind), Var(Var), Value(Value) {}
+  bool run(llvm::raw_ostream , QuerySession ) const override {
+QS.*Var = Value;
+return true;
+  }
+
+  bool QuerySession::*Var;
+  bool Value;
+};
+
+struct EnableOutputQuery : SetNonExclusiveOutputQuery {
+  EnableOutputQuery(bool QuerySession::*Var)
+  : SetNonExclusiveOutputQuery(QK_EnableOutputKind, Var, true) {}
+
+  static bool classof(const Query *Q) { return Q->Kind == QK_EnableOutputKind; }
+};
+
+struct DisableOutputQuery : SetNonExclusiveOutputQuery {
+  DisableOutputQuery(bool QuerySession::*Var)
+  : SetNonExclusiveOutputQuery(QK_DisableOutputKind, Var, false) {}
+
+  static bool classof(const Query *Q) {
+return Q->Kind == QK_DisableOutputKind;
+  }
+};
+
 } // namespace query
 } // namespace clang
 
Index: clang-tools-extra/trunk/clang-query/QueryParser.h
===
--- clang-tools-extra/trunk/clang-query/QueryParser.h
+++ clang-tools-extra/trunk/clang-query/QueryParser.h
@@ -44,7 +44,7 @@
   template  struct LexOrCompleteWord;
 
   QueryRef parseSetBool(bool QuerySession::*Var);
-  QueryRef parseSetOutputKind();
+  template  QueryRef parseSetOutputKind();
   QueryRef completeMatcherExpression();
 
   QueryRef endQuery(QueryRef Q);
Index: clang-tools-extra/trunk/clang-query/Query.cpp
===
--- clang-tools-extra/trunk/clang-query/Query.cpp
+++ clang-tools-extra/trunk/clang-query/Query.cpp
@@ -45,6 +45,10 @@
 "Set whether to print the current matcher,\n"
 "  set output   "
 "Set whether to output only  content.\n"
+"  enable output"
+"Enable  content non-exclusively.\n"
+"  disable output   "
+"Disable  content non-exclusively.\n"
 "  quit, q   "
 "Terminates the query session.\n\n"
 "Several commands accept a  parameter. The available features "
Index: clang-tools-extra/trunk/clang-query/QueryParser.cpp
===
--- clang-tools-extra/trunk/clang-query/QueryParser.cpp
+++ clang-tools-extra/trunk/clang-query/QueryParser.cpp
@@ -106,7 +106,7 @@
   return new SetQuery(Var, Value);
 }
 
-QueryRef QueryParser::parseSetOutputKind() {
+template  QueryRef QueryParser::parseSetOutputKind() {
   StringRef ValStr;
   unsigned OutKind = LexOrCompleteWord(this, ValStr)
  .Case("diag", OK_Diag)
@@ -122,11 +122,11 @@
 
   switch (OutKind) {
   case OK_DetailedAST:
-return new SetExclusiveOutputQuery(::DetailedASTOutput);
+return new QueryType(::DetailedASTOutput);
   case OK_Diag:
-return new SetExclusiveOutputQuery(::DiagOutput);
+return new QueryType(::DiagOutput);
   case OK_Print:
-return new SetExclusiveOutputQuery(::PrintOutput);
+return new QueryType(::PrintOutput);
   }
 
   llvm_unreachable("Invalid output kind");
@@ -151,7 +151,9 @@
   PQK_Match,
   PQK_Set,
   PQK_Unlet,
-  PQK_Quit
+  PQK_Quit,
+  PQK_Enable,
+  PQK_Disable
 };
 
 enum ParsedQueryVariable {
@@ -193,6 +195,8 @@
   .Case("q", PQK_Quit,  /*IsCompletion=*/false)
   .Case("quit", PQK_Quit)
   .Case("set", PQK_Set)
+  .Case("enable", PQK_Enable)
+  .Case("disable", PQK_Disable)
   .Case("unlet", PQK_Unlet)
   .Default(PQK_Invalid);
 
@@ -256,7 +260,7 @@
 QueryRef Q;
 switch (Var) {
 case 

[clang-tools-extra] r345522 - [clang-query] Add non-exclusive output API

[Stephen Kelly via cfe-commits]

Author: steveire
Date: Mon Oct 29 11:59:56 2018
New Revision: 345522

URL: http://llvm.org/viewvc/llvm-project?rev=345522=rev
Log:
[clang-query] Add non-exclusive output API

Summary:
Add granular options for AST dumping, text printing and diagnostics.

This makes it possible to

* Have both diag and dump active at once
* Extend the output with other queryable content in the future.

Reviewers: aaron.ballman, pcc, ioeric, ilya-biryukov, klimek, sammccall

Reviewed By: aaron.ballman

Subscribers: cfe-commits

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

Modified:
clang-tools-extra/trunk/clang-query/Query.cpp
clang-tools-extra/trunk/clang-query/Query.h
clang-tools-extra/trunk/clang-query/QueryParser.cpp
clang-tools-extra/trunk/clang-query/QueryParser.h
clang-tools-extra/trunk/unittests/clang-query/QueryEngineTest.cpp
clang-tools-extra/trunk/unittests/clang-query/QueryParserTest.cpp

Modified: clang-tools-extra/trunk/clang-query/Query.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-query/Query.cpp?rev=345522=345521=345522=diff
==
--- clang-tools-extra/trunk/clang-query/Query.cpp (original)
+++ clang-tools-extra/trunk/clang-query/Query.cpp Mon Oct 29 11:59:56 2018
@@ -45,6 +45,10 @@ bool HelpQuery::run(llvm::raw_ostream 
 "Set whether to print the current matcher,\n"
 "  set output   "
 "Set whether to output only  content.\n"
+"  enable output"
+"Enable  content non-exclusively.\n"
+"  disable output   "
+"Disable  content non-exclusively.\n"
 "  quit, q   "
 "Terminates the query session.\n\n"
 "Several commands accept a  parameter. The available features 
"

Modified: clang-tools-extra/trunk/clang-query/Query.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-query/Query.h?rev=345522=345521=345522=diff
==
--- clang-tools-extra/trunk/clang-query/Query.h (original)
+++ clang-tools-extra/trunk/clang-query/Query.h Mon Oct 29 11:59:56 2018
@@ -29,6 +29,8 @@ enum QueryKind {
   QK_Match,
   QK_SetBool,
   QK_SetOutputKind,
+  QK_EnableOutputKind,
+  QK_DisableOutputKind,
   QK_Quit
 };
 
@@ -151,6 +153,36 @@ struct SetExclusiveOutputQuery : Query {
   bool QuerySession::*Var;
 };
 
+// Implements the non-exclusive 'set output dump|diag|print' options.
+struct SetNonExclusiveOutputQuery : Query {
+  SetNonExclusiveOutputQuery(QueryKind Kind, bool QuerySession::*Var,
+ bool Value)
+  : Query(Kind), Var(Var), Value(Value) {}
+  bool run(llvm::raw_ostream , QuerySession ) const override {
+QS.*Var = Value;
+return true;
+  }
+
+  bool QuerySession::*Var;
+  bool Value;
+};
+
+struct EnableOutputQuery : SetNonExclusiveOutputQuery {
+  EnableOutputQuery(bool QuerySession::*Var)
+  : SetNonExclusiveOutputQuery(QK_EnableOutputKind, Var, true) {}
+
+  static bool classof(const Query *Q) { return Q->Kind == QK_EnableOutputKind; 
}
+};
+
+struct DisableOutputQuery : SetNonExclusiveOutputQuery {
+  DisableOutputQuery(bool QuerySession::*Var)
+  : SetNonExclusiveOutputQuery(QK_DisableOutputKind, Var, false) {}
+
+  static bool classof(const Query *Q) {
+return Q->Kind == QK_DisableOutputKind;
+  }
+};
+
 } // namespace query
 } // namespace clang
 

Modified: clang-tools-extra/trunk/clang-query/QueryParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-query/QueryParser.cpp?rev=345522=345521=345522=diff
==
--- clang-tools-extra/trunk/clang-query/QueryParser.cpp (original)
+++ clang-tools-extra/trunk/clang-query/QueryParser.cpp Mon Oct 29 11:59:56 2018
@@ -106,7 +106,7 @@ QueryRef QueryParser::parseSetBool(bool
   return new SetQuery(Var, Value);
 }
 
-QueryRef QueryParser::parseSetOutputKind() {
+template  QueryRef QueryParser::parseSetOutputKind() {
   StringRef ValStr;
   unsigned OutKind = LexOrCompleteWord(this, ValStr)
  .Case("diag", OK_Diag)
@@ -122,11 +122,11 @@ QueryRef QueryParser::parseSetOutputKind
 
   switch (OutKind) {
   case OK_DetailedAST:
-return new SetExclusiveOutputQuery(::DetailedASTOutput);
+return new QueryType(::DetailedASTOutput);
   case OK_Diag:
-return new SetExclusiveOutputQuery(::DiagOutput);
+return new QueryType(::DiagOutput);
   case OK_Print:
-return new SetExclusiveOutputQuery(::PrintOutput);
+return new QueryType(::PrintOutput);
   }
 
   llvm_unreachable("Invalid output kind");
@@ -151,7 +151,9 @@ enum ParsedQueryKind {
   PQK_Match,
   PQK_Set,
   PQK_Unlet,
-  PQK_Quit
+  PQK_Quit,
+  PQK_Enable,
+  PQK_Disable
 };
 
 enum ParsedQueryVariable {
@@ -193,6 +195,8 @@ QueryRef QueryParser::doParse() {

Re: r344957 - Give Multiversion-inline functions linkonce linkage

[David Blaikie via cfe-commits]

On Mon, Oct 29, 2018 at 11:46 AM Keane, Erich  wrote:

>
>
>
>
> *From:* David Blaikie [mailto:dblai...@gmail.com]
> *Sent:* Monday, October 29, 2018 11:41 AM
> *To:* Keane, Erich 
> *Cc:* Eric Christopher ; cfe-commits@lists.llvm.org
>
>
> *Subject:* Re: r344957 - Give Multiversion-inline functions linkonce
> linkage
>
>
>
>
>
> On Mon, Oct 29, 2018 at 11:30 AM Keane, Erich 
> wrote:
>
> GCC actually doesn’t support function multiversioning in C mode, so this
> fix is part of our extension to support C with multiversioning.
>
>
> Ah, what's the motivation for that?
>
> *[Keane, Erich] Basically, I identified no good reason to not support it.
> GCC’s motivation is lost to time as far as I can tell.  This
> multiversioning is super useful in my employer’s math/intrinsic libraries
> (MKL uses this feature extensively), so enabling it in C seemed like a
> positive improvement.  We’ve made some other behavioral changes to make
> ‘target’ work more sanely to suppress some of the worst bugs (becoming MV
> after usage) as well as make it work better with Clang’s
> emit-functionality.*
>
>
>
>
>   I perhaps wonder if this is part of the reason GCC only supports it in
> C++ mode...
>
>
> Yeah... could be.
>
> I mean I suppose using linkonce isn't making multiversioning any worse in
> C than it already is in C++ inline functions (which is where it's pretty
> easy to misuse, as I understand it - two files, one compiled with a CPU
> feature, one without, linkonce functions get linked together & the linker
> picks one - then code built without the feature calls code that uses the
> feature - but, I mean, that can happen even witohut multiversioning, if you
> compile some source files with a feature and some without) - I guess the
> main risk would be if it confuses users by making multiversioned C inline
> functions end up behaving too differently from non-multiversioned ones...
>
> *[Keane, Erich] I suspect that multiversioning is such a sharp edged
> feature, that a change like this makes it a touch more sane.  Since the
> individual versions are independently mangled, you don’t have to worry
> about merging different versions.  The only real issue comes from merging
> the resolver functions if the visible list of declarations is different,
> but we treat that as an ODR violation.*
>
>
>
Rightio - thanks for all the context (:


>
> - Dave
>
>
>
>
> *From:* David Blaikie [mailto:dblai...@gmail.com]
> *Sent:* Monday, October 29, 2018 11:25 AM
> *To:* Keane, Erich ; Eric Christopher <
> echri...@gmail.com>
> *Cc:* cfe-commits@lists.llvm.org
> *Subject:* Re: r344957 - Give Multiversion-inline functions linkonce
> linkage
>
>
>
> Does this match GCC's approach here?
>
> (I ask this sort of as throwaway/conversation starter - because the
> linkage/behavior around multiversion functions and their inlining is full
> of sharp corners/risks of code moving out of the areas appropriately
> restricted based on the cpu features)
>
> On Mon, Oct 22, 2018 at 2:22 PM Erich Keane via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> Author: erichkeane
> Date: Mon Oct 22 14:20:45 2018
> New Revision: 344957
>
> URL: http://llvm.org/viewvc/llvm-project?rev=344957=rev
> Log:
> Give Multiversion-inline functions linkonce linkage
>
> Since multiversion variant functions can be inline, in C they become
> available-externally linkage.  This ends up causing the variants to not
> be emitted, and not available to the linker.
>
> The solution is to make sure that multiversion functions are always
> emitted by marking them linkonce.
>
> Change-Id: I897aa37c7cbba0c1eb2c57ee881d5000a2113b75
>
> Modified:
> cfe/trunk/lib/CodeGen/CodeGenModule.cpp
> cfe/trunk/test/CodeGen/attr-target-mv.c
>
> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=344957=344956=344957=diff
>
> ==
> --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Mon Oct 22 14:20:45 2018
> @@ -3669,6 +3669,10 @@ llvm::GlobalValue::LinkageTypes CodeGenM
>return llvm::GlobalVariable::WeakAnyLinkage;
>}
>
> +  if (const auto *FD = D->getAsFunction())
> +if (FD->isMultiVersion() && Linkage == GVA_AvailableExternally)
> +  return llvm::GlobalVariable::LinkOnceAnyLinkage;
> +
>// We are guaranteed to have a strong definition somewhere else,
>// so we can use available_externally linkage.
>if (Linkage == GVA_AvailableExternally)
>
> Modified: cfe/trunk/test/CodeGen/attr-target-mv.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/attr-target-mv.c?rev=344957=344956=344957=diff
>
> ==
> --- cfe/trunk/test/CodeGen/attr-target-mv.c (original)
> +++ cfe/trunk/test/CodeGen/attr-target-mv.c Mon Oct 22 14:20:45 2018
> @@ -88,19 +88,19 @@ void bar4() {

RE: r344957 - Give Multiversion-inline functions linkonce linkage

[Keane, Erich via cfe-commits]

From: David Blaikie [mailto:dblai...@gmail.com]
Sent: Monday, October 29, 2018 11:41 AM
To: Keane, Erich 
Cc: Eric Christopher ; cfe-commits@lists.llvm.org
Subject: Re: r344957 - Give Multiversion-inline functions linkonce linkage


On Mon, Oct 29, 2018 at 11:30 AM Keane, Erich 
mailto:erich.ke...@intel.com>> wrote:
GCC actually doesn’t support function multiversioning in C mode, so this fix is 
part of our extension to support C with multiversioning.

Ah, what's the motivation for that?
[Keane, Erich] Basically, I identified no good reason to not support it.  GCC’s 
motivation is lost to time as far as I can tell.  This multiversioning is super 
useful in my employer’s math/intrinsic libraries (MKL uses this feature 
extensively), so enabling it in C seemed like a positive improvement.  We’ve 
made some other behavioral changes to make ‘target’ work more sanely to 
suppress some of the worst bugs (becoming MV after usage) as well as make it 
work better with Clang’s emit-functionality.


  I perhaps wonder if this is part of the reason GCC only supports it in C++ 
mode...

Yeah... could be.

I mean I suppose using linkonce isn't making multiversioning any worse in C 
than it already is in C++ inline functions (which is where it's pretty easy to 
misuse, as I understand it - two files, one compiled with a CPU feature, one 
without, linkonce functions get linked together & the linker picks one - then 
code built without the feature calls code that uses the feature - but, I mean, 
that can happen even witohut multiversioning, if you compile some source files 
with a feature and some without) - I guess the main risk would be if it 
confuses users by making multiversioned C inline functions end up behaving too 
differently from non-multiversioned ones...
[Keane, Erich] I suspect that multiversioning is such a sharp edged feature, 
that a change like this makes it a touch more sane.  Since the individual 
versions are independently mangled, you don’t have to worry about merging 
different versions.  The only real issue comes from merging the resolver 
functions if the visible list of declarations is different, but we treat that 
as an ODR violation.


- Dave


From: David Blaikie [mailto:dblai...@gmail.com]
Sent: Monday, October 29, 2018 11:25 AM
To: Keane, Erich mailto:erich.ke...@intel.com>>; Eric 
Christopher mailto:echri...@gmail.com>>
Cc: cfe-commits@lists.llvm.org
Subject: Re: r344957 - Give Multiversion-inline functions linkonce linkage

Does this match GCC's approach here?

(I ask this sort of as throwaway/conversation starter - because the 
linkage/behavior around multiversion functions and their inlining is full of 
sharp corners/risks of code moving out of the areas appropriately restricted 
based on the cpu features)
On Mon, Oct 22, 2018 at 2:22 PM Erich Keane via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: erichkeane
Date: Mon Oct 22 14:20:45 2018
New Revision: 344957

URL: http://llvm.org/viewvc/llvm-project?rev=344957=rev
Log:
Give Multiversion-inline functions linkonce linkage

Since multiversion variant functions can be inline, in C they become
available-externally linkage.  This ends up causing the variants to not
be emitted, and not available to the linker.

The solution is to make sure that multiversion functions are always
emitted by marking them linkonce.

Change-Id: I897aa37c7cbba0c1eb2c57ee881d5000a2113b75

Modified:
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/test/CodeGen/attr-target-mv.c

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=344957=344956=344957=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Mon Oct 22 14:20:45 2018
@@ -3669,6 +3669,10 @@ llvm::GlobalValue::LinkageTypes CodeGenM
   return llvm::GlobalVariable::WeakAnyLinkage;
   }

+  if (const auto *FD = D->getAsFunction())
+if (FD->isMultiVersion() && Linkage == GVA_AvailableExternally)
+  return llvm::GlobalVariable::LinkOnceAnyLinkage;
+
   // We are guaranteed to have a strong definition somewhere else,
   // so we can use available_externally linkage.
   if (Linkage == GVA_AvailableExternally)

Modified: cfe/trunk/test/CodeGen/attr-target-mv.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/attr-target-mv.c?rev=344957=344956=344957=diff
==
--- cfe/trunk/test/CodeGen/attr-target-mv.c (original)
+++ cfe/trunk/test/CodeGen/attr-target-mv.c Mon Oct 22 14:20:45 2018
@@ -88,19 +88,19 @@ void bar4() {

 // CHECK: declare i32 @foo.arch_sandybridge()

-// CHECK: define available_externally i32 @foo_inline.sse4.2()
+// CHECK: define linkonce i32 @foo_inline.sse4.2()
 // CHECK: ret i32 0

 // CHECK: declare 

Re: r344957 - Give Multiversion-inline functions linkonce linkage

[David Blaikie via cfe-commits]

On Mon, Oct 29, 2018 at 11:30 AM Keane, Erich  wrote:

> GCC actually doesn’t support function multiversioning in C mode, so this
> fix is part of our extension to support C with multiversioning.
>

Ah, what's the motivation for that?


>   I perhaps wonder if this is part of the reason GCC only supports it in
> C++ mode...
>

Yeah... could be.

I mean I suppose using linkonce isn't making multiversioning any worse in C
than it already is in C++ inline functions (which is where it's pretty easy
to misuse, as I understand it - two files, one compiled with a CPU feature,
one without, linkonce functions get linked together & the linker picks one
- then code built without the feature calls code that uses the feature -
but, I mean, that can happen even witohut multiversioning, if you compile
some source files with a feature and some without) - I guess the main risk
would be if it confuses users by making multiversioned C inline functions
end up behaving too differently from non-multiversioned ones...

- Dave


>
>
> *From:* David Blaikie [mailto:dblai...@gmail.com]
> *Sent:* Monday, October 29, 2018 11:25 AM
> *To:* Keane, Erich ; Eric Christopher <
> echri...@gmail.com>
> *Cc:* cfe-commits@lists.llvm.org
> *Subject:* Re: r344957 - Give Multiversion-inline functions linkonce
> linkage
>
>
>
> Does this match GCC's approach here?
>
> (I ask this sort of as throwaway/conversation starter - because the
> linkage/behavior around multiversion functions and their inlining is full
> of sharp corners/risks of code moving out of the areas appropriately
> restricted based on the cpu features)
>
> On Mon, Oct 22, 2018 at 2:22 PM Erich Keane via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> Author: erichkeane
> Date: Mon Oct 22 14:20:45 2018
> New Revision: 344957
>
> URL: http://llvm.org/viewvc/llvm-project?rev=344957=rev
> Log:
> Give Multiversion-inline functions linkonce linkage
>
> Since multiversion variant functions can be inline, in C they become
> available-externally linkage.  This ends up causing the variants to not
> be emitted, and not available to the linker.
>
> The solution is to make sure that multiversion functions are always
> emitted by marking them linkonce.
>
> Change-Id: I897aa37c7cbba0c1eb2c57ee881d5000a2113b75
>
> Modified:
> cfe/trunk/lib/CodeGen/CodeGenModule.cpp
> cfe/trunk/test/CodeGen/attr-target-mv.c
>
> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=344957=344956=344957=diff
>
> ==
> --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Mon Oct 22 14:20:45 2018
> @@ -3669,6 +3669,10 @@ llvm::GlobalValue::LinkageTypes CodeGenM
>return llvm::GlobalVariable::WeakAnyLinkage;
>}
>
> +  if (const auto *FD = D->getAsFunction())
> +if (FD->isMultiVersion() && Linkage == GVA_AvailableExternally)
> +  return llvm::GlobalVariable::LinkOnceAnyLinkage;
> +
>// We are guaranteed to have a strong definition somewhere else,
>// so we can use available_externally linkage.
>if (Linkage == GVA_AvailableExternally)
>
> Modified: cfe/trunk/test/CodeGen/attr-target-mv.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/attr-target-mv.c?rev=344957=344956=344957=diff
>
> ==
> --- cfe/trunk/test/CodeGen/attr-target-mv.c (original)
> +++ cfe/trunk/test/CodeGen/attr-target-mv.c Mon Oct 22 14:20:45 2018
> @@ -88,19 +88,19 @@ void bar4() {
>
>  // CHECK: declare i32 @foo.arch_sandybridge()
>
> -// CHECK: define available_externally i32 @foo_inline.sse4.2()
> +// CHECK: define linkonce i32 @foo_inline.sse4.2()
>  // CHECK: ret i32 0
>
>  // CHECK: declare i32 @foo_inline.arch_sandybridge()
>  //
> -// CHECK: define available_externally i32 @foo_inline.arch_ivybridge()
> +// CHECK: define linkonce i32 @foo_inline.arch_ivybridge()
>  // CHECK: ret i32 1
> -// CHECK: define available_externally i32 @foo_inline()
> +// CHECK: define linkonce i32 @foo_inline()
>  // CHECK: ret i32 2
>
> -// CHECK: define available_externally void @foo_decls()
> -// CHECK: define available_externally void @foo_decls.sse4.2()
> +// CHECK: define linkonce void @foo_decls()
> +// CHECK: define linkonce void @foo_decls.sse4.2()
>
> -// CHECK: define available_externally void @foo_multi.avx_sse4.2()
> -// CHECK: define available_externally void @foo_multi.fma4_sse4.2()
> -// CHECK: define available_externally void
> @foo_multi.arch_ivybridge_fma4_sse4.2()
> +// CHECK: define linkonce void @foo_multi.avx_sse4.2()
> +// CHECK: define linkonce void @foo_multi.fma4_sse4.2()
> +// CHECK: define linkonce void @foo_multi.arch_ivybridge_fma4_sse4.2()
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> 

[PATCH] D53814: Allow the analyzer to output to a SARIF file

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 171545.
aaron.ballman marked 3 inline comments as done.
aaron.ballman added a comment.

Updated based on initial review feedback, and added more context to the patch.


https://reviews.llvm.org/D53814

Files:
  Analysis/diagnostics/sarif-check.py
  Analysis/diagnostics/sarif-diagnostics-taint-test.c
  StaticAnalyzer/Core/CMakeLists.txt
  StaticAnalyzer/Core/SarifDiagnostics.cpp
  clang/StaticAnalyzer/Core/Analyses.def
  clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h

Index: Analysis/diagnostics/sarif-diagnostics-taint-test.c
===
--- /dev/null
+++ Analysis/diagnostics/sarif-diagnostics-taint-test.c
@@ -0,0 +1,32 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.security.taint,debug.TaintTest %s -verify -analyzer-output=sarif -o %t.sarif | %python %S/sarif-check.py %s %t.sarif
+#include "../Inputs/system-header-simulator.h"
+
+int atoi(const char *nptr);
+
+void f(void) {
+  char s[80];
+  scanf("%s", s);
+  int d = atoi(s); // expected-warning {{tainted}}
+}
+
+int main(void) {
+  f();
+  return 0;
+}
+
+// Test the basics for sanity.
+// CHECK: sarifLog['version'].startswith("2.0.0")
+// CHECK: sarifLog['runs'][0]['tool']['fullName'] == "clang static analyzer"
+// CHECK: sarifLog['runs'][0]['tool']['name'] == "clang"
+// CHECK: sarifLog['runs'][0]['tool']['language'] == "en-US"
+
+// Test the specifics of this taint test.
+// CHECK: len(result(0)['codeFlows'][0]['threadFlows'][0]['locations']) == 2
+// CHECK: flow(0)['locations'][0]['step'] == 1
+// CHECK: flow(0)['locations'][0]['importance'] == "essential"
+// CHECK: flow(0)['locations'][0]['location']['message']['text'] == "Calling 'f'"
+// CHECK: flow(0)['locations'][0]['location']['physicalLocation']['region']['startLine'] == 13
+// CHECK: flow(0)['locations'][1]['step'] == 2
+// CHECK: flow(0)['locations'][1]['importance'] == "essential"
+// CHECK: flow(0)['locations'][1]['location']['message']['text'] == "tainted"
+// CHECK: flow(0)['locations'][1]['location']['physicalLocation']['region']['startLine'] == 9
Index: Analysis/diagnostics/sarif-check.py
===
--- /dev/null
+++ Analysis/diagnostics/sarif-check.py
@@ -0,0 +1,61 @@
+#!/usr/bin/env python
+
+import sys
+import subprocess
+import traceback
+import json
+
+testfile = sys.argv[1]
+with open(sys.argv[2]) as datafh:
+data = json.load(datafh)
+
+prefix = "CHECK: "
+
+def sarifLogFirstResult(idx):
+return data['runs'][0]['results'][idx]
+
+def threadFlow(idx):
+return data['runs'][0]['results'][0]['codeFlows'][0]['threadFlows'][idx]
+
+fails = 0
+passes = 0
+with open(testfile) as testfh:
+lineno = 0
+for line in iter(testfh.readline, ""):
+lineno += 1
+line = line.rstrip("\r\n")
+try:
+prefix_pos = line.index(prefix)
+except ValueError:
+continue
+check_expr = line[prefix_pos + len(prefix):]
+
+try:
+exception = None
+result = eval(check_expr, {"sarifLog":data,
+   "result":sarifLogFirstResult,
+   "flow":threadFlow})
+except Exception:
+result = False
+exception = traceback.format_exc().splitlines()[-1]
+
+if exception is not None:
+sys.stderr.write(
+"{file}:{line:d}: check threw exception: {expr}\n"
+"{file}:{line:d}: exception was: {exception}\n".format(
+file=testfile, line=lineno,
+expr=check_expr, exception=exception))
+fails += 1
+elif not result:
+sys.stderr.write(
+"{file}:{line:d}: check returned False: {expr}\n".format(
+file=testfile, line=lineno, expr=check_expr))
+fails += 1
+else:
+passes += 1
+
+if fails != 0:
+sys.exit("{} checks failed".format(fails))
+else:
+sys.stdout.write("{} checks passed\n".format(passes))
+
Index: StaticAnalyzer/Core/SarifDiagnostics.cpp
===
--- /dev/null
+++ StaticAnalyzer/Core/SarifDiagnostics.cpp
@@ -0,0 +1,267 @@
+//===--- SarifDiagnostics.cpp - Sarif Diagnostics for Paths -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+//  This file defines the SarifDiagnostics object.
+//
+//===--===//
+
+#include "clang/Basic/Version.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h"

RE: r344957 - Give Multiversion-inline functions linkonce linkage

[Keane, Erich via cfe-commits]

GCC actually doesn’t support function multiversioning in C mode, so this fix is 
part of our extension to support C with multiversioning.  I perhaps wonder if 
this is part of the reason GCC only supports it in C++ mode...

From: David Blaikie [mailto:dblai...@gmail.com]
Sent: Monday, October 29, 2018 11:25 AM
To: Keane, Erich ; Eric Christopher 
Cc: cfe-commits@lists.llvm.org
Subject: Re: r344957 - Give Multiversion-inline functions linkonce linkage

Does this match GCC's approach here?

(I ask this sort of as throwaway/conversation starter - because the 
linkage/behavior around multiversion functions and their inlining is full of 
sharp corners/risks of code moving out of the areas appropriately restricted 
based on the cpu features)
On Mon, Oct 22, 2018 at 2:22 PM Erich Keane via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: erichkeane
Date: Mon Oct 22 14:20:45 2018
New Revision: 344957

URL: http://llvm.org/viewvc/llvm-project?rev=344957=rev
Log:
Give Multiversion-inline functions linkonce linkage

Since multiversion variant functions can be inline, in C they become
available-externally linkage.  This ends up causing the variants to not
be emitted, and not available to the linker.

The solution is to make sure that multiversion functions are always
emitted by marking them linkonce.

Change-Id: I897aa37c7cbba0c1eb2c57ee881d5000a2113b75

Modified:
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/test/CodeGen/attr-target-mv.c

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=344957=344956=344957=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Mon Oct 22 14:20:45 2018
@@ -3669,6 +3669,10 @@ llvm::GlobalValue::LinkageTypes CodeGenM
   return llvm::GlobalVariable::WeakAnyLinkage;
   }

+  if (const auto *FD = D->getAsFunction())
+if (FD->isMultiVersion() && Linkage == GVA_AvailableExternally)
+  return llvm::GlobalVariable::LinkOnceAnyLinkage;
+
   // We are guaranteed to have a strong definition somewhere else,
   // so we can use available_externally linkage.
   if (Linkage == GVA_AvailableExternally)

Modified: cfe/trunk/test/CodeGen/attr-target-mv.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/attr-target-mv.c?rev=344957=344956=344957=diff
==
--- cfe/trunk/test/CodeGen/attr-target-mv.c (original)
+++ cfe/trunk/test/CodeGen/attr-target-mv.c Mon Oct 22 14:20:45 2018
@@ -88,19 +88,19 @@ void bar4() {

 // CHECK: declare i32 @foo.arch_sandybridge()

-// CHECK: define available_externally i32 @foo_inline.sse4.2()
+// CHECK: define linkonce i32 @foo_inline.sse4.2()
 // CHECK: ret i32 0

 // CHECK: declare i32 @foo_inline.arch_sandybridge()
 //
-// CHECK: define available_externally i32 @foo_inline.arch_ivybridge()
+// CHECK: define linkonce i32 @foo_inline.arch_ivybridge()
 // CHECK: ret i32 1
-// CHECK: define available_externally i32 @foo_inline()
+// CHECK: define linkonce i32 @foo_inline()
 // CHECK: ret i32 2

-// CHECK: define available_externally void @foo_decls()
-// CHECK: define available_externally void @foo_decls.sse4.2()
+// CHECK: define linkonce void @foo_decls()
+// CHECK: define linkonce void @foo_decls.sse4.2()

-// CHECK: define available_externally void @foo_multi.avx_sse4.2()
-// CHECK: define available_externally void @foo_multi.fma4_sse4.2()
-// CHECK: define available_externally void 
@foo_multi.arch_ivybridge_fma4_sse4.2()
+// CHECK: define linkonce void @foo_multi.avx_sse4.2()
+// CHECK: define linkonce void @foo_multi.fma4_sse4.2()
+// CHECK: define linkonce void @foo_multi.arch_ivybridge_fma4_sse4.2()


___
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: r345109 - Debug Info (-gmodules): emit full types for non-anchored template specializations

[David Blaikie via cfe-commits]

Is this a workaround for now with the intent to fix this to allow such
implicit specializations to have their debug info modularized? I believe
this does work correctly in modular debug info with expliict modules, would
probably be sort of nice to have these things be consistent/similar?

On Tue, Oct 23, 2018 at 5:08 PM Adrian Prantl via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: adrian
> Date: Tue Oct 23 17:06:02 2018
> New Revision: 345109
>
> URL: http://llvm.org/viewvc/llvm-project?rev=345109=rev
> Log:
> Debug Info (-gmodules): emit full types for non-anchored template
> specializations
>
> Before this patch, clang would emit a (module-)forward declaration for
> template instantiations that are not anchored by an explicit template
> instantiation, but still are guaranteed to be available in an imported
> module. Unfortunately detecting the owning module doesn't reliably
> work when local submodule visibility is enabled and the template is
> inside a cross-module namespace.
>
> This make clang debuggable again with -gmodules and LSV enabled.
>
> rdar://problem/41552377
>
> Added:
> cfe/trunk/test/Modules/Inputs/lsv-debuginfo/
> cfe/trunk/test/Modules/Inputs/lsv-debuginfo/A/
> cfe/trunk/test/Modules/Inputs/lsv-debuginfo/A/ADT.h
> cfe/trunk/test/Modules/Inputs/lsv-debuginfo/B/
> cfe/trunk/test/Modules/Inputs/lsv-debuginfo/B/B.h
> cfe/trunk/test/Modules/Inputs/lsv-debuginfo/C/
> cfe/trunk/test/Modules/Inputs/lsv-debuginfo/C/C.h
> cfe/trunk/test/Modules/Inputs/lsv-debuginfo/module.modulemap
> cfe/trunk/test/Modules/lsv-debuginfo.cpp   (with props)
> Modified:
> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> cfe/trunk/test/Modules/ExtDebugInfo.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=345109=345108=345109=diff
>
> ==
> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Tue Oct 23 17:06:02 2018
> @@ -1955,8 +1955,17 @@ static bool isDefinedInClangModule(const
>if (auto *CXXDecl = dyn_cast(RD)) {
>  if (!CXXDecl->isCompleteDefinition())
>return false;
> +// Check wether RD is a template.
>  auto TemplateKind = CXXDecl->getTemplateSpecializationKind();
>  if (TemplateKind != TSK_Undeclared) {
> +  // Unfortunately getOwningModule() isn't accurate enough to find the
> +  // owning module of a ClassTemplateSpecializationDecl that is
> inside a
> +  // namespace spanning multiple modules.
> +  bool Explicit = false;
> +  if (auto *TD = dyn_cast(CXXDecl))
> +Explicit = TD->isExplicitInstantiationOrSpecialization();
> +  if (!Explicit && CXXDecl->getEnclosingNamespaceContext())
> +return false;
>// This is a template, check the origin of the first member.
>if (CXXDecl->field_begin() == CXXDecl->field_end())
>  return TemplateKind == TSK_ExplicitInstantiationDeclaration;
>
> Modified: cfe/trunk/test/Modules/ExtDebugInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ExtDebugInfo.cpp?rev=345109=345108=345109=diff
>
> ==
> --- cfe/trunk/test/Modules/ExtDebugInfo.cpp (original)
> +++ cfe/trunk/test/Modules/ExtDebugInfo.cpp Tue Oct 23 17:06:02 2018
> @@ -83,11 +83,11 @@ void foo() {
>  // CHECK: ![[NS]] = !DINamespace(name: "DebugCXX", scope: ![[MOD:[0-9]+]])
>  // CHECK: ![[MOD]] = !DIModule(scope: null, name: {{.*}}DebugCXX
>
> -// This type is anchored in the module by an explicit template
> instantiation.
> +// This type is not anchored in the module by an explicit template
> instantiation.
>  // CHECK: !DICompositeType(tag: DW_TAG_class_type,
>  // CHECK-SAME: name: "Template
> >",
>  // CHECK-SAME: scope: ![[NS]],
> -// CHECK-SAME: flags: DIFlagFwdDecl,
> +// CHECK-SAME: elements:
>  // CHECK-SAME: identifier:
> "_ZTSN8DebugCXX8TemplateIlNS_6traitsIl")
>
>  // This type is anchored in the module by an explicit template
> instantiation.
>
> Added: cfe/trunk/test/Modules/Inputs/lsv-debuginfo/A/ADT.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/lsv-debuginfo/A/ADT.h?rev=345109=auto
>
> ==
> --- cfe/trunk/test/Modules/Inputs/lsv-debuginfo/A/ADT.h (added)
> +++ cfe/trunk/test/Modules/Inputs/lsv-debuginfo/A/ADT.h Tue Oct 23
> 17:06:02 2018
> @@ -0,0 +1,45 @@
> +#ifndef ADT
> +#define ADT
> +
> +#ifdef WITH_NAMESPACE
> +namespace llvm {
> +#endif
> +template 
> +struct AlignedCharArray {
> +  alignas(Alignment) char buffer[Size];
> +};
> +
> +template 
> +class AlignerImpl {
> +  T1 t1;
> +};
> +
> +template 
> +union SizerImpl {
> +  char arr1[sizeof(T1)];
> +};
> +
> +template 
> +struct 

Re: r344957 - Give Multiversion-inline functions linkonce linkage

[David Blaikie via cfe-commits]

Does this match GCC's approach here?

(I ask this sort of as throwaway/conversation starter - because the
linkage/behavior around multiversion functions and their inlining is full
of sharp corners/risks of code moving out of the areas appropriately
restricted based on the cpu features)

On Mon, Oct 22, 2018 at 2:22 PM Erich Keane via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: erichkeane
> Date: Mon Oct 22 14:20:45 2018
> New Revision: 344957
>
> URL: http://llvm.org/viewvc/llvm-project?rev=344957=rev
> Log:
> Give Multiversion-inline functions linkonce linkage
>
> Since multiversion variant functions can be inline, in C they become
> available-externally linkage.  This ends up causing the variants to not
> be emitted, and not available to the linker.
>
> The solution is to make sure that multiversion functions are always
> emitted by marking them linkonce.
>
> Change-Id: I897aa37c7cbba0c1eb2c57ee881d5000a2113b75
>
> Modified:
> cfe/trunk/lib/CodeGen/CodeGenModule.cpp
> cfe/trunk/test/CodeGen/attr-target-mv.c
>
> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=344957=344956=344957=diff
>
> ==
> --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Mon Oct 22 14:20:45 2018
> @@ -3669,6 +3669,10 @@ llvm::GlobalValue::LinkageTypes CodeGenM
>return llvm::GlobalVariable::WeakAnyLinkage;
>}
>
> +  if (const auto *FD = D->getAsFunction())
> +if (FD->isMultiVersion() && Linkage == GVA_AvailableExternally)
> +  return llvm::GlobalVariable::LinkOnceAnyLinkage;
> +
>// We are guaranteed to have a strong definition somewhere else,
>// so we can use available_externally linkage.
>if (Linkage == GVA_AvailableExternally)
>
> Modified: cfe/trunk/test/CodeGen/attr-target-mv.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/attr-target-mv.c?rev=344957=344956=344957=diff
>
> ==
> --- cfe/trunk/test/CodeGen/attr-target-mv.c (original)
> +++ cfe/trunk/test/CodeGen/attr-target-mv.c Mon Oct 22 14:20:45 2018
> @@ -88,19 +88,19 @@ void bar4() {
>
>  // CHECK: declare i32 @foo.arch_sandybridge()
>
> -// CHECK: define available_externally i32 @foo_inline.sse4.2()
> +// CHECK: define linkonce i32 @foo_inline.sse4.2()
>  // CHECK: ret i32 0
>
>  // CHECK: declare i32 @foo_inline.arch_sandybridge()
>  //
> -// CHECK: define available_externally i32 @foo_inline.arch_ivybridge()
> +// CHECK: define linkonce i32 @foo_inline.arch_ivybridge()
>  // CHECK: ret i32 1
> -// CHECK: define available_externally i32 @foo_inline()
> +// CHECK: define linkonce i32 @foo_inline()
>  // CHECK: ret i32 2
>
> -// CHECK: define available_externally void @foo_decls()
> -// CHECK: define available_externally void @foo_decls.sse4.2()
> +// CHECK: define linkonce void @foo_decls()
> +// CHECK: define linkonce void @foo_decls.sse4.2()
>
> -// CHECK: define available_externally void @foo_multi.avx_sse4.2()
> -// CHECK: define available_externally void @foo_multi.fma4_sse4.2()
> -// CHECK: define available_externally void
> @foo_multi.arch_ivybridge_fma4_sse4.2()
> +// CHECK: define linkonce void @foo_multi.avx_sse4.2()
> +// CHECK: define linkonce void @foo_multi.fma4_sse4.2()
> +// CHECK: define linkonce void @foo_multi.arch_ivybridge_fma4_sse4.2()
>
>
> ___
> 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

[PATCH] D52794: [analyzer][PlistMacroExpansion] Part 2.: Retrieving the macro name and primitive expansion

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added inline comments.



Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:787-791
+  // Acquire the macro's name.
+  Token TheTok;
+  RawLexer.LexFromRawLexer(TheTok);
+
+  std::string MacroName = PP.getSpelling(TheTok);

NoQ wrote:
> Not sure, random thought: Could this work in fact be done with the //static// 
> `Lexer::getImmediateMacroName()` helper?
I need to create a lexer to lex the arguments of function-like macros (which is 
in part 3), so I can't get away with it I'm afraid :/


https://reviews.llvm.org/D52794



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

[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

[Taewook Oh via Phabricator via cfe-commits]

twoh added a comment.

@rsmith @dblaikie Thank you for the comments! It seems that this is not the 
appropriate way to handle the issue. I'll find different way to resolve the 
problem.


Repository:
  rC Clang

https://reviews.llvm.org/D38061



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

[PATCH] D53276: [analyzer][NFC] Fix some incorrect uses of -analyzer-config options

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added a comment.

@NoQ did you have time to ping Devin about this? (Sorry for the early ping, but 
this patch is blocking a lot of my other patches.)


https://reviews.llvm.org/D53276



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

[PATCH] D53066: [Driver] Use forward slashes in most linker arguments

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht resigned from this revision.
rupprecht added a comment.

I'm not familiar with this code or anything windows related, and it looks like 
@rnk is, so I'll remove myself and let him approve


Repository:
  rL LLVM

https://reviews.llvm.org/D53066



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

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 171538.
JonasToth added a comment.

- Merge branch 'master' into experiment_isolate_decl
- adjust doc slighty to comment


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/IsolateDeclarationCheck.cpp
  clang-tidy/readability/IsolateDeclarationCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/utils/LexerUtils.cpp
  clang-tidy/utils/LexerUtils.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-isolate-declaration.rst
  test/clang-tidy/readability-isolate-declaration-cxx17.cpp
  test/clang-tidy/readability-isolate-declaration-fixing.cpp
  test/clang-tidy/readability-isolate-declaration.c
  test/clang-tidy/readability-isolate-declaration.cpp

Index: test/clang-tidy/readability-isolate-declaration.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-isolate-declaration.cpp
@@ -0,0 +1,412 @@
+// RUN: %check_clang_tidy %s readability-isolate-declaration %t
+
+void f() {
+  int i;
+}
+
+void f2() {
+  int i, j, *k, lala = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int j;
+  // CHECK-FIXES: {{^  }}int *k;
+  // CHECK-FIXES: {{^  }}int lala = 42;
+
+  int normal, weird = /* comment */ 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int normal;
+  // CHECK-FIXES: {{^  }}int weird = /* comment */ 42;
+
+  int /* here is a comment */ v1,
+  // another comment
+  v2 = 42 // Ok, more comments
+  ;
+  // CHECK-MESSAGES: [[@LINE-4]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int /* here is a comment */ v1;
+  // CHECK-FIXES: {{^  }}int /* here is a comment */ // another comment
+  // CHECK-FIXES: {{^  }}v2 = 42 // Ok, more comments
+  // CHECK-FIXES: {{^  }};
+
+  auto int1 = 42, int2 = 0, int3 = 43;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: auto int1 = 42;
+  // CHECK-FIXES: {{^  }}auto int2 = 0;
+  // CHECK-FIXES: {{^  }}auto int3 = 43;
+
+  decltype(auto) ptr1 = , ptr2 = 
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: decltype(auto) ptr1 = 
+  // CHECK-FIXES: {{^  }}decltype(auto) ptr2 = 
+
+  decltype(k) ptr3 = , ptr4 = 
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: decltype(k) ptr3 = 
+  // CHECK-FIXES: {{^  }}decltype(k) ptr4 = 
+}
+
+void f3() {
+  int i, *pointer1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int *pointer1;
+  //
+  int *pointer2 = nullptr, *pointer3 = 
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int *pointer2 = nullptr;
+  // CHECK-FIXES: {{^  }}int *pointer3 = 
+
+  int *(i_ptr) = nullptr, *((i_ptr2));
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int *(i_ptr) = nullptr;
+  // CHECK-FIXES: {{^  }}int *((i_ptr2));
+
+  float(*f_ptr)[42], (((f_value))) = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: float (*f_ptr)[42];
+  // CHECK-FIXES: {{^  }}float (((f_value))) = 42;
+
+  float(((*f_ptr2)))[42], ((*f_ptr3)), f_value2 = 42.f;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: float (((*f_ptr2)))[42];
+  // CHECK-FIXES: {{^  }}float ((*f_ptr3));
+  // CHECK-FIXES: {{^  }}float f_value2 = 42.f;
+
+  float(((*f_ptr4)))[42], *f_ptr5, ((f_value3));
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: float (((*f_ptr4)))[42];
+  // CHECK-FIXES: {{^  }}float *f_ptr5;
+  // CHECK-FIXES: {{^  }}float ((f_value3));
+
+  void(((*f2))(int)), (*g2)(int, float);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: void (((*f2))(int));
+  // CHECK-FIXES: {{^  }}void (*g2)(int, float);
+
+  float(*(*(*f_ptr6)))[42], (*f_ptr7);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: float (*(*(*f_ptr6)))[42];
+  // CHECK-FIXES: {{^  }}float (*f_ptr7);
+}
+
+void f4() {
+  double d = 42. /* foo */, z = 43., /* hi */ y, c /* */ /*  */, l = 2.;
+  // CHECK-MESSAGES: 

[PATCH] D53705: [OpenCL] Postpone PSV address space diagnostic

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D53705#1279068, @svenvh wrote:

> Unlikely, since address spaces are provided in a different way in OpenCL C++ 
> vs OpenCL C.
>
> OpenCL C provides qualifiers such as `global` as part of the language.  
> OpenCL C++ provides template classes such as `cl::global` through a header 
> file.


So OpenCL C code has to be completely rewritten if it needs to be used as part 
of an OpenCL C++ program?  And it doesn't really compose like a type if it's 
supposed to change how a variable is stored.  What a terrible little mess 
they've made for themselves.

I think a better solution might be to hack the parser to recognize cases like 
this.  It's more annoying work, but it'll lead to much better results.


Repository:
  rC Clang

https://reviews.llvm.org/D53705



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

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D53738#1278528, @ebevhan wrote:

> In https://reviews.llvm.org/D53738#1278078, @rjmccall wrote:
>
> > I don't think we should add *types* just for this, but if you need to make 
> > a different kind of `BinaryOperator` that represents that the semantics 
> > aren't quite the same (the same way that the compound assignments have 
> > their own subclass), that seems natural to me.
>
>
> I don't know if adding a new operator kind was what I had in mind. The 
> operator hasn't changed, it's still a normal binary operator. Compound 
> assignments are their own operator with its own syntax; it doesn't really 
> strike me as the same thing.


Sorry for being unclear.  I wasn't suggesting adding a new 
`BinaryOperatorKind`, I was suggesting making a subclass of `BinaryOperator` 
that stored the extra information you'd like to store.

> The important difference would be that we separate the semantics of 
> performing the conversion and the operation. I understand that adding a new 
> type could be a bit more involved than baking the conversion into the 
> operator, but I really do not enjoy seeing so much implicit, 
> implementation-specific logic encapsulated in the same AST element. Anyone 
> who wants to look at BinaryOperators with fixed-point types will need to know 
> all of the details of how the finding the common type and conversion is done, 
> rather than simply "it does an addition".

It's not just that adding a new type is "involved", it's that it's a bad 
solution.  Those types can't actually be expressed in the language, and 
changing the type system to be able to express them is going to lead to a lot 
of pain elsewhere.


Repository:
  rC Clang

https://reviews.llvm.org/D53738



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

[PATCH] D53817: [clang-tidy] cppcoreguidelines-macro-usage: print macro names

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D53817#1279053, @JonasToth wrote:

> > I personally haven't yet seen this check firing on *predefined* macros.
>
> I did specifically check the compiler defined macros I could see in the AST 
> and they were not diagnosed from the beginning.
>  I would guess, only the compiler-flag macros are passed in the PPCallbacks, 
> which would explain the current behaviour.


Thanks for checking; I hadn't had the chance to try it out yet and was making a 
guess.

>> The only problem i have are these command-line-based macros.
>>  They *may* be outside of programmer's control, but if one has access to the 
>> buildsystem
>>  (as in, not *just* to the source code to be compiled, but to the build 
>> system definitions
>>  that say *how* to build it), then one can easily add such 
>> command-line-based macros.
> 
> It is a burden with little to gain as they the toggle-stuff use-case is not 
> replaceable with other constructs.
>  Having it as opt-in might be ok, but by default the check should not 
> diagnose them.

+1; I'd be fine with an opt-in option for the behavior.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53817



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

r345517 - Revert "Revert "Support for groups of attributes in #pragma clang attribute""

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Mon Oct 29 10:38:42 2018
New Revision: 345517

URL: http://llvm.org/viewvc/llvm-project?rev=345517=rev
Log:
Revert "Revert "Support for groups of attributes in #pragma clang attribute""

This reverts commit r345487, which reverted r345486. I think the crashes were
caused by an OOM on the builder, trying again to confirm...

Modified:
cfe/trunk/docs/LanguageExtensions.rst
cfe/trunk/docs/ReleaseNotes.rst
cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Parse/ParsePragma.cpp
cfe/trunk/lib/Sema/SemaAttr.cpp
cfe/trunk/test/Parser/pragma-attribute.cpp
cfe/trunk/test/Sema/pragma-attribute.c

Modified: cfe/trunk/docs/LanguageExtensions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LanguageExtensions.rst?rev=345517=345516=345517=diff
==
--- cfe/trunk/docs/LanguageExtensions.rst (original)
+++ cfe/trunk/docs/LanguageExtensions.rst Mon Oct 29 10:38:42 2018
@@ -2651,17 +2651,19 @@ Specifying an attribute for multiple dec
 
 The ``#pragma clang attribute`` directive can be used to apply an attribute to
 multiple declarations. The ``#pragma clang attribute push`` variation of the
-directive pushes a new attribute to the attribute stack. The declarations that
-follow the pragma receive the attributes that are on the attribute stack, until
-the stack is cleared using a ``#pragma clang attribute pop`` directive. 
Multiple
-push directives can be nested inside each other.
+directive pushes a new "scope" of ``#pragma clang attribute`` that attributes
+can be added to. The ``#pragma clang attribute (...)`` variation adds an
+attribute to that scope, and the ``#pragma clang attribute pop`` variation pops
+the scope. You can also use ``#pragma clang attribute push (...)``, which is a
+shorthand for when you want to add one attribute to a new scope. Multiple push
+directives can be nested inside each other.
 
 The attributes that are used in the ``#pragma clang attribute`` directives
 can be written using the GNU-style syntax:
 
 .. code-block:: c++
 
-  #pragma clang attribute push(__attribute__((annotate("custom"))), apply_to = 
function)
+  #pragma clang attribute push (__attribute__((annotate("custom"))), apply_to 
= function)
 
   void function(); // The function now has the annotate("custom") attribute
 
@@ -2671,7 +2673,7 @@ The attributes can also be written using
 
 .. code-block:: c++
 
-  #pragma clang attribute push([[noreturn]], apply_to = function)
+  #pragma clang attribute push ([[noreturn]], apply_to = function)
 
   void function(); // The function now has the [[noreturn]] attribute
 
@@ -2681,7 +2683,7 @@ The ``__declspec`` style syntax is also
 
 .. code-block:: c++
 
-  #pragma clang attribute push(__declspec(dllexport), apply_to = function)
+  #pragma clang attribute push (__declspec(dllexport), apply_to = function)
 
   void function(); // The function now has the __declspec(dllexport) attribute
 

Modified: cfe/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=345517=345516=345517=diff
==
--- cfe/trunk/docs/ReleaseNotes.rst (original)
+++ cfe/trunk/docs/ReleaseNotes.rst Mon Oct 29 10:38:42 2018
@@ -86,8 +86,8 @@ Modified Compiler Flags
 New Pragmas in Clang
 
 
-Clang now supports the ...
-
+- Clang now supports adding multiple `#pragma clang attribute` attributes into
+  a scope of pushed attributes.
 
 Attribute Changes in Clang
 --

Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=345517=345516=345517=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Mon Oct 29 10:38:42 
2018
@@ -1032,8 +1032,8 @@ def err_pragma_optimize_invalid_argument
 def err_pragma_optimize_extra_argument : Error<
   "unexpected extra argument '%0' to '#pragma clang optimize'">;
 // - #pragma clang attribute
-def err_pragma_attribute_expected_push_pop : Error<
-  "expected 'push' or 'pop' after '#pragma clang attribute'">;
+def err_pragma_attribute_expected_push_pop_paren : Error<
+  "expected 'push', 'pop', or '(' after '#pragma clang attribute'">;
 def err_pragma_attribute_invalid_argument : Error<
   "unexpected argument '%0' to '#pragma clang attribute'; "
   "expected 'push' or 'pop'">;

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=345517=345516=345517=diff

[PATCH] D50147: clang-format: support external styles

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In https://reviews.llvm.org/D50147#1272742, @sammccall wrote:

> Being able to discover the right style from the filesystem is powerful, and 
> if I was going to use this flag, I'd consider symlinking the style-file to 
> `subproject/.clang_format` instead. That way the setting is persistent and 
> available to all tools, rather than us needing to get it right on each 
> invocation.


I did not think of this solution. However:

- I am not sure how it works when the .clang-format link is stored in SCM: not 
sure git will allow storing a link which points outside of the repo... But it 
may be worth trying, it could be an alternative
- Making a symlink will hardcode the full path of the style, which may not suit 
different users: maybe in some cases the styles could be installed per-user (as 
users do not have root permissions), which they would be installed system-wide 
(in containers used for CI builds, where we may not know which user will 
actually run the build)
- Making a symlink will definitely not be portable to different OS : esp. on 
Windows, the path would probably not be the same
- I am not sure a symlink can point to a "~"-relative path, and automatically 
perform the user HOME path resolution




Comment at: lib/Format/Format.cpp:938
+  const llvm::SmallVector paths = {
+{"/usr/local/share/clang-format/", Name},
+{"~/.local/share/clang-format/", Name},

sammccall wrote:
> I'm not sure these prefixes are a good idea - can you explain what the 
> purpose is, and why the simpler model of just using the path doesn't work?
> 
> Certainly this needs some more thought about how it would work on windows 
> etc. I'd suggest limiting this patch to filenames.
the goal is actually to store these "base" styles in some global folder, so 
that multiple projects can reference it (through their own .clang-format file), 
without having to make any copy.

The idea is that the project is under SCM, and simply includes a reference to 
the style: in its own .clang-format, which would ideally only contain a single 
//basedOn// tag. The styles are defined globally, then each project only 
indicates which styles in uses.

But indeed, the 'HOME' folder on windows is probably not this one...


Repository:
  rC Clang

https://reviews.llvm.org/D50147



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

[PATCH] D53705: [OpenCL] Postpone PSV address space diagnostic

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added a comment.

Unlikely, since address spaces are provided in a different way in OpenCL C++ vs 
OpenCL C.

OpenCL C provides qualifiers such as `global` as part of the language.  OpenCL 
C++ provides template classes such as `cl::global` through a header file.


Repository:
  rC Clang

https://reviews.llvm.org/D53705



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

[PATCH] D50147: clang-format: support external styles

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In https://reviews.llvm.org/D50147#1279056, @Typz wrote:

> In https://reviews.llvm.org/D50147#1272742, @sammccall wrote:
>
> > The idea here does seem to be a natural extension of -style, at least for 
> > the case when the arg is a filename directly. I'm not opposed, happy to 
> > review this.
> >
> > I do want to probe the use case a bit though: we have found that 
> > configuring via -style= doesn't scale that well to lots of codebases and 
> > tools. e.g. if someone's running clang-tidy or clangd via some driver, are 
> > they going to have a way to plumb through the right -style=?
> >
> > Being able to discover the right style from the filesystem is powerful, and 
> > if I was going to use this flag, I'd consider symlinking the style-file to 
> > `subproject/.clang_format` instead. That way the setting is persistent and 
> > available to all tools, rather than us needing to get it right on each 
> > invocation.
>
>
> I totally agree: using '-style' does not scale well. However this patch works 
> also when the style is used in the 'basedOn' tag. This is actually the way we 
> expect to use this: store a small .clang_format file in each repo, which 
> simply references a clang-format file which must be installed (manually, for 
> now) on each user's computer.


`BasedOn: "/path/to/some/file"` would certainly work; but why not just use a 
symlink?


Repository:
  rC Clang

https://reviews.llvm.org/D50147



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

[PATCH] D53817: [clang-tidy] cppcoreguidelines-macro-usage: print macro names

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

> I personally haven't yet seen this check firing on *predefined* macros.

I did specifically check the compiler defined macros I could see in the AST and 
they were not diagnosed from the beginning.
I would guess, only the compiler-flag macros are passed in the PPCallbacks, 
which would explain the current behaviour.

> The only problem i have are these command-line-based macros.
>  They *may* be outside of programmer's control, but if one has access to the 
> buildsystem
>  (as in, not *just* to the source code to be compiled, but to the build 
> system definitions
>  that say *how* to build it), then one can easily add such command-line-based 
> macros.

It is a burden with little to gain as they the toggle-stuff use-case is not 
replaceable with other constructs.
Having it as opt-in might be ok, but by default the check should not diagnose 
them.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53817



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

[PATCH] D50147: clang-format: support external styles

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In https://reviews.llvm.org/D50147#1272742, @sammccall wrote:

> The idea here does seem to be a natural extension of -style, at least for the 
> case when the arg is a filename directly. I'm not opposed, happy to review 
> this.
>
> I do want to probe the use case a bit though: we have found that configuring 
> via -style= doesn't scale that well to lots of codebases and tools. e.g. if 
> someone's running clang-tidy or clangd via some driver, are they going to 
> have a way to plumb through the right -style=?
>
> Being able to discover the right style from the filesystem is powerful, and 
> if I was going to use this flag, I'd consider symlinking the style-file to 
> `subproject/.clang_format` instead. That way the setting is persistent and 
> available to all tools, rather than us needing to get it right on each 
> invocation.


I totally agree: using '-style' does not scale well. However this patch works 
also when the style is used in the 'basedOn' tag. This is actually the way we 
expect to use this: store a small .clang_format file in each repo, which simply 
references a clang-format file which must be installed (manually, for now) on 
each user's computer.


Repository:
  rC Clang

https://reviews.llvm.org/D50147



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