[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

2018-04-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In https://reviews.llvm.org/D44883#1068400, @brooksmoses wrote:

> I have noticed two things when attempting to release LLVM with this revision 
> internally at Google:
>
> 1. It's catching real bugs, all in constructors where someone wrote "member_ 
> = member_" when they meant "member_ = member".


Nice, just like the one that caused me to write this :)

> 2. It's catching at least as many cases of tests where people are 
> intentionally testing that self-assignment doesn't corrupt the data values.
> 
>   Thus, this seems valuable but problematic, and the problems mean that 
> initially we're facing turning off -Wself-assign completely until this is 
> resolved.  That's definitely an issue, and at least means that this needs to 
> be placed under its own -W option.  And the real bugs it's finding seem to be 
> very specific, and could be found by a more-focused warning.

See all the previous disscussion about that earlier in the differential / mail 
thread..
I guess some consensus needs to be found, and then i'll implement it.

> EDIT: Looks like "var = *&var" is supposed to avoid the warning, so my 
> (now-deleted) complaints about this not having a source workaround are 
> erroneous.

See release notes.

> I still find the fact that "var = (var)" doesn't suppress this to be 
> surprising, but it's not critical.


Repository:
  rC Clang

https://reviews.llvm.org/D44883



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


[PATCH] D44948: Add diagnostic -Waggregate-ctors, "aggregate type has user-declared constructors"

2018-04-16 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone abandoned this revision.
Quuxplusone added a comment.

This patch's new home is 
https://github.com/Quuxplusone/clang/commits/aggregate-ctors — which is where I 
should have put it to begin with.


Repository:
  rC Clang

https://reviews.llvm.org/D44948



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


[PATCH] D45613: [X86] Introduce archs: goldmont-plus & tremont

2018-04-16 Thread Gabor Buella via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL330110: [X86] Introduce archs: goldmont-plus & tremont 
(authored by GBuella, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D45613

Files:
  cfe/trunk/include/clang/Basic/X86Target.def
  cfe/trunk/lib/Basic/Targets/X86.cpp
  cfe/trunk/test/Driver/x86-march.c
  cfe/trunk/test/Misc/target-invalid-cpu-note.c
  cfe/trunk/test/Preprocessor/predefined-arch-macros.c

Index: cfe/trunk/lib/Basic/Targets/X86.cpp
===
--- cfe/trunk/lib/Basic/Targets/X86.cpp
+++ cfe/trunk/lib/Basic/Targets/X86.cpp
@@ -244,6 +244,14 @@
 setFeatureEnabledImpl(Features, "fxsr", true);
 break;
 
+  case CK_Tremont:
+setFeatureEnabledImpl(Features, "cldemote", true);
+setFeatureEnabledImpl(Features, "gfni", true);
+LLVM_FALLTHROUGH;
+  case CK_GoldmontPlus:
+setFeatureEnabledImpl(Features, "rdpid", true);
+setFeatureEnabledImpl(Features, "sgx", true);
+LLVM_FALLTHROUGH;
   case CK_Goldmont:
 setFeatureEnabledImpl(Features, "sha", true);
 setFeatureEnabledImpl(Features, "rdseed", true);
@@ -930,6 +938,12 @@
   case CK_Goldmont:
 defineCPUMacros(Builder, "goldmont");
 break;
+  case CK_GoldmontPlus:
+defineCPUMacros(Builder, "goldmont_plus");
+break;
+  case CK_Tremont:
+defineCPUMacros(Builder, "tremont");
+break;
   case CK_Nehalem:
   case CK_Westmere:
   case CK_SandyBridge:
Index: cfe/trunk/include/clang/Basic/X86Target.def
===
--- cfe/trunk/include/clang/Basic/X86Target.def
+++ cfe/trunk/include/clang/Basic/X86Target.def
@@ -104,6 +104,9 @@
 PROC_ALIAS(Silvermont, "slm")
 
 PROC(Goldmont, "goldmont", PROC_64_BIT)
+PROC(GoldmontPlus, "goldmont-plus", PROC_64_BIT)
+
+PROC(Tremont, "tremont", PROC_64_BIT)
 //@}
 
 /// \name Nehalem
Index: cfe/trunk/test/Misc/target-invalid-cpu-note.c
===
--- cfe/trunk/test/Misc/target-invalid-cpu-note.c
+++ cfe/trunk/test/Misc/target-invalid-cpu-note.c
@@ -13,7 +13,7 @@
 // X86: note: valid target CPU values are: i386, i486, winchip-c6, winchip2, c3,
 // X86-SAME: i586, pentium, pentium-mmx, pentiumpro, i686, pentium2, pentium3,
 // X86-SAME: pentium3m, pentium-m, c3-2, yonah, pentium4, pentium4m, prescott,
-// X86-SAME: nocona, core2, penryn, bonnell, atom, silvermont, slm, goldmont,
+// X86-SAME: nocona, core2, penryn, bonnell, atom, silvermont, slm, goldmont, goldmont-plus, tremont,
 // X86-SAME: nehalem, corei7, westmere, sandybridge, corei7-avx, ivybridge,
 // X86-SAME: core-avx-i, haswell, core-avx2, broadwell, skylake, skylake-avx512,
 // X86-SAME: skx, cannonlake, icelake-client, icelake-server, knl, knm, lakemont, k6, k6-2, k6-3,
@@ -25,7 +25,7 @@
 // RUN: not %clang_cc1 -triple x86_64--- -target-cpu not-a-cpu -fsyntax-only %s 2>&1 | FileCheck %s --check-prefix X86_64
 // X86_64: error: unknown target CPU 'not-a-cpu'
 // X86_64: note: valid target CPU values are: nocona, core2, penryn, bonnell,
-// X86_64-SAME: atom, silvermont, slm, goldmont, nehalem, corei7, westmere,
+// X86_64-SAME: atom, silvermont, slm, goldmont, goldmont-plus, tremont, nehalem, corei7, westmere,
 // X86_64-SAME: sandybridge, corei7-avx, ivybridge, core-avx-i, haswell,
 // X86_64-SAME: core-avx2, broadwell, skylake, skylake-avx512, skx, cannonlake,
 // X86_64-SAME: icelake-client, icelake-server, knl, knm, k8, athlon64, athlon-fx, opteron, k8-sse3,
Index: cfe/trunk/test/Preprocessor/predefined-arch-macros.c
===
--- cfe/trunk/test/Preprocessor/predefined-arch-macros.c
+++ cfe/trunk/test/Preprocessor/predefined-arch-macros.c
@@ -1371,6 +1371,7 @@
 // CHECK_GLM_M64: #define __PRFCHW__ 1
 // CHECK_GLM_M64: #define __RDRND__ 1
 // CHECK_GLM_M64: #define __RDSEED__ 1
+// CHECK_GLM_M64: #define __SHA__ 1
 // CHECK_GLM_M64: #define __SSE2__ 1
 // CHECK_GLM_M64: #define __SSE3__ 1
 // CHECK_GLM_M64: #define __SSE4_1__ 1
@@ -1387,6 +1388,146 @@
 // CHECK_GLM_M64: #define __x86_64 1
 // CHECK_GLM_M64: #define __x86_64__ 1
 
+// RUN: %clang -march=goldmont-plus -m32 -E -dM %s -o - 2>&1 \
+// RUN: -target i386-unknown-linux \
+// RUN:   | FileCheck %s -check-prefix=CHECK_GLMP_M32
+// CHECK_GLMP_M32: #define __AES__ 1
+// CHECK_GLMP_M32: #define __CLFLUSHOPT__ 1
+// CHECK_GLMP_M32: #define __FSGSBASE__ 1
+// CHECK_GLMP_M32: #define __FXSR__ 1
+// CHECK_GLMP_M32: #define __MMX__ 1
+// CHECK_GLMP_M32: #define __MPX__ 1
+// CHECK_GLMP_M32: #define __PCLMUL__ 1
+// CHECK_GLMP_M32: #define __POPCNT__ 1
+// CHECK_GLMP_M32: #define __PRFCHW__ 1
+// CHECK_GLMP_M32: #define __RDPID__ 1
+// CHECK_GLMP_M32: #define __RDRND__ 1
+// CHECK_GLMP_M32: #define __RDSEED__ 1
+// CHECK_GLMP_M32: #define __SGX__ 1
+// CHECK_GLMP_M32: #define __SHA__ 1
+// CHECK_GLMP_M32: #define __SSE2__ 1
+

[PATCH] D45680: [C++2a] Add operator<=> Rewriting - Early Attempt

2018-04-16 Thread Eric Fiselier via Phabricator via cfe-commits
EricWF created this revision.
EricWF added a reviewer: rsmith.
Herald added a subscriber: mgrang.

This is a work-in-progress attempt to add `operator<=>` rewriting. It's nowhere 
close to complete, but I would like some initial feedback on the direction.

As currently implemented, rewritten and synthesized candidates are only 
partially checked for viability when they're added to the overload candidates 
(the conversion sequence from the argument type -> parameter type is checked, 
but nothing else). This can lead non-viable candidates being selected or 
causing ambiguity when final overload resolution is attempted.

The solution implemented in this patch is to fully build "potentially viable" 
candidates if they are selected or cause ambiguity. If building the candidate 
fails, it is marked as non-viable, and a new best viable function is computed.

The problem with this approach is that it's expensive and potentially wasteful. 
For example, if overload resolution results in ambiguity with `N` rewritten 
candidates with the same partial ordering, then all `N` candidates are fully 
built, and those results potentially thrown away.

For builtin candidates this can be avoided by separating out the bits of 
`CheckBinaryOperands` which compute it's result type from the bits which 
actually build the expression and convert the arguments. Once we know the 
return type of the builtin, we can deduce if the comparison category type can 
be used in the expression `0 @ ` without much effect.

However, for non-builtin overloads of `operator<=>` (which don't return a 
comparison category type), it seems non-trivial to check that `0 @ ` is 
valid without actually attempting to build the overloaded binary operator.

@rsmith Could you provide initial feedback on this direction? Or any ideas you 
have about how to best implement it?


Repository:
  rC Clang

https://reviews.llvm.org/D45680

Files:
  include/clang/Sema/Overload.h
  include/clang/Sema/Sema.h
  lib/Sema/SemaOverload.cpp
  test/SemaCXX/compare-cxx2a.cpp

Index: test/SemaCXX/compare-cxx2a.cpp
===
--- test/SemaCXX/compare-cxx2a.cpp
+++ test/SemaCXX/compare-cxx2a.cpp
@@ -293,20 +293,21 @@
 
 template 
 struct Tag {};
-// expected-note@+1 {{candidate}}
-Tag<0> operator<=>(EnumA, EnumA) {
-  return {};
+std::strong_ordering operator<=>(EnumA, EnumA) {
+  return std::strong_ordering::equal;
 }
-Tag<1> operator<=>(EnumA, EnumB) {
-  return {};
+// expected-note@+1 {{candidate function}},
+std::strong_ordering operator<=>(EnumA a, EnumB b) {
+  return ((int)a <=> (int)b);
 }
 
 void test_enum_ovl_provided() {
   auto r1 = (EnumA::A <=> EnumA::A);
-  ASSERT_EXPR_TYPE(r1, Tag<0>);
+  ASSERT_EXPR_TYPE(r1, std::strong_ordering);
   auto r2 = (EnumA::A <=> EnumB::B);
-  ASSERT_EXPR_TYPE(r2, Tag<1>);
-  (void)(EnumB::B <=> EnumA::A); // expected-error {{invalid operands to binary expression ('EnumCompareTests::EnumB' and 'EnumCompareTests::EnumA')}}
+  ASSERT_EXPR_TYPE(r2, std::strong_ordering);
+  (void)(EnumB::B <=> EnumA::A); // OK, chooses reverse order synthesized candidate.
+  (void)(EnumB::B <=> EnumC::C); // expected-error {{invalid operands to binary expression ('EnumCompareTests::EnumB' and 'EnumCompareTests::EnumC')}}
 }
 
 void enum_float_test() {
@@ -375,3 +376,60 @@
   ASSERT_EXPR_TYPE(r4, std::partial_ordering);
 
 }
+
+namespace TestRewritting {
+
+struct T {
+  int x;
+  // expected-note@+1 {{candidate}}
+  constexpr std::strong_ordering operator<=>(T y) const {
+return (x <=> y.x);
+  }
+};
+
+struct U {
+  int x;
+  // FIXME: This diagnostic is terrible.
+  // expected-note@+1 {{candidate function not viable: requires single argument 'y', but 2 arguments were provided}}
+  constexpr std::strong_equality operator<=>(T y) const {
+if (x == y.x)
+  return std::strong_equality::equal;
+return std::strong_equality::nonequal;
+  }
+};
+
+struct X { int x; };
+struct Y { int x; };
+struct Tag {};
+// expected-note@+1 2 {{candidate}}
+Tag operator<=>(X, Y) {
+  return {};
+}
+// expected-note@+1 2 {{candidate}}
+constexpr auto operator<=>(Y y, X x) {
+  return y.x <=> x.x;
+}
+
+void foo() {
+  T t{42};
+  T t2{0};
+  U u{101};
+  auto r1 = (t <=> u);
+  ASSERT_EXPR_TYPE(r1, std::strong_equality);
+  auto r2 = (t <=> t2);
+  ASSERT_EXPR_TYPE(r2, std::strong_ordering);
+
+  auto r3 = t == u;
+  ASSERT_EXPR_TYPE(r3, bool);
+
+  (void)(t < u); // expected-error {{invalid operands to binary expression ('TestRewritting::T' and 'TestRewritting::U')}}
+
+  constexpr X x{1};
+  constexpr Y y{2};
+  constexpr auto r4 = (y < x);
+  static_assert(r4 == false);
+  constexpr auto r5 = (x < y);
+  static_assert(r5 == true);
+}
+
+} // namespace TestRewritting
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -831,6 +831,29 @@
   }
 }
 
+const ImplicitConversionSequence &
+OverloadCandi

[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

2018-04-16 Thread Brooks Moses via Phabricator via cfe-commits
brooksmoses added a comment.

Some further statistics, now that I've done a full cleanup on our code:

8 actual bugs newly found by -Wself-assign-field.  (Thank you!)
2 actual bugs newly found by -Wself-assign
6 instances of redundant code newly found by -Wself-assign
90 (approximately) instances of intentional self assignments in tests that now 
need "*&" annotations.

That seems like an awfully large amount of noise for the -Wself-assign part of 
this, and I continue feeling rather dubious about whether it should be part of 
-Wall.  By contrast, the -Wself-assign-field part has been entirely true 
positives.


Repository:
  rC Clang

https://reviews.llvm.org/D44883



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


[PATCH] D43578: -ftime-report switch support in Clang

2018-04-16 Thread Michael Zolotukhin via Phabricator via cfe-commits
mzolotukhin added a comment.

> What kinds of  would be useful to you? (How do you want the 
> runtime of Clang broken down?) Are vertical slices (through all Clang's 
> various layers and potentially parts of LLVM) -- as this patch will produce 
> -- useful, or would you really want horizontal slices (as in, what part of 
> Clang is actually spending the time)? Or just anything that's basically 
> expected to be consistent from run to run, so you can identify that something 
> has slowed down, even if you don't have enough information to really know 
> what?

For me "something has slowed down" would be enough. I.e. even if "parse 
templates" would be erroneously attributed to 90% time spent in front-end, I 
would be able to see a jump from 90% to 95%. While these numbers are not 
reflecting the actual ratio, they still will indicate changes.

I would find horizontal slicing more interesting, as we can get vertical slices 
from profilers. I.e. if in LLVM profile I see that time spent in APInt::add has 
grown a lot, I'd like to know which pass started to use it more extensively - 
it's rarely the case that APInt::add slowed down itself.

> I think we need to fix the overlap issue as a prerequisite to adding timers 
> with large amounts of overlap, especially self-overlap. Otherwise the numbers 
> will likely do more harm than good, as they will significantly misattribute 
> runtime. Fortunately, I think that should only require some relatively small 
> changes to the LLVM timer infrastructure.

I definitely would support that :)

> Well, that depends on the profiler. With some profilers, you can just attach 
> a profiler to your entire compilation and then ask it what the hottest 
> functions were. But sure, if you have existing infrastructure built around 
> -ftime-report, I can see that it makes sense to reuse that infrastructure.

Yeah, that was exactly my point.

> While LLVM may have some overlap between regions, the vertical timing slices 
> are still pretty well aligned with the horizontal functionality slices. I 
> expect the problems in Clang to be a lot worse. Suppose you enter N levels of 
> parsing templates, and then trigger a whole bunch of template instantiation, 
> AST deserialization, and code generation. Let's say that takes 1s in total. 
> With this patch, I think we'd end up attributing Ns of compile time to 
> "parsing templates", which is clearly very far from the truth, but will 
> likely be listed as (by far) the slowest thing in the compilation. This does 
> not even rise to the level of "not-perfect", it's going to be actively 
> misleading in a lot of cases, and won't even necessarily point anywhere near 
> the problematic spot.
>  I think with this patch and no fix to the overlap problem, we will find 
> ourselves frequently fielding bugs where people say "X is slow" when it 
> actually isn't. Plus I think we have the opportunity to deliver something 
> that's hugely useful and not much harder to achieve (providing timing 
> information that relates back to the source code), and I'd prefer we spend 
> our complexity budget for this feature there.

I see your point, and I agree it would be really misleading if e.g the sum of 
timers won't match the total time due to self-overlaps. I think adding these 
timers still might be worthwhile as my guess is that no one usually uses them 
anyway unless they know what to expect from the reported timers, but I might be 
mistaken here. And anyway, if you think it's possible to fix the issue first, 
it's totally fine with me.

Thanks,
Michael


https://reviews.llvm.org/D43578



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


r330112 - Clean carriage returns from lib/ and include/. NFC.

2018-04-16 Thread Malcolm Parsons via cfe-commits
Author: malcolm.parsons
Date: Mon Apr 16 01:31:08 2018
New Revision: 330112

URL: http://llvm.org/viewvc/llvm-project?rev=330112&view=rev
Log:
Clean carriage returns from lib/ and include/. NFC.

Summary:
Clean carriage returns from lib/ and include/. NFC.
(I have to make this change locally in order for `git diff` to show sane output 
after I edit a file, so I might as well ask for it to be committed. I don't 
have commit privs myself.)
(Without this patch, `git rebase`ing any change involving SemaDeclCXX.cpp is a 
real nightmare. :( So while I have no right to ask for this to be committed, 
geez would it make my workflow easier if it were.)

Here's the command I used to reformat things. (Requires bash and OSX/FreeBSD 
sed.)

git grep -l $'\r' lib include | xargs sed -i -e $'s/\r//'
find lib include -name '*-e' -delete

Reviewers: malcolm.parsons

Reviewed By: malcolm.parsons

Subscribers: emaste, krytarowski, cfe-commits

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

Patch by Arthur O'Dwyer.

Modified:
cfe/trunk/lib/AST/ASTDumper.cpp
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaPseudoObject.cpp
cfe/trunk/lib/Sema/SemaTemplate.cpp
cfe/trunk/lib/Serialization/ASTWriterStmt.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp

