[PATCH] D53475: Create ConstantExpr class

[Bill Wendling via Phabricator via cfe-commits]

void added a subscriber: rsmith.
void added a comment.

I didn't see that during my tests. Probably different -W flags or
something. I reverted that bit. I hope it fixes the problem.

-bw


Repository:
  rC Clang

https://reviews.llvm.org/D53475



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

Re: [PATCH] D53475: Create ConstantExpr class

[Bill Wendling via cfe-commits]

I didn't see that during my tests. Probably different -W flags or
something. I reverted that bit. I hope it fixes the problem.

-bw

On Tue, Oct 30, 2018 at 10:02 PM Kristina Brooks via Phabricator <
revi...@reviews.llvm.org> wrote:

> kristina added a comment.
>
> Ah, it was causing this warning during build:
>
>   /SourceCache/llvm-trunk-8.0/tools/clang/include/clang/AST/Expr.h:903:1:
> warning: 'ConstantExpr' defined as a struct here but previously declared as
> a class [-Wmismatched-tags]
>
>
> Repository:
>   rC Clang
>
> https://reviews.llvm.org/D53475
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53475: Create ConstantExpr class

[Kristina Brooks via Phabricator via cfe-commits]

kristina added a comment.

Ah, it was causing this warning during build:

  /SourceCache/llvm-trunk-8.0/tools/clang/include/clang/AST/Expr.h:903:1: 
warning: 'ConstantExpr' defined as a struct here but previously declared as a 
class [-Wmismatched-tags]


Repository:
  rC Clang

https://reviews.llvm.org/D53475



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

r345695 - Change "struct" to "class" to avoid warnings

[Bill Wendling via cfe-commits]

Author: void
Date: Tue Oct 30 21:58:34 2018
New Revision: 345695

URL: http://llvm.org/viewvc/llvm-project?rev=345695=rev
Log:
Change "struct" to "class" to avoid warnings

Modified:
cfe/trunk/include/clang/AST/Expr.h

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=345695=345694=345695=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Tue Oct 30 21:58:34 2018
@@ -900,7 +900,8 @@ public:
 };
 
 /// ConstantExpr - An expression that occurs in a constant context.
-struct ConstantExpr : public FullExpr {
+class ConstantExpr : public FullExpr {
+public:
   ConstantExpr(Expr *subexpr)
 : FullExpr(ConstantExprClass, subexpr) {}
 


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

[PATCH] D53475: Create ConstantExpr class

[Bill Wendling via Phabricator via cfe-commits]

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

That change was intentional. But I guess it's causing issues. I'll change it to 
a `class`.


Repository:
  rC Clang

https://reviews.llvm.org/D53475



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

[PATCH] D53919: [X86] Don't allow illegal vector types to return by direct value.

[Craig Topper via Phabricator via cfe-commits]

craig.topper created this revision.
craig.topper added reviewers: efriedma, rnk, echristo.

The backend can't lower this correctly and will try to split the return value 
into multiple registers.

This patches forces it to return via memory similar to what was already done 
for arguments.

Fixes PR39501.


Repository:
  rC Clang

https://reviews.llvm.org/D53919

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/x86_64-arguments.c


Index: test/CodeGen/x86_64-arguments.c
===
--- test/CodeGen/x86_64-arguments.c
+++ test/CodeGen/x86_64-arguments.c
@@ -545,3 +545,13 @@
 // AVX: @f65(<8 x float> %{{[^,)]+}})
 void f65(struct t65 a0) {
 }
+
+// SSE-LABEL: @f66(<32 x i8>* noalias sret %{{[^,)]+}})
+// AVX: <32 x i8> @f66()
+typedef char v66 __attribute__((vector_size (32))) ;
+v66 f66()
+{
+ v66 y = {'0','1','2','3','4','5','6','7','8','9','a','b','c','d','e','f',
+  '0','1','2','3','4','5','6','7','8','9','a','b','c','d','e','f'};
+ return y;
+}
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -2859,7 +2859,7 @@
 ABIArgInfo X86_64ABIInfo::getIndirectReturnResult(QualType Ty) const {
   // If this is a scalar LLVM value then assume LLVM will pass it in the right
   // place naturally.
-  if (!isAggregateTypeForABI(Ty)) {
+  if (!isAggregateTypeForABI(Ty) && !IsIllegalVectorType(Ty)) {
 // Treat an enum type as its underlying type.
 if (const EnumType *EnumTy = Ty->getAs())
   Ty = EnumTy->getDecl()->getIntegerType();


Index: test/CodeGen/x86_64-arguments.c
===
--- test/CodeGen/x86_64-arguments.c
+++ test/CodeGen/x86_64-arguments.c
@@ -545,3 +545,13 @@
 // AVX: @f65(<8 x float> %{{[^,)]+}})
 void f65(struct t65 a0) {
 }
+
+// SSE-LABEL: @f66(<32 x i8>* noalias sret %{{[^,)]+}})
+// AVX: <32 x i8> @f66()
+typedef char v66 __attribute__((vector_size (32))) ;
+v66 f66()
+{
+ v66 y = {'0','1','2','3','4','5','6','7','8','9','a','b','c','d','e','f',
+  '0','1','2','3','4','5','6','7','8','9','a','b','c','d','e','f'};
+ return y;
+}
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -2859,7 +2859,7 @@
 ABIArgInfo X86_64ABIInfo::getIndirectReturnResult(QualType Ty) const {
   // If this is a scalar LLVM value then assume LLVM will pass it in the right
   // place naturally.
-  if (!isAggregateTypeForABI(Ty)) {
+  if (!isAggregateTypeForABI(Ty) && !IsIllegalVectorType(Ty)) {
 // Treat an enum type as its underlying type.
 if (const EnumType *EnumTy = Ty->getAs())
   Ty = EnumTy->getDecl()->getIntegerType();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53475: Create ConstantExpr class

[Kristina Brooks via Phabricator via cfe-commits]

kristina added a comment.

I think you may have accidentally commited the wrong patch.

  +struct ConstantExpr : public FullExpr {

Is causing a warning right now, not sure where that came from.


Repository:
  rC Clang

https://reviews.llvm.org/D53475



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

[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D52674#1271814, @smeenai wrote:

> @rjmccall I prototyped the ForRTTI parameter approach in 
> https://reviews.llvm.org/D53546. It could definitely be cleaned up a bit, but 
> it demonstrates the problems I saw with the added parameter. Namely, 
> `mangleType(QualType, SourceRange, QualifierMangleMode)` has a bunch of 
> additional logic which is needed for correctness, so we need to thread the 
> parameter through the entire chain, and the only way I could think of doing 
> that without adding the parameter to every single `mangleType` overload was 
> to have an additional switch to dispatch to the overloads with the added 
> ForRTTI parameter, which is pretty ugly.
>
> As I see it, there are a few ways to proceed here:
>
> - The approach in https://reviews.llvm.org/D53546 (cleaned up a bit). I think 
> it's pretty ugly, but you may have suggestions on how to do it better.
> - In the `mangleType` overload for `ObjCObjectPointerType`, skipping the 
> generic `mangleType` dispatch and going directly to either the 
> `ObjCObjectType` or `ObjCInterfaceType` overloads. That avoids the ugliness 
> in the generic `mangleType`, but it also requires us to duplicate some logic 
> from it in the `ObjCObjectPointerType` overload, which doesn't seem great 
> either.
> - Maintaining `ForRTTI` state in the mangler instead of threading a parameter 
> through (I'm generally not a fan of state like that, but it might be cleaner 
> than the threading in this case?)
> - Just sticking with what I'm doing in this patch.
>
>   What do you think?


Sorry for the delay.  I think duplicating the dispatch logic for 
`ObjCObjectPointerType` is probably the best approach; the pointee type will 
always an `ObjCObjectType` of some sort, and there are only two kinds of those.


Repository:
  rC Clang

https://reviews.llvm.org/D52674



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

r345692 - Create ConstantExpr class

[Bill Wendling via cfe-commits]

Author: void
Date: Tue Oct 30 20:48:47 2018
New Revision: 345692

URL: http://llvm.org/viewvc/llvm-project?rev=345692=rev
Log:
Create ConstantExpr class

A ConstantExpr class represents a full expression that's in a context where a
constant expression is required. This class reflects the path the evaluator
took to reach the expression rather than the syntactic context in which the
expression occurs.

In the future, the class will be expanded to cache the result of the evaluated
expression so that it's not needlessly re-evaluated

Reviewed By: rsmith

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

Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/include/clang/AST/ExprCXX.h
cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
cfe/trunk/include/clang/Basic/StmtNodes.td
cfe/trunk/include/clang/Serialization/ASTBitCodes.h
cfe/trunk/lib/ARCMigrate/TransAutoreleasePool.cpp
cfe/trunk/lib/ARCMigrate/TransRetainReleaseDealloc.cpp
cfe/trunk/lib/ARCMigrate/TransUnbridgedCasts.cpp
cfe/trunk/lib/ARCMigrate/Transforms.cpp
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/AST/Expr.cpp
cfe/trunk/lib/AST/ExprCXX.cpp
cfe/trunk/lib/AST/ExprClassification.cpp
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/AST/ItaniumMangle.cpp
cfe/trunk/lib/AST/ParentMap.cpp
cfe/trunk/lib/AST/Stmt.cpp
cfe/trunk/lib/AST/StmtPrinter.cpp
cfe/trunk/lib/AST/StmtProfile.cpp
cfe/trunk/lib/Analysis/LiveVariables.cpp
cfe/trunk/lib/Analysis/ThreadSafety.cpp
cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp
cfe/trunk/lib/CodeGen/CGBlocks.cpp
cfe/trunk/lib/CodeGen/CGDecl.cpp
cfe/trunk/lib/CodeGen/CGStmt.cpp
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/lib/Sema/SemaExceptionSpec.cpp
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/lib/Sema/TreeTransform.h
cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
cfe/trunk/lib/Serialization/ASTWriterStmt.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/tools/libclang/CXCursor.cpp

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=345692=345691=345692=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Tue Oct 30 20:48:47 2018
@@ -869,6 +869,64 @@ public:
 };
 
 
//===--===//
+// Wrapper Expressions.
+//===--===//
+
+/// FullExpr - Represents a "full-expression" node.
+class FullExpr : public Expr {
+protected:
+ Stmt *SubExpr;
+
+ FullExpr(StmtClass SC, Expr *subexpr)
+: Expr(SC, subexpr->getType(),
+   subexpr->getValueKind(), subexpr->getObjectKind(),
+   subexpr->isTypeDependent(), subexpr->isValueDependent(),
+   subexpr->isInstantiationDependent(),
+   subexpr->containsUnexpandedParameterPack()), SubExpr(subexpr) {}
+  FullExpr(StmtClass SC, EmptyShell Empty)
+: Expr(SC, Empty) {}
+public:
+  const Expr *getSubExpr() const { return cast(SubExpr); }
+  Expr *getSubExpr() { return cast(SubExpr); }
+
+  /// As with any mutator of the AST, be very careful when modifying an
+  /// existing AST to preserve its invariants.
+  void setSubExpr(Expr *E) { SubExpr = E; }
+
+  static bool classof(const Stmt *T) {
+return T->getStmtClass() >= firstFullExprConstant &&
+   T->getStmtClass() <= lastFullExprConstant;
+  }
+};
+
+/// ConstantExpr - An expression that occurs in a constant context.
+struct ConstantExpr : public FullExpr {
+  ConstantExpr(Expr *subexpr)
+: FullExpr(ConstantExprClass, subexpr) {}
+
+  /// Build an empty constant expression wrapper.
+  explicit ConstantExpr(EmptyShell Empty)
+: FullExpr(ConstantExprClass, Empty) {}
+
+  SourceLocation getBeginLoc() const LLVM_READONLY {
+return SubExpr->getBeginLoc();
+  }
+  SourceLocation getEndLoc() const LLVM_READONLY {
+return SubExpr->getEndLoc();
+  }
+
+  static bool classof(const Stmt *T) {
+return T->getStmtClass() == ConstantExprClass;
+  }
+
+  // Iterators
+  child_range children() { return child_range(, +1); }
+  const_child_range children() const {
+return const_child_range(,  + 1);
+  }
+};
+
+//===--===//
 // Primary Expressions.
 
//===--===//
 

Modified: cfe/trunk/include/clang/AST/ExprCXX.h
URL: 

[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#1281738, @keryell wrote:

> In https://reviews.llvm.org/D53705#1278066, @rjmccall wrote:
>
> > I don't suppose there's any chance you can just tell Khronos to fix their 
> > stuff.  It's a little funny to be more conservative about keywords in C++ 
> > when the C++ committee is actually much more aggressive about adding 
> > keywords in the non-reserved space than C is.
>
>
> You are actually telling this to Khronos since a lot of Khronos members are 
> on the LLVM mailing lists and are reading this right now... :-)


Great.

> I am unsure to understand what you mean about "the C++ committee is actually 
> much more aggressive about adding keywords in the non-reserved space than C 
> is".

New versions of C++ have semi-regularly added keywords like `static_assert` and 
`thread_local` that are not in the reserved space for identifiers.  When C 
adopted these, it spelled them `_Static_assert` and `_Thread_local`, which are 
in the reserved space.  I was under the mistaken (right?) impression thought 
that e.g. `__constant` was a standardized spelling, and I thought that Khronos 
had just decided to avoid using the unreserved identifiers in C++ when it had 
gladly adopted them in C.  But now I understand that OpenCL C++ is attempting 
to be a radically different language model that does not actually have 
qualifiers at all.

> The motivation behind this was to have 0 language extension to C++ in OpenCL 
> C++, like with the single-source version of the standard, SYCL, where you can 
> have a program which is executed purely on CPU without any compiler support, 
> to avoid the CUDA & C++AMP atrocities.

By "0 language extension", do you just mean no *syntactic* extensions?  Because 
something like `cl::priv` certainly seems like it requires intrinsic language 
support, and I don't know what `add_global_t` could possibly do besides add a 
not-described-in-the-standard-but-still-obviously-there address-space qualifier 
so that member accesses continue to work and properly preserve the qualifier on 
the resulting l-value.  C++ doesn't provide adequate language tools to replace 
pointers and references at a language level, not by a long shot; the tools are 
only really good enough for resource management.

The "just compile C++ code for the GPU" idea that (IIRC) AMD has been exploring 
seems quite interesting, but unlike OpenCL C++ it really is a largely 
no-extensions approach: it's implementing the classic C++ language model (with 
facilities for taking more control) like any other frontend, just with a weird 
target on the backend.  It's also just hoping that the optimizer will be able 
to eliminate the overheads when mapping to a multiple-address-space model, and 
I'm not sure whether that's been borne out.  OpenCL C++ seems to be trying to 
strike a middle ground where there are minimal syntactic extensions but the 
type system is rather radically different in ways that often work but also 
often don't, and it's basically held together by the underlying extensions that 
it's pretending aren't there.  That doesn't seem like a successful language 
approach.

Also, I have no opinion about CUDA or C++AMP but feel I need to distance myself 
from your comment about them.

> That seems like a good discussion topic next Thursday at the LLVM Bay Area 
> Meetup. :-)

Alas, I live in New York; I was just out there for the Developer's Meeting but 
won't be back for some time.


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] D53475: Create ConstantExpr class

[Bill Wendling via Phabricator via cfe-commits]

void closed this revision.
void marked 2 inline comments as done.
void added inline comments.



Comment at: include/clang/AST/Expr.h:898
+class ConstantExpr : public FullExpr {
+  Stmt *Val;
+public:

rsmith wrote:
> I think it'd be cleaner and simpler to move this field into the base class, 
> merging it with the corresponding member of `ExprWithCleanups`.
Oh good. I was wanting to do that. Done. :-)


Repository:
  rC Clang

https://reviews.llvm.org/D53475



___
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

[Ronan Keryell via Phabricator via cfe-commits]

keryell added a comment.

In https://reviews.llvm.org/D53705#1278066, @rjmccall wrote:

> I don't suppose there's any chance you can just tell Khronos to fix their 
> stuff.  It's a little funny to be more conservative about keywords in C++ 
> when the C++ committee is actually much more aggressive about adding keywords 
> in the non-reserved space than C is.


You are actually telling this to Khronos since a lot of Khronos members are on 
the LLVM mailing lists and are reading this right now... :-)
I am unsure to understand what you mean about "the C++ committee is actually 
much more aggressive about adding keywords in the non-reserved space than C is".
The motivation behind this was to have 0 language extension to C++ in OpenCL 
C++, like with the single-source version of the standard, SYCL, where you can 
have a program which is executed purely on CPU without any compiler support, to 
avoid the CUDA & C++AMP atrocities.
That seems like a good discussion topic next Thursday at the LLVM Bay Area 
Meetup. :-)


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] D53860: [SemaCXX] Don't check base's dtor is accessible

[Brian Gesiak via Phabricator via cfe-commits]

modocache abandoned this revision.
modocache added a comment.

Yes, thanks @rsmith! And sorry @ahatanak for the trouble of explaining your 
code here.

The new standards behavior does impact @twoh and I's codebase in a few places, 
but as you explained we can simply change the source to not use 
list-initialization.

Thanks all!


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] D53705: [OpenCL] Postpone PSV address space diagnostic

[Ronan Keryell via Phabricator via cfe-commits]

keryell added a comment.

We could submit to the standard an OpenCL C++ extension to accept the OpenCL C 
syntax...


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] D53475: Create ConstantExpr class

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

Thanks, Richard! :-)


Repository:
  rC Clang

https://reviews.llvm.org/D53475



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

Re: r345676 - [Win64] Handle passing i128 by value

[Richard Trieu via cfe-commits]

I have reverted this in r345691 because it caused test
CodeGen/mingw-long-double.c to start failing.

Command Output (stderr):
--
/usr/local/google/clang/install/llvm/tools/clang/test/CodeGen/mingw-long-double.c:36:11:
error: MSC64: expected string not found in input
// MSC64: define dso_local double @TestLD(double %x)
  ^
:12:1: note: scanning from here
; Function Attrs: noinline nounwind optnone
^
:35:1: note: possible intended match here
define dso_local void @TestLDC({ double, double }* noalias sret
%agg.result, { double, double }* %x) #2 {
^

--

I suspect your patch has changed the type of "double" to a different
floating point type, causing the failure.


On Tue, Oct 30, 2018 at 5:00 PM Reid Kleckner via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rnk
> Date: Tue Oct 30 16:58:41 2018
> New Revision: 345676
>
> URL: http://llvm.org/viewvc/llvm-project?rev=345676=rev
> Log:
> [Win64] Handle passing i128 by value
>
> For arguments, pass it indirectly, since the ABI doc says pretty clearly
> that arguments larger than 8 bytes are passed indirectly. This makes
> va_list handling easier, anyway.
>
> When returning, GCC returns in XMM0, and we match them.
>
> Fixes PR39492.
>
> Added:
> cfe/trunk/test/CodeGen/win64-i128.c
> Modified:
> cfe/trunk/lib/CodeGen/TargetInfo.cpp
>
> Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=345676=345675=345676=diff
>
> ==
> --- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Tue Oct 30 16:58:41 2018
> @@ -3944,18 +3944,39 @@ ABIArgInfo WinX86_64ABIInfo::classify(Qu
>  return ABIArgInfo::getDirect(llvm::IntegerType::get(getVMContext(),
> Width));
>}
>
> -  // Bool type is always extended to the ABI, other builtin types are not
> -  // extended.
> -  const BuiltinType *BT = Ty->getAs();
> -  if (BT && BT->getKind() == BuiltinType::Bool)
> -return ABIArgInfo::getExtend(Ty);
> -
> -  // Mingw64 GCC uses the old 80 bit extended precision floating point
> unit. It
> -  // passes them indirectly through memory.
> -  if (IsMingw64 && BT && BT->getKind() == BuiltinType::LongDouble) {
> -const llvm::fltSemantics *LDF = ().getLongDoubleFormat();
> -if (LDF == ::APFloat::x87DoubleExtended())
> -  return ABIArgInfo::getIndirect(Align, /*ByVal=*/false);
> +  if (const BuiltinType *BT = Ty->getAs()) {
> +switch (BT->getKind()) {
> +case BuiltinType::Bool:
> +  // Bool type is always extended to the ABI, other builtin types are
> not
> +  // extended.
> +  return ABIArgInfo::getExtend(Ty);
> +
> +case BuiltinType::LongDouble:
> +  // Mingw64 GCC uses the old 80 bit extended precision floating point
> +  // unit. It passes them indirectly through memory.
> +  if (IsMingw64) {
> +const llvm::fltSemantics *LDF =
> ().getLongDoubleFormat();
> +if (LDF == ::APFloat::x87DoubleExtended())
> +  return ABIArgInfo::getIndirect(Align, /*ByVal=*/false);
> +break;
> +  }
> +
> +case BuiltinType::Int128:
> +case BuiltinType::UInt128:
> +  // If it's a parameter type, the normal ABI rule is that arguments
> larger
> +  // than 8 bytes are passed indirectly. GCC follows it. We follow it
> too,
> +  // even though it isn't particularly efficient.
> +  if (!IsReturnType)
> +return ABIArgInfo::getIndirect(Align, /*ByVal=*/false);
> +
> +  // Mingw64 GCC returns i128 in XMM0. Coerce to v2i64 to handle that.
> +  // Clang matches them for compatibility.
> +  return ABIArgInfo::getDirect(
> +  llvm::VectorType::get(llvm::Type::getInt64Ty(getVMContext()),
> 2));
> +
> +default:
> +  break;
> +}
>}
>
>return ABIArgInfo::getDirect();
>
> Added: cfe/trunk/test/CodeGen/win64-i128.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/win64-i128.c?rev=345676=auto
>
> ==
> --- cfe/trunk/test/CodeGen/win64-i128.c (added)
> +++ cfe/trunk/test/CodeGen/win64-i128.c Tue Oct 30 16:58:41 2018
> @@ -0,0 +1,16 @@
> +// RUN: %clang_cc1 -triple x86_64-windows-gnu -emit-llvm -o - %s \
> +// RUN:| FileCheck %s --check-prefix=GNU64
> +// RUN: %clang_cc1 -triple x86_64-windows-msvc -emit-llvm -o - %s \
> +// RUN:| FileCheck %s --check-prefix=MSC64
> +
> +typedef int int128_t __attribute__((mode(TI)));
> +
> +int128_t foo() { return 0; }
> +
> +// GNU64: define dso_local <2 x i64> @foo()
> +// MSC64: define dso_local <2 x i64> @foo()
> +
> +int128_t bar(int128_t a, int128_t b) { return a * b; }
> +
> +// GNU64: define dso_local <2 x i64> @bar(i128*, i128*)
> +// MSC64: define dso_local <2 x i64> @bar(i128*, i128*)
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> 

r345691 - Revert r345676 due to test failure.

[Richard Trieu via cfe-commits]

Author: rtrieu
Date: Tue Oct 30 19:10:51 2018
New Revision: 345691

URL: http://llvm.org/viewvc/llvm-project?rev=345691=rev
Log:
Revert r345676 due to test failure.

This was causing CodeGen/mingw-long-double.c to start failing.

Removed:
cfe/trunk/test/CodeGen/win64-i128.c
Modified:
cfe/trunk/lib/CodeGen/TargetInfo.cpp

Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=345691=345690=345691=diff
==
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Tue Oct 30 19:10:51 2018
@@ -3943,40 +3943,18 @@ ABIArgInfo WinX86_64ABIInfo::classify(Qu
 // Otherwise, coerce it to a small integer.
 return ABIArgInfo::getDirect(llvm::IntegerType::get(getVMContext(), 
Width));
   }
-
-  if (const BuiltinType *BT = Ty->getAs()) {
-switch (BT->getKind()) {
-case BuiltinType::Bool:
-  // Bool type is always extended to the ABI, other builtin types are not
-  // extended.
-  return ABIArgInfo::getExtend(Ty);
-
-case BuiltinType::LongDouble:
-  // Mingw64 GCC uses the old 80 bit extended precision floating point
-  // unit. It passes them indirectly through memory.
-  if (IsMingw64) {
-const llvm::fltSemantics *LDF = ().getLongDoubleFormat();
-if (LDF == ::APFloat::x87DoubleExtended())
-  return ABIArgInfo::getIndirect(Align, /*ByVal=*/false);
-break;
-  }
-
-case BuiltinType::Int128:
-case BuiltinType::UInt128:
-  // If it's a parameter type, the normal ABI rule is that arguments larger
-  // than 8 bytes are passed indirectly. GCC follows it. We follow it too,
-  // even though it isn't particularly efficient.
-  if (!IsReturnType)
-return ABIArgInfo::getIndirect(Align, /*ByVal=*/false);
-
-  // Mingw64 GCC returns i128 in XMM0. Coerce to v2i64 to handle that.
-  // Clang matches them for compatibility.
-  return ABIArgInfo::getDirect(
-  llvm::VectorType::get(llvm::Type::getInt64Ty(getVMContext()), 2));
-
-default:
-  break;
-}
+  // Bool type is always extended to the ABI, other builtin types are not
+  // extended.
+  const BuiltinType *BT = Ty->getAs();
+  if (BT && BT->getKind() == BuiltinType::Bool)
+return ABIArgInfo::getExtend(Ty);
+
+  // Mingw64 GCC uses the old 80 bit extended precision floating point unit. It
+  // passes them indirectly through memory.
+  if (IsMingw64 && BT && BT->getKind() == BuiltinType::LongDouble) {
+const llvm::fltSemantics *LDF = ().getLongDoubleFormat();
+if (LDF == ::APFloat::x87DoubleExtended())
+  return ABIArgInfo::getIndirect(Align, /*ByVal=*/false);
   }
 
   return ABIArgInfo::getDirect();

Removed: cfe/trunk/test/CodeGen/win64-i128.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/win64-i128.c?rev=345690=auto
==
--- cfe/trunk/test/CodeGen/win64-i128.c (original)
+++ cfe/trunk/test/CodeGen/win64-i128.c (removed)
@@ -1,16 +0,0 @@
-// RUN: %clang_cc1 -triple x86_64-windows-gnu -emit-llvm -o - %s \
-// RUN:| FileCheck %s --check-prefix=GNU64
-// RUN: %clang_cc1 -triple x86_64-windows-msvc -emit-llvm -o - %s \
-// RUN:| FileCheck %s --check-prefix=MSC64
-
-typedef int int128_t __attribute__((mode(TI)));
-
-int128_t foo() { return 0; }
-
-// GNU64: define dso_local <2 x i64> @foo()
-// MSC64: define dso_local <2 x i64> @foo()
-
-int128_t bar(int128_t a, int128_t b) { return a * b; }
-
-// GNU64: define dso_local <2 x i64> @bar(i128*, i128*)
-// MSC64: define dso_local <2 x i64> @bar(i128*, i128*)


___
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

[Taewook Oh via Phabricator via cfe-commits]

twoh added a comment.

@rsmith I see. Thank you for the clarification!


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

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

rsmith added a comment.

In https://reviews.llvm.org/D53860#1280959, @twoh wrote:

> For me it seems that the bug is checking destructor accessibility of Base 
> itself, not fields of Base.


The code is correct (or at least, following the current standard rule) as-is. 
The base class subobject itself is an element of the aggregate. Note that 
aggregate-initialization of `Derived` does not require `Base` to be an 
aggregate at all, so recursing to the fields of `Base` would be inappropriate.

If this is causing significant problems for real code, we can certainly try to 
take this back to the C++ committee and request a do-over, but I think it's 
unlikely the rule would be changed: for aggregate initialization, direct base 
class subobjects are (by design) treated exactly the same as fields, and the 
destructors for all such subobjects are potentially invoked by aggregate 
initialization (because if an exception is thrown after an element is 
initialized and before initialization of the complete object finishes, the 
element's destructor will be invoked). You could argue that the context for the 
access check should be the derived class and not the context in which aggregate 
initialization is performed. I've checked the minutes, and we did indeed 
discuss that when resolving C++ DR 2227, and decided that the context for the 
access check should be the context of the aggregate initialization, not the 
context of the class definition.

For what it's worth, this is not the only way that the C++17 extensions to 
aggregate initialization break existing code. There are easy workarounds: make 
sure that your class is not an aggregate (add a constructor), or don't use 
list-initialization.


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] D53916: [ARM64] [Windows] Implement _InterlockedExchangeAdd*_* builtins.

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm




Comment at: lib/CodeGen/CGBuiltin.cpp:100
+CodeGenFunction , llvm::AtomicRMWInst::BinOp Kind, const CallExpr *E,
+AtomicOrdering Ordering = AtomicOrdering::SequentiallyConsistent) {
   QualType T = E->getType();

This generalized nicely. :)


Repository:
  rC Clang

https://reviews.llvm.org/D53916



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

[PATCH] D53725: [CodeGen] Move `emitConstant` from ScalarExprEmitter to CodeGenFunction. NFC.

[John McCall via Phabricator via cfe-commits]

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

Well, that's the old style, but we've been slowly moving to the camelCase style 
instead.  Very, very slowly.  I won't hold up your patch over it.


https://reviews.llvm.org/D53725



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

[PATCH] D53916: [ARM64] [Windows] Implement _InterlockedExchangeAdd*_* builtins.

[Eli Friedman via Phabricator via cfe-commits]

efriedma updated this revision to Diff 171853.
efriedma added a comment.

Upload correct diff.


Repository:
  rC Clang

https://reviews.llvm.org/D53916

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

Index: test/CodeGen/ms-intrinsics.c
===
--- test/CodeGen/ms-intrinsics.c
+++ test/CodeGen/ms-intrinsics.c
@@ -3,13 +3,13 @@
 // RUN: | FileCheck %s -check-prefixes CHECK,CHECK-I386,CHECK-INTEL
 // RUN: %clang_cc1 -ffreestanding -fms-extensions -fms-compatibility -fms-compatibility-version=17.00 \
 // RUN: -triple thumbv7--windows -Oz -emit-llvm %s -o - \
-// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-ARM,CHECK-ARM-X64
+// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-ARM,CHECK-ARM-ARM64,CHECK-ARM-X64
 // RUN: %clang_cc1 -ffreestanding -fms-extensions -fms-compatibility -fms-compatibility-version=17.00 \
 // RUN: -triple x86_64--windows -Oz -emit-llvm -target-feature +cx16 %s -o - \
 // RUN: | FileCheck %s --check-prefixes CHECK,CHECK-X64,CHECK-ARM-X64,CHECK-INTEL
 // RUN: %clang_cc1 -ffreestanding -fms-extensions -fms-compatibility -fms-compatibility-version=17.00 \
 // RUN: -triple aarch64-windows -Oz -emit-llvm %s -o - \
-// RUN: | FileCheck %s --check-prefix CHECK-ARM-X64
+// RUN: | FileCheck %s --check-prefixes CHECK-ARM-ARM64,CHECK-ARM-X64
 
 // intrin.h needs size_t, but -ffreestanding prevents us from getting it from
 // stddef.h.  Work around it with this typedef.
@@ -612,6 +612,94 @@
 }
 #endif
 
