[PATCH] D55662: [Sema][ObjC] Do not warn about repeated uses of weak variables when the variables are accessed in an unevaluated context.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D55662#1346059 , @ahatanak wrote:

> In D55662#1346055 , @rjmccall wrote:
>
> > I think you could just disable the diagnostic in an unevaluated context.
>
>
> The call to `isUnevaluatedContext` in `ObjCPropertyOpBuilder::complete` 
> returns false when the type of `auto __weak wp` in `testAuto` is being 
> deduced because the `ExpressionEvaluationContextRecord` on the stack isn't an 
> unevaluated context. I can silence the warning if I can push an unevaluated 
> context at the entry of `Sema::DeduceAutoType`, but it's not clear to me that 
> it's safe to do so.


Hmm.  Are we resolving the placeholder expression twice here?  That might be 
the basic problem.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55662/new/

https://reviews.llvm.org/D55662



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

[PATCH] D56241: expose a control flag for interger to pointer ext warning

[Roman Lebedev via Phabricator via cfe-commits]

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

ok, LGTM.




Comment at: test/Sema/ext-typecheck-comparison-of-pointer-integer.c:1
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only  -verify 
-Wpointer-integer-compare %s 
+

Kader wrote:
> lebedev.ri wrote:
> > I guess you want a run-line with no flag, this runline, and a run-line with 
> > `-Wno-pointer-integer-compare`
> done
In the future, do use that checkbox on the inline comments to mark them as done.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56241/new/

https://reviews.llvm.org/D56241



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

For the record, another option is to actually fix other software not to call LD 
directly.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56215/new/

https://reviews.llvm.org/D56215



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

[PATCH] D55662: [Sema][ObjC] Do not warn about repeated uses of weak variables when the variables are accessed in an unevaluated context.

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

In D55662#1346055 , @rjmccall wrote:

> I think you could just disable the diagnostic in an unevaluated context.


The call to `isUnevaluatedContext` in `ObjCPropertyOpBuilder::complete` returns 
false when the type of `auto __weak wp` in `testAuto` is being deduced because 
the `ExpressionEvaluationContextRecord` on the stack isn't an unevaluated 
context. I can silence the warning if I can push an unevaluated context at the 
entry of `Sema::DeduceAutoType`, but it's not clear to me that it's safe to do 
so.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55662/new/

https://reviews.llvm.org/D55662



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

[PATCH] D55865: [ObjC] Add a new attribute to opt-out of implicit callee retain/release in ARC

[Erik Pilkington via Phabricator via cfe-commits]

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

Mention `__has_attribute` and the ignored without -fobjc-arc thing. 
@aaron.ballman: Do you have any more thoughts here?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55865/new/

https://reviews.llvm.org/D55865

Files:
  clang/docs/AutomaticReferenceCounting.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclObjC.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/CodeGenObjC/externally-retained.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/externally-retained-no-arc.m
  clang/test/SemaObjC/externally-retained.m

Index: clang/test/SemaObjC/externally-retained.m
===
--- /dev/null
+++ clang/test/SemaObjC/externally-retained.m
@@ -0,0 +1,114 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.13.0 -fobjc-runtime=macosx-10.13.0 -fblocks -fobjc-arc %s -verify
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.13.0 -fobjc-runtime=macosx-10.13.0 -fblocks -fobjc-arc -xobjective-c++ %s -verify
+
+#define EXT_RET __attribute__((objc_externally_retained))
+
+@interface ObjCTy
+@end
+
+void test1() {
+  EXT_RET int a; // expected-warning{{'objc_externally_retained' can only be applied to}}
+  EXT_RET __weak ObjCTy *b; // expected-warning{{'objc_externally_retained' can only be applied to}}
+  EXT_RET __weak int (^c)(); // expected-warning{{'objc_externally_retained' can only be applied to}}
+
+  EXT_RET int (^d)() = ^{return 0;};
+  EXT_RET ObjCTy *e = 0;
+  EXT_RET __strong ObjCTy *f = 0;
+
+  e = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+  f = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+  d = ^{ return 0; }; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+}
+
+void test2(ObjCTy *a);
+
+void test2(ObjCTy *a) EXT_RET {
+  a = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+}
+
+EXT_RET ObjCTy *test3; // expected-warning{{'objc_externally_retained' can only be applied to}}
+
+@interface X // expected-warning{{defined without specifying a base class}} expected-note{{add a super class}}
+-(void)m: (ObjCTy *) p;
+@end
+@implementation X
+-(void)m: (ObjCTy *) p EXT_RET {
+  p = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+}
+@end
+
+void test4() {
+  __attribute__((objc_externally_retained(0))) ObjCTy *a; // expected-error{{'objc_externally_retained' attribute takes no arguments}}
+}
+
+void test5(ObjCTy *first, __strong ObjCTy *second) EXT_RET {
+  first = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+  second = 0; // fine
+}
+
+void test6(ObjCTy *first,
+   __strong ObjCTy *second) EXT_RET {
+  first = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+  second = 0;
+}
+
+__attribute__((objc_root_class)) @interface Y @end
+
+@implementation Y
+- (void)test7:(__strong ObjCTy *)first
+withThird:(ObjCTy *)second EXT_RET {
+  first = 0;
+  second = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+}
+@end
+
+void (^blk)(ObjCTy *, ObjCTy *) =
+^(__strong ObjCTy *first, ObjCTy *second) EXT_RET {
+  first = 0;
+  second = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+};
+
+void test8(EXT_RET ObjCTy *x) {} // expected-warning{{'objc_externally_retained' attribute only applies to variables}}
+
+#pragma clang attribute ext_ret.push(__attribute__((objc_externally_retained)), apply_to=any(function, block, objc_method))
+void test9(ObjCTy *first, __strong ObjCTy *second) {
+  first = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+  second = 0;
+}
+void (^test10)(ObjCTy *first, ObjCTy *second) = ^(ObjCTy *first, __strong ObjCTy *second) {
+  first = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+  second = 0;
+};
+__attribute__((objc_root_class)) @interface Test11 @end
+@implementation Test11
+-(void)meth: (ObjCTy *)first withSecond:(__strong ObjCTy *)second {
+  first = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+  second = 0;
+}
++(void)othermeth: (ObjCTy *)first withSecond:(__strong ObjCTy *)second {
+  first = 0

[PATCH] D55662: [Sema][ObjC] Do not warn about repeated uses of weak variables when the variables are accessed in an unevaluated context.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I think you could just disable the diagnostic in an unevaluated context.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55662/new/

https://reviews.llvm.org/D55662



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

[PATCH] D55865: [ObjC] Add a new attribute to opt-out of implicit callee retain/release in ARC

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Sorry, I keep coming up with small tweaks to the documentation.  These should 
be the last of them, so if Aaron's also satisfied, feel free to commit with 
those modifications.




Comment at: clang/docs/AutomaticReferenceCounting.rst:1788
+// ...
+  }
+

Consider adding a paragraph describing how to test for support for this 
attribute.



Comment at: clang/include/clang/Basic/AttrDocs.td:3801
+Likewise, when applied to a strong local variable, that variable becomes
+``const`` and is considered externally-retained.
+}]; }

Please add a paragraph saying that this attribute has no effect when ARC isn't 
enabled.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55865/new/

https://reviews.llvm.org/D55865



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

[PATCH] D56241: expose a control flag for interger to pointer ext warning

[abdoul-kader keita via Phabricator via cfe-commits]

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



Comment at: test/Sema/ext-typecheck-comparison-of-pointer-integer.c:1
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only  -verify 
-Wpointer-integer-compare %s 
+

lebedev.ri wrote:
> I guess you want a run-line with no flag, this runline, and a run-line with 
> `-Wno-pointer-integer-compare`
done


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56241/new/

https://reviews.llvm.org/D56241



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

[PATCH] D55662: [Sema][ObjC] Do not warn about repeated uses of weak variables when the variables are accessed in an unevaluated context.

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 180186.
ahatanak marked an inline comment as done.
ahatanak added a comment.

Do not record use of weak variables when the type of an auto variable is being 
deduced.

I'm not sure creating an RAII class is the best way to silence the warning, but 
I haven't been able to come up with something better.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55662/new/

https://reviews.llvm.org/D55662

Files:
  include/clang/Sema/Sema.h
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprCXX.cpp
  lib/Sema/SemaExprMember.cpp
  lib/Sema/SemaExprObjC.cpp
  lib/Sema/SemaPseudoObject.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaType.cpp
  test/SemaObjC/arc-repeated-weak.mm

Index: test/SemaObjC/arc-repeated-weak.mm
===
--- test/SemaObjC/arc-repeated-weak.mm
+++ test/SemaObjC/arc-repeated-weak.mm
@@ -462,6 +462,13 @@
   NSString * t2 = NSBundle.foo2.prop;
   use(NSBundle.foo2.weakProp); // expected-warning{{weak property 'weakProp' may be accessed multiple times}}
   use(NSBundle2.foo2.weakProp); // expected-note{{also accessed here}}