Modified: cfe/trunk/lib/AST/ASTDumper.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDumper.cpp?rev=330112&r1=330111&r2=330112&view=diff
==
--- cfe/trunk/lib/AST/ASTDumper.cpp (original)
+++ cfe/trunk/lib/AST/ASTDumper.cpp Mon Apr 16 01:31:08 2018
@@ -2213,14 +2213,14 @@ void ASTDumper::VisitArrayInitIndexExpr(
 }
 
 void ASTDumper::VisitUnaryOperator(const UnaryOperator *Node) {
-  VisitExpr(Node);
-  OS << " " << (Node->isPostfix() ? "postfix" : "prefix")
- << " '" << UnaryOperator::getOpcodeStr(Node->getOpcode()) << "'";
-  if (!Node->canOverflow())
-OS << " cannot overflow";
-}
-
-void ASTDumper::VisitUnaryExprOrTypeTraitExpr(
+  VisitExpr(Node);
+  OS << " " << (Node->isPostfix() ? "postfix" : "prefix")
+ << " '" << UnaryOperator::getOpcodeStr(Node->getOpcode()) << "'";
+  if (!Node->canOverflow())
+OS << " cannot overflow";
+}
+
+void ASTDumper::VisitUnaryExprOrTypeTraitExpr(
 const UnaryExprOrTypeTraitExpr *Node) {
   VisitExpr(Node);
   switch(Node->getKind()) {

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=330112&r1=330111&r2=330112&view=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Mon Apr 16 01:31:08 2018
@@ -3328,12 +3328,12 @@ static bool handleAssignment(EvalInfo &I
   }
 
   CompleteObject Obj = findCompleteObject(Info, E, AK_Assign, LVal, LValType);
-  return Obj && modifySubobject(Info, E, Obj, LVal.Designator, Val);
-}
-
-namespace {
-struct CompoundAssignSubobjectHandler {
-  EvalInfo &Info;
+  return Obj && modifySubobject(Info, E, Obj, LVal.Designator, Val);
+}
+
+namespace {
+struct CompoundAssignSubobjectHandler {
+  EvalInfo &Info;
   const Expr *E;
   QualType PromotedLHSType;
   BinaryOperatorKind Opcode;
@@ -3449,13 +3449,13 @@ static bool handleCompoundAssignment(
   return Obj && findSubobject(Info, E, Obj, LVal.Designator, Handler);
 }
 
-namespace {
-struct IncDecSubobjectHandler {
-  EvalInfo &Info;
-  const UnaryOperator *E;
-  AccessKinds AccessKind;
-  APValue *Old;
-
+namespace {
+struct IncDecSubobjectHandler {
+  EvalInfo &Info;
+  const UnaryOperator *E;
+  AccessKinds AccessKind;
+  APValue *Old;
+
   typedef bool result_type;
 
   bool checkConst(QualType QT) {
@@ -3521,20 +3521,20 @@ struct IncDecSubobjectHandler {
 }
 
 bool WasNegative = Value.isNegative();
-if (AccessKind == AK_Increment) {
-  ++Value;
-
-  if (!WasNegative && Value.isNegative() && E->canOverflow()) {
-APSInt ActualValue(Value, /*IsUnsigned*/true);
-return HandleOverflow(Info, E, ActualValue, SubobjType);
-  }
-} else {
-  --Value;
-
-  if (WasNegative && !Value.isNegative() && E->canOverflow()) {
-unsigned BitWidth = Value.getBitWidth();
-APSInt ActualValue(Value.sext(BitWidth + 1), /*IsUnsigned*/false);
-ActualValue.setBit(BitWidth);
+if (AccessKind == AK_Increment) {
+  ++Value;
+
+  if (!WasNegative && Value.isNegative() && E->canOverflow()) {
+APSInt ActualValue(Value, /*IsUnsigned*/true);
+return HandleOverflow(Info, E, ActualValue, SubobjType);
+  }
+} else {
+  --Value;
+
+  if (WasNegative && !Value.isNegative() && E->canOverflow()) {
+unsigned BitWidth = Value.getBitWidth();
+APSInt ActualValue(Value.sext(BitWidth + 1), /*IsUnsigned*/false);
+ActualValue.setBit(B

[PATCH] D45591: Clean carriage returns from lib/ and include/. NFC.

2018-04-16 Thread Malcolm Parsons via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC330112: Clean carriage returns from lib/ and include/. NFC. 
(authored by malcolm.parsons, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D45591

Files:
  lib/AST/ASTDumper.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaPseudoObject.cpp
  lib/Sema/SemaTemplate.cpp
  lib/Serialization/ASTWriterStmt.cpp
  lib/StaticAnalyzer/Checkers/PaddingChecker.cpp

Index: lib/CodeGen/CGExprScalar.cpp
===
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -162,13 +162,13 @@
   // we can elide the overflow check.
   if (!Op.mayHaveIntegerOverflow())
 return true;
-
-  // If a unary op has a widened operand, the op cannot overflow.
-  if (const auto *UO = dyn_cast(Op.E))
-return !UO->canOverflow();
-
-  // We usually don't need overflow checks for binops with widened operands.
-  // Multiplication with promoted unsigned operands is a special case.
+
+  // If a unary op has a widened operand, the op cannot overflow.
+  if (const auto *UO = dyn_cast(Op.E))
+return !UO->canOverflow();
+
+  // We usually don't need overflow checks for binops with widened operands.
+  // Multiplication with promoted unsigned operands is a special case.
   const auto *BO = cast(Op.E);
   auto OptionalLHSTy = getUnwidenedIntegerType(Ctx, BO->getLHS());
   if (!OptionalLHSTy)
@@ -1871,13 +1871,13 @@
 return Builder.CreateAdd(InVal, Amount, Name);
   case LangOptions::SOB_Undefined:
 if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow))
-  return Builder.CreateNSWAdd(InVal, Amount, Name);
-// Fall through.
-  case LangOptions::SOB_Trapping:
-if (!E->canOverflow())
-  return Builder.CreateNSWAdd(InVal, Amount, Name);
-return EmitOverflowCheckedBinOp(createBinOpInfoFromIncDec(E, InVal, IsInc));
-  }
+  return Builder.CreateNSWAdd(InVal, Amount, Name);
+// Fall through.
+  case LangOptions::SOB_Trapping:
+if (!E->canOverflow())
+  return Builder.CreateNSWAdd(InVal, Amount, Name);
+return EmitOverflowCheckedBinOp(createBinOpInfoFromIncDec(E, InVal, IsInc));
+  }
   llvm_unreachable("Unknown SignedOverflowBehaviorTy");
 }
 
@@ -1953,15 +1953,15 @@
 value = Builder.getTrue();
 
   // Most common case by far: integer increment.
-  } else if (type->isIntegerType()) {
-// Note that signed integer inc/dec with width less than int can't
-// overflow because of promotion rules; we're just eliding a few steps here.
-if (E->canOverflow() && type->isSignedIntegerOrEnumerationType()) {
-  value = EmitIncDecConsiderOverflowBehavior(E, value, isInc);
-} else if (E->canOverflow() && type->isUnsignedIntegerType() &&
-   CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow)) {
-  value =
-  EmitOverflowCheckedBinOp(createBinOpInfoFromIncDec(E, value, isInc));
+  } else if (type->isIntegerType()) {
+// Note that signed integer inc/dec with width less than int can't
+// overflow because of promotion rules; we're just eliding a few steps here.
+if (E->canOverflow() && type->isSignedIntegerOrEnumerationType()) {
+  value = EmitIncDecConsiderOverflowBehavior(E, value, isInc);
+} else if (E->canOverflow() && type->isUnsignedIntegerType() &&
+   CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow)) {
+  value =
+  EmitOverflowCheckedBinOp(createBinOpInfoFromIncDec(E, value, isInc));
 } else {
   llvm::Value *amt = llvm::ConstantInt::get(value->getType(), amount, true);
   value = Builder.CreateAdd(value, amt, isInc ? "inc" : "dec");
Index: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
+++ lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
@@ -1,330 +1,330 @@
-//===- PaddingChecker.cpp *- 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 a checker that checks for padding that could be
-//  removed by re-ordering members.
-//
-//===--===//
-
-#include "ClangSACheckers.h"
-#include "clang/AST/CharUnits.h"
-#include "clang/AST/DeclTemplate.h"
-#include "clang/AST/RecordLayout.h"
-#include "clang/AST/RecursiveASTVisitor.h"
-#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
-#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
-#include "clang/StaticAnalyzer/Core/Checker.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
-#include "llvm/ADT/SmallString.h

[PATCH] D43578: -ftime-report switch support in Clang

2018-04-16 Thread Andrew V. Tischenko via Phabricator via cfe-commits
avt77 added a comment.

In https://reviews.llvm.org/D43578#1067995, @rsmith wrote:

> I think https://reviews.llvm.org/D45619 is a good change, and I'd like to see 
> that get committed.


Could you give me LGTM in this case? I'm going to publish "recursive"(ovelaped) 
timers but I'd like to base it on https://reviews.llvm.org/D45619.


https://reviews.llvm.org/D43578



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


[PATCH] D43578: -ftime-report switch support in Clang

2018-04-16 Thread Andrew V. Tischenko via Phabricator via cfe-commits
avt77 added a comment.

In https://reviews.llvm.org/D43578#1068417, @rsmith wrote:

> I think we need to fix the overlap issue as a prerequisite to adding timers 
> with large amounts of overlap, especially self-overlap. Otherwise the numbers 
> will likely do more harm than good, as they will significantly misattribute 
> runtime. Fortunately, I think that should only require some relatively small 
> changes to the LLVM timer infrastructure.


JFYI: the given patch introduces non-recursive (self-overlaps) timers only. I'm 
going to introduce self-overlaps timers when  https://reviews.llvm.org/D45619 
is committed.


https://reviews.llvm.org/D43578



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


[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

2018-04-16 Thread Brooks Moses via Phabricator via cfe-commits
brooksmoses added a comment.

A further concern about this in the general case from the reviewer of one of my 
test-cleanup changes: The "var = *&var" idiom is not necessarily equivalent to 
"var = var" in cases of user-defined types, because operator& may be overloaded.


Repository:
  rC Clang

https://reviews.llvm.org/D44883



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


[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

2018-04-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

Re false-positives - at least two [post-]reviewers need to agree on the way 
forward (see previous comments, mail thread), and then i will implement it.

In https://reviews.llvm.org/D44883#1068576, @brooksmoses wrote:

> A further concern about this in the general case from the reviewer of one of 
> my test-cleanup changes: The "var = *&var" idiom is not necessarily 
> equivalent to "var = var" in cases of user-defined types, because operator& 
> may be overloaded.


Release notes state:

  If you are doing such an assignment intentionally, e.g. in a unit test for
  a data structure, the warning can be suppressed by adding ``*&`` to the
  right-hand side or casting it to the appropriate reference type.

So it could also be `var = static_cast(var);`


Repository:
  rC Clang

https://reviews.llvm.org/D44883



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


[PATCH] D45578: Add a command line option `fregister_global_dtors_with_atexit` to register destructor functions annotated with __attribute__((destructor)) using __cxa_atexit or atexit.

2018-04-16 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

Thank you.  Minor request and then LGTM.




Comment at: lib/CodeGen/ItaniumCXXABI.cpp:2202
+// __attribute__((destructor)) in a constructor function.
+addr = llvm::Constant::getNullValue(CGF.Int8PtrTy);
+

You should probably mention that using null here is okay because this value is 
just passed back as an argument to the destructor function.


Repository:
  rC Clang

https://reviews.llvm.org/D45578



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


[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

2018-04-16 Thread Chandler Carruth via Phabricator via cfe-commits
chandlerc added a comment.

These kinds of improvements to warnings are awesome, but the way we are 
deploying them presents serious challenges to adoption which I think we need to 
address.

I think significant new warning functionality should as a matter of course go 
behind some warning group at least initially. I'd be fine with this being a 
generic "-beta" group or some such, and I would be very happy with a clear 
timeline for merging some of these flags away. But I think we need to give 
users of Clang the ability to stage the deployment of a new compiler and the 
cleanup of their code more effectively.

Secondly, there seems to be some interest in splitting off `OP=` combined 
assignement-and-functionality warnings, but delays around getting consensus. 
While we wait, users (including us) are forced to work around a problematic 
warning. This is pretty disruptive. I think we need to be much more rapid it 
moving this into a separate flag while it is under discussion because we don't 
have anything like the above policy in place and well followed, we are 
currently facing a really bad choice: disable the *entire* warning and 
potentially regress our codebase, or commit really ugly hacks to work around a 
bad warning. We can probably make it through this for this warning, but we need 
to prioritize cleaning this up because it didn't go behind any kind of staging, 
and going forward i strongly think we need a better way of staging these kinds 
of improvements.

Note that this isn't novel -- the thread-safety warnings created exactly such a 
staging arrangement to help address these kinds of rollout issues.


Repository:
  rC Clang

https://reviews.llvm.org/D44883



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


[PATCH] D45407: [StaticAnalyzer] Added notes to the plist output

2018-04-16 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment.

I just had a look with `-analyzer-config notes-as-events=true`, and it works 
(with CodeChecker, at least). I'd still implement a better support for notes, 
but this is a great workaround for now. Thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D45407



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


[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

2018-04-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

There are several options:

1. @rjmccall's idea: `-wtest` (lowercase), which in this case will disable that 
new code in `BuildOverloadedBinOp()`. i quite like it actually.
2. split it up like i had in the first revision - ``-Wself-assign-builtin``, 
``-Wself-assign-field-builtin``; ``-Wself-assign-overloaded``, 
``-Wself-assign-field-overloaded``
  - we could just assume that `BuildOverloadedBinOp()` implies overloaded,
  - or check that the particular operator is non-trivial
3. ???

@rjmccall, @thakis, @dblaikie, @aaron.ballman, @brooksmoses, @chandlerc

I'm going to go ahead and look into `1.`, since it does not seem there will be 
any consensus in a timely manner.


Repository:
  rC Clang

https://reviews.llvm.org/D44883



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


[PATCH] D44381: [mips] Prevent PIC to be set to level 2

2018-04-16 Thread Aleksandar Beserminji via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC330118: [mips] Prevent PIC to be set to level 2 (authored by 
abeserminji, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D44381

Files:
  lib/Driver/ToolChains/CommonArgs.cpp
  test/Driver/pic.c


Index: test/Driver/pic.c
===
--- test/Driver/pic.c
+++ test/Driver/pic.c
@@ -292,9 +292,9 @@
 // RUN: %clang -c %s -target mipsel-linux-android14 -### 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-PIC1
 // RUN: %clang -c %s -target mipsel-linux-android16 -### 2>&1 \
-// RUN:   | FileCheck %s --check-prefix=CHECK-PIE2
+// RUN:   | FileCheck %s --check-prefix=CHECK-PIE1
 // RUN: %clang -c %s -target mipsel-linux-android24 -### 2>&1 \
-// RUN:   | FileCheck %s --check-prefix=CHECK-PIE2
+// RUN:   | FileCheck %s --check-prefix=CHECK-PIE1
 //
 // 64-bit Android targets are always PIE.
 // RUN: %clang -c %s -target aarch64-linux-android -### 2>&1 \
Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -1007,12 +1007,17 @@
   if ((ROPI || RWPI) && (PIC || PIE))
 ToolChain.getDriver().Diag(diag::err_drv_ropi_rwpi_incompatible_with_pic);
 
-  // When targettng MIPS64 with N64, the default is PIC, unless -mno-abicalls 
is
-  // used.
-  if ((Triple.getArch() == llvm::Triple::mips64 ||
-   Triple.getArch() == llvm::Triple::mips64el) &&
-  Args.hasArg(options::OPT_mno_abicalls))
-return std::make_tuple(llvm::Reloc::Static, 0U, false);
+  if (Triple.getArch() == llvm::Triple::mips ||
+   Triple.getArch() == llvm::Triple::mipsel ||
+   Triple.getArch() == llvm::Triple::mips64 ||
+   Triple.getArch() == llvm::Triple::mips64el) {
+// When targettng MIPS with -mno-abicalls, it's always static.
+if(Args.hasArg(options::OPT_mno_abicalls))
+  return std::make_tuple(llvm::Reloc::Static, 0U, false);
+// Unlike other architectures, MIPS, even with -fPIC/-mxgot/multigot,
+// does not use PIC level 2 for historical reasons.
+IsPICLevelTwo = false;
+  }
 
   if (PIC)
 return std::make_tuple(llvm::Reloc::PIC_, IsPICLevelTwo ? 2U : 1U, PIE);


Index: test/Driver/pic.c
===
--- test/Driver/pic.c
+++ test/Driver/pic.c
@@ -292,9 +292,9 @@
 // RUN: %clang -c %s -target mipsel-linux-android14 -### 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-PIC1
 // RUN: %clang -c %s -target mipsel-linux-android16 -### 2>&1 \
-// RUN:   | FileCheck %s --check-prefix=CHECK-PIE2
+// RUN:   | FileCheck %s --check-prefix=CHECK-PIE1
 // RUN: %clang -c %s -target mipsel-linux-android24 -### 2>&1 \
-// RUN:   | FileCheck %s --check-prefix=CHECK-PIE2
+// RUN:   | FileCheck %s --check-prefix=CHECK-PIE1
 //
 // 64-bit Android targets are always PIE.
 // RUN: %clang -c %s -target aarch64-linux-android -### 2>&1 \
Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -1007,12 +1007,17 @@
   if ((ROPI || RWPI) && (PIC || PIE))
 ToolChain.getDriver().Diag(diag::err_drv_ropi_rwpi_incompatible_with_pic);
 
-  // When targettng MIPS64 with N64, the default is PIC, unless -mno-abicalls is
-  // used.
-  if ((Triple.getArch() == llvm::Triple::mips64 ||
-   Triple.getArch() == llvm::Triple::mips64el) &&
-  Args.hasArg(options::OPT_mno_abicalls))
-return std::make_tuple(llvm::Reloc::Static, 0U, false);
+  if (Triple.getArch() == llvm::Triple::mips ||
+   Triple.getArch() == llvm::Triple::mipsel ||
+   Triple.getArch() == llvm::Triple::mips64 ||
+   Triple.getArch() == llvm::Triple::mips64el) {
+// When targettng MIPS with -mno-abicalls, it's always static.
+if(Args.hasArg(options::OPT_mno_abicalls))
+  return std::make_tuple(llvm::Reloc::Static, 0U, false);
+// Unlike other architectures, MIPS, even with -fPIC/-mxgot/multigot,
+// does not use PIC level 2 for historical reasons.
+IsPICLevelTwo = false;
+  }
 
   if (PIC)
 return std::make_tuple(llvm::Reloc::PIC_, IsPICLevelTwo ? 2U : 1U, PIE);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D44381: [mips] Prevent PIC to be set to level 2

2018-04-16 Thread Aleksandar Beserminji via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL330118: [mips] Prevent PIC to be set to level 2 (authored by 
abeserminji, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D44381?vs=142406&id=142609#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D44381

Files:
  cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
  cfe/trunk/test/Driver/pic.c


Index: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1007,12 +1007,17 @@
   if ((ROPI || RWPI) && (PIC || PIE))
 ToolChain.getDriver().Diag(diag::err_drv_ropi_rwpi_incompatible_with_pic);
 
-  // When targettng MIPS64 with N64, the default is PIC, unless -mno-abicalls 
is
-  // used.
-  if ((Triple.getArch() == llvm::Triple::mips64 ||
-   Triple.getArch() == llvm::Triple::mips64el) &&
-  Args.hasArg(options::OPT_mno_abicalls))
-return std::make_tuple(llvm::Reloc::Static, 0U, false);
+  if (Triple.getArch() == llvm::Triple::mips ||
+   Triple.getArch() == llvm::Triple::mipsel ||
+   Triple.getArch() == llvm::Triple::mips64 ||
+   Triple.getArch() == llvm::Triple::mips64el) {
+// When targettng MIPS with -mno-abicalls, it's always static.
+if(Args.hasArg(options::OPT_mno_abicalls))
+  return std::make_tuple(llvm::Reloc::Static, 0U, false);
+// Unlike other architectures, MIPS, even with -fPIC/-mxgot/multigot,
+// does not use PIC level 2 for historical reasons.
+IsPICLevelTwo = false;
+  }
 
   if (PIC)
 return std::make_tuple(llvm::Reloc::PIC_, IsPICLevelTwo ? 2U : 1U, PIE);
Index: cfe/trunk/test/Driver/pic.c
===
--- cfe/trunk/test/Driver/pic.c
+++ cfe/trunk/test/Driver/pic.c
@@ -292,9 +292,9 @@
 // RUN: %clang -c %s -target mipsel-linux-android14 -### 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-PIC1
 // RUN: %clang -c %s -target mipsel-linux-android16 -### 2>&1 \
-// RUN:   | FileCheck %s --check-prefix=CHECK-PIE2
+// RUN:   | FileCheck %s --check-prefix=CHECK-PIE1
 // RUN: %clang -c %s -target mipsel-linux-android24 -### 2>&1 \
-// RUN:   | FileCheck %s --check-prefix=CHECK-PIE2
+// RUN:   | FileCheck %s --check-prefix=CHECK-PIE1
 //
 // 64-bit Android targets are always PIE.
 // RUN: %clang -c %s -target aarch64-linux-android -### 2>&1 \


Index: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1007,12 +1007,17 @@
   if ((ROPI || RWPI) && (PIC || PIE))
 ToolChain.getDriver().Diag(diag::err_drv_ropi_rwpi_incompatible_with_pic);
 
-  // When targettng MIPS64 with N64, the default is PIC, unless -mno-abicalls is
-  // used.
-  if ((Triple.getArch() == llvm::Triple::mips64 ||
-   Triple.getArch() == llvm::Triple::mips64el) &&
-  Args.hasArg(options::OPT_mno_abicalls))
-return std::make_tuple(llvm::Reloc::Static, 0U, false);
+  if (Triple.getArch() == llvm::Triple::mips ||
+   Triple.getArch() == llvm::Triple::mipsel ||
+   Triple.getArch() == llvm::Triple::mips64 ||
+   Triple.getArch() == llvm::Triple::mips64el) {
+// When targettng MIPS with -mno-abicalls, it's always static.
+if(Args.hasArg(options::OPT_mno_abicalls))
+  return std::make_tuple(llvm::Reloc::Static, 0U, false);
+// Unlike other architectures, MIPS, even with -fPIC/-mxgot/multigot,
+// does not use PIC level 2 for historical reasons.
+IsPICLevelTwo = false;
+  }
 
   if (PIC)
 return std::make_tuple(llvm::Reloc::PIC_, IsPICLevelTwo ? 2U : 1U, PIE);
Index: cfe/trunk/test/Driver/pic.c
===
--- cfe/trunk/test/Driver/pic.c
+++ cfe/trunk/test/Driver/pic.c
@@ -292,9 +292,9 @@
 // RUN: %clang -c %s -target mipsel-linux-android14 -### 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-PIC1
 // RUN: %clang -c %s -target mipsel-linux-android16 -### 2>&1 \
-// RUN:   | FileCheck %s --check-prefix=CHECK-PIE2
+// RUN:   | FileCheck %s --check-prefix=CHECK-PIE1
 // RUN: %clang -c %s -target mipsel-linux-android24 -### 2>&1 \
-// RUN:   | FileCheck %s --check-prefix=CHECK-PIE2
+// RUN:   | FileCheck %s --check-prefix=CHECK-PIE1
 //
 // 64-bit Android targets are always PIE.
 // RUN: %clang -c %s -target aarch64-linux-android -### 2>&1 \
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45680: [C++2a] Add operator<=> Rewriting - Early Attempt

2018-04-16 Thread Eric Fiselier via Phabricator via cfe-commits
EricWF added a comment.

I'm guessing this implementation is super non-conforming, since expressions are 
evaluated during overload resolution.

My plan is to keep the general shape of this patch intact, but fix the eager 
building of expressions with something less wasteful.


Repository:
  rC Clang

https://reviews.llvm.org/D45680



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


[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

2018-04-16 Thread Bart Peeters via Phabricator via cfe-commits
bartpeeters added a comment.

We too would like this feature to be in the official release


Repository:
  rL LLVM

https://reviews.llvm.org/D28462



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


[PATCH] D45284: [RISCV] More validations on the input value of -march=

2018-04-16 Thread Alex Bradbury via Phabricator via cfe-commits
asb added inline comments.



Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:50
+
+static void getExtensionVersion(StringRef In, std::string &Version) {
+  auto I = In.begin();

You should probably document the limitation that this doesn't currently parse 
minor versions e.g. i2p0.



Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:144-146
+  // Currently LLVM does not support 'e'.
+  D.Diag(diag::err_drv_invalid_riscv_arch_name)
+<< MArch << "unsupported standard user-level extension 'e'";

This could be tightened up by also rejected rv64e as invalid.



Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:201-202
+  if (StdExtsItr == StdExtsEnd) {
+size_t Pos;
+if (hasExtension(StdExts, std::string(1, c), Pos)) {
+  D.Diag(diag::err_drv_invalid_riscv_ext_arch_name)

I'd suggest either just using StringRef::contains and getting rid of 
hasExtension, or adding a doc comment to hasExtension to explain its semantics.

It might also be worth adding a comment to explain why you want to check the 
extension is present in the StdExts string (e.g. We have reached the end of the 
StdExts string. Either the current extension was given outside of the canonical 
order (in which case issue an error), or else no canonical ordering is defined 
meaning no error should be generated'.



Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:211
   // Move to next char to prevent repeated letter.
   ++StdExtsItr;
 

Won't this now iterate StdExtsItr past StdExtsEnd if StdExtsItr == StdExtsEnd 
but the hasExtension call is false?



Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:265-267
 if (HasD && !HasF)
-  D.Diag(diag::err_drv_invalid_arch_name) << MArch;
+  D.Diag(diag::err_drv_invalid_riscv_arch_name) << MArch
+<< "d requires f extension to also be specified";

Add a TODO about other dependencies perhaps? e.g. ef and efd are invalid, and q 
requires rv64.


https://reviews.llvm.org/D45284



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


[PATCH] D45679: [clang-tidy] Add a new check, readability-unmodified-non-const-variable, that finds declarations of non-const variables that never get modified.

2018-04-16 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

Hi @shuaiwang, i am currently working on the same check and we should definilty 
coordinate our work. :)

What do you think about joining the forces and to work together on such a check?

Things i would like to mention:

- i differentiate between values and handles. I want to be able to transform to 
`const int * const`  if possible, similar for references
- i believe you do not test on all occasion, a variable can not be const, e.g. 
if it is casted, if it is catched by a lambda, array access, access through 
member functions and object members

Did you run the check over a codebase like llvm yet and what did you find?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45679



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


[PATCH] D45679: [clang-tidy] Add a new check, readability-unmodified-non-const-variable, that finds declarations of non-const variables that never get modified.

2018-04-16 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments.



Comment at: clang-tidy/utils/ASTUtils.cpp:85
+  // LHS of any assignment operators.
+  const auto AsAssignmentLhs = binaryOperator(
+  anyOf(hasOperatorName("="), hasOperatorName("+="), hasOperatorName("-="),

there is a `util::isAssignmentOperator` in clang-tidy already.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45679



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


[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

2018-04-16 Thread Daniel Krupp via Phabricator via cfe-commits
dkrupp added a comment.

Would be interesting to extend this checker (maybe in an upcoming patch) to 
report on uninitialized members not only in constructors, but also copy 
constructors and move constructors.

See related https://bugs.llvm.org/show_bug.cgi?id=37086

This bug report also mentions assignment operator. But for that a warning may 
be not so useful. In that case the members of the assigned to object should 
have some initialized value already which the programmer may not want to 
overwrite in the assignment operator.


https://reviews.llvm.org/D45532



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


[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

2018-04-16 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

> Coming from https://reviews.llvm.org/D45679, which I should have sent out 
> much earlier to get in front of you :p
> 
> Kidding aside this check is more mature than mine and I'm happy to help 
> incorporate mine into this one.

I would love to, yours is more elegant :)

> I do have a strong opinion that requires non-trivial refactoring of your diff 
> though: having a standalone reusable helper function like isModified(Expr, 
> Stmt), that checks whether Expr is (potentially) modified within Stmt, is 
> something I believe to be quite useful:

Yes please!

>   performance/{ForRangeCopyCheck, UnnecessaryCopyInitialization} are using a 
> much less sophisticated isOnlyUsedAsConst(), and can benefit from a more 
> powerful one.

I did not think about such possibilities, but maybe other analysis task could 
benefit too? Warnings?

>   I would imagine things could get messier if this check expands to also 
> check for turning member functions const: it's basically checking 
> CxxThisExpr, being a handle, is not modified within a member function, but 
> note there's no VarDecl for "this".

Using my approach, you can check if any member variable is used as non-const. 
Then mark this method as const, if it is not virtual.
Similar for member variables: private non-consts can be converted into consts.

>   It's cleaner to follow (non-const) reference chains and only mark a decl as 
> "can't be const" if everything on the chain is not modified, avoiding false 
> negatives when a variable is bound to a never-modified 
>   non-const reference.

For warning only yes. But i want automatic code transformation which will break 
such code if there are non-const references taken. Having it in one pass is 
certainly nice. With my approach multiple runs are required if the handles are 
marked as const.

>   It's also cleaner to follow member access chains, e.g. "v.a.b.c = 10" 
> modifies "v". This one is particularly tricky because the modifying behavior 
> happens at "c = 10" while the variable we'd like to mark as "can't be const" 
> is arbitrary layers away, and MemberExpr alone doesn't warrant marking "can't 
> be const" because that'll introduce way too many false negatives, ... and I 
> haven't figure out a way to write a matcher to match 
> memberExpr(hasObjectExpression(memberExpr(hasObjectExpression(...  layers deep> ... (VarDeclRef) ...), not to mention any layer could either 
> member access or array subscript access.

Does your approach work with the nesting? Maybe something recursive or 
`hasDescendent(modifying)` could do it?

Is read your comment on this check later, i first saw your new check. Maybe we 
should discuss only under one of both :) (which one is not important for me :))


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45444



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


[PATCH] D44568: Fix emission of phony dependency targets when adding extra deps

2018-04-16 Thread David Stenberg via Phabricator via cfe-commits
dstenb added a comment.

Ping.


Repository:
  rC Clang

https://reviews.llvm.org/D44568



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


[PATCH] D45668: [NEON] Define vget_high_f16() and vget_low_f16() intrinsics in AArch64 mode only

2018-04-16 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added a comment.

Not really familiar with these 2 intrinsics, I had a quick look at the ACLE:

> T vget_high_ST(T 2 a);
>  T vget_low_ST(T 2 a);
> 
> Gets the high, or low, half of a 128-bit vector. There are 24 intrinsics. 
> ARMv8
>  adds 4 more intrinsics for 128-bit vectors with float64_t and poly64_t lane
>  type.

I don't read here that they are unavailable in AArch32. Have I missed something?


https://reviews.llvm.org/D45668



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


[PATCH] D45682: [analyzer] Move `TaintBugVisitor` from `GenericTaintChecker.cpp` to `BugReporterVisitors.h`.

2018-04-16 Thread Henry Wong via Phabricator via cfe-commits
MTC created this revision.
MTC added reviewers: NoQ, george.karpenkov, xazax.hun.
Herald added subscribers: cfe-commits, a.sidorin, rnkovacs, szepet.

`TaintBugVisitor` is a universal visitor, and many checkers rely on it, such as 
`ArrayBoundCheckerV2.cpp`, `DivZeroChecker.cpp` and `VLASizeChecker.cpp`. 
Moving `TaintBugVisitor` to `BugReporterVisitors.h` enables other checker can 
also track where `tainted` value came from.


Repository:
  rC Clang

https://reviews.llvm.org/D45682

Files:
  include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -2333,3 +2333,24 @@
 
   return std::move(Piece);
 }
+
+std::shared_ptr
+TaintBugVisitor::VisitNode(const ExplodedNode *N, const ExplodedNode *PrevN,
+   BugReporterContext &BRC, BugReport &BR) {
+
+  // Find the ExplodedNode where the taint was first introduced
+  if (!N->getState()->isTainted(V) || PrevN->getState()->isTainted(V))
+return nullptr;
+
+  const Stmt *S = PathDiagnosticLocation::getStmt(N);
+  if (!S)
+return nullptr;
+
+  const LocationContext *NCtx = N->getLocationContext();
+  PathDiagnosticLocation L =
+  PathDiagnosticLocation::createBegin(S, BRC.getSourceManager(), NCtx);
+  if (!L.isValid() || !L.asLocation().isValid())
+return nullptr;
+
+  return std::make_shared(L, "Taint originated here");
+}
Index: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -100,23 +100,6 @@
   bool generateReportIfTainted(const Expr *E, const char Msg[],
CheckerContext &C) const;
 
-  /// The bug visitor prints a diagnostic message at the location where a given
-  /// variable was tainted.
-  class TaintBugVisitor
-  : public BugReporterVisitorImpl {
-  private:
-const SVal V;
-
-  public:
-TaintBugVisitor(const SVal V) : V(V) {}
-void Profile(llvm::FoldingSetNodeID &ID) const override { ID.Add(V); }
-
-std::shared_ptr VisitNode(const ExplodedNode *N,
-   const ExplodedNode *PrevN,
-   BugReporterContext &BRC,
-   BugReport &BR) override;
-  };
-
   typedef SmallVector ArgVector;
 
   /// \brief A struct used to specify taint propagation rules for a function.
@@ -214,28 +197,6 @@
 /// points to data, which should be tainted on return.
 REGISTER_SET_WITH_PROGRAMSTATE(TaintArgsOnPostVisit, unsigned)
 
-std::shared_ptr
-GenericTaintChecker::TaintBugVisitor::VisitNode(const ExplodedNode *N,
-const ExplodedNode *PrevN, BugReporterContext &BRC, BugReport &BR) {
-
-  // Find the ExplodedNode where the taint was first introduced
-  if (!N->getState()->isTainted(V) || PrevN->getState()->isTainted(V))
-return nullptr;
-
-  const Stmt *S = PathDiagnosticLocation::getStmt(N);
-  if (!S)
-return nullptr;
-
-  const LocationContext *NCtx = N->getLocationContext();
-  PathDiagnosticLocation L =
-  PathDiagnosticLocation::createBegin(S, BRC.getSourceManager(), NCtx);
-  if (!L.isValid() || !L.asLocation().isValid())
-return nullptr;
-
-  return std::make_shared(
-  L, "Taint originated here");
-}
-
 GenericTaintChecker::TaintPropagationRule
 GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule(
  const FunctionDecl *FDecl,
Index: include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
===
--- include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
+++ include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
@@ -343,6 +343,22 @@
  BugReport &BR) override;
 };
 
+/// The bug visitor prints a diagnostic message at the location where a given
+/// variable was tainted.
+class TaintBugVisitor final : public BugReporterVisitorImpl {
+private:
+  const SVal V;
+
+public:
+  TaintBugVisitor(const SVal V) : V(V) {}
+  void Profile(llvm::FoldingSetNodeID &ID) const override { ID.Add(V); }
+
+  std::shared_ptr VisitNode(const ExplodedNode *N,
+ const ExplodedNode *PrevN,
+ BugReporterContext &BRC,
+ BugReport &BR) override;
+};
+
 namespace bugreporter {
 
 /// Attempts to add visitors to trace a null or undefined value back to its
___

r330118 - [mips] Prevent PIC to be set to level 2

2018-04-16 Thread Aleksandar Beserminji via cfe-commits
Author: abeserminji
Date: Mon Apr 16 03:21:24 2018
New Revision: 330118

URL: http://llvm.org/viewvc/llvm-project?rev=330118&view=rev
Log:
[mips] Prevent PIC to be set to level 2

MIPS does not use PIC level 2 for historical reasons,
even with -fPIC/-mxgot/multigot options. This patch
prevents PIC to be set to level 2 for MIPS.

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


Modified:
cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
cfe/trunk/test/Driver/pic.c

Modified: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp?rev=330118&r1=330117&r2=330118&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp Mon Apr 16 03:21:24 2018
@@ -1007,12 +1007,17 @@ tools::ParsePICArgs(const ToolChain &Too
   if ((ROPI || RWPI) && (PIC || PIE))
 ToolChain.getDriver().Diag(diag::err_drv_ropi_rwpi_incompatible_with_pic);
 
-  // When targettng MIPS64 with N64, the default is PIC, unless -mno-abicalls 
is
-  // used.
-  if ((Triple.getArch() == llvm::Triple::mips64 ||
-   Triple.getArch() == llvm::Triple::mips64el) &&
-  Args.hasArg(options::OPT_mno_abicalls))
-return std::make_tuple(llvm::Reloc::Static, 0U, false);
+  if (Triple.getArch() == llvm::Triple::mips ||
+   Triple.getArch() == llvm::Triple::mipsel ||
+   Triple.getArch() == llvm::Triple::mips64 ||
+   Triple.getArch() == llvm::Triple::mips64el) {
+// When targettng MIPS with -mno-abicalls, it's always static.
+if(Args.hasArg(options::OPT_mno_abicalls))
+  return std::make_tuple(llvm::Reloc::Static, 0U, false);
+// Unlike other architectures, MIPS, even with -fPIC/-mxgot/multigot,
+// does not use PIC level 2 for historical reasons.
+IsPICLevelTwo = false;
+  }
 
   if (PIC)
 return std::make_tuple(llvm::Reloc::PIC_, IsPICLevelTwo ? 2U : 1U, PIE);

Modified: cfe/trunk/test/Driver/pic.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/pic.c?rev=330118&r1=330117&r2=330118&view=diff
==
--- cfe/trunk/test/Driver/pic.c (original)
+++ cfe/trunk/test/Driver/pic.c Mon Apr 16 03:21:24 2018
@@ -292,9 +292,9 @@
 // RUN: %clang -c %s -target mipsel-linux-android14 -### 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-PIC1
 // RUN: %clang -c %s -target mipsel-linux-android16 -### 2>&1 \
-// RUN:   | FileCheck %s --check-prefix=CHECK-PIE2
+// RUN:   | FileCheck %s --check-prefix=CHECK-PIE1
 // RUN: %clang -c %s -target mipsel-linux-android24 -### 2>&1 \
-// RUN:   | FileCheck %s --check-prefix=CHECK-PIE2
+// RUN:   | FileCheck %s --check-prefix=CHECK-PIE1
 //
 // 64-bit Android targets are always PIE.
 // RUN: %clang -c %s -target aarch64-linux-android -### 2>&1 \


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


r330110 - [X86] Introduce archs: goldmont-plus & tremont

2018-04-16 Thread Gabor Buella via cfe-commits
Author: gbuella
Date: Mon Apr 16 01:10:10 2018
New Revision: 330110

URL: http://llvm.org/viewvc/llvm-project?rev=330110&view=rev
Log:
[X86] Introduce archs: goldmont-plus & tremont

Reviewers: craig.topper

Reviewed By: craig.topper

Subscribers: cfe-commits

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

Modified:
cfe/trunk/include/clang/Basic/X86Target.def
cfe/trunk/lib/Basic/Targets/X86.cpp
cfe/trunk/test/Driver/x86-march.c
cfe/trunk/test/Misc/target-invalid-cpu-note.c
cfe/trunk/test/Preprocessor/predefined-arch-macros.c

Modified: cfe/trunk/include/clang/Basic/X86Target.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/X86Target.def?rev=330110&r1=330109&r2=330110&view=diff
==
--- cfe/trunk/include/clang/Basic/X86Target.def (original)
+++ cfe/trunk/include/clang/Basic/X86Target.def Mon Apr 16 01:10:10 2018
@@ -104,6 +104,9 @@ PROC_WITH_FEAT(Silvermont, "silvermont",
 PROC_ALIAS(Silvermont, "slm")
 
 PROC(Goldmont, "goldmont", PROC_64_BIT)
+PROC(GoldmontPlus, "goldmont-plus", PROC_64_BIT)
+
+PROC(Tremont, "tremont", PROC_64_BIT)
 //@}
 
 /// \name Nehalem

Modified: cfe/trunk/lib/Basic/Targets/X86.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.cpp?rev=330110&r1=330109&r2=330110&view=diff
==
--- cfe/trunk/lib/Basic/Targets/X86.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/X86.cpp Mon Apr 16 01:10:10 2018
@@ -244,6 +244,14 @@ bool X86TargetInfo::initFeatureMap(
 setFeatureEnabledImpl(Features, "fxsr", true);
 break;
 
+  case CK_Tremont:
+setFeatureEnabledImpl(Features, "cldemote", true);
+setFeatureEnabledImpl(Features, "gfni", true);
+LLVM_FALLTHROUGH;
+  case CK_GoldmontPlus:
+setFeatureEnabledImpl(Features, "rdpid", true);
+setFeatureEnabledImpl(Features, "sgx", true);
+LLVM_FALLTHROUGH;
   case CK_Goldmont:
 setFeatureEnabledImpl(Features, "sha", true);
 setFeatureEnabledImpl(Features, "rdseed", true);
@@ -930,6 +938,12 @@ void X86TargetInfo::getTargetDefines(con
   case CK_Goldmont:
 defineCPUMacros(Builder, "goldmont");
 break;
+  case CK_GoldmontPlus:
+defineCPUMacros(Builder, "goldmont_plus");
+break;
+  case CK_Tremont:
+defineCPUMacros(Builder, "tremont");
+break;
   case CK_Nehalem:
   case CK_Westmere:
   case CK_SandyBridge:

Modified: cfe/trunk/test/Driver/x86-march.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/x86-march.c?rev=330110&r1=330109&r2=330110&view=diff
==
--- cfe/trunk/test/Driver/x86-march.c (original)
+++ cfe/trunk/test/Driver/x86-march.c Mon Apr 16 01:10:10 2018
@@ -84,6 +84,14 @@
 // RUN:   | FileCheck %s -check-prefix=goldmont
 // goldmont: "-target-cpu" "goldmont"
 //
+// RUN: %clang -target x86_64-unknown-unknown -c -### %s -march=goldmont-plus 
2>&1 \
+// RUN:   | FileCheck %s -check-prefix=goldmont-plus
+// goldmont-plus: "-target-cpu" "goldmont-plus"
+//
+// RUN: %clang -target x86_64-unknown-unknown -c -### %s -march=tremont 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=tremont
+// tremont: "-target-cpu" "tremont"
+//
 // RUN: %clang -target x86_64-unknown-unknown -c -### %s -march=k8 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=k8
 // k8: "-target-cpu" "k8"

Modified: cfe/trunk/test/Misc/target-invalid-cpu-note.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/target-invalid-cpu-note.c?rev=330110&r1=330109&r2=330110&view=diff
==
--- cfe/trunk/test/Misc/target-invalid-cpu-note.c (original)
+++ cfe/trunk/test/Misc/target-invalid-cpu-note.c Mon Apr 16 01:10:10 2018
@@ -13,7 +13,7 @@
 // X86: note: valid target CPU values are: i386, i486, winchip-c6, winchip2, 
c3,
 // X86-SAME: i586, pentium, pentium-mmx, pentiumpro, i686, pentium2, pentium3,
 // X86-SAME: pentium3m, pentium-m, c3-2, yonah, pentium4, pentium4m, prescott,
-// X86-SAME: nocona, core2, penryn, bonnell, atom, silvermont, slm, goldmont,
+// X86-SAME: nocona, core2, penryn, bonnell, atom, silvermont, slm, goldmont, 
goldmont-plus, tremont,
 // X86-SAME: nehalem, corei7, westmere, sandybridge, corei7-avx, ivybridge,
 // X86-SAME: core-avx-i, haswell, core-avx2, broadwell, skylake, 
skylake-avx512,
 // X86-SAME: skx, cannonlake, icelake-client, icelake-server, knl, knm, 
lakemont, k6, k6-2, k6-3,
@@ -25,7 +25,7 @@
 // RUN: not %clang_cc1 -triple x86_64--- -target-cpu not-a-cpu -fsyntax-only 
%s 2>&1 | FileCheck %s --check-prefix X86_64
 // X86_64: error: unknown target CPU 'not-a-cpu'
 // X86_64: note: valid target CPU values are: nocona, core2, penryn, bonnell,
-// X86_64-SAME: atom, silvermont, slm, goldmont, nehalem, corei7, westmere,
+// X86_64-SAME: atom, silvermont, slm, goldmont, goldmont-plus, tremont, 
nehalem, corei7, w

[PATCH] D45669: [NEON] Fix the architecture condition for the crypto intrinsics

2018-04-16 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

This looks correct to me, the ACLE indeed says that:

> This is only available when  __ARM_ARCH >= 8

Thanks for fixing this.


https://reviews.llvm.org/D45669



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


[PATCH] D45569: [Sema] Disable built-in increment operator for bool in overload resolution in C++17

2018-04-16 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: SemaCXX/overloaded-builtin-operators-cxx17.cpp:8
+void foo(BoolRef br) {
+  // C++ [over.built]p3: Increment for bool got deprecated in C++17.
+  bool b = br++; // expected-error{{cannot increment value of type 'BoolRef'}}

Removed, not deprecated.


Repository:
  rC Clang

https://reviews.llvm.org/D45569



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


[PATCH] D45670: [NEON} Define vfma_n_f32() and vfmaq_n_f32() intrinsics in AArch32 mode

2018-04-16 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added inline comments.



Comment at: test/CodeGen/arm-neon-fma.c:27
+// CHECK:   [[VECINIT1_I:%.*]] = insertelement <2 x float> [[VECINIT_I]], 
float %n, i32 1
+// CHECK:   [[TMP0:%.*]] = bitcast <2 x float> %a to <8 x i8>
+// CHECK:   [[TMP1:%.*]] = bitcast <2 x float> %b to <8 x i8>

If these bitcasts are not used, we also don't need to check for them; that 
looks to be inline with the other tests here.


https://reviews.llvm.org/D45670



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


[PATCH] D45680: [C++2a] Add operator<=> Rewriting - Early Attempt

2018-04-16 Thread Eric Fiselier via Phabricator via cfe-commits
EricWF planned changes to this revision.
EricWF added a comment.

I know how to do better.




Comment at: include/clang/Sema/Overload.h:938
+/// exited.
+struct RewrittenCandidateContextGuard {
+  RewrittenCandidateContextGuard(OverloadCandidateSet &CS)

This is unneeded. As long as the rewritten candidates are added last, restoring 
the `Functions` member is unneeded. 


Repository:
  rC Clang

https://reviews.llvm.org/D45680



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


Re: r329780 - [Analyzer] SValBuilder Comparison Rearrangement (with Restrictions and Analyzer Option)

2018-04-16 Thread Mikael Holmén via cfe-commits

Hi Adam and Artem,

If I run

clang-tidy -header-filter=.\* 
-checks=clang-analyzer-\*,-clang-analyzer-unix\*,-clang-analyzer-osx\*,-clang-analyzer-security\*,-clang-analyzer-valist\*,-clang-analyzer-optin\*,-clang-analyzer-nullability\*,-clang-analyzer-llvm.Conventions,-clang-analyzer-deadcode.DeadStores,-clang-analyzer-cplusplus\*,-clang-analyzer-apiModeling\* 
test.c --


on the attached test.c it takes seemingly forever, but if I peel off a 
number of calls to "f" it starts terminating.


It seems like every additional level of calls to "f" doubles the 
compilation time with your commit.


Without your commit even the full repriducer terminates quickly.

Regards,
Mikael

On 04/11/2018 08:21 AM, Adam Balogh via cfe-commits wrote:

Author: baloghadamsoftware
Date: Tue Apr 10 23:21:12 2018
New Revision: 329780

URL: http://llvm.org/viewvc/llvm-project?rev=329780&view=rev
Log:
[Analyzer] SValBuilder Comparison Rearrangement (with Restrictions and Analyzer 
Option)

Since the range-based constraint manager (default) is weak in handling comparisons 
where symbols are on both sides it is wise to rearrange them to have symbols only on 
the left side. Thus e.g. A + n >= B + m becomes A - B >= m - n which enables 
the constraint manager to store a range m - n .. MAX_VALUE for the symbolic 
expression A - B. This can be used later to check whether e.g. A + k == B + l can be 
true, which is also rearranged to A - B == l - k so the constraint manager can check 
whether l - k is in the range (thus greater than or equal to m - n).

The restriction in this version is the the rearrangement happens only if both 
the symbols and the concrete integers are within the range [min/4 .. max/4] 
where min and max are the minimal and maximal values of their type.

The rearrangement is not enabled by default. It has to be enabled by using 
-analyzer-config aggressive-relational-comparison-simplification=true.

Co-author of this patch is Artem Dergachev (NoQ).

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


Added:
 cfe/trunk/test/Analysis/svalbuilder-rearrange-comparisons.c
Modified:
 cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
 cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
 cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
 cfe/trunk/test/Analysis/conditional-path-notes.c
 cfe/trunk/test/Analysis/explain-svals.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h?rev=329780&r1=329779&r2=329780&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h Tue Apr 10 
23:21:12 2018
@@ -312,6 +312,9 @@ private:
/// \sa shouldDisplayNotesAsEvents
Optional DisplayNotesAsEvents;
  
+  /// \sa shouldAggressivelySimplifyRelationalComparison

+  Optional AggressiveRelationalComparisonSimplification;
+
/// \sa getCTUDir
Optional CTUDir;
  
@@ -666,6 +669,20 @@ public:

/// to false when unset.
bool shouldDisplayNotesAsEvents();
  
+  /// Returns true if SValBuilder should rearrange comparisons of symbolic

+  /// expressions which consist of a sum of a symbol and a concrete integer
+  /// into the format where symbols are on the left-hand side and the integer
+  /// is on the right. This is only done if both symbols and both concrete
+  /// integers are signed, greater than or equal to the quarter of the minimum
+  /// value of the type and less than or equal to the quarter of the maximum
+  /// value of that type.
+  ///
+  /// A + n  B + m becomes A - B  m - n, where A and B symbolic,
+  /// n and m are integers.  is any of '==', '!=', '<', '<=', '>' or '>='.
+  /// The rearrangement also happens with '-' instead of '+' on either or both
+  /// side and also if any or both integers are missing.
+  bool shouldAggressivelySimplifyRelationalComparison();
+
/// Returns the directory containing the CTU related files.
StringRef getCTUDir();
  


Modified: cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp?rev=329780&r1=329779&r2=329780&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp Tue Apr 10 23:21:12 
2018
@@ -445,6 +445,14 @@ bool AnalyzerOptions::shouldDisplayNotes
return DisplayNotesAsEvents.getValue();
  }
  
+bool AnalyzerOptions::shouldAggressivelySimplifyRelationalComparison() {

+  if (!AggressiveRelationalComparisonSimplification.hasValue())
+AggressiveRelationalComparisonSimplification =
+  getBooleanOption("aggressive-relational-comparison-simplification",
+   

[PATCH] D45383: Limit types of builtins that can be redeclared.

2018-04-16 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

In https://reviews.llvm.org/D45383#1068085, @compnerd wrote:

>   Snipping bits from `va_defs.h`:
>   
>
>   #elif defined _M_ARM64
>  
>   void __cdecl __va_start(va_list*, ...);
>  
>   #define __crt_va_start_a(ap,v) ((void)(__va_start(&ap, _ADDRESSOF(v), 
> _SLOTSIZEOF(v), __alignof(v), _ADDRESSOF(v
>   ...
>  
>   #elif defined _M_X64
>  
>   void __cdecl __va_start(va_list* , ...);
>  
>   #define __crt_va_start_a(ap, x) ((void)(__va_start(&ap, x)))
>  
>   ...
>
>
> This looks like a declaration to me.  Although, this is in system headers, so 
> maybe we can ignore it by means of the system header suppression?  The minor 
> problem with that is that clang-cl (and the clang driver with the windows 
> triple) do not support `-isystem` or `-isysroot` or `--sysroot` arguments.  I 
> suppose that as long as we expose the cc1 option (I imagine that clang-cl 
> will pass the system paths appropriately), that is one option.


Ah, thank you!  I couldn't find that when I looked after your last message, 
perhaps my grep skills are lacking.  Unfortunately, allowing this in headers 
won't fix the actual bug, it'll just let it continue to crash on these headers.
I'm kind of out of ideas here...  @efriedma : Ideas?


https://reviews.llvm.org/D45383



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


[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

2018-04-16 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus updated this revision to Diff 142624.
Szelethus added a comment.

> Would be interesting to extend this checker (maybe in an upcoming patch) to 
> report on uninitialized members not only in constructors, but also copy 
> constructors and move constructors.

Added 3 new test cases. to cover them. Interestingly, move constructors don't 
emit any warnings - the core can only assert that the fields after a move 
construction are valid (returns `true` for Val::isValid()`).
Came to think of it, I'm not 100% confident in the checkers name. It could be 
misleading, as this checker doesn't check constructors, but rather objects 
after construction. The end of a constructor call is only the point at which we 
know that analysis can be done.

> This bug report also mentions assignment operator. But for that a warning may 
> be not so useful. In that case the members of the assigned to object should 
> have some initialized value already which the programmer may not want to 
> overwrite in the assignment operator.

I believe there's a checker for that already, but I'm really not sure whether 
`UndefinedAssignmentChecker` covers all such cases.

Also, I managed to cause a crash with the class `linked_ptr_internal` from 
google's boringssl when I analyzed the grpc project. I'll look deeper into 
this, but I have a strong suspicion that the error lies within the CSA core.


https://reviews.llvm.org/D45532

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp
  test/Analysis/ctor-uninitialized-member-inheritance.cpp
  test/Analysis/ctor-uninitialized-member.cpp

Index: test/Analysis/ctor-uninitialized-member.cpp
===
--- /dev/null
+++ test/Analysis/ctor-uninitialized-member.cpp
@@ -0,0 +1,960 @@
+//RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.CtorUninitializedMember -std=c++11 -verify %s
+
+//===--===//
+// Default constructor test.
+//===--===//
+
+class CompilerGeneratedConstructorTest {
+  int a, b, c, d, e, f, g, h, i, j;
+
+public:
+  CompilerGeneratedConstructorTest() = default;
+};
+
+void f000() {
+  CompilerGeneratedConstructorTest();
+}
+
+class DefaultConstructorTest {
+  int a; // expected-note{{uninitialized field 'this->a'}}
+
+public:
+  DefaultConstructorTest();
+};
+
+DefaultConstructorTest::DefaultConstructorTest() = default;
+
+void f00() {
+  DefaultConstructorTest(); // expected-warning{{1 uninitialized field}}
+}
+
+//===--===//
+// Initializer list test.
+//===--===//
+
+class InitListTest1 {
+  int a;
+  int b;
+
+public:
+  InitListTest1()
+  : a(1),
+b(2) {
+// All good!
+  }
+};
+
+void f01() {
+  InitListTest1();
+}
+
+class InitListTest2 {
+  int a;
+  int b; // expected-note{{uninitialized field 'this->b'}}
+
+public:
+  InitListTest2()
+  : a(3) {} // expected-warning{{1 uninitialized field}}
+};
+
+void f02() {
+  InitListTest2();
+}
+
+class InitListTest3 {
+  int a; // expected-note{{uninitialized field 'this->a'}}
+  int b;
+
+public:
+  InitListTest3()
+  : b(4) {} // expected-warning{{1 uninitialized field}}
+};
+
+void f03() {
+  InitListTest3();
+}
+
+//===--===//
+// Constructor body test.
+//===--===//
+
+class CtorBodyTest1 {
+  int a, b;
+
+public:
+  CtorBodyTest1() {
+a = 5;
+b = 6;
+// All good!
+  }
+};
+
+void f04() {
+  CtorBodyTest1();
+}
+
+class CtorBodyTest2 {
+  int a;
+  int b; // expected-note{{uninitialized field 'this->b'}}
+
+public:
+  CtorBodyTest2() {
+a = 7; // expected-warning{{1 uninitialized field}}
+  }
+};
+
+void f05() {
+  CtorBodyTest2();
+}
+
+class CtorBodyTest3 {
+  int a; // expected-note{{uninitialized field 'this->a'}}
+  int b;
+
+public:
+  CtorBodyTest3() {
+b = 8; // expected-warning{{1 uninitialized field}}
+  }
+};
+
+void f06() {
+  CtorBodyTest3();
+}
+
+class CtorBodyTest4 {
+  int a; // expected-note{{uninitialized field 'this->a'}}
+  int b; // expected-note{{uninitialized field 'this->b'}}
+
+public:
+  CtorBodyTest4() {}
+};
+
+void f07() {
+  CtorBodyTest4(); // expected-warning{{2 uninitialized fields}}
+}
+
+//===--===//
+// Constructor delegation test.
+//===--===//
+
+class CtorDelegationTest1 {
+  int a;
+  int b;
+
+public:
+  CtorDelegationTest1(int)
+  : a(9) {
+// leaves 'b' unintialized, but we'll never check this function
+  }
+
+  C

[PATCH] D45685: [Sema] Add -Wtest global flag that silences -Wself-assign for overloaded operators.

2018-04-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision.
lebedev.ri added reviewers: rjmccall, aaron.ballman, dblaikie, thakis, rsmith, 
chandlerc, brooksmoses.
Herald added a subscriber: klimek.

Credit for the idea goes to @rjmccall.
I'm not quite sure how to do it with `-wtest` (lowercase). so i just modelled 
it after `-Wsystem-headers`.

As tests show, only the overloaded non-field assign is silenced by it.

Testing: `$ ninja check-clang-sema check-clang-semacxx check-clang`


Repository:
  rC Clang

https://reviews.llvm.org/D45685

Files:
  docs/ReleaseNotes.rst
  docs/UsersManual.rst
  include/clang/AST/ASTDiagnostic.h
  include/clang/AST/CommentDiagnostic.h
  include/clang/Basic/Diagnostic.h
  include/clang/Basic/Diagnostic.td
  include/clang/Basic/DiagnosticIDs.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/CrossTU/CrossTUDiagnostic.h
  include/clang/Driver/DriverDiagnostic.h
  include/clang/Frontend/FrontendDiagnostic.h
  include/clang/Lex/LexDiagnostic.h
  include/clang/Parse/ParseDiagnostic.h
  include/clang/Sema/SemaDiagnostic.h
  include/clang/Serialization/SerializationDiagnostic.h
  include/clang/Tooling/Refactoring/RefactoringDiagnostic.h
  lib/Basic/DiagnosticIDs.cpp
  lib/Basic/Warnings.cpp
  lib/Sema/SemaExpr.cpp
  test/Sema/warn-self-assign-builtin-warn-tests.c
  test/SemaCXX/warn-self-assign-builtin-warn-tests.cpp
  test/SemaCXX/warn-self-assign-field-builtin-warn-tests.cpp
  test/SemaCXX/warn-self-assign-field-overloaded-warn-tests.cpp
  test/SemaCXX/warn-self-assign-overloaded-warn-tests.cpp
  tools/diagtool/DiagnosticNames.cpp
  utils/TableGen/ClangDiagnosticsEmitter.cpp

Index: utils/TableGen/ClangDiagnosticsEmitter.cpp
===
--- utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -552,6 +552,11 @@
 else
   OS << ", false";
 
+if (R.getValueAsBit("HideInTests"))
+  OS << ", true";
+else
+  OS << ", false";
+
 // Category number.
 OS << ", " << CategoryIDs.getID(getDiagnosticCategory(&R, DGParentMap));
 OS << ")\n";
Index: tools/diagtool/DiagnosticNames.cpp
===
--- tools/diagtool/DiagnosticNames.cpp
+++ tools/diagtool/DiagnosticNames.cpp
@@ -28,9 +28,9 @@
 // FIXME: Is it worth having two tables, especially when this one can get
 // out of sync easily?
 static const DiagnosticRecord BuiltinDiagnosticsByID[] = {
-#define DIAG(ENUM,CLASS,DEFAULT_MAPPING,DESC,GROUP,   \
- SFINAE,NOWERROR,SHOWINSYSHEADER,CATEGORY)\
-  { #ENUM, diag::ENUM, STR_SIZE(#ENUM, uint8_t) },
+#define DIAG(ENUM, CLASS, DEFAULT_MAPPING, DESC, GROUP, SFINAE, NOWERROR,  \
+ SHOWINSYSHEADER, HIDEINTESTS, CATEGORY)   \
+  {#ENUM, diag::ENUM, STR_SIZE(#ENUM, uint8_t)},
 #include "clang/Basic/DiagnosticCommonKinds.inc"
 #include "clang/Basic/DiagnosticCrossTUKinds.inc"
 #include "clang/Basic/DiagnosticDriverKinds.inc"
Index: test/SemaCXX/warn-self-assign-overloaded-warn-tests.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-self-assign-overloaded-warn-tests.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -Wno-tests -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -Wtests -verify -DSILENCE %s
+
+struct S {};
+
+void f() {
+  S a;
+#ifndef SILENCE
+  a = a; // expected-warning{{explicitly assigning}}
+#else
+  // expected-no-diagnostics
+  a = a;
+#endif
+}
Index: test/SemaCXX/warn-self-assign-field-overloaded-warn-tests.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-self-assign-field-overloaded-warn-tests.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -Wno-tests -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -Wtests -verify %s
+
+struct S {};
+
+struct C {
+  S a;
+
+  void f() {
+#ifndef SILENCE
+a = a; // expected-warning{{assigning field to itself}}
+#else
+// expected-no-diagnostics
+a = a;
+#endif
+  }
+};
Index: test/SemaCXX/warn-self-assign-field-builtin-warn-tests.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-self-assign-field-builtin-warn-tests.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -Wno-tests -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -Wtests -verify %s
+
+struct C {
+  int a;
+
+  void f() {
+#ifndef SILENCE
+a = a; // expected-warning{{assigning field to itself}}
+#else
+// expected-no-diagnostics
+a = a;
+#endif
+  }
+};
Index: test/SemaCXX/warn-self-assign-builtin-warn-tests.cpp
=

[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

2018-04-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In https://reviews.llvm.org/D44883#1068602, @lebedev.ri wrote:

> There are several options:
>
> 1. @rjmccall's idea: `-wtest` (lowercase), which in this case will disable 
> that new code in `BuildOverloadedBinOp()`. i quite like it actually.
> 2. split it up like i had in the first revision - ``-Wself-assign-builtin``, 
> ``-Wself-assign-field-builtin``; ``-Wself-assign-overloaded``, 
> ``-Wself-assign-field-overloaded``
>   - we could just assume that `BuildOverloadedBinOp()` implies overloaded,
>   - or check that the particular operator is non-trivial
> 3. ???
>
>   @rjmccall, @thakis, @dblaikie, @aaron.ballman, @brooksmoses, @chandlerc
>
>   I'm going to go ahead and look into `1.`, since it does not seem there will 
> be any consensus in a timely manner.


... and here it is https://reviews.llvm.org/D45685, please take a look


Repository:
  rC Clang

https://reviews.llvm.org/D44883



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


[PATCH] D45685: [Sema] Add -Wtest global flag that silences -Wself-assign for overloaded operators.

2018-04-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 142627.
lebedev.ri added a comment.

- Don't mis-spell the name of the flag.

FIXME: So do we want it to be `-Wtests`, `-Wtest`, or we really want it to be 
`-wtest` ?


Repository:
  rC Clang

https://reviews.llvm.org/D45685

Files:
  docs/ReleaseNotes.rst
  docs/UsersManual.rst
  include/clang/AST/ASTDiagnostic.h
  include/clang/AST/CommentDiagnostic.h
  include/clang/Basic/Diagnostic.h
  include/clang/Basic/Diagnostic.td
  include/clang/Basic/DiagnosticIDs.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/CrossTU/CrossTUDiagnostic.h
  include/clang/Driver/DriverDiagnostic.h
  include/clang/Frontend/FrontendDiagnostic.h
  include/clang/Lex/LexDiagnostic.h
  include/clang/Parse/ParseDiagnostic.h
  include/clang/Sema/SemaDiagnostic.h
  include/clang/Serialization/SerializationDiagnostic.h
  include/clang/Tooling/Refactoring/RefactoringDiagnostic.h
  lib/Basic/DiagnosticIDs.cpp
  lib/Basic/Warnings.cpp
  lib/Sema/SemaExpr.cpp
  test/Sema/warn-self-assign-builtin-warn-tests.c
  test/SemaCXX/warn-self-assign-builtin-warn-tests.cpp
  test/SemaCXX/warn-self-assign-field-builtin-warn-tests.cpp
  test/SemaCXX/warn-self-assign-field-overloaded-warn-tests.cpp
  test/SemaCXX/warn-self-assign-overloaded-warn-tests.cpp
  tools/diagtool/DiagnosticNames.cpp
  utils/TableGen/ClangDiagnosticsEmitter.cpp

Index: utils/TableGen/ClangDiagnosticsEmitter.cpp
===
--- utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -552,6 +552,11 @@
 else
   OS << ", false";
 
+if (R.getValueAsBit("HideInTests"))
+  OS << ", true";
+else
+  OS << ", false";
+
 // Category number.
 OS << ", " << CategoryIDs.getID(getDiagnosticCategory(&R, DGParentMap));
 OS << ")\n";
Index: tools/diagtool/DiagnosticNames.cpp
===
--- tools/diagtool/DiagnosticNames.cpp
+++ tools/diagtool/DiagnosticNames.cpp
@@ -28,9 +28,9 @@
 // FIXME: Is it worth having two tables, especially when this one can get
 // out of sync easily?
 static const DiagnosticRecord BuiltinDiagnosticsByID[] = {
-#define DIAG(ENUM,CLASS,DEFAULT_MAPPING,DESC,GROUP,   \
- SFINAE,NOWERROR,SHOWINSYSHEADER,CATEGORY)\
-  { #ENUM, diag::ENUM, STR_SIZE(#ENUM, uint8_t) },
+#define DIAG(ENUM, CLASS, DEFAULT_MAPPING, DESC, GROUP, SFINAE, NOWERROR,  \
+ SHOWINSYSHEADER, HIDEINTESTS, CATEGORY)   \
+  {#ENUM, diag::ENUM, STR_SIZE(#ENUM, uint8_t)},
 #include "clang/Basic/DiagnosticCommonKinds.inc"
 #include "clang/Basic/DiagnosticCrossTUKinds.inc"
 #include "clang/Basic/DiagnosticDriverKinds.inc"
Index: test/SemaCXX/warn-self-assign-overloaded-warn-tests.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-self-assign-overloaded-warn-tests.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -Wno-tests -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -Wtests -verify -DSILENCE %s
+
+struct S {};
+
+void f() {
+  S a;
+#ifndef SILENCE
+  a = a; // expected-warning{{explicitly assigning}}
+#else
+  // expected-no-diagnostics
+  a = a;
+#endif
+}
Index: test/SemaCXX/warn-self-assign-field-overloaded-warn-tests.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-self-assign-field-overloaded-warn-tests.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -Wno-tests -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -Wtests -verify %s
+
+struct S {};
+
+struct C {
+  S a;
+
+  void f() {
+#ifndef SILENCE
+a = a; // expected-warning{{assigning field to itself}}
+#else
+// expected-no-diagnostics
+a = a;
+#endif
+  }
+};
Index: test/SemaCXX/warn-self-assign-field-builtin-warn-tests.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-self-assign-field-builtin-warn-tests.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -Wno-tests -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -Wtests -verify %s
+
+struct C {
+  int a;
+
+  void f() {
+#ifndef SILENCE
+a = a; // expected-warning{{assigning field to itself}}
+#else
+// expected-no-diagnostics
+a = a;
+#endif
+  }
+};
Index: test/SemaCXX/warn-self-assign-builtin-warn-tests.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-self-assign-builtin-warn-tests.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -Wno-tests -verify %s
+// RUN: 

[PATCH] D45407: [StaticAnalyzer] Added notes to the plist output

2018-04-16 Thread Whisperity via Phabricator via cfe-commits
whisperity added a comment.

@NoQ The problem with emitting notes as events is that we lose the information 
that the node was a `node`. How does Xcode behave with these notes? Does it 
ignore them, or can read them from the command-line output of the analyser?


Repository:
  rC Clang

https://reviews.llvm.org/D45407



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


[PATCH] D45686: [Tooling] Clean up tmp files when creating a fixed compilation database

2018-04-16 Thread David Stenberg via Phabricator via cfe-commits
dstenb created this revision.
dstenb added reviewers: klimek, sepavloff, arphaman.
Herald added a subscriber: cfe-commits.

In https://reviews.llvm.org/rL327851 the createUniqueFile() and 
createTemporaryFile()
variants that do not return the file descriptors were changed to
create empty files, rather than only check if the paths are free.
This change was done in order to make the functions race-free.

That change led to clang-tidy (and possibly other tools) leaving
behind temporary assembly files, of the form placeholder-*, when
using a target that does not support the internal assembler.

The temporary files are created when building the Compilation
object in stripPositionalArgs(), as a part of creating the
compilation database for the arguments after the double-dash. The
files are created by Driver::GetNamedOutputPath().

Fix this issue by cleaning out the Compilation object's temporary
file list before the object is deleted at end-of-scope in
stripPositionalArgs().

I am not very well-versed in Driver/Tooling, so I don't know if this
should be seen as a proper fix, or as a temporary workaround.
I at least assume that we want to use a race-free variant rather
than getPotentiallyUniqueTempFileName() in
Driver::GetNamedOutputPath().

This fixes https://bugs.llvm.org/show_bug.cgi?id=37091.


Repository:
  rC Clang

https://reviews.llvm.org/D45686

Files:
  lib/Tooling/CompilationDatabase.cpp


Index: lib/Tooling/CompilationDatabase.cpp
===
--- lib/Tooling/CompilationDatabase.cpp
+++ lib/Tooling/CompilationDatabase.cpp
@@ -299,6 +299,9 @@
 }
   }
 
+  // Remove temp files.
+  Compilation->CleanupFileList(Compilation->getTempFiles());
+
   if (CompileAnalyzer.Inputs.empty()) {
 ErrorMsg = "warning: no compile jobs found\n";
 return false;


Index: lib/Tooling/CompilationDatabase.cpp
===
--- lib/Tooling/CompilationDatabase.cpp
+++ lib/Tooling/CompilationDatabase.cpp
@@ -299,6 +299,9 @@
 }
   }
 
+  // Remove temp files.
+  Compilation->CleanupFileList(Compilation->getTempFiles());
+
   if (CompileAnalyzer.Inputs.empty()) {
 ErrorMsg = "warning: no compile jobs found\n";
 return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45045: [DebugInfo] Generate debug information for labels.

2018-04-16 Thread Hsiangkai Wang via Phabricator via cfe-commits
HsiangKai marked 2 inline comments as done.
HsiangKai added a comment.

Always add labels to DISubprogram even unreachable.


Repository:
  rC Clang

https://reviews.llvm.org/D45045



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


[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

2018-04-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 142636.
lebedev.ri retitled this revision from "[Sema] Add -Wtest global flag that 
silences -Wself-assign for overloaded operators." to "[Sema] Add -wtest global 
flag that silences -Wself-assign for overloaded operators.".
lebedev.ri edited the summary of this revision.
lebedev.ri added a comment.

Actually make it `-wtest` (all lowercase).


Repository:
  rC Clang

https://reviews.llvm.org/D45685

Files:
  docs/ReleaseNotes.rst
  docs/UsersManual.rst
  include/clang/AST/ASTDiagnostic.h
  include/clang/AST/CommentDiagnostic.h
  include/clang/Basic/Diagnostic.h
  include/clang/Basic/Diagnostic.td
  include/clang/Basic/DiagnosticIDs.h
  include/clang/Basic/DiagnosticOptions.def
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/CrossTU/CrossTUDiagnostic.h
  include/clang/Driver/DriverDiagnostic.h
  include/clang/Driver/Options.td
  include/clang/Frontend/FrontendDiagnostic.h
  include/clang/Lex/LexDiagnostic.h
  include/clang/Parse/ParseDiagnostic.h
  include/clang/Sema/SemaDiagnostic.h
  include/clang/Serialization/SerializationDiagnostic.h
  include/clang/Tooling/Refactoring/RefactoringDiagnostic.h
  lib/Basic/DiagnosticIDs.cpp
  lib/Basic/Warnings.cpp
  lib/Driver/Driver.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaExpr.cpp
  test/Sema/warn-self-assign-builtin-warn-test.c
  test/SemaCXX/warn-self-assign-builtin-warn-test.cpp
  test/SemaCXX/warn-self-assign-field-builtin-warn-test.cpp
  test/SemaCXX/warn-self-assign-field-overloaded-warn-test.cpp
  test/SemaCXX/warn-self-assign-overloaded-warn-test.cpp
  tools/diagtool/DiagnosticNames.cpp
  utils/TableGen/ClangDiagnosticsEmitter.cpp

Index: utils/TableGen/ClangDiagnosticsEmitter.cpp
===
--- utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -552,6 +552,11 @@
 else
   OS << ", false";
 
+if (R.getValueAsBit("HideInTests"))
+  OS << ", true";
+else
+  OS << ", false";
+
 // Category number.
 OS << ", " << CategoryIDs.getID(getDiagnosticCategory(&R, DGParentMap));
 OS << ")\n";
Index: tools/diagtool/DiagnosticNames.cpp
===
--- tools/diagtool/DiagnosticNames.cpp
+++ tools/diagtool/DiagnosticNames.cpp
@@ -28,9 +28,9 @@
 // FIXME: Is it worth having two tables, especially when this one can get
 // out of sync easily?
 static const DiagnosticRecord BuiltinDiagnosticsByID[] = {
-#define DIAG(ENUM,CLASS,DEFAULT_MAPPING,DESC,GROUP,   \
- SFINAE,NOWERROR,SHOWINSYSHEADER,CATEGORY)\
-  { #ENUM, diag::ENUM, STR_SIZE(#ENUM, uint8_t) },
+#define DIAG(ENUM, CLASS, DEFAULT_MAPPING, DESC, GROUP, SFINAE, NOWERROR,  \
+ SHOWINSYSHEADER, HIDEINTESTS, CATEGORY)   \
+  {#ENUM, diag::ENUM, STR_SIZE(#ENUM, uint8_t)},
 #include "clang/Basic/DiagnosticCommonKinds.inc"
 #include "clang/Basic/DiagnosticCrossTUKinds.inc"
 #include "clang/Basic/DiagnosticDriverKinds.inc"
Index: test/SemaCXX/warn-self-assign-overloaded-warn-test.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-self-assign-overloaded-warn-test.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -wtest -verify -DSILENCE %s
+
+struct S {};
+
+void f() {
+  S a;
+#ifndef SILENCE
+  a = a; // expected-warning{{explicitly assigning}}
+#else
+  // expected-no-diagnostics
+  a = a;
+#endif
+}
Index: test/SemaCXX/warn-self-assign-field-overloaded-warn-test.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-self-assign-field-overloaded-warn-test.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -wtest -verify %s
+
+struct S {};
+
+struct C {
+  S a;
+
+  void f() {
+#ifndef SILENCE
+a = a; // expected-warning{{assigning field to itself}}
+#else
+// expected-no-diagnostics
+a = a;
+#endif
+  }
+};
Index: test/SemaCXX/warn-self-assign-field-builtin-warn-test.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-self-assign-field-builtin-warn-test.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -wtest -verify %s
+
+struct C {
+  int a;
+
+  void f() {
+#ifndef SILENCE
+a = a; // expected-warning{{assigning field to itself}}
+#else
+// expected-no-diagnostics
+a = a;
+#endif
+  }
+};
Index: test/SemaCXX/warn-self-assign-builtin-warn-test.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-self-assign-builtin-warn-test.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -verify %s

[PATCH] D44888: [RISCV] Default enable linker relaxation and add -mrelax, -mno-relax flags

2018-04-16 Thread Alex Bradbury via Phabricator via cfe-commits
asb requested changes to this revision.
asb added a comment.
This revision now requires changes to proceed.

Thanks Kito. -mrelax and -mno-relax currently only affect the backend. For 
completeness, I think this patch needs to pass the appropriate flag to the 
linker depending on relax/no-relax.


Repository:
  rL LLVM

https://reviews.llvm.org/D44888



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


[PATCH] D45692: [clangd] Fix "fail to create file URI" warnings in FileIndexTest.

2018-04-16 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
hokein added a reviewer: ioeric.
Herald added subscribers: MaskRay, jkorous-apple, ilya-biryukov, klimek.

When running the FileIndexTest, it shows "Failed to create an URI
for file XXX: not a valid absolute file path" warnings, and the
generated Symbols is missing Location information.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45692

Files:
  unittests/clangd/FileIndexTests.cpp


Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -94,8 +94,8 @@
  "BasePath must be a base file path without extension.");
   llvm::IntrusiveRefCntPtr VFS(
   new vfs::InMemoryFileSystem);
-  std::string Path = (BasePath + ".cpp").str();
-  std::string Header = (BasePath + ".h").str();
+  std::string Path = testPath((BasePath + ".cpp").str());
+  std::string Header = testPath((BasePath + ".h").str());
   VFS->addFile(Path, 0, llvm::MemoryBuffer::getMemBuffer(""));
   VFS->addFile(Header, 0, llvm::MemoryBuffer::getMemBuffer(Code));
   const char *Args[] = {"clang", "-xc++", "-include", Header.c_str(),


Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -94,8 +94,8 @@
  "BasePath must be a base file path without extension.");
   llvm::IntrusiveRefCntPtr VFS(
   new vfs::InMemoryFileSystem);
-  std::string Path = (BasePath + ".cpp").str();
-  std::string Header = (BasePath + ".h").str();
+  std::string Path = testPath((BasePath + ".cpp").str());
+  std::string Header = testPath((BasePath + ".h").str());
   VFS->addFile(Path, 0, llvm::MemoryBuffer::getMemBuffer(""));
   VFS->addFile(Header, 0, llvm::MemoryBuffer::getMemBuffer(Code));
   const char *Args[] = {"clang", "-xc++", "-include", Header.c_str(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45643: [Failing one test] Reword [-Wreturn-type] messages to "non-void x does not return a value"

2018-04-16 Thread easyaspi314 (Devin) via Phabricator via cfe-commits
easyaspi314 added inline comments.



Comment at: docs/DiagnosticsReference.rst:9097
 |   |++|   
  |
 
+---+--+-+

rsmith wrote:
> This is a generated file; please don't manually update it. (Though we should 
> regenerate it, it's probably quite stale...)
Serious question though, why in the world do we have generated files in an open 
source repo? 

Isn't is as stupid as putting the precompiled binaries in the source tree? I'd 
rather wait a few minutes to generate some files than spend 4 hours trying to 
figure out how to deal with serialized-diags-stable.dia.


Repository:
  rC Clang

https://reviews.llvm.org/D45643



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


[PATCH] D45680: [C++2a] Add operator<=> Rewriting - Early Attempt

2018-04-16 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: lib/Sema/SemaOverload.cpp:839
+
+unsigned OverloadCandidate::getTrueArgIndex(unsigned Idx) const {
+  if (getRewrittenKind() != ROC_Synthesized)

I think this might be clearer if named `getParamIndexForArgIndex` or similar.



Comment at: lib/Sema/SemaOverload.cpp:8880-8882
+/// \brief Add the rewritten and synthesized candidates for binary comparison
+//operators. No additional semantic checking is done to see if the 
candidate
+//is well formed.

No `\brief` here (we use autobrief), and please use `///` for continuation 
lines.



Comment at: lib/Sema/SemaOverload.cpp:
+  bool PerformADL) {
+  assert(getLangOpts().CPlusPlus2a);
+  auto Opc = BinaryOperator::getOverloadedOpcode(Op);

Does the implementation below actually rely on this?



Comment at: lib/Sema/SemaOverload.cpp:8901
+  // Lookup possible candidates for the rewritten operator.
+  // FIXME:  should this really be done in the current scope?
+  LookupResult Operators(*this, OpName, SourceLocation(),

No, you shouldn't be doing any lookups here. Instead, what you should do is to 
change the `LookupOverloadedOperatorName` call in `BuildOverloadedBinOp` to 
look up both `operator@` and `operator<=>` in the case where `@` is an equality 
or relational op, and then filter `Fns` by operator name both here and when 
adding the non-member function candidates.

The reason is that we need to do these lookups in the context of the template 
definition when an equality or relational expression appears within a template, 
so we need to look up both the original operator and `<=>` eagerly.



Comment at: lib/Sema/SemaOverload.cpp:8936-8938
+  // TODO: We should be able to avoid adding synthesized candidates when LHS 
and
+  // RHS have the same type, since the synthesized candidates for <=> should be
+  // the same as the rewritten ones. Note: It's still possible for the result

Type isn't sufficient to conclude this; you'll need to check the other relevant 
properties of LHS and RHS too (at least value kind, but maybe also 
bit-field-ness and others?).



Comment at: lib/Sema/SemaOverload.cpp:9218-9219
+// --- F2 is a rewritten candidate ([over.match.oper]) and F1 is not.
+if (Cand2.getRewrittenKind() && !Cand1.getRewrittenKind())
+  return true;
+if (Cand1.getRewrittenKind() && Cand2.getRewrittenKind() &&

You also need to check the reverse condition and return false (the "or if not 
that" is ... somehow ... supposed to imply that).



Comment at: lib/Sema/SemaOverload.cpp:12502-12535
+ExprResult RewrittenCandidateOverloadResolver::BuildRewrittenCandidate(
+const OverloadCandidate &Ovl) {
+  Expr *RewrittenArgs[2] = {Args[0], Args[1]};
+  bool IsSynthesized = Ovl.getRewrittenKind() == ROC_Synthesized;
+  if (IsSynthesized)
+std::swap(RewrittenArgs[0], RewrittenArgs[1]);
+

This results in an inadequate representation for the rewritten `<=>` form. We 
need to retain the "as-written" form of the operator, for multiple reasons 
(source fidelity for tooling, expression mangling, pretty-printing, ...). 
Generally, Clang's philosophy is to desugar the minimum amount that's necessary 
to capture both the syntactic and semantic forms.

A couple of possible representations come to mind here:

1) A specific `Expr` subclass for a rewritten `<=>` operator, which is just a 
wrapper around a `(a <=> b) op 0` expression, and knows how to extract the 
subexpressions itself, or...
2) A general "rewritten operator wrapper" `Expr` subclass, which holds its 
operands and a rewritten expression, and for which the rewritten expression 
refers to its operands via `OpaqueValueExpr`s. We already have such a class 
(`PseudoObjectExpr`).

The former is a smaller and simpler representation, but will likely be more 
work to implement.



Comment at: lib/Sema/SemaOverload.cpp:12537-12565
+/// Rewritten candidates have been added but not checked for validity. They
+/// could still be non-viable if:
+///  (A) The rewritten call (x <=> y) is a builtin, but it will be ill-formed
+///  when built (for example it has narrowing conversions).
+///  (B) The expression (x <=> y) @ 0 is ill-formed for the result of (x <=> 
y).
+///
+/// If either is the case, this function should be considered non-viable and

Hmm, this is an interesting approach. It's quite a divergence from our usual 
approach, which is to perform the viability checks first, before partial 
ordering, even if we could eliminate some candidates based on partial ordering. 
And it's observable -- it will result in fewer template instantiations being 
performed, which will mean we do not diagnose some ill-formed (no diagnostic 
required) situations which would

[PATCH] D45643: [Failing one test] Reword [-Wreturn-type] messages to "non-void x does not return a value"

2018-04-16 Thread easyaspi314 (Devin) via Phabricator via cfe-commits
easyaspi314 added inline comments.



Comment at: bindings/python/tests/cindex/test_diagnostics.py:18
 self.assertEqual(tu.diagnostics[0].spelling,
-'control reaches end of non-void function')
+'non-void function does not return a value')
 

Quuxplusone wrote:
> rsmith wrote:
> > It seems like part of the problem here is that "non-void function" is 
> > sort-of nonsense due to a few missing words. How about:
> > 
> > > "control can reach end of function with non-void return type"
> > 
> > or similar?
> > 
> > I think we still need the mention of control flow, because we're *not* 
> > saying the function contains no return statements, we're saying there's a 
> > control flow path that reaches the end of the function.
> I think OP's issue here is that the current message about "control" is 
> intrinsically confusing. //Of course// "control can reach end of function"; 
> if control never reaches the end of the function, you must have an infinite 
> loop somewhere! The important missing piece of the current message is that 
> control reaches the end of the function //without encountering any return 
> statement//.
> 
> > we're *not* saying the function contains no return statements
> 
> Not sure, but I think in this case we //are// saying that. There's a 
> different message, "non-void function //might// not return a value," for 
> those other cases.
> 
> However, if I'm wrong about that, then another wording option close to OP's 
> suggestion would be "non-void function does not //always// return a value" / 
> "non-void function might not //always// return a value."
> I think we still need the mention of control flow.

I see where you are going.

However, I have another idea, but I can't implement it myself.

What if we give the user a note like this:

```
int someCondition;

int a() {
if (someCondition) {
return 3;
}
}
```
```
foo.c:3:1: warning: non-void function might not return a value [-Wreturn-type]
int a() {
^
foo.c:4:5: note: assuming 'someCondition' is false
if (someCondition) {
^
foo.c:7:1: note: control flow reaches end of function without a return value
}
^
1 warning generated.
```



Repository:
  rC Clang

https://reviews.llvm.org/D45643



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


[PATCH] D39053: [Bitfield] Add more cases to making the bitfield a separate location

2018-04-16 Thread Strahinja Petrovic via Phabricator via cfe-commits
spetrovic added a comment.

ping


https://reviews.llvm.org/D39053



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


[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

2018-04-16 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment.

In https://reviews.llvm.org/D45444#1068496, @shuaiwang wrote:

> - I would imagine things could get messier if this check expands to also 
> check for turning member functions const: it's basically checking 
> CxxThisExpr, being a handle, is not modified within a member function, but 
> note there's no VarDecl for "this".


Probably this should be separate check. See also PR21981.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45444



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


[PATCH] D45093: [AST] Fix -ast-print for _Bool when have diagnostics

2018-04-16 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny updated this revision to Diff 142662.
jdenny added a comment.

Rebased onto a more recent master.


https://reviews.llvm.org/D45093

Files:
  include/clang/AST/ASTContext.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseDecl.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaCodeComplete.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExceptionSpec.cpp
  lib/Sema/SemaExprCXX.cpp
  lib/Sema/SemaLookup.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplate.cpp
  lib/Sema/SemaTemplateInstantiate.cpp
  test/Misc/ast-print-bool.c

Index: test/Misc/ast-print-bool.c
===
--- /dev/null
+++ test/Misc/ast-print-bool.c
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -verify -ast-print %s -xc -DDEF_BOOL_CBOOL \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-CBOOL,CBOOL
+//
+// RUN: %clang_cc1 -verify -ast-print %s -xc -DDEF_BOOL_CBOOL -DDIAG \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-CBOOL,CBOOL
+//
+// RUN: %clang_cc1 -verify -ast-print %s -xc -DDEF_BOOL_INT \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-INT,CBOOL
+//
+// RUN: %clang_cc1 -verify -ast-print %s -xc -DDEF_BOOL_INT -DDIAG \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-INT,CBOOL
+//
+// RUN: %clang_cc1 -verify -ast-print %s -xc++ \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-BOOL
+//
+// RUN: %clang_cc1 -verify -ast-print %s -xc++ -DDIAG \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-BOOL
+
+#if DEF_BOOL_CBOOL
+# define bool _Bool
+#elif DEF_BOOL_INT
+# define bool int
+#endif
+
+// BOOL-AS-CBOOL: _Bool i;
+// BOOL-AS-INT:   int i;
+// BOOL-AS-BOOL:  bool i;
+bool i;
+
+#ifndef __cplusplus
+// CBOOL: _Bool j;
+_Bool j;
+#endif
+
+// Induce a diagnostic (and verify we actually managed to do so), which used to
+// permanently alter the -ast-print printing policy for _Bool.  How bool is
+// defined by the preprocessor is examined only once per compilation, when the
+// diagnostic is emitted, and it used to affect the entirety of -ast-print, so
+// test only one definition of bool per compilation.
+#if DIAG
+void fn() { 1; } // expected-warning {{expression result unused}}
+#else
+// expected-no-diagnostics
+#endif
Index: lib/Sema/SemaTemplateInstantiate.cpp
===
--- lib/Sema/SemaTemplateInstantiate.cpp
+++ lib/Sema/SemaTemplateInstantiate.cpp
@@ -505,7 +505,7 @@
   llvm::raw_svector_ostream OS(TemplateArgsStr);
   Template->printName(OS);
   printTemplateArgumentList(OS, Active->template_arguments(),
-getPrintingPolicy());
+getDiagPrintingPolicy());
   Diags.Report(Active->PointOfInstantiation,
diag::note_default_arg_instantiation_here)
 << OS.str()
@@ -571,7 +571,7 @@
   llvm::raw_svector_ostream OS(TemplateArgsStr);
   FD->printName(OS);
   printTemplateArgumentList(OS, Active->template_arguments(),
-getPrintingPolicy());
+getDiagPrintingPolicy());
   Diags.Report(Active->PointOfInstantiation,
diag::note_default_function_arg_instantiation_here)
 << OS.str()
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -3012,7 +3012,7 @@
   std::string Description;
   {
 llvm::raw_string_ostream Out(Description);
-FailedCond->printPretty(Out, nullptr, getPrintingPolicy());
+FailedCond->printPretty(Out, nullptr, getDiagPrintingPolicy());
   }
   return { FailedCond, Description };
 }
@@ -9821,7 +9821,7 @@
 }
 
 Out << " = ";
-Args[I].print(getPrintingPolicy(), Out);
+Args[I].print(getDiagPrintingPolicy(), Out);
   }
 
   Out << ']';
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -5546,7 +5546,7 @@
 // conversion; use it.
 QualType ConvTy = Conversion->getConversionType().getNonReferenceType();
 std::string TypeStr;
-ConvTy.getAsStringInternal(TypeStr, SemaRef.getPrintingPolicy());
+ConvTy.getAsStringInternal(TypeStr, SemaRef.getDiagPrintingPolicy());
 
 Converter.diagnoseExplicitConv(SemaRef, Loc, T, ConvTy)
 << FixItHint::CreateInsertion(From->getLocStart(),
Index: lib/Sema/SemaLookup.cpp
===
--- lib/Sema/SemaLookup.cpp
+++ lib/Sema/SemaLookup.cpp
@@ -4202,7 +4202,7 @@
   std::string NewQualified = TC.getAsString(SemaRef.getLangOpts());
   std::string OldQualified;
   llvm::raw_string_ostream OldOStream(OldQualified);
-  SS->getScopeRep()->print(OldOStream, SemaRef.getPrintingPolicy());
+  SS->getScopeRep()->print(OldOStream, SemaRef.getDiagPrintingPolicy());
   OldOStream << Typ

[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

2018-04-16 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment.

I don't understand the spelling of this option. You spell it `-wtest` (because 
rjmccall suggested that spelling); but for my money it should be spelled 
`-Wno-self-assign-nonfield`.
Also, if you go this route, you miss the true positive reported by someone on 
the other patch, where the self-assignment involved a local variable named 
`color`.
Sorry I can't find the example right now. You'd have to trawl through all the 
patches opened on this subject in the past couple of weeks.


Repository:
  rC Clang

https://reviews.llvm.org/D45685



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


Re: [PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

2018-04-16 Thread Nico Weber via cfe-commits
On Mon, Apr 16, 2018 at 5:11 AM, Roman Lebedev via Phabricator via
cfe-commits  wrote:

> lebedev.ri added a comment.
>
> Re false-positives - at least two [post-]reviewers need to agree on the
> way forward (see previous comments, mail thread), and then i will implement
> it.
>
> In https://reviews.llvm.org/D44883#1068576, @brooksmoses wrote:
>
> > A further concern about this in the general case from the reviewer of
> one of my test-cleanup changes: The "var = *&var" idiom is not necessarily
> equivalent to "var = var" in cases of user-defined types, because operator&
> may be overloaded.
>
>
> Release notes state:
>
>   If you are doing such an assignment intentionally, e.g. in a unit test
> for
>   a data structure, the warning can be suppressed by adding ``*&`` to the
>   right-hand side or casting it to the appropriate reference type.
>
> So it could also be `var = static_cast(var);`
>

Most people using clang won't read release notes. Maybe this could print a
"to silence this warning, insert *&" fixit note, like many other warnings
do.

I think of the 3 points you mention, "separate flag for this" is probably
the way to go.


>
>
> Repository:
>   rC Clang
>
> https://reviews.llvm.org/D44883
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

2018-04-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In https://reviews.llvm.org/D45685#1068981, @Quuxplusone wrote:

> I don't understand the spelling of this option. You spell it `-wtest` 
> (because rjmccall suggested that spelling); but for my money it should be 
> spelled `-Wno-self-assign-nonfield`.


You did read the documentation change, right?

> Also, if you go this route, you miss the true positive reported by someone on 
> the other patch, where the self-assignment involved a local variable named 
> `color`.

Uhm, why? `-wtest` is not the default, so the warning would still have been 
issued there.

> Sorry I can't find the example right now. You'd have to trawl through all the 
> patches opened on this subject in the past couple of weeks.




Repository:
  rC Clang

https://reviews.llvm.org/D45685



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


[PATCH] D45217: [ThinLTO] Pass -save-temps to LTO backend for distributed ThinLTO builds

2018-04-16 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson marked 2 inline comments as done.
tejohnson added inline comments.



Comment at: include/clang/Driver/Options.td:2255
   HelpText<"Save intermediate compilation results.">;
-def save_temps : Flag<["-", "--"], "save-temps">, Flags<[DriverOption]>,
+def save_temps : Flag<["-", "--"], "save-temps">, Flags<[CC1Option, 
DriverOption]>,
   Alias, AliasArgs<["cwd"]>,

pcc wrote:
> This is still just a DriverOption, right?
No, we need to pass to cc1 to get it into the CodeGenOptions. See the changes 
to Clang.cpp (passes to cc1 invocation) and CompilerInvocation.cpp (consumes 
during cc1 invocation).



Comment at: lib/Frontend/CompilerInvocation.cpp:747
+.Case("obj", FrontendOpts.OutputFile)
+.Default("./" +
+ llvm::sys::path::filename(FrontendOpts.OutputFile).str());

pcc wrote:
> Why do you need the `"./"` part?
Not needed, removed.



Comment at: test/CodeGen/thinlto_backend.ll:30
 ; Ensure f2 was imported
-; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c 
-fthinlto-index=%t.thinlto.bc
+; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c 
-fthinlto-index=%t.thinlto.bc -save-temps=obj
 ; RUN: llvm-nm %t3.o | FileCheck --check-prefix=CHECK-OBJ %s

pcc wrote:
> Should there be another test for `-save-temps=cwd`?
Added -save-temps=cwd and -save-temps


Repository:
  rC Clang

https://reviews.llvm.org/D45217



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


[PATCH] D45217: [ThinLTO] Pass -save-temps to LTO backend for distributed ThinLTO builds

2018-04-16 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson updated this revision to Diff 142668.
tejohnson marked 2 inline comments as done.
tejohnson added a comment.

Addressed comments


Repository:
  rC Clang

https://reviews.llvm.org/D45217

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/BackendUtil.cpp
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/thinlto_backend.ll
  test/Driver/thinlto_backend.c

Index: test/Driver/thinlto_backend.c
===
--- test/Driver/thinlto_backend.c
+++ test/Driver/thinlto_backend.c
@@ -5,6 +5,15 @@
 // RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -### 2>&1 | FileCheck %s -check-prefix=CHECK-THINLTOBE-ACTION
 // CHECK-THINLTOBE-ACTION: -fthinlto-index=
 
+// -save-temps should be passed to cc1
+// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -save-temps -### 2>&1 | FileCheck %s -check-prefix=CHECK-SAVE-TEMPS -check-prefix=CHECK-SAVE-TEMPS-CWD
+// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -save-temps=cwd -### 2>&1 | FileCheck %s -check-prefix=CHECK-SAVE-TEMPS -check-prefix=CHECK-SAVE-TEMPS-CWD
+// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -save-temps=obj -### 2>&1 | FileCheck %s -check-prefix=CHECK-SAVE-TEMPS -check-prefix=CHECK-SAVE-TEMPS-OBJ
+// CHECK-SAVE-TEMPS-NOT: -emit-llvm-bc
+// CHECK-SAVE-TEMPS-CWD: -save-temps=cwd
+// CHECK-SAVE-TEMPS-OBJ: -save-temps=obj
+// CHECK-SAVE-TEMPS-NOT: -emit-llvm-bc
+
 // Ensure clang driver gives the expected error for incorrect input type
 // RUN: not %clang -O2 -o %t1.o %s -c -fthinlto-index=%t.thinlto.bc 2>&1 | FileCheck %s -check-prefix=CHECK-WARNING
 // CHECK-WARNING: error: invalid argument '-fthinlto-index={{.*}}' only allowed with '-x ir'
Index: test/CodeGen/thinlto_backend.ll
===
--- test/CodeGen/thinlto_backend.ll
+++ test/CodeGen/thinlto_backend.ll
@@ -26,8 +26,17 @@
 ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t4.o -x ir %t5.o -c -fthinlto-index=%t.thinlto.bc
 ; RUN: llvm-nm %t4.o | count 0
 
-; Ensure f2 was imported
-; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc
+; Ensure f2 was imported. Check for all 3 flavors of -save-temps[=cwd|obj].
+; RUN: rm -f %t1.s.0.3.import.bc
+; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps=obj
+; RUN: llvm-dis %t1.s.0.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s
+; RUN: rm -f %t1.s.0.3.import.bc
+; RUN: (cd %T && %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps=cwd)
+; RUN: llvm-dis %t1.s.0.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s
+; RUN: rm -f %t1.s.0.3.import.bc
+; RUN: (cd %T && %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps)
+; RUN: llvm-dis %t1.s.0.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s
+; CHECK-IMPORT: define available_externally void @f2()
 ; RUN: llvm-nm %t3.o | FileCheck --check-prefix=CHECK-OBJ %s
 ; CHECK-OBJ: T f1
 ; CHECK-OBJ-NOT: U f2
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -487,7 +487,8 @@
 
 static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
  DiagnosticsEngine &Diags,
- const TargetOptions &TargetOpts) {
+ const TargetOptions &TargetOpts,
+ const FrontendOptions &FrontendOpts) {
   bool Success = true;
   llvm::Triple Triple = llvm::Triple(TargetOpts.Triple);
 
@@ -739,6 +740,12 @@
   << A->getAsString(Args) << "-x ir";
 Opts.ThinLTOIndexFile = Args.getLastArgValue(OPT_fthinlto_index_EQ);
   }
+  if (Arg *A = Args.getLastArg(OPT_save_temps_EQ))
+Opts.SaveTempsFilePrefix =
+llvm::StringSwitch(A->getValue())
+.Case("obj", FrontendOpts.OutputFile)
+.Default(llvm::sys::path::filename(FrontendOpts.OutputFile).str());
+
   Opts.ThinLinkBitcodeFile = Args.getLastArgValue(OPT_fthin_link_bitcode_EQ);
 
   Opts.MSVolatile = Args.hasArg(OPT_fms_volatile);
@@ -2890,7 +2897,7 @@
   LangOpts.IsHeaderFile);
   ParseTargetArgs(Res.getTargetOpts(), Args, Diags);
   Success &= ParseCodeGenArgs(Res.getCodeGenOpts(), Args, DashX, Diags,
-  Res.getTargetOpts());
+  Res.getTargetOpts(), Res.getFrontendOpts());
   ParseHeaderSearchArgs(Res.getHeaderSearchOpts(), Args,
 Res.getFileSystemOpts().WorkingDir);
   if (DashX.getFormat() == InputKind::Precom

[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

2018-04-16 Thread Shuai Wang via Phabricator via cfe-commits
shuaiwang added a comment.

>>   I would imagine things could get messier if this check expands to also 
>> check for turning member functions const: it's basically checking 
>> CxxThisExpr, being a handle, is not modified within a member function, but 
>> note there's no VarDecl for "this".
> 
> Using my approach, you can check if any member variable is used as non-const. 
> Then mark this method as const, if it is not virtual.
>  Similar for member variables: private non-consts can be converted into 
> consts.

You'll also need to check:

- a member function calling another non-const member function -> *this can't be 
const
- *this is passed as non-const reference param to a function -> *this can't be 
const

Also when marking decls as "can't be const" we'll have to do record separately 
for modifying behaviors coming from different functions.
Which of course are all possible but code will get too complex than necessary 
IMO.

I think member variables are a separate topic: I think we should just treat 
them as globals and not check on them, the same argument that they can be 
accessed from multiple translation unit applies to global/namespace scoped 
variables and class scoped variables. We can only reliably check function/block 
scope variables.
(reliable meaning being able to achieve zero false positives with useful level 
of recall, false negatives are inevitable because whenever a modifiable 
reference/handle escape the current block/translation unit we'll have to assume 
it's modified.)

>>   It's also cleaner to follow member access chains, e.g. "v.a.b.c = 10" 
>> modifies "v". This one is particularly tricky because the modifying behavior 
>> happens at "c = 10" while the variable we'd like to mark as "can't be const" 
>> is arbitrary layers away, and MemberExpr alone doesn't warrant marking 
>> "can't be const" because that'll introduce way too many false negatives, ... 
>> and I haven't figure out a way to write a matcher to match 
>> memberExpr(hasObjectExpression(memberExpr(hasObjectExpression(... > layers deep> ... (VarDeclRef) ...), not to mention any layer could either 
>> member access or array subscript access.
> 
> Does your approach work with the nesting? Maybe something recursive or 
> `hasDescendent(modifying)` could do it?

Yes my approach is doing multi-pass matching by calling isModified() 
recursively. I consider the multi-pass matching "necessary evil" because 
otherwise we'll have too many false negatives.
I thought about hasDescendent (hasAncestor actually) but I don't think that 
makes things easier: varDeclRef(hasAncestor(modifying)) would match both 
"v.a.b.c. = 10" and "map[v] = 10" and we'll still need to double check the 
modifying behavior does link back to the particular varDeclRef.

Reviewers, what are your thoughts about the approaches?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45444



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


[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

2018-04-16 Thread Shuai Wang via Phabricator via cfe-commits
shuaiwang added a comment.

In https://reviews.llvm.org/D45444#1068967, @Eugene.Zelenko wrote:

> In https://reviews.llvm.org/D45444#1068496, @shuaiwang wrote:
>
> > - I would imagine things could get messier if this check expands to also 
> > check for turning member functions const: it's basically checking 
> > CxxThisExpr, being a handle, is not modified within a member function, but 
> > note there's no VarDecl for "this".
>
>
> Probably this should be separate check. See also PR21981.


Sure, a separate check sounds better.
Which makes even strong argument of having a reusable utility checking whether 
something is modified that can be shared between different checks :)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45444



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


[PATCH] D45217: [ThinLTO] Pass -save-temps to LTO backend for distributed ThinLTO builds

2018-04-16 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc added inline comments.



Comment at: include/clang/Driver/Options.td:2255
   HelpText<"Save intermediate compilation results.">;
-def save_temps : Flag<["-", "--"], "save-temps">, Flags<[DriverOption]>,
+def save_temps : Flag<["-", "--"], "save-temps">, Flags<[CC1Option, 
DriverOption]>,
   Alias, AliasArgs<["cwd"]>,

tejohnson wrote:
> pcc wrote:
> > This is still just a DriverOption, right?
> No, we need to pass to cc1 to get it into the CodeGenOptions. See the changes 
> to Clang.cpp (passes to cc1 invocation) and CompilerInvocation.cpp (consumes 
> during cc1 invocation).
You are producing a `-save-temps=` flag, not `-save-temps`. 

i.e. I think you should be able to remove the CC1Option from this line and keep 
it on line 2253.



Comment at: test/CodeGen/thinlto_backend.ll:30
 ; Ensure f2 was imported
-; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c 
-fthinlto-index=%t.thinlto.bc
+; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c 
-fthinlto-index=%t.thinlto.bc -save-temps=obj
 ; RUN: llvm-nm %t3.o | FileCheck --check-prefix=CHECK-OBJ %s

tejohnson wrote:
> pcc wrote:
> > Should there be another test for `-save-temps=cwd`?
> Added -save-temps=cwd and -save-temps
This isn't actually testing that the behaviour is different between 
`-save-temps=obj` and `-save-temps=cwd`, right?

Maybe create a new directory, cd there and verify that the files are created 
there?


Repository:
  rC Clang

https://reviews.llvm.org/D45217



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


[PATCH] D45217: [ThinLTO] Pass -save-temps to LTO backend for distributed ThinLTO builds

2018-04-16 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson marked 2 inline comments as done.
tejohnson added inline comments.



Comment at: include/clang/Driver/Options.td:2255
   HelpText<"Save intermediate compilation results.">;
-def save_temps : Flag<["-", "--"], "save-temps">, Flags<[DriverOption]>,
+def save_temps : Flag<["-", "--"], "save-temps">, Flags<[CC1Option, 
DriverOption]>,
   Alias, AliasArgs<["cwd"]>,

pcc wrote:
> tejohnson wrote:
> > pcc wrote:
> > > This is still just a DriverOption, right?
> > No, we need to pass to cc1 to get it into the CodeGenOptions. See the 
> > changes to Clang.cpp (passes to cc1 invocation) and CompilerInvocation.cpp 
> > (consumes during cc1 invocation).
> You are producing a `-save-temps=` flag, not `-save-temps`. 
> 
> i.e. I think you should be able to remove the CC1Option from this line and 
> keep it on line 2253.
Oh, I see what you are saying. Removed CC1Option from the plain -save-temps.



Comment at: test/CodeGen/thinlto_backend.ll:30
 ; Ensure f2 was imported
-; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c 
-fthinlto-index=%t.thinlto.bc
+; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c 
-fthinlto-index=%t.thinlto.bc -save-temps=obj
 ; RUN: llvm-nm %t3.o | FileCheck --check-prefix=CHECK-OBJ %s

pcc wrote:
> tejohnson wrote:
> > pcc wrote:
> > > Should there be another test for `-save-temps=cwd`?
> > Added -save-temps=cwd and -save-temps
> This isn't actually testing that the behaviour is different between 
> `-save-temps=obj` and `-save-temps=cwd`, right?
> 
> Maybe create a new directory, cd there and verify that the files are created 
> there?
Using different directories for all 3 cases now.


Repository:
  rC Clang

https://reviews.llvm.org/D45217



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


[PATCH] D45217: [ThinLTO] Pass -save-temps to LTO backend for distributed ThinLTO builds

2018-04-16 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson updated this revision to Diff 142669.
tejohnson marked 2 inline comments as done.
tejohnson added a comment.

Address comments


Repository:
  rC Clang

https://reviews.llvm.org/D45217

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/BackendUtil.cpp
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/thinlto_backend.ll
  test/Driver/thinlto_backend.c

Index: test/Driver/thinlto_backend.c
===
--- test/Driver/thinlto_backend.c
+++ test/Driver/thinlto_backend.c
@@ -5,6 +5,15 @@
 // RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -### 2>&1 | FileCheck %s -check-prefix=CHECK-THINLTOBE-ACTION
 // CHECK-THINLTOBE-ACTION: -fthinlto-index=
 
+// -save-temps should be passed to cc1
+// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -save-temps -### 2>&1 | FileCheck %s -check-prefix=CHECK-SAVE-TEMPS -check-prefix=CHECK-SAVE-TEMPS-CWD
+// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -save-temps=cwd -### 2>&1 | FileCheck %s -check-prefix=CHECK-SAVE-TEMPS -check-prefix=CHECK-SAVE-TEMPS-CWD
+// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -save-temps=obj -### 2>&1 | FileCheck %s -check-prefix=CHECK-SAVE-TEMPS -check-prefix=CHECK-SAVE-TEMPS-OBJ
+// CHECK-SAVE-TEMPS-NOT: -emit-llvm-bc
+// CHECK-SAVE-TEMPS-CWD: -save-temps=cwd
+// CHECK-SAVE-TEMPS-OBJ: -save-temps=obj
+// CHECK-SAVE-TEMPS-NOT: -emit-llvm-bc
+
 // Ensure clang driver gives the expected error for incorrect input type
 // RUN: not %clang -O2 -o %t1.o %s -c -fthinlto-index=%t.thinlto.bc 2>&1 | FileCheck %s -check-prefix=CHECK-WARNING
 // CHECK-WARNING: error: invalid argument '-fthinlto-index={{.*}}' only allowed with '-x ir'
Index: test/CodeGen/thinlto_backend.ll
===
--- test/CodeGen/thinlto_backend.ll
+++ test/CodeGen/thinlto_backend.ll
@@ -26,8 +26,16 @@
 ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t4.o -x ir %t5.o -c -fthinlto-index=%t.thinlto.bc
 ; RUN: llvm-nm %t4.o | count 0
 
-; Ensure f2 was imported
-; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc
+; Ensure f2 was imported. Check for all 3 flavors of -save-temps[=cwd|obj].
+; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps=obj
+; RUN: llvm-dis %t1.s.0.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s
+; RUN: mkdir -p %T/dir1
+; RUN: (cd %T/dir1 && %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps=cwd)
+; RUN: llvm-dis %T/dir1/*1.s.0.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s
+; RUN: mkdir -p %T/dir2
+; RUN: (cd %T/dir2 && %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps)
+; RUN: llvm-dis %T/dir2/*1.s.0.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s
+; CHECK-IMPORT: define available_externally void @f2()
 ; RUN: llvm-nm %t3.o | FileCheck --check-prefix=CHECK-OBJ %s
 ; CHECK-OBJ: T f1
 ; CHECK-OBJ-NOT: U f2
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -487,7 +487,8 @@
 
 static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
  DiagnosticsEngine &Diags,
- const TargetOptions &TargetOpts) {
+ const TargetOptions &TargetOpts,
+ const FrontendOptions &FrontendOpts) {
   bool Success = true;
   llvm::Triple Triple = llvm::Triple(TargetOpts.Triple);
 
@@ -739,6 +740,12 @@
   << A->getAsString(Args) << "-x ir";
 Opts.ThinLTOIndexFile = Args.getLastArgValue(OPT_fthinlto_index_EQ);
   }
+  if (Arg *A = Args.getLastArg(OPT_save_temps_EQ))
+Opts.SaveTempsFilePrefix =
+llvm::StringSwitch(A->getValue())
+.Case("obj", FrontendOpts.OutputFile)
+.Default(llvm::sys::path::filename(FrontendOpts.OutputFile).str());
+
   Opts.ThinLinkBitcodeFile = Args.getLastArgValue(OPT_fthin_link_bitcode_EQ);
 
   Opts.MSVolatile = Args.hasArg(OPT_fms_volatile);
@@ -2890,7 +2897,7 @@
   LangOpts.IsHeaderFile);
   ParseTargetArgs(Res.getTargetOpts(), Args, Diags);
   Success &= ParseCodeGenArgs(Res.getCodeGenOpts(), Args, DashX, Diags,
-  Res.getTargetOpts());
+  Res.getTargetOpts(), Res.getFrontendOpts());
   ParseHeaderSearchArgs(Res.getHeaderSearchOpts(), Args,
 Res.getFileSystemOpts().WorkingDir);
   if (DashX.getFormat() == InputKind::Precompiled ||
Index: lib/Driver/Too

[PATCH] D45697: [clang-tidy] Fix clang-tidy doesn't read .clangtidy configuration file.

2018-04-16 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
hokein added a reviewer: alexfh.
Herald added subscribers: xazax.hun, klimek.

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


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45697

Files:
  clang-tidy/ClangTidyOptions.cpp
  clang-tidy/ClangTidyOptions.h
  clang-tidy/tool/ClangTidyMain.cpp
  test/clang-tidy/read_file_config.cpp

Index: test/clang-tidy/read_file_config.cpp
===
--- /dev/null
+++ test/clang-tidy/read_file_config.cpp
@@ -0,0 +1,11 @@
+// RUN: mkdir -p %T/read-file-config/
+// RUN: cp %s %T/read-file-config/test.cpp
+// RUN: echo 'Checks: "-*,modernize-use-nullptr"' > %T/read-file-config/.clang-tidy
+// RUN: echo '[{"command": "cc -c -o test.o test.cpp", "directory": "%T/read-file-config", "file": "%T/read-file-config/test.cpp"}]' > %T/read-file-config/compile_commands.json
+// RUN: clang-tidy %T/read-file-config/test.cpp | not grep "clang-analyzer-*"
+
+void f() {
+  int x;
+  x = 1;
+  x = 2;
+}
Index: clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -286,7 +286,8 @@
   OS.flush();
 }
 
-static std::unique_ptr createOptionsProvider() {
+static std::unique_ptr createOptionsProvider(
+   llvm::IntrusiveRefCntPtr FS) {
   ClangTidyGlobalOptions GlobalOptions;
   if (std::error_code Err = parseLineFilter(LineFilter, GlobalOptions)) {
 llvm::errs() << "Invalid LineFilter: " << Err.message() << "\n\nUsage:\n";
@@ -334,7 +335,7 @@
 }
   }
   return llvm::make_unique(GlobalOptions, DefaultOptions,
-OverrideOptions);
+OverrideOptions, std::move(FS));
 }
 
 llvm::IntrusiveRefCntPtr
@@ -364,8 +365,13 @@
 static int clangTidyMain(int argc, const char **argv) {
   CommonOptionsParser OptionsParser(argc, argv, ClangTidyCategory,
 cl::ZeroOrMore);
+  llvm::IntrusiveRefCntPtr BaseFS(
+  VfsOverlay.empty() ? vfs::getRealFileSystem()
+ : getVfsOverlayFromFile(VfsOverlay));
+  if (!BaseFS)
+return 1;
 
-  auto OwningOptionsProvider = createOptionsProvider();
+  auto OwningOptionsProvider = createOptionsProvider(BaseFS);
   auto *OptionsProvider = OwningOptionsProvider.get();
   if (!OptionsProvider)
 return 1;
@@ -432,12 +438,6 @@
 llvm::cl::PrintHelpMessage(/*Hidden=*/false, /*Categorized=*/true);
 return 1;
   }
-  llvm::IntrusiveRefCntPtr BaseFS(
-  VfsOverlay.empty() ? vfs::getRealFileSystem()
- : getVfsOverlayFromFile(VfsOverlay));
-  if (!BaseFS)
-return 1;
-
   ProfileData Profile;
 
   llvm::InitializeAllTargetInfos();
Index: clang-tidy/ClangTidyOptions.h
===
--- clang-tidy/ClangTidyOptions.h
+++ clang-tidy/ClangTidyOptions.h
@@ -13,7 +13,9 @@
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/Support/ErrorOr.h"
+#include "clang/Basic/VirtualFileSystem.h"
 #include 
 #include 
 #include 
@@ -221,7 +223,8 @@
   /// whatever options are read from the configuration file.
   FileOptionsProvider(const ClangTidyGlobalOptions &GlobalOptions,
   const ClangTidyOptions &DefaultOptions,
-  const ClangTidyOptions &OverrideOptions);
+  const ClangTidyOptions &OverrideOptions,
+  llvm::IntrusiveRefCntPtr FS = nullptr);
 
   /// \brief Initializes the \c FileOptionsProvider instance with a custom set
   /// of configuration file handlers.
@@ -255,6 +258,7 @@
   llvm::StringMap CachedOptions;
   ClangTidyOptions OverrideOptions;
   ConfigFileHandlers ConfigHandlers;
+  llvm::IntrusiveRefCntPtr FS;
 };
 
 /// \brief Parses LineFilter from JSON and stores it to the \p Options.
Index: clang-tidy/ClangTidyOptions.cpp
===
--- clang-tidy/ClangTidyOptions.cpp
+++ clang-tidy/ClangTidyOptions.cpp
@@ -204,9 +204,12 @@
 FileOptionsProvider::FileOptionsProvider(
 const ClangTidyGlobalOptions &GlobalOptions,
 const ClangTidyOptions &DefaultOptions,
-const ClangTidyOptions &OverrideOptions)
+const ClangTidyOptions &OverrideOptions,
+llvm::IntrusiveRefCntPtr VFS)
 : DefaultOptionsProvider(GlobalOptions, DefaultOptions),
-  OverrideOptions(OverrideOptions) {
+  OverrideOptions(OverrideOptions), FS(std::move(VFS)) {
+  if (!FS)
+FS = vfs::getRealFileSystem();
   ConfigHandlers.emplace_back(".clang-tidy", parseConfiguration);
 }
 
@@ -224,14 +227,19 @@
 std::vector
 FileOptionsProvider::getRawOptions(StringRef FileName) {
   DEBUG(llvm::dbgs() << "Getting options for file " << FileName << "...\n");
+  assert(FS && "FS must be set.");
+
+  ll

[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

2018-04-16 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a subscriber: lebedev.ri.
dblaikie added a comment.

I'm not sure this is a practical direction to pursue - though perhaps
others disagree.

It's likely non-trivial to plumb a flag through most build systems to be
applied only to test code (& likely would suppress the warning in headers
only included in test code - so for example, in a header-only library if
the user ran their tests they wouldn't see the warning, but when the users
code was used in some other production code it might warn (& possibly break
the build if -Werror is in use))


Repository:
  rC Clang

https://reviews.llvm.org/D45685



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


Re: [PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

2018-04-16 Thread David Blaikie via cfe-commits
I'm not sure this is a practical direction to pursue - though perhaps
others disagree.

It's likely non-trivial to plumb a flag through most build systems to be
applied only to test code (& likely would suppress the warning in headers
only included in test code - so for example, in a header-only library if
the user ran their tests they wouldn't see the warning, but when the users
code was used in some other production code it might warn (& possibly break
the build if -Werror is in use))

On Mon, Apr 16, 2018 at 8:10 AM Roman Lebedev via Phabricator <
revi...@reviews.llvm.org> wrote:

> lebedev.ri updated this revision to Diff 142636.
> lebedev.ri retitled this revision from "[Sema] Add -Wtest global flag that
> silences -Wself-assign for overloaded operators." to "[Sema] Add -wtest
> global flag that silences -Wself-assign for overloaded operators.".
> lebedev.ri edited the summary of this revision.
> lebedev.ri added a comment.
>
> Actually make it `-wtest` (all lowercase).
>
>
> Repository:
>   rC Clang
>
> https://reviews.llvm.org/D45685
>
> Files:
>   docs/ReleaseNotes.rst
>   docs/UsersManual.rst
>   include/clang/AST/ASTDiagnostic.h
>   include/clang/AST/CommentDiagnostic.h
>   include/clang/Basic/Diagnostic.h
>   include/clang/Basic/Diagnostic.td
>   include/clang/Basic/DiagnosticIDs.h
>   include/clang/Basic/DiagnosticOptions.def
>   include/clang/Basic/DiagnosticSemaKinds.td
>   include/clang/CrossTU/CrossTUDiagnostic.h
>   include/clang/Driver/DriverDiagnostic.h
>   include/clang/Driver/Options.td
>   include/clang/Frontend/FrontendDiagnostic.h
>   include/clang/Lex/LexDiagnostic.h
>   include/clang/Parse/ParseDiagnostic.h
>   include/clang/Sema/SemaDiagnostic.h
>   include/clang/Serialization/SerializationDiagnostic.h
>   include/clang/Tooling/Refactoring/RefactoringDiagnostic.h
>   lib/Basic/DiagnosticIDs.cpp
>   lib/Basic/Warnings.cpp
>   lib/Driver/Driver.cpp
>   lib/Frontend/CompilerInvocation.cpp
>   lib/Sema/SemaExpr.cpp
>   test/Sema/warn-self-assign-builtin-warn-test.c
>   test/SemaCXX/warn-self-assign-builtin-warn-test.cpp
>   test/SemaCXX/warn-self-assign-field-builtin-warn-test.cpp
>   test/SemaCXX/warn-self-assign-field-overloaded-warn-test.cpp
>   test/SemaCXX/warn-self-assign-overloaded-warn-test.cpp
>   tools/diagtool/DiagnosticNames.cpp
>   utils/TableGen/ClangDiagnosticsEmitter.cpp
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r330132 - [CodeView] Initial support for emitting S_THUNK32 symbols for compiler...

2018-04-16 Thread Brock Wyma via cfe-commits
Author: bwyma
Date: Mon Apr 16 09:53:57 2018
New Revision: 330132

URL: http://llvm.org/viewvc/llvm-project?rev=330132&view=rev
Log:
[CodeView] Initial support for emitting S_THUNK32 symbols for compiler...

When emitting CodeView debug information, compiler-generated thunk routines
should be emitted using S_THUNK32 symbols instead of S_GPROC32_ID symbols so
Visual Studio can properly step into the user code.  This initial support only
handles standard thunk ordinals.

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


Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.h
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/test/CodeGenCXX/debug-info-thunk.cpp

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=330132&r1=330131&r2=330132&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Mon Apr 16 09:53:57 2018
@@ -3217,7 +3217,8 @@ llvm::DISubroutineType *CGDebugInfo::get
 
 void CGDebugInfo::EmitFunctionStart(GlobalDecl GD, SourceLocation Loc,
 SourceLocation ScopeLoc, QualType FnType,
-llvm::Function *Fn, CGBuilderTy &Builder) {
+llvm::Function *Fn, bool CurFuncIsThunk,
+CGBuilderTy &Builder) {
 
   StringRef Name;
   StringRef LinkageName;
@@ -3263,6 +3264,10 @@ void CGDebugInfo::EmitFunctionStart(Glob
 // Artificial functions should not silently reuse CurLoc.
 CurLoc = SourceLocation();
   }
+
+  if (CurFuncIsThunk)
+Flags |= llvm::DINode::FlagThunk;
+
   unsigned LineNo = getLineNumber(Loc);
   unsigned ScopeLine = getLineNumber(ScopeLoc);
 

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.h?rev=330132&r1=330131&r2=330132&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.h (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.h Mon Apr 16 09:53:57 2018
@@ -366,7 +366,8 @@ public:
   /// \param ScopeLoc  The location of the function body.
   void EmitFunctionStart(GlobalDecl GD, SourceLocation Loc,
  SourceLocation ScopeLoc, QualType FnType,
- llvm::Function *Fn, CGBuilderTy &Builder);
+ llvm::Function *Fn, bool CurFnIsThunk,
+ CGBuilderTy &Builder);
 
   /// Start a new scope for an inlined function.
   void EmitInlineFunctionStart(CGBuilderTy &Builder, GlobalDecl GD);

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=330132&r1=330131&r2=330132&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Mon Apr 16 09:53:57 2018
@@ -1018,7 +1018,8 @@ void CodeGenFunction::StartFunction(Glob
   ArgTypes.push_back(VD->getType());
 QualType FnType = getContext().getFunctionType(
 RetTy, ArgTypes, FunctionProtoType::ExtProtoInfo(CC));
-DI->EmitFunctionStart(GD, Loc, StartLoc, FnType, CurFn, Builder);
+DI->EmitFunctionStart(GD, Loc, StartLoc, FnType, CurFn, CurFuncIsThunk,
+  Builder);
   }
 
   if (ShouldInstrumentFunction()) {

Modified: cfe/trunk/test/CodeGenCXX/debug-info-thunk.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-thunk.cpp?rev=330132&r1=330131&r2=330132&view=diff
==
--- cfe/trunk/test/CodeGenCXX/debug-info-thunk.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/debug-info-thunk.cpp Mon Apr 16 09:53:57 2018
@@ -1,29 +1,277 @@
-// RUN: %clang_cc1 %s -triple %itanium_abi_triple -debug-info-kind=limited -S 
-emit-llvm -o - | FileCheck %s
-
-struct A {
-  virtual void f();
-};
-
-struct B {
-  virtual void f();
-};
-
-struct C : A, B {
-  virtual void f();
-};
-
-void C::f() { }
-// CHECK: define {{.*}}void @_ZThn{{[48]}}_N1C1fEv
-// CHECK-SAME: !dbg ![[SP:[0-9]+]]
-// CHECK-NOT: {{ret }}
-// CHECK: = load{{.*}} !dbg ![[DBG:[0-9]+]]
-// CHECK-NOT: {{ret }}
-// CHECK: ret void, !dbg ![[DBG]]
+// RUN: %clang_cc1 %s -triple=x86_64-pc-windows-msvc -debug-info-kind=limited 
-S -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -triple %itanium_abi_triple -debug-info-kind=limited -S 
-emit-llvm -o - | FileCheck %s -check-prefix=ITANIUM
+//
+// Validate we emit a "DIFlagThunk" flag on DISubprogram entries for thunks.
+// This flag is used for emitting S_THUNK32 symbols for CodeView debugging.
+//
+// NOTE:
+// Because thunks are compiler generated and don't exist in the source, 

[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

2018-04-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In https://reviews.llvm.org/D45685#1069040, @dblaikie wrote:

> I'm not sure this is a practical direction to pursue - though perhaps
>  others disagree.




> It's likely non-trivial to plumb a flag through most build systems to be
>  applied only to test code

I'm sorry, I don't understand.

If you don't separate between source code and `*_test.cpp` sources, sure,
you will loose the warning coverage either way.

What difference is there between passing `-wtest` and passing 
`-Wno-self-assign-overloaded`?
Just pass it alongside with the googletest include paths so to speak.

> (& likely would suppress the warning in headers
>  only included in test code - so for example, in a header-only library if
>  the user ran their tests they wouldn't see the warning, but when the users
>  code was used in some other production code it might warn (& possibly break
>  the build if -Werror is in use))

Could you please explain how it is not an issue if this would be a 
`-Wno-self-assign-overloaded`?


Repository:
  rC Clang

https://reviews.llvm.org/D45685



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


[PATCH] D45383: Limit types of builtins that can be redeclared.

2018-04-16 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

We can could add an exception to the "don't allow redeclarations with custom 
type-checking" rule to specifically allow redeclaring `__va_start`.  The 
general rule is that we don't want user code redeclaring them because they 
don't have a specific signature, so our internal representation could change 
between releases.  But `__va_start` has a specific fixed signature from the 
Microsoft SDK headers, so it should be fine to allow redeclaring it without a 
warning.


https://reviews.llvm.org/D45383



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


Re: [PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

2018-04-16 Thread David Blaikie via cfe-commits
On Mon, Apr 16, 2018 at 12:08 PM Roman Lebedev via Phabricator <
revi...@reviews.llvm.org> wrote:

> lebedev.ri added a comment.
>
> In https://reviews.llvm.org/D45685#1069040, @dblaikie wrote:
>
> > I'm not sure this is a practical direction to pursue - though perhaps
> >  others disagree.
>
>
>
>
> > It's likely non-trivial to plumb a flag through most build systems to be
> >  applied only to test code
>
> I'm sorry, I don't understand.
>
> If you don't separate between source code and `*_test.cpp` sources, sure,
> you will loose the warning coverage either way.
>
> What difference is there between passing `-wtest` and passing
> `-Wno-self-assign-overloaded`?
>

Not much, if any.


> Just pass it alongside with the googletest include paths so to speak.
>

But build systems don't necessarily expose that kind of ability. (For
example, googletest (not the only kind of test suite/code) is checked into
the LLVM codebase like another library - so depending on it is just another
library dependency, not a special "test" library dependency).


>
> > (& likely would suppress the warning in headers
> >  only included in test code - so for example, in a header-only library if
> >  the user ran their tests they wouldn't see the warning, but when the
> users
> >  code was used in some other production code it might warn (& possibly
> break
> >  the build if -Werror is in use))
>
> Could you please explain how it is not an issue if this would be a
> `-Wno-self-assign-overloaded`?
>

It isn't any different - though it sounds like "-wtest" is being proposed
as a way that test code could specifically be corrected without impacting
the rest of the code. I'm not sure that's true/viable/accessible to most
developers, so the justification seems a bit infeasible - likely naming it
-Wno-self-assign-overloaded would at least name it how it'll be used, to
turn the warning off across a whole codebase because the false positive
rate across the whole codebase is unacceptable for a given user.


>
>
> Repository:
>   rC Clang
>
> https://reviews.llvm.org/D45685
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45456: [Attr] Print enum attributes at correct position

2018-04-16 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny updated this revision to Diff 142672.
jdenny edited the summary of this revision.
jdenny added a comment.

Rebased onto a more recent master.

Added example to summary.


https://reviews.llvm.org/D45456

Files:
  lib/AST/DeclPrinter.cpp
  test/Sema/ast-print.c


Index: test/Sema/ast-print.c
===
--- test/Sema/ast-print.c
+++ test/Sema/ast-print.c
@@ -1,5 +1,12 @@
-// RUN: %clang_cc1 %s -ast-print | FileCheck %s
-// RUN: %clang_cc1 %s -ast-print | %clang_cc1 -fsyntax-only -
+// RUN: %clang_cc1 %s -ast-print -verify > %t.c
+// RUN: FileCheck %s --input-file %t.c
+//
+// RUN: echo >> %t.c "// expected""-warning@* {{use of GNU old-style field 
designator extension}}"
+// RUN: echo >> %t.c "// expected""-warning@* {{'EnumWithAttributes' is 
deprecated}}"
+// RUN: echo >> %t.c "// expected""-note@* {{'EnumWithAttributes' has been 
explicitly marked deprecated here}}"
+// RUN: echo >> %t.c "// expected""-warning@* {{'EnumWithAttributes3' is 
deprecated}}"
+// RUN: echo >> %t.c "// expected""-note@* {{'EnumWithAttributes3' has been 
explicitly marked deprecated here}}"
+// RUN: %clang_cc1 -fsyntax-only %t.c -verify
 
 typedef void func_typedef();
 func_typedef xxx;
@@ -58,7 +65,7 @@
 };
 
 // CHECK: struct pair_t p = {a: 3, .b = 4};
-struct pair_t p = {a: 3, .b = 4};
+struct pair_t p = {a: 3, .b = 4}; // expected-warning {{use of GNU old-style 
field designator extension}}
 
 void initializers() {
   // CHECK: int *x = ((void *)0), *y = ((void *)0);
@@ -70,11 +77,30 @@
   } z = {(struct Z){}};
 }
 
-// CHECK-LABEL: enum EnumWithAttributes {
-enum EnumWithAttributes {
+// CHECK-LABEL: enum __attribute__((deprecated(""))) EnumWithAttributes {
+enum EnumWithAttributes { // expected-warning {{'EnumWithAttributes' is 
deprecated}}
   // CHECK-NEXT: EnumWithAttributesFoo __attribute__((deprecated(""))),
   EnumWithAttributesFoo __attribute__((deprecated)),
   // CHECK-NEXT: EnumWithAttributesBar __attribute__((unavailable(""))) = 50
   EnumWithAttributesBar __attribute__((unavailable)) = 50
-  // CHECK-NEXT: } __attribute__((deprecated("")))
-} __attribute__((deprecated));
+  // CHECK-NEXT: };
+  // CHECK-NEXT: enum EnumWithAttributes *EnumWithAttributesPtr;
+} __attribute__((deprecated)) *EnumWithAttributesPtr; // expected-note 
{{'EnumWithAttributes' has been explicitly marked deprecated here}}
+
+// FIXME: If enum is forward-declared at file scope, attributes are lost.
+// CHECK-LABEL: enum EnumWithAttributes2 *EnumWithAttributes2Ptr;
+// expected-warning@+2 {{'EnumWithAttributes2' is deprecated}}
+// expected-note@+1 {{'EnumWithAttributes2' has been explicitly marked 
deprecated here}}
+enum __attribute__((deprecated)) EnumWithAttributes2 *EnumWithAttributes2Ptr;
+
+// CHECK-LABEL: EnumWithAttributes3Fn
+void EnumWithAttributes3Fn() {
+  // CHECK-NEXT: enum __attribute__((deprecated(""))) EnumWithAttributes3 
*EnumWithAttributes3Ptr;
+  // expected-warning@+2 {{'EnumWithAttributes3' is deprecated}}
+  // expected-note@+1 {{'EnumWithAttributes3' has been explicitly marked 
deprecated here}}
+  enum __attribute__((deprecated)) EnumWithAttributes3 *EnumWithAttributes3Ptr;
+  // Printing must not put the attribute after the tag where it would apply to
+  // the variable instead of the type, and then our deprecation warning would
+  // move to this use of the variable.
+  void *p = EnumWithAttributes3Ptr;
+}
Index: lib/AST/DeclPrinter.cpp
===
--- lib/AST/DeclPrinter.cpp
+++ lib/AST/DeclPrinter.cpp
@@ -496,14 +496,17 @@
 void DeclPrinter::VisitEnumDecl(EnumDecl *D) {
   if (!Policy.SuppressSpecifiers && D->isModulePrivate())
 Out << "__module_private__ ";
-  Out << "enum ";
+  Out << "enum";
   if (D->isScoped()) {
 if (D->isScopedUsingClassTag())
-  Out << "class ";
+  Out << " class";
 else
-  Out << "struct ";
+  Out << " struct";
   }
-  Out << *D;
+
+  prettyPrintAttributes(D);
+
+  Out << ' ' << *D;
 
   if (D->isFixed() && D->getASTContext().getLangOpts().CPlusPlus11)
 Out << " : " << D->getIntegerType().stream(Policy);
@@ -513,7 +516,6 @@
 VisitDeclContext(D);
 Indent() << "}";
   }
-  prettyPrintAttributes(D);
 }
 
 void DeclPrinter::VisitRecordDecl(RecordDecl *D) {


Index: test/Sema/ast-print.c
===
--- test/Sema/ast-print.c
+++ test/Sema/ast-print.c
@@ -1,5 +1,12 @@
-// RUN: %clang_cc1 %s -ast-print | FileCheck %s
-// RUN: %clang_cc1 %s -ast-print | %clang_cc1 -fsyntax-only -
+// RUN: %clang_cc1 %s -ast-print -verify > %t.c
+// RUN: FileCheck %s --input-file %t.c
+//
+// RUN: echo >> %t.c "// expected""-warning@* {{use of GNU old-style field designator extension}}"
+// RUN: echo >> %t.c "// expected""-warning@* {{'EnumWithAttributes' is deprecated}}"
+// RUN: echo >> %t.c "// expected""-note@* {{'EnumWithAttributes' has been explicitly marked deprecated here}}"
+// RUN: echo >>

r330150 - [Hexagon] Emit a warning when -fvectorize is given without -mhvx

2018-04-16 Thread Krzysztof Parzyszek via cfe-commits
Author: kparzysz
Date: Mon Apr 16 12:11:17 2018
New Revision: 330150

URL: http://llvm.org/viewvc/llvm-project?rev=330150&view=rev
Log:
[Hexagon] Emit a warning when -fvectorize is given without -mhvx

Modified:
cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp
cfe/trunk/lib/Driver/ToolChains/Hexagon.h
cfe/trunk/test/Driver/hexagon-vectorize.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=330150&r1=330149&r2=330150&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Mon Apr 16 12:11:17 
2018
@@ -288,6 +288,9 @@ def err_analyzer_config_multiple_values
 
 def err_drv_invalid_hvx_length : Error<
   "-mhvx-length is not supported without a -mhvx/-mhvx= flag">;
+def warn_drv_vectorize_needs_hvx : Warning<
+  "auto-vectorization requires HVX, use -mhvx to enable it">,
+  InGroup;
 
 def err_drv_modules_validate_once_requires_timestamp : Error<
   "option '-fmodules-validate-once-per-build-session' requires "

Modified: cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp?rev=330150&r1=330149&r2=330150&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp Mon Apr 16 12:11:17 2018
@@ -108,8 +108,11 @@ void hexagon::getHexagonTargetFeatures(c
 
   Features.push_back(UseLongCalls ? "+long-calls" : "-long-calls");
 
-  bool HasHVX(false);
+  bool HasHVX = false;
   handleHVXTargetFeatures(D, Args, Features, HasHVX);
+
+  if (HexagonToolChain::isAutoHVXEnabled(Args) && !HasHVX)
+D.Diag(diag::warn_drv_vectorize_needs_hvx);
 }
 
 // Hexagon tools start.
@@ -520,12 +523,9 @@ void HexagonToolChain::addClangTargetOpt
 CC1Args.push_back("-target-feature");
 CC1Args.push_back("+reserved-r19");
   }
-  if (Arg *A = DriverArgs.getLastArg(options::OPT_fvectorize,
- options::OPT_fno_vectorize)) {
-if (A->getOption().matches(options::OPT_fvectorize)) {
-  CC1Args.push_back("-mllvm");
-  CC1Args.push_back("-hexagon-autohvx");
-}
+  if (isAutoHVXEnabled(DriverArgs)) {
+CC1Args.push_back("-mllvm");
+CC1Args.push_back("-hexagon-autohvx");
   }
 }
 
@@ -564,6 +564,13 @@ HexagonToolChain::GetCXXStdlibType(const
   return ToolChain::CST_Libstdcxx;
 }
 
+bool HexagonToolChain::isAutoHVXEnabled(const llvm::opt::ArgList &Args) {
+  if (Arg *A = Args.getLastArg(options::OPT_fvectorize,
+   options::OPT_fno_vectorize))
+return A->getOption().matches(options::OPT_fvectorize);
+  return false;
+}
+
 //
 // Returns the default CPU for Hexagon. This is the default compilation target
 // if no Hexagon processor is selected at the command-line.

Modified: cfe/trunk/lib/Driver/ToolChains/Hexagon.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Hexagon.h?rev=330150&r1=330149&r2=330150&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Hexagon.h (original)
+++ cfe/trunk/lib/Driver/ToolChains/Hexagon.h Mon Apr 16 12:11:17 2018
@@ -94,6 +94,7 @@ public:
   void getHexagonLibraryPaths(const llvm::opt::ArgList &Args,
   ToolChain::path_list &LibPaths) const;
 
+  static bool isAutoHVXEnabled(const llvm::opt::ArgList &Args);
   static const StringRef GetDefaultCPU();
   static const StringRef GetTargetCPUVersion(const llvm::opt::ArgList &Args);
 

Modified: cfe/trunk/test/Driver/hexagon-vectorize.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/hexagon-vectorize.c?rev=330150&r1=330149&r2=330150&view=diff
==
--- cfe/trunk/test/Driver/hexagon-vectorize.c (original)
+++ cfe/trunk/test/Driver/hexagon-vectorize.c Mon Apr 16 12:11:17 2018
@@ -1,7 +1,9 @@
 // RUN: %clang -target hexagon -### %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-DEFAULT
 // RUN: %clang -target hexagon -fvectorize -### %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-VECTOR
 // RUN: %clang -target hexagon -fvectorize -fno-vectorize -### %s 2>&1 | 
FileCheck %s --check-prefix=CHECK-NOVECTOR
+// RUN: %clang -target hexagon -fvectorize -### %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-NEEDHVX
 
 // CHECK-DEFAULT-NOT: hexagon-autohvx
 // CHECK-VECTOR: "-mllvm" "-hexagon-autohvx"
 // CHECK-NOVECTOR-NOT: hexagon-autohvx
+// CHECK-NEEDHVX: warning: auto-vectorization requires HVX, use -mhvx to 
enable it


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

[PATCH] D45383: Limit types of builtins that can be redeclared.

2018-04-16 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

In https://reviews.llvm.org/D45383#1069049, @efriedma wrote:

> We can could add an exception to the "don't allow redeclarations with custom 
> type-checking" rule to specifically allow redeclaring `__va_start`.  The 
> general rule is that we don't want user code redeclaring them because they 
> don't have a specific signature, so our internal representation could change 
> between releases.  But `__va_start` has a specific fixed signature from the 
> Microsoft SDK headers, so it should be fine to allow redeclaring it without a 
> warning.


That still doesn't fix the bug, since redeclaring __va_start in 'C' mode when 
va_list is a reference type causes the assert.


https://reviews.llvm.org/D45383



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


[PATCH] D45383: Limit types of builtins that can be redeclared.

2018-04-16 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

I don't see an `A` in `LANGBUILTIN(__va_start,   "vc**.", "nt", 
ALL_MS_LANGUAGES)`


https://reviews.llvm.org/D45383



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


[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

2018-04-16 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

> You'll also need to check:
> 
> - a member function calling another non-const member function -> *this can't 
> be const
> - *this is passed as non-const reference param to a function -> *this can't 
> be const

Yes. That is similar behaviour to checking if a function can be `noexcept`. But 
this is all future stuff :)

> Also when marking decls as "can't be const" we'll have to do record 
> separately for modifying behaviors coming from different functions.
>  Which of course are all possible but code will get too complex than 
> necessary IMO.

I cant follow you on that one. I consider every path that allows future 
modification (like taking a non-const reference/pointer) as `cant be const`. 
That would be enough, wouldnt it?
A separate function can only modify a variable if it has some form of non-const 
handle to it, which must have been created at some point.

> I think member variables are a separate topic: I think we should just treat 
> them as globals and not check on them, the same argument that they can be 
> accessed from multiple translation unit applies to global/namespace scoped 
> variables and class scoped variables. We can only reliably check 
> function/block scope variables.
>  (reliable meaning being able to achieve zero false positives with useful 
> level of recall, false negatives are inevitable because whenever a modifiable 
> reference/handle escape the current block/translation unit we'll have to 
> assume it's modified.)

You are probably right. The only zero-false positive case is: "only const 
methods called". One could split implementation of a class into several 
translation units and render the analysis approach useless.

> Yes my approach is doing multi-pass matching by calling isModified() 
> recursively. I consider the multi-pass matching "necessary evil" because 
> otherwise we'll have too many false negatives.
>  I thought about hasDescendent (hasAncestor actually) but I don't think that 
> makes things easier: varDeclRef(hasAncestor(modifying)) would match both 
> "v.a.b.c. = 10" and "map[v] = 10" and we'll still need to double check the 
> modifying behavior does link back to the particular varDeclRef.

Example as reference for later: https://godbolt.org/g/cvhoUN
I will add such cases to my tests.

@shuaiwang Are you in IRC from time to time? I think it will be better to 
discuss changes in chat, when i start to introduce your approach here.

> isModified(Expr, Stmt), that checks whether Expr is (potentially) modified 
> within Stmt, is something I believe to be quite useful:

What i understand from your comments and code:

- implement a utility taking `DeclRefExpr` and check if there is modification 
in some scope (`Stmt`, e.g. `CompoundStmt`) -> `isModified` or 
`cantBeModifiedWithin`
- match all relevant non-const variable declarations as potential candidates
- track all uses of the candidates and check for modification in their scope

One Note: We could include variables in unnamed namespaces and `static` 
declared variables. They have TU scope.

My deviations:

- a variable should be considered modified if a non-const handle is create from 
it, even if that handle does not modify its referenced value. As first step, 
those handles should be marked const, then the original value can be marked as 
const. That is required to produce compiling code after potential 
code-transformation.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45444



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


[PATCH] D45383: Limit types of builtins that can be redeclared.

2018-04-16 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

In https://reviews.llvm.org/D45383#1069057, @efriedma wrote:

> I don't see an `A` in `LANGBUILTIN(__va_start,   "vc**.", "nt", 
> ALL_MS_LANGUAGES)`


Ah, you're right!  I was confusing it with LIBBUILTIN(va_start, "vA.", "fn", 
"stdarg.h", ALL_LANGUAGES) and BUILTIN(__builtin_va_start, "vA.", "nt").

It seems that the MS builtins dont' actually use 'A'.  I'll implement your 
suggestion, thanks for your patience :)


https://reviews.llvm.org/D45383



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


[PATCH] D45463: [AST] Print correct tag decl for tag specifier

2018-04-16 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny updated this revision to Diff 142673.
jdenny added a comment.

Rebased onto a more recent master.


https://reviews.llvm.org/D45463

Files:
  include/clang-c/Index.h
  include/clang/AST/PrettyPrinter.h
  lib/AST/DeclPrinter.cpp
  lib/AST/TypePrinter.cpp
  test/Misc/ast-print-enum-decl.c
  test/Misc/ast-print-record-decl.c
  tools/c-index-test/c-index-test.c
  tools/libclang/CIndex.cpp

Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -4739,8 +4739,6 @@
 return P->SuppressSpecifiers;
   case CXPrintingPolicy_SuppressTagKeyword:
 return P->SuppressTagKeyword;
-  case CXPrintingPolicy_IncludeTagDefinition:
-return P->IncludeTagDefinition;
   case CXPrintingPolicy_SuppressScope:
 return P->SuppressScope;
   case CXPrintingPolicy_SuppressUnwrittenScope:
@@ -4808,9 +4806,6 @@
   case CXPrintingPolicy_SuppressTagKeyword:
 P->SuppressTagKeyword = Value;
 return;
-  case CXPrintingPolicy_IncludeTagDefinition:
-P->IncludeTagDefinition = Value;
-return;
   case CXPrintingPolicy_SuppressScope:
 P->SuppressScope = Value;
 return;
Index: tools/c-index-test/c-index-test.c
===
--- tools/c-index-test/c-index-test.c
+++ tools/c-index-test/c-index-test.c
@@ -97,8 +97,6 @@
CXPrintingPolicy_SuppressSpecifiers},
   {"CINDEXTEST_PRINTINGPOLICY_SUPPRESSTAGKEYWORD",
CXPrintingPolicy_SuppressTagKeyword},
-  {"CINDEXTEST_PRINTINGPOLICY_INCLUDETAGDEFINITION",
-   CXPrintingPolicy_IncludeTagDefinition},
   {"CINDEXTEST_PRINTINGPOLICY_SUPPRESSSCOPE",
CXPrintingPolicy_SuppressScope},
   {"CINDEXTEST_PRINTINGPOLICY_SUPPRESSUNWRITTENSCOPE",
Index: test/Misc/ast-print-record-decl.c
===
--- /dev/null
+++ test/Misc/ast-print-record-decl.c
@@ -0,0 +1,251 @@
+// Check struct:
+//
+//   First check compiling and printing of this file.
+//
+//   RUN: %clang -Xclang -verify -S -emit-llvm -DKW=struct -DBASES= -o - %s \
+//   RUN: | FileCheck --check-prefixes=CHECK,LLVM %s
+//
+//   RUN: %clang_cc1 -verify -ast-print -DKW=struct -DBASES= %s > %t.c
+//   RUN: FileCheck --check-prefixes=CHECK,PRINT -DKW=struct -DBASES= %s \
+//   RUN:   --input-file %t.c
+//
+//   Now check compiling and printing of the printed file.
+//
+//   RUN: echo "// expected""-warning@* 10 {{'T' is deprecated}}" >> %t.c
+//   RUN: echo "// expected""-note@* 10 {{'T' has been explicitly marked deprecated here}}" >> %t.c
+//
+//   RUN: %clang -Xclang -verify -S -emit-llvm -o - %t.c \
+//   RUN: | FileCheck --check-prefixes=CHECK,LLVM %s
+//
+//   RUN: %clang_cc1 -verify -ast-print %t.c \
+//   RUN: | FileCheck --check-prefixes=CHECK,PRINT -DKW=struct -DBASES= %s
+
+// Repeat for union:
+//
+//   First check compiling and printing of this file.
+//
+//   RUN: %clang -Xclang -verify -S -emit-llvm -DKW=union -DBASES= -o - %s \
+//   RUN: | FileCheck --check-prefixes=CHECK,LLVM %s
+//
+//   RUN: %clang_cc1 -verify -ast-print -DKW=union -DBASES= %s > %t.c
+//   RUN: FileCheck --check-prefixes=CHECK,PRINT -DKW=union -DBASES= %s \
+//   RUN:   --input-file %t.c
+//
+//   Now check compiling and printing of the printed file.
+//
+//   RUN: echo "// expected""-warning@* 10 {{'T' is deprecated}}" >> %t.c
+//   RUN: echo "// expected""-note@* 10 {{'T' has been explicitly marked deprecated here}}" >> %t.c
+//
+//   RUN: %clang -Xclang -verify -S -emit-llvm -o - %t.c \
+//   RUN: | FileCheck --check-prefixes=CHECK,LLVM %s
+//
+//   RUN: %clang_cc1 -verify -ast-print %t.c \
+//   RUN: | FileCheck --check-prefixes=CHECK,PRINT -DKW=union -DBASES= %s
+
+// Repeat for C++ (BASES helps ensure we're printing as C++ not as C):
+//
+//   First check compiling and printing of this file.
+//
+//   RUN: %clang -Xclang -verify -S -emit-llvm -DKW=struct -DBASES=' : B' -o - \
+//   RUN:-xc++ %s \
+//   RUN: | FileCheck --check-prefixes=CHECK,LLVM %s
+//
+//   RUN: %clang_cc1 -verify -ast-print -DKW=struct -DBASES=' : B' -xc++ %s \
+//   RUN: > %t.cpp
+//   RUN: FileCheck --check-prefixes=CHECK,PRINT,CXX -DKW=struct \
+//   RUN:   -DBASES=' : B' %s --input-file %t.cpp
+//
+//   Now check compiling and printing of the printed file.
+//
+//   RUN: echo "// expected""-warning@* 10 {{'T' is deprecated}}" >> %t.cpp
+//   RUN: echo "// expected""-note@* 10 {{'T' has been explicitly marked deprecated here}}" >> %t.cpp
+//
+//   RUN: %clang -Xclang -verify -S -emit-llvm -o - %t.cpp \
+//   RUN: | FileCheck --check-prefixes=CHECK,LLVM %s
+//
+//   RUN: %clang_cc1 -verify -ast-print %t.cpp \
+//   RUN: | FileCheck --check-prefixes=CHECK,PRINT,CXX -DKW=struct \
+//   RUN: -DBASES=' : B' %s
+
+// END.
+
+#ifndef KW
+# error KW undefined
+# define KW struct // help syntax checkers
+#endif
+
+#ifndef BASES
+# error BASES undefined
+# define BASES //

r330151 - [CodeGen] Fix a crash that occurs when a non-trivial C struct with a

2018-04-16 Thread Akira Hatanaka via cfe-commits
Author: ahatanak
Date: Mon Apr 16 12:38:00 2018
New Revision: 330151

URL: http://llvm.org/viewvc/llvm-project?rev=330151&view=rev
Log:
[CodeGen] Fix a crash that occurs when a non-trivial C struct with a
volatile array field is copied.

The crash occurs because method 'visitArray' passes a null FieldDecl to
method 'visit' and some of the methods called downstream expect a
non-null FieldDecl to be passed.

rdar://problem/33599681

Modified:
cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp
cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m

Modified: cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp?rev=330151&r1=330150&r2=330151&view=diff
==
--- cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp Mon Apr 16 12:38:00 2018
@@ -21,10 +21,11 @@ using namespace clang;
 using namespace CodeGen;
 
 // Return the size of a field in number of bits.
-static uint64_t getFieldSize(const FieldDecl *FD, ASTContext &Ctx) {
-  if (FD->isBitField())
+static uint64_t getFieldSize(const FieldDecl *FD, QualType FT,
+ ASTContext &Ctx) {
+  if (FD && FD->isBitField())
 return FD->getBitWidthValue(Ctx);
-  return Ctx.getTypeSize(FD->getType());
+  return Ctx.getTypeSize(FT);
 }
 
 namespace {
@@ -187,7 +188,7 @@ struct CopyStructVisitor : StructVisitor
 Ts... Args) {
 assert(!FT.isVolatileQualified() && "volatile field not expected");
 ASTContext &Ctx = asDerived().getContext();
-uint64_t FieldSize = getFieldSize(FD, Ctx);
+uint64_t FieldSize = getFieldSize(FD, FT, Ctx);
 
 // Ignore zero-sized fields.
 if (FieldSize == 0)
@@ -336,7 +337,7 @@ struct GenBinaryFuncName : CopyStructVis
 uint64_t OffsetInBits =
 this->Ctx.toBits(CurStackOffset) + this->getFieldOffsetInBits(FD);
 this->appendStr("_tv" + llvm::to_string(OffsetInBits) + "w" +
-llvm::to_string(getFieldSize(FD, this->Ctx)));
+llvm::to_string(getFieldSize(FD, FT, this->Ctx)));
   }
 };
 
@@ -595,16 +596,25 @@ struct GenBinaryFunc : CopyStructVisitor
   template 
   void visitVolatileTrivial(QualType FT, const FieldDecl *FD, CharUnits Offset,
 std::array Addrs) {
-QualType RT = QualType(FD->getParent()->getTypeForDecl(), 0);
-llvm::PointerType *PtrTy = this->CGF->ConvertType(RT)->getPointerTo();
-Address DstAddr = this->getAddrWithOffset(Addrs[DstIdx], Offset);
-LValue DstBase = this->CGF->MakeAddrLValue(
-this->CGF->Builder.CreateBitCast(DstAddr, PtrTy), FT);
-LValue DstLV = this->CGF->EmitLValueForField(DstBase, FD);
-Address SrcAddr = this->getAddrWithOffset(Addrs[SrcIdx], Offset);
-LValue SrcBase = this->CGF->MakeAddrLValue(
-this->CGF->Builder.CreateBitCast(SrcAddr, PtrTy), FT);
-LValue SrcLV = this->CGF->EmitLValueForField(SrcBase, FD);
+LValue DstLV, SrcLV;
+if (FD) {
+  QualType RT = QualType(FD->getParent()->getTypeForDecl(), 0);
+  llvm::PointerType *PtrTy = this->CGF->ConvertType(RT)->getPointerTo();
+  Address DstAddr = this->getAddrWithOffset(Addrs[DstIdx], Offset);
+  LValue DstBase = this->CGF->MakeAddrLValue(
+  this->CGF->Builder.CreateBitCast(DstAddr, PtrTy), FT);
+  DstLV = this->CGF->EmitLValueForField(DstBase, FD);
+  Address SrcAddr = this->getAddrWithOffset(Addrs[SrcIdx], Offset);
+  LValue SrcBase = this->CGF->MakeAddrLValue(
+  this->CGF->Builder.CreateBitCast(SrcAddr, PtrTy), FT);
+  SrcLV = this->CGF->EmitLValueForField(SrcBase, FD);
+} else {
+  llvm::PointerType *Ty = this->CGF->ConvertType(FT)->getPointerTo();
+  Address DstAddr = this->CGF->Builder.CreateBitCast(Addrs[DstIdx], Ty);
+  Address SrcAddr = this->CGF->Builder.CreateBitCast(Addrs[SrcIdx], Ty);
+  DstLV = this->CGF->MakeAddrLValue(DstAddr, FT);
+  SrcLV = this->CGF->MakeAddrLValue(SrcAddr, FT);
+}
 RValue SrcVal = this->CGF->EmitLoadOfLValue(SrcLV, SourceLocation());
 this->CGF->EmitStoreThroughLValue(SrcVal, DstLV);
   }

Modified: cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m?rev=330151&r1=330150&r2=330151&view=diff
==
--- cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m (original)
+++ cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m Mon Apr 16 12:38:00 2018
@@ -70,6 +70,11 @@ typedef struct {
   volatile char i6;
 } Bitfield1;
 
+typedef struct {
+  id x;
+  volatile int a[16];
+} VolatileArray ;
+
 #endif
 
 #ifdef USESTRUCT
@@ -540,4 +545,17 @@ void test_strong_in_union() {
   U t;
 }
 
+// CHECK: define void @test_copy_constructor_VolatileArray(
+// CHECK: call void @__copy_constructor_8_8_s0_AB8s4n16_tv64w32_AE

[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

2018-04-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

Uuuh, the fact that phab posts the top-postings, but silently ignores inline 
replies is annoying.

>> lebedev.ri added a comment.
>> 
>> In https://reviews.llvm.org/D45685#1069040, @dblaikie wrote:
>> 
>>> I'm not sure this is a practical direction to pursue - though perhaps
>>  >  others disagree.
>> 
>>> It's likely non-trivial to plumb a flag through most build systems to be
>>  >  applied only to test code
>> 
>> I'm sorry, I don't understand.
>> 
>> If you don't separate between source code and `*_test.cpp` sources, sure,
>>  you will loose the warning coverage either way.
>> 
>> What difference is there between passing `-wtest` and passing
>>  `-Wno-self-assign-overloaded`?
> 
> Not much, if any.

Ok, so this was a non-argument then?

>> Just pass it alongside with the googletest include paths so to speak.
> 
> But build systems don't necessarily expose that kind of ability. (For
>  example, googletest (not the only kind of test suite/code) is checked into
>  the LLVM codebase like another library - so depending on it is just another
>  library dependency, not a special "test" library dependency).

It's a bit hard to talk about all and every spherical build system / project
in the vacuum, because there is an infinite number of possibilities.
Of course some build systems are horribly designed, and it will be hard to
do that there. But I sure hope that isn't the case in most of the cases.
It might be more productive to talk about a few good known realities.

In llvm's case you would simply add `-wtest` (or `-Wno-self-assign-overloaded`)
in here 
https://github.com/llvm-mirror/llvm/blob/2a6cf85828509e89e18739e5f4b9a958820d66d4/cmake/modules/AddLLVM.cmake#L1079-L1085
and around here 
https://github.com/llvm-mirror/libcxx/blob/73e00f8321b13559b3c41f6656686d980fe92fbe/utils/libcxx/test/config.py#L914
I'd say that is rather trivial.


Repository:
  rC Clang

https://reviews.llvm.org/D45685



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


r330152 - Use export_as for autolinking frameworks

2018-04-16 Thread Bruno Cardoso Lopes via cfe-commits
Author: bruno
Date: Mon Apr 16 12:42:32 2018
New Revision: 330152

URL: http://llvm.org/viewvc/llvm-project?rev=330152&view=rev
Log:
Use export_as for autolinking frameworks

framework module SomeKitCore {
  ...
  export_as SomeKit
}

Given the module above, while generting autolink information during
codegen, clang should to emit '-framework SomeKitCore' only if SomeKit
was not imported in the relevant TU, otherwise it should use '-framework
SomeKit' instead.

rdar://problem/38269782

Added:
cfe/trunk/test/Modules/Inputs/exportas-link/
cfe/trunk/test/Modules/Inputs/exportas-link/OtherKit.framework/
cfe/trunk/test/Modules/Inputs/exportas-link/OtherKit.framework/Headers/

cfe/trunk/test/Modules/Inputs/exportas-link/OtherKit.framework/Headers/OtherKit.h
cfe/trunk/test/Modules/Inputs/exportas-link/OtherKit.framework/Modules/

cfe/trunk/test/Modules/Inputs/exportas-link/OtherKit.framework/Modules/module.modulemap
cfe/trunk/test/Modules/Inputs/exportas-link/SomeKit.framework/
cfe/trunk/test/Modules/Inputs/exportas-link/SomeKit.framework/Headers/

cfe/trunk/test/Modules/Inputs/exportas-link/SomeKit.framework/Headers/SKWidget.h

cfe/trunk/test/Modules/Inputs/exportas-link/SomeKit.framework/Headers/SomeKit.h
cfe/trunk/test/Modules/Inputs/exportas-link/SomeKit.framework/Modules/

cfe/trunk/test/Modules/Inputs/exportas-link/SomeKit.framework/Modules/module.modulemap
cfe/trunk/test/Modules/Inputs/exportas-link/SomeKit.framework/SomeKit.tbd
cfe/trunk/test/Modules/Inputs/exportas-link/SomeKitCore.framework/
cfe/trunk/test/Modules/Inputs/exportas-link/SomeKitCore.framework/Headers/

cfe/trunk/test/Modules/Inputs/exportas-link/SomeKitCore.framework/Headers/SKWidget.h

cfe/trunk/test/Modules/Inputs/exportas-link/SomeKitCore.framework/Headers/SomeKitCore.h
cfe/trunk/test/Modules/Inputs/exportas-link/SomeKitCore.framework/Modules/

cfe/trunk/test/Modules/Inputs/exportas-link/SomeKitCore.framework/Modules/module.modulemap

cfe/trunk/test/Modules/Inputs/exportas-link/SomeKitCore.framework/SomeKitCore.tbd
cfe/trunk/test/Modules/use-exportas-for-link.m
Modified:
cfe/trunk/include/clang/Basic/Module.h
cfe/trunk/include/clang/Lex/ModuleMap.h
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/lib/Lex/ModuleMap.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp

Modified: cfe/trunk/include/clang/Basic/Module.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Module.h?rev=330152&r1=330151&r2=330152&view=diff
==
--- cfe/trunk/include/clang/Basic/Module.h (original)
+++ cfe/trunk/include/clang/Basic/Module.h Mon Apr 16 12:42:32 2018
@@ -331,6 +331,10 @@ public:
   /// an entity from this module is used.
   llvm::SmallVector LinkLibraries;
 
+  /// Autolinking uses the framework name for linking purposes
+  /// when this is false and the export_as name otherwise.
+  bool UseExportAsModuleLinkName = false;
+
   /// \brief The set of "configuration macros", which are macros that
   /// (intentionally) change how this module is built.
   std::vector ConfigMacros;

Modified: cfe/trunk/include/clang/Lex/ModuleMap.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/ModuleMap.h?rev=330152&r1=330151&r2=330152&view=diff
==
--- cfe/trunk/include/clang/Lex/ModuleMap.h (original)
+++ cfe/trunk/include/clang/Lex/ModuleMap.h Mon Apr 16 12:42:32 2018
@@ -21,6 +21,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/PointerIntPair.h"
+#include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
@@ -104,7 +105,19 @@ class ModuleMap {
   /// \brief The number of modules we have created in total.
   unsigned NumCreatedModules = 0;
 
+  /// In case a module has a export_as entry, it might have a pending link
+  /// name to be determined if that module is imported.
+  llvm::StringMap> PendingLinkAsModule;
+
 public:
+  /// Use PendingLinkAsModule information to mark top level link names that
+  /// are going to be replaced by export_as aliases.
+  void resolveLinkAsDependencies(Module *Mod);
+
+  /// Make module to use export_as as the link dependency name if enough
+  /// information is available or add it to a pending list otherwise.
+  void addLinkAsDependency(Module *Mod);
+
   /// \brief Flags describing the role of a module header.
   enum ModuleHeaderRole {
 /// \brief This header is normally included in the module.

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=330152&r1=330151&r2=330152&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenModule

[PATCH] D45665: [builtin-dump-struct] printf formats generation testing

2018-04-16 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rC Clang

https://reviews.llvm.org/D45665



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


[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

2018-04-16 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments.



Comment at: lib/Basic/FrontendTiming.cpp:18
+
+bool FrontendTimesIsEnabled = false;
+

avt77 wrote:
> efriedma wrote:
> > Why is this in lib/Basic, when the declaration is in 
> > include/clang/Frontend/?
> Because this library is being linked to all others and as result this global 
> variable could be seen in any Clang library w/o any changes in config files. 
> Or you mean it should be moved from Frontend? But where? It's a frontend 
> feature that's why I put its declaration there.
The include structure should match the library structure; if the definition 
needs to be in lib/Basic/, the declaration should be in include/clang/Basic.


https://reviews.llvm.org/D45619



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


r330153 - Revert "[CodeGen] Fix a crash that occurs when a non-trivial C struct with a"

2018-04-16 Thread Akira Hatanaka via cfe-commits
Author: ahatanak
Date: Mon Apr 16 12:53:59 2018
New Revision: 330153

URL: http://llvm.org/viewvc/llvm-project?rev=330153&view=rev
Log:
Revert "[CodeGen] Fix a crash that occurs when a non-trivial C struct with a"

This reverts commit r330151, which caused bots to fail.

Modified:
cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp
cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m

Modified: cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp?rev=330153&r1=330152&r2=330153&view=diff
==
--- cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp Mon Apr 16 12:53:59 2018
@@ -21,11 +21,10 @@ using namespace clang;
 using namespace CodeGen;
 
 // Return the size of a field in number of bits.
-static uint64_t getFieldSize(const FieldDecl *FD, QualType FT,
- ASTContext &Ctx) {
-  if (FD && FD->isBitField())
+static uint64_t getFieldSize(const FieldDecl *FD, ASTContext &Ctx) {
+  if (FD->isBitField())
 return FD->getBitWidthValue(Ctx);
-  return Ctx.getTypeSize(FT);
+  return Ctx.getTypeSize(FD->getType());
 }
 
 namespace {
@@ -188,7 +187,7 @@ struct CopyStructVisitor : StructVisitor
 Ts... Args) {
 assert(!FT.isVolatileQualified() && "volatile field not expected");
 ASTContext &Ctx = asDerived().getContext();
-uint64_t FieldSize = getFieldSize(FD, FT, Ctx);
+uint64_t FieldSize = getFieldSize(FD, Ctx);
 
 // Ignore zero-sized fields.
 if (FieldSize == 0)
@@ -337,7 +336,7 @@ struct GenBinaryFuncName : CopyStructVis
 uint64_t OffsetInBits =
 this->Ctx.toBits(CurStackOffset) + this->getFieldOffsetInBits(FD);
 this->appendStr("_tv" + llvm::to_string(OffsetInBits) + "w" +
-llvm::to_string(getFieldSize(FD, FT, this->Ctx)));
+llvm::to_string(getFieldSize(FD, this->Ctx)));
   }
 };
 
@@ -596,25 +595,16 @@ struct GenBinaryFunc : CopyStructVisitor
   template 
   void visitVolatileTrivial(QualType FT, const FieldDecl *FD, CharUnits Offset,
 std::array Addrs) {
-LValue DstLV, SrcLV;
-if (FD) {
-  QualType RT = QualType(FD->getParent()->getTypeForDecl(), 0);
-  llvm::PointerType *PtrTy = this->CGF->ConvertType(RT)->getPointerTo();
-  Address DstAddr = this->getAddrWithOffset(Addrs[DstIdx], Offset);
-  LValue DstBase = this->CGF->MakeAddrLValue(
-  this->CGF->Builder.CreateBitCast(DstAddr, PtrTy), FT);
-  DstLV = this->CGF->EmitLValueForField(DstBase, FD);
-  Address SrcAddr = this->getAddrWithOffset(Addrs[SrcIdx], Offset);
-  LValue SrcBase = this->CGF->MakeAddrLValue(
-  this->CGF->Builder.CreateBitCast(SrcAddr, PtrTy), FT);
-  SrcLV = this->CGF->EmitLValueForField(SrcBase, FD);
-} else {
-  llvm::PointerType *Ty = this->CGF->ConvertType(FT)->getPointerTo();
-  Address DstAddr = this->CGF->Builder.CreateBitCast(Addrs[DstIdx], Ty);
-  Address SrcAddr = this->CGF->Builder.CreateBitCast(Addrs[SrcIdx], Ty);
-  DstLV = this->CGF->MakeAddrLValue(DstAddr, FT);
-  SrcLV = this->CGF->MakeAddrLValue(SrcAddr, FT);
-}
+QualType RT = QualType(FD->getParent()->getTypeForDecl(), 0);
+llvm::PointerType *PtrTy = this->CGF->ConvertType(RT)->getPointerTo();
+Address DstAddr = this->getAddrWithOffset(Addrs[DstIdx], Offset);
+LValue DstBase = this->CGF->MakeAddrLValue(
+this->CGF->Builder.CreateBitCast(DstAddr, PtrTy), FT);
+LValue DstLV = this->CGF->EmitLValueForField(DstBase, FD);
+Address SrcAddr = this->getAddrWithOffset(Addrs[SrcIdx], Offset);
+LValue SrcBase = this->CGF->MakeAddrLValue(
+this->CGF->Builder.CreateBitCast(SrcAddr, PtrTy), FT);
+LValue SrcLV = this->CGF->EmitLValueForField(SrcBase, FD);
 RValue SrcVal = this->CGF->EmitLoadOfLValue(SrcLV, SourceLocation());
 this->CGF->EmitStoreThroughLValue(SrcVal, DstLV);
   }

Modified: cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m?rev=330153&r1=330152&r2=330153&view=diff
==
--- cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m (original)
+++ cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m Mon Apr 16 12:53:59 2018
@@ -70,11 +70,6 @@ typedef struct {
   volatile char i6;
 } Bitfield1;
 
-typedef struct {
-  id x;
-  volatile int a[16];
-} VolatileArray ;
-
 #endif
 
 #ifdef USESTRUCT
@@ -545,17 +540,4 @@ void test_strong_in_union() {
   U t;
 }
 
-// CHECK: define void @test_copy_constructor_VolatileArray(
-// CHECK: call void @__copy_constructor_8_8_s0_AB8s4n16_tv64w32_AE(
-
-// CHECK: define linkonce_odr hidden void 
@__copy_constructor_8_8_s0_AB8s4n16_tv64w32_AE(
-// CHECK: %[[V12:.*]] = bitcast i8** %{{.*}} to i32*
-// CHECK: %[

[PATCH] D45383: Limit types of builtins that can be redeclared.

2018-04-16 Thread Erich Keane via Phabricator via cfe-commits
erichkeane updated this revision to Diff 142685.
erichkeane added a comment.

Added the exception for MSVC's __va_start.  __va_start is a normal function on 
Linux, so it isn't an error on non-windows either.


https://reviews.llvm.org/D45383

Files:
  include/clang/AST/ASTContext.h
  include/clang/Basic/Builtins.h
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/AST/ASTContext.cpp
  lib/Basic/Builtins.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaOverload.cpp
  test/Sema/MicrosoftExtensions.c
  test/Sema/builtin-redecl.cpp
  test/SemaCXX/microsoft-varargs.cpp

Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -7244,6 +7244,10 @@
   return BuiltinMSVaListDecl;
 }
 
+bool ASTContext::canBuiltinBeRedeclared(const FunctionDecl *FD) const {
+  return BuiltinInfo.canBeRedeclared(FD->getBuiltinID());
+}
+
 void ASTContext::setObjCConstantStringInterface(ObjCInterfaceDecl *Decl) {
   assert(ObjCConstantStringType.isNull() &&
  "'NSConstantString' type already set!");
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -998,6 +998,13 @@
 Match = *I;
 return Ovl_Match;
   }
+
+  // Builtins that have custom typechecking or have a reference should
+  // not be overloadable or redeclarable.
+  if (!getASTContext().canBuiltinBeRedeclared(OldF)) {
+Match = *I;
+return Ovl_NonFunction;
+  }
 } else if (isa(OldD) || isa(OldD)) {
   // We can overload with these, which can show up when doing
   // redeclaration checks for UsingDecls.
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -3011,6 +3011,14 @@
   if (Old->isInvalidDecl())
 return true;
 
+  // Disallow redeclaration of some builtins.
+  if (!getASTContext().canBuiltinBeRedeclared(Old)) {
+Diag(New->getLocation(), diag::err_builtin_redeclare) << Old->getDeclName();
+Diag(Old->getLocation(), diag::note_previous_builtin_declaration)
+<< Old << Old->getType();
+return true;
+  }
+
   diag::kind PrevDiag;
   SourceLocation OldLocation;
   std::tie(PrevDiag, OldLocation) =
Index: lib/Basic/Builtins.cpp
===
--- lib/Basic/Builtins.cpp
+++ lib/Basic/Builtins.cpp
@@ -139,3 +139,10 @@
bool &HasVAListArg) {
   return isLike(ID, FormatIdx, HasVAListArg, "sS");
 }
+
+bool Builtin::Context::canBeRedeclared(unsigned ID) const {
+  return ID == Builtin::NotBuiltin ||
+ ID == Builtin::BI__va_start ||
+ (!hasReferenceArgsOrResult(ID) &&
+  !hasCustomTypechecking(ID));
+}
Index: include/clang/AST/ASTContext.h
===
--- include/clang/AST/ASTContext.h
+++ include/clang/AST/ASTContext.h
@@ -1881,6 +1881,10 @@
 return getTypeDeclType(getBuiltinMSVaListDecl());
   }
 
+  /// Return whether a declaration to a builtin is allowed to be
+  /// overloaded/redeclared.
+  bool canBuiltinBeRedeclared(const FunctionDecl *) const;
+
   /// \brief Return a type with additional \c const, \c volatile, or
   /// \c restrict qualifiers.
   QualType getCVRQualifiedType(QualType T, unsigned CVR) const {
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -602,6 +602,7 @@
   "incompatible redeclaration of library function %0">,
   InGroup>;
 def err_builtin_definition : Error<"definition of builtin function %0">;
+def err_builtin_redeclare : Error<"cannot redeclare builtin function %0">;
 def err_arm_invalid_specialreg : Error<"invalid special register for builtin">;
 def err_invalid_cpu_supports : Error<"invalid cpu feature string for builtin">;
 def err_invalid_cpu_is : Error<"invalid cpu name for builtin">;
Index: include/clang/Basic/Builtins.h
===
--- include/clang/Basic/Builtins.h
+++ include/clang/Basic/Builtins.h
@@ -167,6 +167,13 @@
 return strchr(getRecord(ID).Type, '*') != nullptr;
   }
 
+  /// \brief Return true if this builtin has a result or any arguments which are
+  /// reference types.
+  bool hasReferenceArgsOrResult(unsigned ID) const {
+return strchr(getRecord(ID).Type, '&') != nullptr ||
+   strchr(getRecord(ID).Type, 'A') != nullptr;
+  }
+
   /// \brief Completely forget that the given ID was ever considered a builtin,
   /// e.g., because the user provided a conflicting signature.
   void forgetBuiltin(unsigned ID, IdentifierTable &Table);
@@ -212,6 +219,10 @@
   /// prefix.
   static bool isBuiltinFunc(const char *Name);
 
+  /// Returns true if this

r330154 - [OPENMP] General code improvements.

2018-04-16 Thread Alexey Bataev via cfe-commits
Author: abataev
Date: Mon Apr 16 13:16:21 2018
New Revision: 330154

URL: http://llvm.org/viewvc/llvm-project?rev=330154&view=rev
Log:
[OPENMP] General code improvements.

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

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp?rev=330154&r1=330153&r2=330154&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Mon Apr 16 13:16:21 2018
@@ -91,11 +91,11 @@ enum OpenMPRTLFunctionNVPTX {
 
 /// Pre(post)-action for different OpenMP constructs specialized for NVPTX.
 class NVPTXActionTy final : public PrePostActionTy {
-  llvm::Value *EnterCallee;
+  llvm::Value *EnterCallee = nullptr;
   ArrayRef EnterArgs;
-  llvm::Value *ExitCallee;
+  llvm::Value *ExitCallee = nullptr;
   ArrayRef ExitArgs;
-  bool Conditional;
+  bool Conditional = false;
   llvm::BasicBlock *ContBlock = nullptr;
 
 public:
@@ -179,7 +179,7 @@ class CheckVarsEscapingDeclContext final
 
   static llvm::Optional
   isDeclareTargetDeclaration(const ValueDecl *VD) {
-for (const auto *D : VD->redecls()) {
+for (const Decl *D : VD->redecls()) {
   if (!D->hasAttrs())
 continue;
   if (const auto *Attr = D->getAttr())
@@ -233,7 +233,7 @@ class CheckVarsEscapingDeclContext final
   void VisitOpenMPCapturedStmt(const CapturedStmt *S) {
 if (!S)
   return;
-for (const auto &C : S->captures()) {
+for (const CapturedStmt::Capture &C : S->captures()) {
   if (C.capturesVariable() && !C.capturesVariableByCopy()) {
 const ValueDecl *VD = C.getCapturedVar();
 markAsEscaped(VD);
@@ -255,7 +255,7 @@ class CheckVarsEscapingDeclContext final
   return;
 ASTContext &C = CGF.getContext();
 SmallVector GlobalizedVars;
-for (const auto *D : EscapedDecls)
+for (const ValueDecl *D : EscapedDecls)
   GlobalizedVars.emplace_back(C.getDeclAlign(D), D);
 std::stable_sort(GlobalizedVars.begin(), GlobalizedVars.end(),
  stable_sort_comparator);
@@ -296,7 +296,7 @@ public:
   void VisitDeclStmt(const DeclStmt *S) {
 if (!S)
   return;
-for (const auto *D : S->decls())
+for (const Decl *D : S->decls())
   if (const auto *VD = dyn_cast_or_null(D))
 VisitValueDecl(VD);
   }
@@ -312,7 +312,7 @@ public:
   void VisitCapturedStmt(const CapturedStmt *S) {
 if (!S)
   return;
-for (const auto &C : S->captures()) {
+for (const CapturedStmt::Capture &C : S->captures()) {
   if (C.capturesVariable() && !C.capturesVariableByCopy()) {
 const ValueDecl *VD = C.getCapturedVar();
 markAsEscaped(VD);
@@ -324,7 +324,7 @@ public:
   void VisitLambdaExpr(const LambdaExpr *E) {
 if (!E)
   return;
-for (const auto &C : E->captures()) {
+for (const LambdaCapture &C : E->captures()) {
   if (C.capturesVariable()) {
 if (C.getCaptureKind() == LCK_ByRef) {
   const ValueDecl *VD = C.getCapturedVar();
@@ -338,7 +338,7 @@ public:
   void VisitBlockExpr(const BlockExpr *E) {
 if (!E)
   return;
-for (const auto &C : E->getBlockDecl()->captures()) {
+for (const BlockDecl::Capture &C : E->getBlockDecl()->captures()) {
   if (C.isByRef()) {
 const VarDecl *VD = C.getVariable();
 markAsEscaped(VD);
@@ -358,8 +358,9 @@ public:
 AllEscaped = true;
 Visit(Arg);
 AllEscaped = SavedAllEscaped;
-  } else
+  } else {
 Visit(Arg);
+  }
 }
 Visit(E->getCallee());
   }
@@ -383,8 +384,9 @@ public:
   AllEscaped = true;
   Visit(E->getSubExpr());
   AllEscaped = SavedAllEscaped;
-} else
+} else {
   Visit(E->getSubExpr());
+}
   }
   void VisitImplicitCastExpr(const ImplicitCastExpr *E) {
 if (!E)
@@ -394,8 +396,9 @@ public:
   AllEscaped = true;
   Visit(E->getSubExpr());
   AllEscaped = SavedAllEscaped;
-} else
+} else {
   Visit(E->getSubExpr());
+}
   }
   void VisitExpr(const Expr *E) {
 if (!E)
@@ -403,7 +406,7 @@ public:
 bool SavedAllEscaped = AllEscaped;
 if (!E->isLValue())
   AllEscaped = false;
-for (const auto *Child : E->children())
+for (const Stmt *Child : E->children())
   if (Child)
 Visit(Child);
 AllEscaped = SavedAllEscaped;
@@ -411,7 +414,7 @@ public:
   void VisitStmt(const Stmt *S) {
 if (!S)
   return;
-for (const auto *Child : S->children())
+for (const Stmt *Child : S->children())
   if (Child)
 Visit(Child);
   }
@@ -553,19 +556,19 @@ static llvm::Value *getMasterThreadID(Co
 
 CGOpenMPRuntimeNVPTX::WorkerFunctionState::WorkerFunctionState(
 CodeGenModule &CGM, Sour

[PATCH] D45662: Fuzzer, add libcxx for OpenBSD

2018-04-16 Thread David CARLIER via Phabricator via cfe-commits
devnexen updated this revision to Diff 142686.
devnexen added a comment.

- Putting the change on the driver itself.


https://reviews.llvm.org/D45662

Files:
  lib/Driver/ToolChains/OpenBSD.cpp


Index: lib/Driver/ToolChains/OpenBSD.cpp
===
--- lib/Driver/ToolChains/OpenBSD.cpp
+++ lib/Driver/ToolChains/OpenBSD.cpp
@@ -194,6 +194,9 @@
 }
 if (NeedsSanitizerDeps) {
   CmdArgs.push_back(ToolChain.getCompilerRTArgString(Args, "builtins", 
false));
+  if (getToolChain().getSanitizerArgs().needsFuzzer() &&
+!Args.hasArg(options::OPT_shared))
+CmdArgs.push_back(Args.hasArg(options::OPT_pg) ? "-lc++_p" : "-lc++");
   linkSanitizerRuntimeDeps(ToolChain, CmdArgs);
 }
 if (NeedsXRayDeps) {


Index: lib/Driver/ToolChains/OpenBSD.cpp
===
--- lib/Driver/ToolChains/OpenBSD.cpp
+++ lib/Driver/ToolChains/OpenBSD.cpp
@@ -194,6 +194,9 @@
 }
 if (NeedsSanitizerDeps) {
   CmdArgs.push_back(ToolChain.getCompilerRTArgString(Args, "builtins", false));
+  if (getToolChain().getSanitizerArgs().needsFuzzer() &&
+!Args.hasArg(options::OPT_shared))
+CmdArgs.push_back(Args.hasArg(options::OPT_pg) ? "-lc++_p" : "-lc++");
   linkSanitizerRuntimeDeps(ToolChain, CmdArgs);
 }
 if (NeedsXRayDeps) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r330155 - [CodeGen] Fix a crash that occurs when a non-trivial C struct with a

2018-04-16 Thread Akira Hatanaka via cfe-commits
Author: ahatanak
Date: Mon Apr 16 13:23:52 2018
New Revision: 330155

URL: http://llvm.org/viewvc/llvm-project?rev=330155&view=rev
Log:
[CodeGen] Fix a crash that occurs when a non-trivial C struct with a
volatile array field is copied.

The crash occurs because method 'visitArray' passes a null FieldDecl to
method 'visit' and some of the methods called downstream expect a
non-null FieldDecl to be passed.

This reapplies r330151 with a fix to the test case.

rdar://problem/33599681

Modified:
cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp
cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m

Modified: cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp?rev=330155&r1=330154&r2=330155&view=diff
==
--- cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp Mon Apr 16 13:23:52 2018
@@ -21,10 +21,11 @@ using namespace clang;
 using namespace CodeGen;
 
 // Return the size of a field in number of bits.
-static uint64_t getFieldSize(const FieldDecl *FD, ASTContext &Ctx) {
-  if (FD->isBitField())
+static uint64_t getFieldSize(const FieldDecl *FD, QualType FT,
+ ASTContext &Ctx) {
+  if (FD && FD->isBitField())
 return FD->getBitWidthValue(Ctx);
-  return Ctx.getTypeSize(FD->getType());
+  return Ctx.getTypeSize(FT);
 }
 
 namespace {
@@ -187,7 +188,7 @@ struct CopyStructVisitor : StructVisitor
 Ts... Args) {
 assert(!FT.isVolatileQualified() && "volatile field not expected");
 ASTContext &Ctx = asDerived().getContext();
-uint64_t FieldSize = getFieldSize(FD, Ctx);
+uint64_t FieldSize = getFieldSize(FD, FT, Ctx);
 
 // Ignore zero-sized fields.
 if (FieldSize == 0)
@@ -336,7 +337,7 @@ struct GenBinaryFuncName : CopyStructVis
 uint64_t OffsetInBits =
 this->Ctx.toBits(CurStackOffset) + this->getFieldOffsetInBits(FD);
 this->appendStr("_tv" + llvm::to_string(OffsetInBits) + "w" +
-llvm::to_string(getFieldSize(FD, this->Ctx)));
+llvm::to_string(getFieldSize(FD, FT, this->Ctx)));
   }
 };
 
@@ -595,16 +596,25 @@ struct GenBinaryFunc : CopyStructVisitor
   template 
   void visitVolatileTrivial(QualType FT, const FieldDecl *FD, CharUnits Offset,
 std::array Addrs) {
-QualType RT = QualType(FD->getParent()->getTypeForDecl(), 0);
-llvm::PointerType *PtrTy = this->CGF->ConvertType(RT)->getPointerTo();
-Address DstAddr = this->getAddrWithOffset(Addrs[DstIdx], Offset);
-LValue DstBase = this->CGF->MakeAddrLValue(
-this->CGF->Builder.CreateBitCast(DstAddr, PtrTy), FT);
-LValue DstLV = this->CGF->EmitLValueForField(DstBase, FD);
-Address SrcAddr = this->getAddrWithOffset(Addrs[SrcIdx], Offset);
-LValue SrcBase = this->CGF->MakeAddrLValue(
-this->CGF->Builder.CreateBitCast(SrcAddr, PtrTy), FT);
-LValue SrcLV = this->CGF->EmitLValueForField(SrcBase, FD);
+LValue DstLV, SrcLV;
+if (FD) {
+  QualType RT = QualType(FD->getParent()->getTypeForDecl(), 0);
+  llvm::PointerType *PtrTy = this->CGF->ConvertType(RT)->getPointerTo();
+  Address DstAddr = this->getAddrWithOffset(Addrs[DstIdx], Offset);
+  LValue DstBase = this->CGF->MakeAddrLValue(
+  this->CGF->Builder.CreateBitCast(DstAddr, PtrTy), FT);
+  DstLV = this->CGF->EmitLValueForField(DstBase, FD);
+  Address SrcAddr = this->getAddrWithOffset(Addrs[SrcIdx], Offset);
+  LValue SrcBase = this->CGF->MakeAddrLValue(
+  this->CGF->Builder.CreateBitCast(SrcAddr, PtrTy), FT);
+  SrcLV = this->CGF->EmitLValueForField(SrcBase, FD);
+} else {
+  llvm::PointerType *Ty = this->CGF->ConvertType(FT)->getPointerTo();
+  Address DstAddr = this->CGF->Builder.CreateBitCast(Addrs[DstIdx], Ty);
+  Address SrcAddr = this->CGF->Builder.CreateBitCast(Addrs[SrcIdx], Ty);
+  DstLV = this->CGF->MakeAddrLValue(DstAddr, FT);
+  SrcLV = this->CGF->MakeAddrLValue(SrcAddr, FT);
+}
 RValue SrcVal = this->CGF->EmitLoadOfLValue(SrcLV, SourceLocation());
 this->CGF->EmitStoreThroughLValue(SrcVal, DstLV);
   }

Modified: cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m?rev=330155&r1=330154&r2=330155&view=diff
==
--- cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m (original)
+++ cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m Mon Apr 16 13:23:52 2018
@@ -70,6 +70,11 @@ typedef struct {
   volatile char i6;
 } Bitfield1;
 
+typedef struct {
+  id x;
+  volatile int a[16];
+} VolatileArray ;
+
 #endif
 
 #ifdef USESTRUCT
@@ -540,4 +545,19 @@ void test_strong_in_union() {
   U t;
 }
 
+// CHECK: define void @test_copy_constructor_VolatileArray(
+// CHECK: cal

[PATCH] D45465: [AST] Fix printing tag decl groups in decl contexts

2018-04-16 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny updated this revision to Diff 142687.
jdenny added a comment.

Rebased onto a more recent master.


https://reviews.llvm.org/D45465

Files:
  lib/AST/DeclPrinter.cpp
  test/Misc/ast-print-enum-decl.c
  test/Misc/ast-print-record-decl.c
  test/Sema/ast-print.c
  test/SemaCXX/ast-print.cpp

Index: test/SemaCXX/ast-print.cpp
===
--- test/SemaCXX/ast-print.cpp
+++ test/SemaCXX/ast-print.cpp
@@ -214,10 +214,13 @@
 struct [[gnu::visibility("hidden")]] S;
 }
 
-// CHECK: struct CXXFunctionalCastExprPrint fce = CXXFunctionalCastExprPrint{};
+// CHECK:  struct CXXFunctionalCastExprPrint {
+// CHECK-NEXT: } fce = CXXFunctionalCastExprPrint{};
 struct CXXFunctionalCastExprPrint {} fce = CXXFunctionalCastExprPrint{};
 
-// CHECK: struct CXXTemporaryObjectExprPrint toe = CXXTemporaryObjectExprPrint{};
+// CHECK:  struct CXXTemporaryObjectExprPrint {
+// CHECK-NEXT:   CXXTemporaryObjectExprPrint();
+// CHECK-NEXT: } toe = CXXTemporaryObjectExprPrint{};
 struct CXXTemporaryObjectExprPrint { CXXTemporaryObjectExprPrint(); } toe = CXXTemporaryObjectExprPrint{};
 
 namespace PR24872 {
Index: test/Sema/ast-print.c
===
--- test/Sema/ast-print.c
+++ test/Sema/ast-print.c
@@ -83,8 +83,7 @@
   EnumWithAttributesFoo __attribute__((deprecated)),
   // CHECK-NEXT: EnumWithAttributesBar __attribute__((unavailable(""))) = 50
   EnumWithAttributesBar __attribute__((unavailable)) = 50
-  // CHECK-NEXT: };
-  // CHECK-NEXT: enum EnumWithAttributes *EnumWithAttributesPtr;
+  // CHECK-NEXT: } *EnumWithAttributesPtr;
 } __attribute__((deprecated)) *EnumWithAttributesPtr; // expected-note {{'EnumWithAttributes' has been explicitly marked deprecated here}}
 
 // FIXME: If enum is forward-declared at file scope, attributes are lost.
Index: test/Misc/ast-print-record-decl.c
===
--- test/Misc/ast-print-record-decl.c
+++ test/Misc/ast-print-record-decl.c
@@ -6,8 +6,8 @@
 //   RUN: | FileCheck --check-prefixes=CHECK,LLVM %s
 //
 //   RUN: %clang_cc1 -verify -ast-print -DKW=struct -DBASES= %s > %t.c
-//   RUN: FileCheck --check-prefixes=CHECK,PRINT -DKW=struct -DBASES= %s \
-//   RUN:   --input-file %t.c
+//   RUN: FileCheck --check-prefixes=CHECK,PRINT,PRINT-C -DKW=struct -DBASES= \
+//   RUN:   %s --input-file %t.c
 //
 //   Now check compiling and printing of the printed file.
 //
@@ -18,7 +18,8 @@
 //   RUN: | FileCheck --check-prefixes=CHECK,LLVM %s
 //
 //   RUN: %clang_cc1 -verify -ast-print %t.c \
-//   RUN: | FileCheck --check-prefixes=CHECK,PRINT -DKW=struct -DBASES= %s
+//   RUN: | FileCheck --check-prefixes=CHECK,PRINT,PRINT-C -DKW=struct \
+//   RUN: -DBASES= %s
 
 // Repeat for union:
 //
@@ -28,8 +29,8 @@
 //   RUN: | FileCheck --check-prefixes=CHECK,LLVM %s
 //
 //   RUN: %clang_cc1 -verify -ast-print -DKW=union -DBASES= %s > %t.c
-//   RUN: FileCheck --check-prefixes=CHECK,PRINT -DKW=union -DBASES= %s \
-//   RUN:   --input-file %t.c
+//   RUN: FileCheck --check-prefixes=CHECK,PRINT,PRINT-C -DKW=union -DBASES= \
+//   RUN:   %s --input-file %t.c
 //
 //   Now check compiling and printing of the printed file.
 //
@@ -40,7 +41,8 @@
 //   RUN: | FileCheck --check-prefixes=CHECK,LLVM %s
 //
 //   RUN: %clang_cc1 -verify -ast-print %t.c \
-//   RUN: | FileCheck --check-prefixes=CHECK,PRINT -DKW=union -DBASES= %s
+//   RUN: | FileCheck --check-prefixes=CHECK,PRINT,PRINT-C -DKW=union \
+//   RUN: -DBASES= %s
 
 // Repeat for C++ (BASES helps ensure we're printing as C++ not as C):
 //
@@ -52,7 +54,7 @@
 //
 //   RUN: %clang_cc1 -verify -ast-print -DKW=struct -DBASES=' : B' -xc++ %s \
 //   RUN: > %t.cpp
-//   RUN: FileCheck --check-prefixes=CHECK,PRINT,CXX -DKW=struct \
+//   RUN: FileCheck --check-prefixes=CHECK,PRINT,PRINT-CXX -DKW=struct \
 //   RUN:   -DBASES=' : B' %s --input-file %t.cpp
 //
 //   Now check compiling and printing of the printed file.
@@ -64,7 +66,7 @@
 //   RUN: | FileCheck --check-prefixes=CHECK,LLVM %s
 //
 //   RUN: %clang_cc1 -verify -ast-print %t.cpp \
-//   RUN: | FileCheck --check-prefixes=CHECK,PRINT,CXX -DKW=struct \
+//   RUN: | FileCheck --check-prefixes=CHECK,PRINT,PRINT-CXX -DKW=struct \
 //   RUN: -DBASES=' : B' %s
 
 // END.
@@ -161,29 +163,41 @@
   // expected-note@+1 2 {{'T' has been explicitly marked deprecated here}}
   KW __attribute__((deprecated(""))) T *p0;
 
-  // PRINT-NEXT: [[KW]]
-  // PRINT-DAG:  __attribute__((deprecated("")))
-  // PRINT-DAG:  __attribute__((aligned(16)))
-  // PRINT-NOT:  __attribute__
-  // PRINT-SAME: T[[BASES]] {
-  // PRINT-NEXT:   int i;
-  // PRINT-NEXT:   [[KW]] T *p2;
-  // PRINT-NEXT: } *p1;
-  KW __attribute__((aligned(16))) T BASES { // expected-warning {{'T' is deprecated}}
+  // PRINT-NEXT:  [[KW]]
+  // PRINT-DAG:   __attribute__((deprecated("")))
+  // PRINT-DAG:   __attribute

r330156 - [OPENMP] Allow to use declare target variables in map clauses

2018-04-16 Thread Alexey Bataev via cfe-commits
Author: abataev
Date: Mon Apr 16 13:34:41 2018
New Revision: 330156

URL: http://llvm.org/viewvc/llvm-project?rev=330156&view=rev
Log:
[OPENMP] Allow to use declare target variables in map clauses

Global variables marked as declare target are allowed to be used in map
clauses. Patch fixes the crash of the compiler on the declare target
variables in map clauses.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/test/OpenMP/declare_target_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=330156&r1=330155&r2=330156&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Mon Apr 16 13:34:41 2018
@@ -6799,16 +6799,13 @@ public:
   if (const auto *VD =
   dyn_cast_or_null(I->getAssociatedDeclaration())) {
 if (llvm::Optional Res =
-isDeclareTargetDeclaration(VD)) {
-  assert(*Res == OMPDeclareTargetDeclAttr::MT_Link &&
- "Declare target link is expected.");
-  // Avoid warning in release build.
-  (void)*Res;
-  IsLink = true;
-  BP = CGF.CGM.getOpenMPRuntime()
-   .getAddrOfDeclareTargetLink(VD)
-   .getPointer();
-}
+isDeclareTargetDeclaration(VD))
+  if (*Res == OMPDeclareTargetDeclAttr::MT_Link) {
+IsLink = true;
+BP = CGF.CGM.getOpenMPRuntime()
+ .getAddrOfDeclareTargetLink(VD)
+ .getPointer();
+  }
   }
 
   // If the variable is a pointer and is being dereferenced (i.e. is not

Modified: cfe/trunk/test/OpenMP/declare_target_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/declare_target_codegen.cpp?rev=330156&r1=330155&r2=330156&view=diff
==
--- cfe/trunk/test/OpenMP/declare_target_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/declare_target_codegen.cpp Mon Apr 16 13:34:41 2018
@@ -45,7 +45,7 @@ int maini1() {
   static long aa = 32;
 // CHECK-DAG: define void 
@__omp_offloading_{{.*}}maini1{{.*}}_l[[@LINE+1]](i32* dereferenceable{{.*}}, 
i64 {{.*}}, i64 {{.*}})
 #pragma omp target map(tofrom \
-   : a)
+   : a, b)
   {
 static long aaa = 23;
 a = foo() + bar() + b + c + d + aa + aaa;


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


[PATCH] D45698: [analyzer] When we fail to evaluate a pointer cast, escape the pointer.

2018-04-16 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.

If a pointer cast fails (evaluates to an `UnknownVal`) and such cast is the 
last use of the pointer, the pointer is no longer referenced by the program 
state and a leak is (mis-)diagnosed. Produce pointer escape (but not 
invalidation) when the cast fails to avoid such false positives.


Repository:
  rC Clang

https://reviews.llvm.org/D45698

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  test/Analysis/malloc.mm
  test/Analysis/pr22954.c

Index: test/Analysis/pr22954.c
===
--- test/Analysis/pr22954.c
+++ test/Analysis/pr22954.c
@@ -624,9 +624,10 @@
   clang_analyzer_eval(m29[i].s3[1] == 1); // expected-warning{{UNKNOWN}}
   clang_analyzer_eval(m29[i].s3[2] == 1); // expected-warning{{UNKNOWN}}
   clang_analyzer_eval(m29[i].s3[3] == 1); // expected-warning{{UNKNOWN}}
-  clang_analyzer_eval(m29[j].s3[k] == 1); // expected-warning{{TRUE}}\
-  expected-warning{{Potential leak of memory pointed to by field 's4'}}
+  clang_analyzer_eval(m29[j].s3[k] == 1); // expected-warning{{TRUE}}
   clang_analyzer_eval(l29->s1[m] == 2); // expected-warning{{UNKNOWN}}
+  // FIXME: Should warn that m29[i].s4 leaks. But not on the previous line,
+  // because l29 and m29 alias.
   return 0;
 }
 
Index: test/Analysis/malloc.mm
===
--- test/Analysis/malloc.mm
+++ test/Analysis/malloc.mm
@@ -320,3 +320,13 @@
   NSString *string = [[NSString alloc] initWithCharactersNoCopy:characters length:12 freeWhenDone:1];
   if (string) free(characters); // expected-warning{{Attempt to free non-owned memory}}
 }
+
+void *test_reinterpret_cast_to_block() {
+  // Used to leak because the pointer was disappearing
+  // during the reinterpret_cast.
+  using BlockPtrTy = void (^)();
+  struct Block {};
+  Block* block = static_cast(malloc(sizeof(Block)));
+  BlockPtrTy blockPtr = reinterpret_cast(block); // no-warning
+  return blockPtr;
+}
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -258,12 +258,15 @@
 QualType T, QualType ExTy, const CastExpr* CastE, StmtNodeBuilder& Bldr,
 ExplodedNode* Pred) {
   // Delegate to SValBuilder to process.
-  SVal V = state->getSVal(Ex, LCtx);
-  V = svalBuilder.evalCast(V, T, ExTy);
+  SVal OrigV = state->getSVal(Ex, LCtx);
+  SVal V = svalBuilder.evalCast(OrigV, T, ExTy);
   // Negate the result if we're treating the boolean as a signed i1
   if (CastE->getCastKind() == CK_BooleanToSignedIntegral)
 V = evalMinus(V);
   state = state->BindExpr(CastE, LCtx, V);
+  if (V.isUnknown() && !OrigV.isUnknown()) {
+state = escapeValue(state, OrigV, PSK_EscapeOther);
+  }
   Bldr.generateNode(CastE, Pred, state);
 
   return state;
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1231,23 +1231,27 @@
   }
 }
 
-namespace {
+ProgramStateRef ExprEngine::escapeValue(ProgramStateRef State, SVal V,
+PointerEscapeKind K) const {
+  class CollectReachableSymbolsCallback final : public SymbolVisitor {
+InvalidatedSymbols Symbols;
 
-class CollectReachableSymbolsCallback final : public SymbolVisitor {
-  InvalidatedSymbols Symbols;
+  public:
+explicit CollectReachableSymbolsCallback(ProgramStateRef State) {}
 
-public:
-  explicit CollectReachableSymbolsCallback(ProgramStateRef State) {}
+const InvalidatedSymbols &getSymbols() const { return Symbols; }
 
-  const InvalidatedSymbols &getSymbols() const { return Symbols; }
-
-  bool VisitSymbol(SymbolRef Sym) override {
-Symbols.insert(Sym);
-return true;
-  }
-};
+bool VisitSymbol(SymbolRef Sym) override {
+  Symbols.insert(Sym);
+  return true;
+}
+  };
 
-} // namespace
+  const CollectReachableSymbolsCallback &Scanner =
+  State->scanReachableSymbols(V);
+  return getCheckerManager().runCheckersForPointerEscape(
+  State, Scanner.getSymbols(), /*CallEvent*/ nullptr, K, nullptr);
+}
 
 void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
ExplodedNodeSet &DstTop) {
@@ -1529,17 +1533,8 @@
   ->getType()->isRecordType()))
   for (auto Child : Ex->children()) {
 assert(Child);
-
 SVal Val = State->getSVal(Child, LCtx);
-
-CollectReachableSymbolsCallback Scanner =
-State->scanReachableSymbols(
-Val);
-const InvalidatedSymbols &EscapedSy

[PATCH] D45217: [ThinLTO] Pass -save-temps to LTO backend for distributed ThinLTO builds

2018-04-16 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc accepted this revision.
pcc added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D45217



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


[PATCH] D45310: Warn about memcpy'ing non-trivial C structs

2018-04-16 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 142691.
ahatanak marked an inline comment as done.
ahatanak added a comment.

Move method visitArray to the derived classes and pass the array type to the 
method instead of the QualType.


Repository:
  rC Clang

https://reviews.llvm.org/D45310

Files:
  include/clang/AST/NonTrivialTypeVisitor.h
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/CodeGen/CGNonTrivialStruct.cpp
  lib/Sema/SemaChecking.cpp
  test/SemaObjC/warn-nontrivial-struct-memaccess.m

Index: test/SemaObjC/warn-nontrivial-struct-memaccess.m
===
--- /dev/null
+++ test/SemaObjC/warn-nontrivial-struct-memaccess.m
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fobjc-runtime-has-weak -x objective-c -fobjc-arc -verify %s
+
+void *memset(void *, int, __SIZE_TYPE__);
+void bzero(void *, __SIZE_TYPE__);
+void *memcpy(void *, const void *, __SIZE_TYPE__);
+void *memmove(void *, const void *, __SIZE_TYPE__);
+
+struct Trivial {
+  int f0;
+  volatile int f1;
+};
+
+struct NonTrivial0 {
+  int f0;
+  __weak id f1; // expected-note 2 {{non-trivial to default-initialize}} expected-note 2 {{non-trivial to copy}}
+  volatile int f2;
+  id f3[10]; // expected-note 2 {{non-trivial to default-initialize}} expected-note 2 {{non-trivial to copy}}
+};
+
+struct NonTrivial1 {
+  id f0; // expected-note 2 {{non-trivial to default-initialize}} expected-note 2 {{non-trivial to copy}}
+  int f1;
+  struct NonTrivial0 f2;
+};
+
+void testTrivial(struct Trivial *d, struct Trivial *s) {
+  memset(d, 0, sizeof(struct Trivial));
+  bzero(d, sizeof(struct Trivial));
+  memcpy(d, s, sizeof(struct Trivial));
+  memmove(d, s, sizeof(struct Trivial));
+}
+
+void testNonTrivial1(struct NonTrivial1 *d, struct NonTrivial1 *s) {
+  memset(d, 0, sizeof(struct NonTrivial1)); // expected-warning {{that is not trivial to primitive-default-initialize}} expected-note {{explicitly cast the pointer to silence}}
+  memset((void *)d, 0, sizeof(struct NonTrivial1));
+  bzero(d, sizeof(struct NonTrivial1)); // expected-warning {{that is not trivial to primitive-default-initialize}} expected-note {{explicitly cast the pointer to silence}}
+  memcpy(d, s, sizeof(struct NonTrivial1)); // expected-warning {{that is not trivial to primitive-copy}} expected-note {{explicitly cast the pointer to silence}}
+  memmove(d, s, sizeof(struct NonTrivial1)); // expected-warning {{that is not trivial to primitive-copy}} expected-note {{explicitly cast the pointer to silence}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -28,6 +28,7 @@
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/ExprOpenMP.h"
 #include "clang/AST/NSAPI.h"
+#include "clang/AST/NonTrivialTypeVisitor.h"
 #include "clang/AST/OperationKinds.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/TemplateBase.h"
@@ -7378,6 +7379,98 @@
   return QualType();
 }
 
+namespace {
+
+struct SearchNonTrivialToInitializeField
+: DefaultInitializedTypeVisitor {
+  using Super =
+  DefaultInitializedTypeVisitor;
+
+  SearchNonTrivialToInitializeField(const Expr *E, Sema &S) : E(E), S(S) {}
+
+  void visitWithKind(QualType::PrimitiveDefaultInitializeKind PDIK, QualType FT,
+ SourceLocation SL) {
+if (const auto *AT = asDerived().getContext().getAsArrayType(FT)) {
+  asDerived().visitArray(PDIK, AT, SL);
+  return;
+}
+
+Super::visitWithKind(PDIK, FT, SL);
+  }
+
+  void visitARCStrong(QualType FT, SourceLocation SL) {
+S.DiagRuntimeBehavior(SL, E, S.PDiag(diag::note_nontrivial_field) << 1);
+  }
+  void visitARCWeak(QualType FT, SourceLocation SL) {
+S.DiagRuntimeBehavior(SL, E, S.PDiag(diag::note_nontrivial_field) << 1);
+  }
+  void visitStruct(QualType FT, SourceLocation SL) {
+for (const FieldDecl *FD : FT->castAs()->getDecl()->fields())
+  visit(FD->getType(), FD->getLocation());
+  }
+  void visitArray(QualType::PrimitiveDefaultInitializeKind PDIK,
+  const ArrayType *AT, SourceLocation SL) {
+visit(getContext().getBaseElementType(AT), SL);
+  }
+  void visitTrivial(QualType FT, SourceLocation SL) {}
+
+  static void diag(QualType RT, const Expr *E, Sema &S) {
+SearchNonTrivialToInitializeField(E, S).visitStruct(RT, SourceLocation());
+  }
+
+  ASTContext &getContext() { return S.getASTContext(); }
+
+  const Expr *E;
+  Sema &S;
+};
+
+struct SearchNonTrivialToCopyField
+: CopiedTypeVisitor {
+  using Super = CopiedTypeVisitor;
+
+  SearchNonTrivialToCopyField(const Expr *E, Sema &S) : E(E), S(S) {}
+
+  void visitWithKind(QualType::PrimitiveCopyKind PCK, QualType FT,
+ SourceLocation SL) {
+if (const auto *AT = asDerived().getContext().getAsArrayType(FT)) {
+  asDerived().visitArray(PCK, AT, SL);
+  return;
+}
+
+Super::visitWithKind(PCK, FT, SL);
+  }
+
+  void visitARC

[PATCH] D45310: Warn about memcpy'ing non-trivial C structs

2018-04-16 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added inline comments.



Comment at: include/clang/AST/NonTrivialCStructTypeVisitor.h:30
+if (asDerived().getContext().getAsArrayType(FT))
+  return asDerived().visitArray(DK, FT, std::forward(Args)...);
+

rjmccall wrote:
> ahatanak wrote:
> > rjmccall wrote:
> > > Should you have this pass the array type down?  And is it really 
> > > important to do this in the generic visitor?  It seems like something you 
> > > could do in an IRGen subclass.
> > The subclasses in CGNonTrivialStruct.cpp need the size and the element type 
> > of the array to be passed to visitArray, so I think we have to pass the 
> > array type to visitArray. I guess it's possible to move this to the 
> > subclasses, but then the visit methods in the subclasses have to check 
> > whether the type is an array or not. I think we had a discussion on how 
> > arrays should be handled in this review: https://reviews.llvm.org/D41228.
> > 
> > But perhaps you have a better idea in mind?
> Well, you could "override" the visit method in the subclass, e.g.:
> 
>   template 
>   class CGDestructedTypeVisitor : public DestructedTypeVisitor RetTy> {
> using super = DestructedTypeVisitor;
> 
>   public:
> using super::asDerived;
> using super::visit;
> 
> template 
> RetTy visit(QualType::DestructionKind DK, QualType FT, Ts &&... Args) {
>   if (asDerived().getContext().getAsArrayType(FT))
> return asDerived().visitArray(DK, FT, std::forward(Args)...);
> 
>   return super::visit(DK, FT, std::forward(Args)...);
> }
>   };
> 
> It's a bit more boilerplate, but I really feel like the array logic doesn't 
> belong in the generic visitor.
> 
> About the array type: I wasn't trying to suggest that you should pass the 
> element type to visitArray, I was suggesting you could just pass the array 
> type as an `ArrayType*`, since that's what `visitArray` actually wants.
I incorporated both of your suggestions. I renamed the overloaded method 
'visit' to 'visitWithKind' so that the 'visit' method that doesn't take the 
DestructionKind doesn't get hidden by the one that takes DestructionKind and is 
defined in the derived classes. There are a couple of places in 
CGNonTrivialStruct.cpp that call the former method.


Repository:
  rC Clang

https://reviews.llvm.org/D45310



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


[PATCH] D45698: [analyzer] When we fail to evaluate a pointer cast, escape the pointer.

2018-04-16 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment.

I am in favour of this is what. This is what I suggested back than in 
https://reviews.llvm.org/D23014 but it was somehowoverlooked.


Repository:
  rC Clang

https://reviews.llvm.org/D45698



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


[PATCH] D45699: [Availability] Improve availability to consider functions run at load time

2018-04-16 Thread Steven Wu via Phabricator via cfe-commits
steven_wu created this revision.
steven_wu added a reviewer: arphaman.

There are some functions/methods that run when the application launches
or the library loads. Those functions will run reguardless the OS
version as long as it satifies the minimum deployment target. Annotate
them with availability attributes doesn't really make sense because they
are essentially available on all targets since minimum deployment
target.

rdar://problem/36093384


Repository:
  rC Clang

https://reviews.llvm.org/D45699

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaDeclObjC.cpp
  test/SemaObjC/unguarded-availability.m

Index: test/SemaObjC/unguarded-availability.m
===
--- test/SemaObjC/unguarded-availability.m
+++ test/SemaObjC/unguarded-availability.m
@@ -7,7 +7,7 @@
 
 typedef int AVAILABLE_10_12 new_int; // expected-note + {{marked partial here}}
 
-int func_10_11() AVAILABLE_10_11; // expected-note 4 {{'func_10_11' has been explicitly marked partial here}}
+int func_10_11() AVAILABLE_10_11; // expected-note 8 {{'func_10_11' has been explicitly marked partial here}}
 
 #ifdef OBJCPP
 // expected-note@+2 6 {{marked partial here}}
@@ -311,3 +311,41 @@
   (void)AK_Cat; // no warning
   (void)AK_CyborgCat; // expected-warning{{'AK_CyborgCat' is only available on macOS 10.12 or newer}} expected-note {{@available}}
 }
+
+
+// test static initializers has the same availability as the deployment target and it cannot be overwritten.
+@interface HasStaticInitializer : BaseClass
++ (void)load AVAILABLE_10_11; // expected-warning{{ignoring availability attribute on '+load' method}}
+@end
+
+@implementation HasStaticInitializer
++ (void)load {
+  func_10_11(); // expected-warning{{'func_10_11' is only available on macOS 10.11 or newer}} expected-note{{enclose 'func_10_11' in an @available check to silence this warning}}
+}
+@end
+
+// test availability from interface is ignored when checking the unguarded availability in +load method.
+AVAILABLE_10_11
+@interface HasStaticInitializer1 : BaseClass
++ (void)load;
+@end
+
+@implementation HasStaticInitializer1
++ (void)load {
+  func_10_11(); // expected-warning{{'func_10_11' is only available on macOS 10.11 or newer}} expected-note{{enclose 'func_10_11' in an @available check to silence this warning}}
+}
+@end
+
+__attribute__((constructor))
+void is_constructor();
+
+AVAILABLE_10_11 // expected-warning{{ignoring availability attribute with constructor attribute}}
+void is_constructor() {
+  func_10_11(); // expected-warning{{'func_10_11' is only available on macOS 10.11 or newer}} expected-note{{enclose 'func_10_11' in an @available check to silence this warning}}
+}
+
+AVAILABLE_10_11 // expected-warning{{ignoring availability attribute with desctructor attribute}}
+__attribute__((destructor))
+void is_destructor() {
+  func_10_11(); // expected-warning{{'func_10_11' is only available on macOS 10.11 or newer}} expected-note{{enclose 'func_10_11' in an @available check to silence this warning}}
+}
Index: lib/Sema/SemaDeclObjC.cpp
===
--- lib/Sema/SemaDeclObjC.cpp
+++ lib/Sema/SemaDeclObjC.cpp
@@ -4734,6 +4734,20 @@
   Context.getTargetInfo().getTriple().getArch() == llvm::Triple::x86)
 checkObjCMethodX86VectorTypes(*this, ObjCMethod);
 
+  // + load method cannot have availability attributes. It has to be the same
+  // as deployment target.
+  for (const auto& attr: ObjCMethod->attrs()) {
+if (!isa(attr))
+  continue;
+
+if (ObjCMethod->isClassMethod() &&
+ObjCMethod->getNameAsString() == "load") {
+  Diag(attr->getLocation(), diag::warn_availability_on_static_initializer)
+  << 0;
+  ObjCMethod->dropAttr();
+}
+  }
+
   ActOnDocumentableDecl(ObjCMethod);
 
   return ObjCMethod;
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -6823,6 +6823,11 @@
   return false;
 
 // An implementation implicitly has the availability of the interface.
+// Unless it is "+load" method.
+if (const auto *MethodD = dyn_cast(Ctx))
+  if (MethodD->isClassMethod() && MethodD->getNameAsString() == "load")
+return true;
+
 if (const auto *CatOrImpl = dyn_cast(Ctx)) {
   if (const ObjCInterfaceDecl *Interface = CatOrImpl->getClassInterface())
 if (CheckContext(Interface))
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -9131,6 +9131,25 @@
 AddToScope = false;
   }
 
+  // Diagnose availability attributes. Availability cannot be used on functions
+  // that are run during load/unload.
+  for (const auto& attr: NewFD->attrs()) {
+if (!isa(attr))
+  continue;
+
+if (NewFD->hasAttr()) {
+

r330159 - Defer adding keywords to the identifier table until after the language options have been loaded from the AST file.

2018-04-16 Thread Aaron Ballman via cfe-commits
Author: aaronballman
Date: Mon Apr 16 14:07:08 2018
New Revision: 330159

URL: http://llvm.org/viewvc/llvm-project?rev=330159&view=rev
Log:
Defer adding keywords to the identifier table until after the language options 
have been loaded from the AST file.

This fixes issues with "class" being reported as an identifier in "enum class" 
because the construct is not present when using default language options.

Patch by Johann Klähn.

Modified:
cfe/trunk/include/clang/Basic/IdentifierTable.h
cfe/trunk/lib/Basic/IdentifierTable.cpp
cfe/trunk/lib/Lex/Preprocessor.cpp
cfe/trunk/unittests/libclang/LibclangTest.cpp

Modified: cfe/trunk/include/clang/Basic/IdentifierTable.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/IdentifierTable.h?rev=330159&r1=330158&r2=330159&view=diff
==
--- cfe/trunk/include/clang/Basic/IdentifierTable.h (original)
+++ cfe/trunk/include/clang/Basic/IdentifierTable.h Mon Apr 16 14:07:08 2018
@@ -467,10 +467,13 @@ class IdentifierTable {
   IdentifierInfoLookup* ExternalLookup;
 
 public:
+  /// \brief Create the identifier table.
+  explicit IdentifierTable(IdentifierInfoLookup *ExternalLookup = nullptr);
+
   /// \brief Create the identifier table, populating it with info about the
   /// language keywords for the language specified by \p LangOpts.
-  IdentifierTable(const LangOptions &LangOpts,
-  IdentifierInfoLookup* externalLookup = nullptr);
+  explicit IdentifierTable(const LangOptions &LangOpts,
+   IdentifierInfoLookup *ExternalLookup = nullptr);
 
   /// \brief Set the external identifier lookup mechanism.
   void setExternalIdentifierLookup(IdentifierInfoLookup *IILookup) {
@@ -558,6 +561,8 @@ public:
   /// hashing is doing.
   void PrintStats() const;
 
+  /// \brief Populate the identifier table with info about the language 
keywords
+  /// for the language specified by \p LangOpts.
   void AddKeywords(const LangOptions &LangOpts);
 };
 

Modified: cfe/trunk/lib/Basic/IdentifierTable.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/IdentifierTable.cpp?rev=330159&r1=330158&r2=330159&view=diff
==
--- cfe/trunk/lib/Basic/IdentifierTable.cpp (original)
+++ cfe/trunk/lib/Basic/IdentifierTable.cpp Mon Apr 16 14:07:08 2018
@@ -79,16 +79,16 @@ IdentifierIterator *IdentifierInfoLookup
   return new EmptyLookupIterator();
 }
 
+IdentifierTable::IdentifierTable(IdentifierInfoLookup *ExternalLookup)
+: HashTable(8192), // Start with space for 8K identifiers.
+  ExternalLookup(ExternalLookup) {}
+
 IdentifierTable::IdentifierTable(const LangOptions &LangOpts,
- IdentifierInfoLookup* externalLookup)
-  : HashTable(8192), // Start with space for 8K identifiers.
-ExternalLookup(externalLookup) {
+ IdentifierInfoLookup *ExternalLookup)
+: IdentifierTable(ExternalLookup) {
   // Populate the identifier table with info about keywords for the current
   // language.
   AddKeywords(LangOpts);
-
-  // Add the '_experimental_modules_import' contextual keyword.
-  get("import").setModulesImport(true);
 }
 
 
//===--===//
@@ -237,6 +237,9 @@ void IdentifierTable::AddKeywords(const
 
   if (LangOpts.DeclSpecKeyword)
 AddKeyword("__declspec", tok::kw___declspec, KEYALL, LangOpts, *this);
+
+  // Add the '_experimental_modules_import' contextual keyword.
+  get("import").setModulesImport(true);
 }
 
 /// \brief Checks if the specified token kind represents a keyword in the

Modified: cfe/trunk/lib/Lex/Preprocessor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Preprocessor.cpp?rev=330159&r1=330158&r2=330159&view=diff
==
--- cfe/trunk/lib/Lex/Preprocessor.cpp (original)
+++ cfe/trunk/lib/Lex/Preprocessor.cpp Mon Apr 16 14:07:08 2018
@@ -85,12 +85,14 @@ Preprocessor::Preprocessor(std::shared_p
IdentifierInfoLookup *IILookup, bool OwnsHeaders,
TranslationUnitKind TUKind)
 : PPOpts(std::move(PPOpts)), Diags(&diags), LangOpts(opts),
-  FileMgr(Headers.getFileMgr()), SourceMgr(SM),
-  PCMCache(PCMCache), ScratchBuf(new ScratchBuffer(SourceMgr)),
-  HeaderInfo(Headers), TheModuleLoader(TheModuleLoader),
-  ExternalSource(nullptr), Identifiers(opts, IILookup),
-  PragmaHandlers(new PragmaNamespace(StringRef())), TUKind(TUKind),
-  SkipMainFilePreamble(0, true),
+  FileMgr(Headers.getFileMgr()), SourceMgr(SM), PCMCache(PCMCache),
+  ScratchBuf(new ScratchBuffer(SourceMgr)), HeaderInfo(Headers),
+  TheModuleLoader(TheModuleLoader), ExternalSource(nullptr),
+  // As the language options may have not been loaded yet (when
+  /

[PATCH] D35181: Defer addition of keywords to identifier table when loading AST

2018-04-16 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision.
aaron.ballman added a comment.

Committed in r330159. Thank you for the patch!


https://reviews.llvm.org/D35181



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


[PATCH] D45383: Limit types of builtins that can be redeclared.

2018-04-16 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

It looks like you didn't fix the tests?


https://reviews.llvm.org/D45383



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


[PATCH] D34331: func.wrap.func.con: Unset function before destroying anything

2018-04-16 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added a comment.

Ping.


https://reviews.llvm.org/D34331



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


[PATCH] D45698: [analyzer] When we fail to evaluate a pointer cast, escape the pointer.

2018-04-16 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

Hmm, yeah, indeed, i must have overlooked it, nice one :) I'll see if i can fix 
the other place as well.


Repository:
  rC Clang

https://reviews.llvm.org/D45698



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


[PATCH] D45383: Limit types of builtins that can be redeclared.

2018-04-16 Thread Erich Keane via Phabricator via cfe-commits
erichkeane updated this revision to Diff 142695.
erichkeane added a comment.

Revert unnecessary test changes...  :/


https://reviews.llvm.org/D45383

Files:
  include/clang/AST/ASTContext.h
  include/clang/Basic/Builtins.h
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/AST/ASTContext.cpp
  lib/Basic/Builtins.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaOverload.cpp
  test/Sema/builtin-redecl.cpp

Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -7244,6 +7244,10 @@
   return BuiltinMSVaListDecl;
 }
 
+bool ASTContext::canBuiltinBeRedeclared(const FunctionDecl *FD) const {
+  return BuiltinInfo.canBeRedeclared(FD->getBuiltinID());
+}
+
 void ASTContext::setObjCConstantStringInterface(ObjCInterfaceDecl *Decl) {
   assert(ObjCConstantStringType.isNull() &&
  "'NSConstantString' type already set!");
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -998,6 +998,13 @@
 Match = *I;
 return Ovl_Match;
   }
+
+  // Builtins that have custom typechecking or have a reference should
+  // not be overloadable or redeclarable.
+  if (!getASTContext().canBuiltinBeRedeclared(OldF)) {
+Match = *I;
+return Ovl_NonFunction;
+  }
 } else if (isa(OldD) || isa(OldD)) {
   // We can overload with these, which can show up when doing
   // redeclaration checks for UsingDecls.
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -3011,6 +3011,14 @@
   if (Old->isInvalidDecl())
 return true;
 
+  // Disallow redeclaration of some builtins.
+  if (!getASTContext().canBuiltinBeRedeclared(Old)) {
+Diag(New->getLocation(), diag::err_builtin_redeclare) << Old->getDeclName();
+Diag(Old->getLocation(), diag::note_previous_builtin_declaration)
+<< Old << Old->getType();
+return true;
+  }
+
   diag::kind PrevDiag;
   SourceLocation OldLocation;
   std::tie(PrevDiag, OldLocation) =
Index: lib/Basic/Builtins.cpp
===
--- lib/Basic/Builtins.cpp
+++ lib/Basic/Builtins.cpp
@@ -139,3 +139,10 @@
bool &HasVAListArg) {
   return isLike(ID, FormatIdx, HasVAListArg, "sS");
 }
+
+bool Builtin::Context::canBeRedeclared(unsigned ID) const {
+  return ID == Builtin::NotBuiltin ||
+ ID == Builtin::BI__va_start ||
+ (!hasReferenceArgsOrResult(ID) &&
+  !hasCustomTypechecking(ID));
+}
Index: include/clang/AST/ASTContext.h
===
--- include/clang/AST/ASTContext.h
+++ include/clang/AST/ASTContext.h
@@ -1881,6 +1881,10 @@
 return getTypeDeclType(getBuiltinMSVaListDecl());
   }
 
+  /// Return whether a declaration to a builtin is allowed to be
+  /// overloaded/redeclared.
+  bool canBuiltinBeRedeclared(const FunctionDecl *) const;
+
   /// \brief Return a type with additional \c const, \c volatile, or
   /// \c restrict qualifiers.
   QualType getCVRQualifiedType(QualType T, unsigned CVR) const {
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -602,6 +602,7 @@
   "incompatible redeclaration of library function %0">,
   InGroup>;
 def err_builtin_definition : Error<"definition of builtin function %0">;
+def err_builtin_redeclare : Error<"cannot redeclare builtin function %0">;
 def err_arm_invalid_specialreg : Error<"invalid special register for builtin">;
 def err_invalid_cpu_supports : Error<"invalid cpu feature string for builtin">;
 def err_invalid_cpu_is : Error<"invalid cpu name for builtin">;
Index: include/clang/Basic/Builtins.h
===
--- include/clang/Basic/Builtins.h
+++ include/clang/Basic/Builtins.h
@@ -167,6 +167,13 @@
 return strchr(getRecord(ID).Type, '*') != nullptr;
   }
 
+  /// \brief Return true if this builtin has a result or any arguments which are
+  /// reference types.
+  bool hasReferenceArgsOrResult(unsigned ID) const {
+return strchr(getRecord(ID).Type, '&') != nullptr ||
+   strchr(getRecord(ID).Type, 'A') != nullptr;
+  }
+
   /// \brief Completely forget that the given ID was ever considered a builtin,
   /// e.g., because the user provided a conflicting signature.
   void forgetBuiltin(unsigned ID, IdentifierTable &Table);
@@ -212,6 +219,10 @@
   /// prefix.
   static bool isBuiltinFunc(const char *Name);
 
+  /// Returns true if this is a builtin that can be redeclared.  Returns true
+  /// for non-builtins.
+  bool canBeRedeclared(unsigned ID) const;
+
 private:
   const Info &getRecord(unsig

  1   2   >