+#if defined(__arm__) || defined(__aarch64__)
+char test_InterlockedExchangeAdd8_acq(char volatile *value, char mask) {
+  return _InterlockedExchangeAdd8_acq(value, mask);
+}
+// CHECK-ARM-ARM64: define{{.*}}i8 @test_InterlockedExchangeAdd8_acq(i8*{{[a-z_ ]*}}%value, i8{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM-ARM64:   [[RESULT:%[0-9]+]] = atomicrmw add i8* %value, i8 %mask acquire
+// CHECK-ARM-ARM64:   ret i8 [[RESULT:%[0-9]+]]
+// CHECK-ARM-ARM64: }
+char test_InterlockedExchangeAdd8_rel(char volatile *value, char mask) {
+  return _InterlockedExchangeAdd8_rel(value, mask);
+}
+// CHECK-ARM-ARM64: define{{.*}}i8 @test_InterlockedExchangeAdd8_rel(i8*{{[a-z_ ]*}}%value, i8{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM-ARM64:   [[RESULT:%[0-9]+]] = atomicrmw add i8* %value, i8 %mask release
+// CHECK-ARM-ARM64:   ret i8 [[RESULT:%[0-9]+]]
+// CHECK-ARM-ARM64: }
+char test_InterlockedExchangeAdd8_nf(char volatile *value, char mask) {
+  return _InterlockedExchangeAdd8_nf(value, mask);
+}
+// CHECK-ARM-ARM64: define{{.*}}i8 @test_InterlockedExchangeAdd8_nf(i8*{{[a-z_ ]*}}%value, i8{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM-ARM64:   [[RESULT:%[0-9]+]] = atomicrmw add i8* %value, i8 %mask monotonic
+// CHECK-ARM-ARM64:   ret i8 [[RESULT:%[0-9]+]]
+// CHECK-ARM-ARM64: }
+short test_InterlockedExchangeAdd16_acq(short volatile *value, short mask) {
+  return _InterlockedExchangeAdd16_acq(value, mask);
+}
+// CHECK-ARM-ARM64: define{{.*}}i16 @test_InterlockedExchangeAdd16_acq(i16*{{[a-z_ ]*}}%value, i16{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM-ARM64:   [[RESULT:%[0-9]+]] = atomicrmw add i16* %value, i16 %mask acquire
+// CHECK-ARM-ARM64:   ret i16 [[RESULT:%[0-9]+]]
+// CHECK-ARM-ARM64: }
+short test_InterlockedExchangeAdd16_rel(short volatile *value, short mask) {
+  return _InterlockedExchangeAdd16_rel(value, mask);
+}
+// CHECK-ARM-ARM64: define{{.*}}i16 @test_InterlockedExchangeAdd16_rel(i16*{{[a-z_ ]*}}%value, i16{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM-ARM64:   [[RESULT:%[0-9]+]] = atomicrmw add i16* %value, i16 %mask release
+// CHECK-ARM-ARM64:   ret i16 [[RESULT:%[0-9]+]]
+// CHECK-ARM-ARM64: }
+short test_InterlockedExchangeAdd16_nf(short volatile *value, short mask) {
+  return _InterlockedExchangeAdd16_nf(value, mask);
+}
+// CHECK-ARM-ARM64: define{{.*}}i16 @test_InterlockedExchangeAdd16_nf(i16*{{[a-z_ ]*}}%value, i16{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM-ARM64:   [[RESULT:%[0-9]+]] = atomicrmw add i16* %value, i16 %mask monotonic
+// CHECK-ARM-ARM64:   ret i16 [[RESULT:%[0-9]+]]
+// CHECK-ARM-ARM64: }
+long test_InterlockedExchangeAdd_acq(long volatile *value, long mask) {
+  return _InterlockedExchangeAdd_acq(value, mask);
+}
+// CHECK-ARM-ARM64: define{{.*}}i32 @test_InterlockedExchangeAdd_acq(i32*{{[a-z_ ]*}}%value, i32{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM-ARM64:   [[RESULT:%[0-9]+]] = atomicrmw add i32* %value, i32 %mask acquire
+// CHECK-ARM-ARM64:   ret i32 [[RESULT:%[0-9]+]]
+// CHECK-ARM-ARM64: }
+long test_InterlockedExchangeAdd_rel(long volatile *value, long mask) {
+  return _InterlockedExchangeAdd_rel(value, mask);
+}
+// CHECK-ARM-ARM64: define{{.*}}i32 @test_InterlockedExchangeAdd_rel(i32*{{[a-z_ ]*}}%value, i32{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM-ARM64:   [[RESULT:%[0-9]+]] = atomicrmw add i32* %value, i32 %mask release
+// 

[PATCH] D53916: [ARM64] [Windows] Implement _InterlockedExchangeAdd*_* builtins.

[Eli Friedman via Phabricator via cfe-commits]

efriedma updated this revision to Diff 171852.
efriedma added a comment.

Get rid of unnecessary constants.


Repository:
  rC Clang

https://reviews.llvm.org/D53916

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

Index: test/CodeGen/ms-intrinsics.c
===
--- test/CodeGen/ms-intrinsics.c
+++ test/CodeGen/ms-intrinsics.c
@@ -3,13 +3,13 @@
 // RUN: | FileCheck %s -check-prefixes CHECK,CHECK-I386,CHECK-INTEL
 // RUN: %clang_cc1 -ffreestanding -fms-extensions -fms-compatibility -fms-compatibility-version=17.00 \
 // RUN: -triple thumbv7--windows -Oz -emit-llvm %s -o - \
-// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-ARM,CHECK-ARM-X64
+// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-ARM,CHECK-ARM-ARM64,CHECK-ARM-X64
 // RUN: %clang_cc1 -ffreestanding -fms-extensions -fms-compatibility -fms-compatibility-version=17.00 \
 // RUN: -triple x86_64--windows -Oz -emit-llvm -target-feature +cx16 %s -o - \
 // RUN: | FileCheck %s --check-prefixes CHECK,CHECK-X64,CHECK-ARM-X64,CHECK-INTEL
 // RUN: %clang_cc1 -ffreestanding -fms-extensions -fms-compatibility -fms-compatibility-version=17.00 \
 // RUN: -triple aarch64-windows -Oz -emit-llvm %s -o - \
-// RUN: | FileCheck %s --check-prefix CHECK-ARM-X64
+// RUN: | FileCheck %s --check-prefixes CHECK-ARM-ARM64,CHECK-ARM-X64
 
 // intrin.h needs size_t, but -ffreestanding prevents us from getting it from
 // stddef.h.  Work around it with this typedef.
@@ -612,6 +612,94 @@
 }
 #endif
 
+#if defined(__arm__) || defined(__aarch64__)
+char test_InterlockedExchangeAdd8_acq(char volatile *value, char mask) {
+  return _InterlockedExchangeAdd8_acq(value, mask);
+}
+// CHECK-ARM-ARM64: define{{.*}}i8 @test_InterlockedExchangeAdd8_acq(i8*{{[a-z_ ]*}}%value, i8{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM-ARM64:   [[RESULT:%[0-9]+]] = atomicrmw add i8* %value, i8 %mask acquire
+// CHECK-ARM-ARM64:   ret i8 [[RESULT:%[0-9]+]]
+// CHECK-ARM-ARM64: }
+char test_InterlockedExchangeAdd8_rel(char volatile *value, char mask) {
+  return _InterlockedExchangeAdd8_rel(value, mask);
+}
+// CHECK-ARM-ARM64: define{{.*}}i8 @test_InterlockedExchangeAdd8_rel(i8*{{[a-z_ ]*}}%value, i8{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM-ARM64:   [[RESULT:%[0-9]+]] = atomicrmw add i8* %value, i8 %mask release
+// CHECK-ARM-ARM64:   ret i8 [[RESULT:%[0-9]+]]
+// CHECK-ARM-ARM64: }
+char test_InterlockedExchangeAdd8_nf(char volatile *value, char mask) {
+  return _InterlockedExchangeAdd8_nf(value, mask);
+}
+// CHECK-ARM-ARM64: define{{.*}}i8 @test_InterlockedExchangeAdd8_nf(i8*{{[a-z_ ]*}}%value, i8{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM-ARM64:   [[RESULT:%[0-9]+]] = atomicrmw add i8* %value, i8 %mask monotonic
+// CHECK-ARM-ARM64:   ret i8 [[RESULT:%[0-9]+]]
+// CHECK-ARM-ARM64: }
+short test_InterlockedExchangeAdd16_acq(short volatile *value, short mask) {
+  return _InterlockedExchangeAdd16_acq(value, mask);
+}
+// CHECK-ARM-ARM64: define{{.*}}i16 @test_InterlockedExchangeAdd16_acq(i16*{{[a-z_ ]*}}%value, i16{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM-ARM64:   [[RESULT:%[0-9]+]] = atomicrmw add i16* %value, i16 %mask acquire
+// CHECK-ARM-ARM64:   ret i16 [[RESULT:%[0-9]+]]
+// CHECK-ARM-ARM64: }
+short test_InterlockedExchangeAdd16_rel(short volatile *value, short mask) {
+  return _InterlockedExchangeAdd16_rel(value, mask);
+}
+// CHECK-ARM-ARM64: define{{.*}}i16 @test_InterlockedExchangeAdd16_rel(i16*{{[a-z_ ]*}}%value, i16{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM-ARM64:   [[RESULT:%[0-9]+]] = atomicrmw add i16* %value, i16 %mask release
+// CHECK-ARM-ARM64:   ret i16 [[RESULT:%[0-9]+]]
+// CHECK-ARM-ARM64: }
+short test_InterlockedExchangeAdd16_nf(short volatile *value, short mask) {
+  return _InterlockedExchangeAdd16_nf(value, mask);
+}
+// CHECK-ARM-ARM64: define{{.*}}i16 @test_InterlockedExchangeAdd16_nf(i16*{{[a-z_ ]*}}%value, i16{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM-ARM64:   [[RESULT:%[0-9]+]] = atomicrmw add i16* %value, i16 %mask monotonic
+// CHECK-ARM-ARM64:   ret i16 [[RESULT:%[0-9]+]]
+// CHECK-ARM-ARM64: }
+long test_InterlockedExchangeAdd_acq(long volatile *value, long mask) {
+  return _InterlockedExchangeAdd_acq(value, mask);
+}
+// CHECK-ARM-ARM64: define{{.*}}i32 @test_InterlockedExchangeAdd_acq(i32*{{[a-z_ ]*}}%value, i32{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM-ARM64:   [[RESULT:%[0-9]+]] = atomicrmw add i32* %value, i32 %mask acquire
+// CHECK-ARM-ARM64:   ret i32 [[RESULT:%[0-9]+]]
+// CHECK-ARM-ARM64: }
+long test_InterlockedExchangeAdd_rel(long volatile *value, long mask) {
+  return _InterlockedExchangeAdd_rel(value, mask);
+}
+// CHECK-ARM-ARM64: define{{.*}}i32 @test_InterlockedExchangeAdd_rel(i32*{{[a-z_ ]*}}%value, i32{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM-ARM64:   [[RESULT:%[0-9]+]] = atomicrmw add i32* %value, i32 %mask 

[PATCH] D53916: [ARM64] [Windows] Implement _InterlockedExchangeAdd*_* builtins.

[Eli Friedman via Phabricator via cfe-commits]

efriedma created this revision.
efriedma added reviewers: rnk, mstorsjo.
Herald added subscribers: kristina, jfb, chrib, kristof.beyls, javed.absar.

These apparently need to be proper builtins to handle the Windows SDK.


Repository:
  rC Clang

https://reviews.llvm.org/D53916

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

Index: test/CodeGen/ms-intrinsics.c
===
--- test/CodeGen/ms-intrinsics.c
+++ test/CodeGen/ms-intrinsics.c
@@ -3,13 +3,13 @@
 // RUN: | FileCheck %s -check-prefixes CHECK,CHECK-I386,CHECK-INTEL
 // RUN: %clang_cc1 -ffreestanding -fms-extensions -fms-compatibility -fms-compatibility-version=17.00 \
 // RUN: -triple thumbv7--windows -Oz -emit-llvm %s -o - \
-// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-ARM,CHECK-ARM-X64
+// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-ARM,CHECK-ARM-ARM64,CHECK-ARM-X64
 // RUN: %clang_cc1 -ffreestanding -fms-extensions -fms-compatibility -fms-compatibility-version=17.00 \
 // RUN: -triple x86_64--windows -Oz -emit-llvm -target-feature +cx16 %s -o - \
 // RUN: | FileCheck %s --check-prefixes CHECK,CHECK-X64,CHECK-ARM-X64,CHECK-INTEL
 // RUN: %clang_cc1 -ffreestanding -fms-extensions -fms-compatibility -fms-compatibility-version=17.00 \
 // RUN: -triple aarch64-windows -Oz -emit-llvm %s -o - \
-// RUN: | FileCheck %s --check-prefix CHECK-ARM-X64
+// RUN: | FileCheck %s --check-prefixes CHECK-ARM-ARM64,CHECK-ARM-X64
 
 // intrin.h needs size_t, but -ffreestanding prevents us from getting it from
 // stddef.h.  Work around it with this typedef.
@@ -612,6 +612,94 @@
 }
 #endif
 
+#if defined(__arm__) || defined(__aarch64__)
+char test_InterlockedExchangeAdd8_acq(char volatile *value, char mask) {
+  return _InterlockedExchangeAdd8_acq(value, mask);
+}
+// CHECK-ARM-ARM64: define{{.*}}i8 @test_InterlockedExchangeAdd8_acq(i8*{{[a-z_ ]*}}%value, i8{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM-ARM64:   [[RESULT:%[0-9]+]] = atomicrmw add i8* %value, i8 %mask acquire
+// CHECK-ARM-ARM64:   ret i8 [[RESULT:%[0-9]+]]
+// CHECK-ARM-ARM64: }
+char test_InterlockedExchangeAdd8_rel(char volatile *value, char mask) {
+  return _InterlockedExchangeAdd8_rel(value, mask);
+}
+// CHECK-ARM-ARM64: define{{.*}}i8 @test_InterlockedExchangeAdd8_rel(i8*{{[a-z_ ]*}}%value, i8{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM-ARM64:   [[RESULT:%[0-9]+]] = atomicrmw add i8* %value, i8 %mask release
+// CHECK-ARM-ARM64:   ret i8 [[RESULT:%[0-9]+]]
+// CHECK-ARM-ARM64: }
+char test_InterlockedExchangeAdd8_nf(char volatile *value, char mask) {
+  return _InterlockedExchangeAdd8_nf(value, mask);
+}
+// CHECK-ARM-ARM64: define{{.*}}i8 @test_InterlockedExchangeAdd8_nf(i8*{{[a-z_ ]*}}%value, i8{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM-ARM64:   [[RESULT:%[0-9]+]] = atomicrmw add i8* %value, i8 %mask monotonic
+// CHECK-ARM-ARM64:   ret i8 [[RESULT:%[0-9]+]]
+// CHECK-ARM-ARM64: }
+short test_InterlockedExchangeAdd16_acq(short volatile *value, short mask) {
+  return _InterlockedExchangeAdd16_acq(value, mask);
+}
+// CHECK-ARM-ARM64: define{{.*}}i16 @test_InterlockedExchangeAdd16_acq(i16*{{[a-z_ ]*}}%value, i16{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM-ARM64:   [[RESULT:%[0-9]+]] = atomicrmw add i16* %value, i16 %mask acquire
+// CHECK-ARM-ARM64:   ret i16 [[RESULT:%[0-9]+]]
+// CHECK-ARM-ARM64: }
+short test_InterlockedExchangeAdd16_rel(short volatile *value, short mask) {
+  return _InterlockedExchangeAdd16_rel(value, mask);
+}
+// CHECK-ARM-ARM64: define{{.*}}i16 @test_InterlockedExchangeAdd16_rel(i16*{{[a-z_ ]*}}%value, i16{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM-ARM64:   [[RESULT:%[0-9]+]] = atomicrmw add i16* %value, i16 %mask release
+// CHECK-ARM-ARM64:   ret i16 [[RESULT:%[0-9]+]]
+// CHECK-ARM-ARM64: }
+short test_InterlockedExchangeAdd16_nf(short volatile *value, short mask) {
+  return _InterlockedExchangeAdd16_nf(value, mask);
+}
+// CHECK-ARM-ARM64: define{{.*}}i16 @test_InterlockedExchangeAdd16_nf(i16*{{[a-z_ ]*}}%value, i16{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM-ARM64:   [[RESULT:%[0-9]+]] = atomicrmw add i16* %value, i16 %mask monotonic
+// CHECK-ARM-ARM64:   ret i16 [[RESULT:%[0-9]+]]
+// CHECK-ARM-ARM64: }
+long test_InterlockedExchangeAdd_acq(long volatile *value, long mask) {
+  return _InterlockedExchangeAdd_acq(value, mask);
+}
+// CHECK-ARM-ARM64: define{{.*}}i32 @test_InterlockedExchangeAdd_acq(i32*{{[a-z_ ]*}}%value, i32{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM-ARM64:   [[RESULT:%[0-9]+]] = atomicrmw add i32* %value, i32 %mask acquire
+// CHECK-ARM-ARM64:   ret i32 [[RESULT:%[0-9]+]]
+// CHECK-ARM-ARM64: }
+long test_InterlockedExchangeAdd_rel(long volatile *value, long mask) {
+  return _InterlockedExchangeAdd_rel(value, mask);
+}
+// CHECK-ARM-ARM64: define{{.*}}i32 @test_InterlockedExchangeAdd_rel(i32*{{[a-z_ ]*}}%value, 

[PATCH] D53475: Create ConstantExpr class

[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, looks good. I don't mind if you address the comments below before this 
commit or in a separate commit.




Comment at: include/clang/AST/Expr.h:898
+class ConstantExpr : public FullExpr {
+  Stmt *Val;
+public:

I think it'd be cleaner and simpler to move this field into the base class, 
merging it with the corresponding member of `ExprWithCleanups`.



Comment at: include/clang/AST/StmtDataCollectors.td:17-23
+//--- Wrappers ---//
+class ConstantExpr {
+  code Code = [{
+addData(S->getType());
+  }];
+}
+

Do we need this? I think we probably don't:
a) we don't have corresponding code for `ExprWithCleanups`
b) we already add the type via the `Expr` handler
c) the type of `ConstantExpr` doesn't actually add any new information because 
it's always the same as the type of the wrapped expression



Comment at: lib/AST/StmtProfile.cpp:1001
+  VisitExpr(S);
+  VisitExpr(S->getSubExpr());
+}

void wrote:
> rsmith wrote:
> > This is unnecessary: this visitor visits the children of the node for you.
> If I don't have this then the link fails because it cannot find this.
No worries: I think this comment was against a previous version where you were 
also explicitly visiting the substatement of `S`.


Repository:
  rC Clang

https://reviews.llvm.org/D53475



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

[PATCH] D53770: Support g++ headers in include/g++

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

rsmith added a comment.

Looks good with an accompanying test. (You can probably copy whatever was added 
for the FreeScale line above.)


Repository:
  rC Clang

https://reviews.llvm.org/D53770



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

[PATCH] D52296: [Clang] - Add -gsingle-file-split-dwarf option.

[Eric Christopher via Phabricator via cfe-commits]

echristo added a comment.

In https://reviews.llvm.org/D52296#1246142, @grimar wrote:

> In https://reviews.llvm.org/D52296#1243688, @echristo wrote:
>
> > In https://reviews.llvm.org/D52296#1241928, @probinson wrote:
> >
> > > Do we generate the .dwo file directly these days?  If not, I can imagine 
> > > wanting to avoid the overhead of the objcopy hack; as long as the linker 
> > > is smart enough not to bother with the .debug_*.dwo sections this seems 
> > > like a build-time win.
> >
> >
> > We do generate them generically with no objcopy hack.
>
>
> Eric, could you elaborate for me your position, please


We don't use objcopy on linux or fuschia to generate .dwo files.

> 
> 
>> As far as the standard text here, IMO it was just there in case people 
>> didn't have an objcopy around or don't want to split it.
> 
> Yeah, we do not want to split it and I see no other way to say clang to keep 
> them in a .o files rather than introducing the new flag.
>  Am I missing something?
> 
>> I'm not sure why we would want the ability. 
>>  That said, if we do I'd rather have it as dwarf5 without split-dwarf as an 
>> option rather than a -gsingle-file-split-dwarf option.
> 
> What do you mean as "dwarf5 without split-dwarf as an option" here? Do you 
> mean to do split-dwarf by default? It is orthogonal to what this patch does.

So, what are you trying to do here is, I guess, my other question.

Are you trying to take advantage of dwarf5 features, or are you just trying to 
have everything in one file for later splitting? Or something else? If you want 
to generate split dwarf (as opposed to a lot of dwarf5 features some of which 
are split dwarf) I'll bike shed that we need to come up with a better set of 
option naming here - I'd probably repurpose/alias -gsplit-dwarf and use -f 
options for whether or not to use specific features etc.

Can you elaborate on your motivations and what you're trying to do?

Thanks!


https://reviews.llvm.org/D52296



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

LLVM buildmaster will be restarted tonight

[Galina Kistanova via cfe-commits]

 Hello everyone,

LLVM buildmaster will be updated and restarted after 6PM Pacific time today.

Thanks

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

r345676 - [Win64] Handle passing i128 by value

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Tue Oct 30 16:58:41 2018
New Revision: 345676

URL: http://llvm.org/viewvc/llvm-project?rev=345676=rev
Log:
[Win64] Handle passing i128 by value

For arguments, pass it indirectly, since the ABI doc says pretty clearly
that arguments larger than 8 bytes are passed indirectly. This makes
va_list handling easier, anyway.