+  decltype([NSBundle2.foo2 weakProp]) t3;
+  decltype(NSBundle2.foo2.weakProp) t4;
+  __typeof__(NSBundle2.foo2.weakProp) t5;
+}
+
+void testAuto() {
+  auto __weak wp = NSBundle2.foo2.weakProp;
 }
 
 // This used to crash in the constructor of WeakObjectProfileTy when a
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -8056,9 +8056,7 @@
 }
 
 QualType Sema::BuildTypeofExprType(Expr *E, SourceLocation Loc) {
-  ExprResult ER = CheckPlaceholderExpr(E);
-  if (ER.isInvalid()) return QualType();
-  E = ER.get();
+  assert(!E->hasPlaceholderType() && "unexpected placeholder");
 
   if (!getLangOpts().CPlusPlus && E->refersToBitField())
 Diag(E->getExprLoc(), diag::err_sizeof_alignof_typeof_bitfield) << 2;
@@ -8143,9 +8141,7 @@
 
 QualType Sema::BuildDecltypeType(Expr *E, SourceLocation Loc,
  bool AsUnevaluated) {
-  ExprResult ER = CheckPlaceholderExpr(E);
-  if (ER.isInvalid()) return QualType();
-  E = ER.get();
+  assert(!E->hasPlaceholderType() && "unexpected placeholder");
 
   if (AsUnevaluated && CodeSynthesisContexts.empty() &&
   E->HasSideEffects(Context, false)) {
Index: lib/Sema/SemaTemplateDeduction.cpp
===
--- lib/Sema/SemaTemplateDeduction.cpp
+++ lib/Sema/SemaTemplateDeduction.cpp
@@ -4402,6 +4402,8 @@
 Sema::DeduceAutoResult
 Sema::DeduceAutoType(TypeLoc Type, Expr *&Init, QualType &Result,
  Optional DependentDeductionDepth) {
+  DeduceAutoTypeRAII RAII(*this);
+
   if (Init->getType()->isNonOverloadPlaceholderType()) {
 ExprResult NonPlaceholder = CheckPlaceholderExpr(Init);
 if (NonPlaceholder.isInvalid())
@@ -4429,6 +4431,10 @@
 return DAR_FailedAlreadyDiagnosed;
   }
 
+  ExprResult ER = CheckPlaceholderExpr(Init);
+  if (ER.isInvalid())
+return DAR_FailedAlreadyDiagnosed;
+  Init = ER.get();
   QualType Deduced = BuildDecltypeType(Init, Init->getBeginLoc(), false);
   if (Deduced.isNull())
 return DAR_FailedAlreadyDiagnosed;
Index: lib/Sema/SemaPseudoObject.cpp
===
--- lib/Sema/SemaPseudoObject.cpp
+++ lib/Sema/SemaPseudoObject.cpp
@@ -975,7 +975,7 @@
 }
 
 ExprResult ObjCPropertyOpBuilder::complete(Expr *SyntacticForm) {
-  if (isWeakProperty() && !S.isUnevaluatedContext() &&
+  if (isWeakProperty() && !S.isUnevaluatedContext() && !S.deducingAutoType() &&
   !S.Diags.isIgnored(diag::warn_arc_repeated_use_of_weak,
  SyntacticForm->getBeginLoc()))
 S.getCurFunction()->recordUseOfWeak(SyntacticRefExpr,
Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -3161,7 +3161,7 @@
   Prop->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_weak;
 if (!IsWeak && Sel.isUnarySelector())
   IsWeak = ReturnType.getObjCLifetime() & Qualifiers::OCL_Weak;
-if (IsWeak &&
+if (IsWeak && !isUnevaluatedContext() &&
 !Diags.isIgnored(diag::warn_arc_repeated_use_of_weak, LBracLoc))
   getCurFunction()->recordUseOfWeak(Result, Prop);
   }
Index: lib/Sema/SemaExprMember.cpp
===
--- lib/Sema/SemaExprMember.cpp
+++ lib/Sema/SemaExprMember.cpp
@@ -1712,6 +1712,8 @@
   Base, Base->getType(), OpLoc, IsArrow, SS, TemplateKWLoc,
   FirstQualifierInScope, NameInfo, TemplateArgs, S, &ExtraArgs);
 
+  Res = CheckPlaceholderExpr(Res.get());
+
   if (!Res.isInvalid() && isa(Res.get()))
 CheckMemberAccessOfNoDeref(cast(Res.get()));
 
Index: lib/Sema/SemaE

[PATCH] D56303: Handle case/default statements when simplifying boolean expressions

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood updated this revision to Diff 180184.
LegalizeAdulthood added a comment.

clang-format


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56303/new/

https://reviews.llvm.org/D56303

Files:
  clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tidy/readability/SimplifyBooleanExprCheck.h
  test/clang-tidy/readability-simplify-bool-expr-case.cpp

Index: test/clang-tidy/readability-simplify-bool-expr-case.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-simplify-bool-expr-case.cpp
@@ -0,0 +1,586 @@
+// RUN: %check_clang_tidy %s readability-simplify-boolean-expr %t
+
+bool switch_stmt(int i, int j, bool b) {
+  switch (i) {
+  case 0:
+if (b == true)
+  j = 10;
+break;
+// CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{if \(b\)}}
+// CHECK-FIXES-NEXT: {{j = 10;}}
+
+  case 1:
+if (b == false)
+  j = -20;
+break;
+// CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{if \(!b\)}}
+// CHECK-FIXES-NEXT: {{j = -20;}}
+
+  case 2:
+if (b && true)
+  j = 10;
+break;
+// CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{if \(b\)}}
+// CHECK-FIXES-NEXT: {{j = 10;}}
+
+  case 3:
+if (b && false)
+  j = -20;
+break;
+// CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{if \(false\)}}
+// CHECK-FIXES-NEXT: {{j = -20;}}
+
+  case 4:
+if (b || true)
+  j = 10;
+break;
+// CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{if \(true\)}}
+// CHECK-FIXES-NEXT: {{j = 10;}}
+// CHECK-FIXES-NEXT: {{break;}}
+
+  case 5:
+if (b || false)
+  j = -20;
+break;
+// CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{if \(b\)}}
+// CHECK-FIXES-NEXT: {{j = -20;}}
+
+  case 6:
+return i > 0 ? true : false;
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: {{.*}} in ternary expression result
+// CHECK-FIXES: {{return i > 0;}}
+
+  case 7:
+return i > 0 ? false : true;
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: {{.*}} in ternary expression result
+// CHECK-FIXES: {{return i <= 0;}}
+
+  case 8:
+if (true)
+  j = 10;
+else
+  j = -20;
+break;
+// CHECK-MESSAGES: :[[@LINE-5]]:9: warning: {{.*}} in if statement condition
+// CHECK-FIXES:  {{j = 10;$}}
+// CHECK-FIXES-NEXT: {{break;$}}
+
+  case 9:
+if (false)
+  j = -20;
+else
+  j = 10;
+break;
+// CHECK-MESSAGES: :[[@LINE-5]]:9: warning: {{.*}} in if statement condition
+// CHECK-FIXES: {{j = 10;}}
+// CHECK-FIXES-NEXT: {{break;}}
+
+  case 10:
+if (j > 10)
+  return true;
+else
+  return false;
+// CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} in conditional return statement
+// CHECK-FIXES: {{return j > 10;}}
+
+  case 11:
+if (j > 10)
+  return false;
+else
+  return true;
+// CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} in conditional return statement
+// CHECK-FIXES: {{return j <= 10;}}
+
+  case 12:
+if (j > 10)
+  b = true;
+else
+  b = false;
+return b;
+// CHECK-MESSAGES: :[[@LINE-4]]:11: warning: {{.*}} in conditional assignment
+// CHECK-FIXES: {{b = j > 10;}}
+// CHECK-FIXES-NEXT: {{return b;}}
+
+  case 13:
+if (j > 10)
+  b = false;
+else
+  b = true;
+return b;
+// CHECK-MESSAGES: :[[@LINE-4]]:11: warning: {{.*}} in conditional assignment
+// CHECK-FIXES: {{b = j <= 10;}}
+// CHECK-FIXES-NEXT: {{return b;}}
+
+  case 14:
+if (j > 10)
+  return true;
+return false;
+// CHECK-MESSAGES: :[[@LINE-2]]:14: warning: {{.*}} in conditional return
+// FIXES: {{return j > 10;}}
+
+  case 15:
+if (j > 10)
+  return false;
+return true;
+// CHECK-MESSAGES: :[[@LINE-2]]:14: warning: {{.*}} in conditional return
+// FIXES: {{return j <= 10;}}
+
+  case 16:
+if (j > 10)
+  return true;
+return true;
+return false;
+
+  case 17:
+if (j > 10)
+  return false;
+return false;
+return true;
+
+  case 100: {
+if (b == true)
+  j = 10;
+break;
+// CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{if \(b\)}}
+// CHECK-FIXES-NEXT: {{j = 10;}}
+  }
+
+  case 101: {
+if (b == false)
+  j = -20;
+break;
+// CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{if \(!b\)}}
+// CHECK-FIXES-NEXT: {{j = -20;}}
+  }
+
+  case 102: {
+if (b && true)
+  j = 10;
+break;
+// CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{if \(b\)}}
+// CHECK-FIXES-NEXT: {{j = 10;}}
+  }
+
+  case 103: {
+

[PATCH] D56303: Handle case/default statements when simplifying boolean expressions

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood created this revision.
LegalizeAdulthood added a reviewer: cfe-commits.
LegalizeAdulthood added a project: clang-tools-extra.

Structure this similarly to the way replaceCompoundReturnWithCondition works.

- Add readability-simplify-boolean-expr test cases for case stmt
- Add readability-simplify-boolean-expr test cases for default stmt
- Add negative test cases

Fixes #26704


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D56303

Files:
  clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tidy/readability/SimplifyBooleanExprCheck.h
  test/clang-tidy/readability-simplify-bool-expr-case.cpp

Index: test/clang-tidy/readability-simplify-bool-expr-case.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-simplify-bool-expr-case.cpp
@@ -0,0 +1,586 @@
+// RUN: %check_clang_tidy %s readability-simplify-boolean-expr %t
+
+bool switch_stmt(int i, int j, bool b) {
+  switch (i) {
+  case 0:
+if (b == true)
+  j = 10;
+break;
+// CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{if \(b\)}}
+// CHECK-FIXES-NEXT: {{j = 10;}}
+
+  case 1:
+if (b == false)
+  j = -20;
+break;
+// CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{if \(!b\)}}
+// CHECK-FIXES-NEXT: {{j = -20;}}
+
+  case 2:
+if (b && true)
+  j = 10;
+break;
+// CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{if \(b\)}}
+// CHECK-FIXES-NEXT: {{j = 10;}}
+
+  case 3:
+if (b && false)
+  j = -20;
+break;
+// CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{if \(false\)}}
+// CHECK-FIXES-NEXT: {{j = -20;}}
+
+  case 4:
+if (b || true)
+  j = 10;
+break;
+// CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{if \(true\)}}
+// CHECK-FIXES-NEXT: {{j = 10;}}
+// CHECK-FIXES-NEXT: {{break;}}
+
+  case 5:
+if (b || false)
+  j = -20;
+break;
+// CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{if \(b\)}}
+// CHECK-FIXES-NEXT: {{j = -20;}}
+
+  case 6:
+return i > 0 ? true : false;
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: {{.*}} in ternary expression result
+// CHECK-FIXES: {{return i > 0;}}
+
+  case 7:
+return i > 0 ? false : true;
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: {{.*}} in ternary expression result
+// CHECK-FIXES: {{return i <= 0;}}
+
+  case 8:
+if (true)
+  j = 10;
+else
+  j = -20;
+break;
+// CHECK-MESSAGES: :[[@LINE-5]]:9: warning: {{.*}} in if statement condition
+// CHECK-FIXES:  {{j = 10;$}}
+// CHECK-FIXES-NEXT: {{break;$}}
+
+  case 9:
+if (false)
+  j = -20;
+else
+  j = 10;
+break;
+// CHECK-MESSAGES: :[[@LINE-5]]:9: warning: {{.*}} in if statement condition
+// CHECK-FIXES: {{j = 10;}}
+// CHECK-FIXES-NEXT: {{break;}}
+
+  case 10:
+if (j > 10)
+  return true;
+else
+  return false;
+// CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} in conditional return statement
+// CHECK-FIXES: {{return j > 10;}}
+
+  case 11:
+if (j > 10)
+  return false;
+else
+  return true;
+// CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} in conditional return statement
+// CHECK-FIXES: {{return j <= 10;}}
+
+  case 12:
+if (j > 10)
+  b = true;
+else
+  b = false;
+return b;
+// CHECK-MESSAGES: :[[@LINE-4]]:11: warning: {{.*}} in conditional assignment
+// CHECK-FIXES: {{b = j > 10;}}
+// CHECK-FIXES-NEXT: {{return b;}}
+
+  case 13:
+if (j > 10)
+  b = false;
+else
+  b = true;
+return b;
+// CHECK-MESSAGES: :[[@LINE-4]]:11: warning: {{.*}} in conditional assignment
+// CHECK-FIXES: {{b = j <= 10;}}
+// CHECK-FIXES-NEXT: {{return b;}}
+
+  case 14:
+if (j > 10)
+  return true;
+return false;
+// CHECK-MESSAGES: :[[@LINE-2]]:14: warning: {{.*}} in conditional return
+// FIXES: {{return j > 10;}}
+
+  case 15:
+if (j > 10)
+  return false;
+return true;
+// CHECK-MESSAGES: :[[@LINE-2]]:14: warning: {{.*}} in conditional return
+// FIXES: {{return j <= 10;}}
+
+  case 16:
+if (j > 10)
+  return true;
+return true;
+return false;
+
+  case 17:
+if (j > 10)
+  return false;
+return false;
+return true;
+
+  case 100: {
+if (b == true)
+  j = 10;
+break;
+// CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{if \(b\)}}
+// CHECK-FIXES-NEXT: {{j = 10;}}
+  }
+
+  case 101: {
+if (b == false)
+  j = -20;
+break;
+// CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{if \(!b\)}}
+// CHECK-FI

[PATCH] D52847: [clang-doc] Handle anonymous namespaces

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 180182.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D52847/new/

https://reviews.llvm.org/D52847

Files:
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp

Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -132,6 +132,7 @@
 
   NamespaceInfo *A = InfoAsNamespace(Infos[0].get());
   NamespaceInfo ExpectedA(EmptySID);
+  ExpectedA.Name = "@nonymous_namespace";
   CheckNamespaceInfo(&ExpectedA, A);
 }
 
Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -133,9 +133,6 @@
   if (CreateDirectory(Path))
 return llvm::make_error("Unable to create directory.\n",
llvm::inconvertibleErrorCode());
-
-  if (Name.empty())
-Name = "GlobalNamespace";
   llvm::sys::path::append(Path, Name + Ext);
   return Path;
 }
@@ -223,8 +220,8 @@
 
 doc::Info *I = Reduced.get().get();
 
-auto InfoPath =
-getInfoOutputFile(OutDirectory, I->Namespace, I->Name, "." + Format);
+auto InfoPath = getInfoOutputFile(OutDirectory, I->Namespace,
+  I->extractName(), "." + Format);
 if (!InfoPath) {
   llvm::errs() << toString(InfoPath.takeError()) << "\n";
   continue;
Index: clang-tools-extra/clang-doc/Serialize.cpp
===
--- clang-tools-extra/clang-doc/Serialize.cpp
+++ clang-tools-extra/clang-doc/Serialize.cpp
@@ -268,13 +268,19 @@
 template 
 static void
 populateParentNamespaces(llvm::SmallVector &Namespaces,
- const T *D) {
+ const T *D, bool &IsAnonymousNamespace) {
   const auto *DC = dyn_cast(D);
   while ((DC = DC->getParent())) {
-if (const auto *N = dyn_cast(DC))
-  Namespaces.emplace_back(getUSRForDecl(N), N->getNameAsString(),
+if (const auto *N = dyn_cast(DC)) {
+  std::string Namespace;
+  if (N->isAnonymousNamespace()) {
+Namespace = "@nonymous_namespace";
+IsAnonymousNamespace = true;
+  } else
+Namespace = N->getNameAsString();
+  Namespaces.emplace_back(getUSRForDecl(N), Namespace,
   InfoType::IT_namespace);
-else if (const auto *N = dyn_cast(DC))
+} else if (const auto *N = dyn_cast(DC))
   Namespaces.emplace_back(getUSRForDecl(N), N->getNameAsString(),
   InfoType::IT_record);
 else if (const auto *N = dyn_cast(DC))
@@ -287,10 +293,11 @@
 }
 
 template 
-static void populateInfo(Info &I, const T *D, const FullComment *C) {
+static void populateInfo(Info &I, const T *D, const FullComment *C,
+ bool &IsInAnonymousNamespace) {
   I.USR = getUSRForDecl(D);
   I.Name = D->getNameAsString();
-  populateParentNamespaces(I.Namespace, D);
+  populateParentNamespaces(I.Namespace, D, IsInAnonymousNamespace);
   if (C) {
 I.Description.emplace_back();
 parseFullComment(C, I.Description.back());
@@ -299,8 +306,9 @@
 
 template 
 static void populateSymbolInfo(SymbolInfo &I, const T *D, const FullComment *C,
-   int LineNumber, StringRef Filename) {
-  populateInfo(I, D, C);
+   int LineNumber, StringRef Filename,
+   bool &IsInAnonymousNamespace) {
+  populateInfo(I, D, C, IsInAnonymousNamespace);
   if (D->isThisDeclarationADefinition())
 I.DefLoc.emplace(LineNumber, Filename);
   else
@@ -309,8 +317,9 @@
 
 static void populateFunctionInfo(FunctionInfo &I, const FunctionDecl *D,
  const FullComment *FC, int LineNumber,
- StringRef Filename) {
-  populateSymbolInfo(I, D, FC, LineNumber, Filename);
+ StringRef Filename,
+ bool &IsInAnonymousNamespace) {
+  populateSymbolInfo(I, D, FC, LineNumber, Filename, IsInAnonymousNamespace);
   if (const auto *T = getDeclForType(D->getReturnType())) {
 if (dyn_cast(T))
   I.ReturnType =
@@ -327,21 +336,28 @@
 std::unique_ptr emitInfo(const NamespaceDecl *D, const FullComment *FC,
int LineNumber, llvm::StringRef File,
bool PublicOnly) {
-  if (PublicOnly && ((D->isAnonymousNamespace()) ||
+  auto I = llvm::make_unique();
+  bool IsInAnonymousNamespace = false;
+  populateInfo(*I, D, FC, IsInAnonymousNam

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

In D56215#1345845 , @ruiu wrote:

> Not sure what I understand the point, but as to make lld work on/for NetBSD, 
> I wonder if you can just add -L to the command line in the compiler 
> driver. Isn't a NetBSD triple passed to the compiler driver? If so, I wonder 
> if you can add a few extra options when invoking the linker.


This describes the original patch of passing flags from compiler driver and 
breaks GNU ld compat. Joerg expects to pass no extra `-L` or 
`--disable-new-dtags` options, treating lld as a drop-in replacement for GNU ld.

I'm trying to determine how the idea of Joerg can be implemented.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56215/new/

https://reviews.llvm.org/D56215



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

[PATCH] D56299: [OpenMP] Refactor const restriction for linear

[Joel E. Denny via Phabricator via cfe-commits]

jdenny created this revision.
jdenny added a reviewer: ABataev.
Herald added a subscriber: guansong.

As discussed in D56113 , this patch refactors 
the implementation of the
const restriction for linear to reuse a function introduced by D56113 
.
A side effect is that, if a variable has mutable members, this
diagnostic is now skipped, and the diagnostic for the variable not
being an integer or pointer is reported instead.


Repository:
  rC Clang

https://reviews.llvm.org/D56299

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_linear_messages.cpp
  clang/test/OpenMP/distribute_simd_linear_messages.cpp
  clang/test/OpenMP/for_linear_messages.cpp
  clang/test/OpenMP/for_simd_linear_messages.cpp
  clang/test/OpenMP/parallel_for_linear_messages.cpp
  clang/test/OpenMP/parallel_for_simd_linear_messages.cpp
  clang/test/OpenMP/simd_linear_messages.cpp
  clang/test/OpenMP/target_parallel_for_linear_messages.cpp
  clang/test/OpenMP/target_parallel_for_simd_linear_messages.cpp
  clang/test/OpenMP/target_simd_linear_messages.cpp
  
clang/test/OpenMP/target_teams_distribute_parallel_for_simd_linear_messages.cpp
  clang/test/OpenMP/target_teams_distribute_simd_linear_messages.cpp
  clang/test/OpenMP/taskloop_simd_linear_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_simd_linear_messages.cpp
  clang/test/OpenMP/teams_distribute_simd_linear_messages.cpp

Index: clang/test/OpenMP/teams_distribute_simd_linear_messages.cpp
===
--- clang/test/OpenMP/teams_distribute_simd_linear_messages.cpp
+++ clang/test/OpenMP/teams_distribute_simd_linear_messages.cpp
@@ -169,7 +169,7 @@
   for (int k = 0; k < argc; ++k) ++k;
 
 #pragma omp target
-#pragma omp teams distribute simd linear (a, b:B::ib) // expected-error {{linear variable with incomplete type 'S1'}} expected-error {{const-qualified variable cannot be linear}}
+#pragma omp teams distribute simd linear (a, b:B::ib) // expected-error {{linear variable with incomplete type 'S1'}} expected-error {{argument of a linear clause should be of integral or pointer type, not 'S2'}}
   for (int k = 0; k < argc; ++k) ++k;
 
 #pragma omp target
@@ -250,7 +250,7 @@
 
 
 #pragma omp target
-#pragma omp teams distribute simd linear (a, b) // expected-error {{linear variable with incomplete type 'S1'}} expected-error {{const-qualified variable cannot be linear}}
+#pragma omp teams distribute simd linear (a, b) // expected-error {{linear variable with incomplete type 'S1'}} expected-error {{argument of a linear clause should be of integral or pointer type, not 'S2'}}
   for (int k = 0; k < argc; ++k) ++k;
 
 #pragma omp target
Index: clang/test/OpenMP/teams_distribute_parallel_for_simd_linear_messages.cpp
===
--- clang/test/OpenMP/teams_distribute_parallel_for_simd_linear_messages.cpp
+++ clang/test/OpenMP/teams_distribute_parallel_for_simd_linear_messages.cpp
@@ -169,7 +169,7 @@
   for (int k = 0; k < argc; ++k) ++k;
 
 #pragma omp target
-#pragma omp teams distribute parallel for simd linear (a, b:B::ib) // expected-error {{linear variable with incomplete type 'S1'}} expected-error {{const-qualified variable cannot be linear}}
+#pragma omp teams distribute parallel for simd linear (a, b:B::ib) // expected-error {{linear variable with incomplete type 'S1'}} expected-error {{argument of a linear clause should be of integral or pointer type, not 'S2'}}
   for (int k = 0; k < argc; ++k) ++k;
 
 #pragma omp target
@@ -250,7 +250,7 @@
 
 
 #pragma omp target
-#pragma omp teams distribute parallel for simd linear (a, b) // expected-error {{linear variable with incomplete type 'S1'}} expected-error {{const-qualified variable cannot be linear}}
+#pragma omp teams distribute parallel for simd linear (a, b) // expected-error {{linear variable with incomplete type 'S1'}} expected-error {{argument of a linear clause should be of integral or pointer type, not 'S2'}}
   for (int k = 0; k < argc; ++k) ++k;
 
 #pragma omp target
Index: clang/test/OpenMP/taskloop_simd_linear_messages.cpp
===
--- clang/test/OpenMP/taskloop_simd_linear_messages.cpp
+++ clang/test/OpenMP/taskloop_simd_linear_messages.cpp
@@ -132,7 +132,7 @@
   #pragma omp taskloop simd linear (S1) // expected-error {{'S1' does not refer to a value}}
   for (int k = 0; k < argc; ++k) ++k;
   // expected-error@+2 {{linear variable with incomplete type 'S1'}}
-  // expected-error@+1 {{const-qualified variable cannot be linear}}
+  // expected-error@+1 {{argument of a linear clause should be of integral or pointer type, not 'S2'}}
   #pragma omp taskloop simd linear (val(a, b):B::ib)
   for (int k = 0; k < argc; ++k) ++k;
   #pragma omp taskloop simd linear (argv[1]) // expected-error {{expected variable name}}
@@ -221,7 +221,7 @@
   #pragma

[PATCH] D56188: Adopt SwiftABIInfo for WebAssembly.

[Daniel Dunbar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC350372: Adopt SwiftABIInfo for WebAssembly. (authored by 
ddunbar, committed by ).

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56188/new/

https://reviews.llvm.org/D56188

Files:
  lib/CodeGen/TargetInfo.cpp


Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -720,10 +720,12 @@
 // This is a very simple ABI that relies a lot on DefaultABIInfo.
 
//===--===//
 
-class WebAssemblyABIInfo final : public DefaultABIInfo {
+class WebAssemblyABIInfo final : public SwiftABIInfo {
+  DefaultABIInfo defaultInfo;
+
 public:
   explicit WebAssemblyABIInfo(CodeGen::CodeGenTypes &CGT)
-  : DefaultABIInfo(CGT) {}
+  : SwiftABIInfo(CGT), defaultInfo(CGT) {}
 
 private:
   ABIArgInfo classifyReturnType(QualType RetTy) const;
@@ -741,6 +743,15 @@
 
   Address EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
 QualType Ty) const override;
+
+  bool shouldPassIndirectlyForSwift(ArrayRef scalars,
+bool asReturnValue) const override {
+return occupiesMoreThan(CGT, scalars, /*total*/ 4);
+  }
+
+  bool isSwiftErrorInRegister() const override {
+return false;
+  }
 };
 
 class WebAssemblyTargetCodeGenInfo final : public TargetCodeGenInfo {
@@ -778,7 +789,7 @@
   }
 
   // Otherwise just do the default thing.
-  return DefaultABIInfo::classifyArgumentType(Ty);
+  return defaultInfo.classifyArgumentType(Ty);
 }
 
 ABIArgInfo WebAssemblyABIInfo::classifyReturnType(QualType RetTy) const {
@@ -798,7 +809,7 @@
   }
 
   // Otherwise just do the default thing.
-  return DefaultABIInfo::classifyReturnType(RetTy);
+  return defaultInfo.classifyReturnType(RetTy);
 }
 
 Address WebAssemblyABIInfo::EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
@@ -8307,7 +8318,7 @@
 }
 
 ABIArgInfo ARCABIInfo::getIndirectByValue(QualType Ty) const {
-  // Compute the byval alignment. 
+  // Compute the byval alignment.
   const unsigned MinABIStackAlignInBytes = 4;
   unsigned TypeAlign = getContext().getTypeAlign(Ty) / 8;
   return ABIArgInfo::getIndirect(CharUnits::fromQuantity(4), /*ByVal=*/true,
@@ -8371,7 +8382,7 @@
   if (RetTy->isAnyComplexType())
 return ABIArgInfo::getDirectInReg();
 
-  // Arguments of size > 4 registers are indirect.  
+  // Arguments of size > 4 registers are indirect.
   auto RetSize = llvm::alignTo(getContext().getTypeSize(RetTy), 32) / 32;
   if (RetSize > 4)
 return getIndirectByRef(RetTy, /*HasFreeRegs*/ true);


Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -720,10 +720,12 @@
 // This is a very simple ABI that relies a lot on DefaultABIInfo.
 //===--===//
 
-class WebAssemblyABIInfo final : public DefaultABIInfo {
+class WebAssemblyABIInfo final : public SwiftABIInfo {
+  DefaultABIInfo defaultInfo;
+
 public:
   explicit WebAssemblyABIInfo(CodeGen::CodeGenTypes &CGT)
-  : DefaultABIInfo(CGT) {}
+  : SwiftABIInfo(CGT), defaultInfo(CGT) {}
 
 private:
   ABIArgInfo classifyReturnType(QualType RetTy) const;
@@ -741,6 +743,15 @@
 
   Address EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
 QualType Ty) const override;
+
+  bool shouldPassIndirectlyForSwift(ArrayRef scalars,
+bool asReturnValue) const override {
+return occupiesMoreThan(CGT, scalars, /*total*/ 4);
+  }
+
+  bool isSwiftErrorInRegister() const override {
+return false;
+  }
 };
 
 class WebAssemblyTargetCodeGenInfo final : public TargetCodeGenInfo {
@@ -778,7 +789,7 @@
   }
 
   // Otherwise just do the default thing.
-  return DefaultABIInfo::classifyArgumentType(Ty);
+  return defaultInfo.classifyArgumentType(Ty);
 }
 
 ABIArgInfo WebAssemblyABIInfo::classifyReturnType(QualType RetTy) const {
@@ -798,7 +809,7 @@
   }
 
   // Otherwise just do the default thing.
-  return DefaultABIInfo::classifyReturnType(RetTy);
+  return defaultInfo.classifyReturnType(RetTy);
 }
 
 Address WebAssemblyABIInfo::EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
@@ -8307,7 +8318,7 @@
 }
 
 ABIArgInfo ARCABIInfo::getIndirectByValue(QualType Ty) const {
-  // Compute the byval alignment. 
+  // Compute the byval alignment.
   const unsigned MinABIStackAlignInBytes = 4;
   unsigned TypeAlign = getContext().getTypeAlign(Ty) / 8;
   return ABIArgInfo::getIndirect(CharUnits::fromQuantity(4), /*ByVal=*/true,
@@ -8371,7 +8382,7 @@
   if (RetTy->isAnyComplexType())
 return ABIArgInfo::getDirectInReg();
 
-  // Arguments of size > 4 registers are indirect.  
+  // Arguments of size > 4 registers are indirect.
   auto RetSize = ll

r350372 - Adopt SwiftABIInfo for WebAssembly.

[Daniel Dunbar via cfe-commits]

Author: ddunbar
Date: Thu Jan  3 15:24:50 2019
New Revision: 350372

URL: http://llvm.org/viewvc/llvm-project?rev=350372&view=rev
Log:
Adopt SwiftABIInfo for WebAssembly.

Summary:
 - This adopts SwiftABIInfo as the base class for WebAssemblyABIInfo, which is 
in keeping with what is done for other targets for which Swift is supported.

 - This is a minimal patch to unblock exploration of WASM support for Swift 
(https://bugs.swift.org/browse/SR-9307)

Reviewers: rjmccall, sunfish

Reviewed By: rjmccall

Subscribers: ahti, dschuff, sbc100, jgravelle-google, aheejin, cfe-commits

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

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

Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=350372&r1=350371&r2=350372&view=diff
==
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Thu Jan  3 15:24:50 2019
@@ -720,10 +720,12 @@ ABIArgInfo DefaultABIInfo::classifyRetur
 // This is a very simple ABI that relies a lot on DefaultABIInfo.
 
//===--===//
 
-class WebAssemblyABIInfo final : public DefaultABIInfo {
+class WebAssemblyABIInfo final : public SwiftABIInfo {
+  DefaultABIInfo defaultInfo;
+
 public:
   explicit WebAssemblyABIInfo(CodeGen::CodeGenTypes &CGT)
-  : DefaultABIInfo(CGT) {}
+  : SwiftABIInfo(CGT), defaultInfo(CGT) {}
 
 private:
   ABIArgInfo classifyReturnType(QualType RetTy) const;
@@ -741,6 +743,15 @@ private:
 
   Address EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
 QualType Ty) const override;
+
+  bool shouldPassIndirectlyForSwift(ArrayRef scalars,
+bool asReturnValue) const override {
+return occupiesMoreThan(CGT, scalars, /*total*/ 4);
+  }
+
+  bool isSwiftErrorInRegister() const override {
+return false;
+  }
 };
 
 class WebAssemblyTargetCodeGenInfo final : public TargetCodeGenInfo {
@@ -778,7 +789,7 @@ ABIArgInfo WebAssemblyABIInfo::classifyA
   }
 
   // Otherwise just do the default thing.
-  return DefaultABIInfo::classifyArgumentType(Ty);
+  return defaultInfo.classifyArgumentType(Ty);
 }
 
 ABIArgInfo WebAssemblyABIInfo::classifyReturnType(QualType RetTy) const {
@@ -798,7 +809,7 @@ ABIArgInfo WebAssemblyABIInfo::classifyR
   }
 
   // Otherwise just do the default thing.
-  return DefaultABIInfo::classifyReturnType(RetTy);
+  return defaultInfo.classifyReturnType(RetTy);
 }
 
 Address WebAssemblyABIInfo::EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
@@ -8307,7 +8318,7 @@ ABIArgInfo ARCABIInfo::getIndirectByRef(
 }
 
 ABIArgInfo ARCABIInfo::getIndirectByValue(QualType Ty) const {
-  // Compute the byval alignment. 
+  // Compute the byval alignment.
   const unsigned MinABIStackAlignInBytes = 4;
   unsigned TypeAlign = getContext().getTypeAlign(Ty) / 8;
   return ABIArgInfo::getIndirect(CharUnits::fromQuantity(4), /*ByVal=*/true,
@@ -8371,7 +8382,7 @@ ABIArgInfo ARCABIInfo::classifyReturnTyp
   if (RetTy->isAnyComplexType())
 return ABIArgInfo::getDirectInReg();
 
-  // Arguments of size > 4 registers are indirect.  
+  // Arguments of size > 4 registers are indirect.
   auto RetSize = llvm::alignTo(getContext().getTypeSize(RetTy), 32) / 32;
   if (RetSize > 4)
 return getIndirectByRef(RetTy, /*HasFreeRegs*/ true);


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

r350371 - Fix MSVC visualizations for ActionResult and OpaquePtr

[Mike Spertus via cfe-commits]

Author: mps
Date: Thu Jan  3 15:24:39 2019
New Revision: 350371

URL: http://llvm.org/viewvc/llvm-project?rev=350371&view=rev
Log:
Fix MSVC visualizations for ActionResult and OpaquePtr

Mainly just fixing buggy code. Also removed unnecessary visualizers
for specializations of OpaquePtr


Modified:
cfe/trunk/utils/ClangVisualizers/clang.natvis

Modified: cfe/trunk/utils/ClangVisualizers/clang.natvis
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/ClangVisualizers/clang.natvis?rev=350371&r1=350370&r2=350371&view=diff
==
--- cfe/trunk/utils/ClangVisualizers/clang.natvis (original)
+++ cfe/trunk/utils/ClangVisualizers/clang.natvis Thu Jan  3 15:24:39 2019
@@ -492,18 +492,6 @@ For later versions of Visual Studio, no
   *(clang::Type *)this, view(cmn)
 
   
-  
-{*(clang::QualType *)this}
-
-  *(clang::QualType *)this
-
-  
-  
-{*(clang::QualType *)this}
-
-  *(clang::QualType *)this
-
-  
   
 {($T1 *)Ptr}
 
@@ -568,30 +556,18 @@ For later versions of Visual Studio, no
 {Ambiguity,en}: {Decls}
 {ResultKind,en}: {Decls}
   
-  
-Invalid
-Valid
-   
-  
+  
 Invalid
-Valid
+Unset
+{Val}
   
-  
-{*this,view(packedValidity)}: {($T1 
*)(PtrWithInvalid&~1)}
+  
+Invalid
+Unset
+{($T1)(PtrWithInvalid&~1)}
 
   (bool)(PtrWithInvalid&1)
-  ($T1 *)(PtrWithInvalid&~1)
-
-  
-  
-{*this,view(unpackedValidity)}: {Val}
-  
-  
-{*this,view(packed)}
-{*this,view(unpacked)}
-
-  *this,view(packed)
-  *this,view(unpacked)
+  ($T1)(PtrWithInvalid&~1)
 
   
 


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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Rui Ueyama via Phabricator via cfe-commits]

ruiu added a comment.

Not sure what I understand the point, but as to make lld work on/for NetBSD, I 
wonder if you can just add -L to the command line in the compiler driver. 
Isn't a NetBSD triple passed to the compiler driver? If so, I wonder if you can 
add a few extra options when invoking the linker.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56215/new/

https://reviews.llvm.org/D56215



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

[PATCH] D55865: [ObjC] Add a new attribute to opt-out of implicit callee retain/release in ARC

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington updated this revision to Diff 180154.
erik.pilkington marked an inline comment as done.
erik.pilkington added a comment.

Remove support for parameter indices.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55865/new/

https://reviews.llvm.org/D55865

Files:
  clang/docs/AutomaticReferenceCounting.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclObjC.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/CodeGenObjC/externally-retained.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/externally-retained-no-arc.m
  clang/test/SemaObjC/externally-retained.m

Index: clang/test/SemaObjC/externally-retained.m
===
--- /dev/null
+++ clang/test/SemaObjC/externally-retained.m
@@ -0,0 +1,114 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.13.0 -fobjc-runtime=macosx-10.13.0 -fblocks -fobjc-arc %s -verify
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.13.0 -fobjc-runtime=macosx-10.13.0 -fblocks -fobjc-arc -xobjective-c++ %s -verify
+
+#define EXT_RET __attribute__((objc_externally_retained))
+
+@interface ObjCTy
+@end
+
+void test1() {
+  EXT_RET int a; // expected-warning{{'objc_externally_retained' can only be applied to}}
+  EXT_RET __weak ObjCTy *b; // expected-warning{{'objc_externally_retained' can only be applied to}}
+  EXT_RET __weak int (^c)(); // expected-warning{{'objc_externally_retained' can only be applied to}}
+
+  EXT_RET int (^d)() = ^{return 0;};
+  EXT_RET ObjCTy *e = 0;
+  EXT_RET __strong ObjCTy *f = 0;
+
+  e = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+  f = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+  d = ^{ return 0; }; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+}
+
+void test2(ObjCTy *a);
+
+void test2(ObjCTy *a) EXT_RET {
+  a = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+}
+
+EXT_RET ObjCTy *test3; // expected-warning{{'objc_externally_retained' can only be applied to}}
+
+@interface X // expected-warning{{defined without specifying a base class}} expected-note{{add a super class}}
+-(void)m: (ObjCTy *) p;
+@end
+@implementation X
+-(void)m: (ObjCTy *) p EXT_RET {
+  p = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+}
+@end
+
+void test4() {
+  __attribute__((objc_externally_retained(0))) ObjCTy *a; // expected-error{{'objc_externally_retained' attribute takes no arguments}}
+}
+
+void test5(ObjCTy *first, __strong ObjCTy *second) EXT_RET {
+  first = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+  second = 0; // fine
+}
+
+void test6(ObjCTy *first,
+   __strong ObjCTy *second) EXT_RET {
+  first = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+  second = 0;
+}
+
+__attribute__((objc_root_class)) @interface Y @end
+
+@implementation Y
+- (void)test7:(__strong ObjCTy *)first
+withThird:(ObjCTy *)second EXT_RET {
+  first = 0;
+  second = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+}
+@end
+
+void (^blk)(ObjCTy *, ObjCTy *) =
+^(__strong ObjCTy *first, ObjCTy *second) EXT_RET {
+  first = 0;
+  second = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+};
+
+void test8(EXT_RET ObjCTy *x) {} // expected-warning{{'objc_externally_retained' attribute only applies to variables}}
+
+#pragma clang attribute ext_ret.push(__attribute__((objc_externally_retained)), apply_to=any(function, block, objc_method))
+void test9(ObjCTy *first, __strong ObjCTy *second) {
+  first = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+  second = 0;
+}
+void (^test10)(ObjCTy *first, ObjCTy *second) = ^(ObjCTy *first, __strong ObjCTy *second) {
+  first = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+  second = 0;
+};
+__attribute__((objc_root_class)) @interface Test11 @end
+@implementation Test11
+-(void)meth: (ObjCTy *)first withSecond:(__strong ObjCTy *)second {
+  first = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+  second = 0;
+}
++(void)othermeth: (ObjCTy *)first withSecond:(__strong ObjCTy *)second {
+  first = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be mo

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

Actually I think that we can handle two cases witch a combination of proposals:

- native mode
- cross mode

For the native mode we go for something like:

  #if defined(__NetBSD__)
  #define NATIVE_TARGET LLD_NETBSD
  #else
  ...

and for cross mode read `argv[0]` to detect target triple in program name.

If `argv[0] == ld` we assume native mode and set target to native, otherwise 
try to parse target from `argv[0]`, if the target is not parsable bail out with 
error.

This way we can transplant cc driver information into lld.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56215/new/

https://reviews.llvm.org/D56215



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

r350363 - Fix check-hwasan with LLVM_BUILD_EXTERNAL_COMPILER_RT=ON

[Evgeniy Stepanov via cfe-commits]

Author: eugenis
Date: Thu Jan  3 14:50:45 2019
New Revision: 350363

URL: http://llvm.org/viewvc/llvm-project?rev=350363&view=rev
Log:
Fix check-hwasan with LLVM_BUILD_EXTERNAL_COMPILER_RT=ON

Add a forwarding target for check-hwasan in clang.

Modified:
cfe/trunk/runtime/CMakeLists.txt

Modified: cfe/trunk/runtime/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/runtime/CMakeLists.txt?rev=350363&r1=350362&r2=350363&view=diff
==
--- cfe/trunk/runtime/CMakeLists.txt (original)
+++ cfe/trunk/runtime/CMakeLists.txt Thu Jan  3 14:50:45 2019
@@ -126,7 +126,7 @@ if(LLVM_BUILD_EXTERNAL_COMPILER_RT AND E
   FileCheck count not llvm-nm llvm-objdump llvm-symbolizer)
 
 # Add top-level targets for various compiler-rt test suites.
-set(COMPILER_RT_TEST_SUITES check-fuzzer check-asan check-asan-dynamic 
check-dfsan
+set(COMPILER_RT_TEST_SUITES check-fuzzer check-asan check-hwasan 
check-asan-dynamic check-dfsan
   check-lsan check-msan check-sanitizer check-tsan check-ubsan 
check-ubsan-minimal
   check-profile check-cfi check-cfi-and-supported check-safestack)
 foreach(test_suite ${COMPILER_RT_TEST_SUITES})


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

[PATCH] D55955: Properly diagnose [[nodiscard]] on the body of a range-based for loop

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

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

Thanks, this looks great. A couple of the changes to the tests look like the 
diagnostic output is slightly worse in some error recovery conditions, but 
generally this is a nice improvement.




Comment at: test/Parser/switch-recovery.cpp:108
   expected-error {{no member named 'x' in the global namespace; 
did you mean simply 'x'?}} \
-  expected-warning 2 {{expression result unused}}
+  expected-warning {{expression result unused}}
 9:: :y; // expected-error {{expected ';' after expression}} \

Hmm, why do we only get one warning here? I'd expect one warning for the `8;` 
and one for the `x;` (after applying the fixes from the errors).



Comment at: test/SemaCXX/for-range-examples.cpp:181
 for (+x : {1, 2, 3}) {} // expected-error {{undeclared identifier}} 
expected-error {{expected ';'}}
-for (+y : {1, 2, 3}) {} // expected-error {{must declare a variable}}
+for (+y : {1, 2, 3}) {} // expected-error {{must declare a variable}} 
expected-warning {{expression result unused}}
   }

The new warnings here aren't ideal; do you know why they show up?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55955/new/

https://reviews.llvm.org/D55955



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

r350361 - [cmake] Fix monorepo + LLVM_BUILD_EXTERNAL_COMPILER_RT=ON.

[Evgeniy Stepanov via cfe-commits]

Author: eugenis
Date: Thu Jan  3 14:41:10 2019
New Revision: 350361

URL: http://llvm.org/viewvc/llvm-project?rev=350361&view=rev
Log:
[cmake] Fix monorepo + LLVM_BUILD_EXTERNAL_COMPILER_RT=ON.

In cmake 3.10.2, if (${VARIABLE_NAME}) seems to always be false no
matter what documentation says (or maybe I just failed at reading).

Anyway, if (VARIABLE_NAME) seems to do what this code intended.

Modified:
cfe/trunk/runtime/CMakeLists.txt

Modified: cfe/trunk/runtime/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/runtime/CMakeLists.txt?rev=350361&r1=350360&r2=350361&view=diff
==
--- cfe/trunk/runtime/CMakeLists.txt (original)
+++ cfe/trunk/runtime/CMakeLists.txt Thu Jan  3 14:41:10 2019
@@ -28,7 +28,7 @@ set(COMPILER_RT_SRC_ROOT ${LLVM_MAIN_SRC
 # variable) as in add_llvm_external_project
 if(NOT EXISTS ${COMPILER_RT_SRC_ROOT})
   # We don't want to set it if LLVM_EXTERNAL_COMPILER_RT_SOURCE_DIR is ""
-  if(${LLVM_EXTERNAL_COMPILER_RT_SOURCE_DIR})
+  if(LLVM_EXTERNAL_COMPILER_RT_SOURCE_DIR)
 set(COMPILER_RT_SRC_ROOT ${LLVM_EXTERNAL_COMPILER_RT_SOURCE_DIR})
   endif()
 endif()


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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

In D56215#1345695 , @ruiu wrote:

> In D56215#1345417 , @joerg wrote:
>
> > Talking from the perspective of having had to deal with thousands of 
> > packages in pkgsrc over the years: it is naive to believe that there isn't 
> > a lot of software that calls the linker directly for various reasons. As 
> > such, it is very important to have a useful configuration in the linker by 
> > default. Such a configuration is by its very nature target specific. This 
> > doesn't mean it can't be done in a cross-compiler friendly way, on the 
> > contrary. Over the years NetBSD has been pushing its toolchain to be as 
> > similar for the native build and a cross-target as reasonable possible. 
> > Modulo the build time choices in the config.h sense, the only difference 
> > between the native and cross tools is the built-in default of the former.
>
>
> It might be naive but I don't think it's too naive. Most programs use ld via 
> cc, and I don't think it is too unreasonable to not implement a host-specific 
> logic for a very small percentage of programs that directly use ld. If we do, 
> that logic would be the same or very similar to the one that we already have 
> in cc. I think we should avoid that duplication of the host-specific config 
> in the toolchain.
>
> I see there are pros and cons to not have host-specific config in ld. That 
> said, if other operating systems can live without host-specific config in 
> lld, why can't NetBSD do? I really don't like to be in a situation where only 
> one operating system have a host-specific config while others don't.


Before we even can start a philosophical dispute, is there some way to even get 
that setup functional in lld? How to check if the target is supposed to be a 
NetBSD binary? @mgorny raised some proposals.. 
https://reviews.llvm.org/D56215#1345563 but all of them seem to be either 
dysfunctional (ifdefs) or hackish (renaming emul names or renaming lld binary 
name to netbsd-*).

Looking at the code of lld, the detection of FreeBSD is done by checking 
emulation name (*fbsd) and anything else by a flag passed to a linker such as 
`--do-something-for-android` (e.g. "use-android-relr-tags")


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56215/new/

https://reviews.llvm.org/D56215



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

[PATCH] D56288: [ELF] Do not enable 'new dtags' on NetBSD by default

[Rui Ueyama via Phabricator via cfe-commits]

ruiu added a comment.

The compiler driver should pass -disable-new-tags to the linker, but let's 
discuss that in https://reviews.llvm.org/D56215 instead of here.


Repository:
  rLLD LLVM Linker

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56288/new/

https://reviews.llvm.org/D56288



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

[PATCH] D56241: expose a control flag for interger to pointer ext warning

[abdoul-kader keita via Phabricator via cfe-commits]

Kader updated this revision to Diff 180143.
Kader added a comment.

remove typo


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56241/new/

https://reviews.llvm.org/D56241

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  test/Misc/warning-flags.c
  test/Sema/ext-typecheck-comparison-of-pointer-integer.c


Index: test/Sema/ext-typecheck-comparison-of-pointer-integer.c
===
--- /dev/null
+++ test/Sema/ext-typecheck-comparison-of-pointer-integer.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only  -verify 
-DEXPECTWARNING %s 
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only  -verify 
-Wno-pointer-integer-compare %s 
+
+#ifdef EXPECTWARNING
+// expected-warning@+6 {{comparison between pointer and integer ('int' and 
'int *')}}
+#else
+// expected-no-diagnostics 
+#endif
+
+int test_ext_typecheck_comparison_of_pointer_integer(int integer, int * 
pointer) {
+   return integer != pointer; 
+}
Index: test/Misc/warning-flags.c
===
--- test/Misc/warning-flags.c
+++ test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (75):
+CHECK: Warnings without flags (74):
 CHECK-NEXT:   ext_excess_initializers
 CHECK-NEXT:   ext_excess_initializers_in_char_array_initializer
 CHECK-NEXT:   ext_expected_semi_decl_list
@@ -29,7 +29,6 @@
 CHECK-NEXT:   ext_new_paren_array_nonconst
 CHECK-NEXT:   ext_plain_complex
 CHECK-NEXT:   ext_template_arg_extra_parens
-CHECK-NEXT:   ext_typecheck_comparison_of_pointer_integer
 CHECK-NEXT:   ext_typecheck_cond_incompatible_operands
 CHECK-NEXT:   ext_typecheck_ordered_comparison_of_pointer_integer
 CHECK-NEXT:   ext_using_undefined_std
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5875,7 +5875,8 @@
 def err_typecheck_comparison_of_fptr_to_void : Error<
   "equality comparison between function pointer and void pointer (%0 and %1)">;
 def ext_typecheck_comparison_of_pointer_integer : ExtWarn<
-  "comparison between pointer and integer (%0 and %1)">;
+  "comparison between pointer and integer (%0 and %1)">,
+  InGroup>;
 def err_typecheck_comparison_of_pointer_integer : Error<
   "comparison between pointer and integer (%0 and %1)">;
 def ext_typecheck_comparison_of_distinct_pointers : ExtWarn<


Index: test/Sema/ext-typecheck-comparison-of-pointer-integer.c
===
--- /dev/null
+++ test/Sema/ext-typecheck-comparison-of-pointer-integer.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only  -verify -DEXPECTWARNING %s 
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only  -verify -Wno-pointer-integer-compare %s 
+
+#ifdef EXPECTWARNING
+// expected-warning@+6 {{comparison between pointer and integer ('int' and 'int *')}}
+#else
+// expected-no-diagnostics 
+#endif
+
+int test_ext_typecheck_comparison_of_pointer_integer(int integer, int * pointer) {
+	return integer != pointer; 
+}
Index: test/Misc/warning-flags.c
===
--- test/Misc/warning-flags.c
+++ test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (75):
+CHECK: Warnings without flags (74):
 CHECK-NEXT:   ext_excess_initializers
 CHECK-NEXT:   ext_excess_initializers_in_char_array_initializer
 CHECK-NEXT:   ext_expected_semi_decl_list
@@ -29,7 +29,6 @@
 CHECK-NEXT:   ext_new_paren_array_nonconst
 CHECK-NEXT:   ext_plain_complex
 CHECK-NEXT:   ext_template_arg_extra_parens
-CHECK-NEXT:   ext_typecheck_comparison_of_pointer_integer
 CHECK-NEXT:   ext_typecheck_cond_incompatible_operands
 CHECK-NEXT:   ext_typecheck_ordered_comparison_of_pointer_integer
 CHECK-NEXT:   ext_using_undefined_std
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5875,7 +5875,8 @@
 def err_typecheck_comparison_of_fptr_to_void : Error<
   "equality comparison between function pointer and void pointer (%0 and %1)">;
 def ext_typecheck_comparison_of_pointer_integer : ExtWarn<
-  "comparison between pointer and integer (%0 and %1)">;
+  "comparison between pointer and integer (%0 and %1)">,
+  InGroup>;
 def err_typecheck_comparison_of_pointer_integer : Error<
   "comparison between pointer and integer (%0 and %1)">;
 def ext_typecheck_comparison_of_distinct_pointers : ExtWarn<
___
cfe-commits mailing list
cfe-commits@lists.llv

[PATCH] D56241: expose a control flag for interger to pointer ext warning

[abdoul-kader keita via Phabricator via cfe-commits]

Kader updated this revision to Diff 180142.
Kader added a comment.

address comments


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56241/new/

https://reviews.llvm.org/D56241

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  test/Misc/warning-flags.c
  test/Sema/ext-typecheck-comparison-of-pointer-integer.c


Index: test/Sema/ext-typecheck-comparison-of-pointer-integer.c
===
--- /dev/null
+++ test/Sema/ext-typecheck-comparison-of-pointer-integer.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only  -verify 
-DEXPECTWARNING -Wpointer-integer-compare %s 
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only  -verify 
-Wno-pointer-integer-compare %s 
+
+#ifdef EXPECTWARNING
+// expected-warning@+6 {{comparison between pointer and integer ('int' and 
'int *')}}
+#else
+// expected-no-diagnostics 
+#endif
+
+int test_ext_typecheck_comparison_of_pointer_integer(int integer, int * 
pointer) {
+   return integer != pointer; 
+}
Index: test/Misc/warning-flags.c
===
--- test/Misc/warning-flags.c
+++ test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (75):
+CHECK: Warnings without flags (74):
 CHECK-NEXT:   ext_excess_initializers
 CHECK-NEXT:   ext_excess_initializers_in_char_array_initializer
 CHECK-NEXT:   ext_expected_semi_decl_list
@@ -29,7 +29,6 @@
 CHECK-NEXT:   ext_new_paren_array_nonconst
 CHECK-NEXT:   ext_plain_complex
 CHECK-NEXT:   ext_template_arg_extra_parens
-CHECK-NEXT:   ext_typecheck_comparison_of_pointer_integer
 CHECK-NEXT:   ext_typecheck_cond_incompatible_operands
 CHECK-NEXT:   ext_typecheck_ordered_comparison_of_pointer_integer
 CHECK-NEXT:   ext_using_undefined_std
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5875,7 +5875,8 @@
 def err_typecheck_comparison_of_fptr_to_void : Error<
   "equality comparison between function pointer and void pointer (%0 and %1)">;
 def ext_typecheck_comparison_of_pointer_integer : ExtWarn<
-  "comparison between pointer and integer (%0 and %1)">;
+  "comparison between pointer and integer (%0 and %1)">,
+  InGroup>;
 def err_typecheck_comparison_of_pointer_integer : Error<
   "comparison between pointer and integer (%0 and %1)">;
 def ext_typecheck_comparison_of_distinct_pointers : ExtWarn<


Index: test/Sema/ext-typecheck-comparison-of-pointer-integer.c
===
--- /dev/null
+++ test/Sema/ext-typecheck-comparison-of-pointer-integer.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only  -verify -DEXPECTWARNING -Wpointer-integer-compare %s 
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only  -verify -Wno-pointer-integer-compare %s 
+
+#ifdef EXPECTWARNING
+// expected-warning@+6 {{comparison between pointer and integer ('int' and 'int *')}}
+#else
+// expected-no-diagnostics 
+#endif
+
+int test_ext_typecheck_comparison_of_pointer_integer(int integer, int * pointer) {
+	return integer != pointer; 
+}
Index: test/Misc/warning-flags.c
===
--- test/Misc/warning-flags.c
+++ test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (75):
+CHECK: Warnings without flags (74):
 CHECK-NEXT:   ext_excess_initializers
 CHECK-NEXT:   ext_excess_initializers_in_char_array_initializer
 CHECK-NEXT:   ext_expected_semi_decl_list
@@ -29,7 +29,6 @@
 CHECK-NEXT:   ext_new_paren_array_nonconst
 CHECK-NEXT:   ext_plain_complex
 CHECK-NEXT:   ext_template_arg_extra_parens
-CHECK-NEXT:   ext_typecheck_comparison_of_pointer_integer
 CHECK-NEXT:   ext_typecheck_cond_incompatible_operands
 CHECK-NEXT:   ext_typecheck_ordered_comparison_of_pointer_integer
 CHECK-NEXT:   ext_using_undefined_std
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5875,7 +5875,8 @@
 def err_typecheck_comparison_of_fptr_to_void : Error<
   "equality comparison between function pointer and void pointer (%0 and %1)">;
 def ext_typecheck_comparison_of_pointer_integer : ExtWarn<
-  "comparison between pointer and integer (%0 and %1)">;
+  "comparison between pointer and integer (%0 and %1)">,
+  InGroup>;
 def err_typecheck_comparison_of_pointer_integer : Error<
   "comparison between pointer and integer (%0 and %1)">;
 def ext_typecheck_comparison_of_distinct_pointers : ExtWarn<
_

Re: [PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Joerg Sonnenberger via cfe-commits]

On Thu, Jan 03, 2019 at 10:26:38PM +, Rui Ueyama via Phabricator via 
cfe-commits wrote:
> I see there are pros and cons to not have host-specific config in ld.
> That said, if other operating systems can live without host-specific
> config in lld, why can't NetBSD do? I really don't like to be in a
> situation where only one operating system have a host-specific config
> while others don't.

They might not care because they use the thousand monkey approach to
software patching. At least for NetBSD, that's certainly man power
that's better spend elsewhere. They might simply have ignored lld so far
for any of the more interesting cases (like supporting multi-abi).
Lots of possible reasons :)

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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Rui Ueyama via Phabricator via cfe-commits]

ruiu added a comment.

In D56215#1345417 , @joerg wrote:

> Talking from the perspective of having had to deal with thousands of packages 
> in pkgsrc over the years: it is naive to believe that there isn't a lot of 
> software that calls the linker directly for various reasons. As such, it is 
> very important to have a useful configuration in the linker by default. Such 
> a configuration is by its very nature target specific. This doesn't mean it 
> can't be done in a cross-compiler friendly way, on the contrary. Over the 
> years NetBSD has been pushing its toolchain to be as similar for the native 
> build and a cross-target as reasonable possible. Modulo the build time 
> choices in the config.h sense, the only difference between the native and 
> cross tools is the built-in default of the former.


It might be naive but I don't think it's too naive. Most programs use ld via 
cc, and I don't think it is too unreasonable to not implement a host-specific 
logic for a very small percentage of programs that directly use ld. If we do, 
that logic would be the same or very similar to the one that we already have in 
cc. I think we should avoid that duplication of the host-specific config in the 
toolchain.

I see there are pros and cons to not have host-specific config in ld. That 
said, if other operating systems can live without host-specific config in lld, 
why can't NetBSD do? I really don't like to be in a situation where only one 
operating system have a host-specific config while others don't.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56215/new/

https://reviews.llvm.org/D56215



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

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D56113#1345529 , @jdenny wrote:

> In D56113#1345238 , @ABataev wrote:
>
> > >>> By the way, is there any value to keeping the predetermined shared for 
> > >>> const if -openmp-version=3.1 or earlier?
> > >> 
> > >> Yes, you can check for the value of `LangOpts.OpenMP`. For OpenMP 3.1 it 
> > >> will have the value `31`.
> > > 
> > > How far back should we take this?  I'm inclined to check for `30` and 
> > > `31` only and assume anything else is newer, but let me know if we need 
> > > to check for earlier versions.
> >
> > I think `<= 31` is good. Clang always supported only OpenMP 3.1 and higher.
>
>
> I'm planning to let this affect the behavior of `default(none)` 
> (predetermined shared means no explicit attribute is needed).
>
> I don't plan to let it affect which version of the diagnostics are produced.  
> I think the newer diagnostics are clearer even though they are not expressed 
> precisely in terms of 3.1 semantics.


I missed something: 3.1 restricts const and not mutable variables for 
lastprivate and private too.  However, clang didn't previously implement those 
restrictions probably because the check for conflicts with predetermined shared 
was sufficient to catch those cases.

So, for 3.1, it should be correct for clang to report either diagnostic, but 
again I think the new diagnostic is clearer.  For 4.0 and later, only the new 
diagnostic makes sense.

I had already updated all the tests for the new diagnostic, so it's easiest to 
keep the new diagnostic for all versions.  Moreover, it means less cases to 
check in the test suite.

Of course, I can adjust if this doesn't make sense to you.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56113/new/

https://reviews.llvm.org/D56113



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

  On 03.01.2019 23:15, Joerg Sonnenberger wrote:
  > On Thu, Jan 03, 2019 at 09:38:49PM +, Kamil Rytarowski via Phabricator 
via cfe-commits wrote:
  >> I think that this place is not the right place to bash GNU ld for 
performance issues.
  > 
  > I didn't.
  > 
  >> I will refer just to slides and paper from Ian Lance Taylor to get 
overview why it is unacceptably slow:
  >>
  >> https://www.airs.com/ian/gold-slides.pdf
  >> https://ai.google/research/pubs/pub34417.pdf
  > 
  > We all know that gold and lld are faster. It's a huge step from
  > "somewhat faster" to "unacceptably slow". But that's again a completely
  > separate topic.

I used to waste like an hour daily average, any test-build of a local change 
and break of 5-10 min is `unacceptably slow`.

  >> I will add that (unless nothing changed recently) ignoring lack of
  >> features (like thinlto) GNU ld cannot produce >=4GB executables and
  >> this makes it even more unusable.
  > 
  > That sounds seriously like a troll. I already mentioned DWARF fission
  > for the one reasonable case for > 100MB binaries and that's in short
  > "don't touch most of the data"...
  > 
  
  I've edited the entry above that it's already more than 4GB of unoptimized 
webkit build with debuginfo, and gnu ld is truncating files on 4GB boundary.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56215/new/

https://reviews.llvm.org/D56215



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

Re: [PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Joerg Sonnenberger via cfe-commits]

On Thu, Jan 03, 2019 at 09:38:49PM +, Kamil Rytarowski via Phabricator via 
cfe-commits wrote:
> I think that this place is not the right place to bash GNU ld for performance 
> issues.

I didn't.

> I will refer just to slides and paper from Ian Lance Taylor to get overview 
> why it is unacceptably slow:
> 
> https://www.airs.com/ian/gold-slides.pdf
> https://ai.google/research/pubs/pub34417.pdf

We all know that gold and lld are faster. It's a huge step from
"somewhat faster" to "unacceptably slow". But that's again a completely
separate topic.

> I will add that (unless nothing changed recently) ignoring lack of
> features (like thinlto) GNU ld cannot produce >=4GB executables and
> this makes it even more unusable.

That sounds seriously like a troll. I already mentioned DWARF fission
for the one reasonable case for > 100MB binaries and that's in short
"don't touch most of the data"...

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

[PATCH] D55865: [ObjC] Add a new attribute to opt-out of implicit callee retain/release in ARC

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington marked an inline comment as done.
erik.pilkington added inline comments.



Comment at: clang/docs/AutomaticReferenceCounting.rst:1779
+externally-retained, unless the variable was explicitly qualified with
+``__strong``. For instance, ``first_param`` is externally-retained below, but
+not ``second_param``:

rjmccall wrote:
> I hadn't noticed the parameter-index feature.  Is that a good idea?  Surely 
> if this level of control is needed, it would be better to add the attribute 
> to each parameter individually — or still better, just use explicit ownership 
> qualifiers to mark the exceptions.  And it creates all sorts of ugly 
> questions about the indexing base and how it accounts for implicit parameters.
Oh, good point. I never even considered just opting out with `__strong` instead 
of indices. That makes a lot more sense and has better ergonomics. I'll remove 
that feature and update the docs...

FWIW: I explained why I went with the attribute on the FunctionDecl instead of 
the ParmVarDecl a few comments up if you missed it.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55865/new/

https://reviews.llvm.org/D55865



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

[PATCH] D55865: [ObjC] Add a new attribute to opt-out of implicit callee retain/release in ARC

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/docs/AutomaticReferenceCounting.rst:1749
+instance, capturing an externally-retained variable in a block will retain the
+value when capturing it, and release it when the block is destroyed.
+

Nit-picks: `it's` should be `its`, `An` shouldn't be capitalized, and the comma 
before "and release it" is a splice.



Comment at: clang/docs/AutomaticReferenceCounting.rst:1779
+externally-retained, unless the variable was explicitly qualified with
+``__strong``. For instance, ``first_param`` is externally-retained below, but
+not ``second_param``:

I hadn't noticed the parameter-index feature.  Is that a good idea?  Surely if 
this level of control is needed, it would be better to add the attribute to 
each parameter individually — or still better, just use explicit ownership 
qualifiers to mark the exceptions.  And it creates all sorts of ugly questions 
about the indexing base and how it accounts for implicit parameters.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55865/new/

https://reviews.llvm.org/D55865



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

  On Thu, Jan 03, 2019 at 08:31:53PM +, Kamil Rytarowski via Phabricator 
via cfe-commits wrote:
  > krytarowski added a comment.
  >
  > On 03.01.2019 21:19, Joerg Sonnenberger wrote:
  >
  >> On Thu, Jan 03, 2019 at 06:34:22PM +, Kamil Rytarowski via Phabricator 
via cfe-commits wrote:
  >>
  >>> krytarowski added a comment.
  >>>
  >>> Actually I find it frustrating and unfair as GNU ld doesn't have builtin
  >>>  knowledge either.. it's passed with gross 'super hack' comments from 
build scripts... but we are forced to push it to lld in order to move on.
  >>
  >> I'm puzzled. Seriously, when was the last time you actually checked how
  >>  much customization contains on a per-OS base in GNU ld? Yes, I'm
  >>  including the various build scripts because GNU ld is generally build by
  >>  a mix of hard-coded logic in the tool itself and various adjustments in
  >>  the linker scripts it is shipped with. But they are all a builtin part
  >>  of GNU ld as far as the end user is concerned. It is pretty much
  >>  irrelevant from a point of functionality where that logic is, but
  >>  skipping is a serious usability issue.
  >>
  >> Joerg
  >
  > I'm referring to code '/usr/src/external/gpl3/binutils/usr.bin/ld/Makefile':
  
  I think that's a left over from old times before the emulparams/* logic
  was done properly. But that's more a case of GNU ld's build system being
  gross than anything else.
  
  Joerg



  On Thu, Jan 03, 2019 at 06:58:41PM +, Kamil Rytarowski via Phabricator 
wrote:
  > But the result is that we don't have GNU gold either and are stuck with
  > 40min linking times of LLVM. It's destructive to productivity and
  > damages any LLVM related development.
  
  The reason noone cared much about GNU gold is that it supports only a
  limited set of platforms and forces a lot of modern GNU "innovations"
  without any chance of fixing them. To a degree, both concerns apply to
  lld as well, but reasonable well integrated LTO support with Clang
  provides at least something in return. I have no idea about your link
  times, the only situation where linking is taking a really significant
  chunk of time is for full debug builds and the general solution for that
  is DWARF fission.
  
  Joerg

Actually I do cared to port gold but it didn't work and I wasn't able to spare 
more time. My work and a fixup for one feature is in pkgsrc-wip.

I think that this place is not the right place to bash GNU ld for performance 
issues.

I will refer just to slides and paper from Ian Lance Taylor to get overview why 
it is unacceptably slow:

https://www.airs.com/ian/gold-slides.pdf
https://ai.google/research/pubs/pub34417.pdf

I will add that (unless nothing changed recently) ignoring lack of features 
(like thinlto) GNU ld cannot produce >=4GB executables and this makes it even 
more unusable.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56215/new/

https://reviews.llvm.org/D56215



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

[PATCH] D55865: [ObjC] Add a new attribute to opt-out of implicit callee retain/release in ARC

[Erik Pilkington via Phabricator via cfe-commits]

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

Address John's comments. Thanks!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55865/new/

https://reviews.llvm.org/D55865

Files:
  clang/docs/AutomaticReferenceCounting.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclObjC.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/CodeGenObjC/externally-retained.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/externally-retained-no-arc.m
  clang/test/SemaObjC/externally-retained.m

Index: clang/test/SemaObjC/externally-retained.m
===
--- /dev/null
+++ clang/test/SemaObjC/externally-retained.m
@@ -0,0 +1,127 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.13.0 -fobjc-runtime=macosx-10.13.0 -fblocks -fobjc-arc %s -verify
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.13.0 -fobjc-runtime=macosx-10.13.0 -fblocks -fobjc-arc -xobjective-c++ %s -verify
+
+#define EXT_RET __attribute__((objc_externally_retained))
+#define CALL_RET(...) __attribute__((objc_externally_retained(__VA_ARGS__)))
+
+@interface ObjCTy
+@end
+
+void test1() {
+  EXT_RET int a; // expected-warning{{'objc_externally_retained' can only be applied to}}
+  EXT_RET __weak ObjCTy *b; // expected-warning{{'objc_externally_retained' can only be applied to}}
+  EXT_RET __weak int (^c)(); // expected-warning{{'objc_externally_retained' can only be applied to}}
+
+  EXT_RET int (^d)() = ^{return 0;};
+  EXT_RET ObjCTy *e = 0;
+  EXT_RET __strong ObjCTy *f = 0;
+
+  e = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+  f = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+  d = ^{ return 0; }; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+}
+
+void test2(ObjCTy *a);
+
+void test2(ObjCTy *a) CALL_RET(1) {
+  a = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+}
+
+EXT_RET ObjCTy *test3; // expected-warning{{'objc_externally_retained' can only be applied to}}
+
+@interface X // expected-warning{{defined without specifying a base class}} expected-note{{add a super class}}
+-(void)m: (ObjCTy *) p;
+@end
+@implementation X
+-(void)m: (ObjCTy *) p CALL_RET(1) {
+  p = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+}
+@end
+
+void test4() {
+  __attribute__((objc_externally_retained(0))) ObjCTy *a; // expected-error{{'objc_externally_retained' attribute takes no arguments}}
+}
+
+void test5(ObjCTy *first, __strong ObjCTy *second) CALL_RET() {
+  first = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+  second = 0; // fine
+}
+
+void test6(ObjCTy *first,
+   ObjCTy *second,
+   __strong ObjCTy *third) CALL_RET(1, 3) {
+  first = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+  second = 0;
+  third = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+}
+
+__attribute__((objc_root_class)) @interface Y @end
+
+@implementation Y
+- (void)test7:(ObjCTy *)first
+   withSecond:(ObjCTy *)second
+withThird:(ObjCTy *)third
+  CALL_RET(1, 2) {
+  first = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+  second = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+  third = 0;
+}
+@end
+
+void (^blk)() = ^CALL_RET(1) {}; // expected-error{{'objc_externally_retained' attribute parameter 1 is out of bounds}}
+void (^blk2)(ObjCTy *, ObjCTy *, ObjCTy *) = ^(ObjCTy *first, ObjCTy *second,
+   ObjCTy *third) CALL_RET(2, 3) {
+  first = 0;
+  second = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+  third = 0;  // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+};
+
+CALL_RET(1) void test7(__strong ObjCTy *first, ObjCTy *second) {
+  first = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+  second = 0;
+}
+
+void test8(EXT_RET ObjCTy *x) {} // expected-warning{{'objc_externally_retained' attribute only applies to variables}}
+
+#pragma clang attribute ext_ret.push(__attribute__((objc_externally_retained)), apply_to=any(function, block,

[PATCH] D56188: Adopt SwiftABIInfo for WebAssembly.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I don't mind you committing it like this.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56188/new/

https://reviews.llvm.org/D56188



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

I think that ifdefining the linker options with `__NetBSD__` is no-go.

In D56215#1345563 , @mgorny wrote:

> We've discussed this a bit and given other changes we need to do, and I see 
> pretty much three options here:
>
> 1. We hardcode stuff under `defined(__NetBSD__)` which kinda solves the 
> problem, except lld won't be very cross-friendly.


I think that ifdefining the linker options with `__NetBSD__` is no-go. We 
expect to get lld to link NetBSD programs from any host, or from NetBSD to any 
other.

> 2. We try to do conditionals based on triple but this works only when we 
> customize the install to include it in executable name. We probably would 
> still need to default based on `defined(__NetBSD__)` when triple isn't 
> available via process name.

I think it won't work for us.

> 3. We create `*nbsd*` emulations for all arches (e.g. amd64/x86 don't have 
> such emulations right now), and use that to switch logic. This would be 
> closer to what FreeBSD does, I think. However, if we do this, then I suppose 
> we should also add similar aliases to GNU ld.

Probably too gross hack just for the gain redesigned model of a linker 
functional.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56215/new/

https://reviews.llvm.org/D56215



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

[PATCH] D54450: Get the correct range of tokens for preprocessor conditions

[Roman Lebedev via Phabricator via cfe-commits]

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

To me this looks like a reasonably straight-forward refactoring.
I'm guessing the initial code had good test coverage, and none of those tests 
break; and the new code appears to have reasonable test coverage.
(Mind you, i'm not a code owner of that code.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54450/new/

https://reviews.llvm.org/D54450



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Alexey Lapshin via Phabricator via cfe-commits]

alexey.lapshin added a comment.

Thank you!  Aaron, Could you integrate this patch, please? I do not have commit 
access yet.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55710/new/

https://reviews.llvm.org/D55710



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

[PATCH] D54450: Get the correct range of tokens for preprocessor conditions

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added a comment.

Could you please review this one? It would be especially helpful, due to the 
depending other review. Thanks!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54450/new/

https://reviews.llvm.org/D54450



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

[PATCH] D56188: Adopt SwiftABIInfo for WebAssembly.

[Daniel Dunbar via Phabricator via cfe-commits]

ddunbar added a comment.

In D56188#1343667 , @rjmccall wrote:

> ...although it might be reasonable to extract the method implementations on 
> `DefaultABIInfo` as helper functions so that the code can be reused without 
> requiring a particular inheritance hierarchy.


I agree that would be cleaner, but don't work on Clang much anymore so was 
doing the minimal thing. Do you want me to go ahead and do this?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56188/new/

https://reviews.llvm.org/D56188



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

[PATCH] D56241: expose a control flag for interger to pointer ext warning

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: test/Sema/ext-typecheck-comparison-of-pointer-integer.c:1
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only  -verify 
-Wpointer-integer-compare %s 
+

I guess you want a run-line with no flag, this runline, and a run-line with 
`-Wno-pointer-integer-compare`


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56241/new/

https://reviews.llvm.org/D56241



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

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D56113#1345588 , @jdenny wrote:

> In D56113#1345578 , @ABataev wrote:
>
> > >>> By the way, LangOpts.OpenMP currently defaults to 0.  Should it?
> > >> 
> > >> Yes, it means it is disabled by default.
> > > 
> > > Ah, it becomes 1 when we have `-fopenmp` but not `-fopenmp-version`.  But 
> > > still `LangOpts.OpenMP <= 31` is true by default, so the new behavior 
> > > would be off by default.
> >
> > No, by default it gets value 31.
>
>
> In `DSAStackTy::getTopDSA` in SemaOpenMP.cpp, I added this line:
>
>   llvm::errs() << SemaRef.LangOpts.OpenMP << '\n';
>
>
> `clang -fopenmp test.c` prints 1.
>
> I'm at r350238, which I pulled yesterday.
>
> For my purposes, 1 and 31 are equivalent, so I can move on.


Ah, yes, forgot already. You can consider 1 as the same value as 31 by default.

> 
> 
>> If -fopenmp-targets= is used, then the default version is 45.
> 
> That I can confirm.




CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56113/new/

https://reviews.llvm.org/D56113



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

LLVM buildmaster will be restarted tonight

[Galina Kistanova via cfe-commits]

 Hello everyone,

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

Thanks

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

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D56113#1345578 , @ABataev wrote:

> >>> By the way, LangOpts.OpenMP currently defaults to 0.  Should it?
> >> 
> >> Yes, it means it is disabled by default.
> > 
> > Ah, it becomes 1 when we have `-fopenmp` but not `-fopenmp-version`.  But 
> > still `LangOpts.OpenMP <= 31` is true by default, so the new behavior would 
> > be off by default.
>
> No, by default it gets value 31.


In `DSAStackTy::getTopDSA` in SemaOpenMP.cpp, I added this line:

  llvm::errs() << SemaRef.LangOpts.OpenMP << '\n';

`clang -fopenmp test.c` prints 1.

I'm at r350238, which I pulled yesterday.

For my purposes, 1 and 31 are equivalent, so I can move on.

> If -fopenmp-targets= is used, then the default version is 45.

That I can confirm.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56113/new/

https://reviews.llvm.org/D56113



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

Re: [PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Joerg Sonnenberger via cfe-commits]

On Thu, Jan 03, 2019 at 08:31:53PM +, Kamil Rytarowski via Phabricator via 
cfe-commits wrote:
> krytarowski added a comment.
> 
> On 03.01.2019 21:19, Joerg Sonnenberger wrote:
> 
> > On Thu, Jan 03, 2019 at 06:34:22PM +, Kamil Rytarowski via Phabricator 
> > via cfe-commits wrote:
> > 
> >> krytarowski added a comment.
> >> 
> >> Actually I find it frustrating and unfair as GNU ld doesn't have builtin
> >>  knowledge either.. it's passed with gross 'super hack' comments from 
> >> build scripts... but we are forced to push it to lld in order to move on.
> > 
> > I'm puzzled. Seriously, when was the last time you actually checked how
> >  much customization contains on a per-OS base in GNU ld? Yes, I'm
> >  including the various build scripts because GNU ld is generally build by
> >  a mix of hard-coded logic in the tool itself and various adjustments in
> >  the linker scripts it is shipped with. But they are all a builtin part
> >  of GNU ld as far as the end user is concerned. It is pretty much
> >  irrelevant from a point of functionality where that logic is, but
> >  skipping is a serious usability issue.
> > 
> > Joerg
> 
> I'm referring to code '/usr/src/external/gpl3/binutils/usr.bin/ld/Makefile':

I think that's a left over from old times before the emulparams/* logic
was done properly. But that's more a case of GNU ld's build system being
gross than anything else.

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

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Joerg Sonnenberger via cfe-commits]

On Thu, Jan 03, 2019 at 05:01:00PM +, Ximin Luo via cfe-commits wrote:
> My patch that Roman quoted in the email below was an additional
> mechanism to set -fmacro-prefix-map and -fdebug-prefix-map via an
> environment variable. This was not accepted into GCC because they are
> (in summary) allergic against using environment variables to set anything.

The main problem here is that GCC insists on leaking the full command
line into the output and that makes it difficult to impossible to
actually provide sane mappings without using the environment.

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

[PATCH] D56288: [ELF] Do not enable 'new dtags' on NetBSD by default

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

We want to keep it disabled by default on NetBSD.. but it would be to keep 
dynamic detection of target NetBSD rather than hardcoded ifdef.

GNU ld sets it by default to false, following it would be the easiest option.


Repository:
  rLLD LLVM Linker

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56288/new/

https://reviews.llvm.org/D56288



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

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D56113#1345564 , @jdenny wrote:

> In D56113#1345535 , @ABataev wrote:
>
> > In D56113#1345529 , @jdenny wrote:
> >
> > > In D56113#1345238 , @ABataev 
> > > wrote:
> > >
> > > > >>> By the way, is there any value to keeping the predetermined shared 
> > > > >>> for const if -openmp-version=3.1 or earlier?
> > > > >> 
> > > > >> Yes, you can check for the value of `LangOpts.OpenMP`. For OpenMP 
> > > > >> 3.1 it will have the value `31`.
> > > > > 
> > > > > How far back should we take this?  I'm inclined to check for `30` and 
> > > > > `31` only and assume anything else is newer, but let me know if we 
> > > > > need to check for earlier versions.
> > > >
> > > > I think `<= 31` is good. Clang always supported only OpenMP 3.1 and 
> > > > higher.
> > >
> > >
> > > I'm planning to let this affect the behavior of `default(none)` 
> > > (predetermined shared means no explicit attribute is needed).
> > >
> > > I don't plan to let it affect which version of the diagnostics are 
> > > produced.  I think the newer diagnostics are clearer even though they are 
> > > not expressed precisely in terms of 3.1 semantics.  Moreover, there are 
> > > less cases to test this way.  Let me know if you think this is wrong.  If 
> > > you want to review the updated patch first, I'll be posting it soon.
> >
> >
> > It would be good if could keep the original implementation for 3.1.
>
>
> Should I parameterize all tests affected by this change to test both sets of 
> diagnostics?  Or should we pick one version and just add a few additional 
> tests for the other?


It is enough just to add the new test for the new version, no need to 
copy/update all the tests.

> (I'm sorry to ask so many questions, but again there are many tests to 
> update.)
> 
>>> By the way, LangOpts.OpenMP currently defaults to 0.  Should it?
>> 
>> Yes, it means it is disabled by default.
> 
> Ah, it becomes 1 when we have `-fopenmp` but not `-fopenmp-version`.  But 
> still `LangOpts.OpenMP <= 31` is true by default, so the new behavior would 
> be off by default.

No, by default it gets value 31. If -fopenmp-targets= is used, then the default 
version is 45. And yes, you can use -fopenmp-version to set the required 
version of OpenMP.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56113/new/

https://reviews.llvm.org/D56113



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

[PATCH] D56241: expose a control flag for interger to pointer ext warning

[abdoul-kader keita via Phabricator via cfe-commits]

Kader updated this revision to Diff 180125.
Kader added a comment.

added missing test file


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56241/new/

https://reviews.llvm.org/D56241

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  test/Misc/warning-flags.c
  test/Sema/ext-typecheck-comparison-of-pointer-integer.c


Index: test/Sema/ext-typecheck-comparison-of-pointer-integer.c
===
--- /dev/null
+++ test/Sema/ext-typecheck-comparison-of-pointer-integer.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only  -verify 
-Wpointer-integer-compare %s 
+
+int test_ext_typecheck_comparison_of_pointer_integer(int integer, int * 
pointer) {
+   return integer != pointer;  // expected-warning {{comparison between 
pointer and integer ('int' and 'int *')}}
+}
Index: test/Misc/warning-flags.c
===
--- test/Misc/warning-flags.c
+++ test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (75):
+CHECK: Warnings without flags (74):
 CHECK-NEXT:   ext_excess_initializers
 CHECK-NEXT:   ext_excess_initializers_in_char_array_initializer
 CHECK-NEXT:   ext_expected_semi_decl_list
@@ -29,7 +29,6 @@
 CHECK-NEXT:   ext_new_paren_array_nonconst
 CHECK-NEXT:   ext_plain_complex
 CHECK-NEXT:   ext_template_arg_extra_parens
-CHECK-NEXT:   ext_typecheck_comparison_of_pointer_integer
 CHECK-NEXT:   ext_typecheck_cond_incompatible_operands
 CHECK-NEXT:   ext_typecheck_ordered_comparison_of_pointer_integer
 CHECK-NEXT:   ext_using_undefined_std
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5875,7 +5875,8 @@
 def err_typecheck_comparison_of_fptr_to_void : Error<
   "equality comparison between function pointer and void pointer (%0 and %1)">;
 def ext_typecheck_comparison_of_pointer_integer : ExtWarn<
-  "comparison between pointer and integer (%0 and %1)">;
+  "comparison between pointer and integer (%0 and %1)">,
+  InGroup>;
 def err_typecheck_comparison_of_pointer_integer : Error<
   "comparison between pointer and integer (%0 and %1)">;
 def ext_typecheck_comparison_of_distinct_pointers : ExtWarn<


Index: test/Sema/ext-typecheck-comparison-of-pointer-integer.c
===
--- /dev/null
+++ test/Sema/ext-typecheck-comparison-of-pointer-integer.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only  -verify -Wpointer-integer-compare %s 
+
+int test_ext_typecheck_comparison_of_pointer_integer(int integer, int * pointer) {
+	return integer != pointer;  // expected-warning {{comparison between pointer and integer ('int' and 'int *')}}
+}
Index: test/Misc/warning-flags.c
===
--- test/Misc/warning-flags.c
+++ test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (75):
+CHECK: Warnings without flags (74):
 CHECK-NEXT:   ext_excess_initializers
 CHECK-NEXT:   ext_excess_initializers_in_char_array_initializer
 CHECK-NEXT:   ext_expected_semi_decl_list
@@ -29,7 +29,6 @@
 CHECK-NEXT:   ext_new_paren_array_nonconst
 CHECK-NEXT:   ext_plain_complex
 CHECK-NEXT:   ext_template_arg_extra_parens
-CHECK-NEXT:   ext_typecheck_comparison_of_pointer_integer
 CHECK-NEXT:   ext_typecheck_cond_incompatible_operands
 CHECK-NEXT:   ext_typecheck_ordered_comparison_of_pointer_integer
 CHECK-NEXT:   ext_using_undefined_std
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5875,7 +5875,8 @@
 def err_typecheck_comparison_of_fptr_to_void : Error<
   "equality comparison between function pointer and void pointer (%0 and %1)">;
 def ext_typecheck_comparison_of_pointer_integer : ExtWarn<
-  "comparison between pointer and integer (%0 and %1)">;
+  "comparison between pointer and integer (%0 and %1)">,
+  InGroup>;
 def err_typecheck_comparison_of_pointer_integer : Error<
   "comparison between pointer and integer (%0 and %1)">;
 def ext_typecheck_comparison_of_distinct_pointers : ExtWarn<
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56241: expose a control flag for interger to pointer ext warning

[abdoul-kader keita via Phabricator via cfe-commits]

Kader added a comment.

@lebedev.ri  Thanks for the quick review ;

> Perhaps there should be a test for this flag specifically?

I am not really familiar with this part of the code base, but it seems that the 
warnings is already tested in both test/Sema/Compare.c and 
test/SemaCXX/compare.cpp. I have none the less added a new test file for c_lang.

> Also, should "pointer-integer-compare" be in some group?

I modeled my change on ext_typecheck_ordered_comparison_of_function_pointers 
which doesn't belong to any group. Not sure if that's the right thing to do 
though.

Also, this flag is only valid for C_lang and not for Cpp_lang. But right now 
the flag will be silently ignore when compiling cpp files. Is that an 
acceptable behavior ?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56241/new/

https://reviews.llvm.org/D56241



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

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D56113#1345535 , @ABataev wrote:

> In D56113#1345529 , @jdenny wrote:
>
> > In D56113#1345238 , @ABataev wrote:
> >
> > > >>> By the way, is there any value to keeping the predetermined shared 
> > > >>> for const if -openmp-version=3.1 or earlier?
> > > >> 
> > > >> Yes, you can check for the value of `LangOpts.OpenMP`. For OpenMP 3.1 
> > > >> it will have the value `31`.
> > > > 
> > > > How far back should we take this?  I'm inclined to check for `30` and 
> > > > `31` only and assume anything else is newer, but let me know if we need 
> > > > to check for earlier versions.
> > >
> > > I think `<= 31` is good. Clang always supported only OpenMP 3.1 and 
> > > higher.
> >
> >
> > I'm planning to let this affect the behavior of `default(none)` 
> > (predetermined shared means no explicit attribute is needed).
> >
> > I don't plan to let it affect which version of the diagnostics are 
> > produced.  I think the newer diagnostics are clearer even though they are 
> > not expressed precisely in terms of 3.1 semantics.  Moreover, there are 
> > less cases to test this way.  Let me know if you think this is wrong.  If 
> > you want to review the updated patch first, I'll be posting it soon.
>
>
> It would be good if could keep the original implementation for 3.1.


Should I parameterize all tests affected by this change to test both sets of 
diagnostics?  Or should we pick one version and just add a few additional tests 
for the other?

(I'm sorry to ask so many questions, but again there are many tests to update.)

>> By the way, LangOpts.OpenMP currently defaults to 0.  Should it?
> 
> Yes, it means it is disabled by default.

Ah, it becomes 1 when we have `-fopenmp` but not `-fopenmp-version`.  But still 
`LangOpts.OpenMP <= 31` is true by default, so the new behavior would be off by 
default.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56113/new/

https://reviews.llvm.org/D56113



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

We've discussed this a bit and given other changes we need to do, and I see 
pretty much three options here:

1. We hardcode stuff under `defined(__NetBSD__)` which kinda solves the 
problem, except lld won't be very cross-friendly.
2. We try to do conditionals based on triple but this works only when we 
customize the install to include it in executable name. We probably would still 
need to default based on `defined(__NetBSD__)` when triple isn't available via 
process name.
3. We create `*nbsd*` emulations for all arches (e.g. amd64/x86 don't have such 
emulations right now), and use that to switch logic. This would be closer to 
what FreeBSD does, I think. However, if we do this, then I suppose we should 
also add similar aliases to GNU ld.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56215/new/

https://reviews.llvm.org/D56215



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

[PATCH] D56241: expose a control flag for interger to pointer ext warning

[abdoul-kader keita via Phabricator via cfe-commits]

Kader updated this revision to Diff 180124.
Kader added a comment.

Added test case for C_lang


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56241/new/

https://reviews.llvm.org/D56241

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  test/Misc/warning-flags.c


Index: test/Misc/warning-flags.c
===
--- test/Misc/warning-flags.c
+++ test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (75):
+CHECK: Warnings without flags (74):
 CHECK-NEXT:   ext_excess_initializers
 CHECK-NEXT:   ext_excess_initializers_in_char_array_initializer
 CHECK-NEXT:   ext_expected_semi_decl_list
@@ -29,7 +29,6 @@
 CHECK-NEXT:   ext_new_paren_array_nonconst
 CHECK-NEXT:   ext_plain_complex
 CHECK-NEXT:   ext_template_arg_extra_parens
-CHECK-NEXT:   ext_typecheck_comparison_of_pointer_integer
 CHECK-NEXT:   ext_typecheck_cond_incompatible_operands
 CHECK-NEXT:   ext_typecheck_ordered_comparison_of_pointer_integer
 CHECK-NEXT:   ext_using_undefined_std
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5875,7 +5875,8 @@
 def err_typecheck_comparison_of_fptr_to_void : Error<
   "equality comparison between function pointer and void pointer (%0 and %1)">;
 def ext_typecheck_comparison_of_pointer_integer : ExtWarn<
-  "comparison between pointer and integer (%0 and %1)">;
+  "comparison between pointer and integer (%0 and %1)">,
+  InGroup>;
 def err_typecheck_comparison_of_pointer_integer : Error<
   "comparison between pointer and integer (%0 and %1)">;
 def ext_typecheck_comparison_of_distinct_pointers : ExtWarn<


Index: test/Misc/warning-flags.c
===
--- test/Misc/warning-flags.c
+++ test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (75):
+CHECK: Warnings without flags (74):
 CHECK-NEXT:   ext_excess_initializers
 CHECK-NEXT:   ext_excess_initializers_in_char_array_initializer
 CHECK-NEXT:   ext_expected_semi_decl_list
@@ -29,7 +29,6 @@
 CHECK-NEXT:   ext_new_paren_array_nonconst
 CHECK-NEXT:   ext_plain_complex
 CHECK-NEXT:   ext_template_arg_extra_parens
-CHECK-NEXT:   ext_typecheck_comparison_of_pointer_integer
 CHECK-NEXT:   ext_typecheck_cond_incompatible_operands
 CHECK-NEXT:   ext_typecheck_ordered_comparison_of_pointer_integer
 CHECK-NEXT:   ext_using_undefined_std
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5875,7 +5875,8 @@
 def err_typecheck_comparison_of_fptr_to_void : Error<
   "equality comparison between function pointer and void pointer (%0 and %1)">;
 def ext_typecheck_comparison_of_pointer_integer : ExtWarn<
-  "comparison between pointer and integer (%0 and %1)">;
+  "comparison between pointer and integer (%0 and %1)">,
+  InGroup>;
 def err_typecheck_comparison_of_pointer_integer : Error<
   "comparison between pointer and integer (%0 and %1)">;
 def ext_typecheck_comparison_of_distinct_pointers : ExtWarn<
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

On 03.01.2019 21:19, Joerg Sonnenberger wrote:

> On Thu, Jan 03, 2019 at 06:34:22PM +, Kamil Rytarowski via Phabricator 
> via cfe-commits wrote:
> 
>> krytarowski added a comment.
>> 
>> Actually I find it frustrating and unfair as GNU ld doesn't have builtin
>>  knowledge either.. it's passed with gross 'super hack' comments from build 
>> scripts... but we are forced to push it to lld in order to move on.
> 
> I'm puzzled. Seriously, when was the last time you actually checked how
>  much customization contains on a per-OS base in GNU ld? Yes, I'm
>  including the various build scripts because GNU ld is generally build by
>  a mix of hard-coded logic in the tool itself and various adjustments in
>  the linker scripts it is shipped with. But they are all a builtin part
>  of GNU ld as far as the end user is concerned. It is pretty much
>  irrelevant from a point of functionality where that logic is, but
>  skipping is a serious usability issue.
> 
> Joerg

I'm referring to code '/usr/src/external/gpl3/binutils/usr.bin/ld/Makefile':

  # XXX super hack
  . if (${BINUTILS_MACHINE_ARCH} == "x86_64" && \
("${f}" == "elf_i386" || "${f}" == "i386nbsd"))
  EMUL_LIB_PATH.${f}=/usr/lib/i386
  . elif (${BINUTILS_MACHINE_ARCH} == "sparc64" && \
("${f}" == "elf32_sparc" || "${f}" == "sparcnbsd"))
  EMUL_LIB_PATH.${f}=/usr/lib/sparc
  . elif !empty(BINUTILS_MACHINE_ARCH:Mmips64*)
  .  if "${f}" == "elf32ltsmip" || "${f}" == "elf32btsmip"
  EMUL_LIB_PATH.${f}:=/usr/lib/o32
  .  elif "${f}" == "elf64ltsmip" || "${f}" == "elf64btsmip"
  EMUL_LIB_PATH.${f}:=/usr/lib/64
  .  else
  EMUL_LIB_PATH.${f}=/usr/lib
  .  endif
  . else
  EMUL_LIB_PATH.${f}=/usr/lib
  . endif
  
  e${f}.c: ${DIST}/ld/genscripts.sh ${.CURDIR}/Makefile stringify.sed
  ${_MKTARGET_CREATE}
  unset MACHINE || true; \
  LIB_PATH=${EMUL_LIB_PATH.${f}} NATIVE=yes \
  ${HOST_SH} ${DIST}/ld/genscripts.sh ${DIST}/ld \
  ${LIBDIR} "/usr" "/usr/bin" \
  ${G_target_alias} ${G_target_alias} ${G_target_alias} \
  ${G_EMUL} ${LIBDIR} yes ${G_enable_initfini_array} \
  ${f} "${G_target_alias}"

So we are now trying to put the logic directly into lld.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56215/new/

https://reviews.llvm.org/D56215



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

Re: [PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Joerg Sonnenberger via cfe-commits]

On Thu, Jan 03, 2019 at 06:58:41PM +, Kamil Rytarowski via Phabricator 
wrote:
> But the result is that we don't have GNU gold either and are stuck with
> 40min linking times of LLVM. It's destructive to productivity and
> damages any LLVM related development.

The reason noone cared much about GNU gold is that it supports only a
limited set of platforms and forces a lot of modern GNU "innovations"
without any chance of fixing them. To a degree, both concerns apply to
lld as well, but reasonable well integrated LTO support with Clang
provides at least something in return. I have no idea about your link
times, the only situation where linking is taking a really significant
chunk of time is for full debug builds and the general solution for that
is DWARF fission.

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

Re: [PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Joerg Sonnenberger via cfe-commits]

On Thu, Jan 03, 2019 at 06:34:22PM +, Kamil Rytarowski via Phabricator via 
cfe-commits wrote:
> krytarowski added a comment.
> 
> Actually I find it frustrating and unfair as GNU ld doesn't have builtin
> knowledge either.. it's passed with gross 'super hack' comments from build 
> scripts... but we are forced to push it to lld in order to move on.

I'm puzzled. Seriously, when was the last time you actually checked how
much customization contains on a per-OS base in GNU ld? Yes, I'm
including the various build scripts because GNU ld is generally build by
a mix of hard-coded logic in the tool itself and various adjustments in
the linker scripts it is shipped with. But they are all a builtin part
of GNU ld as far as the end user is concerned. It is pretty much
irrelevant from a point of functionality where that logic is, but
skipping is a serious usability issue.

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

[PATCH] D56288: [ELF] Do not enable 'new dtags' on NetBSD by default

[Michał Górny via Phabricator via cfe-commits]

mgorny created this revision.
mgorny added reviewers: ruiu, krytarowski, joerg.
Herald added subscribers: llvm-commits, arichardson, emaste.
Herald added a reviewer: espindola.

NetBSD strongly prefers using 'old' dtag (RPATH) over Linux-specific
RUNPATH, and did not support the latter until very recently.  Therefore,
defaulting to RUNPATH causes lld to produce non-working executables
by default.  Switch the default appropriately when building on NetBSD.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D56288

Files:
  ELF/Driver.cpp


Index: ELF/Driver.cpp
===
--- ELF/Driver.cpp
+++ ELF/Driver.cpp
@@ -766,6 +766,14 @@
 
 // Initializes Config members by the command line options.
 void LinkerDriver::readConfigs(opt::InputArgList &Args) {
+#if defined(__NetBSD__)
+// NetBSD prefers RPATH, and does not allow RUNPATH until very recently
+#  define DEFAULT_NEW_DTAGS false
+#else
+#  define DEFAULT_NEW_DTAGS true
+#endif
+
+
   errorHandler().Verbose = Args.hasArg(OPT_verbose);
   errorHandler().FatalWarnings =
   Args.hasFlag(OPT_fatal_warnings, OPT_no_fatal_warnings, false);
@@ -797,7 +805,8 @@
   Config->CallGraphProfileSort = Args.hasFlag(
   OPT_call_graph_profile_sort, OPT_no_call_graph_profile_sort, true);
   Config->EnableNewDtags =
-  Args.hasFlag(OPT_enable_new_dtags, OPT_disable_new_dtags, true);
+  Args.hasFlag(OPT_enable_new_dtags, OPT_disable_new_dtags,
+   DEFAULT_NEW_DTAGS);
   Config->Entry = Args.getLastArgValue(OPT_entry);
   Config->ExecuteOnly =
   Args.hasFlag(OPT_execute_only, OPT_no_execute_only, false);


Index: ELF/Driver.cpp
===
--- ELF/Driver.cpp
+++ ELF/Driver.cpp
@@ -766,6 +766,14 @@
 
 // Initializes Config members by the command line options.
 void LinkerDriver::readConfigs(opt::InputArgList &Args) {
+#if defined(__NetBSD__)
+// NetBSD prefers RPATH, and does not allow RUNPATH until very recently
+#  define DEFAULT_NEW_DTAGS false
+#else
+#  define DEFAULT_NEW_DTAGS true
+#endif
+
+
   errorHandler().Verbose = Args.hasArg(OPT_verbose);
   errorHandler().FatalWarnings =
   Args.hasFlag(OPT_fatal_warnings, OPT_no_fatal_warnings, false);
@@ -797,7 +805,8 @@
   Config->CallGraphProfileSort = Args.hasFlag(
   OPT_call_graph_profile_sort, OPT_no_call_graph_profile_sort, true);
   Config->EnableNewDtags =
-  Args.hasFlag(OPT_enable_new_dtags, OPT_disable_new_dtags, true);
+  Args.hasFlag(OPT_enable_new_dtags, OPT_disable_new_dtags,
+   DEFAULT_NEW_DTAGS);
   Config->Entry = Args.getLastArgValue(OPT_entry);
   Config->ExecuteOnly =
   Args.hasFlag(OPT_execute_only, OPT_no_execute_only, false);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[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!




Comment at: test/Sema/pragma-pipeline.cpp:3
+
+#pragma clang loop pipeline(disable) /* expected-error {{expected 
unqualified-id}} */
+int main() {

alexey.lapshin wrote:
> aaron.ballman wrote:
> > I think this error is pretty unfortunate -- it doesn't really help the user 
> > to understand what's wrong with their code. Can it be improved?
> Agreed, this does not look very informative. It surely can be improved. 
> Though it looks like not related to my fix. My fix is to add two additional 
> loop hints. That kind of diagnostic related to all loop hints "clang loop". 
> I.e. all pragmas "clang loop" can be checked for various cases and for better 
> diagnostic. It looks like separate task. But OK, I will check that case.
That's a fair point -- I'm fine with doing that work in a separate patch. I 
don't think we need to hold this patch up further for it, anyway.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55710/new/

https://reviews.llvm.org/D55710



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

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D56113#1345529 , @jdenny wrote:

> In D56113#1345238 , @ABataev wrote:
>
> > >>> By the way, is there any value to keeping the predetermined shared for 
> > >>> const if -openmp-version=3.1 or earlier?
> > >> 
> > >> Yes, you can check for the value of `LangOpts.OpenMP`. For OpenMP 3.1 it 
> > >> will have the value `31`.
> > > 
> > > How far back should we take this?  I'm inclined to check for `30` and 
> > > `31` only and assume anything else is newer, but let me know if we need 
> > > to check for earlier versions.
> >
> > I think `<= 31` is good. Clang always supported only OpenMP 3.1 and higher.
>
>
> I'm planning to let this affect the behavior of `default(none)` 
> (predetermined shared means no explicit attribute is needed).
>
> I don't plan to let it affect which version of the diagnostics are produced.  
> I think the newer diagnostics are clearer even though they are not expressed 
> precisely in terms of 3.1 semantics.  Moreover, there are less cases to test 
> this way.  Let me know if you think this is wrong.  If you want to review the 
> updated patch first, I'll be posting it soon.


It would be good if could keep the original implementation for 3.1.

> By the way, LangOpts.OpenMP currently defaults to 0.  Should it?

Yes, it means it is disabled by default.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56113/new/

https://reviews.llvm.org/D56113



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Michał Górny via Phabricator via cfe-commits]

mgorny updated this revision to Diff 180121.
mgorny edited the summary of this revision.
mgorny added a comment.

Removed non-x86.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56215/new/

https://reviews.llvm.org/D56215

Files:
  ELF/Driver.cpp
  ELF/Driver.h


Index: ELF/Driver.h
===
--- ELF/Driver.h
+++ ELF/Driver.h
@@ -32,6 +32,7 @@
 
 private:
   void readConfigs(llvm::opt::InputArgList &Args);
+  void appendDefaultSearchPaths();
   void createFiles(llvm::opt::InputArgList &Args);
   void inferMachineType();
   template  void link(llvm::opt::InputArgList &Args);
Index: ELF/Driver.cpp
===
--- ELF/Driver.cpp
+++ ELF/Driver.cpp
@@ -365,6 +365,23 @@
   error("unknown -z value: " + StringRef(Arg->getValue()));
 }
 
+void LinkerDriver::appendDefaultSearchPaths() {
+#if defined(__NetBSD__)
+  // NetBSD driver relies on the linker knowing the default search paths.
+  // Please keep this in sync with clang/lib/Driver/ToolChains/NetBSD.cpp
+  // (NetBSD::NetBSD constructor)
+  switch (Config->EMachine) {
+case EM_386:
+  Config->SearchPaths.push_back("/usr/lib/i386");
+  break;
+case EM_X86_64:
+  break;
+// TODO: support non-x86 architectures
+  }
+  Config->SearchPaths.push_back("/usr/lib");
+#endif
+}
+
 void LinkerDriver::main(ArrayRef ArgsArr) {
   ELFOptTable Parser;
   opt::InputArgList Args = Parser.parse(ArgsArr.slice(1));
@@ -412,6 +429,7 @@
 
   readConfigs(Args);
   checkZOptions(Args);
+  appendDefaultSearchPaths();
 
   // The behavior of -v or --version is a bit strange, but this is
   // needed for compatibility with GNU linkers.


Index: ELF/Driver.h
===
--- ELF/Driver.h
+++ ELF/Driver.h
@@ -32,6 +32,7 @@
 
 private:
   void readConfigs(llvm::opt::InputArgList &Args);
+  void appendDefaultSearchPaths();
   void createFiles(llvm::opt::InputArgList &Args);
   void inferMachineType();
   template  void link(llvm::opt::InputArgList &Args);
Index: ELF/Driver.cpp
===
--- ELF/Driver.cpp
+++ ELF/Driver.cpp
@@ -365,6 +365,23 @@
   error("unknown -z value: " + StringRef(Arg->getValue()));
 }
 
+void LinkerDriver::appendDefaultSearchPaths() {
+#if defined(__NetBSD__)
+  // NetBSD driver relies on the linker knowing the default search paths.
+  // Please keep this in sync with clang/lib/Driver/ToolChains/NetBSD.cpp
+  // (NetBSD::NetBSD constructor)
+  switch (Config->EMachine) {
+case EM_386:
+  Config->SearchPaths.push_back("/usr/lib/i386");
+  break;
+case EM_X86_64:
+  break;
+// TODO: support non-x86 architectures
+  }
+  Config->SearchPaths.push_back("/usr/lib");
+#endif
+}
+
 void LinkerDriver::main(ArrayRef ArgsArr) {
   ELFOptTable Parser;
   opt::InputArgList Args = Parser.parse(ArgsArr.slice(1));
@@ -412,6 +429,7 @@
 
   readConfigs(Args);
   checkZOptions(Args);
+  appendDefaultSearchPaths();
 
   // The behavior of -v or --version is a bit strange, but this is
   // needed for compatibility with GNU linkers.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D56113#1345238 , @ABataev wrote:

> >>> By the way, is there any value to keeping the predetermined shared for 
> >>> const if -openmp-version=3.1 or earlier?
> >> 
> >> Yes, you can check for the value of `LangOpts.OpenMP`. For OpenMP 3.1 it 
> >> will have the value `31`.
> > 
> > How far back should we take this?  I'm inclined to check for `30` and `31` 
> > only and assume anything else is newer, but let me know if we need to check 
> > for earlier versions.
>
> I think `<= 31` is good. Clang always supported only OpenMP 3.1 and higher.


I'm planning to let this affect the behavior of `default(none)` (predetermined 
shared means no explicit attribute is needed).

I don't plan to let it affect which version of the diagnostics are produced.  I 
think the newer diagnostics are clearer even though they are not expressed 
precisely in terms of 3.1 semantics.  Moreover, there are less cases to test 
this way.  Let me know if you think this is wrong.  If you want to review the 
updated patch first, I'll be posting it soon.

By the way, LangOpts.OpenMP currently defaults to 0.  Should it?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56113/new/

https://reviews.llvm.org/D56113



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Michał Górny via Phabricator via cfe-commits]

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



Comment at: ELF/Driver.cpp:369
+void LinkerDriver::appendDefaultSearchPaths() {
+#if defined(__NetBSD__)
+  // NetBSD driver relies on the linker knowing the default search paths.

krytarowski wrote:
> is it possible to use some Triple NetBSD target here?
Technically, you can get triple from process name but that obviously works only 
when lld is run as `$CHOST-lld` and not plain `lld`, which is not really the 
case right now.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56215/new/

https://reviews.llvm.org/D56215



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added inline comments.



Comment at: ELF/Driver.cpp:369
+void LinkerDriver::appendDefaultSearchPaths() {
+#if defined(__NetBSD__)
+  // NetBSD driver relies on the linker knowing the default search paths.

is it possible to use some Triple NetBSD target here?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56215/new/

https://reviews.llvm.org/D56215



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added inline comments.



Comment at: ELF/Driver.cpp:377
+  break;
+case EM_MIPS:
+  if (Config->EKind == ELF64LEKind || Config->EKind == ELF64BEKind)

Please drop MIPS/PPC for now.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56215/new/

https://reviews.llvm.org/D56215



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Alexey Lapshin via Phabricator via cfe-commits]

alexey.lapshin marked an inline comment as done.
alexey.lapshin added inline comments.



Comment at: test/Sema/pragma-pipeline.cpp:3
+
+#pragma clang loop pipeline(disable) /* expected-error {{expected 
unqualified-id}} */
+int main() {

aaron.ballman wrote:
> I think this error is pretty unfortunate -- it doesn't really help the user 
> to understand what's wrong with their code. Can it be improved?
Agreed, this does not look very informative. It surely can be improved. Though 
it looks like not related to my fix. My fix is to add two additional loop 
hints. That kind of diagnostic related to all loop hints "clang loop". I.e. all 
pragmas "clang loop" can be checked for various cases and for better 
diagnostic. It looks like separate task. But OK, I will check that case.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55710/new/

https://reviews.llvm.org/D55710



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Michał Górny via Phabricator via cfe-commits]

mgorny updated this revision to Diff 180114.
mgorny edited the summary of this revision.
mgorny added a comment.
Herald added subscribers: atanasyan, sdardis.

Updated to use clang's libdir logic.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56215/new/

https://reviews.llvm.org/D56215

Files:
  ELF/Driver.cpp
  ELF/Driver.h


Index: ELF/Driver.h
===
--- ELF/Driver.h
+++ ELF/Driver.h
@@ -32,6 +32,7 @@
 
 private:
   void readConfigs(llvm::opt::InputArgList &Args);
+  void appendDefaultSearchPaths();
   void createFiles(llvm::opt::InputArgList &Args);
   void inferMachineType();
   template  void link(llvm::opt::InputArgList &Args);
Index: ELF/Driver.cpp
===
--- ELF/Driver.cpp
+++ ELF/Driver.cpp
@@ -365,6 +365,28 @@
   error("unknown -z value: " + StringRef(Arg->getValue()));
 }
 
+void LinkerDriver::appendDefaultSearchPaths() {
+#if defined(__NetBSD__)
+  // NetBSD driver relies on the linker knowing the default search paths.
+  // Please keep this in sync with clang/lib/Driver/ToolChains/NetBSD.cpp
+  // (NetBSD::NetBSD constructor)
+  switch (Config->EMachine) {
+case EM_386:
+  Config->SearchPaths.push_back("/usr/lib/i386");
+  break;
+case EM_MIPS:
+  if (Config->EKind == ELF64LEKind || Config->EKind == ELF64BEKind)
+Config->SearchPaths.push_back("/usr/lib/64");
+  // TODO: how to detect o32?
+  break;
+case EM_PPC64:
+  break;
+// TODO: add *nbsd* emulations when supported
+  }
+  Config->SearchPaths.push_back("/usr/lib");
+#endif
+}
+
 void LinkerDriver::main(ArrayRef ArgsArr) {
   ELFOptTable Parser;
   opt::InputArgList Args = Parser.parse(ArgsArr.slice(1));
@@ -412,6 +434,7 @@
 
   readConfigs(Args);
   checkZOptions(Args);
+  appendDefaultSearchPaths();
 
   // The behavior of -v or --version is a bit strange, but this is
   // needed for compatibility with GNU linkers.


Index: ELF/Driver.h
===
--- ELF/Driver.h
+++ ELF/Driver.h
@@ -32,6 +32,7 @@
 
 private:
   void readConfigs(llvm::opt::InputArgList &Args);
+  void appendDefaultSearchPaths();
   void createFiles(llvm::opt::InputArgList &Args);
   void inferMachineType();
   template  void link(llvm::opt::InputArgList &Args);
Index: ELF/Driver.cpp
===
--- ELF/Driver.cpp
+++ ELF/Driver.cpp
@@ -365,6 +365,28 @@
   error("unknown -z value: " + StringRef(Arg->getValue()));
 }
 
+void LinkerDriver::appendDefaultSearchPaths() {
+#if defined(__NetBSD__)
+  // NetBSD driver relies on the linker knowing the default search paths.
+  // Please keep this in sync with clang/lib/Driver/ToolChains/NetBSD.cpp
+  // (NetBSD::NetBSD constructor)
+  switch (Config->EMachine) {
+case EM_386:
+  Config->SearchPaths.push_back("/usr/lib/i386");
+  break;
+case EM_MIPS:
+  if (Config->EKind == ELF64LEKind || Config->EKind == ELF64BEKind)
+Config->SearchPaths.push_back("/usr/lib/64");
+  // TODO: how to detect o32?
+  break;
+case EM_PPC64:
+  break;
+// TODO: add *nbsd* emulations when supported
+  }
+  Config->SearchPaths.push_back("/usr/lib");
+#endif
+}
+
 void LinkerDriver::main(ArrayRef ArgsArr) {
   ELFOptTable Parser;
   opt::InputArgList Args = Parser.parse(ArgsArr.slice(1));
@@ -412,6 +434,7 @@
 
   readConfigs(Args);
   checkZOptions(Args);
+  appendDefaultSearchPaths();
 
   // The behavior of -v or --version is a bit strange, but this is
   // needed for compatibility with GNU linkers.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

We will prepare a patch for i386 x86_64 only for now. Until we will get lld 
fully functional on NetBSD/amd64 we will skip other architectures.


Repository:
  rLLD LLVM Linker

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56215/new/

https://reviews.llvm.org/D56215



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

[PATCH] D56226: [clang-format] square parens that are followed by '=' are not Objective-C message sends

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 180110.
arphaman added a comment.

Do not annotate one token `[  ]` expressions as Objective-C message sends 
as suggested by Duncan.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56226/new/

https://reviews.llvm.org/D56226

Files:
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -166,6 +166,20 @@
   EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
 }
 
+TEST(FormatTestObjCStyle, AvoidDetectingDesignatedInitializersAsObjCInHeaders) 
{
+  auto Style = getStyle("LLVM", "a.h", "none",
+"static const char *names[] = {[0] = \"foo\",\n"
+"[kBar] = \"bar\"};");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
+
+  Style = getStyle("LLVM", "a.h", "none",
+   "static const char *names[] = {[0] EQ \"foo\",\n"
+   "[kBar] EQ \"bar\"};");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
+}
+
 TEST_F(FormatTestObjC, FormatObjCTryCatch) {
   verifyFormat("@try {\n"
"  f();\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -495,9 +495,13 @@
   if (CurrentToken->is(tok::r_square)) {
 if (IsCpp11AttributeSpecifier)
   CurrentToken->Type = TT_AttributeSquare;
-else if (CurrentToken->Next && CurrentToken->Next->is(tok::l_paren) &&
+else if (((CurrentToken->Next &&
+   (CurrentToken->Next->is(tok::l_paren) ||
+CurrentToken->Next->is(tok::equal))) ||
+  (CurrentToken->Previous->Previous == Left)) &&
  Left->is(TT_ObjCMethodExpr)) {
-  // An ObjC method call is rarely followed by an open parenthesis.
+  // An ObjC method call is rarely followed by an open parenthesis or
+  // an assignment. It also usually contains more than one token.
   // FIXME: Do we incorrectly label ":" with this?
   StartsObjCMethodExpr = false;
   Left->Type = TT_Unknown;


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -166,6 +166,20 @@
   EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
 }
 
+TEST(FormatTestObjCStyle, AvoidDetectingDesignatedInitializersAsObjCInHeaders) {
+  auto Style = getStyle("LLVM", "a.h", "none",
+"static const char *names[] = {[0] = \"foo\",\n"
+"[kBar] = \"bar\"};");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
+
+  Style = getStyle("LLVM", "a.h", "none",
+   "static const char *names[] = {[0] EQ \"foo\",\n"
+   "[kBar] EQ \"bar\"};");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
+}
+
 TEST_F(FormatTestObjC, FormatObjCTryCatch) {
   verifyFormat("@try {\n"
"  f();\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -495,9 +495,13 @@
   if (CurrentToken->is(tok::r_square)) {
 if (IsCpp11AttributeSpecifier)
   CurrentToken->Type = TT_AttributeSquare;
-else if (CurrentToken->Next && CurrentToken->Next->is(tok::l_paren) &&
+else if (((CurrentToken->Next &&
+   (CurrentToken->Next->is(tok::l_paren) ||
+CurrentToken->Next->is(tok::equal))) ||
+  (CurrentToken->Previous->Previous == Left)) &&
  Left->is(TT_ObjCMethodExpr)) {
-  // An ObjC method call is rarely followed by an open parenthesis.
+  // An ObjC method call is rarely followed by an open parenthesis or
+  // an assignment. It also usually contains more than one token.
   // FIXME: Do we incorrectly label ":" with this?
   StartsObjCMethodExpr = false;
   Left->Type = TT_Unknown;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Kristina Brooks via Phabricator via cfe-commits]

kristina added a comment.

I should add that this is not just about reproducible builds but about code 
size as mentioned by @NSProgrammer. There are ways to cheat with builtins but 
the problem is, entire __FILE__ string is still emitted into read-only data, 
despite the constexpressiveness of the value. The common way of getting the 
short filename in code would the the following (the `#else` case) which is a 
constant evaluated but has a side effect of dumping the full paths in rodata 
regardless (ICF seems unable to clean them up even in full LTO builds).

  #ifdef __clang_extension_generate
  #define OS_CUR_FILENAME  __generate("file!0")
  #else
  #define OS_CUR_FILENAME (__builtin_strrchr(__FILE__, '/') ? \
  __builtin_strrchr(__FILE__, '/') + 1 : __FILE__)
  #endif

The extension on the other hand avoids that. So it's a win for code size and 
reproducible builds. I would urge reconsidering something like `__FILE_NAME__`, 
or whatever name is preferable for just getting the last path component at 
preprocessor stage, despite it being poorly received by other developers the 
last time it seems to be a feature wanted by consumers. I would urge 
maintainers to reply to get another consensus regarding this since my stance is 
still the same, if a "missing" feature is widely hacked around, it's clearly 
desirable. While I understand that a lot have an aversion to nonstandard 
Clang-specific extensions, as long as a feature check is available it should be 
down to the consumer to decide if they want to make use of such extensions.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D17741/new/

https://reviews.llvm.org/D17741



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Talking from the perspective of having had to deal with thousands of packages 
in pkgsrc over the years: it is naive to believe that there isn't a lot of 
software that calls the linker directly for various reasons. As such, it is 
very important to have a useful configuration in the linker by default. Such a 
configuration is by its very nature target specific. This doesn't mean it can't 
be done in a cross-compiler friendly way, on the contrary. Over the years 
NetBSD has been pushing its toolchain to be as similar for the native build and 
a cross-target as reasonable possible. Modulo the build time choices in the 
config.h sense, the only difference between the native and cross tools is the 
built-in default of the former.

Clang works extremely well in that regard and perfectly supports a universal 
toolchain. LLD should as well. Consistent behavior of ELF across different OS 
is a fiction, as such some OS and/or CPU family specific defaults are naturally 
different. This can be grouped in general:
(1) The target operating system specified either from the builtin default, the 
name of the tool (triple-ld), an option like clang's --target or if desirable 
the name of the emulation when specified. I don't have a strong reason for 
supporting the last as it is typically not unique even with binutils and more a 
case of historic legacy than useful feature. For the perspective of supporting 
both native and cross toolchains, the first three options are enough and when 
using the compiler driver, the first two will generally work fine.
(2) The target CPU family. While it can be partially guessed from the object 
files, there are fun expects. ARM instruction encodings are one of the 
pecularities a linker has to deal with for example. Is BX a valid jump 
instruction?
(3) Whether the target CPU family is the primary ABI. This can generally not be 
determined from the object files and can affect the default search paths. Hard 
vs soft floating point is a good example here. Other cases are easier like 
linking i386 binaries on x86_64. N32 vs N64 on MIPS is more difficult again. 
This is a bit tricky in that it often can be drived only from the emulation 
name.

In terms of architecture, it doesn't need much, but it needs some thought. 
Identifying the target OS is likely the easiest. Most of the rest is OS 
specific adjustment. Having access to the binary name and the the arguments 
should be enough though. Doing it properly avoids the fun of having to patch a 
lot of software without costing that much in terms of code.


Repository:
  rLLD LLVM Linker

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56215/new/

https://reviews.llvm.org/D56215



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

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

let me look further into that, it works for std::function but not for const 
std::function or std::function &,  const std::function &


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55433/new/

https://reviews.llvm.org/D55433



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

In D56215#1345400 , @ruiu wrote:

> Joerg, what is special about NetBSD?


Nothing.

But the result is that we don't have GNU gold either and are stuck with 40min 
linking times of LLVM. It's destructive to productivity and damages any LLVM 
related development.


Repository:
  rLLD LLVM Linker

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56215/new/

https://reviews.llvm.org/D56215



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Rui Ueyama via Phabricator via cfe-commits]

ruiu added a comment.

I sympathize and understand your frustration, but I don't know what I can do to 
you. The thing that lld doesn't have host-specific config is a policy matter 
we've been maintaining so far for all operating systems. I don't think NetBSD 
should be the only exception of the policy. In addition to that, I strongly 
believe the fact that lld's behavior only depends on the command line options 
given to the linker is easier to understand and generally a good thing.

Joerg, what is special about NetBSD?


Repository:
  rLLD LLVM Linker

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56215/new/

https://reviews.llvm.org/D56215



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

[PATCH] D56271: [SemaCXX] Fix ICE for unexpanded parameter pack

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

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

Looks good, thank you!


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56271/new/

https://reviews.llvm.org/D56271



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

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D56113#1345372 , @jdenny wrote:

> In D56113#1345337 , @ABataev wrote:
>
> > > In D56113#1345238 , @ABataev 
> > > wrote:
> > > 
> > >> In D56113#1345232 , @jdenny 
> > >> wrote:
> > >>
> > >> > In D56113#1345047 , @ABataev 
> > >> > wrote:
> > >> >
> > >> > > In D56113#1344210 , @jdenny 
> > >> > > wrote:
> > >> > >
> > >> > > > In D56113#1342879 , 
> > >> > > > @ABataev wrote:
> > >> > > >
> > >> > > > > But you will need another serie of patches for `reduction` and 
> > >> > > > > `linear` clauses to update their error messages.
> > >> > > >
> > >> > > >
> > >> > > > Those already had their own checks for const.  I'm not aware of 
> > >> > > > any cases not handled, and the test suite does pass after my patch.
> > >> > >
> > >> > >
> > >> > > Yes, they have the checks for constness, but you need to  update 
> > >> > > those checks to emit this new error message for const values with 
> > >> > > mutable fields.
> > >> >
> > >> >
> > >> > I believe you mean we should reuse `rejectConstNotMutableType` for 
> > >> > `reduction` and `linear` rather than leave some of its implementation 
> > >> > duplicated for those.
> > >>
> > > 
> > > 
> > > For reductions, this will change the message from "const-qualified list 
> > > item cannot be reduction" to "const-qualified variable cannot be 
> > > reduction".  Is that OK?  (There are many tests to update, so I want to 
> > > ask first.)
> >
> > Mmmm. For the reductions better to keep the original message, because the 
> > list items also might be the array sections, not only the variables.
>
>
> I need to add a parameter to `rejectConstNotMutableType` to specify whether 
> decl/def notes are included, and I can reuse that parameter for this purpose. 
>  Whether the diagnostic says "list item" or "variable" will then depend on 
> what the list item is (variable, array section, etc.) rather than what the 
> clause is (private, reduction, etc.).  I think that's ok.


Ok, sounds good


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56113/new/

https://reviews.llvm.org/D56113



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

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D56113#1345337 , @ABataev wrote:

> > In D56113#1345238 , @ABataev wrote:
> > 
> >> In D56113#1345232 , @jdenny wrote:
> >>
> >> > In D56113#1345047 , @ABataev 
> >> > wrote:
> >> >
> >> > > In D56113#1344210 , @jdenny 
> >> > > wrote:
> >> > >
> >> > > > In D56113#1342879 , 
> >> > > > @ABataev wrote:
> >> > > >
> >> > > > > But you will need another serie of patches for `reduction` and 
> >> > > > > `linear` clauses to update their error messages.
> >> > > >
> >> > > >
> >> > > > Those already had their own checks for const.  I'm not aware of any 
> >> > > > cases not handled, and the test suite does pass after my patch.
> >> > >
> >> > >
> >> > > Yes, they have the checks for constness, but you need to  update those 
> >> > > checks to emit this new error message for const values with mutable 
> >> > > fields.
> >> >
> >> >
> >> > I believe you mean we should reuse `rejectConstNotMutableType` for 
> >> > `reduction` and `linear` rather than leave some of its implementation 
> >> > duplicated for those.
> >>
> > 
> > 
> > For reductions, this will change the message from "const-qualified list 
> > item cannot be reduction" to "const-qualified variable cannot be 
> > reduction".  Is that OK?  (There are many tests to update, so I want to ask 
> > first.)
>
> Mmmm. For the reductions better to keep the original message, because the 
> list items also might be the array sections, not only the variables.


I need to add a parameter to `rejectConstNotMutableType` to specify whether 
decl/def notes are included, and I can reuse that parameter for this purpose.  
Whether the diagnostic says "list item" or "variable" will then depend on what 
the list item is (variable, array section, etc.) rather than what the clause is 
(private, reduction, etc.).  I think that's ok.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56113/new/

https://reviews.llvm.org/D56113



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

Actually I find it frustrating and unfair as GNU ld doesn't have builtin 
knowledge either.. it's passed with gross 'super hack' comments from build 
scripts... but we are forced to push it to lld in order to move on.


Repository:
  rLLD LLVM Linker

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56215/new/

https://reviews.llvm.org/D56215



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

It's an option but Joerg insists on using lld standalone.


Repository:
  rLLD LLVM Linker

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56215/new/

https://reviews.llvm.org/D56215



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Rui Ueyama via Phabricator via cfe-commits]

ruiu added a comment.

As you can see, lld doesn't really have any host-dependent knowledge so far, 
even though there are a few OSes that use lld as the default linker. We've 
added host-dependent knowledge to the compiler driver instead of to the linker 
for various operating systems. I guess that we could do the same thing for 
NetBSD, no?


Repository:
  rLLD LLVM Linker

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56215/new/

https://reviews.llvm.org/D56215



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

[PATCH] D56273: Validate -add-plugin arguments.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC350340: Validate -add-plugin arguments. (authored by nico, 
committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D56273?vs=180085&id=180099#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56273/new/

https://reviews.llvm.org/D56273

Files:
  lib/Frontend/CompilerInvocation.cpp
  test/Frontend/plugin-unknown.c


Index: test/Frontend/plugin-unknown.c
===
--- test/Frontend/plugin-unknown.c
+++ test/Frontend/plugin-unknown.c
@@ -0,0 +1,5 @@
+// RUN: not %clang_cc1 -plugin asdf %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -add-plugin asdf %s 2>&1 | FileCheck --check-prefix=ADD 
%s
+
+// CHECK: unable to find plugin 'asdf'
+// ADD: unable to find plugin 'asdf'
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -33,6 +33,7 @@
 #include "clang/Frontend/DependencyOutputOptions.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/Frontend/FrontendOptions.h"
+#include "clang/Frontend/FrontendPluginRegistry.h"
 #include "clang/Frontend/LangStandard.h"
 #include "clang/Frontend/MigratorOptions.h"
 #include "clang/Frontend/PreprocessorOutputOptions.h"
@@ -1663,7 +1664,20 @@
 Opts.ProgramAction = frontend::PluginAction;
 Opts.ActionName = A->getValue();
   }
-  Opts.AddPluginActions = Args.getAllArgValues(OPT_add_plugin);
+  for (const std::string &Arg : Args.getAllArgValues(OPT_add_plugin)) {
+bool Found = false;
+for (FrontendPluginRegistry::iterator it = FrontendPluginRegistry::begin(),
+  ie = FrontendPluginRegistry::end();
+ it != ie; ++it) {
+  if (it->getName() == Arg)
+Found = true;
+}
+if (!Found) {
+  Diags.Report(diag::err_fe_invalid_plugin_name) << Arg;
+  continue;
+}
+Opts.AddPluginActions.push_back(Arg);
+  }
   for (const auto *AA : Args.filtered(OPT_plugin_arg))
 Opts.PluginArgs[AA->getValue(0)].emplace_back(AA->getValue(1));
 


Index: test/Frontend/plugin-unknown.c
===
--- test/Frontend/plugin-unknown.c
+++ test/Frontend/plugin-unknown.c
@@ -0,0 +1,5 @@
+// RUN: not %clang_cc1 -plugin asdf %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -add-plugin asdf %s 2>&1 | FileCheck --check-prefix=ADD %s
+
+// CHECK: unable to find plugin 'asdf'
+// ADD: unable to find plugin 'asdf'
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -33,6 +33,7 @@
 #include "clang/Frontend/DependencyOutputOptions.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/Frontend/FrontendOptions.h"
+#include "clang/Frontend/FrontendPluginRegistry.h"
 #include "clang/Frontend/LangStandard.h"
 #include "clang/Frontend/MigratorOptions.h"
 #include "clang/Frontend/PreprocessorOutputOptions.h"
@@ -1663,7 +1664,20 @@
 Opts.ProgramAction = frontend::PluginAction;
 Opts.ActionName = A->getValue();
   }
-  Opts.AddPluginActions = Args.getAllArgValues(OPT_add_plugin);
+  for (const std::string &Arg : Args.getAllArgValues(OPT_add_plugin)) {
+bool Found = false;
+for (FrontendPluginRegistry::iterator it = FrontendPluginRegistry::begin(),
+  ie = FrontendPluginRegistry::end();
+ it != ie; ++it) {
+  if (it->getName() == Arg)
+Found = true;
+}
+if (!Found) {
+  Diags.Report(diag::err_fe_invalid_plugin_name) << Arg;
+  continue;
+}
+Opts.AddPluginActions.push_back(Arg);
+  }
   for (const auto *AA : Args.filtered(OPT_plugin_arg))
 Opts.PluginArgs[AA->getValue(0)].emplace_back(AA->getValue(1));
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r350340 - Validate -add-plugin arguments.

[Nico Weber via cfe-commits]

Author: nico
Date: Thu Jan  3 10:26:06 2019
New Revision: 350340

URL: http://llvm.org/viewvc/llvm-project?rev=350340&view=rev
Log:
Validate -add-plugin arguments.

-plugin already prints an error if the name of an unknown plugin is passed.
-add-plugin used to silently ignore that, now it errors too.

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

Added:
cfe/trunk/test/Frontend/plugin-unknown.c
Modified:
cfe/trunk/lib/Frontend/CompilerInvocation.cpp

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=350340&r1=350339&r2=350340&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Thu Jan  3 10:26:06 2019
@@ -33,6 +33,7 @@
 #include "clang/Frontend/DependencyOutputOptions.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/Frontend/FrontendOptions.h"
+#include "clang/Frontend/FrontendPluginRegistry.h"
 #include "clang/Frontend/LangStandard.h"
 #include "clang/Frontend/MigratorOptions.h"
 #include "clang/Frontend/PreprocessorOutputOptions.h"
@@ -1663,7 +1664,20 @@ static InputKind ParseFrontendArgs(Front
 Opts.ProgramAction = frontend::PluginAction;
 Opts.ActionName = A->getValue();
   }
-  Opts.AddPluginActions = Args.getAllArgValues(OPT_add_plugin);
+  for (const std::string &Arg : Args.getAllArgValues(OPT_add_plugin)) {
+bool Found = false;
+for (FrontendPluginRegistry::iterator it = FrontendPluginRegistry::begin(),
+  ie = FrontendPluginRegistry::end();
+ it != ie; ++it) {
+  if (it->getName() == Arg)
+Found = true;
+}
+if (!Found) {
+  Diags.Report(diag::err_fe_invalid_plugin_name) << Arg;
+  continue;
+}
+Opts.AddPluginActions.push_back(Arg);
+  }
   for (const auto *AA : Args.filtered(OPT_plugin_arg))
 Opts.PluginArgs[AA->getValue(0)].emplace_back(AA->getValue(1));
 

Added: cfe/trunk/test/Frontend/plugin-unknown.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Frontend/plugin-unknown.c?rev=350340&view=auto
==
--- cfe/trunk/test/Frontend/plugin-unknown.c (added)
+++ cfe/trunk/test/Frontend/plugin-unknown.c Thu Jan  3 10:26:06 2019
@@ -0,0 +1,5 @@
+// RUN: not %clang_cc1 -plugin asdf %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -add-plugin asdf %s 2>&1 | FileCheck --check-prefix=ADD 
%s
+
+// CHECK: unable to find plugin 'asdf'
+// ADD: unable to find plugin 'asdf'


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

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D56113#1345336 , @jdenny wrote:

> In D56113#1345238 , @ABataev wrote:
>
> > >>> By the way, is there any value to keeping the predetermined shared for 
> > >>> const if -openmp-version=3.1 or earlier?
> > >> 
> > >> Yes, you can check for the value of `LangOpts.OpenMP`. For OpenMP 3.1 it 
> > >> will have the value `31`.
> > > 
> > > How far back should we take this?  I'm inclined to check for `30` and 
> > > `31` only and assume anything else is newer, but let me know if we need 
> > > to check for earlier versions.
> >
> > I think `<= 31` is good. Clang always supported only OpenMP 3.1 and higher.
>
>
> Ah, for some reason I assumed it was a string not an integer.
>
> In D56113#1345238 , @ABataev wrote:
>
> > In D56113#1345232 , @jdenny wrote:
> >
> > > In D56113#1345047 , @ABataev 
> > > wrote:
> > >
> > > > In D56113#1344210 , @jdenny 
> > > > wrote:
> > > >
> > > > > In D56113#1342879 , @ABataev 
> > > > > wrote:
> > > > >
> > > > > > But you will need another serie of patches for `reduction` and 
> > > > > > `linear` clauses to update their error messages.
> > > > >
> > > > >
> > > > > Those already had their own checks for const.  I'm not aware of any 
> > > > > cases not handled, and the test suite does pass after my patch.
> > > >
> > > >
> > > > Yes, they have the checks for constness, but you need to  update those 
> > > > checks to emit this new error message for const values with mutable 
> > > > fields.
> > >
> > >
> > > I believe you mean we should reuse `rejectConstNotMutableType` for 
> > > `reduction` and `linear` rather than leave some of its implementation 
> > > duplicated for those.
> >
>
>
> For reductions, this will change the message from "const-qualified list item 
> cannot be reduction" to "const-qualified variable cannot be reduction".  Is 
> that OK?  (There are many tests to update, so I want to ask first.)


Mmmm. For the reductions better to keep the original message, because the list 
items also might be the array sections, not only the variables.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56113/new/

https://reviews.llvm.org/D56113



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

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D56113#1345238 , @ABataev wrote:

> >>> By the way, is there any value to keeping the predetermined shared for 
> >>> const if -openmp-version=3.1 or earlier?
> >> 
> >> Yes, you can check for the value of `LangOpts.OpenMP`. For OpenMP 3.1 it 
> >> will have the value `31`.
> > 
> > How far back should we take this?  I'm inclined to check for `30` and `31` 
> > only and assume anything else is newer, but let me know if we need to check 
> > for earlier versions.
>
> I think `<= 31` is good. Clang always supported only OpenMP 3.1 and higher.


Ah, for some reason I assumed it was a string not an integer.

In D56113#1345238 , @ABataev wrote:

> In D56113#1345232 , @jdenny wrote:
>
> > In D56113#1345047 , @ABataev wrote:
> >
> > > In D56113#1344210 , @jdenny 
> > > wrote:
> > >
> > > > In D56113#1342879 , @ABataev 
> > > > wrote:
> > > >
> > > > > But you will need another serie of patches for `reduction` and 
> > > > > `linear` clauses to update their error messages.
> > > >
> > > >
> > > > Those already had their own checks for const.  I'm not aware of any 
> > > > cases not handled, and the test suite does pass after my patch.
> > >
> > >
> > > Yes, they have the checks for constness, but you need to  update those 
> > > checks to emit this new error message for const values with mutable 
> > > fields.
> >
> >
> > I believe you mean we should reuse `rejectConstNotMutableType` for 
> > `reduction` and `linear` rather than leave some of its implementation 
> > duplicated for those.
>


For reductions, this will change the message from "const-qualified list item 
cannot be reduction" to "const-qualified variable cannot be reduction".  Is 
that OK?  (There are many tests to update, so I want to ask first.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56113/new/

https://reviews.llvm.org/D56113



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

In D56215#1345326 , @ruiu wrote:

> In D56215#1344279 , @krytarowski 
> wrote:
>
> > In D56215#1344233 , @ruiu wrote:
> >
> > > lld's driver is intentionally designed to be agnostic of the host that 
> > > the linker is running for the reasons described at the beginning of 
> > > Driver.cpp: 
> > > https://github.com/llvm-project/lld/blob/master/ELF/Driver.cpp#L13 I 
> > > think I like that approach. If you need to do this, you can do this in 
> > > the compiler driver rather than in the linker itself. Is there any reason 
> > > you need to do this in the linker?
> >
> >
> > This breaks compat with GNU ld here and the linker is intended to be used 
> > standalone.
>
>
> This is where lld is not 100% compatible with GNU ld, but I'd think that's 
> not a bad thing. I'd like to make lld agnostics of host OS so that the linker 
> works exactly in the same way on any operating systems, which makes cross 
> linking much easier to do. So, both a run-time detection of a host OS or a 
> configure-time customization are I think undesirable.


Personally I have no strong opinion either way. Joerg is blocking a patch to 
handle it in clang: https://reviews.llvm.org/D33726

I find it very silly reason to brick NetBSD support in lld. I expect that it's 
easier to push paths into lld than convince @joerg to accept clang patch.


Repository:
  rLLD LLVM Linker

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56215/new/

https://reviews.llvm.org/D56215



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

[PATCH] D56273: Validate -add-plugin arguments.

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56273/new/

https://reviews.llvm.org/D56273



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Rui Ueyama via Phabricator via cfe-commits]

ruiu added a comment.

In D56215#1344279 , @krytarowski wrote:

> In D56215#1344233 , @ruiu wrote:
>
> > lld's driver is intentionally designed to be agnostic of the host that the 
> > linker is running for the reasons described at the beginning of Driver.cpp: 
> > https://github.com/llvm-project/lld/blob/master/ELF/Driver.cpp#L13 I think 
> > I like that approach. If you need to do this, you can do this in the 
> > compiler driver rather than in the linker itself. Is there any reason you 
> > need to do this in the linker?
>
>
> This breaks compat with GNU ld here and the linker is intended to be used 
> standalone.


This is where lld is not 100% compatible with GNU ld, but I'd think that's not 
a bad thing. I'd like to make lld agnostics of host OS so that the linker works 
exactly in the same way on any operating systems, which makes cross linking 
much easier to do. So, both a run-time detection of a host OS or a 
configure-time customization are I think undesirable.


Repository:
  rLLD LLVM Linker

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56215/new/

https://reviews.llvm.org/D56215



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

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner added a comment.

OK, thanks Serge! :)

For the record, I updated the patch one last time after it was accepted to 
remove my change to constant-expression-cxx1y.cpp since someone else did the 
same change in a separate commit meanwhile.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55382/new/

https://reviews.llvm.org/D55382



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

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC350335: Make -Wstring-plus-int warns even if when the result 
is not out of bounds (authored by abien, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D55382?vs=180075&id=180093#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55382/new/

https://reviews.llvm.org/D55382

Files:
  bindings/python/tests/cindex/test_diagnostics.py
  lib/Sema/SemaExpr.cpp
  test/SemaCXX/string-plus-int.cpp


Index: bindings/python/tests/cindex/test_diagnostics.py
===
--- bindings/python/tests/cindex/test_diagnostics.py
+++ bindings/python/tests/cindex/test_diagnostics.py
@@ -51,7 +51,7 @@
 self.assertEqual(tu.diagnostics[0].fixits[0].value, '.f0 = ')
 
 def test_diagnostic_range(self):
-tu = get_tu('void f() { int i = "a" + 1; }')
+tu = get_tu('void f() { int i = "a"; }')
 self.assertEqual(len(tu.diagnostics), 1)
 self.assertEqual(tu.diagnostics[0].severity, Diagnostic.Warning)
 self.assertEqual(tu.diagnostics[0].location.line, 1)
@@ -63,7 +63,7 @@
 self.assertEqual(tu.diagnostics[0].ranges[0].start.line, 1)
 self.assertEqual(tu.diagnostics[0].ranges[0].start.column, 20)
 self.assertEqual(tu.diagnostics[0].ranges[0].end.line, 1)
-self.assertEqual(tu.diagnostics[0].ranges[0].end.column, 27)
+self.assertEqual(tu.diagnostics[0].ranges[0].end.column, 23)
 with self.assertRaises(IndexError):
 tu.diagnostics[0].ranges[1].start.line
 
Index: test/SemaCXX/string-plus-int.cpp
===
--- test/SemaCXX/string-plus-int.cpp
+++ test/SemaCXX/string-plus-int.cpp
@@ -31,37 +31,36 @@
   consume("foo" + 5);  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume("foo" + index);  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume("foo" + kMyEnum);  // expected-warning {{adding 'MyEnum' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
+  consume("foo" + kMySmallEnum); // expected-warning {{adding 'MyEnum' to a 
string does not append to the string}} expected-note {{use array indexing to 
silence this warning}}
 
   consume(5 + "foo");  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume(index + "foo");  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume(kMyEnum + "foo");  // expected-warning {{adding 'MyEnum' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
+  consume(kMySmallEnum + "foo"); // expected-warning {{adding 'MyEnum' to a 
string does not append to the string}} expected-note {{use array indexing to 
silence this warning}}
 
   // FIXME: suggest replacing with "foo"[5]
   consumeChar(*("foo" + 5));  // expected-warning {{adding 'int' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
   consumeChar(*(5 + "foo"));  // expected-warning {{adding 'int' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
 
   consume(L"foo" + 5);  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
+  consume(L"foo" + 2); // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
+
+  consume("foo" + 3);  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
+  consume("foo" + 4);  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
+  consume("\pfoo" + 4);  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
+
+  #define A "foo"
+  #define B "bar"
+  consume(A B + sizeof(A) - 1); // expected-warning {{to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
 
   // Should not warn.
   consume(&("foo"[3]));
   consume(&("foo"[index]));
   consume(&("foo"[kMyEnum]));
-  consume("foo" + kMySmallEnum);
-  consume(kMySmallEnum + "foo");
 
-  consume(L"foo" + 2);
-
-  consume("foo" + 3);  // Points at the \0
-  consume("foo" + 4);  // Points 1 past the \0, which is legal too.
-  consume(

r350335 - Make -Wstring-plus-int warns even if when the result is not out of bounds

[Arnaud Bienner via cfe-commits]

Author: abien
Date: Thu Jan  3 09:45:28 2019
New Revision: 350335

URL: http://llvm.org/viewvc/llvm-project?rev=350335&view=rev
Log:
Make -Wstring-plus-int warns even if when the result is not out of bounds

Summary: Patch by Arnaud Bienner

Reviewers: sylvestre.ledru, thakis, serge-sans-paille

Reviewed By: thakis

Subscribers: arphaman, dyung, anemet, llvm-commits, cfe-commits

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

Modified:
cfe/trunk/bindings/python/tests/cindex/test_diagnostics.py
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/SemaCXX/string-plus-int.cpp

Modified: cfe/trunk/bindings/python/tests/cindex/test_diagnostics.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/bindings/python/tests/cindex/test_diagnostics.py?rev=350335&r1=350334&r2=350335&view=diff
==
--- cfe/trunk/bindings/python/tests/cindex/test_diagnostics.py (original)
+++ cfe/trunk/bindings/python/tests/cindex/test_diagnostics.py Thu Jan  3 
09:45:28 2019
@@ -51,7 +51,7 @@ class TestDiagnostics(unittest.TestCase)
 self.assertEqual(tu.diagnostics[0].fixits[0].value, '.f0 = ')
 
 def test_diagnostic_range(self):
-tu = get_tu('void f() { int i = "a" + 1; }')
+tu = get_tu('void f() { int i = "a"; }')
 self.assertEqual(len(tu.diagnostics), 1)
 self.assertEqual(tu.diagnostics[0].severity, Diagnostic.Warning)
 self.assertEqual(tu.diagnostics[0].location.line, 1)
@@ -63,7 +63,7 @@ class TestDiagnostics(unittest.TestCase)
 self.assertEqual(tu.diagnostics[0].ranges[0].start.line, 1)
 self.assertEqual(tu.diagnostics[0].ranges[0].start.column, 20)
 self.assertEqual(tu.diagnostics[0].ranges[0].end.line, 1)
-self.assertEqual(tu.diagnostics[0].ranges[0].end.column, 27)
+self.assertEqual(tu.diagnostics[0].ranges[0].end.column, 23)
 with self.assertRaises(IndexError):
 tu.diagnostics[0].ranges[1].start.line
 

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=350335&r1=350334&r2=350335&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Jan  3 09:45:28 2019
@@ -9143,16 +9143,6 @@ static void diagnoseStringPlusInt(Sema &
   if (!IsStringPlusInt || IndexExpr->isValueDependent())
 return;
 
-  Expr::EvalResult Result;
-  if (IndexExpr->EvaluateAsInt(Result, Self.getASTContext())) {
-llvm::APSInt index = Result.Val.getInt();
-unsigned StrLenWithNull = StrExpr->getLength() + 1;
-if (index.isNonNegative() &&
-index <= llvm::APSInt(llvm::APInt(index.getBitWidth(), StrLenWithNull),
-  index.isUnsigned()))
-  return;
-  }
-
   SourceRange DiagRange(LHSExpr->getBeginLoc(), RHSExpr->getEndLoc());
   Self.Diag(OpLoc, diag::warn_string_plus_int)
   << DiagRange << IndexExpr->IgnoreImpCasts()->getType();

Modified: cfe/trunk/test/SemaCXX/string-plus-int.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/string-plus-int.cpp?rev=350335&r1=350334&r2=350335&view=diff
==
--- cfe/trunk/test/SemaCXX/string-plus-int.cpp (original)
+++ cfe/trunk/test/SemaCXX/string-plus-int.cpp Thu Jan  3 09:45:28 2019
@@ -31,37 +31,36 @@ void f(int index) {
   consume("foo" + 5);  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume("foo" + index);  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume("foo" + kMyEnum);  // expected-warning {{adding 'MyEnum' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
+  consume("foo" + kMySmallEnum); // expected-warning {{adding 'MyEnum' to a 
string does not append to the string}} expected-note {{use array indexing to 
silence this warning}}
 
   consume(5 + "foo");  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume(index + "foo");  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume(kMyEnum + "foo");  // expected-warning {{adding 'MyEnum' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
+  consume(kMySmallEnum + "foo"); // expected-warning {{adding 'MyEnum' to a 
string does not append to the string}} expected-note {{use array indexing to 
silence this warning}}
 
   // FIXME: suggest replacing with "foo"[5]
   consumeChar(*("foo" + 5));  // expected-warning {{adding 'int' to a stri

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: test/Sema/pragma-pipeline.cpp:3
+
+#pragma clang loop pipeline(disable) /* expected-error {{expected 
unqualified-id}} */
+int main() {

I think this error is pretty unfortunate -- it doesn't really help the user to 
understand what's wrong with their code. Can it be improved?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55710/new/

https://reviews.llvm.org/D55710



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

[PATCH] D56090: Add a matcher for members of an initializer list expression

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: klimek.
aaron.ballman added a comment.

Adding Manuel for the `hasArg()` questions.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56090/new/

https://reviews.llvm.org/D56090



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

[PATCH] D56090: Add a matcher for members of an initializer list expression

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a subscriber: klimek.
aaron.ballman added inline comments.



Comment at: include/clang/ASTMatchers/ASTMatchers.h:3527
+  return N < Node.getNumInits() &&
+  InnerMatcher.matches(*Node.getInit(N)->IgnoreParenImpCasts(), Finder,
+   Builder);

hwright wrote:
> aaron.ballman wrote:
> > I'm not certain we want the `IgnoreParenImpCasts()` here -- what if someone 
> > wants to match an initializer that uses one of those properties?
> The `hasArg` implementation directly above has the same call to 
> `IgnoreParenImpCasts()`.  Is it also in error?  (It would seem that they 
> should both be consistent.)
Yeah, I noticed that as well. Doing some code archaeology, it seems that 
`hasArg()` has had that form since its inception and it was never explicitly 
discussed what should happen there.

@klimek -- do you have opinions here, since you wrote the original code for 
`hasArg()`? Should we be leaving the paren and implicit cast stripping up to 
the caller rather than doing it automatically?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56090/new/

https://reviews.llvm.org/D56090



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

[PATCH] D56090: Add a matcher for members of an initializer list expression

[Hyrum Wright via Phabricator via cfe-commits]

hwright added inline comments.



Comment at: include/clang/ASTMatchers/ASTMatchers.h:3527
+  return N < Node.getNumInits() &&
+  InnerMatcher.matches(*Node.getInit(N)->IgnoreParenImpCasts(), Finder,
+   Builder);

aaron.ballman wrote:
> I'm not certain we want the `IgnoreParenImpCasts()` here -- what if someone 
> wants to match an initializer that uses one of those properties?
The `hasArg` implementation directly above has the same call to 
`IgnoreParenImpCasts()`.  Is it also in error?  (It would seem that they should 
both be consistent.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56090/new/

https://reviews.llvm.org/D56090



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

[PATCH] D56090: Add a matcher for members of an initializer list expression

[Hyrum Wright via Phabricator via cfe-commits]

hwright updated this revision to Diff 180089.
hwright marked an inline comment as done.
hwright added a comment.

Added tests, update docs.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56090/new/

https://reviews.llvm.org/D56090

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp


Index: unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2255,6 +2255,18 @@
   notMatches("void x() { int a; if(a == 0) return; }", BinAsgmtOperator));
 }
 
+TEST(HasInit, Basic) {
+  EXPECT_TRUE(
+matches("int x{0};",
+initListExpr(hasInit(0, expr();
+  EXPECT_FALSE(
+matches("int x{0};",
+initListExpr(hasInit(1, expr();
+  EXPECT_FALSE(
+matches("int x;",
+initListExpr(hasInit(0, expr();
+}
+
 TEST(Matcher, isMain) {
   EXPECT_TRUE(
 matches("int main() {}", functionDecl(isMain(;
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -273,6 +273,7 @@
   REGISTER_MATCHER(hasInClassInitializer);
   REGISTER_MATCHER(hasIncrement);
   REGISTER_MATCHER(hasIndex);
+  REGISTER_MATCHER(hasInit);
   REGISTER_MATCHER(hasInitializer);
   REGISTER_MATCHER(hasKeywordSelector);
   REGISTER_MATCHER(hasLHS);
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -3514,6 +3514,20 @@
   *Node.getArg(N)->IgnoreParenImpCasts(), Finder, Builder));
 }
 
+/// Matches the n'th item of an initializer list expression.
+///
+/// Example matches y.
+/// (matcher = initListExpr(hasInit(0, expr(
+/// \code
+///   int x{y}.
+/// \endcode
+AST_MATCHER_P2(InitListExpr, hasInit, unsigned, N,
+   ast_matchers::internal::Matcher, InnerMatcher) {
+  return N < Node.getNumInits() &&
+  InnerMatcher.matches(*Node.getInit(N)->IgnoreParenImpCasts(), Finder,
+   Builder);
+}
+
 /// Matches declaration statements that contain a specific number of
 /// declarations.
 ///
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -718,7 +718,7 @@
 
 
 MatcherStmt>blockExprMatcherBlockExpr>...
-MAtches a reference to a 
block.
+Matches a reference to a 
block.
 
 Example: matches "^{}":
   void f() { ^{}(); }
@@ -5866,6 +5866,15 @@
 
 
 
+MatcherInitListExpr>hasInitunsigned N, 
ast_matchers::MatcherExpr> 
InnerMatcher
+Matches the n'th item of an 
initializer list expression.
+
+Example matches y.
+(matcher = initListExpr(hasInit(0, expr(
+  int x{y}.
+
+
+
 MatcherInitListExpr>hasSyntacticFormMatcherExpr> 
InnerMatcher
 Matches the 
syntactic form of init list expressions
 (if expression have it).


Index: unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2255,6 +2255,18 @@
   notMatches("void x() { int a; if(a == 0) return; }", BinAsgmtOperator));
 }
 
+TEST(HasInit, Basic) {
+  EXPECT_TRUE(
+matches("int x{0};",
+initListExpr(hasInit(0, expr();
+  EXPECT_FALSE(
+matches("int x{0};",
+initListExpr(hasInit(1, expr();
+  EXPECT_FALSE(
+matches("int x;",
+initListExpr(hasInit(0, expr();
+}
+
 TEST(Matcher, isMain) {
   EXPECT_TRUE(
 matches("int main() {}", functionDecl(isMain(;
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -273,6 +273,7 @@
   REGISTER_MATCHER(hasInClassInitializer);
   REGISTER_MATCHER(hasIncrement);
   REGISTER_MATCHER(hasIndex);
+  REGISTER_MATCHER(hasInit);
   REGISTER_MATCHER(hasInitializer);
   REGISTER_MATCHER(hasKeywordSelector);
   REGISTER_MATCHER(hasLHS);
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Ximin Luo via cfe-commits]

Hi all, more strict interpretations of reproducible builds want build paths to 
not appear in the final output *regardless of the build location* and so 
/tmp/build would not be appropriate. Rough justification is that we want 
different people building under different starting conditions to be able 
reproduce identical final binaries, this is a *stronger* property than 
reproduction under *same* starting conditions.

I believe newer versions of GCC have an existing flag called -fmacro-prefix-map:

https://gcc.gnu.org/onlinedocs/gcc/Preprocessor-Options.html

My patch that Roman quoted in the email below was an additional mechanism to 
set -fmacro-prefix-map and -fdebug-prefix-map via an environment variable. This 
was not accepted into GCC because they are (in summary) allergic against using 
environment variables to set anything.

I agree envvars are not great in general, but think an exception in this case 
was appropriate. It is very common for higher-level build scripts to store 
command-line options in various parts of the build output (making it again 
dependent on the build path, since the argument to -fmacro-prefix-map contains 
the build path). By contrast, it is almost unheard-of for tools to store 
arbitrary environment variables in the build output, and that usually indicates 
a bug.

This is why we the reproducible builds project defined some standardised 
environment variables for these purposes:

https://reproducible-builds.org/specs/source-date-epoch/
https://reproducible-builds.org/specs/build-path-prefix-map/

Inconsistently, GCC accepted our (The Reproducible Builds Project's) patch for 
the former but not the latter, due to different reviewers. (FWIW, the reviewer 
that rejected the latter patch stated that he would not have accepted the 
former patch.)

Ed Maste (added to CC) had previously tried to get SOURCE_DATE_EPOCH accepted 
into LLVM but the effort seems to have stalled.

https://reviews.llvm.org/D20791
https://reviews.llvm.org/D23934

I'll leave it to you guys to decide where to go from here.

X

Roman Lebedev:
> No, that is the opposite of the solution, actually, i think.
> https://tests.reproducible-builds.org/debian/issues/unstable/gcc_captures_build_path_issue.html
> 
> And:
> https://gcc.gnu.org/ml/gcc-patches/2017-04/msg00513.html
> ^ i suspect clang should be compatible with whatever approach is
> suggested there,
> though i have not looked what it is. (cc'd the author of that patch here)
> 
> On Thu, Jan 3, 2019 at 6:54 PM Nolan O'Brien  wrote:
>>
>> I believe reproducible build projects simply move their source to 
>> `/tmp/build` or something else that’s consistent to deal with `__FILE__` 
>> currently.  They’d probably appreciate this to avoid file copying during 
>> `make` builds
>>
>> Nolan O'Brien
>> Sent from my iPhone
>>
>>> On Jan 3, 2019, at 7:47 AM, Roman Lebedev  wrote:
>>>
>>> Just a thought: reproducible builds will surely be interested in this.
>>> Did gcc already solve this problem? And if yes, it would be best to be
>>> compatible.
>>>
>>> On Thu, Jan 3, 2019 at 6:41 PM Nolan O'Brien via Phabricator via
>>> cfe-commits  wrote:

 NSProgrammer added a comment.

 To throw in my 2 cents.  I don’t really have a preference between a 
 compiler flag vs a different macro that’s just for the file name sans path 
 prefix.  But I have a real need for this to get into clang:  with 1.2 
 million lines of code, the regular placement of log statements and custom 
 asserts leads to megabytes in binary size from all the __FILE__ usages, 
 and that could easily be a few hundred KB with this kind of support in 
 clang.


 CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D17741/new/

 https://reviews.llvm.org/D17741



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


-- 
GPG: ed25519/56034877E1F87C35
GPG: rsa4096/1318EFAC5FBBDBCE
https://github.com/infinity0/pubkeys.git
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55932: [Sema] Simplfy static_assert diagnostic code.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: test/SemaCXX/static-assert.cpp:130
 static_assert(std::is_same()), 
int>::value, "message");
-// expected-error@-1{{static_assert failed due to requirement 
'std::is_same, int>::value' "message"}}
+// expected-error@-1{{static_assert failed due to requirement 
'std::is_same, int>::value' "message"}}
 static_assert(std::is_const::value, "message");

courbet wrote:
> aaron.ballman wrote:
> > courbet wrote:
> > > aaron.ballman wrote:
> > > > courbet wrote:
> > > > > aaron.ballman wrote:
> > > > > > Any idea why the `std::` was dropped here?
> > > > > `NestedNameSpecifier::print()` explicitly does:
> > > > > 
> > > > > ```
> > > > >  PrintingPolicy InnerPolicy(Policy);
> > > > >  InnerPolicy.SuppressScope = true;
> > > > > ```
> > > > > 
> > > > Ah, good point, but is that a good behavioral change? I slightly prefer 
> > > > printing the namespace name there -- it will likely be redundant 
> > > > information most of the time, but when the namespace actually matters, 
> > > > having it printed could save someone a lot of head scratching.
> > > > I slightly prefer printing the namespace name there
> > > 
> > > I tend to agree, so it's more a trade-off of code complexity vs better 
> > > diagnostic - I tend to err on the side of simplifying the code :)
> > > 
> > > Another option is to add yet another boolean to PrintingPolicy, but I 
> > > htink this is too narrow a use case.
> > Heh, I tend to err on the side of helping the user unless the code will be 
> > truly awful. I agree that another option on PrintingPolicy may not be the 
> > best approach. Do you know why the namespace is being suppressed in the 
> > first place? Another option would be to always print the namespace, but I 
> > don't know what that might regress (if anything).
> Unfortunately always printing the qualification breaks 30 tests, including 
> some in a very bad way:
> 
> ```
> /usr/local/google/home/courbet/llvm/llvm/tools/clang/test/AST/ast-print-out-of-line-func.cpp:29:11:
>  error: CHECK: expected string not found in input
> // CHECK: void Wrapper::Inner::operator+=(int)
>   ^
> :14:43: note: scanning from here
>  ns::Wrapper::ns::Wrapper::Inner::Inner() {
>   ^
> :16:19: note: possible intended match here
>  void ns::Wrapper::ns::Wrapper::Inner::operator+=(int) {
> ```
That looks like a bug with the printer, no?

As it stands, I think this is a regression with the diagnostic printing. I 
think it would be reasonable if the namespace were dropped only when the 
namespace reduces clarity (e.g., there are no other conflicting names in other 
namespaces, so the namespace is superfluous), but I don't think that's what's 
happening here.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55932/new/

https://reviews.llvm.org/D55932



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

[PATCH] D55933: [Sema] Do not print default template parameters.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: lib/AST/TypePrinter.cpp:179
+// `X` gets canonicalized to `X`. We disable
+// canonicalization so that `printTag()`` can see the template parameters 
as
+// written.

Nit: there's an extra ` on this line.

I think I vaguely understand the purpose of this switch now. It feels ugly, but 
I have no concrete suggestion for improvement. Do you not need a case here to 
delay canonicalization of `X&` also?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55933/new/

https://reviews.llvm.org/D55933



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

[PATCH] D47817: [compiler-rt] [sanitizer_common] Remove support for tirpc/rpc/xdr.h

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

@ygribov, ping.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D47817/new/

https://reviews.llvm.org/D47817



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