When returning, GCC returns in XMM0, and we match them.

Fixes PR39492.

Added:
cfe/trunk/test/CodeGen/win64-i128.c
Modified:
cfe/trunk/lib/CodeGen/TargetInfo.cpp

Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=345676=345675=345676=diff
==
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Tue Oct 30 16:58:41 2018
@@ -3944,18 +3944,39 @@ ABIArgInfo WinX86_64ABIInfo::classify(Qu
 return ABIArgInfo::getDirect(llvm::IntegerType::get(getVMContext(), 
Width));
   }
 
-  // Bool type is always extended to the ABI, other builtin types are not
-  // extended.
-  const BuiltinType *BT = Ty->getAs();
-  if (BT && BT->getKind() == BuiltinType::Bool)
-return ABIArgInfo::getExtend(Ty);
-
-  // Mingw64 GCC uses the old 80 bit extended precision floating point unit. It
-  // passes them indirectly through memory.
-  if (IsMingw64 && BT && BT->getKind() == BuiltinType::LongDouble) {
-const llvm::fltSemantics *LDF = ().getLongDoubleFormat();
-if (LDF == ::APFloat::x87DoubleExtended())
-  return ABIArgInfo::getIndirect(Align, /*ByVal=*/false);
+  if (const BuiltinType *BT = Ty->getAs()) {
+switch (BT->getKind()) {
+case BuiltinType::Bool:
+  // Bool type is always extended to the ABI, other builtin types are not
+  // extended.
+  return ABIArgInfo::getExtend(Ty);
+
+case BuiltinType::LongDouble:
+  // Mingw64 GCC uses the old 80 bit extended precision floating point
+  // unit. It passes them indirectly through memory.
+  if (IsMingw64) {
+const llvm::fltSemantics *LDF = ().getLongDoubleFormat();
+if (LDF == ::APFloat::x87DoubleExtended())
+  return ABIArgInfo::getIndirect(Align, /*ByVal=*/false);
+break;
+  }
+
+case BuiltinType::Int128:
+case BuiltinType::UInt128:
+  // If it's a parameter type, the normal ABI rule is that arguments larger
+  // than 8 bytes are passed indirectly. GCC follows it. We follow it too,
+  // even though it isn't particularly efficient.
+  if (!IsReturnType)
+return ABIArgInfo::getIndirect(Align, /*ByVal=*/false);
+
+  // Mingw64 GCC returns i128 in XMM0. Coerce to v2i64 to handle that.
+  // Clang matches them for compatibility.
+  return ABIArgInfo::getDirect(
+  llvm::VectorType::get(llvm::Type::getInt64Ty(getVMContext()), 2));
+
+default:
+  break;
+}
   }
 
   return ABIArgInfo::getDirect();

Added: cfe/trunk/test/CodeGen/win64-i128.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/win64-i128.c?rev=345676=auto
==
--- cfe/trunk/test/CodeGen/win64-i128.c (added)
+++ cfe/trunk/test/CodeGen/win64-i128.c Tue Oct 30 16:58:41 2018
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple x86_64-windows-gnu -emit-llvm -o - %s \
+// RUN:| FileCheck %s --check-prefix=GNU64
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -emit-llvm -o - %s \
+// RUN:| FileCheck %s --check-prefix=MSC64
+
+typedef int int128_t __attribute__((mode(TI)));
+
+int128_t foo() { return 0; }
+
+// GNU64: define dso_local <2 x i64> @foo()
+// MSC64: define dso_local <2 x i64> @foo()
+
+int128_t bar(int128_t a, int128_t b) { return a * b; }
+
+// GNU64: define dso_local <2 x i64> @bar(i128*, i128*)
+// MSC64: define dso_local <2 x i64> @bar(i128*, i128*)


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

[PATCH] D53912: [Headers] [MS] Add intrin0.h

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

This sounds like it would defeat what I'm assuming is the intended purpose of 
intrin0.h, which is to reduce compile time. intrin.h is kind of enormous, and 
the compile time problems are well-documented. We should investigate what's up 
with intrin0.h and implement as many builtins as we need to support it.


Repository:
  rC Clang

https://reviews.llvm.org/D53912



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

[PATCH] D53912: [Headers] [MS] Add intrin0.h

[Azharuddin Mohammed via Phabricator via cfe-commits]

azharudd created this revision.
azharudd added reviewers: rnk, mgrang.
Herald added subscribers: jfb, mgorny.

xatomic.h header in VS 2017 includes  instead of  like
before. Adding an intrin0.h header which internally includes  when
compiling for the Windows platform.


Repository:
  rC Clang

https://reviews.llvm.org/D53912

Files:
  lib/Headers/CMakeLists.txt
  lib/Headers/intrin0.h


Index: lib/Headers/intrin0.h
===
--- /dev/null
+++ lib/Headers/intrin0.h
@@ -0,0 +1,30 @@
+/* === intrin0.h --===
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ *===---===
+ */
+
+/* xatomic.h in VS 2017 includes  instead of .
+ * If compiling for the Windows platform, we'll internally just include
+ * . */
+
+#ifdef _MSC_VER
+#include 
+#endif /* _MSC_VER */
Index: lib/Headers/CMakeLists.txt
===
--- lib/Headers/CMakeLists.txt
+++ lib/Headers/CMakeLists.txt
@@ -57,6 +57,7 @@
   ia32intrin.h
   immintrin.h
   intrin.h
+  intrin0.h
   inttypes.h
   invpcidintrin.h
   iso646.h


Index: lib/Headers/intrin0.h
===
--- /dev/null
+++ lib/Headers/intrin0.h
@@ -0,0 +1,30 @@
+/* === intrin0.h --===
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ *===---===
+ */
+
+/* xatomic.h in VS 2017 includes  instead of .
+ * If compiling for the Windows platform, we'll internally just include
+ * . */
+
+#ifdef _MSC_VER
+#include 
+#endif /* _MSC_VER */
Index: lib/Headers/CMakeLists.txt
===
--- lib/Headers/CMakeLists.txt
+++ lib/Headers/CMakeLists.txt
@@ -57,6 +57,7 @@
   ia32intrin.h
   immintrin.h
   intrin.h
+  intrin0.h
   inttypes.h
   invpcidintrin.h
   iso646.h
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:125
!Args.hasArg(options::OPT_static);
+CmdArgs.push_back("-push-state");
+CmdArgs.push_back("-as-needed");

I'd use the `--` version of all these GNU-compatible options since that's the 
GNU-compatible syntax.
(But `-static` is still single-dash.)



Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:132
   }
   CmdArgs.push_back("-lm");
 }

`-lm` belongs inside the `--as-needed` umbrella too.


Repository:
  rC Clang

https://reviews.llvm.org/D53854



___
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

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

We discussed adding a new path style that opts into a / preferred separator on 
Windows, something like path::Style::windows_forward. All of the absolute path 
handling routines would behave the same, but the preferred separator would now 
be `/`. We can make the default path style for mingw be mingw_forward.


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] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

Ping @rjmccall. Let me know if the approach in https://reviews.llvm.org/D53546 
is what you'd been envisioning, or if I'm just doing something completely 
brain-dead :)


Repository:
  rC Clang

https://reviews.llvm.org/D52674



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

r345669 - Silence unused variable warnings. NFC

[Richard Trieu via cfe-commits]

Author: rtrieu
Date: Tue Oct 30 16:01:15 2018
New Revision: 345669

URL: http://llvm.org/viewvc/llvm-project?rev=345669=rev
Log:
Silence unused variable warnings.  NFC

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

Modified: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprScalar.cpp?rev=345669=345668=345669=diff
==
--- cfe/trunk/lib/CodeGen/CGExprScalar.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp Tue Oct 30 16:01:15 2018
@@ -965,6 +965,7 @@ EmitIntegerTruncationCheckHelper(Value *
  QualType DstType, CGBuilderTy ) {
   llvm::Type *SrcTy = Src->getType();
   llvm::Type *DstTy = Dst->getType();
+  (void)DstTy; // Only used in assert()
 
   // This should be truncation of integral types.
   assert(Src != Dst);
@@ -1058,6 +1059,8 @@ EmitIntegerSignChangeCheckHelper(Value *
 
   bool SrcSigned = SrcType->isSignedIntegerOrEnumerationType();
   bool DstSigned = DstType->isSignedIntegerOrEnumerationType();
+  (void)SrcSigned; // Only used in assert()
+  (void)DstSigned; // Only used in assert()
   unsigned SrcBits = SrcTy->getScalarSizeInBits();
   unsigned DstBits = DstTy->getScalarSizeInBits();
   (void)SrcBits; // Only used in assert()


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

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:149
+  const QualType RhsType = Rhs->getType();
+  if (Lhs->isValueDependent() || Rhs->isValueDependent() ||
+  LhsType->isDependentType() || RhsType->isDependentType())

gchatelet wrote:
> JonasToth wrote:
> > you can shorten this condition with `isDependentType`
> I tried using the following but it crashes when I run clang tidy other our 
> codebase:
> ```
> if (LhsType->isDependentType() || RhsType->isDependentType())
> return false;
> ```
> 
> Maybe I misunderstood what you suggested?
You understood correct, but I was wrong. Could you please try 
`LhsType->isInstantiationDependentType()`, as this should involve all kind of 
template surprises.



Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:165
+  if (const auto *CO = llvm::dyn_cast(Rhs)) {
+const bool NarrowLhs = diagIfNarrowConstant(
+Context, CO->getLHS()->getExprLoc(), Lhs, CO->getLHS());

gchatelet wrote:
> JonasToth wrote:
> > you could remove both temporaries and return directly and then ellide the 
> > braces.
> > If you don't like that please remove the `const`
> I have to keep both variables or the first one might shortcircuit the second 
> that goes undetected.
> e.g. `unsigned char c = b ? 1 : -1;`
Indeed, no problem.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53488



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

[PATCH] D53882: [clang-tidy] Adding Zircon checker for std namespace

[Stephen Kelly via Phabricator via cfe-commits]

steveire added inline comments.



Comment at: clang-tools-extra/clang-tidy/zircon/NoStdNamespaceCheck.cpp:52
+  Finder->addMatcher(
+  valueDecl(hasType(decl(hasDeclContext(namespaceDecl(isStdNamespace())
+  .bind("stdVar"),

Recommend extracting `namespaceDecl(isStdNamespace())` to a local variable.



Comment at: clang-tools-extra/test/clang-tidy/zircon-no-std-namespace.cpp:13
+int b = func();
+std_int c;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of the 'std' namespace is not 
allowed in Zircon kernel code

Are you sure it's wise to warn on every declaration inside the `std` namespace? 
Surely warning only on declarations outside it is enough.


https://reviews.llvm.org/D53882



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

[PATCH] D53628: [analyzer] Remove custom rule for OSIterator in RetainCountChecker

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added a comment.

Actually, I'm reverting this: the rule seems fairly ubiquitous.


Repository:
  rC Clang

https://reviews.llvm.org/D53628



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

[PATCH] D53699: [ASTImporter] Fix inequality of functions with different attributes

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin added a comment.

Hi Gabor,
Thank you for the patch. The reason for this change looks clear. However, I 
don't think omitting this comparison completely is what we want here. Instead, 
we can do a dance similar to `ASTContext::mergeFunctionTypes()` where all 
attributes but NoReturn are compared. What do you think?


Repository:
  rC Clang

https://reviews.llvm.org/D53699



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

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet marked an inline comment as done.
gchatelet added inline comments.



Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:42
+   unless(hasParent(castExpr())),
+   unless(isInTemplateInstantiation()))
+  .bind("cast"),

JonasToth wrote:
> Here and in the matcher below:
> 
> `isInTemplateInstantiation` is a wide range, if you match only on 
> `isTypeDependent()` it would remove the false negatives from templates but 
> still catch clear cases that are within a template function.
> 
> With the current implementation non-type-templates would be ignored as well.
> IMHO that could be done in a follow-up to keep the scope on one thing.
Sounds good, let's fix this in a follow-up patch.



Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:92
+
+// 'One' is created with PrecisionBits plus two bytes:
+// - One to express the missing negative value of 2's complement

JonasToth wrote:
> Maybe repleace `One` with `1.0`? It sounds slightly weird with the following 
> `One`s
It's a leftover, I've reworded and changed the code as well.



Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:149
+  const QualType RhsType = Rhs->getType();
+  if (Lhs->isValueDependent() || Rhs->isValueDependent() ||
+  LhsType->isDependentType() || RhsType->isDependentType())

JonasToth wrote:
> you can shorten this condition with `isDependentType`
I tried using the following but it crashes when I run clang tidy other our 
codebase:
```
if (LhsType->isDependentType() || RhsType->isDependentType())
return false;
```

Maybe I misunderstood what you suggested?



Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:165
+  if (const auto *CO = llvm::dyn_cast(Rhs)) {
+const bool NarrowLhs = diagIfNarrowConstant(
+Context, CO->getLHS()->getExprLoc(), Lhs, CO->getLHS());

JonasToth wrote:
> you could remove both temporaries and return directly and then ellide the 
> braces.
> If you don't like that please remove the `const`
I have to keep both variables or the first one might shortcircuit the second 
that goes undetected.
e.g. `unsigned char c = b ? 1 : -1;`


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53488



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

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet updated this revision to Diff 171826.
gchatelet marked 12 inline comments as done.
gchatelet added a comment.

- Address comments


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53488

Files:
  clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
  clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
  test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp

Index: test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp
===
--- test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp
+++ test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp
@@ -1,4 +1,5 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-narrowing-conversions %t
+// RUN: %check_clang_tidy %s cppcoreguidelines-narrowing-conversions %t -- --
+// -target x86_64-unknown-linux
 
 float ceil(float);
 namespace std {
@@ -9,12 +10,12 @@
 namespace floats {
 
 struct ConvertsToFloat {
-  operator float() const { return 0.5; }
+  operator float() const { return 0.5f; }
 };
 
-float operator "" _Pa(unsigned long long);
+float operator"" _float(unsigned long long);
 
-void not_ok(double d) {
+void narrow_double_to_int_not_ok(double d) {
   int i = 0;
   i = d;
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions]
@@ -24,11 +25,11 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
   i = ConvertsToFloat();
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
-  i = 15_Pa;
+  i = 15_float;
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
 }
 
-void not_ok_binary_ops(double d) {
+void narrow_double_to_int_not_ok_binary_ops(double d) {
   int i = 0;
   i += 0.5;
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions]
@@ -52,6 +53,137 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions]
 }
 
+double operator"" _double(unsigned long long);
+
+float narrow_double_to_float_return() {
+  return 0.5;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+}
+
+void narrow_double_to_float_not_ok(double d) {
+  float f;
+  f = d;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+  f = 0.5;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+  f = 15_double;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+}
+
+void narrow_double_to_float_not_ok_binary_ops(double d) {
+  float f;
+  f += 0.5;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+  f += d;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+  // We warn on the following even though it's not dangerous because there is no reason to use a double literal here.
+  // TODO(courbet): Provide an automatic fix.
+  f += 2.0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+
+  f *= 0.5;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+  f /= 0.5;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+  f += (double)0.5f;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+}
+
+void narrow_integer_to_floating() {
+  {
+long long ll; // 64 bits
+float f = ll; // doesn't fit in 24 bits
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: narrowing conversion from 'long long' to 'float' [cppcoreguidelines-narrowing-conversions]
+double d = ll; // doesn't fit in 53 bits.
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: narrowing conversion from 'long long' to 'double' [cppcoreguidelines-narrowing-conversions]
+  }
+  {
+int i;   // 32 bits
+float f = i; // doesn't fit in 24 bits
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: narrowing conversion from 'int' to 'float' [cppcoreguidelines-narrowing-conversions]
+double d = i; // fits in 53 bits.
+  }
+  {
+short n1, n2;
+   

[PATCH] D52984: [analyzer] Checker reviewer's checklist

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Yes. Not just for checkers, not just for beginners :)


https://reviews.llvm.org/D52984



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

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

[Douglas Yung via Phabricator via cfe-commits]

dyung added a comment.

In https://reviews.llvm.org/D45050#1281178, @whisperity wrote:

> I have installed said Ubuntu in a virtual machine for testing this, but 
> unfortunately only the following Clangs are available in the package manager 
> for `Trusty`:
>
>   clang - C, C++ and Objective-C compiler (LLVM based)
>   clang-3.3 - C, C++ and Objective-C compiler (LLVM based)
>   clang-3.4 - C, C++ and Objective-C compiler (LLVM based)
>   clang-3.5 - C, C++ and Objective-C compiler (LLVM based)
>   clang-3.6 - C, C++ and Objective-C compiler (LLVM based)
>   clang-3.8 - C, C++ and Objective-C compiler (LLVM based)
>   clang-3.9 - C, C++ and Objective-C compiler (LLVM based)
>
>
> (Where `clang` is just a synonym for `clang-3.4`.) **There is no Clang 3.7 in 
> the package upstream, it seems.**


Hi, I did not initially setup the machine that hit the failure, so I cannot say 
for certain where it got clang. I did notice though that the llvm.org releases 
page does seem to include a download link for clang 3.7.1 for ubuntu 14.04 
(http://releases.llvm.org/3.7.1/clang+llvm-3.7.1-x86_64-linux-gnu-ubuntu-14.04.tar.xz).

>  --
> 
> However, **`16.04 LTS (Xenial)`** at the time of writing this comment has an 
> `clang-3.7` package, specifically this version:
> 
>   Ubuntu clang version 3.7.1-2ubuntu2 (tags/RELEASE_371/final) (based on LLVM 
> 3.7.1)
>   Target: x86_64-pc-linux-gnu
>   Thread model: posix
> 
> 
> With this I can confirm I get a huge trace and the template depth overflow 
> failure.
> 
> However, from the filename-line mappings of the preprocessed output, I can 
> see that the `type_traits` header comes from 
> `/usr/include/c++/4.8/type_traits`, which is a version **4.8** standard 
> library, but installing the `clang-3.7` package (through some transitivity in 
> `libasan` and such) depended on `gcc-`**`5`**`-base`, upgrading it from the 
> system-default `5.3.1` to `5.4.0`.
> 
> Isn't this a discrepancy, relying on an older standard library than what is 
> seemingly available on the system?

Again, I'm not sure how the toolchain was installed on the system, and if it 
was installed by simply unzipping the tarball above (or similar) then I could 
see how dependencies could go unmet. Offhand I do not even know if gcc 5+ is 
installed on the machine, but I can check if you think it is important.


https://reviews.llvm.org/D45050



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

[PATCH] D53882: [clang-tidy] Adding Zircon checker for std namespace

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tools-extra/clang-tidy/zircon/NoStdNamespaceCheck.cpp:71
+void NoStdNamespaceCheck::check(const MatchFinder::MatchResult ) {
+  if (const auto *D = Result.Nodes.getNodeAs("stdVar"))
+diag(D->getBeginLoc(),

juliehockett wrote:
> JonasToth wrote:
> > Please create a `StringRef` for the diagnostic message and reuse that.
> > 
> > Did you consider merging all `Decl` classes if you just use 
> > `getNodeAs("common_decl_name")`?
> Yes, but getting the location right posed an issue there. The `std` token is 
> not always at `D->getLocation()`.
Ok, thats unfortunate but no problem :)



Comment at: clang-tools-extra/clang-tidy/zircon/NoStdNamespaceCheck.cpp:52
+  Finder->addMatcher(
+  valueDecl(hasType(decl(hasDeclContext(namespaceDecl(isStdNamespace())
+  .bind("stdVar"),

are `FunctionDecl`, `RecordDecl` (maybe better `TagDecl`) covered already?



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:176
+
+  Warns when the `std` namespace is used, as its use is against Zircon's libc++
+  policy for the kernel.

s/its/it's/

Could `std` be considered code here? Not sure, but maybe using quotes is better?



Comment at: clang-tools-extra/test/clang-tidy/zircon-no-std-namespace.cpp:33
+int x = func();
+std::std_int y;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of the 'std' namespace is not 
allowed in Zircon kernel code

juliehockett wrote:
> JonasToth wrote:
> > What about using `int64_t` and these typedefs. Are they forbidden, too?
> Yes, which is why that's tested. Is there an additional test case I'm missing 
> regarding those?
Thanks for clarifying.

I think the usual problems (templates, macros) would need some test coverage.
Especially 

```
template 
void MyFunc();
```

these cases are interesting to check.


https://reviews.llvm.org/D53882



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

[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL345660: [clang][ubsan] Implicit Conversion Sanitizer - 
integer sign change - clang part (authored by lebedevri, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D50250?vs=171688=171814#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D50250

Files:
  cfe/trunk/docs/ReleaseNotes.rst
  cfe/trunk/docs/UndefinedBehaviorSanitizer.rst
  cfe/trunk/include/clang/Basic/Sanitizers.def
  cfe/trunk/lib/CodeGen/CGExprScalar.cpp
  cfe/trunk/test/CodeGen/catch-implicit-conversions-basics.c
  cfe/trunk/test/CodeGen/catch-implicit-integer-arithmetic-value-change-basics.c
  cfe/trunk/test/CodeGen/catch-implicit-integer-conversions-basics.c
  cfe/trunk/test/CodeGen/catch-implicit-integer-sign-changes-basics.c
  cfe/trunk/test/CodeGen/catch-implicit-integer-sign-changes-true-negatives.c
  cfe/trunk/test/CodeGen/catch-implicit-integer-sign-changes.c
  
cfe/trunk/test/CodeGen/catch-implicit-signed-integer-truncation-or-sign-change.c
  
cfe/trunk/test/CodeGenCXX/catch-implicit-integer-sign-changes-true-negatives.cpp
  cfe/trunk/test/Driver/fsanitize.c

Index: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprScalar.cpp
+++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp
@@ -306,22 +306,32 @@
 ICCK_IntegerTruncation = 0, // Legacy, was only used by clang 7.
 ICCK_UnsignedIntegerTruncation = 1,
 ICCK_SignedIntegerTruncation = 2,
+ICCK_IntegerSignChange = 3,
+ICCK_SignedIntegerTruncationOrSignChange = 4,
   };
 
   /// Emit a check that an [implicit] truncation of an integer  does not
   /// discard any bits. It is not UB, so we use the value after truncation.
   void EmitIntegerTruncationCheck(Value *Src, QualType SrcType, Value *Dst,
   QualType DstType, SourceLocation Loc);
 
+  /// Emit a check that an [implicit] conversion of an integer does not change
+  /// the sign of the value. It is not UB, so we use the value after conversion.
+  /// NOTE: Src and Dst may be the exact same value! (point to the same thing)
+  void EmitIntegerSignChangeCheck(Value *Src, QualType SrcType, Value *Dst,
+  QualType DstType, SourceLocation Loc);
+
   /// Emit a conversion from the specified type to the specified destination
   /// type, both of which are LLVM scalar types.
   struct ScalarConversionOpts {
 bool TreatBooleanAsSigned;
 bool EmitImplicitIntegerTruncationChecks;
+bool EmitImplicitIntegerSignChangeChecks;
 
 ScalarConversionOpts()
 : TreatBooleanAsSigned(false),
-  EmitImplicitIntegerTruncationChecks(false) {}
+  EmitImplicitIntegerTruncationChecks(false),
+  EmitImplicitIntegerSignChangeChecks(false) {}
   };
   Value *
   EmitScalarConversion(Value *Src, QualType SrcTy, QualType DstTy,
@@ -947,66 +957,231 @@
 SanitizerHandler::FloatCastOverflow, StaticArgs, OrigSrc);
 }
 
+// Should be called within CodeGenFunction::SanitizerScope RAII scope.
+// Returns 'i1 false' when the truncation Src -> Dst was lossy.
+static std::pair>
+EmitIntegerTruncationCheckHelper(Value *Src, QualType SrcType, Value *Dst,
+ QualType DstType, CGBuilderTy ) {
+  llvm::Type *SrcTy = Src->getType();
+  llvm::Type *DstTy = Dst->getType();
+
+  // This should be truncation of integral types.
+  assert(Src != Dst);
+  assert(SrcTy->getScalarSizeInBits() > Dst->getType()->getScalarSizeInBits());
+  assert(isa(SrcTy) && isa(DstTy) &&
+ "non-integer llvm type");
+
+  bool SrcSigned = SrcType->isSignedIntegerOrEnumerationType();
+  bool DstSigned = DstType->isSignedIntegerOrEnumerationType();
+
+  // If both (src and dst) types are unsigned, then it's an unsigned truncation.
+  // Else, it is a signed truncation.
+  ScalarExprEmitter::ImplicitConversionCheckKind Kind;
+  SanitizerMask Mask;
+  if (!SrcSigned && !DstSigned) {
+Kind = ScalarExprEmitter::ICCK_UnsignedIntegerTruncation;
+Mask = SanitizerKind::ImplicitUnsignedIntegerTruncation;
+  } else {
+Kind = ScalarExprEmitter::ICCK_SignedIntegerTruncation;
+Mask = SanitizerKind::ImplicitSignedIntegerTruncation;
+  }
+
+  llvm::Value *Check = nullptr;
+  // 1. Extend the truncated value back to the same width as the Src.
+  Check = Builder.CreateIntCast(Dst, SrcTy, DstSigned, "anyext");
+  // 2. Equality-compare with the original source value
+  Check = Builder.CreateICmpEQ(Check, Src, "truncheck");
+  // If the comparison result is 'i1 false', then the truncation was lossy.
+  return std::make_pair(Kind, std::make_pair(Check, Mask));
+}
+
 void ScalarExprEmitter::EmitIntegerTruncationCheck(Value *Src, QualType SrcType,
Value *Dst, QualType DstType,
SourceLocation Loc) {
   if 

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Interesting, did you try out `gcc-4.8` and if it is reproducable with
that one? This version of gcc is still supported and if the problem
occurs with the libstdc++ shipped there, we need to consider it.

> However, **`16.04 LTS (Xenial)`** at the time of writing this comment has an 
> `clang-3.7` package, specifically this version:
> 
>   Ubuntu clang version 3.7.1-2ubuntu2 (tags/RELEASE_371/final) (based on LLVM 
> 3.7.1)
>   Target: x86_64-pc-linux-gnu
>   Thread model: posix
> 
> With this I can confirm I get a huge trace and the template depth overflow 
> failure.
> 
> However, from the filename-line mappings of the preprocessed output, I can 
> see that the `type_traits` header comes from 
> `/usr/include/c++/4.8/type_traits`, which is a version **4.8** standard 
> library, but installing the `clang-3.7` package (through some transitivity in 
> `libasan` and such) depended on `gcc-`**`5`**`-base`, upgrading it from the 
> system-default `5.3.1` to `5.4.0`.
> 
> Isn't this a discrepancy, relying on an older standard library than what is 
> seemingly available on the system?
> 
> https://reviews.llvm.org/D45050


https://reviews.llvm.org/D45050



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

r345660 - [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

[Roman Lebedev via cfe-commits]

Author: lebedevri
Date: Tue Oct 30 14:58:56 2018
New Revision: 345660

URL: http://llvm.org/viewvc/llvm-project?rev=345660=rev
Log:
[clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

This is the second half of Implicit Integer Conversion Sanitizer.
It completes the first half, and finally makes the sanitizer
fully functional! Only the bitfield handling is missing.

Summary:
C and C++ are interesting languages. They are statically typed, but weakly.
The implicit conversions are allowed. This is nice, allows to write code
while balancing between getting drowned in everything being convertible,
and nothing being convertible. As usual, this comes with a price:

```
void consume(unsigned int val);

void test(int val) {
  consume(val);
  // The 'val' is `signed int`, but `consume()` takes `unsigned int`.
  // If val is negative, then consume() will be operating on a large
  // unsigned value, and you may or may not have a bug.

  // But yes, sometimes this is intentional.
  // Making the conversion explicit silences the sanitizer.
  consume((unsigned int)val);
}
```

Yes, there is a `-Wsign-conversion`` diagnostic group, but first, it is kinda
noisy, since it warns on everything (unlike sanitizers, warning on an
actual issues), and second, likely there are cases where it does **not** warn.

The actual detection is pretty easy. We just need to check each of the values
whether it is negative, and equality-compare the results of those comparisons.
The unsigned value is obviously non-negative. Zero is non-negative too.
https://godbolt.org/g/w93oj2

We do not have to emit the check *always*, there are obvious situations
where we can avoid emitting it, since it would **always** get optimized-out.
But i do think the tautological IR (`icmp ult %x, 0`, which is always false)
should be emitted, and the middle-end should cleanup it.

This sanitizer is in the `-fsanitize=implicit-conversion` group,
and is a logical continuation of D48958 
`-fsanitize=implicit-integer-truncation`.
As for the ordering, i'we opted to emit the check **after**
`-fsanitize=implicit-integer-truncation`. At least on these simple 16 test 
cases,
this results in 1 of the 12 emitted checks being optimized away,
as compared to 0 checks being optimized away if the order is reversed.

This is a clang part.
The compiler-rt part is D50251.

Finishes fixing [[ https://bugs.llvm.org/show_bug.cgi?id=21530 | PR21530 ]], [[ 
https://bugs.llvm.org/show_bug.cgi?id=37552 | PR37552 ]], [[ 
https://bugs.llvm.org/show_bug.cgi?id=35409 | PR35409 ]].
Finishes partially fixing [[ https://bugs.llvm.org/show_bug.cgi?id=9821 | 
PR9821 ]].
Finishes fixing https://github.com/google/sanitizers/issues/940.

Only the bitfield handling is missing.

Reviewers: vsk, rsmith, rjmccall, #sanitizers, erichkeane

Reviewed By: rsmith

Subscribers: chandlerc, filcab, cfe-commits, regehr

Tags: #sanitizers, #clang

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

Added:
cfe/trunk/test/CodeGen/catch-implicit-conversions-basics.c
  - copied, changed from r345649, 
cfe/trunk/test/CodeGen/catch-implicit-integer-conversions-basics.c

cfe/trunk/test/CodeGen/catch-implicit-integer-arithmetic-value-change-basics.c
  - copied, changed from r345649, 
cfe/trunk/test/CodeGen/catch-implicit-integer-conversions-basics.c
cfe/trunk/test/CodeGen/catch-implicit-integer-sign-changes-basics.c
cfe/trunk/test/CodeGen/catch-implicit-integer-sign-changes-true-negatives.c
cfe/trunk/test/CodeGen/catch-implicit-integer-sign-changes.c

cfe/trunk/test/CodeGen/catch-implicit-signed-integer-truncation-or-sign-change.c

cfe/trunk/test/CodeGenCXX/catch-implicit-integer-sign-changes-true-negatives.cpp
Modified:
cfe/trunk/docs/ReleaseNotes.rst
cfe/trunk/docs/UndefinedBehaviorSanitizer.rst
cfe/trunk/include/clang/Basic/Sanitizers.def
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
cfe/trunk/test/CodeGen/catch-implicit-integer-conversions-basics.c
cfe/trunk/test/Driver/fsanitize.c

Modified: cfe/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=345660=345659=345660=diff
==
--- cfe/trunk/docs/ReleaseNotes.rst (original)
+++ cfe/trunk/docs/ReleaseNotes.rst Tue Oct 30 14:58:56 2018
@@ -198,6 +198,29 @@ Static Analyzer
 Undefined Behavior Sanitizer (UBSan)
 
 
+* The Implicit Conversion Sanitizer (``-fsanitize=implicit-conversion``) group
+  was extended. One more type of issues is caught - implicit integer sign 
change.
+  (``-fsanitize=implicit-integer-sign-change``).
+  This makes the Implicit Conversion Sanitizer feature-complete,
+  with only missing piece being bitfield handling.
+  While there is a ``-Wsign-conversion`` diagnostic group that catches this 
kind
+  of issues, it is both noisy, and does not catch **all** the cases.
+
+  .. code-block:: c++
+
+  bool 

[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#1280264, @Anastasia wrote:

> In https://reviews.llvm.org/D53705#1279086, @rjmccall wrote:
>
> > 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.
>
>
> Fair point. I would like to allow regular OpenCL address space qualifiers as 
> a Clang feature at least, in case we won't be able to alter the spec. But one 
> problem is that the `private` address space qualifier keyword conflicts with 
> the `private` class access modifier. I guess we can only allow the address 
> space qualifiers with the `__` prefix. So some existing code would have to be 
> changed when ported to OpenCL C++, but it should be easier than rewriting 
> using classes.


That's actually a really easy ambiguity to handle given the current uses of the 
access-modifier keywords.  You just don't parse `private` as an access modifier 
if it's not followed by a colon.


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] D53882: [clang-tidy] Adding Zircon checker for std namespace

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 171810.
juliehockett marked 5 inline comments as done.

https://reviews.llvm.org/D53882

Files:
  clang-tools-extra/clang-tidy/zircon/CMakeLists.txt
  clang-tools-extra/clang-tidy/zircon/NoStdNamespaceCheck.cpp
  clang-tools-extra/clang-tidy/zircon/NoStdNamespaceCheck.h
  clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/zircon-no-std-namespace.rst
  clang-tools-extra/test/clang-tidy/zircon-no-std-namespace.cpp

Index: clang-tools-extra/test/clang-tidy/zircon-no-std-namespace.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/zircon-no-std-namespace.cpp
@@ -0,0 +1,50 @@
+// RUN: %check_clang_tidy %s zircon-no-std-namespace %t
+
+int func() { return 1; }
+
+namespace std {
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+
+typedef int std_int;
+int std_func() { return 1; }
+
+int a = 1;
+int b = func();
+std_int c;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+int d = std_func();
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+
+}
+
+// Warn on uses of quailfied std namespace.
+int i = 1;
+int j = func();
+std::std_int k;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+int l = std::std_func();
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+
+
+namespace foo {
+
+int w = 1;
+int x = func();
+std::std_int y;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+int z = std::std_func();
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+
+}
+
+// Warn on the alias declaration, and on users of it.
+namespace bar = std;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+bar::std_int q;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+
+// Warn on the using declaration, and on users of it.
+using namespace std;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+std_int r = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of the 'std' namespace is not allowed in Zircon kernel code
Index: clang-tools-extra/docs/clang-tidy/checks/zircon-no-std-namespace.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/zircon-no-std-namespace.rst
@@ -0,0 +1,30 @@
+.. title:: clang-tidy - zircon-no-std-namespace
+
+zircon-no-std-namespace
+===
+
+Ensures code does not use ``namespace std`` as that violates Zircon's
+kernel libc++ policy. 
+
+Any code that uses:
+
+.. code-block:: c++
+
+ namespace std {
+  ...
+ }
+
+or 
+
+.. code-block:: c++
+
+ using namespace std;
+
+or uses a declaration or function from the ``std`` namespace:
+
+.. code-block:: c++
+
+ std::string foo;
+ std::move(foo);
+
+will be prompted with a warning.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -252,4 +252,5 @@
readability-string-compare
readability-uniqueptr-delete-release
readability-uppercase-literal-suffix
+   zircon-no-std-namespace
zircon-temporary-objects
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -170,6 +170,12 @@
   ` check does not warn
   about calls inside macros anymore by default.
 
+- New :doc:`zircon-no-std-namespace
+  ` check.
+
+  Warns when the `std` namespace is used, as its use is against Zircon's libc++
+  policy for the kernel.
+
 Improvements to include-fixer
 -
 
Index: clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
+++ clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "NoStdNamespaceCheck.h"
 #include "TemporaryObjectsCheck.h"
 
 using namespace clang::ast_matchers;
@@ -22,6 +23,8 @@
 class ZirconModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories ) override {
+

[PATCH] D53882: [clang-tidy] Adding Zircon checker for std namespace

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-tidy/zircon/NoStdNamespaceCheck.cpp:71
+void NoStdNamespaceCheck::check(const MatchFinder::MatchResult ) {
+  if (const auto *D = Result.Nodes.getNodeAs("stdVar"))
+diag(D->getBeginLoc(),

JonasToth wrote:
> Please create a `StringRef` for the diagnostic message and reuse that.
> 
> Did you consider merging all `Decl` classes if you just use 
> `getNodeAs("common_decl_name")`?
Yes, but getting the location right posed an issue there. The `std` token is 
not always at `D->getLocation()`.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/zircon-no-std-namespace.rst:1
+.. title:: clang-tidy - zircon-no-std-namespace
+

JonasToth wrote:
> Could you please clarify what exactly is forbidden?
> 
> __Using__ stuff from `std` like `std::string` in normal "user" code or 
> __opening__ `std` to add stuff?
> 
> If the first thing is the case why are the variables flagged but not the 
> functions? And adding things to namespace `std` is AFAIK UB with the 
> exception of template-function specializations, so maybe that would be worth 
> a general check?
Both. Documentation updated -- the uses in particular are to be flagged.



Comment at: clang-tools-extra/test/clang-tidy/zircon-no-std-namespace.cpp:33
+int x = func();
+std::std_int y;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of the 'std' namespace is not 
allowed in Zircon kernel code

JonasToth wrote:
> What about using `int64_t` and these typedefs. Are they forbidden, too?
Yes, which is why that's tested. Is there an additional test case I'm missing 
regarding those?


https://reviews.llvm.org/D53882



___
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#1280193, @ebevhan wrote:

> In https://reviews.llvm.org/D53738#1279069, @rjmccall wrote:
>
> > > 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.
>
>
> I did figure that it might be a bit of work to adapt other parts of the code 
> to handle the new type, but I just prefer separating the concerns and being 
> explicit about what work is performed. To me, the clearest way of doing that 
> is by handling the conversions explicitly in the AST, and separately from the 
> operators themselves. Also, not being able to deal in QualTypes for the 
> common full precision type handling means that reusing the operator handling 
> code in Sema won't be easy, or even possible. It would have to be computed in 
> CreateBuiltinBinOp, since it's not possible to forward anything but QualType 
> from the CheckXXXOperands functions.
>
> If you don't think it's a good idea I'll concede, but then there's the 
> question of how to get the full precision semantics into the operator (if we 
> store it there). I guess the most straightforward path is something like:
>
> - In CreateBuiltinBinOp, do the normal Check function to get the result type
> - If the result is a fixed-point type, go into a separate code path
> - Ask a method for the common FixedPointSemantics of the given LHS and RHS
> - Build the correct BinaryOperator subclass.
>
>   I need to think about this to see if our downstream implementation can be 
> adapted to work with this design.
>
>   Compound assignments are already their own subclass, though. Unless the 
> full precision semantic information is simply baked into the regular 
> BinaryOperator, it would require two new subclasses: one for normal full 
> precision operators and one for compound ones? Wouldn't adding this require 
> new hooks and code paths in visitors, even though there's really nothing 
> different about the operator?
>
>   The type information that CompoundAssignOperator stores today is rather 
> similar to what a full precision operator would need, though it would need to 
> store a FixedPointSemantics instead.


Well, maybe the cleanest solution would be to actually fold 
`CompoundAssignOperator` back into `BinaryOperator` and just allow 
`BinaryOperator` to optionally store information about the intermediate type of 
the computation and how the operands need to be promoted.  That information 
could be elided in the common cases where it's trivial, of course.

The infinite-precision rule here is still internal to an individual operator, 
right?  The standard's not trying to say that we should evaluate `x + y < z` by 
doing a comparison as if all the operands were individually infinite-precision?


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] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Thanks, and I agree with renaming the existing option.




Comment at: docs/ClangCommandLineReference.rst:805
 
-Enable poisoning array cookies when using class member operator new\[\] in 
AddressSanitizer
+Enable poisoning array cookies when using custom operator new\[\] in 
AddressSanitizer
 

This is user documentation, so it would be good to explain here what exactly 
this does and why you might enable or disable it.  I know the surrounding 
documentation is even more barebones, but that's a problem, and it's a problem 
we won't fix by making it worse.


Repository:
  rC Clang

https://reviews.llvm.org/D52615



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

[PATCH] D50672: [ASTImporter] Change the return result of Decl import to Optional

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin abandoned this revision.
a_sidorin added a comment.

Another version has been committed (https://reviews.llvm.org/D51633).


Repository:
  rC Clang

https://reviews.llvm.org/D50672



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

[PATCH] D53697: [ASTImporter][Structural Eq] Check for isBeingDefined

[Aleksei Sidorin via Phabricator via cfe-commits]

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

Thank you for the explanation!


Repository:
  rC Clang

https://reviews.llvm.org/D53697



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

[PATCH] D53770: Support g++ headers in include/g++

[David Greene via Phabricator via cfe-commits]

greened updated this revision to Diff 171801.
greened edited the summary of this revision.
greened added a comment.

Explain that Cray packages gcc such that headers are in include/g++.


Repository:
  rC Clang

https://reviews.llvm.org/D53770

Files:
  lib/Driver/ToolChains/Linux.cpp


Index: lib/Driver/ToolChains/Linux.cpp
===
--- lib/Driver/ToolChains/Linux.cpp
+++ lib/Driver/ToolChains/Linux.cpp
@@ -924,6 +924,9 @@
   // Freescale SDK C++ headers are directly in /usr/include/c++,
   // without a subdirectory corresponding to the gcc version.
   LibDir.str() + "/../include/c++",
+  // Cray's gcc installation puts headers under "g++" without a
+  // version suffix.
+  LibDir.str() + "/../include/g++",
   };
 
   for (const auto  : LibStdCXXIncludePathCandidates) {


Index: lib/Driver/ToolChains/Linux.cpp
===
--- lib/Driver/ToolChains/Linux.cpp
+++ lib/Driver/ToolChains/Linux.cpp
@@ -924,6 +924,9 @@
   // Freescale SDK C++ headers are directly in /usr/include/c++,
   // without a subdirectory corresponding to the gcc version.
   LibDir.str() + "/../include/c++",
+  // Cray's gcc installation puts headers under "g++" without a
+  // version suffix.
+  LibDir.str() + "/../include/g++",
   };
 
   for (const auto  : LibStdCXXIncludePathCandidates) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53870: [clang-cl] Put dllexport attrs on static locals also in template instantiations (PR39496)

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm




Comment at: test/CodeGenCXX/dllimport.cpp:1010
+int bar() { T t; return t.foo(); }
+// MO1-DAG: @"?x@?{{1|2}}??foo@?$T@H@pr39496@@Q{{[A-Z]*}}HXZ@4HA" = 
available_externally dllimport global i32 0, align 4
+}

I notice that we don't emit `foo` as an available_externally definition right 
now. With your change, will we do so? Should we?


https://reviews.llvm.org/D53870



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

[PATCH] D53900: [CodeComplete] Penalize inherited ObjC properties for auto-completion

[David Goldman via Phabricator via cfe-commits]

dgoldman created this revision.
Herald added subscribers: cfe-commits, arphaman.

Similar to auto-completion for ObjC methods, inherited properties
should be penalized / direct class and category properties should
be prioritized.

Note that currently, the penalty for using a result from a base class
(CCD_InBaseClass) is equal to the penalty for using a method as a
property (CCD_MethodAsProperty).


Repository:
  rC Clang

https://reviews.llvm.org/D53900

Files:
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/objc-protocol-member-access.m
  test/Index/complete-block-properties.m
  test/Index/complete-block-property-assignment.m
  test/Index/complete-member-access.m
  test/Index/complete-properties.m

Index: test/Index/complete-properties.m
===
--- test/Index/complete-properties.m
+++ test/Index/complete-properties.m
@@ -77,11 +77,11 @@
 // CHECK-CC4-NEXT: ObjCPropertyDecl:{ResultType id}{TypedText Prop4}
 
 // RUN: c-index-test -code-completion-at=%s:29:13 %s | FileCheck -check-prefix=CHECK-CC5 %s
-// CHECK-CC5: ObjCPropertyDecl:{ResultType int}{TypedText Prop0} (35)
-// CHECK-CC5-NEXT: ObjCPropertyDecl:{ResultType int}{TypedText Prop1} (35)
-// CHECK-CC5-NEXT: ObjCPropertyDecl:{ResultType float}{TypedText Prop2} (35)
+// CHECK-CC5: ObjCPropertyDecl:{ResultType int}{TypedText Prop0} (37)
+// CHECK-CC5-NEXT: ObjCPropertyDecl:{ResultType int}{TypedText Prop1} (37)
+// CHECK-CC5-NEXT: ObjCPropertyDecl:{ResultType float}{TypedText Prop2} (37)
 // CHECK-CC5-NEXT: ObjCPropertyDecl:{ResultType id}{TypedText Prop3} (35)
-// CHECK-CC5-NEXT: ObjCPropertyDecl:{ResultType id}{TypedText Prop4} (35)
+// CHECK-CC5-NEXT: ObjCPropertyDecl:{ResultType id}{TypedText Prop4} (37)
 
 // RUN: c-index-test -code-completion-at=%s:9:11 %s | FileCheck -check-prefix=CHECK-CC6 %s
 // CHECK-CC6: ObjCInterfaceDecl:{TypedText MyClass} (50)
@@ -93,7 +93,7 @@
 // CHECK-CC7: ObjCIvarDecl:{ResultType id}{TypedText Prop2_} (7)
 
 // RUN: c-index-test -code-completion-at=%s:57:13 -fobjc-nonfragile-abi %s | FileCheck -check-prefix=CHECK-CC8 %s
-// CHECK-CC8: ObjCPropertyDecl:{ResultType int}{TypedText Prop5} (35)
+// CHECK-CC8: ObjCPropertyDecl:{ResultType int}{TypedText Prop5} (37)
 
 @interface ClassProperties
 
@@ -157,12 +157,12 @@
 // CHECK-CC9-NOT: instanceProperty
 
 // RUN: c-index-test -code-completion-at=%s:145:28 -fobjc-nonfragile-abi %s | FileCheck -check-prefix=CHECK-CC10 %s
-// CHECK-CC10: ObjCPropertyDecl:{ResultType int}{TypedText explicit} (35)
-// CHECK-CC10-NEXT: ObjCPropertyDecl:{ResultType int}{TypedText explicitInProtocol} (35)
-// CHECK-CC10-NEXT: ObjCPropertyDecl:{ResultType int}{TypedText explicitReadonly} (35)
-// CHECK-CC10-NEXT: ObjCClassMethodDecl:{ResultType int}{TypedText implicit} (37)
-// CHECK-CC10-NEXT: ObjCClassMethodDecl:{ResultType int}{TypedText implicitInCategory} (37)
-// CHECK-CC10-NEXT: ObjCClassMethodDecl:{ResultType int}{TypedText implicitReadonly} (37)
+// CHECK-CC10: ObjCPropertyDecl:{ResultType int}{TypedText explicit} (37)
+// CHECK-CC10-NEXT: ObjCPropertyDecl:{ResultType int}{TypedText explicitInProtocol} (37)
+// CHECK-CC10-NEXT: ObjCPropertyDecl:{ResultType int}{TypedText explicitReadonly} (37)
+// CHECK-CC10-NEXT: ObjCClassMethodDecl:{ResultType int}{TypedText implicit} (39)
+// CHECK-CC10-NEXT: ObjCClassMethodDecl:{ResultType int}{TypedText implicitInCategory} (39)
+// CHECK-CC10-NEXT: ObjCClassMethodDecl:{ResultType int}{TypedText implicitReadonly} (39)
 // CHECK-CC10-NEXT: ObjCPropertyDecl:{ResultType ClassProperties *}{TypedText shadowedImplicit} (35)
 // CHECK-CC10-NOT: implicitInstance
 // CHECK-CC10-NOT: noProperty
Index: test/Index/complete-member-access.m
===
--- test/Index/complete-member-access.m
+++ test/Index/complete-member-access.m
@@ -61,8 +61,8 @@
 // RUN: c-index-test -code-completion-at=%s:34:12 %s | FileCheck -check-prefix=CHECK-CC3 %s
 // CHECK-CC3: ObjCInstanceMethodDecl:{ResultType int}{TypedText myOtherPropLikeThing} (37)
 // CHECK-CC3: ObjCPropertyDecl:{ResultType int}{TypedText myProp} (35)
-// CHECK-CC3: ObjCPropertyDecl:{ResultType int}{TypedText prop1} (35)
-// CHECK-CC3: ObjCPropertyDecl:{ResultType float}{TypedText ProtoProp} (35)
+// CHECK-CC3: ObjCPropertyDecl:{ResultType int}{TypedText prop1} (37)
+// CHECK-CC3: ObjCPropertyDecl:{ResultType float}{TypedText ProtoProp} (37)
 // CHECK-CC3: Completion contexts:
 // CHECK-CC3-NEXT: Objective-C property access
 // CHECK-CC3-NEXT: Container Kind: ObjCInterfaceDecl
@@ -72,6 +72,6 @@
 // RUN: c-index-test -code-completion-at=%s:42:20 %s | FileCheck -check-prefix=CHECK-CC4 %s
 // CHECK-CC4: ObjCInstanceMethodDecl:{ResultType int}{TypedText myOtherPropLikeThing} (37)
 // CHECK-CC4-NEXT: ObjCPropertyDecl:{ResultType int}{TypedText myProp} (35)
-// CHECK-CC4-NEXT: ObjCPropertyDecl:{ResultType int}{TypedText prop1} (35)
-// CHECK-CC4-NEXT: ObjCPropertyDecl:{ResultType float}{TypedText 

[PATCH] D53850: Declares __cpu_model as dso local

[Haibo Huang via Phabricator via cfe-commits]

hhb 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));
 }

craig.topper wrote:
> 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.
By the way, what is the reason for gcc to add a separate __cpu_feature2, but 
not adding an element to __cpu_model.__cpu_features array?


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] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

[Whisperity via Phabricator via cfe-commits]

whisperity added a reviewer: dyung.
whisperity added a subscriber: dyung.
whisperity added a comment.

In https://reviews.llvm.org/D45050#1280831, @dyung wrote:

> I have attached a bzipped preprocessed file that I can confirm will show the 
> failure if built with clang++ version 3.7.1, specifically the version that 
> shipped with ubuntu 14.04.5 LTS. Here is the full version information:
>
> Ubuntu clang version 3.7.1-svn253742-1~exp1 (branches/release_37) (based on 
> LLVM 3.7.1)
>
> Target: x86_64-pc-linux-gnu
>  Thread model: posix
>
> If you build it with “clang++ -std=c++11 
> NotNullTerminatedResultCheck.preproc.cpp” you should see the failure.


I have installed said Ubuntu in a virtual machine for testing this, but 
unfortunately only the following Clangs are available in the package manager 
for `Trusty`:

  clang - C, C++ and Objective-C compiler (LLVM based)
  clang-3.3 - C, C++ and Objective-C compiler (LLVM based)
  clang-3.4 - C, C++ and Objective-C compiler (LLVM based)
  clang-3.5 - C, C++ and Objective-C compiler (LLVM based)
  clang-3.6 - C, C++ and Objective-C compiler (LLVM based)
  clang-3.8 - C, C++ and Objective-C compiler (LLVM based)
  clang-3.9 - C, C++ and Objective-C compiler (LLVM based)

(Where `clang` is just a synonym for `clang-3.4`.) **There is no Clang 3.7 in 
the package upstream, it seems.**

--

However, **`16.04 LTS (Xenial)`** at the time of writing this comment has an 
`clang-3.7` package, specifically this version:

  Ubuntu clang version 3.7.1-2ubuntu2 (tags/RELEASE_371/final) (based on LLVM 
3.7.1)
  Target: x86_64-pc-linux-gnu
  Thread model: posix

With this I can confirm I get a huge trace and the template depth overflow 
failure.

However, from the filename-line mappings of the preprocessed output, I can see 
that the `type_traits` header comes from `/usr/include/c++/4.8/type_traits`, 
which is a version **4.8** standard library, but installing the `clang-3.7` 
package (through some transitivity in `libasan` and such) depended on 
`gcc-`**`5`**`-base`, upgrading it from the system-default `5.3.1` to `5.4.0`.

Isn't this a discrepancy, relying on an older standard library than what is 
seemingly available on the system?


https://reviews.llvm.org/D45050



___
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]

a_sidorin added a comment.

Oops. Thank you Davide!


Repository:
  rC Clang

https://reviews.llvm.org/D44100



___
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

[Haibo Huang via Phabricator via cfe-commits]

hhb updated this revision to Diff 171789.

https://reviews.llvm.org/D53850

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/builtin-cpu-is.c
  test/CodeGen/builtin-cpu-supports.c


Index: test/CodeGen/builtin-cpu-supports.c
===
--- test/CodeGen/builtin-cpu-supports.c
+++ test/CodeGen/builtin-cpu-supports.c
@@ -4,6 +4,9 @@
 // global, the bit grab, and the icmp correct.
 extern void a(const char *);
 
+// CHECK: @__cpu_model = external dso_local global { i32, i32, i32, [1 x i32] }
+// CHECK: @__cpu_features2 = external dso_local global i32
+
 int main() {
   __builtin_cpu_init();
 
@@ -25,3 +28,5 @@
 
   return 0;
 }
+
+// CHECK: declare dso_local void @__cpu_indicator_init()
Index: test/CodeGen/builtin-cpu-is.c
===
--- test/CodeGen/builtin-cpu-is.c
+++ test/CodeGen/builtin-cpu-is.c
@@ -4,6 +4,8 @@
 // global, the bit grab, and the icmp correct.
 extern void a(const char *);
 
+// CHECK: @__cpu_model = external dso_local global { i32, i32, i32, [1 x i32] }
+
 void intel() {
   if (__builtin_cpu_is("intel"))
 a("intel");
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -9089,6 +9089,7 @@
 
   // Grab the global __cpu_model.
   llvm::Constant *CpuModel = CGM.CreateRuntimeVariable(STy, "__cpu_model");
+  cast(CpuModel)->setDSOLocal(true);
 
   // Calculate the index needed to access the correct field based on the
   // range. Also adjust the expected value.
@@ -9161,6 +9162,7 @@
 
 // Grab the global __cpu_model.
 llvm::Constant *CpuModel = CGM.CreateRuntimeVariable(STy, "__cpu_model");
+cast(CpuModel)->setDSOLocal(true);
 
 // Grab the first (0th) element from the field __cpu_features off of the
 // global in the struct STy.
@@ -9180,6 +9182,8 @@
   if (Features2 != 0) {
 llvm::Constant *CpuFeatures2 = CGM.CreateRuntimeVariable(Int32Ty,
  
"__cpu_features2");
+cast(CpuFeatures2)->setDSOLocal(true);
+
 Value *Features =
 Builder.CreateAlignedLoad(CpuFeatures2, CharUnits::fromQuantity(4));
 
@@ -9197,6 +9201,7 @@
   llvm::FunctionType *FTy = llvm::FunctionType::get(VoidTy,
 /*Variadic*/ false);
   llvm::Constant *Func = CGM.CreateRuntimeFunction(FTy, 
"__cpu_indicator_init");
+  cast(Func)->setDSOLocal(true);
   return Builder.CreateCall(Func);
 }
 


Index: test/CodeGen/builtin-cpu-supports.c
===
--- test/CodeGen/builtin-cpu-supports.c
+++ test/CodeGen/builtin-cpu-supports.c
@@ -4,6 +4,9 @@
 // global, the bit grab, and the icmp correct.
 extern void a(const char *);
 
+// CHECK: @__cpu_model = external dso_local global { i32, i32, i32, [1 x i32] }
+// CHECK: @__cpu_features2 = external dso_local global i32
+
 int main() {
   __builtin_cpu_init();
 
@@ -25,3 +28,5 @@
 
   return 0;
 }
+
+// CHECK: declare dso_local void @__cpu_indicator_init()
Index: test/CodeGen/builtin-cpu-is.c
===
--- test/CodeGen/builtin-cpu-is.c
+++ test/CodeGen/builtin-cpu-is.c
@@ -4,6 +4,8 @@
 // global, the bit grab, and the icmp correct.
 extern void a(const char *);
 
+// CHECK: @__cpu_model = external dso_local global { i32, i32, i32, [1 x i32] }
+
 void intel() {
   if (__builtin_cpu_is("intel"))
 a("intel");
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -9089,6 +9089,7 @@
 
   // Grab the global __cpu_model.
   llvm::Constant *CpuModel = CGM.CreateRuntimeVariable(STy, "__cpu_model");
+  cast(CpuModel)->setDSOLocal(true);
 
   // Calculate the index needed to access the correct field based on the
   // range. Also adjust the expected value.
@@ -9161,6 +9162,7 @@
 
 // Grab the global __cpu_model.
 llvm::Constant *CpuModel = CGM.CreateRuntimeVariable(STy, "__cpu_model");
+cast(CpuModel)->setDSOLocal(true);
 
 // Grab the first (0th) element from the field __cpu_features off of the
 // global in the struct STy.
@@ -9180,6 +9182,8 @@
   if (Features2 != 0) {
 llvm::Constant *CpuFeatures2 = CGM.CreateRuntimeVariable(Int32Ty,
  "__cpu_features2");
+cast(CpuFeatures2)->setDSOLocal(true);
+
 Value *Features =
 Builder.CreateAlignedLoad(CpuFeatures2, CharUnits::fromQuantity(4));
 
@@ -9197,6 +9201,7 @@
   llvm::FunctionType *FTy = llvm::FunctionType::get(VoidTy,
 /*Variadic*/ false);
   llvm::Constant *Func = CGM.CreateRuntimeFunction(FTy, "__cpu_indicator_init");
+  cast(Func)->setDSOLocal(true);
   return Builder.CreateCall(Func);
 }
 

r345649 - Changing the command line parameters sent to diff for this test.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Tue Oct 30 13:55:18 2018
New Revision: 345649

URL: http://llvm.org/viewvc/llvm-project?rev=345649=rev
Log:
Changing the command line parameters sent to diff for this test.

On some systems, -U 1 was being interpreted as -U -1. Trying -U1 to see if 
that's the universally accepted approach instead.

Modified:

cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
cfe/trunk/test/Analysis/diagnostics/sarif-diagnostics-taint-test.c

Modified: 
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif?rev=345649=345648=345649=diff
==
--- 
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
 (original)
+++ 
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
 Tue Oct 30 13:55:18 2018
@@ -7,7 +7,7 @@
   "fileLocation": {
 "uri": "file:sarif-diagnostics-taint-test.c"
   },
-  "length": 501,
+  "length": 500,
   "mimeType": "text/plain",
   "roles": [
 "resultFile"

Modified: cfe/trunk/test/Analysis/diagnostics/sarif-diagnostics-taint-test.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/sarif-diagnostics-taint-test.c?rev=345649=345648=345649=diff
==
--- cfe/trunk/test/Analysis/diagnostics/sarif-diagnostics-taint-test.c 
(original)
+++ cfe/trunk/test/Analysis/diagnostics/sarif-diagnostics-taint-test.c Tue Oct 
30 13:55:18 2018
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 
-analyzer-checker=alpha.security.taint,debug.TaintTest %s -verify 
-analyzer-output=sarif -o - | diff -U 1 -w -I 
".*file:.*sarif-diagnostics-taint-test.c" -I "clang version" -I 
"2\.0\.0\-beta\." - 
%S/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
+// RUN: %clang_analyze_cc1 
-analyzer-checker=alpha.security.taint,debug.TaintTest %s -verify 
-analyzer-output=sarif -o - | diff -U1 -w -I 
".*file:.*sarif-diagnostics-taint-test.c" -I "clang version" -I 
"2\.0\.0\-beta\." - 
%S/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
 #include "../Inputs/system-header-simulator.h"
 
 int atoi(const char *nptr);


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

[PATCH] D52949: [Diagnostics] Implement -Wsizeof-pointer-div

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Any ideas for better warning message? Except for the warning text, I see this 
patch as ready.


https://reviews.llvm.org/D52949



___
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

[Haibo Huang via Phabricator via cfe-commits]

hhb updated this revision to Diff 171787.
hhb added a comment.

Rebase. Change to SetDSOLocal.


https://reviews.llvm.org/D53850

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/builtin-cpu-is.c
  test/CodeGen/builtin-cpu-supports.c


Index: test/CodeGen/builtin-cpu-supports.c
===
--- test/CodeGen/builtin-cpu-supports.c
+++ test/CodeGen/builtin-cpu-supports.c
@@ -4,6 +4,8 @@
 // global, the bit grab, and the icmp correct.
 extern void a(const char *);
 
+// CHECK: @__cpu_model = external hidden global { i32, i32, i32, [1 x i32] }
+
 int main() {
   __builtin_cpu_init();
 
Index: test/CodeGen/builtin-cpu-is.c
===
--- test/CodeGen/builtin-cpu-is.c
+++ test/CodeGen/builtin-cpu-is.c
@@ -4,6 +4,8 @@
 // global, the bit grab, and the icmp correct.
 extern void a(const char *);
 
+// CHECK: @__cpu_model = external hidden global { i32, i32, i32, [1 x i32] }
+
 void intel() {
   if (__builtin_cpu_is("intel"))
 a("intel");
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -9016,6 +9016,8 @@
 
   // Grab the global __cpu_model.
   llvm::Constant *CpuModel = CGM.CreateRuntimeVariable(STy, "__cpu_model");
+  cast(CpuModel)->setVisibility(
+  llvm::GlobalValue::HiddenVisibility);
 
   // Calculate the index needed to access the correct field based on the
   // range. Also adjust the expected value.
@@ -9082,6 +9084,8 @@
 
   // Grab the global __cpu_model.
   llvm::Constant *CpuModel = CGM.CreateRuntimeVariable(STy, "__cpu_model");
+  cast(CpuModel)->setVisibility(
+  llvm::GlobalValue::HiddenVisibility);
 
   // Grab the first (0th) element from the field __cpu_features off of the
   // global in the struct STy.


Index: test/CodeGen/builtin-cpu-supports.c
===
--- test/CodeGen/builtin-cpu-supports.c
+++ test/CodeGen/builtin-cpu-supports.c
@@ -4,6 +4,8 @@
 // global, the bit grab, and the icmp correct.
 extern void a(const char *);
 
+// CHECK: @__cpu_model = external hidden global { i32, i32, i32, [1 x i32] }
+
 int main() {
   __builtin_cpu_init();
 
Index: test/CodeGen/builtin-cpu-is.c
===
--- test/CodeGen/builtin-cpu-is.c
+++ test/CodeGen/builtin-cpu-is.c
@@ -4,6 +4,8 @@
 // global, the bit grab, and the icmp correct.
 extern void a(const char *);
 
+// CHECK: @__cpu_model = external hidden global { i32, i32, i32, [1 x i32] }
+
 void intel() {
   if (__builtin_cpu_is("intel"))
 a("intel");
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -9016,6 +9016,8 @@
 
   // Grab the global __cpu_model.
   llvm::Constant *CpuModel = CGM.CreateRuntimeVariable(STy, "__cpu_model");
+  cast(CpuModel)->setVisibility(
+  llvm::GlobalValue::HiddenVisibility);
 
   // Calculate the index needed to access the correct field based on the
   // range. Also adjust the expected value.
@@ -9082,6 +9084,8 @@
 
   // Grab the global __cpu_model.
   llvm::Constant *CpuModel = CGM.CreateRuntimeVariable(STy, "__cpu_model");
+  cast(CpuModel)->setVisibility(
+  llvm::GlobalValue::HiddenVisibility);
 
   // Grab the first (0th) element from the field __cpu_features off of the
   // global in the struct STy.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53547: NFC: Remove the ObjC1/ObjC2 distinction from clang (and related projects)

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp:23
   // this check should only be applied to ObjC sources.
-  if (!getLangOpts().ObjC1 && !getLangOpts().ObjC2) {
+  if (!getLangOpts().ObjC) {
 return;

theraven wrote:
> LLVM style wants to get rid of the braces here (and the next few instances of 
> this).  Since you're touching this code, would you mind fixing that as well?
Sure, I did this in the commit. Thanks for reviewing!



Comment at: clang/lib/Basic/IdentifierTable.cpp:166-167
   // in non-arc mode.
-  if (LangOpts.ObjC2 && (Flags & KEYARC)) return KS_Enabled;
-  if (LangOpts.ObjC2 && (Flags & KEYOBJC2)) return KS_Enabled;
+  if (LangOpts.ObjC && (Flags & KEYARC)) return KS_Enabled;
+  if (LangOpts.ObjC && (Flags & KEYOBJC)) return KS_Enabled;
   if (LangOpts.ConceptsTS && (Flags & KEYCONCEPTS)) return KS_Enabled;

erik.pilkington wrote:
> rsmith wrote:
> > Would it make sense to fold `KEYOBJC` and `KEYARC` together?
> Yep, good point. Looks like we used to only enable these keywords in 
> -fobjc-arc mode, but now that we're doing it for objective-c there isn't any 
> distinction to be made here. I'll commit this in a follow-up.
Done in r345646.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53547



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

r345646 - NFC: Merge KEYOBJC and KEYARC

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Tue Oct 30 13:51:28 2018
New Revision: 345646

URL: http://llvm.org/viewvc/llvm-project?rev=345646=rev
Log:
NFC: Merge KEYOBJC and KEYARC

We used to only define ARC keywords in -fobjc-arc mode, but now that we define
them in ObjC mode, there isn't any reason to keep them seperate.

Modified:
cfe/trunk/include/clang/Basic/TokenKinds.def
cfe/trunk/lib/Basic/IdentifierTable.cpp

Modified: cfe/trunk/include/clang/Basic/TokenKinds.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/TokenKinds.def?rev=345646=345645=345646=diff
==
--- cfe/trunk/include/clang/Basic/TokenKinds.def (original)
+++ cfe/trunk/include/clang/Basic/TokenKinds.def Tue Oct 30 13:51:28 2018
@@ -576,10 +576,10 @@ ALIAS("__fp16", half, KE
 KEYWORD(half, HALFSUPPORT)
 
 // Objective-C ARC keywords.
-KEYWORD(__bridge , KEYARC)
-KEYWORD(__bridge_transfer, KEYARC)
-KEYWORD(__bridge_retained, KEYARC)
-KEYWORD(__bridge_retain  , KEYARC)
+KEYWORD(__bridge , KEYOBJC)
+KEYWORD(__bridge_transfer, KEYOBJC)
+KEYWORD(__bridge_retained, KEYOBJC)
+KEYWORD(__bridge_retain  , KEYOBJC)
 
 // Objective-C keywords.
 KEYWORD(__covariant  , KEYOBJC)

Modified: cfe/trunk/lib/Basic/IdentifierTable.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/IdentifierTable.cpp?rev=345646=345645=345646=diff
==
--- cfe/trunk/lib/Basic/IdentifierTable.cpp (original)
+++ cfe/trunk/lib/Basic/IdentifierTable.cpp Tue Oct 30 13:51:28 2018
@@ -99,30 +99,29 @@ IdentifierTable::IdentifierTable(const L
 namespace {
 
   enum {
-KEYC99 = 0x1,
-KEYCXX = 0x2,
-KEYCXX11 = 0x4,
-KEYGNU = 0x8,
-KEYMS = 0x10,
-BOOLSUPPORT = 0x20,
-KEYALTIVEC = 0x40,
-KEYNOCXX = 0x80,
-KEYBORLAND = 0x100,
-KEYOPENCLC = 0x200,
-KEYC11 = 0x400,
-KEYARC = 0x800,
-KEYNOMS18 = 0x01000,
-KEYNOOPENCL = 0x02000,
-WCHARSUPPORT = 0x04000,
-HALFSUPPORT = 0x08000,
-CHAR8SUPPORT = 0x1,
-KEYCONCEPTS = 0x2,
-KEYOBJC = 0x4,
-KEYZVECTOR  = 0x8,
-KEYCOROUTINES = 0x10,
-KEYMODULES = 0x20,
-KEYCXX2A = 0x40,
-KEYOPENCLCXX = 0x80,
+KEYC99= 0x1,
+KEYCXX= 0x2,
+KEYCXX11  = 0x4,
+KEYGNU= 0x8,
+KEYMS = 0x10,
+BOOLSUPPORT   = 0x20,
+KEYALTIVEC= 0x40,
+KEYNOCXX  = 0x80,
+KEYBORLAND= 0x100,
+KEYOPENCLC= 0x200,
+KEYC11= 0x400,
+KEYNOMS18 = 0x800,
+KEYNOOPENCL   = 0x1000,
+WCHARSUPPORT  = 0x2000,
+HALFSUPPORT   = 0x4000,
+CHAR8SUPPORT  = 0x8000,
+KEYCONCEPTS   = 0x1,
+KEYOBJC   = 0x2,
+KEYZVECTOR= 0x4,
+KEYCOROUTINES = 0x8,
+KEYMODULES= 0x10,
+KEYCXX2A  = 0x20,
+KEYOPENCLCXX  = 0x40,
 KEYALLCXX = KEYCXX | KEYCXX11 | KEYCXX2A,
 KEYALL = (0xff & ~KEYNOMS18 &
   ~KEYNOOPENCL) // KEYNOMS18 and KEYNOOPENCL are used to exclude.
@@ -163,7 +162,6 @@ static KeywordStatus getKeywordStatus(co
   if (LangOpts.C11 && (Flags & KEYC11)) return KS_Enabled;
   // We treat bridge casts as objective-C keywords so we can warn on them
   // in non-arc mode.
-  if (LangOpts.ObjC && (Flags & KEYARC)) return KS_Enabled;
   if (LangOpts.ObjC && (Flags & KEYOBJC)) return KS_Enabled;
   if (LangOpts.ConceptsTS && (Flags & KEYCONCEPTS)) return KS_Enabled;
   if (LangOpts.CoroutinesTS && (Flags & KEYCOROUTINES)) return KS_Enabled;


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

[PATCH] D53025: [clang-tidy] implement new check for const return types.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Am 30.10.18 um 21:47 schrieb Aaron Ballman via Phabricator:

> aaron.ballman added a comment.
> 
> I think this is getting really close! One question: would it make more sense 
> to name this `readability-const-type-return` or 
> `readability-const-return-type` instead? It's not that the functions return a 
> const *value* that's the issue, it's that the declared return type is 
> top-level const. I think removing "value" and using "type" instead would be 
> an improvement (and similarly, rename the files and the check).

There is a `rename-check.py` script in the repository. Just to prevent a
lot of manual work ;)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53025



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

r345643 - Revert "[ASTImporter] Reorder fields after structure import is finished"

[Davide Italiano via cfe-commits]

Author: davide
Date: Tue Oct 30 13:46:29 2018
New Revision: 345643

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

This reverts commit r345545 because it breaks some lldb tests.

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=345643=345642=345643=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Tue Oct 30 13:46:29 2018
@@ -1658,53 +1658,13 @@ ASTNodeImporter::ImportDeclContext(DeclC
 auto ToDCOrErr = Importer.ImportContext(FromDC);
 return ToDCOrErr.takeError();
   }
-
-  const auto *FromRD = dyn_cast(FromDC);
+  llvm::SmallVector ImportedDecls;
   for (auto *From : FromDC->decls()) {
 ExpectedDecl ImportedOrErr = import(From);
-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();
+if (!ImportedOrErr)
   // Ignore the error, continue with next Decl.
   // FIXME: Handle this case somehow better.
-  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);
-}
+  consumeError(ImportedOrErr.takeError());
   }
 
   return Error::success();

Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=345643=345642=345643=diff
==
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Tue Oct 30 13:46:29 2018
@@ -1457,7 +1457,7 @@ TEST_P(ASTImporterTestBase, CXXRecordDec
 }
 
 TEST_P(ASTImporterTestBase,
-   CXXRecordDeclFieldOrderShouldNotDependOnImportOrder) {
+   DISABLED_CXXRecordDeclFieldOrderShouldNotDependOnImportOrder) {
   Decl *From, *To;
   std::tie(From, To) = getImportedDecl(
   // The original recursive algorithm of ASTImporter first imports 'c' then
@@ -3767,16 +3767,5 @@ 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] D44100: [ASTImporter] Reorder fields after structure import is finished

[Davide Italiano via Phabricator via cfe-commits]

davide added subscribers: shafik, davide.
davide added a comment.

This patch broke lldb, at least on MacOS.
You can reproduce the issue building lldb, applying your patch and then running

  $ ./lldb-dotest -p TestCModules

I'm going to revert this to unblock folks, but don't hesitate to ping me in 
case you need any help reproducing the issue.
cc: @shafik


Repository:
  rC Clang

https://reviews.llvm.org/D44100



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

[PATCH] D53025: [clang-tidy] implement new check for const return types.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I think this is getting really close! One question: would it make more sense to 
name this `readability-const-type-return` or `readability-const-return-type` 
instead? It's not that the functions return a const *value* that's the issue, 
it's that the declared return type is top-level const. I think removing "value" 
and using "type" instead would be an improvement (and similarly, rename the 
files and the check).




Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:111
+DiagnosticBuilder Diagnostic =
+diag(Def->getInnerLocStart(),
+ "return type %0 is 'const'-qualified at the top level, which may "

I think you want `getBeginLoc()` here instead.



Comment at: docs/clang-tidy/checks/readability-const-value-return.rst:4
+readability-const-value-return
+===
+

Underlining here is incorrect.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53025



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

[PATCH] D53025: [clang-tidy] implement new check for const return types.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:107
+diag(Def->getInnerLocStart(), "return type %0 is 'const'-qualified "
+  "hindering compiler optimizations")
+<< Def->getReturnType();

ymandel wrote:
> aaron.ballman wrote:
> > ymandel wrote:
> > > aaron.ballman wrote:
> > > > ymandel wrote:
> > > > > aaron.ballman wrote:
> > > > > > ymandel wrote:
> > > > > > > aaron.ballman wrote:
> > > > > > > > ymandel wrote:
> > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > It seems strange to me that this is in the readability 
> > > > > > > > > > module but the diagnostic here is about compiler 
> > > > > > > > > > optimizations. Should this be in the performance module 
> > > > > > > > > > instead? Or should this diagnostic be reworded about the 
> > > > > > > > > > readability concerns?
> > > > > > > > > Good point. The readability angle is that the const clutters 
> > > > > > > > > the code to no benefit.  That is, even if it was performance 
> > > > > > > > > neutral, I'd still want to discourage this practice.  But, it 
> > > > > > > > > does seem like the primary impact is performance. 
> > > > > > > > > 
> > > > > > > > > I'm fine either way -- moving it to performance or 
> > > > > > > > > emphasizing the clutter of the unhelpful "const".  I'm 
> > > > > > > > > inclined to moving it, but don't have good sense of how 
> > > > > > > > > strict these hierarchies are. What do you think?
> > > > > > > > I'm not sold that `const`-qualified return types always 
> > > > > > > > pessimize optimizations. However, I'm also not sold that it's 
> > > > > > > > *always* a bad idea to have a top-level cv-qualifier on a 
> > > > > > > > return type either (for instance, this is one way to prevent 
> > > > > > > > callers from modifying a temp returned by a function). How 
> > > > > > > > about leaving this in readability and changing the diagnostic 
> > > > > > > > into: "top-level 'const' qualifier on a return type may reduce 
> > > > > > > > code readability with limited benefit" or something equally 
> > > > > > > > weasely?
> > > > > > > I talked this over with other google folks and seems like the 
> > > > > > > consensus is:
> > > > > > > 
> > > > > > > 1.  The readability benefits may be controversial.  Some folks 
> > > > > > > might not view `const` as clutter and there are some corner cases 
> > > > > > > where the qualifier may prevent something harmful.
> > > > > > > 2.  The performance implication is real, if not guaranteed to be 
> > > > > > > a problem.
> > > > > > > 
> > > > > > > Based on this, seems best to move it to performance, but water 
> > > > > > > down the performance claims.  That keeps the focus to an issue we 
> > > > > > > can assume nearly everyone agrees on.
> > > > > > I'm not convinced the performance implications are real compared to 
> > > > > > how the check is currently implemented. I know there are 
> > > > > > performance concerns when you return a const value of class type 
> > > > > > because it pessimizes the ability to use move constructors, but 
> > > > > > that's a very specific performance concern that I don't believe is 
> > > > > > present in general. For instance, I don't know of any performance 
> > > > > > concerns regarding `const int f();` as a declaration. I'd be fine 
> > > > > > moving this to the performance module, but I think the check would 
> > > > > > need to be tightened to only focus on actual performance concerns.
> > > > > > 
> > > > > > FWIW, I do agree that the readability benefits may be 
> > > > > > controversial, but I kind of thought that was the point to the 
> > > > > > check and as such, it's a very reasonable check. Almost everything 
> > > > > > in readability is subjective to some degree and our metric has 
> > > > > > always been "if you agree with a style check, don't enable it".
> > > > > > 
> > > > > > It's up to you how you want to proceed, but I see two ways forward: 
> > > > > > 1) keep this as a readability check that broadly finds top-level 
> > > > > > const qualifiers and removes them, 2) move this to the performance 
> > > > > > module and rework the check to focus on only the scenarios that 
> > > > > > have concrete performance impact.
> > > > > > It's up to you how you want to proceed, but I see two ways forward: 
> > > > > > 1) keep this as a readability check that broadly finds top-level 
> > > > > > const qualifiers and removes them, 2) move this to the performance 
> > > > > > module and rework the check to focus on only the scenarios that 
> > > > > > have concrete performance impact.
> > > > > 
> > > > > Aaron, thank you for the detailed response. I'm inclined to agree 
> > > > > with you that this belongs in readabilty and I spoke with sbenza who 
> > > > > feels the same.  The high-level point is that the `const` is noise in 
> > > > > most cases.
> > > > > 
> > > > > You 

[PATCH] D53882: [clang-tidy] Adding Zircon checker for std namespace

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tools-extra/clang-tidy/zircon/NoStdNamespaceCheck.cpp:29
+AST_MATCHER(NamespaceAliasDecl, isAliasedToStd) {
+  if (const auto *AN = Node.getAliasedNamespace()) {
+// If this aliases to an actual namespace, check if its std. If it doesn't,

please do not use `auto` here as the type is not clear, same below (line 32 
right now)



Comment at: clang-tools-extra/clang-tidy/zircon/NoStdNamespaceCheck.cpp:31
+// If this aliases to an actual namespace, check if its std. If it doesn't,
+// it aliases to another alias and thus shouldn't be flagged.
+if (const auto *N = dyn_cast(AN))

The second sentence sounds a bit weird. I think you can even ellide it 
completely, as the first sentence does clarify quite well.



Comment at: clang-tools-extra/clang-tidy/zircon/NoStdNamespaceCheck.cpp:71
+void NoStdNamespaceCheck::check(const MatchFinder::MatchResult ) {
+  if (const auto *D = Result.Nodes.getNodeAs("stdVar"))
+diag(D->getBeginLoc(),

Please create a `StringRef` for the diagnostic message and reuse that.

Did you consider merging all `Decl` classes if you just use 
`getNodeAs("common_decl_name")`?



Comment at: clang-tools-extra/clang-tidy/zircon/NoStdNamespaceCheck.h:1
+//===--- NoStdNamespace.h - clang-tidy---*- C++ 
-*-===//
+//

There was a bug in the template, please add a blank after `clang-tidy` but keep 
the total length.



Comment at: clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp:28
 "zircon-temporary-objects");
+CheckFactories.registerCheck(
+"zircon-no-std-namespace");

please keep lexigraphical ordering.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/zircon-no-std-namespace.rst:1
+.. title:: clang-tidy - zircon-no-std-namespace
+

Could you please clarify what exactly is forbidden?

__Using__ stuff from `std` like `std::string` in normal "user" code or 
__opening__ `std` to add stuff?

If the first thing is the case why are the variables flagged but not the 
functions? And adding things to namespace `std` is AFAIK UB with the exception 
of template-function specializations, so maybe that would be worth a general 
check?



Comment at: clang-tools-extra/test/clang-tidy/zircon-no-std-namespace.cpp:33
+int x = func();
+std::std_int y;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of the 'std' namespace is not 
allowed in Zircon kernel code

What about using `int64_t` and these typedefs. Are they forbidden, too?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53882



___
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#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:
> > >
> > > > 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.


Mailed https://reviews.llvm.org/D53890 for this part.


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] D53891: [LTO] Pass down LTOUnit codegen flag to bitcode writer

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson created this revision.
tejohnson added a reviewer: pcc.
Herald added subscribers: cfe-commits, dexonsmith, steven_wu, eraman, 
inglorion, mehdi_amini.

Clang side patch for recording the LTOUnit flag in the index.

Depends on https://reviews.llvm.org/D53890.


Repository:
  rC Clang

https://reviews.llvm.org/D53891

Files:
  lib/CodeGen/BackendUtil.cpp
  test/CodeGenCXX/no-lto-unit.cpp
  test/CodeGenCXX/type-metadata-thinlto.cpp


Index: test/CodeGenCXX/type-metadata-thinlto.cpp
===
--- test/CodeGenCXX/type-metadata-thinlto.cpp
+++ test/CodeGenCXX/type-metadata-thinlto.cpp
@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 -flto=thin -flto-unit -triple x86_64-unknown-linux 
-fvisibility hidden -emit-llvm-bc -o %t %s
 // RUN: llvm-modextract -o - -n 1 %t | llvm-dis | FileCheck %s
+// RUN: llvm-modextract -b -o - -n 1 %t | llvm-bcanalyzer -dump | FileCheck %s 
--check-prefix=LTOUNIT
+// LTOUNIT: 
 
 // CHECK: @_ZTV1A = linkonce_odr
 class A {
Index: test/CodeGenCXX/no-lto-unit.cpp
===
--- test/CodeGenCXX/no-lto-unit.cpp
+++ test/CodeGenCXX/no-lto-unit.cpp
@@ -2,6 +2,8 @@
 // RUN: llvm-dis -o - %t | FileCheck %s
 // RUN: %clang_cc1 -flto=thin -flto-unit -fno-lto-unit -triple 
x86_64-unknown-linux -fvisibility hidden -emit-llvm-bc -o %t %s
 // RUN: llvm-dis -o - %t | FileCheck %s
+// RUN: llvm-bcanalyzer -dump %t | FileCheck %s --check-prefix=NOLTOUNIT
+// NOLTOUNIT: 
 
 // CHECK-NOT: !type
 class A {
Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -808,7 +808,7 @@
   return;
   }
   PerModulePasses.add(createWriteThinLTOBitcodePass(
-  *OS, ThinLinkOS ? >os() : nullptr));
+  *OS, ThinLinkOS ? >os() : nullptr, CodeGenOpts.LTOUnit));
 } else {
   // Emit a module summary by default for Regular LTO except for ld64
   // targets
@@ -822,7 +822,8 @@
 
   PerModulePasses.add(
   createBitcodeWriterPass(*OS, CodeGenOpts.EmitLLVMUseLists,
-  EmitLTOSummary));
+  EmitLTOSummary, /*EmitModuleHash=*/false,
+  CodeGenOpts.LTOUnit));
 }
 break;
 
@@ -1042,8 +1043,8 @@
 if (!ThinLinkOS)
   return;
   }
-  MPM.addPass(ThinLTOBitcodeWriterPass(*OS, ThinLinkOS ? >os()
-   : nullptr));
+  MPM.addPass(ThinLTOBitcodeWriterPass(
+  *OS, ThinLinkOS ? >os() : nullptr, CodeGenOpts.LTOUnit));
 } else {
   // Emit a module summary by default for Regular LTO except for ld64
   // targets
@@ -1056,7 +1057,8 @@
 TheModule->addModuleFlag(Module::Error, "ThinLTO", uint32_t(0));
 
   MPM.addPass(BitcodeWriterPass(*OS, CodeGenOpts.EmitLLVMUseLists,
-EmitLTOSummary));
+EmitLTOSummary, /*EmitModuleHash=*/false,
+CodeGenOpts.LTOUnit));
 }
 break;
 


Index: test/CodeGenCXX/type-metadata-thinlto.cpp
===
--- test/CodeGenCXX/type-metadata-thinlto.cpp
+++ test/CodeGenCXX/type-metadata-thinlto.cpp
@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 -flto=thin -flto-unit -triple x86_64-unknown-linux -fvisibility hidden -emit-llvm-bc -o %t %s
 // RUN: llvm-modextract -o - -n 1 %t | llvm-dis | FileCheck %s
+// RUN: llvm-modextract -b -o - -n 1 %t | llvm-bcanalyzer -dump | FileCheck %s --check-prefix=LTOUNIT
+// LTOUNIT: 
 
 // CHECK: @_ZTV1A = linkonce_odr
 class A {
Index: test/CodeGenCXX/no-lto-unit.cpp
===
--- test/CodeGenCXX/no-lto-unit.cpp
+++ test/CodeGenCXX/no-lto-unit.cpp
@@ -2,6 +2,8 @@
 // RUN: llvm-dis -o - %t | FileCheck %s
 // RUN: %clang_cc1 -flto=thin -flto-unit -fno-lto-unit -triple x86_64-unknown-linux -fvisibility hidden -emit-llvm-bc -o %t %s
 // RUN: llvm-dis -o - %t | FileCheck %s
+// RUN: llvm-bcanalyzer -dump %t | FileCheck %s --check-prefix=NOLTOUNIT
+// NOLTOUNIT: 
 
 // CHECK-NOT: !type
 class A {
Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -808,7 +808,7 @@
   return;
   }
   PerModulePasses.add(createWriteThinLTOBitcodePass(
-  *OS, ThinLinkOS ? >os() : nullptr));
+  *OS, ThinLinkOS ? >os() : nullptr, CodeGenOpts.LTOUnit));
 } else {
   // Emit a module summary by default for Regular LTO except for ld64
   // targets
@@ -822,7 +822,8 @@
 
   PerModulePasses.add(
   createBitcodeWriterPass(*OS, CodeGenOpts.EmitLLVMUseLists,
-  EmitLTOSummary));
+  

[PATCH] D53547: NFC: Remove the ObjC1/ObjC2 distinction from clang (and related projects)

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE345637: NFC: Remove the ObjC1/ObjC2 distinction from clang 
(and related projects) (authored by epilk, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D53547?vs=170703=171778#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53547

Files:
  clang-tidy/google/AvoidCStyleCastsCheck.cpp
  clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp
  clang-tidy/google/GlobalVariableDeclarationCheck.cpp
  clang-tidy/objc/AvoidNSErrorInitCheck.cpp
  clang-tidy/objc/ForbiddenSubclassingCheck.cpp
  clang-tidy/objc/PropertyDeclarationCheck.cpp


Index: clang-tidy/google/AvoidCStyleCastsCheck.cpp
===
--- clang-tidy/google/AvoidCStyleCastsCheck.cpp
+++ clang-tidy/google/AvoidCStyleCastsCheck.cpp
@@ -101,7 +101,7 @@
 
   // The rest of this check is only relevant to C++.
   // We also disable it for Objective-C++.
-  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC1 || getLangOpts().ObjC2)
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC)
 return;
   // Ignore code inside extern "C" {} blocks.
   if (!match(expr(hasAncestor(linkageSpecDecl())), *CastExpr, *Result.Context)
Index: clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -55,9 +55,9 @@
 
 void GlobalVariableDeclarationCheck::registerMatchers(MatchFinder *Finder) {
   // The relevant Style Guide rule only applies to Objective-C.
-  if (!getLangOpts().ObjC1 && !getLangOpts().ObjC2) {
+  if (!getLangOpts().ObjC)
 return;
-  }
+
   // need to add two matchers since we need to bind different ids to 
distinguish
   // constants and variables. Since bind() can only be called on node matchers,
   // we cannot make it in one matcher.
Index: clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp
===
--- clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp
+++ clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp
@@ -20,9 +20,8 @@
 
 void AvoidThrowingObjCExceptionCheck::registerMatchers(MatchFinder *Finder) {
   // this check should only be applied to ObjC sources.
-  if (!getLangOpts().ObjC1 && !getLangOpts().ObjC2) {
+  if (!getLangOpts().ObjC)
 return;
-  }
 
   Finder->addMatcher(objcThrowStmt().bind("throwStmt"), this);
   Finder->addMatcher(
Index: clang-tidy/objc/AvoidNSErrorInitCheck.cpp
===
--- clang-tidy/objc/AvoidNSErrorInitCheck.cpp
+++ clang-tidy/objc/AvoidNSErrorInitCheck.cpp
@@ -19,9 +19,9 @@
 
 void AvoidNSErrorInitCheck::registerMatchers(MatchFinder *Finder) {
   // this check should only be applied to ObjC sources.
-  if (!getLangOpts().ObjC1 && !getLangOpts().ObjC2) {
+  if (!getLangOpts().ObjC)
 return;
-  }
+
   Finder->addMatcher(objcMessageExpr(hasSelector("init"),
  hasReceiverType(asString("NSError *")))
  .bind("nserrorInit"),
Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -201,9 +201,9 @@
 
 void PropertyDeclarationCheck::registerMatchers(MatchFinder *Finder) {
   // this check should only be applied to ObjC sources.
-  if (!getLangOpts().ObjC1 && !getLangOpts().ObjC2) {
+  if (!getLangOpts().ObjC)
 return;
-  }
+
   if (IncludeDefaultAcronyms) {
 EscapedAcronyms.reserve(llvm::array_lengthof(DefaultSpecialAcronyms) +
 SpecialAcronyms.size());
Index: clang-tidy/objc/ForbiddenSubclassingCheck.cpp
===
--- clang-tidy/objc/ForbiddenSubclassingCheck.cpp
+++ clang-tidy/objc/ForbiddenSubclassingCheck.cpp
@@ -78,9 +78,9 @@
 
 void ForbiddenSubclassingCheck::registerMatchers(MatchFinder *Finder) {
   // this check should only be applied to ObjC sources.
-  if (!getLangOpts().ObjC1 && !getLangOpts().ObjC2) {
+  if (!getLangOpts().ObjC)
 return;
-  }
+
   Finder->addMatcher(
   objcInterfaceDecl(
   isSubclassOf(


Index: clang-tidy/google/AvoidCStyleCastsCheck.cpp
===
--- clang-tidy/google/AvoidCStyleCastsCheck.cpp
+++ clang-tidy/google/AvoidCStyleCastsCheck.cpp
@@ -101,7 +101,7 @@
 
   // The rest of this check is only relevant to C++.
   // We also disable it for Objective-C++.
-  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC1 || getLangOpts().ObjC2)
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC)
 return;
   // Ignore code inside extern "C" {} blocks.
   if (!match(expr(hasAncestor(linkageSpecDecl())), 

[clang-tools-extra] r345637 - NFC: Remove the ObjC1/ObjC2 distinction from clang (and related projects)

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Tue Oct 30 13:31:30 2018
New Revision: 345637

URL: http://llvm.org/viewvc/llvm-project?rev=345637=rev
Log:
NFC: Remove the ObjC1/ObjC2 distinction from clang (and related projects)

We haven't supported compiling ObjC1 for a long time (and never will again), so
there isn't any reason to keep these separate. This patch replaces
LangOpts::ObjC1 and LangOpts::ObjC2 with LangOpts::ObjC.

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

Modified:
clang-tools-extra/trunk/clang-tidy/google/AvoidCStyleCastsCheck.cpp

clang-tools-extra/trunk/clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp
clang-tools-extra/trunk/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
clang-tools-extra/trunk/clang-tidy/objc/AvoidNSErrorInitCheck.cpp
clang-tools-extra/trunk/clang-tidy/objc/ForbiddenSubclassingCheck.cpp
clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp

Modified: clang-tools-extra/trunk/clang-tidy/google/AvoidCStyleCastsCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/AvoidCStyleCastsCheck.cpp?rev=345637=345636=345637=diff
==
--- clang-tools-extra/trunk/clang-tidy/google/AvoidCStyleCastsCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/google/AvoidCStyleCastsCheck.cpp Tue Oct 
30 13:31:30 2018
@@ -101,7 +101,7 @@ void AvoidCStyleCastsCheck::check(const
 
   // The rest of this check is only relevant to C++.
   // We also disable it for Objective-C++.
-  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC1 || getLangOpts().ObjC2)
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC)
 return;
   // Ignore code inside extern "C" {} blocks.
   if (!match(expr(hasAncestor(linkageSpecDecl())), *CastExpr, *Result.Context)

Modified: 
clang-tools-extra/trunk/clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp?rev=345637=345636=345637=diff
==
--- 
clang-tools-extra/trunk/clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp 
(original)
+++ 
clang-tools-extra/trunk/clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp 
Tue Oct 30 13:31:30 2018
@@ -20,9 +20,8 @@ namespace objc {
 
 void AvoidThrowingObjCExceptionCheck::registerMatchers(MatchFinder *Finder) {
   // this check should only be applied to ObjC sources.
-  if (!getLangOpts().ObjC1 && !getLangOpts().ObjC2) {
+  if (!getLangOpts().ObjC)
 return;
-  }
 
   Finder->addMatcher(objcThrowStmt().bind("throwStmt"), this);
   Finder->addMatcher(

Modified: 
clang-tools-extra/trunk/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/GlobalVariableDeclarationCheck.cpp?rev=345637=345636=345637=diff
==
--- 
clang-tools-extra/trunk/clang-tidy/google/GlobalVariableDeclarationCheck.cpp 
(original)
+++ 
clang-tools-extra/trunk/clang-tidy/google/GlobalVariableDeclarationCheck.cpp 
Tue Oct 30 13:31:30 2018
@@ -55,9 +55,9 @@ FixItHint generateFixItHint(const VarDec
 
 void GlobalVariableDeclarationCheck::registerMatchers(MatchFinder *Finder) {
   // The relevant Style Guide rule only applies to Objective-C.
-  if (!getLangOpts().ObjC1 && !getLangOpts().ObjC2) {
+  if (!getLangOpts().ObjC)
 return;
-  }
+
   // need to add two matchers since we need to bind different ids to 
distinguish
   // constants and variables. Since bind() can only be called on node matchers,
   // we cannot make it in one matcher.

Modified: clang-tools-extra/trunk/clang-tidy/objc/AvoidNSErrorInitCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/objc/AvoidNSErrorInitCheck.cpp?rev=345637=345636=345637=diff
==
--- clang-tools-extra/trunk/clang-tidy/objc/AvoidNSErrorInitCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/objc/AvoidNSErrorInitCheck.cpp Tue Oct 
30 13:31:30 2018
@@ -19,9 +19,9 @@ namespace objc {
 
 void AvoidNSErrorInitCheck::registerMatchers(MatchFinder *Finder) {
   // this check should only be applied to ObjC sources.
-  if (!getLangOpts().ObjC1 && !getLangOpts().ObjC2) {
+  if (!getLangOpts().ObjC)
 return;
-  }
+
   Finder->addMatcher(objcMessageExpr(hasSelector("init"),
  hasReceiverType(asString("NSError *")))
  .bind("nserrorInit"),

Modified: clang-tools-extra/trunk/clang-tidy/objc/ForbiddenSubclassingCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/objc/ForbiddenSubclassingCheck.cpp?rev=345637=345636=345637=diff
==
--- 

r345637 - NFC: Remove the ObjC1/ObjC2 distinction from clang (and related projects)

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Tue Oct 30 13:31:30 2018
New Revision: 345637

URL: http://llvm.org/viewvc/llvm-project?rev=345637=rev
Log:
NFC: Remove the ObjC1/ObjC2 distinction from clang (and related projects)

We haven't supported compiling ObjC1 for a long time (and never will again), so
there isn't any reason to keep these separate. This patch replaces
LangOpts::ObjC1 and LangOpts::ObjC2 with LangOpts::ObjC.

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

Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/Features.def
cfe/trunk/include/clang/Basic/LangOptions.def
cfe/trunk/include/clang/Basic/TokenKinds.def
cfe/trunk/include/clang/Basic/TokenKinds.h
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/lib/ARCMigrate/ARCMT.cpp
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/AST/NSAPI.cpp
cfe/trunk/lib/Analysis/FormatString.cpp
cfe/trunk/lib/Basic/Builtins.cpp
cfe/trunk/lib/Basic/IdentifierTable.cpp
cfe/trunk/lib/Basic/Module.cpp
cfe/trunk/lib/Basic/Targets/OSTargets.cpp
cfe/trunk/lib/CodeGen/CGBlocks.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CGException.cpp
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Frontend/ASTUnit.cpp
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Frontend/FrontendAction.cpp
cfe/trunk/lib/Frontend/InitHeaderSearch.cpp
cfe/trunk/lib/Frontend/InitPreprocessor.cpp
cfe/trunk/lib/Frontend/Rewrite/RewriteModernObjC.cpp
cfe/trunk/lib/Frontend/Rewrite/RewriteObjC.cpp
cfe/trunk/lib/Lex/Lexer.cpp
cfe/trunk/lib/Lex/PPDirectives.cpp
cfe/trunk/lib/Parse/ParseDecl.cpp
cfe/trunk/lib/Parse/ParseDeclCXX.cpp
cfe/trunk/lib/Parse/ParseExpr.cpp
cfe/trunk/lib/Parse/ParseExprCXX.cpp
cfe/trunk/lib/Parse/ParseInit.cpp
cfe/trunk/lib/Parse/ParseObjc.cpp
cfe/trunk/lib/Parse/ParseStmt.cpp
cfe/trunk/lib/Parse/ParseTentative.cpp
cfe/trunk/lib/Parse/Parser.cpp
cfe/trunk/lib/Sema/Sema.cpp
cfe/trunk/lib/Sema/SemaCast.cpp
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaExprMember.cpp
cfe/trunk/lib/Sema/SemaExprObjC.cpp
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/lib/Sema/SemaLambda.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp
cfe/trunk/lib/Tooling/Inclusions/HeaderIncludes.cpp
cfe/trunk/test/Modules/module_file_info.m
cfe/trunk/tools/arcmt-test/arcmt-test.cpp
cfe/trunk/tools/clang-import-test/clang-import-test.cpp
cfe/trunk/tools/libclang/CXType.cpp

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=345637=345636=345637=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Tue Oct 30 13:31:30 2018
@@ -294,7 +294,7 @@ def COnly : LangOpt<"CPlusPlus", 1>;
 def CPlusPlus : LangOpt<"CPlusPlus">;
 def OpenCL : LangOpt<"OpenCL">;
 def RenderScript : LangOpt<"RenderScript">;
-def ObjC : LangOpt<"ObjC1">;
+def ObjC : LangOpt<"ObjC">;
 def BlocksSupported : LangOpt<"Blocks">;
 
 // Defines targets for target-specific attributes. Empty lists are unchecked.

Modified: cfe/trunk/include/clang/Basic/Features.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Features.def?rev=345637=345636=345637=diff
==
--- cfe/trunk/include/clang/Basic/Features.def (original)
+++ cfe/trunk/include/clang/Basic/Features.def Tue Oct 30 13:31:30 2018
@@ -88,11 +88,11 @@ FEATURE(objc_arr, LangOpts.ObjCAutoRefCo
 FEATURE(objc_arc, LangOpts.ObjCAutoRefCount)
 FEATURE(objc_arc_fields, true)
 FEATURE(objc_arc_weak, LangOpts.ObjCWeak)
-FEATURE(objc_default_synthesize_properties, LangOpts.ObjC2)
+FEATURE(objc_default_synthesize_properties, LangOpts.ObjC)
 FEATURE(objc_fixed_enum, true)
-FEATURE(objc_instancetype, LangOpts.ObjC2)
-FEATURE(objc_kindof, LangOpts.ObjC2)
-FEATURE(objc_modules, LangOpts.ObjC2 &)
+FEATURE(objc_instancetype, LangOpts.ObjC)
+FEATURE(objc_kindof, LangOpts.ObjC)
+FEATURE(objc_modules, LangOpts.ObjC && LangOpts.Modules)
 FEATURE(objc_nonfragile_abi, LangOpts.ObjCRuntime.isNonFragile())
 FEATURE(objc_property_explicit_atomic, true)
 FEATURE(objc_protocol_qualifier_mangling, true)
@@ -102,16 +102,16 @@ FEATURE(ownership_returns, true)
 FEATURE(ownership_takes, true)
 FEATURE(objc_bool, true)
 FEATURE(objc_subscripting, LangOpts.ObjCRuntime.isNonFragile())

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

[Taewook Oh via Phabricator via cfe-commits]

twoh added a comment.

I think the context is Derived here. My understanding of 
http://wg21.link/p0968r0#2227 (in this patch's context) is that when Derived is 
aggregate initialized, the destructor for each element of Base is potentially 
invoked as well.

For me it seems that the bug is checking destructor accessibility of Base 
itself, not fields of Base. I think the right fix for 1956-1960 is

  if (!VerifyOnly) {
auto *BaseRD = Base.getType()->getAs()->getDecl();
for (FieldDecl *FD : BaseRD->fields()) {
  QualType ET = SemaRef.Context.getBaseElementType(FD->getType());
  if (hasAccessibleDestructor(ET, InitLoc, SemaRef)) {
hadError = true;
return;
  }
}
  }


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

r345635 - Speculatively attempt to fix a failing testbot.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Tue Oct 30 12:49:17 2018
New Revision: 345635

URL: http://llvm.org/viewvc/llvm-project?rev=345635=rev
Log:
Speculatively attempt to fix a failing testbot.

A testbot ( 
http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/54690/) 
was failing with a complaint about an obsolete option that wasn't present in 
the command line in the first place. This replaces my guess at the "obsolete 
option" with a different spelling that will hopefully be more acceptable to 
this bot without breaking other bots.

Modified:

cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
cfe/trunk/test/Analysis/diagnostics/sarif-diagnostics-taint-test.c

Modified: 
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif?rev=345635=345634=345635=diff
==
--- 
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
 (original)
+++ 
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
 Tue Oct 30 12:49:17 2018
@@ -7,7 +7,7 @@
   "fileLocation": {
 "uri": "file:sarif-diagnostics-taint-test.c"
   },
-  "length": 500,
+  "length": 501,
   "mimeType": "text/plain",
   "roles": [
 "resultFile"

Modified: cfe/trunk/test/Analysis/diagnostics/sarif-diagnostics-taint-test.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/sarif-diagnostics-taint-test.c?rev=345635=345634=345635=diff
==
--- cfe/trunk/test/Analysis/diagnostics/sarif-diagnostics-taint-test.c 
(original)
+++ cfe/trunk/test/Analysis/diagnostics/sarif-diagnostics-taint-test.c Tue Oct 
30 12:49:17 2018
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 
-analyzer-checker=alpha.security.taint,debug.TaintTest %s -verify 
-analyzer-output=sarif -o - | diff -u1 -w -I 
".*file:.*sarif-diagnostics-taint-test.c" -I "clang version" -I 
"2\.0\.0\-beta\." - 
%S/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
+// RUN: %clang_analyze_cc1 
-analyzer-checker=alpha.security.taint,debug.TaintTest %s -verify 
-analyzer-output=sarif -o - | diff -U 1 -w -I 
".*file:.*sarif-diagnostics-taint-test.c" -I "clang version" -I 
"2\.0\.0\-beta\." - 
%S/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
 #include "../Inputs/system-header-simulator.h"
 
 int atoi(const char *nptr);


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

[PATCH] D53025: [clang-tidy] implement new check for const return types.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added inline comments.



Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:107
+diag(Def->getInnerLocStart(), "return type %0 is 'const'-qualified "
+  "hindering compiler optimizations")
+<< Def->getReturnType();

aaron.ballman wrote:
> ymandel wrote:
> > aaron.ballman wrote:
> > > ymandel wrote:
> > > > aaron.ballman wrote:
> > > > > ymandel wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > ymandel wrote:
> > > > > > > > aaron.ballman wrote:
> > > > > > > > > It seems strange to me that this is in the readability module 
> > > > > > > > > but the diagnostic here is about compiler optimizations. 
> > > > > > > > > Should this be in the performance module instead? Or should 
> > > > > > > > > this diagnostic be reworded about the readability concerns?
> > > > > > > > Good point. The readability angle is that the const clutters 
> > > > > > > > the code to no benefit.  That is, even if it was performance 
> > > > > > > > neutral, I'd still want to discourage this practice.  But, it 
> > > > > > > > does seem like the primary impact is performance. 
> > > > > > > > 
> > > > > > > > I'm fine either way -- moving it to performance or emphasizing 
> > > > > > > > the clutter of the unhelpful "const".  I'm inclined to moving 
> > > > > > > > it, but don't have good sense of how strict these hierarchies 
> > > > > > > > are. What do you think?
> > > > > > > I'm not sold that `const`-qualified return types always pessimize 
> > > > > > > optimizations. However, I'm also not sold that it's *always* a 
> > > > > > > bad idea to have a top-level cv-qualifier on a return type either 
> > > > > > > (for instance, this is one way to prevent callers from modifying 
> > > > > > > a temp returned by a function). How about leaving this in 
> > > > > > > readability and changing the diagnostic into: "top-level 'const' 
> > > > > > > qualifier on a return type may reduce code readability with 
> > > > > > > limited benefit" or something equally weasely?
> > > > > > I talked this over with other google folks and seems like the 
> > > > > > consensus is:
> > > > > > 
> > > > > > 1.  The readability benefits may be controversial.  Some folks 
> > > > > > might not view `const` as clutter and there are some corner cases 
> > > > > > where the qualifier may prevent something harmful.
> > > > > > 2.  The performance implication is real, if not guaranteed to be a 
> > > > > > problem.
> > > > > > 
> > > > > > Based on this, seems best to move it to performance, but water down 
> > > > > > the performance claims.  That keeps the focus to an issue we can 
> > > > > > assume nearly everyone agrees on.
> > > > > I'm not convinced the performance implications are real compared to 
> > > > > how the check is currently implemented. I know there are performance 
> > > > > concerns when you return a const value of class type because it 
> > > > > pessimizes the ability to use move constructors, but that's a very 
> > > > > specific performance concern that I don't believe is present in 
> > > > > general. For instance, I don't know of any performance concerns 
> > > > > regarding `const int f();` as a declaration. I'd be fine moving this 
> > > > > to the performance module, but I think the check would need to be 
> > > > > tightened to only focus on actual performance concerns.
> > > > > 
> > > > > FWIW, I do agree that the readability benefits may be controversial, 
> > > > > but I kind of thought that was the point to the check and as such, 
> > > > > it's a very reasonable check. Almost everything in readability is 
> > > > > subjective to some degree and our metric has always been "if you 
> > > > > agree with a style check, don't enable it".
> > > > > 
> > > > > It's up to you how you want to proceed, but I see two ways forward: 
> > > > > 1) keep this as a readability check that broadly finds top-level 
> > > > > const qualifiers and removes them, 2) move this to the performance 
> > > > > module and rework the check to focus on only the scenarios that have 
> > > > > concrete performance impact.
> > > > > It's up to you how you want to proceed, but I see two ways forward: 
> > > > > 1) keep this as a readability check that broadly finds top-level 
> > > > > const qualifiers and removes them, 2) move this to the performance 
> > > > > module and rework the check to focus on only the scenarios that have 
> > > > > concrete performance impact.
> > > > 
> > > > Aaron, thank you for the detailed response. I'm inclined to agree with 
> > > > you that this belongs in readabilty and I spoke with sbenza who feels 
> > > > the same.  The high-level point is that the `const` is noise in most 
> > > > cases.
> > > > 
> > > > You suggested above a warning along the lines of:
> > > >  "top-level 'const' qualifier on a return type may reduce code 
> > > > readability with limited benefit"
> > > > 
> > > > I like this, but think it should be 

[PATCH] D53025: [clang-tidy] implement new check for const return types.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 171765.
ymandel marked 2 inline comments as done.
ymandel added a comment.

Add comment to field.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53025

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ConstValueReturnCheck.cpp
  clang-tidy/readability/ConstValueReturnCheck.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-const-value-return.rst
  test/clang-tidy/readability-const-value-return.cpp

Index: test/clang-tidy/readability-const-value-return.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-const-value-return.cpp
@@ -0,0 +1,227 @@
+// RUN: %check_clang_tidy %s readability-const-value-return %t -- -- -isystem
+
+//  p# = positive test
+//  n# = negative test
+
+#include 
+
+const int p1() {
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const int' is 'const'-qualified at the top level, which may reduce code readability without improving const correctness
+// CHECK-FIXES: int p1() {
+  return 1;
+}
+
+const int p15();
+// CHECK-FIXES: int p15();
+
+template 
+const int p31(T v) { return 2; }
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const int' is 'const'-qu
+// CHECK-FIXES: int p31(T v) { return 2; }
+
+// We detect const-ness even without instantiating T.
+template 
+const T p32(T t) { return t; }
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const T' is 'const'-qual
+// CHECK-FIXES: T p32(T t) { return t; }
+
+// However, if the return type is itself a template instantiation, Clang does
+// not consider it const-qualified without knowing `T`.
+template 
+typename std::add_const::type n15(T v) { return v; }
+
+template 
+class Klazz {
+public:
+  Klazz(A) {}
+};
+
+class Clazz {
+ public:
+  Clazz *const p2() {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'Clazz *const' is 'co
+// CHECK-FIXES: Clazz *p2() {
+return this;
+  }
+
+  Clazz *const p3();
+  // CHECK-FIXES: Clazz *p3();
+
+  const int p4() const {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'const int' is 'const
+// CHECK-FIXES: int p4() const {
+return 4;
+  }
+
+  const Klazz* const p5() const;
+  // CHECK-FIXES: const Klazz* p5() const;
+
+  const Clazz operator++(int x) {  //  p12
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'const Clazz' is 'const
+  // CHECK-FIXES: Clazz operator++(int x) {
+  }
+
+  struct Strukt {
+int i;
+  };
+
+  const Strukt p6() {}
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'const Clazz::Strukt' i
+  // CHECK-FIXES: Strukt p6() {}
+
+  // No warning is emitted here, because this is only the declaration.  The
+  // warning will be associated with the definition, below.
+  const Strukt* const p7();
+  // CHECK-FIXES: const Strukt* p7();
+
+  // const-qualifier is the first `const` token, but not the first token.
+  static const int p8() {}
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'const int' is 'const'-
+  // CHECK-FIXES: static int p8() {}
+
+  static const Strukt p9() {}
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'const Clazz::Strukt' i
+  // CHECK-FIXES: static Strukt p9() {}
+
+  int n0() const { return 0; }
+  const Klazz& n11(const Klazz) const;
+};
+
+Clazz *const Clazz::p3() {
+  // CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'Clazz *const' is 'cons
+  // CHECK-FIXES: Clazz *Clazz::p3() {
+  return this;
+}
+
+const Klazz* const Clazz::p5() const {}
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz *
+// CHECK-FIXES: const Klazz* Clazz::p5() const {}
+
+const Clazz::Strukt* const Clazz::p7() {}
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Clazz::Strukt *con
+// CHECK-FIXES: const Clazz::Strukt* Clazz::p7() {}
+
+Clazz *const p10();
+// CHECK-FIXES: Clazz *p10();
+
+Clazz *const p10() {
+  // CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'Clazz *const' is 'cons
+  // CHECK-FIXES: Clazz *p10() {
+  return new Clazz();
+}
+
+const Clazz bar;
+const Clazz *const p11() {
+  // CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Clazz *const' is
+  // CHECK-FIXES: const Clazz *p11() {
+  return 
+}
+
+const Klazz p12() {}
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz'
+// CHECK-FIXES: Klazz p12() {}
+
+const Klazz* const p13() {}
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz *
+// CHECK-FIXES: const Klazz* p13() {}
+
+// re-declaration of p15.
+const int p15();
+// CHECK-FIXES: int p15();
+
+const int p15() {
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning:
+// CHECK-FIXES: int p15() {
+  return 0;
+}
+
+// Exercise the lexer.
+
+const /* comment */ /* another comment*/ int p16() { return 0; }
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning:
+// CHECK-FIXES: /* 

[PATCH] D53025: [clang-tidy] implement new check for const return types.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 171764.
ymandel marked an inline comment as done.
ymandel added a comment.

Modify dialog printing to highlight const token.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53025

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ConstValueReturnCheck.cpp
  clang-tidy/readability/ConstValueReturnCheck.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-const-value-return.rst
  test/clang-tidy/readability-const-value-return.cpp

Index: test/clang-tidy/readability-const-value-return.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-const-value-return.cpp
@@ -0,0 +1,227 @@
+// RUN: %check_clang_tidy %s readability-const-value-return %t -- -- -isystem
+
+//  p# = positive test
+//  n# = negative test
+
+#include 
+
+const int p1() {
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const int' is 'const'-qualified at the top level, which may reduce code readability without improving const correctness
+// CHECK-FIXES: int p1() {
+  return 1;
+}
+
+const int p15();
+// CHECK-FIXES: int p15();
+
+template 
+const int p31(T v) { return 2; }
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const int' is 'const'-qu
+// CHECK-FIXES: int p31(T v) { return 2; }
+
+// We detect const-ness even without instantiating T.
+template 
+const T p32(T t) { return t; }
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const T' is 'const'-qual
+// CHECK-FIXES: T p32(T t) { return t; }
+
+// However, if the return type is itself a template instantiation, Clang does
+// not consider it const-qualified without knowing `T`.
+template 
+typename std::add_const::type n15(T v) { return v; }
+
+template 
+class Klazz {
+public:
+  Klazz(A) {}
+};
+
+class Clazz {
+ public:
+  Clazz *const p2() {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'Clazz *const' is 'co
+// CHECK-FIXES: Clazz *p2() {
+return this;
+  }
+
+  Clazz *const p3();
+  // CHECK-FIXES: Clazz *p3();
+
+  const int p4() const {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'const int' is 'const
+// CHECK-FIXES: int p4() const {
+return 4;
+  }
+
+  const Klazz* const p5() const;
+  // CHECK-FIXES: const Klazz* p5() const;
+
+  const Clazz operator++(int x) {  //  p12
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'const Clazz' is 'const
+  // CHECK-FIXES: Clazz operator++(int x) {
+  }
+
+  struct Strukt {
+int i;
+  };
+
+  const Strukt p6() {}
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'const Clazz::Strukt' i
+  // CHECK-FIXES: Strukt p6() {}
+
+  // No warning is emitted here, because this is only the declaration.  The
+  // warning will be associated with the definition, below.
+  const Strukt* const p7();
+  // CHECK-FIXES: const Strukt* p7();
+
+  // const-qualifier is the first `const` token, but not the first token.
+  static const int p8() {}
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'const int' is 'const'-
+  // CHECK-FIXES: static int p8() {}
+
+  static const Strukt p9() {}
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'const Clazz::Strukt' i
+  // CHECK-FIXES: static Strukt p9() {}
+
+  int n0() const { return 0; }
+  const Klazz& n11(const Klazz) const;
+};
+
+Clazz *const Clazz::p3() {
+  // CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'Clazz *const' is 'cons
+  // CHECK-FIXES: Clazz *Clazz::p3() {
+  return this;
+}
+
+const Klazz* const Clazz::p5() const {}
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz *
+// CHECK-FIXES: const Klazz* Clazz::p5() const {}
+
+const Clazz::Strukt* const Clazz::p7() {}
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Clazz::Strukt *con
+// CHECK-FIXES: const Clazz::Strukt* Clazz::p7() {}
+
+Clazz *const p10();
+// CHECK-FIXES: Clazz *p10();
+
+Clazz *const p10() {
+  // CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'Clazz *const' is 'cons
+  // CHECK-FIXES: Clazz *p10() {
+  return new Clazz();
+}
+
+const Clazz bar;
+const Clazz *const p11() {
+  // CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Clazz *const' is
+  // CHECK-FIXES: const Clazz *p11() {
+  return 
+}
+
+const Klazz p12() {}
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz'
+// CHECK-FIXES: Klazz p12() {}
+
+const Klazz* const p13() {}
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz *
+// CHECK-FIXES: const Klazz* p13() {}
+
+// re-declaration of p15.
+const int p15();
+// CHECK-FIXES: int p15();
+
+const int p15() {
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning:
+// CHECK-FIXES: int p15() {
+  return 0;
+}
+
+// Exercise the lexer.
+
+const /* comment */ /* another comment*/ int p16() { return 0; }
+// CHECK-MESSAGES: [[@LINE-1]]:1: 

[PATCH] D53882: [clang-tidy] Adding Zircon checker for std namespace

[Julie Hockett via Phabricator via cfe-commits]

juliehockett created this revision.
juliehockett added reviewers: aaron.ballman, hokein, ilya-biryukov.
juliehockett added a project: clang-tools-extra.
Herald added subscribers: xazax.hun, mgorny.

Adds a checker to warn against using the std namespace, as Zircon's kernel 
lib++ policy does not allow it. Written documentation of the policy is not yet 
published, I will add the link when it is.


https://reviews.llvm.org/D53882

Files:
  clang-tools-extra/clang-tidy/zircon/CMakeLists.txt
  clang-tools-extra/clang-tidy/zircon/NoStdNamespaceCheck.cpp
  clang-tools-extra/clang-tidy/zircon/NoStdNamespaceCheck.h
  clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/zircon-no-std-namespace.rst
  clang-tools-extra/test/clang-tidy/zircon-no-std-namespace.cpp

Index: clang-tools-extra/test/clang-tidy/zircon-no-std-namespace.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/zircon-no-std-namespace.cpp
@@ -0,0 +1,50 @@
+// RUN: %check_clang_tidy %s zircon-no-std-namespace %t
+
+int func() { return 1; }
+
+namespace std {
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+
+typedef int std_int;
+int std_func() { return 1; }
+
+int a = 1;
+int b = func();
+std_int c;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+int d = std_func();
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+
+}
+
+// Warn on uses of quailfied std namespace.
+int i = 1;
+int j = func();
+std::std_int k;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+int l = std::std_func();
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+
+
+namespace foo {
+
+int w = 1;
+int x = func();
+std::std_int y;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+int z = std::std_func();
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+
+}
+
+// Warn on the alias declaration, and on users of it.
+namespace bar = std;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+bar::std_int q;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+
+// Warn on the using declaration, and on users of it.
+using namespace std;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use of the 'std' namespace is not allowed in Zircon kernel code
+std_int r = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of the 'std' namespace is not allowed in Zircon kernel code
Index: clang-tools-extra/docs/clang-tidy/checks/zircon-no-std-namespace.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/zircon-no-std-namespace.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - zircon-no-std-namespace
+
+zircon-no-std-namespace
+===
+
+Ensures code does not open ``namespace std`` as that violates Zircon's
+kernel libc++ policy. 
+
+Any code that uses:
+
+.. code-block:: c++
+
+ namespace std {
+  ...
+ }
+
+or 
+
+.. code-block:: c++
+
+ using namespace std;
+
+or uses a declaration from the ``std`` namespace:
+
+.. code-block:: c++
+
+ std::string foo;
+
+will be prompted with a warning.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -252,4 +252,5 @@
readability-string-compare
readability-uniqueptr-delete-release
readability-uppercase-literal-suffix
+   zircon-no-std-namespace
zircon-temporary-objects
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -170,6 +170,12 @@
   ` check does not warn
   about calls inside macros anymore by default.
 
+- New :doc:`zircon-no-std-namespace
+  ` check.
+
+  Warns when the `std` namespace is used, as its use is against Zircon's libc++
+  policy for the kernel.
+
 Improvements to include-fixer
 -
 
Index: clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
+++ clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include 

r345633 - Silencing a -Wunused-variable warning; NFC.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Tue Oct 30 12:23:06 2018
New Revision: 345633

URL: http://llvm.org/viewvc/llvm-project?rev=345633=rev
Log:
Silencing a -Wunused-variable warning; NFC.

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

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp?rev=345633=345632=345633=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp Tue Oct 30 12:23:06 
2018
@@ -174,8 +174,6 @@ static json::Object createLocation(json:
 }
 
 static Importance calculateImportance(const PathDiagnosticPiece ) {
-  StringRef PieceStr = Piece.getString();
-
   switch (Piece.getKind()) {
   case PathDiagnosticPiece::Kind::Call:
   case PathDiagnosticPiece::Kind::Macro:


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

r345630 - Fixing some build bot failures from r345628; NFC intended.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Tue Oct 30 12:06:58 2018
New Revision: 345630

URL: http://llvm.org/viewvc/llvm-project?rev=345630=rev
Log:
Fixing some build bot failures from r345628; NFC intended.

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

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp?rev=345630=345629=345630=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp Tue Oct 30 12:06:58 
2018
@@ -70,7 +70,7 @@ static std::string percentEncodeURIChara
 }
 
 static std::string fileNameToURI(StringRef Filename) {
-  llvm::SmallString<32> Ret = "file://";
+  llvm::SmallString<32> Ret = StringRef("file://");
 
   // Get the root name to see if it has a URI authority.
   StringRef Root = sys::path::root_name(Filename);


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

[PATCH] D53475: Create ConstantExpr class

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

Ping?


Repository:
  rC Clang

https://reviews.llvm.org/D53475



___
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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

In https://reviews.llvm.org/D53814#1280581, @george.karpenkov wrote:

> I much prefer this version.
>  We've had the same problem with diffing plist output.
>  One thing we have learned is using a FileCheck was definitely a bad idea, as 
> it leads to unreadable, unmaintainable, and very hard to update tests,
>  so either diff or your custom tool is way better.
>
> As for the ultimate solution, I'm still not sure: I agree that maintaining 
> those `-I` flags is annoying.


We can go with this approach until we need something more complicated. I 
suspect that as we add SARIF features, we may want to bring back the Python 
script for handling things like "Does every file in the 'files' list appear 
only once and do the files listed correspond exactly to ones in the diagnostic 
locations?". Diff definitely won't handle that sort of thing.

> One option is having an extra flag to produce "stable" output, which does not 
> include absolute URLs/versions/etc.

Worth thinking about. SARIF has the ability to output relative paths as well as 
absolute paths. It also has the notion of redacted paths so that you can remove 
sensitive information from analysis reports. So there's plenty of room for 
changes here.

Thank you for the reviews! I've commit in r345628.


https://reviews.llvm.org/D53814



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

r345628 - Add the ability to output static analysis results to SARIF.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Tue Oct 30 11:55:38 2018
New Revision: 345628

URL: http://llvm.org/viewvc/llvm-project?rev=345628=rev
Log:
Add the ability to output static analysis results to SARIF.

This allows users to specify SARIF (https://github.com/oasis-tcs/sarif-spec) as 
the output from the clang static analyzer so that the results can be read in by 
other tools, such as extensions to Visual Studio and VSCode, as well as static 
analyzers like CodeSonar.

Added:
cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/

cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
cfe/trunk/test/Analysis/diagnostics/sarif-diagnostics-taint-test.c
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/Analyses.def
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/Analyses.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/Analyses.def?rev=345628=345627=345628=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/Analyses.def (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/Analyses.def Tue Oct 30 
11:55:38 2018
@@ -33,6 +33,7 @@ ANALYSIS_DIAGNOSTICS(HTML_SINGLE_FILE, "
 ANALYSIS_DIAGNOSTICS(PLIST, "plist", "Output analysis results using Plists", 
createPlistDiagnosticConsumer)
 ANALYSIS_DIAGNOSTICS(PLIST_MULTI_FILE, "plist-multi-file", "Output analysis 
results using Plists (allowing for multi-file bugs)", 
createPlistMultiFileDiagnosticConsumer)
 ANALYSIS_DIAGNOSTICS(PLIST_HTML, "plist-html", "Output analysis results using 
HTML wrapped with Plists", createPlistHTMLDiagnosticConsumer)
+ANALYSIS_DIAGNOSTICS(SARIF, "sarif", "Output analysis results in a SARIF 
file", createSarifDiagnosticConsumer)
 ANALYSIS_DIAGNOSTICS(TEXT, "text", "Text output of analysis results", 
createTextPathDiagnosticConsumer)
 
 #ifndef ANALYSIS_PURGE

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h?rev=345628=345627=345628=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h 
Tue Oct 30 11:55:38 2018
@@ -118,7 +118,7 @@ public:
 /// Only runs visitors, no output generated.
 None,
 
-/// Used for HTML and text output.
+/// Used for HTML, SARIF, and text output.
 Minimal,
 
 /// Used for plist output, used for "arrows" generation.

Modified: cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt?rev=345628=345627=345628=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt Tue Oct 30 11:55:38 2018
@@ -45,12 +45,13 @@ add_clang_library(clangStaticAnalyzerCor
   RangedConstraintManager.cpp
   RegionStore.cpp
   RetainSummaryManager.cpp
-  SValBuilder.cpp
-  SVals.cpp
+  SarifDiagnostics.cpp
   SimpleConstraintManager.cpp
   SimpleSValBuilder.cpp
   Store.cpp
   SubEngine.cpp
+  SValBuilder.cpp
+  SVals.cpp
   SymbolManager.cpp
   TaintManager.cpp
   WorkList.cpp

Added: cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp?rev=345628=auto
==
--- cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp (added)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp Tue Oct 30 11:55:38 
2018
@@ -0,0 +1,270 @@
+//===--- 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"
+#include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/JSON.h"
+#include "llvm/Support/Path.h"
+
+using namespace llvm;
+using namespace clang;
+using 

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Sorry for responding so late, but i thought i get a shot to look at it. Here is 
the mail from Douglas Yung who was so kind to isolate the breakage and find a 
reproducer.
If you have the chance please take a look at it.

  Hi Jonas,
  
  I have attached a bzipped preprocessed file that I can confirm will show the 
failure if built with clang++ version 3.7.1, specifically the version that 
shipped with ubuntu 14.04.5 LTS. Here is the full version information:
  
  Ubuntu clang version 3.7.1-svn253742-1~exp1 (branches/release_37) (based on 
LLVM 3.7.1)
  
  Target: x86_64-pc-linux-gnu
  Thread model: posix
  
  If you build it with “clang++ -std=c++11 
NotNullTerminatedResultCheck.preproc.cpp” you should see the failure.
  
  Hope this helps.
  Douglas Yung

The file is in the attachement of this comment. F7474876: 
NotNullTerminatedResultCheck.preproc.cpp.bz2 


https://reviews.llvm.org/D45050



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

[PATCH] D53725: [CodeGen] Move `emitConstant` from ScalarExprEmitter to CodeGenFunction. NFC.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

In https://reviews.llvm.org/D53725#1278067, @rjmccall wrote:

> This should at least be named `emitScalarConstant`.


Agree. I've just capitalized 'e' as it looks like the majority of `Emit...` 
methods are capitalized that way.


https://reviews.llvm.org/D53725



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

[PATCH] D53725: [CodeGen] Move `emitConstant` from ScalarExprEmitter to CodeGenFunction. NFC.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai updated this revision to Diff 171751.
vsapsai added a comment.

- Rename `EmitConstant` to `EmitScalarConstant`.


https://reviews.llvm.org/D53725

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.h


Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -3524,6 +3524,7 @@
 
   ConstantEmission tryEmitAsConstant(DeclRefExpr *refExpr);
   ConstantEmission tryEmitAsConstant(const MemberExpr *ME);
+  llvm::Value *EmitScalarConstant(const ConstantEmission , Expr *E);
 
   RValue EmitPseudoObjectRValue(const PseudoObjectExpr *e,
 AggValueSlot slot = AggValueSlot::ignored());
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -456,19 +456,10 @@
 return CGF.getOrCreateOpaqueRValueMapping(E).getScalarVal();
   }
 
-  Value *emitConstant(const CodeGenFunction::ConstantEmission ,
-  Expr *E) {
-assert(Constant && "not a constant");
-if (Constant.isReference())
-  return EmitLoadOfLValue(Constant.getReferenceLValue(CGF, E),
-  E->getExprLoc());
-return Constant.getValue();
-  }
-
   // l-values.
   Value *VisitDeclRefExpr(DeclRefExpr *E) {
 if (CodeGenFunction::ConstantEmission Constant = CGF.tryEmitAsConstant(E))
-  return emitConstant(Constant, E);
+  return CGF.EmitScalarConstant(Constant, E);
 return EmitLoadOfLValue(E);
   }
 
@@ -1545,7 +1536,7 @@
 Value *ScalarExprEmitter::VisitMemberExpr(MemberExpr *E) {
   if (CodeGenFunction::ConstantEmission Constant = CGF.tryEmitAsConstant(E)) {
 CGF.EmitIgnoredExpr(E->getBase());
-return emitConstant(Constant, E);
+return CGF.EmitScalarConstant(Constant, E);
   } else {
 llvm::APSInt Value;
 if (E->EvaluateAsInt(Value, CGF.getContext(), Expr::SE_AllowSideEffects)) {
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -1491,6 +1491,16 @@
   return ConstantEmission();
 }
 
+llvm::Value *CodeGenFunction::EmitScalarConstant(
+const CodeGenFunction::ConstantEmission , Expr *E) {
+  assert(Constant && "not a constant");
+  if (Constant.isReference())
+return EmitLoadOfLValue(Constant.getReferenceLValue(*this, E),
+E->getExprLoc())
+.getScalarVal();
+  return Constant.getValue();
+}
+
 llvm::Value *CodeGenFunction::EmitLoadOfScalar(LValue lvalue,
SourceLocation Loc) {
   return EmitLoadOfScalar(lvalue.getAddress(), lvalue.isVolatile(),


Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -3524,6 +3524,7 @@
 
   ConstantEmission tryEmitAsConstant(DeclRefExpr *refExpr);
   ConstantEmission tryEmitAsConstant(const MemberExpr *ME);
+  llvm::Value *EmitScalarConstant(const ConstantEmission , Expr *E);
 
   RValue EmitPseudoObjectRValue(const PseudoObjectExpr *e,
 AggValueSlot slot = AggValueSlot::ignored());
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -456,19 +456,10 @@
 return CGF.getOrCreateOpaqueRValueMapping(E).getScalarVal();
   }
 
-  Value *emitConstant(const CodeGenFunction::ConstantEmission ,
-  Expr *E) {
-assert(Constant && "not a constant");
-if (Constant.isReference())
-  return EmitLoadOfLValue(Constant.getReferenceLValue(CGF, E),
-  E->getExprLoc());
-return Constant.getValue();
-  }
-
   // l-values.
   Value *VisitDeclRefExpr(DeclRefExpr *E) {
 if (CodeGenFunction::ConstantEmission Constant = CGF.tryEmitAsConstant(E))
-  return emitConstant(Constant, E);
+  return CGF.EmitScalarConstant(Constant, E);
 return EmitLoadOfLValue(E);
   }
 
@@ -1545,7 +1536,7 @@
 Value *ScalarExprEmitter::VisitMemberExpr(MemberExpr *E) {
   if (CodeGenFunction::ConstantEmission Constant = CGF.tryEmitAsConstant(E)) {
 CGF.EmitIgnoredExpr(E->getBase());
-return emitConstant(Constant, E);
+return CGF.EmitScalarConstant(Constant, E);
   } else {
 llvm::APSInt Value;
 if (E->EvaluateAsInt(Value, CGF.getContext(), Expr::SE_AllowSideEffects)) {
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -1491,6 +1491,16 @@
   return ConstantEmission();
 }
 
+llvm::Value 

[PATCH] D53850: Declares __cpu_model as hidden symbol

[Haibo Huang via Phabricator via cfe-commits]

hhb updated this revision to Diff 171748.

https://reviews.llvm.org/D53850

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/builtin-cpu-is.c
  test/CodeGen/builtin-cpu-supports.c


Index: test/CodeGen/builtin-cpu-supports.c
===
--- test/CodeGen/builtin-cpu-supports.c
+++ test/CodeGen/builtin-cpu-supports.c
@@ -4,6 +4,8 @@
 // global, the bit grab, and the icmp correct.
 extern void a(const char *);
 
+// CHECK: @__cpu_model = external hidden global { i32, i32, i32, [1 x i32] }
+
 int main() {
   __builtin_cpu_init();
 
Index: test/CodeGen/builtin-cpu-is.c
===
--- test/CodeGen/builtin-cpu-is.c
+++ test/CodeGen/builtin-cpu-is.c
@@ -4,6 +4,8 @@
 // global, the bit grab, and the icmp correct.
 extern void a(const char *);
 
+// CHECK: @__cpu_model = external hidden global { i32, i32, i32, [1 x i32] }
+
 void intel() {
   if (__builtin_cpu_is("intel"))
 a("intel");
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -9016,6 +9016,8 @@
 
   // Grab the global __cpu_model.
   llvm::Constant *CpuModel = CGM.CreateRuntimeVariable(STy, "__cpu_model");
+  cast(CpuModel)->setVisibility(
+  llvm::GlobalValue::HiddenVisibility);
 
   // Calculate the index needed to access the correct field based on the
   // range. Also adjust the expected value.
@@ -9082,6 +9084,8 @@
 
   // Grab the global __cpu_model.
   llvm::Constant *CpuModel = CGM.CreateRuntimeVariable(STy, "__cpu_model");
+  cast(CpuModel)->setVisibility(
+  llvm::GlobalValue::HiddenVisibility);
 
   // Grab the first (0th) element from the field __cpu_features off of the
   // global in the struct STy.


Index: test/CodeGen/builtin-cpu-supports.c
===
--- test/CodeGen/builtin-cpu-supports.c
+++ test/CodeGen/builtin-cpu-supports.c
@@ -4,6 +4,8 @@
 // global, the bit grab, and the icmp correct.
 extern void a(const char *);
 
+// CHECK: @__cpu_model = external hidden global { i32, i32, i32, [1 x i32] }
+
 int main() {
   __builtin_cpu_init();
 
Index: test/CodeGen/builtin-cpu-is.c
===
--- test/CodeGen/builtin-cpu-is.c
+++ test/CodeGen/builtin-cpu-is.c
@@ -4,6 +4,8 @@
 // global, the bit grab, and the icmp correct.
 extern void a(const char *);
 
+// CHECK: @__cpu_model = external hidden global { i32, i32, i32, [1 x i32] }
+
 void intel() {
   if (__builtin_cpu_is("intel"))
 a("intel");
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -9016,6 +9016,8 @@
 
   // Grab the global __cpu_model.
   llvm::Constant *CpuModel = CGM.CreateRuntimeVariable(STy, "__cpu_model");
+  cast(CpuModel)->setVisibility(
+  llvm::GlobalValue::HiddenVisibility);
 
   // Calculate the index needed to access the correct field based on the
   // range. Also adjust the expected value.
@@ -9082,6 +9084,8 @@
 
   // Grab the global __cpu_model.
   llvm::Constant *CpuModel = CGM.CreateRuntimeVariable(STy, "__cpu_model");
+  cast(CpuModel)->setVisibility(
+  llvm::GlobalValue::HiddenVisibility);
 
   // Grab the first (0th) element from the field __cpu_features off of the
   // global in the struct STy.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53771: [clang-tidy] Avoid C arrays check

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 171747.
lebedev.ri marked 4 inline comments as done.
lebedev.ri added a comment.

Address review notes.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53771

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/hicpp/HICPPTidyModule.cpp
  clang-tidy/modernize/AvoidCArraysCheck.cpp
  clang-tidy/modernize/AvoidCArraysCheck.h
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-avoid-c-arrays.rst
  docs/clang-tidy/checks/hicpp-avoid-c-arrays.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-avoid-c-arrays.rst
  test/clang-tidy/modernize-avoid-c-arrays.cpp

Index: test/clang-tidy/modernize-avoid-c-arrays.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-avoid-c-arrays.cpp
@@ -0,0 +1,82 @@
+// RUN: %check_clang_tidy %s modernize-avoid-c-arrays %t
+
+int a[] = {1, 2};
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: do not declare C-style arrays, use std::array<> instead
+
+int b[1];
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: do not declare C-style arrays, use std::array<> instead
+
+void foo() {
+  int c[b[0]];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
+}
+
+template 
+class array {
+  T d[Size];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
+
+  int e[1];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
+};
+
+array d;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not declare C-style arrays, use std::array<> instead
+
+using k = int[4];
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not declare C-style arrays, use std::array<> instead
+
+array dk;
+
+template 
+class unique_ptr {
+  T *d;
+
+  int e[1];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
+};
+
+unique_ptr d2;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not declare C-style arrays, use std::array<> instead
+
+using k2 = int[];
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not declare C-style arrays, use std::array<> instead
+
+unique_ptr dk2;
+
+// Some header
+extern "C" {
+
+int f[] = {1, 2};
+
+int j[1];
+
+inline void bar() {
+  {
+int j[j[0]];
+  }
+}
+
+extern "C++" {
+int f3[] = {1, 2};
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: do not declare C-style arrays, use std::array<> instead
+
+int j3[1];
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: do not declare C-style arrays, use std::array<> instead
+
+struct Foo {
+  int f3[3] = {1, 2};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
+
+  int j3[1];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
+};
+}
+
+struct Bar {
+
+  int f[3] = {1, 2};
+
+  int j[1];
+};
+}
Index: docs/clang-tidy/checks/modernize-avoid-c-arrays.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-avoid-c-arrays.rst
@@ -0,0 +1,56 @@
+.. title:: clang-tidy - modernize-avoid-c-arrays
+
+modernize-avoid-c-arrays
+
+
+`cppcoreguidelines-avoid-c-arrays` redirects here as an alias for this check.
+
+`hicpp-avoid-c-arrays` redirects here as an alias for this check.
+
+Finds C-style array types and recommend to use ``std::array<>``.
+All types of C arrays are diagnosed.
+
+However, fix-it are potentially dangerous in header files and are therefore not
+emitted right now.
+
+.. code:: c++
+
+  int a[] = {1, 2}; // warning: do not declare C-style arrays, use std::array<> instead
+
+  int b[1]; // warning: do not declare C-style arrays, use std::array<> instead
+
+  void foo() {
+int c[b[0]]; // warning: do not declare C-style arrays, use std::array<> instead
+  }
+
+  template 
+  class array {
+T d[Size]; // warning: do not declare C-style arrays, use std::array<> instead
+
+int e[1]; // warning: do not declare C-style arrays, use std::array<> instead
+  };
+
+  array d; // warning: do not declare C-style arrays, use std::array<> instead
+
+  using k = int[4]; // warning: do not declare C-style arrays, use std::array<> instead
+
+
+However, the ``extern "C"`` code is ignored, since it is common to share
+such headers between C code, and C++ code.
+
+.. code:: c++
+
+  // Some header
+  extern "C" {
+
+  int f[] = {1, 2}; // not diagnosed
+
+  int j[1]; // not diagnosed
+
+  inline void bar() {
+{
+  int j[j[0]]; // not diagnosed
+}
+  }
+
+  }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -87,6 +87,7 @@

[PATCH] D53771: [clang-tidy] Avoid C arrays check

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp:84
 "cppcoreguidelines-c-copy-assignment-signature");
+CheckFactories.registerCheck(
+"cppcoreguidelines-avoid-c-arrays");

JonasToth wrote:
> please conserve the alphabetical order here
Sorted all the `CheckFactories.registerCheck<>();` lines.



Comment at: clang-tidy/modernize/AvoidCArraysCheck.cpp:44
+  unless(anyOf(hasParent(varDecl(isExternC())),
+   hasAncestor(functionDecl(isExternC())
+  .bind("typeloc"),

JonasToth wrote:
> What about struct-decls that are externC?
Hm, what is a `struct-decl`?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53771



___
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

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

http://wg21.link/p0968r0#2227 says:

The destructor for each element of class type is potentially invoked (15.4 
[class.dtor]) from the context where the aggregate initialization occurs. 
[Note: This provision ensures that destructors can be called for 
fully-constructed subobjects in case an exception is thrown (18.2 
[except.ctor]). —end note]

And '15.4 Destructors' has this sentence:
A program is ill-formed if a destructor that is potentially invoked is deleted 
or not accessible from the context of the invocation.

I'm not sure what "context" is in the case above (foo or Derived?), but if it's 
foo, it seems like clang is correct.


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] D53697: [ASTImporter][Structural Eq] Check for isBeingDefined

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 171723.
martong marked 2 inline comments as done.
martong added a comment.

- Remove unrelated test


Repository:
  rC Clang

https://reviews.llvm.org/D53697

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


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -3726,6 +3726,45 @@
   EXPECT_EQ(To1->getPreviousDecl(), To0);
 }
 
+TEST_P(ASTImporterTestBase,
+ImportShouldNotReportFalseODRErrorWhenRecordIsBeingDefined) {
+  {
+Decl *FromTU = getTuDecl(
+R"(
+template 
+struct B;
+)",
+Lang_CXX, "input0.cc");
+auto *FromD = FirstDeclMatcher().match(
+FromTU, classTemplateDecl(hasName("B")));
+
+Import(FromD, Lang_CXX);
+  }
+
+  {
+Decl *FromTU = getTuDecl(
+R"(
+template 
+struct B {
+  void f();
+  B* b;
+};
+)",
+Lang_CXX, "input1.cc");
+FunctionDecl *FromD = FirstDeclMatcher().match(
+FromTU, functionDecl(hasName("f")));
+Import(FromD, Lang_CXX);
+auto *FromCTD = FirstDeclMatcher().match(
+FromTU, classTemplateDecl(hasName("B")));
+auto *ToCTD = cast(Import(FromCTD, Lang_CXX));
+EXPECT_TRUE(ToCTD->isThisDeclarationADefinition());
+
+// We expect no (ODR) warning during the import.
+auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+EXPECT_EQ(0u, ToTU->getASTContext().getDiagnostics().getNumWarnings());
+  }
+}
+
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, DeclContextTest,
 ::testing::Values(ArgVector()), );
 
Index: lib/AST/ASTStructuralEquivalence.cpp
===
--- lib/AST/ASTStructuralEquivalence.cpp
+++ lib/AST/ASTStructuralEquivalence.cpp
@@ -1016,7 +1016,8 @@
 return false;
 
   // Compare the definitions of these two records. If either or both are
-  // incomplete, we assume that they are equivalent.
+  // incomplete (i.e. it is a forward decl), we assume that they are
+  // equivalent.
   D1 = D1->getDefinition();
   D2 = D2->getDefinition();
   if (!D1 || !D2)
@@ -1031,6 +1032,11 @@
 if (D1->hasExternalLexicalStorage() || D2->hasExternalLexicalStorage())
   return true;
 
+  // If one definition is currently being defined, we do not compare for
+  // equality and we assume that the decls are equal.
+  if (D1->isBeingDefined() || D2->isBeingDefined())
+return true;
+
   if (auto *D1CXX = dyn_cast(D1)) {
 if (auto *D2CXX = dyn_cast(D2)) {
   if (D1CXX->hasExternalLexicalStorage() &&


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -3726,6 +3726,45 @@
   EXPECT_EQ(To1->getPreviousDecl(), To0);
 }
 
+TEST_P(ASTImporterTestBase,
+ImportShouldNotReportFalseODRErrorWhenRecordIsBeingDefined) {
+  {
+Decl *FromTU = getTuDecl(
+R"(
+template 
+struct B;
+)",
+Lang_CXX, "input0.cc");
+auto *FromD = FirstDeclMatcher().match(
+FromTU, classTemplateDecl(hasName("B")));
+
+Import(FromD, Lang_CXX);
+  }
+
+  {
+Decl *FromTU = getTuDecl(
+R"(
+template 
+struct B {
+  void f();
+  B* b;
+};
+)",
+Lang_CXX, "input1.cc");
+FunctionDecl *FromD = FirstDeclMatcher().match(
+FromTU, functionDecl(hasName("f")));
+Import(FromD, Lang_CXX);
+auto *FromCTD = FirstDeclMatcher().match(
+FromTU, classTemplateDecl(hasName("B")));
+auto *ToCTD = cast(Import(FromCTD, Lang_CXX));
+EXPECT_TRUE(ToCTD->isThisDeclarationADefinition());
+
+// We expect no (ODR) warning during the import.
+auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+EXPECT_EQ(0u, ToTU->getASTContext().getDiagnostics().getNumWarnings());
+  }
+}
+
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, DeclContextTest,
 ::testing::Values(ArgVector()), );
 
Index: lib/AST/ASTStructuralEquivalence.cpp
===
--- lib/AST/ASTStructuralEquivalence.cpp
+++ lib/AST/ASTStructuralEquivalence.cpp
@@ -1016,7 +1016,8 @@
 return false;
 
   // Compare the definitions of these two records. If either or both are
-  // incomplete, we assume that they are equivalent.
+  // incomplete (i.e. it is a forward decl), we assume that they are
+  // equivalent.
   D1 = D1->getDefinition();
   D2 = D2->getDefinition();
   if (!D1 || !D2)
@@ -1031,6 +1032,11 @@
 if (D1->hasExternalLexicalStorage() || D2->hasExternalLexicalStorage())
   return true;
 
+  // If one definition is currently being defined, we do not 

[PATCH] D53697: [ASTImporter][Structural Eq] Check for isBeingDefined

[Gabor Marton via Phabricator via cfe-commits]

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

> I wonder if it is possible to get into situation where non-equivalent decls 
> are marked equivalent with this patch? If yes, we can create a mapping 
> between decls being imported and original decls as an alternative solution. 
> However, I cannot find any counterexample.

I don't think so.

This change is the natural extension what we already do (in line 1021 and 1022) 
with `getDefinition()`.
`getDefinition()` works just as we would expect with a simple `RecordDecl`: 
`getDefinition()` returns a non-nullptr if `isBeingDefined()` is true.
However, `getDefinition()` may return a non-nullptr if `D` is a `CXXRecordDecl` 
even if `D` is being defined.

  CXXRecordDecl *getDefinition() const {
// We only need an update if we don't already know which
// declaration is the definition.
auto *DD = DefinitionData ? DefinitionData : dataPtr();
return DD ? DD->Definition : nullptr;
  }

This all depends on whether `DefinitionData` is set. And we do set that during 
`ImportDefinition(RecordDecl *,)`.
And then we start importing methods and fields of the `CXXRecordDecl` via 
`ImportDeclContext` before the call to `completeDefinition()` which sets 
`IsBeingDefined`.
During those imports, the `getDefinition()` of a `CXXRecordDecl` will return 
with a non-nullptr value and we would go on checking the fields, but we are in 
the middle of importing the fields (or methods).




Comment at: lib/AST/ASTStructuralEquivalence.cpp:1037
+  // equality and we assume that the decls are equal.
+  if (D1->isBeingDefined() || D2->isBeingDefined())
+return true;

a_sidorin wrote:
> Is it worth it to assert if only one Decl should be in `isBeingDefined()` 
> state at time?
Strictly speaking `D1` will always come from the "From" context and such it 
should report true for `isBeingDefined`. But the structural equivalence logic  
should be independent from the import logic ideally, so I would not add that 
assert.



Comment at: unittests/AST/ASTImporterTest.cpp:3729
 
+TEST_P(ASTImporterTestBase, ImportingTypedefShouldImportTheCompleteType) {
+  // We already have an incomplete underlying type in the "To" context.

a_sidorin wrote:
> Looks like this test is from another patch (D53693)?
Yes, exactly, good catch, I just removed it.


Repository:
  rC Clang

https://reviews.llvm.org/D53697



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

[PATCH] D53522: [Frontend] Include module map header declaration in dependency file output

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added inline comments.



Comment at: clang/include/clang/Lex/ModuleMap.h:649-650
+  ///This can differ from \c Header's name due to symlinks.
   void addHeader(Module *Mod, Module::Header Header,
- ModuleHeaderRole Role, bool Imported = false);
+ ModuleHeaderRole Role, StringRef OrigHeaderName = "",
+ bool Imported = false);

vsapsai wrote:
> How `OrigHeaderName` is different from `Module::Header.NameAsWritten`? Based 
> on the names they should be the same, curious if and when it's not the case.
Oh, good point. I think NameAsWritten is what we're looking for here. Thanks!



Comment at: clang/lib/Frontend/DependencyFile.cpp:94
+  void moduleMapAddHeader(StringRef HeaderPath, bool IsSystem) override {
+if (!llvm::sys::path::is_absolute(HeaderPath))
+  return;

bruno wrote:
> Can you add a testecase for when `HeaderPath` isn't absolute? 
Sure, new patch adds a test. This was meant to avoid including dependencies 
found in -fmodule-files, but the new patch does this more directly by adding a 
`bool Imported` parameter to this function. I sprinkled some comments around 
explaining.


https://reviews.llvm.org/D53522



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

[PATCH] D53522: [Frontend] Include module map header declaration in dependency file output

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington updated this revision to Diff 171719.
erik.pilkington marked 7 inline comments as done.
erik.pilkington added a comment.

Address review comments. Thanks!


https://reviews.llvm.org/D53522

Files:
  clang/include/clang/Lex/ModuleMap.h
  clang/lib/Frontend/DependencyFile.cpp
  clang/lib/Frontend/ModuleDependencyCollector.cpp
  clang/lib/Lex/ModuleMap.cpp
  clang/test/Modules/Inputs/dfg-symlinks.modulemap
  clang/test/Modules/dependency-file-symlinks.c
  clang/test/Modules/dependency-gen-pch.m
  clang/test/Modules/dependency-gen.m
  clang/test/Modules/relative-dep-gen.cpp

Index: clang/test/Modules/relative-dep-gen.cpp
===
--- clang/test/Modules/relative-dep-gen.cpp
+++ clang/test/Modules/relative-dep-gen.cpp
@@ -29,10 +29,8 @@
 
 #include "Inputs/relative-dep-gen-1.h"
 
-// CHECK-BUILD: mod.pcm:
+// CHECK-BUILD: mod.pcm: {{.*}}Inputs{{[/\\]}}relative-dep-gen-1.h {{.*}}Inputs{{[/\\]}}relative-dep-gen-2.h
 // CHECK-BUILD:   {{[ \t]}}Inputs{{[/\\]}}relative-dep-gen{{(-cwd)?}}.modulemap
-// CHECK-BUILD:   {{[ \t]}}Inputs{{[/\\]}}relative-dep-gen-1.h
-// CHECK-BUILD:   {{[ \t]}}Inputs{{[/\\]}}relative-dep-gen-2.h
 // CHECK-USE: use.o:
 // CHECK-USE-DAG:   {{[ \t]}}relative-dep-gen.cpp
 // CHECK-EXPLICIT-DAG:   mod.pcm
Index: clang/test/Modules/dependency-gen.m
===
--- clang/test/Modules/dependency-gen.m
+++ clang/test/Modules/dependency-gen.m
@@ -4,19 +4,19 @@
 // RUN: %clang_cc1 -x objective-c -isystem %S/Inputs/System/usr/include -dependency-file %t.d.1 -MT %s.o -I %S/Inputs -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t-mcp %s
 // RUN: FileCheck %s < %t.d.1
 // CHECK: dependency-gen.m
-// CHECK: Inputs{{.}}module.map
 // CHECK: Inputs{{.}}diamond_top.h
+// CHECK: Inputs{{.}}module.map
 // CHECK-NOT: usr{{.}}include{{.}}module.map
 // CHECK-NOT: stdint.h
 
 
 // RUN: %clang_cc1 -x objective-c -isystem %S/Inputs/System/usr/include -dependency-file %t.d.2 -MT %s.o -I %S/Inputs -sys-header-deps -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t-mcp %s
 // RUN: FileCheck %s -check-prefix=CHECK-SYS < %t.d.2
 // CHECK-SYS: dependency-gen.m
-// CHECK-SYS: Inputs{{.}}module.map
 // CHECK-SYS: Inputs{{.}}diamond_top.h
-// CHECK-SYS: usr{{.}}include{{.}}module.map
+// CHECK-SYS: Inputs{{.}}module.map
 // CHECK-SYS: stdint.h
+// CHECK-SYS: usr{{.}}include{{.}}module.map
 
 #import "diamond_top.h"
 #import "stdint.h" // inside sysroot
Index: clang/test/Modules/dependency-gen-pch.m
===
--- clang/test/Modules/dependency-gen-pch.m
+++ clang/test/Modules/dependency-gen-pch.m
@@ -5,9 +5,9 @@
 // RUN: %clang_cc1 -isysroot %S/Inputs/System -triple x86_64-apple-darwin10 -module-file-deps -dependency-file %t.d -MT %s.o -I %S/Inputs -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t-mcp -emit-pch -o %t.pch %s
 // RUN: FileCheck %s < %t.d
 // CHECK: dependency-gen-pch.m.o
-// CHECK-NEXT: dependency-gen-pch.m
-// CHECK-NEXT: Inputs{{.}}module.map
-// CHECK-NEXT: diamond_top.pcm
-// CHECK-NEXT: Inputs{{.}}diamond_top.h
+// CHECK: dependency-gen-pch.m
+// CHECK: Inputs{{.}}diamond_top.h
+// CHECK: Inputs{{.}}module.map
+// CHECK: diamond_top.pcm
 
 #import "diamond_top.h"
Index: clang/test/Modules/dependency-file-symlinks.c
===
--- /dev/null
+++ clang/test/Modules/dependency-file-symlinks.c
@@ -0,0 +1,55 @@
+// REQUIRES: shell
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/cache
+
+// Set up %t as follows:
+//  * misc.h #includes target.h
+//  * target.h is empty
+//  * link.h is a symlink to target.h
+
+// RUN: cp %S/Inputs/dfg-symlinks.modulemap %t/module.modulemap
+// RUN: echo "int foo();" > %t/target.h
+// RUN: ln -s %t/target.h %t/link.h
+// RUN: echo '#include "target.h"' >> %t/misc.h
+// RUN: echo 'int foo();'  >> %t/misc.h
+
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache %s \
+// RUN:   -I %t -MT dependency-file-symlinks.o -dependency-file - | FileCheck --check-prefix=CHECK_IMP %s
+
+// Run clang again, to make sure that we get identical output with the cached
+// modules.
+
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache %s \
+// RUN:   -I %t -MT dependency-file-symlinks.o -dependency-file - | FileCheck --check-prefix=CHECK_IMP %s
+
+// Verify that the an explicit use of a module via -fmodule-file doesn't include
+// any headers from that files's modulemap.
+
+// RUN: %clang_cc1 -fmodules -fmodule-name="my_module" -emit-module \
+// RUN:   -o %t/my_module.pcm %t/module.modulemap
+
+// RUN: %clang_cc1 -fmodules -fmodule-file=%t/my_module.pcm %s -MT  \
+// RUN:   dependency-file-symlinks.o -I %t -dependency-file - | \
+// RUN:   FileCheck --check-prefix CHECK_EXP %s
+
+#include "misc.h"
+
+int main() {
+  void *p = 

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet added a comment.

Here are 15 random ones from **llvm** synced at 
`8f9fb8bab2e9b5b27fe40d700d2abe967b99fbb5`.

  lib/Target/ARM/AsmParser/ARMAsmParser.cpp:3802:31: warning: narrowing 
conversion from 'int' to 'unsigned int' 
[cppcoreguidelines-narrowing-conversions]
  tools/dsymutil/MachOUtils.cpp:330:10: warning: narrowing conversion from 
'unsigned long' to 'unsigned int' [cppcoreguidelines-narrowing-conversions]
  lib/Target/X86/X86ISelDAGToDAG.cpp:1237:14: warning: narrowing conversion 
from 'int' to 'unsigned int' [cppcoreguidelines-narrowing-conversions]
  tools/llvm-objdump/MachODump.cpp:7998:12: warning: narrowing conversion from 
'int' to 'unsigned int' [cppcoreguidelines-narrowing-conversions]
  lib/CodeGen/MachinePipeliner.cpp:3268:11: warning: narrowing conversion from 
'int' to 'unsigned int' [cppcoreguidelines-narrowing-conversions]
  lib/CodeGen/MIRParser/MIRParser.cpp:823:41: warning: narrowing conversion 
from 'int' to 'unsigned int' [cppcoreguidelines-narrowing-conversions]
  lib/Analysis/MemoryBuiltins.cpp:610:59: warning: narrowing conversion from 
'int' to 'unsigned int' [cppcoreguidelines-narrowing-conversions]
  lib/Analysis/ValueTracking.cpp:2291:21: warning: narrowing conversion from 
'unsigned long' to 'unsigned int' [cppcoreguidelines-narrowing-conversions]
  lib/Target/ARM/ARMLoadStoreOptimizer.cpp:1470:43: warning: narrowing 
conversion from 'int' to 'unsigned int' 
[cppcoreguidelines-narrowing-conversions]
  lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:2300:14: warning: narrowing 
conversion from 'int' to 'unsigned int' 
[cppcoreguidelines-narrowing-conversions]
  lib/Target/X86/X86TargetTransformInfo.cpp:2590:27: warning: narrowing 
conversion from 'int' to 'unsigned int' 
[cppcoreguidelines-narrowing-conversions]
  lib/Target/ARM/ARMFrameLowering.cpp:1770:27: warning: narrowing conversion 
from 'int' to 'unsigned int' [cppcoreguidelines-narrowing-conversions]
  lib/Target/ARM/ARMConstantIslandPass.cpp:514:22: warning: narrowing 
conversion from 'int' to 'unsigned int' 
[cppcoreguidelines-narrowing-conversions]
  lib/Transforms/Vectorize/LoopVectorize.cpp:5500:13: warning: narrowing 
conversion from 'int' to 'unsigned int' 
[cppcoreguidelines-narrowing-conversions]
  lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:1199:54: warning: narrowing 
conversion from 'int' to 'unsigned int' 
[cppcoreguidelines-narrowing-conversions]




Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:14
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/ADT/APInt.h"
+#include "llvm/ADT/APSInt.h"

Szelethus wrote:
> Is  `APInt` used anywhere?
Good catch thx.



Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:32
 
-  const auto IsFloatExpr =
-  expr(hasType(realFloatingPointType()), unless(IsCeilFloorCall));
+  const auto IsBuiltinOrEnumType = anyOf(builtinType(), enumType());
 

JonasToth wrote:
> This matcher seems no to be used, did I overlook sth?
It's a leftover indeed thx for noticing.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53488



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