[PATCH] D59631: [AArch64] Support selecting TPIDR_EL[1-3] as the thread base

2019-03-20 Thread Philip Derrin via Phabricator via cfe-commits
philip.derrin created this revision.
Herald added subscribers: cfe-commits, kristof.beyls, javed.absar.
Herald added a project: clang.

Add an -mtpidr=el[0-3] option to select which of the AArch64 thread ID 
registers will be used for the TLS base pointer.

This is a followup to D54685  which added 
subtarget features to enable accesses to the privileged thread ID registers.


Repository:
  rC Clang

https://reviews.llvm.org/D59631

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/test/Driver/clang-translation.c


Index: clang/test/Driver/clang-translation.c
===
--- clang/test/Driver/clang-translation.c
+++ clang/test/Driver/clang-translation.c
@@ -120,6 +120,36 @@
 // RUN: FileCheck -check-prefix=ARMv7_THREAD_POINTER_NON %s
 // ARMv7_THREAD_POINTER_NON-NOT: "-target-feature" "+read-tp-hard"
 
+// RUN: %clang -target aarch64-linux -### -S %s -arch armv8a 2>&1 | \
+// RUN: FileCheck -check-prefix=ARMv8_THREAD_POINTER_NON %s
+// ARMv8_THREAD_POINTER_NON-NOT: "-target-feature" "+tpidr-el1"
+// ARMv8_THREAD_POINTER_NON-NOT: "-target-feature" "+tpidr-el2"
+// ARMv8_THREAD_POINTER_NON-NOT: "-target-feature" "+tpidr-el3"
+
+// RUN: %clang -target aarch64-linux -### -S %s -arch armv8a -mtpidr=el0 2>&1 
| \
+// RUN: FileCheck -check-prefix=ARMv8_THREAD_POINTER_EL0 %s
+// ARMv8_THREAD_POINTER_EL0-NOT: "-target-feature" "+tpidr-el1"
+// ARMv8_THREAD_POINTER_EL0-NOT: "-target-feature" "+tpidr-el2"
+// ARMv8_THREAD_POINTER_EL0-NOT: "-target-feature" "+tpidr-el3"
+
+// RUN: %clang -target aarch64-linux -### -S %s -arch armv8a -mtpidr=el1 2>&1 
| \
+// RUN: FileCheck -check-prefix=ARMv8_THREAD_POINTER_EL1 %s
+// ARMv8_THREAD_POINTER_EL1: "-target-feature" "+tpidr-el1"
+// ARMv8_THREAD_POINTER_EL1-NOT: "-target-feature" "+tpidr-el2"
+// ARMv8_THREAD_POINTER_EL1-NOT: "-target-feature" "+tpidr-el3"
+
+// RUN: %clang -target aarch64-linux -### -S %s -arch armv8a -mtpidr=el2 2>&1 
| \
+// RUN: FileCheck -check-prefix=ARMv8_THREAD_POINTER_EL2 %s
+// ARMv8_THREAD_POINTER_EL2-NOT: "-target-feature" "+tpidr-el1"
+// ARMv8_THREAD_POINTER_EL2: "-target-feature" "+tpidr-el2"
+// ARMv8_THREAD_POINTER_EL2-NOT: "-target-feature" "+tpidr-el3"
+
+// RUN: %clang -target aarch64-linux -### -S %s -arch armv8a -mtpidr=el3 2>&1 
| \
+// RUN: FileCheck -check-prefix=ARMv8_THREAD_POINTER_EL3 %s
+// ARMv8_THREAD_POINTER_EL3-NOT: "-target-feature" "+tpidr-el1"
+// ARMv8_THREAD_POINTER_EL3-NOT: "-target-feature" "+tpidr-el2"
+// ARMv8_THREAD_POINTER_EL3: "-target-feature" "+tpidr-el3"
+
 // RUN: %clang -target powerpc64-unknown-linux-gnu \
 // RUN: -### -S %s -mcpu=G5 2>&1 | FileCheck -check-prefix=PPCG5 %s
 // PPCG5: clang
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -194,6 +194,18 @@
 Features.push_back("-neon");
   }
 
+  if (Arg *A = Args.getLastArg(options::OPT_mtpidr_EQ)) {
+StringRef Mtpidr = A->getValue();
+if (Mtpidr == "el3")
+  Features.push_back("+tpidr-el3");
+else if (Mtpidr == "el2")
+  Features.push_back("+tpidr-el2");
+else if (Mtpidr == "el1")
+  Features.push_back("+tpidr-el1");
+else if (Mtpidr != "el0")
+  D.Diag(diag::err_drv_invalid_mtpidr) << A->getAsString(Args);
+  }
+
   // En/disable crc
   if (Arg *A = Args.getLastArg(options::OPT_mcrc, options::OPT_mnocrc)) {
 if (A->getOption().matches(options::OPT_mcrc))
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2152,6 +2152,8 @@
 foreach i = {1-7,9-15,18,20-28} in
   def ffixed_x#i : Flag<["-"], "ffixed-x"#i>, Group,
 HelpText<"Reserve the "#i#" register (AArch64 only)">;
+def mtpidr_EQ : Joined<["-"], "mtpidr=">, Group, 
Values<"el0,el1,el2,el3">,
+  HelpText<"Read thread pointer for specified exception level (AArch64 only)">;
 
 foreach i = {8-15,18} in
   def fcall_saved_x#i : Flag<["-"], "fcall-saved-x"#i>, 
Group,
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -118,6 +118,8 @@
   "invalid thread pointer reading mode '%0'">;
 def err_drv_missing_arg_mtp : Error<
   "missing argument to '%0'">;
+def err_drv_invalid_mtpidr : Error<
+  "invalid thread pointer exception level '%0'">;
 def err_drv_invalid_libcxx_deployment : Error<
   "invalid deployment target for -stdlib=libc++ (requires %0 or later)">;
 def err_drv_invalid_argument_to_fdebug_prefix_map : Error<


Index: clang/test/Driver/clang-translation.c

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

2019-03-20 Thread John McCall via Phabricator via cfe-commits
rjmccall requested changes to this revision.
rjmccall added inline comments.
This revision now requires changes to proceed.



Comment at: include/clang/Basic/AttrDocs.td:1118
+let Content = [{
+This attribute specifies that the Objective-C class to which it applies has 
dynamically-allocated metadata. Classes annotated with this attribute cannot be 
subclassed.
+}];

You should probably check that the user doesn't try to subclass classes 
annotated with this attribute, then. :)



Comment at: lib/CodeGen/CGObjCMac.cpp:735
+  llvm::Constant *getLoadClassrefFn() const {
+// FIXME: Other attributes?
+

It should be safe to make it `readnone`, which could theoretically optimize 
something like doing a class message-send in a loop.



Comment at: lib/CodeGen/CGObjCMac.cpp:7274
+// Classrefs pointing at Objective-C stub classes have the least
+// significant bit set to 1.
+auto *Tag = llvm::ConstantInt::get(CGM.IntPtrTy, 1);

This isn't for an arbitrary class ref, it's for the global class list.  I'd say 
something like "the global class list sets the LSB to 1 on any class stubs".



Comment at: lib/CodeGen/CGObjCMac.cpp:7278
+ClassGV = llvm::ConstantExpr::getIntToPtr(ClassGV,
+  ObjCTypes.Int8PtrTy);
+  }

It's more typical to do this with a GEP, although the add should still work.



Comment at: lib/Sema/SemaDeclAttr.cpp:7126
 break;
+  case ParsedAttr::AT_ObjCClassStub:
+handleSimpleAttribute(S, D, AL);

You should emit an error if `!getLangOpts().ObjCRuntime.allowsClassStubs()`, 
which should be straightforward to implement.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59628



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


[PATCH] D59629: [clang-format] correctly format protobuf fields named "enum".

2019-03-20 Thread Donald Chai via Phabricator via cfe-commits
dchai created this revision.
dchai added a reviewer: krasimir.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Similar to TypeScript, "enum" is not a reserved word.


Repository:
  rC Clang

https://reviews.llvm.org/D59629

Files:
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTestProto.cpp


Index: unittests/Format/FormatTestProto.cpp
===
--- unittests/Format/FormatTestProto.cpp
+++ unittests/Format/FormatTestProto.cpp
@@ -107,6 +107,12 @@
"};");
 }
 
+TEST_F(FormatTestProto, EnumAsFieldName) {
+  verifyFormat("message SomeMessage {\n"
+   "  required int32 enum = 1;\n"
+   "}");
+}
+
 TEST_F(FormatTestProto, UnderstandsReturns) {
   verifyFormat("rpc Search(SearchRequest) returns (SearchResponse);");
 }
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -2018,6 +2018,10 @@
   FormatTok->isOneOf(tok::colon, tok::question))
 return false;
 
+  // In protobuf, "enum" can be used as a field name.
+  if (Style.Language == FormatStyle::LK_Proto && FormatTok->is(tok::equal))
+return false;
+
   // Eat up enum class ...
   if (FormatTok->Tok.is(tok::kw_class) || FormatTok->Tok.is(tok::kw_struct))
 nextToken();


Index: unittests/Format/FormatTestProto.cpp
===
--- unittests/Format/FormatTestProto.cpp
+++ unittests/Format/FormatTestProto.cpp
@@ -107,6 +107,12 @@
"};");
 }
 
+TEST_F(FormatTestProto, EnumAsFieldName) {
+  verifyFormat("message SomeMessage {\n"
+   "  required int32 enum = 1;\n"
+   "}");
+}
+
 TEST_F(FormatTestProto, UnderstandsReturns) {
   verifyFormat("rpc Search(SearchRequest) returns (SearchResponse);");
 }
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -2018,6 +2018,10 @@
   FormatTok->isOneOf(tok::colon, tok::question))
 return false;
 
+  // In protobuf, "enum" can be used as a field name.
+  if (Style.Language == FormatStyle::LK_Proto && FormatTok->is(tok::equal))
+return false;
+
   // Eat up enum class ...
   if (FormatTok->Tok.is(tok::kw_class) || FormatTok->Tok.is(tok::kw_struct))
 nextToken();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

2019-03-20 Thread Ronald Wampler via Phabricator via cfe-commits
rdwampler added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2976
+
+return Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_None ||
+   Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_Inline ||

klimek wrote:
> If SLS_All is supposed to not change anything, should we fall through here if 
> (SLS_All)?
Yes, I think that's a better approach to ensure the current behavior is 
unchanged.


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

https://reviews.llvm.org/D57687



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


[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

2019-03-20 Thread Ronald Wampler via Phabricator via cfe-commits
rdwampler updated this revision to Diff 191629.
rdwampler marked 2 inline comments as done.
rdwampler added a comment.

Changes since last revision:

-rebased 
-Fall through when SLS_All.


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

https://reviews.llvm.org/D57687

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12239,6 +12239,43 @@
   "> {\n"
   "  //\n"
   "});");
+
+  FormatStyle DoNotMerge = getLLVMStyle();
+  DoNotMerge.AllowShortLambdasOnASingleLine = FormatStyle::SLS_None;
+  verifyFormat("auto c = []() {\n"
+   "  return b;\n"
+   "};",
+   "auto c = []() { return b; };", DoNotMerge);
+  verifyFormat("auto c = []() {\n"
+   "};",
+   " auto c = []() {};", DoNotMerge);
+
+  FormatStyle MergeEmptyOnly = getLLVMStyle();
+  MergeEmptyOnly.AllowShortLambdasOnASingleLine = FormatStyle::SLS_Empty;
+  verifyFormat("auto c = []() {\n"
+   "  return b;\n"
+   "};",
+   "auto c = []() {\n"
+   "  return b;\n"
+   " };",
+   MergeEmptyOnly);
+  verifyFormat("auto c = []() {};",
+   "auto c = []() {\n"
+   "};",
+   MergeEmptyOnly);
+
+  FormatStyle MergeInline = getLLVMStyle();
+  MergeInline.AllowShortLambdasOnASingleLine = FormatStyle::SLS_Inline;
+  verifyFormat("auto c = []() {\n"
+   "  return b;\n"
+   "};",
+   "auto c = []() { return b; };", MergeInline);
+  verifyFormat("function([]() { return b; })", "function([]() { return b; })",
+   MergeInline);
+  verifyFormat("function([]() { return b; }, a)",
+   "function([]() { return b; }, a)", MergeInline);
+  verifyFormat("function(a, []() { return b; })",
+   "function(a, []() { return b; })", MergeInline);
 }
 
 TEST_F(FormatTest, EmptyLinesInLambdas) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1475,6 +1475,7 @@
   return true;
 }
   }
+  FormatTok->Type = TT_LambdaLBrace;
   LSquare.Type = TT_LambdaLSquare;
   parseChildBlock();
   return true;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1145,11 +1145,11 @@
 
 // Reset token type in case we have already looked at it and then
 // recovered from an error (e.g. failure to find the matching >).
-if (!CurrentToken->isOneOf(TT_LambdaLSquare, TT_ForEachMacro,
-   TT_FunctionLBrace, TT_ImplicitStringLiteral,
-   TT_InlineASMBrace, TT_JsFatArrow, TT_LambdaArrow,
-   TT_OverloadedOperator, TT_RegexLiteral,
-   TT_TemplateString, TT_ObjCStringLiteral))
+if (!CurrentToken->isOneOf(
+TT_LambdaLSquare, TT_LambdaLBrace, TT_ForEachMacro,
+TT_FunctionLBrace, TT_ImplicitStringLiteral, TT_InlineASMBrace,
+TT_JsFatArrow, TT_LambdaArrow, TT_OverloadedOperator,
+TT_RegexLiteral, TT_TemplateString, TT_ObjCStringLiteral))
   CurrentToken->Type = TT_Unknown;
 CurrentToken->Role.reset();
 CurrentToken->MatchingParen = nullptr;
@@ -2849,7 +2849,7 @@
 // Returns 'true' if 'Tok' is a brace we'd want to break before in Allman style.
 static bool isAllmanBrace(const FormatToken ) {
   return Tok.is(tok::l_brace) && Tok.BlockKind == BK_Block &&
- !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral);
+ !Tok.isOneOf(TT_ObjCBlockLBrace, TT_LambdaLBrace, TT_DictLiteral);
 }
 
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine ,
@@ -2977,6 +2977,19 @@
   if (Left.is(TT_ObjCBlockLBrace) && !Style.AllowShortBlocksOnASingleLine)
 return true;
 
+  if (Left.is(TT_LambdaLBrace)) {
+if (Left.MatchingParen && Left.MatchingParen->Next &&
+Left.MatchingParen->Next->isOneOf(tok::comma, tok::r_paren) &&
+Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_Inline)
+  return false;
+
+if (Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_None ||
+Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_Inline ||
+(!Left.Children.empty() &&
+ Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_Empty))
+  

[PATCH] D58121: [analyzer][WIP] Attempt to fix traversing bindings of non-base regions in ClusterAnalysis

2019-03-20 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

Ok, got it! Yeah, this sounds like a valid way of supporting 
non-base-region-based worklist items, i'm seeing no problems with it and i 
don't immediately see a solution that'd be better than gradually supporting 
non-base-regions in more and more places.

Here's the slight modification of the original test case (D57230#1389766 
) that my problem finally boiled down 
to:

  typedef __typeof(sizeof(int)) size_t;
  void *malloc(size_t);
  
  struct S {
int x; // Make sure ptr has a non-zero offset within baseR.
int *ptr;
  };
  
  struct T {
struct S s; // This is baseR.
  };
  
  void escape(struct S *);
  
  void foo() {
struct T t; // This is realBaseR.
t.s.ptr = malloc(sizeof(int));
escape();
  }

This happens because in this patch you're pin-pointing the value by trying to 
look it up via an exact binding key (up to default/direct), however it may also 
reside at a non-zero offset within `baseR`. In my case the offset was also 
symbolic - which kept me confused for a while because i was thinking that 
symbolic offsets are the root of all evil. I think you may be able to 
generalize the patch to cover this scenario by replacing the exact offset 
lookup with a `collectSubRegionBindings()` (it might be cute to make some sort 
of `iterSubRegionBindings()` that takes a lambda, and/or combine it with 
`iterBindings()`).

I'm still worried about D57230#1434161 
 though. It kinda sounds like a good 
idea to suppress store invalidation but not escaping, but it's a very annoying 
thing to implement because our invalidation traits are very inflexible. In 
order to express complex requirements like "please don't invalidate this 
region, except this sub-region", the whole data structure has to be changed. 
This is slightly similar to this problem we have in:

  void escape(int *x, const int *y);
  // ...
  escape(, );

where the value of `a` would not be invalidated because `y` is a const pointer, 
even though it is passed through a non-const pointer `x`. @xazax.hun, do you 
have enough time/inspiration to dig that deeply into this stuff?


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

https://reviews.llvm.org/D58121



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


[PATCH] D59628: Add support for __attribute__((objc_class_stub))

2019-03-20 Thread Slava Pestov via Phabricator via cfe-commits
slavapestov created this revision.
slavapestov added reviewers: rjmccall, dexonsmith, aaron.ballman.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
dexonsmith edited reviewers, added: erik.pilkington, arphaman; removed: 
dexonsmith.
dexonsmith added a comment.

+Erik and Alex, can you take a look at this?


This CL lands the Clang-side support for a new Objective-C runtime feature that 
adds support for dynamically-allocated metadata.

What follows is a basic description of this mechanism:

In Swift, the class metadata contains an Objective-C class object as a prefix, 
followed by a vtable, generic parameters, and field offsets. If the inheritance 
hierarchy of a class crosses a resilience boundary, we cannot statically emit 
the entire metadata. Instead, it is lazily built at runtime when first needed.

In order for such classes to be accessed from Clang, the Objective-C runtime 
will support a new mechanism whereby Swift can instead emit a "class stub" 
object. The class stub object is used by Clang in places where a class 
reference is normally emitted:

- Classrefs emitted when messaging the class
- Categories

A class stub contains an "isa pointer" which is a scalar value 1 followed by a 
function pointer which realizes the real class metadata. A reference to a class 
stub from a classref also has its least significant pointer set to 1.

The Objective-C runtime distinguishes a class stub from a real class using this 
fake "isa pointer". Instead of directly loading from a classref, Clang instead 
calls `objc_loadClassref()`; this function checks if the classref contains a 
class stub, and if so, it calls the function pointer and stores the result back 
into the classref to fast path future loads.

Note that Clang already supports a objc_runtime_visible attribute. The 
objc_class_stub attribute can be thought of as a generalization of 
objc_runtime_visible. Whereas objc_runtime_visible replaces the classref load 
with a runtime lookup of a class by string, the class stub mechanism is more 
efficient because the result of the lookup is cached. Also, 
objc_runtime_visible does not support categories to be defined on the class, 
whereas objc_class_stub does.


Repository:
  rC Clang

https://reviews.llvm.org/D59628

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  lib/CodeGen/CGObjCMac.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGenObjC/class-stubs.m

Index: test/CodeGenObjC/class-stubs.m
===
--- /dev/null
+++ test/CodeGenObjC/class-stubs.m
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -Wno-objc-root-class -emit-llvm -o - %s | \
+// RUN: FileCheck %s
+
+__attribute__((objc_class_stub))
+@interface A
++ (void) classMethod;
+@end
+
+int main() {
+  [A classMethod];
+}
+
+// CHECK-LABEL: @"OBJC_CLASSLIST_REFERENCES_$_" = private global i8* inttoptr (i64 add (i64 ptrtoint (%struct._class_t* @"OBJC_CLASS_$_A" to i64), i64 1) to i8*), section "__DATA,__objc_classrefs,regular,no_dead_strip", align 8
+
+
+// CHECK-LABEL: define i32 @main()
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   [[CLASS:%.*]] = call %struct._class_t* @objc_loadClassref(i8** @"OBJC_CLASSLIST_REFERENCES_$_")
+// CHECK-NEXT:   [[SELECTOR:%.*]] = load i8*, i8** @OBJC_SELECTOR_REFERENCES_
+// CHECK-NEXT:   [[RECEIVER:%.*]] = bitcast %struct._class_t* [[CLASS]] to i8*
+// CHECK-NEXT:   call void bitcast (i8* (i8*, i8*, ...)* @objc_msgSend to void (i8*, i8*)*)(i8* [[RECEIVER]], i8* [[SELECTOR]])
+// CHECK-NEXT:   ret i32 0
+
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -7123,6 +7123,9 @@
   case ParsedAttr::AT_ObjCSubclassingRestricted:
 handleSimpleAttribute(S, D, AL);
 break;
+  case ParsedAttr::AT_ObjCClassStub:
+handleSimpleAttribute(S, D, AL);
+break;
   case ParsedAttr::AT_ObjCCompleteDefinition:
 handleSimpleAttribute(S, D, AL);
 break;
Index: lib/CodeGen/CGObjCMac.cpp
===
--- lib/CodeGen/CGObjCMac.cpp
+++ lib/CodeGen/CGObjCMac.cpp
@@ -726,6 +726,25 @@
  "objc_begin_catch");
   }
 
+  /// Class objc_loadClassref (void *)
+  ///
+  /// Loads from a classref. For Objective-C stub classes, this invokes the
+  /// initialization callback stored inside the stub. For all other classes
+  /// this simply dereferences the pointer.
+  llvm::Constant *getLoadClassrefFn() const {
+// FIXME: Other attributes?
+
+// Add the non-lazy-bind attribute, since objc_loadClassref is likely to
+// be called a lot.
+llvm::Type *params[] = { Int8PtrPtrTy };
+return CGM.CreateRuntimeFunction(
+llvm::FunctionType::get(ClassnfABIPtrTy, params, false),
+"objc_loadClassref",
+llvm::AttributeList::get(CGM.getLLVMContext(),
+ 

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

2019-03-20 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith edited reviewers, added: erik.pilkington, arphaman; removed: 
dexonsmith.
dexonsmith added a comment.

+Erik and Alex, can you take a look at this?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59628



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


[PATCH] D59567: [X86] Add __popcntd and __popcntq to ia32intrin.h to match gcc and icc. Remove popcnt feature flag from _popcnt32/_popcnt64 and move to ia32intrin.h to match gcc

2019-03-20 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added a comment.

In D59567#1436035 , @RKSimon wrote:

> IIRC we don't use libcalls for popcnt - we just expand


Yeah we expand. gcc uses libcall. I think icc also expands.


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

https://reviews.llvm.org/D59567



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


[PATCH] D59627: [clang-format] Keep protobuf "package" statement on one line

2019-03-20 Thread Donald Chai via Phabricator via cfe-commits
dchai created this revision.
dchai added a reviewer: sammccall.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Top-level "package" and "import" statements should generally be kept on one
line, for all languages.


Repository:
  rC Clang

https://reviews.llvm.org/D59627

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


Index: unittests/Format/FormatTestProto.cpp
===
--- unittests/Format/FormatTestProto.cpp
+++ unittests/Format/FormatTestProto.cpp
@@ -387,6 +387,12 @@
"};");
 }
 
+TEST_F(FormatTestProto, DoesntWrapPackageStatements) {
+  verifyFormat(
+  "package"
+  " some.really.long.package.that.exceeds.the.column.limit;");
+}
+
 TEST_F(FormatTestProto, FormatsService) {
   verifyFormat("service SearchService {\n"
"  rpc Search(SearchRequest) returns (SearchResponse) {\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1073,10 +1073,11 @@
   return LT_ImportStatement;
 }
 
-// In .proto files, top-level options are very similar to import statements
-// and should not be line-wrapped.
+// In .proto files, top-level options and package statements are very
+// similar to import statements and should not be line-wrapped.
 if (Style.Language == FormatStyle::LK_Proto && Line.Level == 0 &&
-CurrentToken->is(Keywords.kw_option)) {
+(CurrentToken->is(Keywords.kw_option) ||
+ CurrentToken->is(Keywords.kw_package))) {
   next();
   if (CurrentToken && CurrentToken->is(tok::identifier))
 return LT_ImportStatement;


Index: unittests/Format/FormatTestProto.cpp
===
--- unittests/Format/FormatTestProto.cpp
+++ unittests/Format/FormatTestProto.cpp
@@ -387,6 +387,12 @@
"};");
 }
 
+TEST_F(FormatTestProto, DoesntWrapPackageStatements) {
+  verifyFormat(
+  "package"
+  " some.really.long.package.that.exceeds.the.column.limit;");
+}
+
 TEST_F(FormatTestProto, FormatsService) {
   verifyFormat("service SearchService {\n"
"  rpc Search(SearchRequest) returns (SearchResponse) {\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1073,10 +1073,11 @@
   return LT_ImportStatement;
 }
 
-// In .proto files, top-level options are very similar to import statements
-// and should not be line-wrapped.
+// In .proto files, top-level options and package statements are very
+// similar to import statements and should not be line-wrapped.
 if (Style.Language == FormatStyle::LK_Proto && Line.Level == 0 &&
-CurrentToken->is(Keywords.kw_option)) {
+(CurrentToken->is(Keywords.kw_option) ||
+ CurrentToken->is(Keywords.kw_package))) {
   next();
   if (CurrentToken && CurrentToken->is(tok::identifier))
 return LT_ImportStatement;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D59567: [X86] Add __popcntd and __popcntq to ia32intrin.h to match gcc and icc. Remove popcnt feature flag from _popcnt32/_popcnt64 and move to ia32intrin.h to match gcc

2019-03-20 Thread Craig Topper via Phabricator via cfe-commits
craig.topper updated this revision to Diff 191625.
craig.topper added a comment.

Update doxygen comments


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

https://reviews.llvm.org/D59567

Files:
  lib/Headers/ia32intrin.h
  lib/Headers/popcntintrin.h
  test/CodeGen/popcnt-builtins.c

Index: test/CodeGen/popcnt-builtins.c
===
--- test/CodeGen/popcnt-builtins.c
+++ test/CodeGen/popcnt-builtins.c
@@ -1,24 +1,39 @@
-// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-apple-darwin -target-feature +popcnt -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-apple-darwin -target-feature +popcnt -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-POPCNT
+// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-apple-darwin -emit-llvm -o - | FileCheck %s
 
 
-#include 
+#include 
 
-unsigned int test_mm_popcnt_u32(unsigned int __X) {
-  //CHECK: call i32 @llvm.ctpop.i32
+#ifdef __POPCNT__
+int test_mm_popcnt_u32(unsigned int __X) {
+  //CHECK-POPCNT: call i32 @llvm.ctpop.i32
   return _mm_popcnt_u32(__X);
 }
+#endif
 
-unsigned int test_popcnt_32(int __X) {
+int test_popcnt32(unsigned int __X) {
   //CHECK: call i32 @llvm.ctpop.i32
   return _popcnt32(__X);
 }
 
-unsigned long long test_mm_popcnt_u64(unsigned long long __X) {
-  //CHECK: call i64 @llvm.ctpop.i64
+int test__popcntd(unsigned int __X) {
+  //CHECK: call i32 @llvm.ctpop.i32
+  return __popcntd(__X);
+}
+
+#ifdef __POPCNT__
+long long test_mm_popcnt_u64(unsigned long long __X) {
+  //CHECK-POPCNT: call i64 @llvm.ctpop.i64
   return _mm_popcnt_u64(__X);
 }
+#endif
 
-unsigned long long test_popcnt_64(long long __X) {
+long long test_popcnt64(unsigned long long __X) {
   //CHECK: call i64 @llvm.ctpop.i64
   return _popcnt64(__X);
 }
+
+long long test__popcntq(unsigned long long __X) {
+  //CHECK: call i64 @llvm.ctpop.i64
+  return __popcntq(__X);
+}
Index: lib/Headers/popcntintrin.h
===
--- lib/Headers/popcntintrin.h
+++ lib/Headers/popcntintrin.h
@@ -43,22 +43,6 @@
   return __builtin_popcount(__A);
 }
 
-/// Counts the number of bits in the source operand having a value of 1.
-///
-/// \headerfile 
-///
-/// This intrinsic corresponds to the  POPCNT  instruction.
-///
-/// \param __A
-///A signed 32-bit integer operand.
-/// \returns A 32-bit integer containing the number of bits with value 1 in the
-///source operand.
-static __inline__ int __DEFAULT_FN_ATTRS
-_popcnt32(int __A)
-{
-  return __builtin_popcount(__A);
-}
-
 #ifdef __x86_64__
 /// Counts the number of bits in the source operand having a value of 1.
 ///
@@ -75,22 +59,6 @@
 {
   return __builtin_popcountll(__A);
 }
-
-/// Counts the number of bits in the source operand having a value of 1.
-///
-/// \headerfile 
-///
-/// This intrinsic corresponds to the  POPCNT  instruction.
-///
-/// \param __A
-///A signed 64-bit integer operand.
-/// \returns A 64-bit integer containing the number of bits with value 1 in the
-///source operand.
-static __inline__ long long __DEFAULT_FN_ATTRS
-_popcnt64(long long __A)
-{
-  return __builtin_popcountll(__A);
-}
 #endif /* __x86_64__ */
 
 #undef __DEFAULT_FN_ATTRS
Index: lib/Headers/ia32intrin.h
===
--- lib/Headers/ia32intrin.h
+++ lib/Headers/ia32intrin.h
@@ -28,6 +28,48 @@
 #ifndef __IA32INTRIN_H
 #define __IA32INTRIN_H
 
+/** Counts the number of bits in the source operand having a value of 1.
+ *
+ *  \headerfile 
+ *
+ *  This intrinsic corresponds to the  POPCNT  instruction or a
+ *  a sequence of arithmetic and logic ops to calcute it.
+ *
+ *  \param __A
+ * An unsigned 32-bit integer operand.
+ *  \returns A 32-bit integer containing the number of bits with value 1 in the
+ * source operand.
+ */
+static __inline__ int __attribute__((__always_inline__, __nodebug__))
+__popcntd(unsigned int __A)
+{
+  return __builtin_popcount(__A);
+}
+
+#define _popcnt32(A) __popcntd((A))
+
+#ifdef __x86_64__
+/** Counts the number of bits in the source operand having a value of 1.
+ *
+ *  \headerfile 
+ *
+ *  This intrinsic corresponds to the  POPCNT  instruction or a
+ *  a sequence of arithmetic and logic ops to calcute it.
+ *
+ *  \param __A
+ * An unsigned 64-bit integer operand.
+ *  \returns A 64-bit integer containing the number of bits with value 1 in the
+ * source operand.
+ */
+static __inline__ long long __attribute__((__always_inline__, __nodebug__))
+__popcntq(unsigned long long __A)
+{
+  return __builtin_popcountll(__A);
+}
+
+#define _popcnt64(A) __popcntq((A))
+#endif /* __x86_64__ */
+
 #ifdef __x86_64__
 static __inline__ unsigned long long __attribute__((__always_inline__, __nodebug__))
 __readeflags(void)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

r356628 - Remove extra white spaces

2019-03-20 Thread Jennifer Yu via cfe-commits
Author: jyu2
Date: Wed Mar 20 16:05:18 2019
New Revision: 356628

URL: http://llvm.org/viewvc/llvm-project?rev=356628=rev
Log:
Remove extra white spaces

Modified:
cfe/trunk/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp

Modified: cfe/trunk/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp?rev=356628=356627=356628=diff
==
--- cfe/trunk/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp Wed Mar 20 16:05:18 
2019
@@ -293,7 +293,7 @@ struct class_0 : class_1 {
 };
 
 class_0::class_0() {
-  // WIN32: define dso_local x86_thiscallcc %struct.class_0* 
@"??0class_0@@QAE@XZ"(%struct.class_0* returned %this, i32 %is_most_derived) 
+  // WIN32: define dso_local x86_thiscallcc %struct.class_0* 
@"??0class_0@@QAE@XZ"(%struct.class_0* returned %this, i32 %is_most_derived)
   // WIN32: store i32 %is_most_derived, i32* %[[IS_MOST_DERIVED_VAR:.*]], 
align 4
   // WIN32: %[[IS_MOST_DERIVED_VAL:.*]] = load i32, i32* 
%[[IS_MOST_DERIVED_VAR]]
   // WIN32: %[[SHOULD_CALL_VBASE_CTORS:.*]] = icmp ne i32 
%[[IS_MOST_DERIVED_VAL]], 0
@@ -304,7 +304,7 @@ class_0::class_0() {
 // ehcleanup:
   // WIN32: %[[CLEANUPPAD:.*]] = cleanuppad within none []
   // WIN32-NEXT: bitcast %{{.*}}* %{{.*}} to i8*
-  // WIN32-NEXT: getelementptr inbounds i8, i8* %{{.*}}, i{{.*}} {{.}} 
+  // WIN32-NEXT: getelementptr inbounds i8, i8* %{{.*}}, i{{.*}} {{.}}
   // WIN32-NEXT: bitcast i8* %{{.*}} to %{{.*}}*
   // WIN32-NEXT: %[[SHOULD_CALL_VBASE_DTOR:.*]] = icmp ne i32 
%[[IS_MOST_DERIVED_VAL]], 0
   // WIN32-NEXT: br i1 %[[SHOULD_CALL_VBASE_DTOR]], label %[[DTOR_VBASE:.*]], 
label %[[SKIP_VBASE:.*]]


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


[PATCH] D59440: add steps to preprocess file and reduce command line args

2019-03-20 Thread George Burgess IV via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC356636: creduce-clang-crash.py: preprocess file + reduce 
commandline (authored by gbiv, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D59440?vs=191598=191615#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D59440

Files:
  utils/creduce-clang-crash.py

Index: utils/creduce-clang-crash.py
===
--- utils/creduce-clang-crash.py
+++ utils/creduce-clang-crash.py
@@ -1,7 +1,5 @@
 #!/usr/bin/env python
 """Calls C-Reduce to create a minimal reproducer for clang crashes.
-
-Requires C-Reduce and not (part of LLVM utils) to be installed.
 """
 
 from argparse import ArgumentParser
@@ -11,108 +9,232 @@
 import sys
 import subprocess
 import pipes
+import shlex
+import tempfile
+import shutil
 from distutils.spawn import find_executable
 
-def create_test(build_script, llvm_not):
+verbose = False
+llvm_bin = None
+creduce_cmd = None
+not_cmd = None
+
+def check_file(fname):
+  if not os.path.isfile(fname):
+sys.exit("ERROR: %s does not exist" % (fname))
+  return fname
+
+def check_cmd(cmd_name, cmd_dir, cmd_path=None):
   """
-  Create an interestingness test from the crash output.
-  Return as a string.
+  Returns absolute path to cmd_path if it is given,
+  or absolute path to cmd_dir/cmd_name.
   """
-  # Get clang call from build script
-  # Assumes the call is the last line of the script
-  with open(build_script) as f:
-cmd = f.readlines()[-1].rstrip('\n\r')
+  if cmd_path:
+cmd = find_executable(cmd_path)
+if cmd:
+  return cmd
+sys.exit("ERROR: executable %s not found" % (cmd_path))
+
+  cmd = find_executable(cmd_name, path=cmd_dir)
+  if cmd:
+return cmd
+  sys.exit("ERROR: %s not found in %s" % (cmd_name, cmd_dir))
+
+def quote_cmd(cmd):
+  return ' '.join(arg if arg.startswith('$') else pipes.quote(arg)
+  for arg in cmd)
+
+def get_crash_cmd(crash_script):
+  with open(crash_script) as f:
+# Assume clang call is on the last line of the script
+line = f.readlines()[-1]
+cmd = shlex.split(line)
+
+# Overwrite the script's clang with the user's clang path
+new_clang = check_cmd('clang', llvm_bin)
+cmd[0] = pipes.quote(new_clang)
+return cmd
 
-  # Get crash output
-  p = subprocess.Popen(build_script,
+def has_expected_output(crash_cmd, expected_output):
+  p = subprocess.Popen(crash_cmd,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT)
   crash_output, _ = p.communicate()
+  return all(msg in crash_output for msg in expected_output)
 
-  output = ['#!/bin/bash']
-  output.append('%s --crash %s >& t.log || exit 1' % (pipes.quote(llvm_not),
-  cmd))
-
-  # Add messages from crash output to the test
-  # If there is an Assertion failure, use that; otherwise use the
-  # last five stack trace functions
+def get_expected_output(crash_cmd):
+  p = subprocess.Popen(crash_cmd,
+   stdout=subprocess.PIPE,
+   stderr=subprocess.STDOUT)
+  crash_output, _ = p.communicate()
+
+  # If there is an assertion failure, use that;
+  # otherwise use the last five stack trace functions
   assertion_re = r'Assertion `([^\']+)\' failed'
   assertion_match = re.search(assertion_re, crash_output)
   if assertion_match:
-msg = assertion_match.group(1)
-output.append('grep %s t.log || exit 1' % pipes.quote(msg))
+return [assertion_match.group(1)]
   else:
 stacktrace_re = r'#[0-9]+\s+0[xX][0-9a-fA-F]+\s*([^(]+)\('
 matches = re.findall(stacktrace_re, crash_output)
-del matches[:-5]
-output += ['grep %s t.log || exit 1' % pipes.quote(msg) for msg in matches]
+return matches[-5:]
+
+def write_interestingness_test(testfile, crash_cmd, expected_output,
+   file_to_reduce):
+  filename = os.path.basename(file_to_reduce)
+  if filename not in crash_cmd:
+sys.exit("ERROR: expected %s to be in the crash command" % filename)
+
+  # Replace all instances of file_to_reduce with a command line variable
+  output = ['#!/bin/bash',
+'if [ -z "$1" ] ; then',
+'  f=%s' % (pipes.quote(filename)),
+'else',
+'  f="$1"',
+'fi']
+  cmd = ['$f' if s == filename else s for s in crash_cmd]
+
+  output.append('%s --crash %s >& t.log || exit 1' % (pipes.quote(not_cmd),
+  quote_cmd(cmd)))
+
+  for msg in expected_output:
+output.append('grep %s t.log || exit 1' % pipes.quote(msg))
+
+  with open(testfile, 'w') as f:
+f.write('\n'.join(output))
+  os.chmod(testfile, os.stat(testfile).st_mode | stat.S_IEXEC)
 
-  return output
+def check_interestingness(testfile, file_to_reduce):
+  testfile = os.path.abspath(testfile)
+

r356636 - creduce-clang-crash.py: preprocess file + reduce commandline

2019-03-20 Thread George Burgess IV via cfe-commits
Author: gbiv
Date: Wed Mar 20 18:01:53 2019
New Revision: 356636

URL: http://llvm.org/viewvc/llvm-project?rev=356636=rev
Log:
creduce-clang-crash.py: preprocess file + reduce commandline

This CL causes our creduce-clang-crash.py util to:

- try to preprocess the file before reducing
- try to remove some command line arguments
- now require a llvm bin directory, since the generated crash script
  doesn't have an absolute path for clang

It also marks it as executable, since I forgot to do that in the last
commit. :)

Patch by Amy Huang!

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

Modified:
cfe/trunk/utils/creduce-clang-crash.py

Modified: cfe/trunk/utils/creduce-clang-crash.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/creduce-clang-crash.py?rev=356636=356635=356636=diff
==
--- cfe/trunk/utils/creduce-clang-crash.py (original)
+++ cfe/trunk/utils/creduce-clang-crash.py Wed Mar 20 18:01:53 2019
@@ -1,7 +1,5 @@
 #!/usr/bin/env python
 """Calls C-Reduce to create a minimal reproducer for clang crashes.
-
-Requires C-Reduce and not (part of LLVM utils) to be installed.
 """
 
 from argparse import ArgumentParser
@@ -11,108 +9,232 @@ import stat
 import sys
 import subprocess
 import pipes
+import shlex
+import tempfile
+import shutil
 from distutils.spawn import find_executable
 
-def create_test(build_script, llvm_not):
+verbose = False
+llvm_bin = None
+creduce_cmd = None
+not_cmd = None
+
+def check_file(fname):
+  if not os.path.isfile(fname):
+sys.exit("ERROR: %s does not exist" % (fname))
+  return fname
+
+def check_cmd(cmd_name, cmd_dir, cmd_path=None):
   """
-  Create an interestingness test from the crash output.
-  Return as a string.
+  Returns absolute path to cmd_path if it is given,
+  or absolute path to cmd_dir/cmd_name.
   """
-  # Get clang call from build script
-  # Assumes the call is the last line of the script
-  with open(build_script) as f:
-cmd = f.readlines()[-1].rstrip('\n\r')
+  if cmd_path:
+cmd = find_executable(cmd_path)
+if cmd:
+  return cmd
+sys.exit("ERROR: executable %s not found" % (cmd_path))
+
+  cmd = find_executable(cmd_name, path=cmd_dir)
+  if cmd:
+return cmd
+  sys.exit("ERROR: %s not found in %s" % (cmd_name, cmd_dir))
+
+def quote_cmd(cmd):
+  return ' '.join(arg if arg.startswith('$') else pipes.quote(arg)
+  for arg in cmd)
+
+def get_crash_cmd(crash_script):
+  with open(crash_script) as f:
+# Assume clang call is on the last line of the script
+line = f.readlines()[-1]
+cmd = shlex.split(line)
+
+# Overwrite the script's clang with the user's clang path
+new_clang = check_cmd('clang', llvm_bin)
+cmd[0] = pipes.quote(new_clang)
+return cmd
 
-  # Get crash output
-  p = subprocess.Popen(build_script,
+def has_expected_output(crash_cmd, expected_output):
+  p = subprocess.Popen(crash_cmd,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT)
   crash_output, _ = p.communicate()
+  return all(msg in crash_output for msg in expected_output)
 
-  output = ['#!/bin/bash']
-  output.append('%s --crash %s >& t.log || exit 1' % (pipes.quote(llvm_not),
-  cmd))
-
-  # Add messages from crash output to the test
-  # If there is an Assertion failure, use that; otherwise use the
-  # last five stack trace functions
+def get_expected_output(crash_cmd):
+  p = subprocess.Popen(crash_cmd,
+   stdout=subprocess.PIPE,
+   stderr=subprocess.STDOUT)
+  crash_output, _ = p.communicate()
+
+  # If there is an assertion failure, use that;
+  # otherwise use the last five stack trace functions
   assertion_re = r'Assertion `([^\']+)\' failed'
   assertion_match = re.search(assertion_re, crash_output)
   if assertion_match:
-msg = assertion_match.group(1)
-output.append('grep %s t.log || exit 1' % pipes.quote(msg))
+return [assertion_match.group(1)]
   else:
 stacktrace_re = r'#[0-9]+\s+0[xX][0-9a-fA-F]+\s*([^(]+)\('
 matches = re.findall(stacktrace_re, crash_output)
-del matches[:-5]
-output += ['grep %s t.log || exit 1' % pipes.quote(msg) for msg in matches]
+return matches[-5:]
+
+def write_interestingness_test(testfile, crash_cmd, expected_output,
+   file_to_reduce):
+  filename = os.path.basename(file_to_reduce)
+  if filename not in crash_cmd:
+sys.exit("ERROR: expected %s to be in the crash command" % filename)
+
+  # Replace all instances of file_to_reduce with a command line variable
+  output = ['#!/bin/bash',
+'if [ -z "$1" ] ; then',
+'  f=%s' % (pipes.quote(filename)),
+'else',
+'  f="$1"',
+'fi']
+  cmd = ['$f' if s == filename else s for s in crash_cmd]
+
+  output.append('%s --crash %s >& t.log || exit 1' % (pipes.quote(not_cmd),
+ 

[PATCH] D59624: [Driver] Pass -malign-double from the driver to the cc1 command line

2019-03-20 Thread Craig Topper via Phabricator via cfe-commits
craig.topper created this revision.
craig.topper added a reviewer: rnk.

-malign-double is currently only implemented in the -cc1 interface. But its 
declared in Options.td so it is a driver option too. But you try to use it with 
the driver you'll get a message about the option being unused.

This patch teaches the driver to pass the option through to cc1 so it won't be 
unused. The Options.td says the option is x86 only but I didn't see any x86 
specific code in its impementation in cc1 so not sure if the documentation is 
wrong or if I should only pass this option through the driver on x86 targets.


https://reviews.llvm.org/D59624

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/malign_double.c


Index: clang/test/Driver/malign_double.c
===
--- /dev/null
+++ clang/test/Driver/malign_double.c
@@ -0,0 +1,5 @@
+// RUN: %clang -### -malign-double %s  2>&1 | FileCheck %s
+
+// Make sure -malign-double is passed through the driver.
+
+// CHECK: "-malign-double"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4547,6 +4547,7 @@
   Args.AddLastArg(CmdArgs, options::OPT_fdiagnostics_parseable_fixits);
   Args.AddLastArg(CmdArgs, options::OPT_ftime_report);
   Args.AddLastArg(CmdArgs, options::OPT_ftrapv);
+  Args.AddLastArg(CmdArgs, options::OPT_malign_double);
 
   if (Arg *A = Args.getLastArg(options::OPT_ftrapv_handler_EQ)) {
 CmdArgs.push_back("-ftrapv-handler");


Index: clang/test/Driver/malign_double.c
===
--- /dev/null
+++ clang/test/Driver/malign_double.c
@@ -0,0 +1,5 @@
+// RUN: %clang -### -malign-double %s  2>&1 | FileCheck %s
+
+// Make sure -malign-double is passed through the driver.
+
+// CHECK: "-malign-double"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4547,6 +4547,7 @@
   Args.AddLastArg(CmdArgs, options::OPT_fdiagnostics_parseable_fixits);
   Args.AddLastArg(CmdArgs, options::OPT_ftime_report);
   Args.AddLastArg(CmdArgs, options::OPT_ftrapv);
+  Args.AddLastArg(CmdArgs, options::OPT_malign_double);
 
   if (Arg *A = Args.getLastArg(options::OPT_ftrapv_handler_EQ)) {
 CmdArgs.push_back("-ftrapv-handler");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D59621: [libcxx] [test] Add (void) casts to operator new calls, to suppress warnings generated by [[nodiscard]].

2019-03-20 Thread Billy Robert O'Neal III via Phabricator via cfe-commits
BillyONeal added a comment.

This broke all the build bots, so I reverted the SVN change. I thought the 
nodiscard-ness was tested elsewhere but I'm not sure about interaction between 
this and -faligned-allocation.


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

https://reviews.llvm.org/D59621



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


[PATCH] D59573: [analyzer] C++17: PR41142: Ignore transparent InitListExprs when finding construction contexts.

2019-03-20 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL356634: [CFG] [analyzer] pr41142: C++17: Skip transparent 
InitListExprs in constructors. (authored by dergachev, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59573?vs=191423=191611#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59573

Files:
  cfe/trunk/lib/Analysis/CFG.cpp
  cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
  cfe/trunk/test/Analysis/initializer.cpp


Index: cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
===
--- cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
+++ cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
@@ -1043,3 +1043,23 @@
   C c(variadic(0 ? c : 0)); // no-crash
 }
 } // namespace variadic_function_arguments
+
+// CHECK: void testTransparentInitListExprs()
+// CHECK:[B1]
+// CHECK-NEXT: 1: getC
+// CHECK-NEXT: 2: [B1.1] (ImplicitCastExpr, FunctionToPointerDecay, class 
transparent_init_list_exprs::C (*)(void))
+// CXX11-ELIDE-NEXT: 3: [B1.2]() (CXXRecordTypedCall, [B1.4], [B1.5])
+// CXX11-NOELIDE-NEXT: 3: [B1.2]() (CXXRecordTypedCall, [B1.4])
+// CXX11-NEXT: 4: [B1.3]
+// CXX11-NEXT: 5: {[B1.4]} (CXXConstructExpr, [B1.6], class 
transparent_init_list_exprs::C)
+// CXX11-NEXT: 6: transparent_init_list_exprs::C c{getC()};
+// CXX17-NEXT: 3: [B1.2]() (CXXRecordTypedCall, [B1.5])
+// CXX17-NEXT: 4: {[B1.3]}
+// CXX17-NEXT: 5: transparent_init_list_exprs::C c{getC()};
+namespace transparent_init_list_exprs {
+class C {};
+C getC();
+void testTransparentInitListExprs() {
+  C c{getC()};
+}
+} // namespace transparent_init_list_exprs
Index: cfe/trunk/test/Analysis/initializer.cpp
===
--- cfe/trunk/test/Analysis/initializer.cpp
+++ cfe/trunk/test/Analysis/initializer.cpp
@@ -242,4 +242,22 @@
   B & = C({{}}); // no-crash
 #endif
 }
+} // namespace CXX17_aggregate_construction
+
+namespace CXX17_transparent_init_list_exprs {
+class A {};
+
+class B: private A {};
+
+B boo();
+void foo1() {
+  B b { boo() }; // no-crash
+}
+
+class C: virtual public A {};
+
+C coo();
+void foo2() {
+  C c { coo() }; // no-crash
 }
+} // namespace CXX17_transparent_init_list_exprs
Index: cfe/trunk/lib/Analysis/CFG.cpp
===
--- cfe/trunk/lib/Analysis/CFG.cpp
+++ cfe/trunk/lib/Analysis/CFG.cpp
@@ -1378,6 +1378,15 @@
 findConstructionContexts(Layer, CO->getRHS());
 break;
   }
+  case Stmt::InitListExprClass: {
+auto *ILE = cast(Child);
+if (ILE->isTransparent()) {
+  findConstructionContexts(Layer, ILE->getInit(0));
+  break;
+}
+// TODO: Handle other cases. For now, fail to find construction contexts.
+break;
+  }
   default:
 break;
   }


Index: cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
===
--- cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
+++ cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
@@ -1043,3 +1043,23 @@
   C c(variadic(0 ? c : 0)); // no-crash
 }
 } // namespace variadic_function_arguments
+
+// CHECK: void testTransparentInitListExprs()
+// CHECK:[B1]
+// CHECK-NEXT: 1: getC
+// CHECK-NEXT: 2: [B1.1] (ImplicitCastExpr, FunctionToPointerDecay, class transparent_init_list_exprs::C (*)(void))
+// CXX11-ELIDE-NEXT: 3: [B1.2]() (CXXRecordTypedCall, [B1.4], [B1.5])
+// CXX11-NOELIDE-NEXT: 3: [B1.2]() (CXXRecordTypedCall, [B1.4])
+// CXX11-NEXT: 4: [B1.3]
+// CXX11-NEXT: 5: {[B1.4]} (CXXConstructExpr, [B1.6], class transparent_init_list_exprs::C)
+// CXX11-NEXT: 6: transparent_init_list_exprs::C c{getC()};
+// CXX17-NEXT: 3: [B1.2]() (CXXRecordTypedCall, [B1.5])
+// CXX17-NEXT: 4: {[B1.3]}
+// CXX17-NEXT: 5: transparent_init_list_exprs::C c{getC()};
+namespace transparent_init_list_exprs {
+class C {};
+C getC();
+void testTransparentInitListExprs() {
+  C c{getC()};
+}
+} // namespace transparent_init_list_exprs
Index: cfe/trunk/test/Analysis/initializer.cpp
===
--- cfe/trunk/test/Analysis/initializer.cpp
+++ cfe/trunk/test/Analysis/initializer.cpp
@@ -242,4 +242,22 @@
   B & = C({{}}); // no-crash
 #endif
 }
+} // namespace CXX17_aggregate_construction
+
+namespace CXX17_transparent_init_list_exprs {
+class A {};
+
+class B: private A {};
+
+B boo();
+void foo1() {
+  B b { boo() }; // no-crash
+}
+
+class C: virtual public A {};
+
+C coo();
+void foo2() {
+  C c { coo() }; // no-crash
 }
+} // namespace CXX17_transparent_init_list_exprs
Index: cfe/trunk/lib/Analysis/CFG.cpp
===
--- 

r356634 - [CFG] [analyzer] pr41142: C++17: Skip transparent InitListExprs in constructors.

2019-03-20 Thread Artem Dergachev via cfe-commits
Author: dergachev
Date: Wed Mar 20 17:15:07 2019
New Revision: 356634

URL: http://llvm.org/viewvc/llvm-project?rev=356634=rev
Log:
[CFG] [analyzer] pr41142: C++17: Skip transparent InitListExprs in constructors.

When searching for construction contexts, i.e. figuring out which statements
define the object that is constructed by each construct-expression, ignore
transparent init-list expressions because they don't add anything to the
context. This allows the Static Analyzer to model construction, destruction,
materialization, lifetime extension correctly in more cases. Also fixes
a crash caused by incorrectly evaluating initial values of variables
initialized with such expressions.

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

Modified:
cfe/trunk/lib/Analysis/CFG.cpp
cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
cfe/trunk/test/Analysis/initializer.cpp

Modified: cfe/trunk/lib/Analysis/CFG.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=356634=356633=356634=diff
==
--- cfe/trunk/lib/Analysis/CFG.cpp (original)
+++ cfe/trunk/lib/Analysis/CFG.cpp Wed Mar 20 17:15:07 2019
@@ -1378,6 +1378,15 @@ void CFGBuilder::findConstructionContext
 findConstructionContexts(Layer, CO->getRHS());
 break;
   }
+  case Stmt::InitListExprClass: {
+auto *ILE = cast(Child);
+if (ILE->isTransparent()) {
+  findConstructionContexts(Layer, ILE->getInit(0));
+  break;
+}
+// TODO: Handle other cases. For now, fail to find construction contexts.
+break;
+  }
   default:
 break;
   }

Modified: cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cfg-rich-constructors.cpp?rev=356634=356633=356634=diff
==
--- cfe/trunk/test/Analysis/cfg-rich-constructors.cpp (original)
+++ cfe/trunk/test/Analysis/cfg-rich-constructors.cpp Wed Mar 20 17:15:07 2019
@@ -1043,3 +1043,23 @@ void testCrashOnVariadicArgument() {
   C c(variadic(0 ? c : 0)); // no-crash
 }
 } // namespace variadic_function_arguments
+
+// CHECK: void testTransparentInitListExprs()
+// CHECK:[B1]
+// CHECK-NEXT: 1: getC
+// CHECK-NEXT: 2: [B1.1] (ImplicitCastExpr, FunctionToPointerDecay, class 
transparent_init_list_exprs::C (*)(void))
+// CXX11-ELIDE-NEXT: 3: [B1.2]() (CXXRecordTypedCall, [B1.4], [B1.5])
+// CXX11-NOELIDE-NEXT: 3: [B1.2]() (CXXRecordTypedCall, [B1.4])
+// CXX11-NEXT: 4: [B1.3]
+// CXX11-NEXT: 5: {[B1.4]} (CXXConstructExpr, [B1.6], class 
transparent_init_list_exprs::C)
+// CXX11-NEXT: 6: transparent_init_list_exprs::C c{getC()};
+// CXX17-NEXT: 3: [B1.2]() (CXXRecordTypedCall, [B1.5])
+// CXX17-NEXT: 4: {[B1.3]}
+// CXX17-NEXT: 5: transparent_init_list_exprs::C c{getC()};
+namespace transparent_init_list_exprs {
+class C {};
+C getC();
+void testTransparentInitListExprs() {
+  C c{getC()};
+}
+} // namespace transparent_init_list_exprs

Modified: cfe/trunk/test/Analysis/initializer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/initializer.cpp?rev=356634=356633=356634=diff
==
--- cfe/trunk/test/Analysis/initializer.cpp (original)
+++ cfe/trunk/test/Analysis/initializer.cpp Wed Mar 20 17:15:07 2019
@@ -242,4 +242,22 @@ void foo() {
   B & = C({{}}); // no-crash
 #endif
 }
+} // namespace CXX17_aggregate_construction
+
+namespace CXX17_transparent_init_list_exprs {
+class A {};
+
+class B: private A {};
+
+B boo();
+void foo1() {
+  B b { boo() }; // no-crash
 }
+
+class C: virtual public A {};
+
+C coo();
+void foo2() {
+  C c { coo() }; // no-crash
+}
+} // namespace CXX17_transparent_init_list_exprs


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


[PATCH] D59621: [libcxx] [test] Add (void) casts to operator new calls, to suppress warnings generated by [[nodiscard]].

2019-03-20 Thread Billy Robert O'Neal III via Phabricator via cfe-commits
BillyONeal added a comment.

As an FYI, I committed this to subversion as r356632 since I haven't figured 
out the new git world order yet.


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

https://reviews.llvm.org/D59621



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


[PATCH] D59440: add steps to preprocess file and reduce command line args

2019-03-20 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv accepted this revision.
george.burgess.iv added a comment.
This revision is now accepted and ready to land.

LGTM; thanks again!

Will land shortly


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

https://reviews.llvm.org/D59440



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


[PATCH] D59622: [analyzer] C++17: PR41142: Ignore transparent InitListExprs in ExprEngine as well.

2019-03-20 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, 
mikhail.ramalho, Szelethus, baloghadamsoftware, Charusso.
Herald added subscribers: cfe-commits, jdoerfert, dkrupp, donat.nagy, 
a.sidorin, szepet.
Herald added a project: clang.
NoQ added a parent revision: D59573: [analyzer] C++17: PR41142: Ignore 
transparent InitListExprs when finding construction contexts..

When evaluating a transparent `InitListExpr`, eg. `{{X}}}` (when it's 
equivalent to a plain `X`), don't try to think of it as of a compound value of 
type `X` that contains an `X` - just think of it as of `X` itself.

I hope this should take care of the remaining problems that i identified in 
D59573 , for which this patch is a follow-up.

On second thought, there shouldn't be any correctness problems because of 
multiple inheritance. If there's just one object in the initializer list, it's 
always at offset 0. So i don't think there are going to be any other changes in 
the actual behavior - just less crashes.

There's a chance that i missed other cases when a non-aggregate would be 
accidentally represented as a compound value, but i'm not immediately aware of 
such cases.

Add the original test from https://bugs.llvm.org/show_bug.cgi?id=41142 (the 
top-level recursive call would now evaluate into `Unknown` rather than 
`compoundVal{Unknown}` - which is still worse than the correct `Undefined` 
value, but it's at least not crashing). Btw, once we make sure it's 
`Undefined`, we should probably also add a path note indicating that the 
function returns a garbage value because there's no return statement in it on 
the current execution path.


Repository:
  rC Clang

https://reviews.llvm.org/D59622

Files:
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/test/Analysis/initializer.cpp


Index: clang/test/Analysis/initializer.cpp
===
--- clang/test/Analysis/initializer.cpp
+++ clang/test/Analysis/initializer.cpp
@@ -1,7 +1,17 @@
-// RUN: %clang_analyze_cc1 
-analyzer-checker=core,unix.Malloc,cplusplus.NewDeleteLeaks,debug.ExprInspection
 -analyzer-config c++-inlining=constructors -std=c++11 -verify %s
-// RUN: %clang_analyze_cc1 
-analyzer-checker=core,unix.Malloc,cplusplus.NewDeleteLeaks,debug.ExprInspection
 -analyzer-config c++-inlining=constructors -std=c++17 -DCPLUSPLUS17 -verify %s
-// RUN: %clang_analyze_cc1 
-analyzer-checker=core,unix.Malloc,cplusplus.NewDeleteLeaks,debug.ExprInspection
 -analyzer-config c++-inlining=constructors -std=c++11 
-DTEST_INLINABLE_ALLOCATORS -verify %s
-// RUN: %clang_analyze_cc1 
-analyzer-checker=core,unix.Malloc,cplusplus.NewDeleteLeaks,debug.ExprInspection
 -analyzer-config c++-inlining=constructors -std=c++17 -DCPLUSPLUS17 
-DTEST_INLINABLE_ALLOCATORS -verify %s
+// RUN: %clang_analyze_cc1 -w -verify %s\
+// RUN:   -analyzer-checker=core,unix.Malloc,cplusplus.NewDeleteLeaks\
+// RUN:   -analyzer-checker=debug.ExprInspection -std=c++11
+// RUN: %clang_analyze_cc1 -w -verify %s\
+// RUN:   -analyzer-checker=core,unix.Malloc,cplusplus.NewDeleteLeaks\
+// RUN:   -analyzer-checker=debug.ExprInspection -std=c++17
+// RUN: %clang_analyze_cc1 -w -verify %s\
+// RUN:   -analyzer-checker=core,unix.Malloc,cplusplus.NewDeleteLeaks\
+// RUN:   -analyzer-checker=debug.ExprInspection -std=c++11\
+// RUN:   -DTEST_INLINABLE_ALLOCATORS
+// RUN: %clang_analyze_cc1 -w -verify %s\
+// RUN:   -analyzer-checker=core,unix.Malloc,cplusplus.NewDeleteLeaks\
+// RUN:   -analyzer-checker=debug.ExprInspection -std=c++17\
+// RUN:   -DTEST_INLINABLE_ALLOCATORS
 
 void clang_analyzer_eval(bool);
 
@@ -232,7 +242,7 @@
 
   D d = {}; // no-crash
 
-#ifdef CPLUSPLUS17
+#if __cplusplus >= 201703L
   C cd = {{}}; // no-crash
   const C  = {{}}; // no-crash
   C & = {{}}; // no-crash
@@ -260,4 +270,8 @@
 void foo2() {
   C c { coo() }; // no-crash
 }
+
+B foo_recursive() {
+  B b { foo_recursive() };
+}
 } // namespace CXX17_transparent_init_list_exprs
Index: clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -702,7 +702,7 @@
   QualType T = getContext().getCanonicalType(IE->getType());
   unsigned NumInitElements = IE->getNumInits();
 
-  if (!IE->isGLValue() &&
+  if (!IE->isGLValue() && !IE->isTransparent() &&
   (T->isArrayType() || T->isRecordType() || T->isVectorType() ||
T->isAnyComplexType())) {
 llvm::ImmutableList vals = getBasicVals().getEmptySValList();


Index: clang/test/Analysis/initializer.cpp
===
--- clang/test/Analysis/initializer.cpp
+++ clang/test/Analysis/initializer.cpp
@@ -1,7 +1,17 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,cplusplus.NewDeleteLeaks,debug.ExprInspection -analyzer-config 

[PATCH] D59621: [libcxx] [test] Add (void) casts to operator new calls, to suppress warnings generated by [[nodiscard]].

2019-03-20 Thread Billy Robert O'Neal III via Phabricator via cfe-commits
BillyONeal created this revision.
BillyONeal added reviewers: EricWF, mclow.lists, ldionne.
Herald added a subscriber: jdoerfert.

Add (void) casts to operator new calls, to suppress warnings generated by 
[[nodiscard]].

This allows these tests to pass when compiled by MSVC++.


https://reviews.llvm.org/D59621

Files:
  
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size.sh.cpp
  
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size_align.sh.cpp
  
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size_align_nothrow.sh.cpp
  
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size_nothrow.sh.cpp
  
test/std/language.support/support.dynamic/new.delete/new.delete.single/new_size_align.sh.cpp
  
test/std/language.support/support.dynamic/new.delete/new.delete.single/new_size_align_nothrow.sh.cpp


Index: 
test/std/language.support/support.dynamic/new.delete/new.delete.single/new_size_align_nothrow.sh.cpp
===
--- 
test/std/language.support/support.dynamic/new.delete/new.delete.single/new_size_align_nothrow.sh.cpp
+++ 
test/std/language.support/support.dynamic/new.delete/new.delete.single/new_size_align_nothrow.sh.cpp
@@ -21,5 +21,5 @@
 
 int main ()
 {
-::operator new(4, std::align_val_t{4}, std::nothrow);  // expected-warning 
{{ignoring return value of function declared with 'nodiscard' attribute}}
+(void)::operator new(4, std::align_val_t{4}, std::nothrow);
 }
Index: 
test/std/language.support/support.dynamic/new.delete/new.delete.single/new_size_align.sh.cpp
===
--- 
test/std/language.support/support.dynamic/new.delete/new.delete.single/new_size_align.sh.cpp
+++ 
test/std/language.support/support.dynamic/new.delete/new.delete.single/new_size_align.sh.cpp
@@ -21,5 +21,5 @@
 
 int main ()
 {
-::operator new(4, std::align_val_t{4});  // expected-warning {{ignoring 
return value of function declared with 'nodiscard' attribute}}
+(void)::operator new(4, std::align_val_t{4});
 }
Index: 
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size_nothrow.sh.cpp
===
--- 
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size_nothrow.sh.cpp
+++ 
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size_nothrow.sh.cpp
@@ -21,5 +21,5 @@
 
 int main ()
 {
-::operator new[](4, std::nothrow);  // expected-warning {{ignoring return 
value of function declared with 'nodiscard' attribute}}
+(void)::operator new[](4, std::nothrow);
 }
Index: 
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size_align_nothrow.sh.cpp
===
--- 
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size_align_nothrow.sh.cpp
+++ 
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size_align_nothrow.sh.cpp
@@ -21,5 +21,5 @@
 
 int main ()
 {
-::operator new[](4, std::align_val_t{4}, std::nothrow);  // 
expected-warning {{ignoring return value of function declared with 'nodiscard' 
attribute}}
+(void)::operator new[](4, std::align_val_t{4}, std::nothrow);
 }
Index: 
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size_align.sh.cpp
===
--- 
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size_align.sh.cpp
+++ 
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size_align.sh.cpp
@@ -21,5 +21,5 @@
 
 int main ()
 {
-::operator new[](4, std::align_val_t{4});  // expected-warning {{ignoring 
return value of function declared with 'nodiscard' attribute}}
+(void)::operator new[](4, std::align_val_t{4});
 }
Index: 
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size.sh.cpp
===
--- 
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size.sh.cpp
+++ 
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size.sh.cpp
@@ -21,5 +21,5 @@
 
 int main ()
 {
-::operator new[](4);  // expected-warning {{ignoring return value of 
function declared with 'nodiscard' attribute}}
+(void)::operator new[](4);
 }


Index: test/std/language.support/support.dynamic/new.delete/new.delete.single/new_size_align_nothrow.sh.cpp
===
--- test/std/language.support/support.dynamic/new.delete/new.delete.single/new_size_align_nothrow.sh.cpp
+++ test/std/language.support/support.dynamic/new.delete/new.delete.single/new_size_align_nothrow.sh.cpp
@@ -21,5 +21,5 @@
 
 int main ()
 {
-::operator new(4, std::align_val_t{4}, 

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

2019-03-20 Thread S. B. Tam via Phabricator via cfe-commits
cpplearner added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8166
+def err_multiple_likelihood : Error<
+  "there can only be one %0 attribue in any attribute list">;
+def err_mutuably_exclusive_likelihood : Error<

attribue => attribute



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8172
+def note_previous_likelihood : Note<
+  "previously used %0 attribue">;
+

attribue => attribute



Comment at: clang/lib/Parse/ParseStmt.cpp:1329
+  } else if (ElseBranchAttr) {
+if (ThenBranchAttr->getSpelling()[0] == 'l')
+  WhichBranch = IfStmt::ElseBranch;

Should this use `ElseBranchAttr` instead of `ThenBranchAttr`?


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

https://reviews.llvm.org/D59467



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


RE: [clang-tools-extra] r356565 - Reland r356547 after fixing the tests for Linux.

2019-03-20 Thread via cfe-commits
Hi Zinovy,

I have reverted this change in r356630 in order to get the build bots back to 
green.

I was able to reproduce the issue locally on my machine, it appears that your 
use of “import yaml” is not part of the standard python distribution and so is 
not found.

Douglas Yung

From: cfe-commits  On Behalf Of Galina 
Kistanova via cfe-commits
Sent: Wednesday, March 20, 2019 14:03
To: Zinovy Nis 
Cc: cfe-commits 
Subject: Re: [clang-tools-extra] r356565 - Reland r356547 after fixing the 
tests for Linux.

Hello Zinovy,

This commit broke test on the next builder:
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/45557
. . .
Failing Tests (1):
Clang Tools :: clang-tidy/clang-tidy-diff.cpp

Please have a look?

Thanks

Galina


On Wed, Mar 20, 2019 at 8:49 AM Zinovy Nis via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: zinovy.nis
Date: Wed Mar 20 08:50:26 2019
New Revision: 356565

URL: http://llvm.org/viewvc/llvm-project?rev=356565=rev
Log:
Reland r356547 after fixing the tests for Linux.

[clang-tidy] Parallelize clang-tidy-diff.py

This patch has 2 rationales:

- large patches lead to long command lines and often cause max command line 
length restrictions imposed by OS;
- clang-tidy runs on modified files are independent and can be done in 
parallel, the same as done for run-clang-tidy.

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


Modified:
clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py

Modified: clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py?rev=356565=356564=356565=diff
==
--- clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py (original)
+++ clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py Wed Mar 20 
08:50:26 2019
@@ -24,10 +24,90 @@ Example usage for git/svn users:
 """

 import argparse
+import glob
 import json
+import multiprocessing
+import os
 import re
+import shutil
 import subprocess
 import sys
+import tempfile
+import threading
+import traceback
+import yaml
+
+is_py2 = sys.version[0] == '2'
+
+if is_py2:
+import Queue as queue
+else:
+import queue as queue
+
+
+def run_tidy(task_queue, lock, timeout):
+  watchdog = None
+  while True:
+command = task_queue.get()
+try:
+  proc = subprocess.Popen(command,
+  stdout=subprocess.PIPE,
+  stderr=subprocess.PIPE)
+
+  if timeout is not None:
+watchdog = threading.Timer(timeout, proc.kill)
+watchdog.start()
+
+  stdout, stderr = proc.communicate()
+
+  with lock:
+sys.stdout.write(stdout.decode('utf-8') + '\n')
+sys.stdout.flush()
+if stderr:
+  sys.stderr.write(stderr.decode('utf-8') + '\n')
+  sys.stderr.flush()
+except Exception as e:
+  with lock:
+sys.stderr.write('Failed: ' + str(e) + ': '.join(command) + '\n')
+finally:
+  with lock:
+if (not timeout is None) and (not watchdog is None):
+  if not watchdog.is_alive():
+  sys.stderr.write('Terminated by timeout: ' +
+   ' '.join(command) + '\n')
+  watchdog.cancel()
+  task_queue.task_done()
+
+
+def start_workers(max_tasks, tidy_caller, task_queue, lock, timeout):
+  for _ in range(max_tasks):
+t = threading.Thread(target=tidy_caller, args=(task_queue, lock, timeout))
+t.daemon = True
+t.start()
+
+def merge_replacement_files(tmpdir, mergefile):
+  """Merge all replacement files in a directory into a single file"""
+  # The fixes suggested by clang-tidy >= 4.0.0 are given under
+  # the top level key 'Diagnostics' in the output yaml files
+  mergekey = "Diagnostics"
+  merged = []
+  for replacefile in glob.iglob(os.path.join(tmpdir, '*.yaml')):
+content = yaml.safe_load(open(replacefile, 'r'))
+if not content:
+  continue # Skip empty files.
+merged.extend(content.get(mergekey, []))
+
+  if merged:
+# MainSourceFile: The key is required by the definition inside
+# include/clang/Tooling/ReplacementsYaml.h, but the value
+# is actually never used inside clang-apply-replacements,
+# so we set it to '' here.
+output = { 'MainSourceFile': '', mergekey: merged }
+with open(mergefile, 'w') as out:
+  yaml.safe_dump(output, out)
+  else:
+# Empty the file:
+open(mergefile, 'w').close()


 def main():
@@ -47,7 +127,10 @@ def main():
   r'.*\.(cpp|cc|c\+\+|cxx|c|cl|h|hpp|m|mm|inc)',
   help='custom pattern selecting file paths to check '
   '(case insensitive, overridden by -regex)')
-
+  parser.add_argument('-j', type=int, default=1,
+  help='number of tidy instances to be run in parallel.')
+  parser.add_argument('-timeout', 

[PATCH] D59573: [analyzer] C++17: PR41142: Ignore transparent InitListExprs when finding construction contexts.

2019-03-20 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

Though it's kidna funny that even when we support all of C++, we wouldn't 
necessarily always have construction contexts for all objects in the analyzer 
sense (even if they would be there in the CFG).


Repository:
  rC Clang

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

https://reviews.llvm.org/D59573



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


[clang-tools-extra] r356630 - Revert "Reland r356547 after fixing the tests for Linux."

2019-03-20 Thread Douglas Yung via cfe-commits
Author: dyung
Date: Wed Mar 20 16:21:43 2019
New Revision: 356630

URL: http://llvm.org/viewvc/llvm-project?rev=356630=rev
Log:
Revert "Reland r356547 after fixing the tests for Linux."

This reverts commit 538fb72226cf6dff95af83fe12b8dbd061ea (r356565).

This is still breaking a build bot:
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/45557

Modified:
clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py

Modified: clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py?rev=356630=356629=356630=diff
==
--- clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py (original)
+++ clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py Wed Mar 20 
16:21:43 2019
@@ -24,90 +24,10 @@ Example usage for git/svn users:
 """
 
 import argparse
-import glob
 import json
-import multiprocessing
-import os
 import re
-import shutil
 import subprocess
 import sys
-import tempfile
-import threading
-import traceback
-import yaml
-
-is_py2 = sys.version[0] == '2'
-
-if is_py2:
-import Queue as queue
-else:
-import queue as queue
-
-
-def run_tidy(task_queue, lock, timeout):
-  watchdog = None
-  while True:
-command = task_queue.get()
-try:
-  proc = subprocess.Popen(command,
-  stdout=subprocess.PIPE,
-  stderr=subprocess.PIPE)
-
-  if timeout is not None:
-watchdog = threading.Timer(timeout, proc.kill)
-watchdog.start()
-
-  stdout, stderr = proc.communicate()
-
-  with lock:
-sys.stdout.write(stdout.decode('utf-8') + '\n')
-sys.stdout.flush()
-if stderr:
-  sys.stderr.write(stderr.decode('utf-8') + '\n')
-  sys.stderr.flush()
-except Exception as e:
-  with lock:
-sys.stderr.write('Failed: ' + str(e) + ': '.join(command) + '\n')
-finally:
-  with lock:
-if (not timeout is None) and (not watchdog is None):
-  if not watchdog.is_alive():
-  sys.stderr.write('Terminated by timeout: ' +
-   ' '.join(command) + '\n')
-  watchdog.cancel()
-  task_queue.task_done()
-
-
-def start_workers(max_tasks, tidy_caller, task_queue, lock, timeout):
-  for _ in range(max_tasks):
-t = threading.Thread(target=tidy_caller, args=(task_queue, lock, timeout))
-t.daemon = True
-t.start()
-
-def merge_replacement_files(tmpdir, mergefile):
-  """Merge all replacement files in a directory into a single file"""
-  # The fixes suggested by clang-tidy >= 4.0.0 are given under
-  # the top level key 'Diagnostics' in the output yaml files
-  mergekey = "Diagnostics"
-  merged = []
-  for replacefile in glob.iglob(os.path.join(tmpdir, '*.yaml')):
-content = yaml.safe_load(open(replacefile, 'r'))
-if not content:
-  continue # Skip empty files.
-merged.extend(content.get(mergekey, []))
-
-  if merged:
-# MainSourceFile: The key is required by the definition inside
-# include/clang/Tooling/ReplacementsYaml.h, but the value
-# is actually never used inside clang-apply-replacements,
-# so we set it to '' here.
-output = { 'MainSourceFile': '', mergekey: merged }
-with open(mergefile, 'w') as out:
-  yaml.safe_dump(output, out)
-  else:
-# Empty the file:
-open(mergefile, 'w').close()
 
 
 def main():
@@ -127,10 +47,7 @@ def main():
   r'.*\.(cpp|cc|c\+\+|cxx|c|cl|h|hpp|m|mm|inc)',
   help='custom pattern selecting file paths to check '
   '(case insensitive, overridden by -regex)')
-  parser.add_argument('-j', type=int, default=1,
-  help='number of tidy instances to be run in parallel.')
-  parser.add_argument('-timeout', type=int, default=None,
-  help='timeout per each file in seconds.')
+
   parser.add_argument('-fix', action='store_true', default=False,
   help='apply suggested fixes')
   parser.add_argument('-checks',
@@ -167,7 +84,7 @@ def main():
 match = re.search('^\+\+\+\ \"?(.*?/){%s}([^ \t\n\"]*)' % args.p, line)
 if match:
   filename = match.group(2)
-if filename is None:
+if filename == None:
   continue
 
 if args.regex is not None:
@@ -185,79 +102,44 @@ def main():
 line_count = int(match.group(3))
   if line_count == 0:
 continue
-  end_line = start_line + line_count - 1
+  end_line = start_line + line_count - 1;
   lines_by_file.setdefault(filename, []).append([start_line, end_line])
 
-  if not any(lines_by_file):
+  if len(lines_by_file) == 0:
 print("No relevant changes found.")
 sys.exit(0)
 
-  max_task_count = args.j
-  if max_task_count == 0:
-  max_task_count = multiprocessing.cpu_count()
-  max_task_count = 

[PATCH] D59573: [analyzer] C++17: PR41142: Ignore transparent InitListExprs when finding construction contexts.

2019-03-20 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

Thx!

> The problem here is that we inevitably hit the recursion limit, and then...
> 
> 4. Well, something weird happens. I'll have to have a closer look, but one 
> weird thing i noticed is that the value for `{ boo() }` ends up being 
> `compoundVal{Unknown}`, as if `bindReturnValue()` never gets triggered.

Wait, right, nvm. There's no `return` statement in the function, so it's not 
surprising at all that there's no object-under-construction. There's just no 
construction to have a context for. So this is just a sub-problem of problem 5.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59573



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


[PATCH] D58841: [Diagnostics] Support -Wtype-limits for GCC compatibility

2019-03-20 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

Ping @aaron.ballman @rsmith


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

https://reviews.llvm.org/D58841



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


[PATCH] D59336: [clang-tidy] Disable google-runtime-int in Objective-C++ 

2019-03-20 Thread Stephane Moore via Phabricator via cfe-commits
stephanemoore marked an inline comment as done.
stephanemoore added a comment.

Thanks for the review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59336



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


[PATCH] D59336: [clang-tidy] Disable google-runtime-int in Objective-C++ 

2019-03-20 Thread Stephane Moore via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE356627: [clang-tidy] Disable google-runtime-int in 
Objective-C++  (authored by stephanemoore, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D59336?vs=191219=191599#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59336

Files:
  clang-tidy/google/IntegerTypesCheck.cpp
  docs/ReleaseNotes.rst
  test/clang-tidy/google-runtime-int.m


Index: clang-tidy/google/IntegerTypesCheck.cpp
===
--- clang-tidy/google/IntegerTypesCheck.cpp
+++ clang-tidy/google/IntegerTypesCheck.cpp
@@ -54,7 +54,9 @@
 
 void IntegerTypesCheck::registerMatchers(MatchFinder *Finder) {
   // Find all TypeLocs. The relevant Style Guide rule only applies to C++.
-  if (!getLangOpts().CPlusPlus)
+  // This check is also not applied in Objective-C++ sources as Objective-C
+  // often uses built-in integer types other than `int`.
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC)
 return;
   // Match any integer types, unless they are passed to a printf-based API:
   //
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -115,6 +115,9 @@
   `CommentUserDefiniedLiterals`, `CommentStringLiterals`,
   `CommentCharacterLiterals` & `CommentNullPtrs` options.
 
+- The :doc:`google-runtime-int `
+  check has been disabled in Objective-C++.
+
 - The `Acronyms` and `IncludeDefaultAcronyms` options for the
   :doc:`objc-property-declaration 
`
   check have been removed.
Index: test/clang-tidy/google-runtime-int.m
===
--- test/clang-tidy/google-runtime-int.m
+++ test/clang-tidy/google-runtime-int.m
@@ -0,0 +1,32 @@
+// RUN: clang-tidy -checks=-*,google-runtime-int %s 2>&1 -- | count 0
+// RUN: clang-tidy -checks=-*,google-runtime-int %s 2>&1 -- -x objective-c++ | 
count 0
+
+typedef long NSInteger;
+typedef unsigned long NSUInteger;
+
+@interface NSString
+@property(readonly) NSInteger integerValue;
+@property(readonly) long long longLongValue;
+@property(readonly) NSUInteger length;
+@end
+
+NSInteger Foo(NSString *s) {
+  return [s integerValue];
+}
+
+long long Bar(NSString *s) {
+  return [s longLongValue];
+}
+
+NSUInteger Baz(NSString *s) {
+  return [s length];
+}
+
+unsigned short NSSwapShort(unsigned short inv);
+
+long DoSomeMath(long a, short b) {
+  short c = NSSwapShort(b);
+  long a2 = a * 5L;
+  return a2 + c;
+}
+


Index: clang-tidy/google/IntegerTypesCheck.cpp
===
--- clang-tidy/google/IntegerTypesCheck.cpp
+++ clang-tidy/google/IntegerTypesCheck.cpp
@@ -54,7 +54,9 @@
 
 void IntegerTypesCheck::registerMatchers(MatchFinder *Finder) {
   // Find all TypeLocs. The relevant Style Guide rule only applies to C++.
-  if (!getLangOpts().CPlusPlus)
+  // This check is also not applied in Objective-C++ sources as Objective-C
+  // often uses built-in integer types other than `int`.
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC)
 return;
   // Match any integer types, unless they are passed to a printf-based API:
   //
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -115,6 +115,9 @@
   `CommentUserDefiniedLiterals`, `CommentStringLiterals`,
   `CommentCharacterLiterals` & `CommentNullPtrs` options.
 
+- The :doc:`google-runtime-int `
+  check has been disabled in Objective-C++.
+
 - The `Acronyms` and `IncludeDefaultAcronyms` options for the
   :doc:`objc-property-declaration `
   check have been removed.
Index: test/clang-tidy/google-runtime-int.m
===
--- test/clang-tidy/google-runtime-int.m
+++ test/clang-tidy/google-runtime-int.m
@@ -0,0 +1,32 @@
+// RUN: clang-tidy -checks=-*,google-runtime-int %s 2>&1 -- | count 0
+// RUN: clang-tidy -checks=-*,google-runtime-int %s 2>&1 -- -x objective-c++ | count 0
+
+typedef long NSInteger;
+typedef unsigned long NSUInteger;
+
+@interface NSString
+@property(readonly) NSInteger integerValue;
+@property(readonly) long long longLongValue;
+@property(readonly) NSUInteger length;
+@end
+
+NSInteger Foo(NSString *s) {
+  return [s integerValue];
+}
+
+long long Bar(NSString *s) {
+  return [s longLongValue];
+}
+
+NSUInteger Baz(NSString *s) {
+  return [s length];
+}
+
+unsigned short NSSwapShort(unsigned short inv);
+
+long DoSomeMath(long a, short b) {
+  short c = NSSwapShort(b);
+  long a2 = a * 5L;
+  return a2 + c;
+}
+
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D59440: add steps to preprocess file and reduce command line args

2019-03-20 Thread Amy Huang via Phabricator via cfe-commits
akhuang updated this revision to Diff 191598.
akhuang added a comment.

style nits, fixed thing in getting path to clang


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

https://reviews.llvm.org/D59440

Files:
  clang/utils/creduce-clang-crash.py

Index: clang/utils/creduce-clang-crash.py
===
--- clang/utils/creduce-clang-crash.py
+++ clang/utils/creduce-clang-crash.py
@@ -1,7 +1,5 @@
 #!/usr/bin/env python
 """Calls C-Reduce to create a minimal reproducer for clang crashes.
-
-Requires C-Reduce and not (part of LLVM utils) to be installed.
 """
 
 from argparse import ArgumentParser
@@ -11,108 +9,232 @@
 import sys
 import subprocess
 import pipes
+import shlex
+import tempfile
+import shutil
 from distutils.spawn import find_executable
 
-def create_test(build_script, llvm_not):
+verbose = False
+llvm_bin = None
+creduce_cmd = None
+not_cmd = None
+
+def check_file(fname):
+  if not os.path.isfile(fname):
+sys.exit("ERROR: %s does not exist" % (fname))
+  return fname
+
+def check_cmd(cmd_name, cmd_dir, cmd_path=None):
   """
-  Create an interestingness test from the crash output.
-  Return as a string.
+  Returns absolute path to cmd_path if it is given,
+  or absolute path to cmd_dir/cmd_name.
   """
-  # Get clang call from build script
-  # Assumes the call is the last line of the script
-  with open(build_script) as f:
-cmd = f.readlines()[-1].rstrip('\n\r')
-
-  # Get crash output
-  p = subprocess.Popen(build_script,
+  if cmd_path:
+cmd = find_executable(cmd_path)
+if cmd:
+  return cmd
+sys.exit("ERROR: executable %s not found" % (cmd_path))
+
+  cmd = find_executable(cmd_name, path=cmd_dir)
+  if cmd:
+return cmd
+  sys.exit("ERROR: %s not found in %s" % (cmd_name, cmd_dir))
+
+def quote_cmd(cmd):
+  return ' '.join(arg if arg.startswith('$') else pipes.quote(arg)
+  for arg in cmd)
+
+def get_crash_cmd(crash_script):
+  with open(crash_script) as f:
+# Assume clang call is on the last line of the script
+line = f.readlines()[-1]
+cmd = shlex.split(line)
+
+# Overwrite the script's clang with the user's clang path
+new_clang = check_cmd('clang', llvm_bin)
+cmd[0] = pipes.quote(new_clang)
+return cmd
+
+def has_expected_output(crash_cmd, expected_output):
+  p = subprocess.Popen(crash_cmd,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT)
   crash_output, _ = p.communicate()
+  return all(msg in crash_output for msg in expected_output)
 
-  output = ['#!/bin/bash']
-  output.append('%s --crash %s >& t.log || exit 1' % (pipes.quote(llvm_not),
-  cmd))
+def get_expected_output(crash_cmd):
+  p = subprocess.Popen(crash_cmd,
+   stdout=subprocess.PIPE,
+   stderr=subprocess.STDOUT)
+  crash_output, _ = p.communicate()
 
-  # Add messages from crash output to the test
-  # If there is an Assertion failure, use that; otherwise use the
-  # last five stack trace functions
+  # If there is an assertion failure, use that;
+  # otherwise use the last five stack trace functions
   assertion_re = r'Assertion `([^\']+)\' failed'
   assertion_match = re.search(assertion_re, crash_output)
   if assertion_match:
-msg = assertion_match.group(1)
-output.append('grep %s t.log || exit 1' % pipes.quote(msg))
+return [assertion_match.group(1)]
   else:
 stacktrace_re = r'#[0-9]+\s+0[xX][0-9a-fA-F]+\s*([^(]+)\('
 matches = re.findall(stacktrace_re, crash_output)
-del matches[:-5]
-output += ['grep %s t.log || exit 1' % pipes.quote(msg) for msg in matches]
+return matches[-5:]
+
+def write_interestingness_test(testfile, crash_cmd, expected_output,
+   file_to_reduce):
+  filename = os.path.basename(file_to_reduce)
+  if filename not in crash_cmd:
+sys.exit("ERROR: expected %s to be in the crash command" % filename)
+
+  # Replace all instances of file_to_reduce with a command line variable
+  output = ['#!/bin/bash',
+'if [ -z "$1" ] ; then',
+'  f=%s' % (pipes.quote(filename)),
+'else',
+'  f="$1"',
+'fi']
+  cmd = ['$f' if s == filename else s for s in crash_cmd]
+
+  output.append('%s --crash %s >& t.log || exit 1' % (pipes.quote(not_cmd),
+  quote_cmd(cmd)))
+
+  for msg in expected_output:
+output.append('grep %s t.log || exit 1' % pipes.quote(msg))
 
-  return output
+  with open(testfile, 'w') as f:
+f.write('\n'.join(output))
+  os.chmod(testfile, os.stat(testfile).st_mode | stat.S_IEXEC)
+
+def check_interestingness(testfile, file_to_reduce):
+  testfile = os.path.abspath(testfile)
+
+  # Check that the test considers the original file interesting
+  with open(os.devnull, 'w') as devnull:
+returncode = subprocess.call(testfile, 

[clang-tools-extra] r356623 - Revert "[clangd] Print arguments in template specializations"

2019-03-20 Thread Jordan Rupprecht via cfe-commits
Author: rupprecht
Date: Wed Mar 20 15:51:56 2019
New Revision: 356623

URL: http://llvm.org/viewvc/llvm-project?rev=356623=rev
Log:
Revert "[clangd] Print arguments in template specializations"

This reverts commit 44a63f6a150dec72dea43730d2a89d292e58bd6f. It segfaults on 
an internal test case (will follow up off thread).

Modified:
clang-tools-extra/trunk/clangd/AST.cpp
clang-tools-extra/trunk/clangd/index/MemIndex.cpp
clang-tools-extra/trunk/clangd/index/dex/Dex.cpp
clang-tools-extra/trunk/unittests/clangd/DexTests.cpp
clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp

Modified: clang-tools-extra/trunk/clangd/AST.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/AST.cpp?rev=356623=356622=356623=diff
==
--- clang-tools-extra/trunk/clangd/AST.cpp (original)
+++ clang-tools-extra/trunk/clangd/AST.cpp Wed Mar 20 15:51:56 2019
@@ -11,12 +11,9 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclTemplate.h"
-#include "clang/AST/TemplateBase.h"
-#include "clang/AST/TypeLoc.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Index/USRGeneration.h"
-#include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ScopedPrinter.h"
 
@@ -53,22 +50,6 @@ SourceLocation findNameLoc(const clang::
   return SM.getSpellingLoc(D->getLocation());
 }
 
-static llvm::Optional>
-getTemplateSpecializationArgLocs(const NamedDecl ) {
-  if (auto *Func = llvm::dyn_cast()) {
-if (auto *Args = Func->getTemplateSpecializationArgsAsWritten())
-  return Args->arguments();
-  } else if (auto *Cls =
- llvm::dyn_cast()) {
-if (auto *Args = Cls->getTemplateArgsAsWritten())
-  return Args->arguments();
-  } else if (auto *Var = llvm::dyn_cast())
-return Var->getTemplateArgsInfo().arguments();
-  // We return None for ClassTemplateSpecializationDecls because it does not
-  // contain TemplateArgumentLoc information.
-  return llvm::None;
-}
-
 std::string printQualifiedName(const NamedDecl ) {
   std::string QName;
   llvm::raw_string_ostream OS(QName);
@@ -79,19 +60,6 @@ std::string printQualifiedName(const Nam
   // namespaces to query: the preamble doesn't have a dedicated list.
   Policy.SuppressUnwrittenScope = true;
   ND.printQualifiedName(OS, Policy);
-  if (auto Args = getTemplateSpecializationArgLocs(ND))
-printTemplateArgumentList(OS, *Args, Policy);
-  else if (auto *Cls = llvm::dyn_cast()) {
-if (auto STL = Cls->getTypeAsWritten()
-   ->getTypeLoc()
-   .getAs()) {
-  llvm::SmallVector ArgLocs;
-  ArgLocs.reserve(STL.getNumArgs());
-  for (unsigned I = 0; I < STL.getNumArgs(); ++I)
-ArgLocs.push_back(STL.getArgLoc(I));
-  printTemplateArgumentList(OS, ArgLocs, Policy);
-}
-  }
   OS.flush();
   assert(!StringRef(QName).startswith("::"));
   return QName;

Modified: clang-tools-extra/trunk/clangd/index/MemIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/MemIndex.cpp?rev=356623=356622=356623=diff
==
--- clang-tools-extra/trunk/clangd/index/MemIndex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/MemIndex.cpp Wed Mar 20 15:51:56 2019
@@ -38,6 +38,15 @@ bool MemIndex::fuzzyFind(
   for (const auto Pair : Index) {
 const Symbol *Sym = Pair.second;
 
+// FIXME: Enable fuzzy find on template specializations once we start
+// storing template arguments in the name. Currently we only store name for
+// class template, which would cause duplication in the results.
+if (Sym->SymInfo.Properties &
+(static_cast(
+ index::SymbolProperty::TemplateSpecialization) |
+ static_cast(
+ index::SymbolProperty::TemplatePartialSpecialization)))
+  continue;
 // Exact match against all possible scopes.
 if (!Req.AnyScope && !llvm::is_contained(Req.Scopes, Sym->Scope))
   continue;

Modified: clang-tools-extra/trunk/clangd/index/dex/Dex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Dex.cpp?rev=356623=356622=356623=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/Dex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Dex.cpp Wed Mar 20 15:51:56 2019
@@ -86,6 +86,15 @@ void Dex::buildIndex() {
   llvm::DenseMap> TempInvertedIndex;
   for (DocID SymbolRank = 0; SymbolRank < Symbols.size(); ++SymbolRank) {
 const auto *Sym = Symbols[SymbolRank];
+// FIXME: Enable fuzzy find on template specializations once we start
+// storing template arguments in the name. Currently we only store name for
+// 

r356623 - Revert "[clangd] Print arguments in template specializations"

2019-03-20 Thread Jordan Rupprecht via cfe-commits
Author: rupprecht
Date: Wed Mar 20 15:51:56 2019
New Revision: 356623

URL: http://llvm.org/viewvc/llvm-project?rev=356623=rev
Log:
Revert "[clangd] Print arguments in template specializations"

This reverts commit 44a63f6a150dec72dea43730d2a89d292e58bd6f. It segfaults on 
an internal test case (will follow up off thread).

Modified:
cfe/trunk/lib/AST/TypePrinter.cpp

Modified: cfe/trunk/lib/AST/TypePrinter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/TypePrinter.cpp?rev=356623=356622=356623=diff
==
--- cfe/trunk/lib/AST/TypePrinter.cpp (original)
+++ cfe/trunk/lib/AST/TypePrinter.cpp Wed Mar 20 15:51:56 2019
@@ -1632,21 +1632,6 @@ static const TemplateArgument 
   return A.getArgument();
 }
 
-static void printArgument(const TemplateArgument , const PrintingPolicy ,
-  llvm::raw_ostream ) {
-  A.print(PP, OS);
-}
-
-static void printArgument(const TemplateArgumentLoc ,
-  const PrintingPolicy , llvm::raw_ostream ) {
-  const auto  = A.getArgument().getKind();
-  assert(Kind != TemplateArgument::Null &&
- "TemplateArgumentKind can not be null!");
-  if (Kind == TemplateArgument::ArgKind::Type)
-return A.getTypeSourceInfo()->getType().print(OS, PP);
-  return A.getArgument().print(PP, OS);
-}
-
 template
 static void printTo(raw_ostream , ArrayRef Args,
 const PrintingPolicy , bool SkipBrackets) {
@@ -1668,7 +1653,7 @@ static void printTo(raw_ostream , Arr
 } else {
   if (!FirstArg)
 OS << Comma;
-  printArgument(Arg, Policy, ArgOS);
+  Argument.print(Policy, ArgOS);
 }
 StringRef ArgString = ArgOS.str();
 


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


[PATCH] D59440: add steps to preprocess file and reduce command line args

2019-03-20 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment.

Just a few style nits for you, and this LGTM. I assume rnk and 
serge-sans-paille are content, so I'm happy to check this in for you once these 
are addressed.

Thanks!




Comment at: clang/utils/creduce-clang-crash.py:64
   crash_output, _ = p.communicate()
+  for msg in expected_output:
+if msg not in crash_output:

nit: can be simplified to `return all(msg not in crash_output for msg in 
expected_output)`



Comment at: clang/utils/creduce-clang-crash.py:116
+  with open(os.devnull, 'w') as devnull:
+p = subprocess.Popen(testfile, stdout=devnull)
+p.communicate()

nit: looks like you can use `returncode = subprocess.call(testfile, 
stdout=devnull)` here



Comment at: clang/utils/creduce-clang-crash.py:124
+  with open(os.devnull, 'w') as devnull:
+p = subprocess.Popen([testfile, empty_file], stdout=devnull)
+p.communicate()

same `subprocess.call` nit



Comment at: clang/utils/creduce-clang-crash.py:243
 
+  #FIXME: reduce the clang crash command
+

nit: please add a space: `# FIXME`


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

https://reviews.llvm.org/D59440



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


[PATCH] D59560: Permit redeclarations of a builtin to specify calling convention.

2019-03-20 Thread 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/D59560/new/

https://reviews.llvm.org/D59560



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


[PATCH] D59615: [AArch64] When creating SISD intrinsic calls widen scalar args into a zero vectors, not undef

2019-03-20 Thread Amara Emerson via Phabricator via cfe-commits
aemerson created this revision.
aemerson added reviewers: kristof.beyls, t.p.northover, olista01.
aemerson added a project: clang.
Herald added subscribers: arphaman, javed.absar.

Some intrinsics like saturating operations may set flags, so if the scalar arg 
is inserted into an undef vector, the undef elements may trigger unwanted side 
effects. Using zero should be safer than undef.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59615

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/aarch64-neon-intrinsics.c
  clang/test/CodeGen/aarch64-neon-scalar-x-indexed-elem.c
  clang/test/CodeGen/aarch64-v8.1a-neon-intrinsics.c

Index: clang/test/CodeGen/aarch64-v8.1a-neon-intrinsics.c
===
--- clang/test/CodeGen/aarch64-v8.1a-neon-intrinsics.c
+++ clang/test/CodeGen/aarch64-v8.1a-neon-intrinsics.c
@@ -39,12 +39,12 @@
 
 // CHECK-LABEL: test_vqrdmlahh_s16
 int16_t test_vqrdmlahh_s16(int16_t a, int16_t b, int16_t c) {
-// CHECK: [[insb:%.*]] = insertelement <4 x i16> undef, i16 {{%.*}}, i64 0
-// CHECK: [[insc:%.*]] = insertelement <4 x i16> undef, i16 {{%.*}}, i64 0
+// CHECK: [[insb:%.*]] = insertelement <4 x i16> zeroinitializer, i16 {{%.*}}, i64 0
+// CHECK: [[insc:%.*]] = insertelement <4 x i16> zeroinitializer, i16 {{%.*}}, i64 0
 // CHECK: [[mul:%.*]] = call <4 x i16> @llvm.aarch64.neon.sqrdmulh.v4i16(<4 x i16> [[insb]], <4 x i16> [[insc]])
 // CHECK: extractelement <4 x i16> [[mul]], i64 0
-// CHECK: [[insa:%.*]] = insertelement <4 x i16> undef, i16 {{%.*}}, i64 0
-// CHECK: [[insmul:%.*]] = insertelement <4 x i16> undef, i16 {{%.*}}, i64 0
+// CHECK: [[insa:%.*]] = insertelement <4 x i16> zeroinitializer, i16 {{%.*}}, i64 0
+// CHECK: [[insmul:%.*]] = insertelement <4 x i16> zeroinitializer, i16 {{%.*}}, i64 0
 // CHECK: [[add:%.*]] = call <4 x i16> @llvm.aarch64.neon.sqadd.v4i16(<4 x i16> [[insa]], <4 x i16> [[insmul]])
 // CHECK: extractelement <4 x i16> [[add]], i64 0
   return vqrdmlahh_s16(a, b, c);
@@ -60,12 +60,12 @@
 // CHECK-LABEL: test_vqrdmlahh_lane_s16
 int16_t test_vqrdmlahh_lane_s16(int16_t a, int16_t b, int16x4_t c) {
 // CHECK: extractelement <4 x i16> {{%.*}}, i32 3
-// CHECK: [[insb:%.*]] = insertelement <4 x i16> undef, i16 {{%.*}}, i64 0
-// CHECK: [[insc:%.*]] = insertelement <4 x i16> undef, i16 {{%.*}}, i64 0
+// CHECK: [[insb:%.*]] = insertelement <4 x i16> zeroinitializer, i16 {{%.*}}, i64 0
+// CHECK: [[insc:%.*]] = insertelement <4 x i16> zeroinitializer, i16 {{%.*}}, i64 0
 // CHECK: [[mul:%.*]] = call <4 x i16> @llvm.aarch64.neon.sqrdmulh.v4i16(<4 x i16> [[insb]], <4 x i16> [[insc]])
 // CHECK: extractelement <4 x i16> [[mul]], i64 0
-// CHECK: [[insa:%.*]] = insertelement <4 x i16> undef, i16 {{%.*}}, i64 0
-// CHECK: [[insmul:%.*]] = insertelement <4 x i16> undef, i16 {{%.*}}, i64 0
+// CHECK: [[insa:%.*]] = insertelement <4 x i16> zeroinitializer, i16 {{%.*}}, i64 0
+// CHECK: [[insmul:%.*]] = insertelement <4 x i16> zeroinitializer, i16 {{%.*}}, i64 0
 // CHECK: [[add:%.*]] = call <4 x i16> @llvm.aarch64.neon.sqadd.v4i16(<4 x i16> [[insa]], <4 x i16> [[insmul]])
 // CHECK: extractelement <4 x i16> [[add]], i64 0
   return vqrdmlahh_lane_s16(a, b, c, 3);
@@ -82,12 +82,12 @@
 // CHECK-LABEL: test_vqrdmlahh_laneq_s16
 int16_t test_vqrdmlahh_laneq_s16(int16_t a, int16_t b, int16x8_t c) {
 // CHECK: extractelement <8 x i16> {{%.*}}, i32 7
-// CHECK: [[insb:%.*]] = insertelement <4 x i16> undef, i16 {{%.*}}, i64 0
-// CHECK: [[insc:%.*]] = insertelement <4 x i16> undef, i16 {{%.*}}, i64 0
+// CHECK: [[insb:%.*]] = insertelement <4 x i16> zeroinitializer, i16 {{%.*}}, i64 0
+// CHECK: [[insc:%.*]] = insertelement <4 x i16> zeroinitializer, i16 {{%.*}}, i64 0
 // CHECK: [[mul:%.*]] = call <4 x i16> @llvm.aarch64.neon.sqrdmulh.v4i16(<4 x i16> [[insb]], <4 x i16> [[insc]])
 // CHECK: extractelement <4 x i16> [[mul]], i64 0
-// CHECK: [[insa:%.*]] = insertelement <4 x i16> undef, i16 {{%.*}}, i64 0
-// CHECK: [[insmul:%.*]] = insertelement <4 x i16> undef, i16 {{%.*}}, i64 0
+// CHECK: [[insa:%.*]] = insertelement <4 x i16> zeroinitializer, i16 {{%.*}}, i64 0
+// CHECK: [[insmul:%.*]] = insertelement <4 x i16> zeroinitializer, i16 {{%.*}}, i64 0
 // CHECK: [[add:%.*]] = call <4 x i16> @llvm.aarch64.neon.sqadd.v4i16(<4 x i16> [[insa]], <4 x i16> [[insmul]])
 // CHECK: extractelement <4 x i16> [[add]], i64 0
   return vqrdmlahh_laneq_s16(a, b, c, 7);
@@ -135,12 +135,12 @@
 
 // CHECK-LABEL: test_vqrdmlshh_s16
 int16_t test_vqrdmlshh_s16(int16_t a, int16_t b, int16_t c) {
-// CHECK: [[insb:%.*]] = insertelement <4 x i16> undef, i16 {{%.*}}, i64 0
-// CHECK: [[insc:%.*]] = insertelement <4 x i16> undef, i16 {{%.*}}, i64 0
+// CHECK: [[insb:%.*]] = insertelement <4 x i16> zeroinitializer, i16 {{%.*}}, i64 0
+// CHECK: [[insc:%.*]] = insertelement <4 x i16> zeroinitializer, i16 {{%.*}}, i64 0
 // CHECK: [[mul:%.*]] = call <4 x i16> @llvm.aarch64.neon.sqrdmulh.v4i16(<4 x i16> [[insb]], <4 x i16> 

[PATCH] D59523: Thread Safety: also look at ObjC methods

2019-03-20 Thread JF Bastien via Phabricator via cfe-commits
jfb marked an inline comment as done.
jfb added inline comments.



Comment at: test/SemaObjCXX/no-crash-thread-safety-analysis.mm:1
+// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -Wno-objc-root-class 
%s
+

aaronpuchert wrote:
> Test is fine for me, but I would like if you could integrate it with the 
> existing test/SemaObjCXX/warn-thread-safety-analysis.mm. The thread safety 
> analysis requires a bit of setup, that's why we tend to keep the tests in one 
> file. I'll admit that the C++ tests have grown quite large, but for ObjC++ 
> it's still very manageable. 
Sure thing! I created a header that's shared and simplified this repro a bit. I 
validated that the shared code was crashing before and this fixes the crash.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59523



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


[PATCH] D59523: Thread Safety: also look at ObjC methods

2019-03-20 Thread JF Bastien via Phabricator via cfe-commits
jfb updated this revision to Diff 191580.
jfb marked 3 inline comments as done.
jfb added a comment.

- Simlify tests, share code


Repository:
  rC Clang

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

https://reviews.llvm.org/D59523

Files:
  lib/Analysis/ThreadSafetyCommon.cpp
  test/SemaObjCXX/no-crash-thread-safety-analysis.mm
  test/SemaObjCXX/thread-safety-analysis.h
  test/SemaObjCXX/warn-thread-safety-analysis.mm

Index: test/SemaObjCXX/warn-thread-safety-analysis.mm
===
--- test/SemaObjCXX/warn-thread-safety-analysis.mm
+++ test/SemaObjCXX/warn-thread-safety-analysis.mm
@@ -1,22 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -Wthread-safety-beta -Wno-objc-root-class %s
 
-class __attribute__((lockable)) Lock {
-public:
-  void Acquire() __attribute__((exclusive_lock_function())) {}
-  void Release() __attribute__((unlock_function())) {}
-};
-
-class __attribute__((scoped_lockable)) AutoLock {
-public:
-  AutoLock(Lock ) __attribute__((exclusive_lock_function(lock)))
-  : lock_(lock) {
-lock.Acquire();
-  }
-  ~AutoLock() __attribute__((unlock_function())) { lock_.Release(); }
-
-private:
-  Lock _;
-};
+#include "thread-safety-analysis.h"
 
 @interface MyInterface {
 @private
Index: test/SemaObjCXX/thread-safety-analysis.h
===
--- /dev/null
+++ test/SemaObjCXX/thread-safety-analysis.h
@@ -0,0 +1,17 @@
+class __attribute__((lockable)) Lock {
+public:
+  void Acquire() __attribute__((exclusive_lock_function())) {}
+  void Release() __attribute__((unlock_function())) {}
+};
+
+class __attribute__((scoped_lockable)) AutoLock {
+public:
+  AutoLock(Lock ) __attribute__((exclusive_lock_function(lock)))
+  : lock_(lock) {
+lock.Acquire();
+  }
+  ~AutoLock() __attribute__((unlock_function())) { lock_.Release(); }
+
+private:
+  Lock _;
+};
Index: test/SemaObjCXX/no-crash-thread-safety-analysis.mm
===
--- /dev/null
+++ test/SemaObjCXX/no-crash-thread-safety-analysis.mm
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -Wno-objc-root-class %s
+
+// expected-no-diagnostics
+
+// Thread safety analysis used to crash when used with ObjC methods.
+
+#include "thread-safety-analysis.h"
+
+@interface MyInterface
+- (void)doIt:(class Lock *)myLock;
+@end
+
+@implementation MyInterface
+- (void)doIt:(class Lock *)myLock {
+  AutoLock lock(*myLock);
+}
+@end
Index: lib/Analysis/ThreadSafetyCommon.cpp
===
--- lib/Analysis/ThreadSafetyCommon.cpp
+++ lib/Analysis/ThreadSafetyCommon.cpp
@@ -276,18 +276,23 @@
 
   // Function parameters require substitution and/or renaming.
   if (const auto *PV = dyn_cast_or_null(VD)) {
-const auto *FD =
-cast(PV->getDeclContext())->getCanonicalDecl();
 unsigned I = PV->getFunctionScopeIndex();
-
-if (Ctx && Ctx->FunArgs && FD == Ctx->AttrDecl->getCanonicalDecl()) {
-  // Substitute call arguments for references to function parameters
-  assert(I < Ctx->NumArgs);
-  return translate(Ctx->FunArgs[I], Ctx->Prev);
+const auto *D = PV->getDeclContext();
+if (Ctx && Ctx->FunArgs) {
+  const auto *Canonical = Ctx->AttrDecl->getCanonicalDecl();
+  if (isa(D)
+  ? (cast(D)->getCanonicalDecl() == Canonical)
+  : (cast(D)->getCanonicalDecl() == Canonical)) {
+// Substitute call arguments for references to function parameters
+assert(I < Ctx->NumArgs);
+return translate(Ctx->FunArgs[I], Ctx->Prev);
+  }
 }
 // Map the param back to the param of the original function declaration
 // for consistent comparisons.
-VD = FD->getParamDecl(I);
+VD = isa(D)
+ ? cast(D)->getCanonicalDecl()->getParamDecl(I)
+ : cast(D)->getCanonicalDecl()->getParamDecl(I);
   }
 
   // For non-local variables, treat it as a reference to a named object.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D57497: [RISCV] Passing small data limitation value to RISCV backend

2019-03-20 Thread Ana Pazos via Phabricator via cfe-commits
apazos added inline comments.
Herald added subscribers: benna, psnobl.



Comment at: lib/Driver/ToolChains/Clang.cpp:1803
+
+  // Forward the -msmall-data-limit= option.
+  if (Arg *A = Args.getLastArg(options::OPT_G)) {

Might be simpler to just set it to 0, and if G is present in the command line, 
and the other flags are not present, then you change the value. Also, might be 
good to print a warning to let user know -G is ignored?



Comment at: lib/Driver/ToolChains/RISCVToolchain.cpp:117
 
+  if (D.isUsingLTO()) {
+assert(!Inputs.empty() && "Must have at least one input.");

This change is not part of this patch.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57497



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


[PATCH] D59609: [clang][OpenMP] Fix build when using libgomp

2019-03-20 Thread Jordan Rupprecht via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC356614: [clang][OpenMP] Fix build when using libgomp 
(authored by rupprecht, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D59609?vs=191567=191577#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D59609

Files:
  unittests/AST/OMPStructuredBlockTest.cpp


Index: unittests/AST/OMPStructuredBlockTest.cpp
===
--- unittests/AST/OMPStructuredBlockTest.cpp
+++ unittests/AST/OMPStructuredBlockTest.cpp
@@ -55,7 +55,7 @@
   StringRef ExpectedPrinted,
   PolicyAdjusterType PolicyAdjuster = None) {
   std::vector Args = {
-  "-fopenmp",
+  "-fopenmp=libomp",
   };
   return PrintedStmtMatches(Code, Args, NodeMatch, ExpectedPrinted,
 PolicyAdjuster);


Index: unittests/AST/OMPStructuredBlockTest.cpp
===
--- unittests/AST/OMPStructuredBlockTest.cpp
+++ unittests/AST/OMPStructuredBlockTest.cpp
@@ -55,7 +55,7 @@
   StringRef ExpectedPrinted,
   PolicyAdjusterType PolicyAdjuster = None) {
   std::vector Args = {
-  "-fopenmp",
+  "-fopenmp=libomp",
   };
   return PrintedStmtMatches(Code, Args, NodeMatch, ExpectedPrinted,
 PolicyAdjuster);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D59523: Thread Safety: also look at ObjC methods

2019-03-20 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a reviewer: aaronpuchert.
aaronpuchert added inline comments.



Comment at: lib/Analysis/ThreadSafetyCommon.cpp:282-285
+  const auto *Canonical = Ctx->AttrDecl->getCanonicalDecl();
+  if (isa(D)
+  ? (cast(D)->getCanonicalDecl() == Canonical)
+  : (cast(D)->getCanonicalDecl() == Canonical)) {

Unrelated to your change, but I'm wondering if this is right. `Ctx->AttrDecl` 
is a `NamedDecl`, on which `getCanonicalDecl()` just returns `*this` (so we 
could in fact omit it here without functional change), while on `FunctionDecl` 
and `ObjCMethodDecl` it does something non-trivial instead.

I guess I need to look into this a bit, don't let this bother you. But I think 
we might actually have to do the cast on both sides of the equation to get the 
same kind of canonical declaration. Or we make sure that `Ctx->AttrDecl` is 
already canonical.



Comment at: test/SemaObjCXX/no-crash-thread-safety-analysis.mm:1
+// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -Wno-objc-root-class 
%s
+

Test is fine for me, but I would like if you could integrate it with the 
existing test/SemaObjCXX/warn-thread-safety-analysis.mm. The thread safety 
analysis requires a bit of setup, that's why we tend to keep the tests in one 
file. I'll admit that the C++ tests have grown quite large, but for ObjC++ it's 
still very manageable. 



Comment at: test/SemaObjCXX/no-crash-thread-safety-analysis.mm:10
+
+template  struct __attribute__((scoped_lockable)) Locker {
+  T &_l;

Can we hard-code `T` = `MyLock`? We can still change it if we need it more 
general later.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59523



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


[PATCH] D59609: [clang][OpenMP] Fix build when using libgomp

2019-03-20 Thread Jordan Rupprecht via Phabricator via cfe-commits
rupprecht added a comment.

In D59609#1436975 , @lebedev.ri wrote:

> Interesting.
>  What happens if libomp is not being built?
>  What happens if libomp is not present?


I'm far from an omp expert (only running across this because some tests 
failed), but I would guess both those cases to break.

But this is broken anyway when libomp isn't being used. So the set of broken 
cases is going from (people that don't use libomp) to (people that don't have 
libomp) which I think is a much smaller subset.

> Or more generally, why does it even matter whether there is runtime or not, 
> this only does paring+sema, no codegen/execution.

I'm very surprised myself. Hence my question "has something gone wrong" on 
D59214 . Hopefully this fix/hack can be 
reverted once that's figured out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59609



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


Re: [clang-tools-extra] r356565 - Reland r356547 after fixing the tests for Linux.

2019-03-20 Thread Galina Kistanova via cfe-commits
Hello Zinovy,

This commit broke test on the next builder:
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/45557
. . .
Failing Tests (1):
Clang Tools :: clang-tidy/clang-tidy-diff.cpp

Please have a look?

Thanks

Galina


On Wed, Mar 20, 2019 at 8:49 AM Zinovy Nis via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: zinovy.nis
> Date: Wed Mar 20 08:50:26 2019
> New Revision: 356565
>
> URL: http://llvm.org/viewvc/llvm-project?rev=356565=rev
> Log:
> Reland r356547 after fixing the tests for Linux.
>
> [clang-tidy] Parallelize clang-tidy-diff.py
>
> This patch has 2 rationales:
>
> - large patches lead to long command lines and often cause max command
> line length restrictions imposed by OS;
> - clang-tidy runs on modified files are independent and can be done in
> parallel, the same as done for run-clang-tidy.
>
> Differential Revision: https://reviews.llvm.org/D57662
>
>
> Modified:
> clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
>
> Modified: clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py?rev=356565=356564=356565=diff
>
> ==
> --- clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py (original)
> +++ clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py Wed Mar 20
> 08:50:26 2019
> @@ -24,10 +24,90 @@ Example usage for git/svn users:
>  """
>
>  import argparse
> +import glob
>  import json
> +import multiprocessing
> +import os
>  import re
> +import shutil
>  import subprocess
>  import sys
> +import tempfile
> +import threading
> +import traceback
> +import yaml
> +
> +is_py2 = sys.version[0] == '2'
> +
> +if is_py2:
> +import Queue as queue
> +else:
> +import queue as queue
> +
> +
> +def run_tidy(task_queue, lock, timeout):
> +  watchdog = None
> +  while True:
> +command = task_queue.get()
> +try:
> +  proc = subprocess.Popen(command,
> +  stdout=subprocess.PIPE,
> +  stderr=subprocess.PIPE)
> +
> +  if timeout is not None:
> +watchdog = threading.Timer(timeout, proc.kill)
> +watchdog.start()
> +
> +  stdout, stderr = proc.communicate()
> +
> +  with lock:
> +sys.stdout.write(stdout.decode('utf-8') + '\n')
> +sys.stdout.flush()
> +if stderr:
> +  sys.stderr.write(stderr.decode('utf-8') + '\n')
> +  sys.stderr.flush()
> +except Exception as e:
> +  with lock:
> +sys.stderr.write('Failed: ' + str(e) + ': '.join(command) + '\n')
> +finally:
> +  with lock:
> +if (not timeout is None) and (not watchdog is None):
> +  if not watchdog.is_alive():
> +  sys.stderr.write('Terminated by timeout: ' +
> +   ' '.join(command) + '\n')
> +  watchdog.cancel()
> +  task_queue.task_done()
> +
> +
> +def start_workers(max_tasks, tidy_caller, task_queue, lock, timeout):
> +  for _ in range(max_tasks):
> +t = threading.Thread(target=tidy_caller, args=(task_queue, lock,
> timeout))
> +t.daemon = True
> +t.start()
> +
> +def merge_replacement_files(tmpdir, mergefile):
> +  """Merge all replacement files in a directory into a single file"""
> +  # The fixes suggested by clang-tidy >= 4.0.0 are given under
> +  # the top level key 'Diagnostics' in the output yaml files
> +  mergekey = "Diagnostics"
> +  merged = []
> +  for replacefile in glob.iglob(os.path.join(tmpdir, '*.yaml')):
> +content = yaml.safe_load(open(replacefile, 'r'))
> +if not content:
> +  continue # Skip empty files.
> +merged.extend(content.get(mergekey, []))
> +
> +  if merged:
> +# MainSourceFile: The key is required by the definition inside
> +# include/clang/Tooling/ReplacementsYaml.h, but the value
> +# is actually never used inside clang-apply-replacements,
> +# so we set it to '' here.
> +output = { 'MainSourceFile': '', mergekey: merged }
> +with open(mergefile, 'w') as out:
> +  yaml.safe_dump(output, out)
> +  else:
> +# Empty the file:
> +open(mergefile, 'w').close()
>
>
>  def main():
> @@ -47,7 +127,10 @@ def main():
>r'.*\.(cpp|cc|c\+\+|cxx|c|cl|h|hpp|m|mm|inc)',
>help='custom pattern selecting file paths to check '
>'(case insensitive, overridden by -regex)')
> -
> +  parser.add_argument('-j', type=int, default=1,
> +  help='number of tidy instances to be run in
> parallel.')
> +  parser.add_argument('-timeout', type=int, default=None,
> +  help='timeout per each file in seconds.')
>parser.add_argument('-fix', action='store_true', default=False,
>help='apply suggested fixes')
>parser.add_argument('-checks',
> @@ -84,7 +167,7 @@ def 

r356615 - [clang-format][NFC] fix release notes build issue

2019-03-20 Thread Paul Hoad via cfe-commits
Author: paulhoad
Date: Wed Mar 20 14:02:12 2019
New Revision: 356615

URL: http://llvm.org/viewvc/llvm-project?rev=356615=rev
Log:
[clang-format][NFC] fix release notes build issue

build issue from r356613

Modified:
cfe/trunk/docs/ReleaseNotes.rst

Modified: cfe/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=356615=356614=356615=diff
==
--- cfe/trunk/docs/ReleaseNotes.rst (original)
+++ cfe/trunk/docs/ReleaseNotes.rst Wed Mar 20 14:02:12 2019
@@ -171,7 +171,7 @@ clang-format
 
 
 - Added new option `PPDIS_BeforeHash` (in configuration: `BeforeHash`) to
- `IndentPPDirectives` which indents preprocessor directives before the hash.
+  `IndentPPDirectives` which indents preprocessor directives before the hash.
 
 libclang
 


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


r356614 - [clang][OpenMP] Fix build when using libgomp

2019-03-20 Thread Jordan Rupprecht via cfe-commits
Author: rupprecht
Date: Wed Mar 20 14:01:56 2019
New Revision: 356614

URL: http://llvm.org/viewvc/llvm-project?rev=356614=rev
Log:
[clang][OpenMP] Fix build when using libgomp

Summary: rL356570 introduced a test which only passes with the default openmp 
library, libomp, and fails with other openmp libraries, such as libgomp. 
Explicitly choose libomp.

Reviewers: lebedev.ri

Subscribers: guansong, jdoerfert, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/unittests/AST/OMPStructuredBlockTest.cpp

Modified: cfe/trunk/unittests/AST/OMPStructuredBlockTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/OMPStructuredBlockTest.cpp?rev=356614=356613=356614=diff
==
--- cfe/trunk/unittests/AST/OMPStructuredBlockTest.cpp (original)
+++ cfe/trunk/unittests/AST/OMPStructuredBlockTest.cpp Wed Mar 20 14:01:56 2019
@@ -55,7 +55,7 @@ PrintedOMPStmtMatches(StringRef Code, co
   StringRef ExpectedPrinted,
   PolicyAdjusterType PolicyAdjuster = None) {
   std::vector Args = {
-  "-fopenmp",
+  "-fopenmp=libomp",
   };
   return PrintedStmtMatches(Code, Args, NodeMatch, ExpectedPrinted,
 PolicyAdjuster);


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


[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

2019-03-20 Thread Phabricator via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL356613: [clang-format] BeforeHash added to 
IndentPPDirectives (authored by paulhoad, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52150?vs=191540=191573#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D52150

Files:
  cfe/trunk/docs/ClangFormatStyleOptions.rst
  cfe/trunk/docs/ReleaseNotes.rst
  cfe/trunk/include/clang/Format/Format.h
  cfe/trunk/lib/Format/Format.cpp
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
  cfe/trunk/lib/Format/UnwrappedLineParser.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp

Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -2993,22 +2993,25 @@
 EXPECT_EQ(Expected, format(ToFormat, Style));
 EXPECT_EQ(Expected, format(Expected, Style));
   }
-  // Test with tabs.
-  Style.UseTab = FormatStyle::UT_Always;
-  Style.IndentWidth = 8;
-  Style.TabWidth = 8;
-  verifyFormat("#ifdef _WIN32\n"
-   "#\tdefine A 0\n"
-   "#\tifdef VAR2\n"
-   "#\t\tdefine B 1\n"
-   "#\t\tinclude \n"
-   "#\t\tdefine MACRO  \\\n"
-   "\t\t\tsome_very_long_func_aa();\n"
-   "#\tendif\n"
-   "#else\n"
-   "#\tdefine A 1\n"
-   "#endif",
-   Style);
+  // Test AfterHash with tabs.
+  {
+FormatStyle Tabbed = Style;
+Tabbed.UseTab = FormatStyle::UT_Always;
+Tabbed.IndentWidth = 8;
+Tabbed.TabWidth = 8;
+verifyFormat("#ifdef _WIN32\n"
+ "#\tdefine A 0\n"
+ "#\tifdef VAR2\n"
+ "#\t\tdefine B 1\n"
+ "#\t\tinclude \n"
+ "#\t\tdefine MACRO  \\\n"
+ "\t\t\tsome_very_long_func_aa();\n"
+ "#\tendif\n"
+ "#else\n"
+ "#\tdefine A 1\n"
+ "#endif",
+ Tabbed);
+  }
 
   // Regression test: Multiline-macro inside include guards.
   verifyFormat("#ifndef HEADER_H\n"
@@ -3018,6 +3021,102 @@
"  int j;\n"
"#endif // HEADER_H",
getLLVMStyleWithColumns(20));
+
+  Style.IndentPPDirectives = FormatStyle::PPDIS_BeforeHash;
+  // Basic before hash indent tests
+  verifyFormat("#ifdef _WIN32\n"
+   "  #define A 0\n"
+   "  #ifdef VAR2\n"
+   "#define B 1\n"
+   "#include \n"
+   "#define MACRO  \\\n"
+   "  some_very_long_func_aa();\n"
+   "  #endif\n"
+   "#else\n"
+   "  #define A 1\n"
+   "#endif",
+   Style);
+  verifyFormat("#if A\n"
+   "  #define MACRO\\\n"
+   "void a(int x) {\\\n"
+   "  b(); \\\n"
+   "  c(); \\\n"
+   "  d(); \\\n"
+   "  e(); \\\n"
+   "  f(); \\\n"
+   "}\n"
+   "#endif",
+   Style);
+  // Keep comments aligned with indented directives. These
+  // tests cannot use verifyFormat because messUp manipulates leading
+  // whitespace.
+  {
+const char *Expected = "void f() {\n"
+   "// Aligned to preprocessor.\n"
+   "#if 1\n"
+   "  // Aligned to code.\n"
+   "  int a;\n"
+   "  #if 1\n"
+   "// Aligned to preprocessor.\n"
+   "#define A 0\n"
+   "  // Aligned to code.\n"
+   "  int b;\n"
+   "  #endif\n"
+   "#endif\n"
+   "}";
+const char *ToFormat = "void f() {\n"
+   "// Aligned to preprocessor.\n"
+   "#if 1\n"
+   "// Aligned to code.\n"
+   "int a;\n"
+   "#if 1\n"
+   "// Aligned to preprocessor.\n"
+   "#define A 0\n"
+   "// Aligned to code.\n"
+   "int b;\n"
+   "#endif\n"
+   "#endif\n"
+

r356613 - [clang-format] BeforeHash added to IndentPPDirectives

2019-03-20 Thread Paul Hoad via cfe-commits
Author: paulhoad
Date: Wed Mar 20 13:49:43 2019
New Revision: 356613

URL: http://llvm.org/viewvc/llvm-project?rev=356613=rev
Log:
[clang-format] BeforeHash added to IndentPPDirectives

Summary:
The option BeforeHash added to IndentPPDirectives.
Fixes Bug 36019. https://bugs.llvm.org/show_bug.cgi?id=36019

Reviewers: djasper, klimek, krasimir, sammccall, mprobst, Nicola, MyDeveloperDay

Reviewed By: klimek, MyDeveloperDay

Subscribers: kadircet, MyDeveloperDay, mnussbaum, geleji, ufna, cfe-commits

Patch by to-mix.

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

Modified:
cfe/trunk/docs/ClangFormatStyleOptions.rst
cfe/trunk/docs/ReleaseNotes.rst
cfe/trunk/include/clang/Format/Format.h
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=356613=356612=356613=diff
==
--- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst Wed Mar 20 13:49:43 2019
@@ -1411,6 +1411,16 @@ the configuration (without a prefix: ``A
#  endif
#endif
 
+  * ``PPDIS_BeforeHash`` (in configuration: ``BeforeHash``)
+Indents directives before the hash.
+
+.. code-block:: c++
+
+   #if FOO
+ #if BAR
+   #include 
+ #endif
+   #endif
 
 
 **IndentWidth** (``unsigned``)

Modified: cfe/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=356613=356612=356613=diff
==
--- cfe/trunk/docs/ReleaseNotes.rst (original)
+++ cfe/trunk/docs/ReleaseNotes.rst Wed Mar 20 13:49:43 2019
@@ -170,8 +170,8 @@ AST Matchers
 clang-format
 
 
-
-- ...
+- Added new option `PPDIS_BeforeHash` (in configuration: `BeforeHash`) to
+ `IndentPPDirectives` which indents preprocessor directives before the hash.
 
 libclang
 

Modified: cfe/trunk/include/clang/Format/Format.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=356613=356612=356613=diff
==
--- cfe/trunk/include/clang/Format/Format.h (original)
+++ cfe/trunk/include/clang/Format/Format.h Wed Mar 20 13:49:43 2019
@@ -1126,7 +1126,16 @@ struct FormatStyle {
 ///#  endif
 ///#endif
 /// \endcode
-PPDIS_AfterHash
+PPDIS_AfterHash,
+/// Indents directives before the hash.
+/// \code
+///#if FOO
+///  #if BAR
+///#include 
+///  #endif
+///#endif
+/// \endcode
+PPDIS_BeforeHash
   };
 
   /// The preprocessor directive indenting style to use.

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=356613=356612=356613=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Wed Mar 20 13:49:43 2019
@@ -174,6 +174,7 @@ struct ScalarEnumerationTraitshttp://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=356613=356612=356613=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Wed Mar 20 13:49:43 2019
@@ -1924,12 +1924,15 @@ void TokenAnnotator::setCommentLineLevel
 NextNonCommentLine->First->NewlinesBefore <= 1 &&
 NextNonCommentLine->First->OriginalColumn ==
 (*I)->First->OriginalColumn) {
-  // Align comments for preprocessor lines with the # in column 0.
-  // Otherwise, align with the next line.
-  (*I)->Level = (NextNonCommentLine->Type == LT_PreprocessorDirective ||
- NextNonCommentLine->Type == LT_ImportStatement)
-? 0
-: NextNonCommentLine->Level;
+  // Align comments for preprocessor lines with the # in column 0 if
+  // preprocessor lines are not indented. Otherwise, align with the next
+  // line.
+  (*I)->Level =
+  (Style.IndentPPDirectives != FormatStyle::PPDIS_BeforeHash &&
+   (NextNonCommentLine->Type == LT_PreprocessorDirective ||
+NextNonCommentLine->Type == LT_ImportStatement))
+  ? 0
+  : NextNonCommentLine->Level;
 } else {
   NextNonCommentLine = (*I)->First->isNot(tok::r_brace) ? (*I) : nullptr;
 }

Modified: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
URL: 

[PATCH] D59609: [clang][OpenMP] Fix build when using libgomp

2019-03-20 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev accepted this revision.
ABataev added a comment.
This revision is now accepted and ready to land.

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59609



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


[PATCH] D59609: [clang][OpenMP] Fix build when using libgomp

2019-03-20 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment.

If the compiler is built without OpenMP support -fopenmp is converted to 
-fopenmp=libgomp frontend option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59609



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


[PATCH] D59609: [clang][OpenMP] Fix build when using libgomp

2019-03-20 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a reviewer: ABataev.
lebedev.ri added a comment.

Interesting.
What happens if libomp is not being built?
What happens if libomp is not present?

Or more generally, why does it even matter whether there is runtime or not, 
this only does paring+sema, no codegen/execution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59609



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


[PATCH] D59533: [X86] Add __crc32b/__crc32w/__crc32d/__crc32q intrinsics to match gcc and icc.

2019-03-20 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL356609: [X86] Add __crc32b/__crc32w/__crc32d/__crc32q 
intrinsics to match gcc and icc. (authored by ctopper, committed by ).
Herald added subscribers: llvm-commits, jdoerfert.
Herald added a project: LLVM.

Changed prior to commit:
  https://reviews.llvm.org/D59533?vs=191251=191568#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59533

Files:
  cfe/trunk/lib/Headers/ia32intrin.h
  cfe/trunk/test/CodeGen/x86-crc-builtins.c

Index: cfe/trunk/test/CodeGen/x86-crc-builtins.c
===
--- cfe/trunk/test/CodeGen/x86-crc-builtins.c
+++ cfe/trunk/test/CodeGen/x86-crc-builtins.c
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-apple-darwin -target-feature +sse4.2 -emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefixes=CHECK,CHECK64
+// RUN: %clang_cc1 -ffreestanding %s -triple=i686-apple-darwin -target-feature +sse4.2 -emit-llvm -o - -Wall -Werror | FileCheck %s
+
+#include 
+
+unsigned int test__crc32b(unsigned int CRC, unsigned char V) {
+// CHECK-LABEL: test__crc32b
+// CHECK: call i32 @llvm.x86.sse42.crc32.32.8(i32 %{{.*}}, i8 %{{.*}})
+  return __crc32b(CRC, V);
+}
+
+unsigned int test__crc32w(unsigned int CRC, unsigned short V) {
+// CHECK-LABEL: test__crc32w
+// CHECK: call i32 @llvm.x86.sse42.crc32.32.16(i32 %{{.*}}, i16 %{{.*}})
+  return __crc32w(CRC, V);
+}
+
+unsigned int test__crc32d(unsigned int CRC, unsigned int V) {
+// CHECK-LABEL: test__crc32d
+// CHECK: call i32 @llvm.x86.sse42.crc32.32.32(i32 %{{.*}}, i32 %{{.*}})
+  return __crc32d(CRC, V);
+}
+
+#ifdef __x86_64__
+unsigned long long test__crc32q(unsigned long long CRC, unsigned long long V) {
+// CHECK64-LABEL: test__crc32q
+// CHECK64: call i64 @llvm.x86.sse42.crc32.64.64(i64 %{{.*}}, i64 %{{.*}})
+  return __crc32q(CRC, V);
+}
+#endif
Index: cfe/trunk/lib/Headers/ia32intrin.h
===
--- cfe/trunk/lib/Headers/ia32intrin.h
+++ cfe/trunk/lib/Headers/ia32intrin.h
@@ -55,6 +55,92 @@
 }
 #endif /* !__x86_64__ */
 
+/** Adds the unsigned integer operand to the CRC-32C checksum of the
+ * unsigned char operand.
+ *
+ *  \headerfile 
+ *
+ *  This intrinsic corresponds to the  CRC32B  instruction.
+ *
+ *  \param __C
+ * An unsigned integer operand to add to the CRC-32C checksum of operand
+ * \a  __D.
+ *  \param __D
+ * An unsigned 8-bit integer operand used to compute the CRC-32C checksum.
+ *  \returns The result of adding operand \a __C to the CRC-32C checksum of
+ * operand \a __D.
+ */
+static __inline__ unsigned int __attribute__((__always_inline__, __nodebug__, __target__("sse4.2")))
+__crc32b(unsigned int __C, unsigned char __D)
+{
+  return __builtin_ia32_crc32qi(__C, __D);
+}
+
+/** Adds the unsigned integer operand to the CRC-32C checksum of the
+ * unsigned short operand.
+ *
+ *  \headerfile 
+ *
+ *  This intrinsic corresponds to the  CRC32W  instruction.
+ *
+ *  \param __C
+ * An unsigned integer operand to add to the CRC-32C checksum of operand
+ * \a  __D.
+ *  \param __D
+ * An unsigned 16-bit integer operand used to compute the CRC-32C checksum.
+ *  \returns The result of adding operand \a __C to the CRC-32C checksum of
+ * operand \a __D.
+ */
+static __inline__ unsigned int __attribute__((__always_inline__, __nodebug__, __target__("sse4.2")))
+__crc32w(unsigned int __C, unsigned short __D)
+{
+  return __builtin_ia32_crc32hi(__C, __D);
+}
+
+/** Adds the unsigned integer operand to the CRC-32C checksum of the
+ * second unsigned integer operand.
+ *
+ *  \headerfile 
+ *
+ *  This intrinsic corresponds to the  CRC32D  instruction.
+ *
+ *  \param __C
+ * An unsigned integer operand to add to the CRC-32C checksum of operand
+ * \a  __D.
+ *  \param __D
+ * An unsigned 32-bit integer operand used to compute the CRC-32C checksum.
+ *  \returns The result of adding operand \a __C to the CRC-32C checksum of
+ * operand \a __D.
+ */
+static __inline__ unsigned int __attribute__((__always_inline__, __nodebug__, __target__("sse4.2")))
+__crc32d(unsigned int __C, unsigned int __D)
+{
+  return __builtin_ia32_crc32si(__C, __D);
+}
+
+#ifdef __x86_64__
+/** Adds the unsigned integer operand to the CRC-32C checksum of the
+ * unsigned 64-bit integer operand.
+ *
+ *  \headerfile 
+ *
+ *  This intrinsic corresponds to the  CRC32Q  instruction.
+ *
+ *  \param __C
+ * An unsigned integer operand to add to the CRC-32C checksum of operand
+ * \a  __D.
+ *  \param __D
+ * An unsigned 64-bit integer operand used to compute the CRC-32C checksum.
+ *  \returns The result of adding operand \a __C to the CRC-32C checksum of
+ * operand \a __D.
+ */
+static __inline__ unsigned long long __attribute__((__always_inline__, __nodebug__, __target__("sse4.2")))

r356609 - [X86] Add __crc32b/__crc32w/__crc32d/__crc32q intrinsics to match gcc and icc.

2019-03-20 Thread Craig Topper via cfe-commits
Author: ctopper
Date: Wed Mar 20 13:25:28 2019
New Revision: 356609

URL: http://llvm.org/viewvc/llvm-project?rev=356609=rev
Log:
[X86] Add __crc32b/__crc32w/__crc32d/__crc32q intrinsics to match gcc and icc.

gcc has these intrinsics in ia32intrin.h as well. And icc implements them
though they aren't documented in the Intel Intrinsics Guide.

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

Added:
cfe/trunk/test/CodeGen/x86-crc-builtins.c   (with props)
Modified:
cfe/trunk/lib/Headers/ia32intrin.h

Modified: cfe/trunk/lib/Headers/ia32intrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/ia32intrin.h?rev=356609=356608=356609=diff
==
--- cfe/trunk/lib/Headers/ia32intrin.h (original)
+++ cfe/trunk/lib/Headers/ia32intrin.h Wed Mar 20 13:25:28 2019
@@ -55,6 +55,92 @@ __writeeflags(unsigned int __f)
 }
 #endif /* !__x86_64__ */
 
+/** Adds the unsigned integer operand to the CRC-32C checksum of the
+ * unsigned char operand.
+ *
+ *  \headerfile 
+ *
+ *  This intrinsic corresponds to the  CRC32B  instruction.
+ *
+ *  \param __C
+ * An unsigned integer operand to add to the CRC-32C checksum of operand
+ * \a  __D.
+ *  \param __D
+ * An unsigned 8-bit integer operand used to compute the CRC-32C checksum.
+ *  \returns The result of adding operand \a __C to the CRC-32C checksum of
+ * operand \a __D.
+ */
+static __inline__ unsigned int __attribute__((__always_inline__, __nodebug__, 
__target__("sse4.2")))
+__crc32b(unsigned int __C, unsigned char __D)
+{
+  return __builtin_ia32_crc32qi(__C, __D);
+}
+
+/** Adds the unsigned integer operand to the CRC-32C checksum of the
+ * unsigned short operand.
+ *
+ *  \headerfile 
+ *
+ *  This intrinsic corresponds to the  CRC32W  instruction.
+ *
+ *  \param __C
+ * An unsigned integer operand to add to the CRC-32C checksum of operand
+ * \a  __D.
+ *  \param __D
+ * An unsigned 16-bit integer operand used to compute the CRC-32C checksum.
+ *  \returns The result of adding operand \a __C to the CRC-32C checksum of
+ * operand \a __D.
+ */
+static __inline__ unsigned int __attribute__((__always_inline__, __nodebug__, 
__target__("sse4.2")))
+__crc32w(unsigned int __C, unsigned short __D)
+{
+  return __builtin_ia32_crc32hi(__C, __D);
+}
+
+/** Adds the unsigned integer operand to the CRC-32C checksum of the
+ * second unsigned integer operand.
+ *
+ *  \headerfile 
+ *
+ *  This intrinsic corresponds to the  CRC32D  instruction.
+ *
+ *  \param __C
+ * An unsigned integer operand to add to the CRC-32C checksum of operand
+ * \a  __D.
+ *  \param __D
+ * An unsigned 32-bit integer operand used to compute the CRC-32C checksum.
+ *  \returns The result of adding operand \a __C to the CRC-32C checksum of
+ * operand \a __D.
+ */
+static __inline__ unsigned int __attribute__((__always_inline__, __nodebug__, 
__target__("sse4.2")))
+__crc32d(unsigned int __C, unsigned int __D)
+{
+  return __builtin_ia32_crc32si(__C, __D);
+}
+
+#ifdef __x86_64__
+/** Adds the unsigned integer operand to the CRC-32C checksum of the
+ * unsigned 64-bit integer operand.
+ *
+ *  \headerfile 
+ *
+ *  This intrinsic corresponds to the  CRC32Q  instruction.
+ *
+ *  \param __C
+ * An unsigned integer operand to add to the CRC-32C checksum of operand
+ * \a  __D.
+ *  \param __D
+ * An unsigned 64-bit integer operand used to compute the CRC-32C checksum.
+ *  \returns The result of adding operand \a __C to the CRC-32C checksum of
+ * operand \a __D.
+ */
+static __inline__ unsigned long long __attribute__((__always_inline__, 
__nodebug__, __target__("sse4.2")))
+__crc32q(unsigned long long __C, unsigned long long __D)
+{
+  return __builtin_ia32_crc32di(__C, __D);
+}
+#endif /* __x86_64__ */
+
 static __inline__ unsigned long long __attribute__((__always_inline__, 
__nodebug__))
 __rdpmc(int __A) {
   return __builtin_ia32_rdpmc(__A);

Added: cfe/trunk/test/CodeGen/x86-crc-builtins.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/x86-crc-builtins.c?rev=356609=auto
==
--- cfe/trunk/test/CodeGen/x86-crc-builtins.c (added)
+++ cfe/trunk/test/CodeGen/x86-crc-builtins.c Wed Mar 20 13:25:28 2019
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-apple-darwin 
-target-feature +sse4.2 -emit-llvm -o - -Wall -Werror | FileCheck %s 
--check-prefixes=CHECK,CHECK64
+// RUN: %clang_cc1 -ffreestanding %s -triple=i686-apple-darwin -target-feature 
+sse4.2 -emit-llvm -o - -Wall -Werror | FileCheck %s
+
+#include 
+
+unsigned int test__crc32b(unsigned int CRC, unsigned char V) {
+// CHECK-LABEL: test__crc32b
+// CHECK: call i32 @llvm.x86.sse42.crc32.32.8(i32 %{{.*}}, i8 %{{.*}})
+  return __crc32b(CRC, V);
+}
+
+unsigned int test__crc32w(unsigned int CRC, unsigned short V) {
+// CHECK-LABEL: test__crc32w
+// CHECK: 

[PATCH] D59609: [clang][OpenMP] Fix build when using libgomp

2019-03-20 Thread Jordan Rupprecht via Phabricator via cfe-commits
rupprecht created this revision.
rupprecht added a reviewer: lebedev.ri.
Herald added subscribers: cfe-commits, jdoerfert, guansong.
Herald added a project: clang.

rL356570  introduced a test which only 
passes with the default openmp library, libomp, and fails with other openmp 
libraries, such as libgomp. Explicitly choose libomp.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59609

Files:
  clang/unittests/AST/OMPStructuredBlockTest.cpp


Index: clang/unittests/AST/OMPStructuredBlockTest.cpp
===
--- clang/unittests/AST/OMPStructuredBlockTest.cpp
+++ clang/unittests/AST/OMPStructuredBlockTest.cpp
@@ -55,7 +55,7 @@
   StringRef ExpectedPrinted,
   PolicyAdjusterType PolicyAdjuster = None) {
   std::vector Args = {
-  "-fopenmp",
+  "-fopenmp=libomp",
   };
   return PrintedStmtMatches(Code, Args, NodeMatch, ExpectedPrinted,
 PolicyAdjuster);


Index: clang/unittests/AST/OMPStructuredBlockTest.cpp
===
--- clang/unittests/AST/OMPStructuredBlockTest.cpp
+++ clang/unittests/AST/OMPStructuredBlockTest.cpp
@@ -55,7 +55,7 @@
   StringRef ExpectedPrinted,
   PolicyAdjusterType PolicyAdjuster = None) {
   std::vector Args = {
-  "-fopenmp",
+  "-fopenmp=libomp",
   };
   return PrintedStmtMatches(Code, Args, NodeMatch, ExpectedPrinted,
 PolicyAdjuster);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r356607 - [OPENMP]Improve detection of omp_allocator_handle_t type and predefined

2019-03-20 Thread Alexey Bataev via cfe-commits
Author: abataev
Date: Wed Mar 20 13:14:22 2019
New Revision: 356607

URL: http://llvm.org/viewvc/llvm-project?rev=356607=rev
Log:
[OPENMP]Improve detection of omp_allocator_handle_t type and predefined
allocators.

It is better to deduce omp_allocator_handle_t type from the predefined
allocators, because omp.h header might not define it explicitly. Plus,
it allows to identify the predefined allocators correctly when trying to
build the allcoator for the global variables.

Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp
cfe/trunk/test/OpenMP/allocate_allocator_messages.cpp
cfe/trunk/test/PCH/chain-openmp-allocate.cpp

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=356607=356606=356607=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Wed Mar 20 13:14:22 2019
@@ -3151,7 +3151,22 @@ def OMPAllocateDecl : InheritableAttr {
   // This attribute has no spellings as it is only ever created implicitly.
   let Spellings = [];
   let SemaHandler = 0;
-  let Args = [ExprArgument<"Allocator">];
+  let Args = [
+EnumArgument<"AllocatorType", "AllocatorTypeTy",
+ [
+   "omp_default_mem_alloc", "omp_large_cap_mem_alloc",
+   "omp_const_mem_alloc", "omp_high_bw_mem_alloc",
+   "omp_low_lat_mem_alloc", "omp_cgroup_mem_alloc",
+   "omp_pteam_mem_alloc", "omp_thread_mem_alloc", ""
+ ],
+ [
+   "OMPDefaultMemAlloc", "OMPLargeCapMemAlloc",
+   "OMPConstMemAlloc", "OMPHighBWMemAlloc", 
"OMPLowLatMemAlloc",
+   "OMPCGroupMemAlloc", "OMPPTeamMemAlloc", 
"OMPThreadMemAlloc",
+   "OMPUserDefinedMemAlloc"
+ ]>,
+ExprArgument<"Allocator">
+  ];
   let Documentation = [Undocumented];
 }
 

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=356607=356606=356607=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Wed Mar 20 13:14:22 2019
@@ -8773,8 +8773,6 @@ public:
   //
 private:
   void *VarDataSharingAttributesStack;
-  /// omp_allocator_handle_t type.
-  QualType OMPAllocatorHandleT;
   /// Number of nested '#pragma omp declare target' directives.
   unsigned DeclareTargetNestingLevel = 0;
   /// Initialization of data-sharing attributes stack.

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=356607=356606=356607=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Wed Mar 20 13:14:22 2019
@@ -188,10 +188,29 @@ private:
 
   /// Vector of previously declared requires directives
   SmallVector RequiresDecls;
+  /// omp_allocator_handle_t type.
+  QualType OMPAllocatorHandleT;
+  /// Expression for the predefined allocators.
+  Expr *OMPPredefinedAllocators[OMPAllocateDeclAttr::OMPUserDefinedMemAlloc] = 
{
+  nullptr};
 
 public:
   explicit DSAStackTy(Sema ) : SemaRef(S) {}
 
+  /// Sets omp_allocator_handle_t type.
+  void setOMPAllocatorHandleT(QualType Ty) { OMPAllocatorHandleT = Ty; }
+  /// Gets omp_allocator_handle_t type.
+  QualType getOMPAllocatorHandleT() const { return OMPAllocatorHandleT; }
+  /// Sets the given default allocator.
+  void setAllocator(OMPAllocateDeclAttr::AllocatorTypeTy AllocatorKind,
+Expr *Allocator) {
+OMPPredefinedAllocators[AllocatorKind] = Allocator;
+  }
+  /// Returns the specified default allocator.
+  Expr *getAllocator(OMPAllocateDeclAttr::AllocatorTypeTy AllocatorKind) const 
{
+return OMPPredefinedAllocators[AllocatorKind];
+  }
+
   bool isClauseParsingMode() const { return ClauseKindMode != OMPC_unknown; }
   OpenMPClauseKind getClauseParsingMode() const {
 assert(isClauseParsingMode() && "Must be in clause parsing mode.");
@@ -2194,6 +2213,44 @@ Sema::CheckOMPThreadPrivateDecl(SourceLo
   return D;
 }
 
+static OMPAllocateDeclAttr::AllocatorTypeTy
+getAllocatorKind(Sema , DSAStackTy *Stack, Expr *Allocator) {
+  if (!Allocator)
+return OMPAllocateDeclAttr::OMPDefaultMemAlloc;
+  if (Allocator->isTypeDependent() || Allocator->isValueDependent() ||
+  Allocator->isInstantiationDependent() ||
+  Allocator->containsUnexpandedParameterPack() ||
+  !Allocator->isEvaluatable(S.getASTContext()))
+return OMPAllocateDeclAttr::OMPUserDefinedMemAlloc;
+  bool Suppress 

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

2019-03-20 Thread Jordan Rupprecht via Phabricator via cfe-commits
rupprecht added inline comments.



Comment at: cfe/trunk/unittests/AST/OMPStructuredBlockTest.cpp:58
+  std::vector Args = {
+  "-fopenmp",
+  };

Looks like this test fails when the default is not libomp, e.g. 
DCLANG_DEFAULT_OPENMP_RUNTIME=libgomp
Using -fopenmp=libomp explicitly here causes the test to pass in the case. Does 
that seem like the right fix, or has something gone wrong?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59214



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


r356605 - Fix implicit ios -> watchOS availability version mapping for

2019-03-20 Thread Alex Lorenz via cfe-commits
Author: arphaman
Date: Wed Mar 20 13:02:00 2019
New Revision: 356605

URL: http://llvm.org/viewvc/llvm-project?rev=356605=rev
Log:
Fix implicit ios -> watchOS availability version mapping for
versions that have the major number only

rdar://48018651

Modified:
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/Sema/attr-availability-watchos.c

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=356605=356604=356605=diff
==
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Wed Mar 20 13:02:00 2019
@@ -2508,6 +2508,7 @@ static void handleAvailabilityAttr(Sema
   else
 return VersionTuple(NewMajor, Version.getMinor().getValue());
 }
+return VersionTuple(NewMajor);
   }
 
   return VersionTuple(2, 0);

Modified: cfe/trunk/test/Sema/attr-availability-watchos.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-availability-watchos.c?rev=356605=356604=356605=diff
==
--- cfe/trunk/test/Sema/attr-availability-watchos.c (original)
+++ cfe/trunk/test/Sema/attr-availability-watchos.c Wed Mar 20 13:02:00 2019
@@ -52,3 +52,9 @@ void test_watchos() {
   f5c_watchos(0); // expected-warning {{'f5c_watchos' is deprecated: first 
deprecated in watchOS 2.0}}
   f6_watchos(0); // expected-warning {{'f6_watchos' is deprecated: first 
deprecated in watchOS 3.0}}
 }
+
+void deprecatedAfterIntroduced() 
__attribute__((availability(ios,introduced=9.3,deprecated=10))); // 
expected-note {{here}}
+
+void test_ios_correctly_map_to_watchos() {
+  deprecatedAfterIntroduced(); // expected-warning 
{{'deprecatedAfterIntroduced' is deprecated: first deprecated in watchOS 3}}
+}


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


[PATCH] D17482: [clang-tidy] Allow tests to verify changes made to header files

2019-03-20 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood abandoned this revision.
LegalizeAdulthood added a comment.
Herald added a subscriber: xazax.hun.
Herald added a reviewer: serge-sans-paille.

Review process takes too long to make forward progress; abandoning.


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

https://reviews.llvm.org/D17482



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


[PATCH] D7982: Add readability-duplicate-include check to clang-tidy

2019-03-20 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood abandoned this revision.
LegalizeAdulthood added a comment.
Herald added subscribers: jdoerfert, mgorny.

Review takes too long to make forward progress; abandoning.


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

https://reviews.llvm.org/D7982



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


[PATCH] D18509: clang-tidy: add_new_check.py stubs out release notes

2019-03-20 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood abandoned this revision.
LegalizeAdulthood added a comment.
Herald added a reviewer: serge-sans-paille.

Review takes too long to make forward progress; abandoning.


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

https://reviews.llvm.org/D18509



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


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

2019-03-20 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood marked 4 inline comments as done.
LegalizeAdulthood added inline comments.
Herald added a subscriber: jdoerfert.



Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:386
 
-  bool BoolValue = Bool->getValue();
+  const bool BoolValue = Bool->getValue();
 

JonasToth wrote:
> aaron.ballman wrote:
> > LegalizeAdulthood wrote:
> > > JonasToth wrote:
> > > > `const` on values is uncommon in clang-tidy code. Please keep that 
> > > > consistent.
> > > I can change the code, but I don't understand the push back.
> > > 
> > > "That's the way it's done elsewhere" just doesn't seem like good 
> > > reasoning.
> > > 
> > > I write const on values to signify that they are computed once and only 
> > > once.  What is gained by removing that statement of once-and-only-once?
> > > "That's the way it's done elsewhere" just doesn't seem like good 
> > > reasoning.
> > 
> > Keeping the code consistent with the vast majority of the remainder of the 
> > project is valid reasoning. I am echoing the request to drop the top-level 
> > const. We don't use this pattern for non-pointer/reference types and 
> > there's not a whole lot gained by using it inconsistently.
> Plus we do have a clang-tidy check (in the pipeline) that could do that 
> automatically if the LLVM projects decides to go that route. So we won't 
> suffer in the future, if we add the const.
You haven't addressed my point, which is that `const` is for values that don't 
change.  Instead, you're just repeating "we haven't done it that way" instead 
of refuting the utility of `const`.


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

https://reviews.llvm.org/D56303



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


[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

2019-03-20 Thread Reuben Thomas via Phabricator via cfe-commits
reuk updated this revision to Diff 191560.
reuk added a comment.

Removed unnecessary parens.


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

https://reviews.llvm.org/D55170

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/TokenAnnotator.cpp
  lib/Format/TokenAnnotator.h
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -9337,6 +9337,60 @@
   verifyFormat("T A::operator() ();", Space);
   verifyFormat("X A::operator++ (T);", Space);
   verifyFormat("auto lambda = [] () { return 0; };", Space);
+  verifyFormat("int x = int (y);", Space);
+
+  FormatStyle SomeSpace = getLLVMStyle();
+  SomeSpace.SpaceBeforeParens = FormatStyle::SBPO_NonEmptyParentheses;
+
+  verifyFormat("[]() -> float {}", SomeSpace);
+  verifyFormat("[] (auto foo) {}", SomeSpace);
+  verifyFormat("[foo]() -> int {}", SomeSpace);
+  verifyFormat("int f();", SomeSpace);
+  verifyFormat("void f (int a, T b) {\n"
+   "  while (true)\n"
+   "continue;\n"
+   "}",
+   SomeSpace);
+  verifyFormat("if (true)\n"
+   "  f();\n"
+   "else if (true)\n"
+   "  f();",
+   SomeSpace);
+  verifyFormat("do {\n"
+   "  do_something();\n"
+   "} while (something());",
+   SomeSpace);
+  verifyFormat("switch (x) {\n"
+   "default:\n"
+   "  break;\n"
+   "}",
+   SomeSpace);
+  verifyFormat("A::A() : a (1) {}", SomeSpace);
+  verifyFormat("void f() __attribute__ ((asdf));", SomeSpace);
+  verifyFormat("*( + 1);\n"
+   "&(()[1]);\n"
+   "a[(b + c) * d];\n"
+   "(((a + 1) * 2) + 3) * 4;",
+   SomeSpace);
+  verifyFormat("#define A(x) x", SomeSpace);
+  verifyFormat("#define A (x) x", SomeSpace);
+  verifyFormat("#if defined(x)\n"
+   "#endif",
+   SomeSpace);
+  verifyFormat("auto i = std::make_unique (5);", SomeSpace);
+  verifyFormat("size_t x = sizeof (x);", SomeSpace);
+  verifyFormat("auto f (int x) -> decltype (x);", SomeSpace);
+  verifyFormat("int f (T x) noexcept (x.create());", SomeSpace);
+  verifyFormat("alignas (128) char a[128];", SomeSpace);
+  verifyFormat("size_t x = alignof (MyType);", SomeSpace);
+  verifyFormat("static_assert (sizeof (char) == 1, \"Impossible!\");",
+   SomeSpace);
+  verifyFormat("int f() throw (Deprecated);", SomeSpace);
+  verifyFormat("typedef void (*cb) (int);", SomeSpace);
+  verifyFormat("T A::operator()();", SomeSpace);
+  verifyFormat("X A::operator++ (T);", SomeSpace);
+  verifyFormat("int x = int (y);", SomeSpace);
+  verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
 }
 
 TEST_F(FormatTest, ConfigurableSpacesInParentheses) {
@@ -11121,6 +11175,8 @@
   FormatStyle::SBPO_Always);
   CHECK_PARSE("SpaceBeforeParens: ControlStatements", SpaceBeforeParens,
   FormatStyle::SBPO_ControlStatements);
+  CHECK_PARSE("SpaceBeforeParens: NonEmptyParentheses", SpaceBeforeParens,
+  FormatStyle::SBPO_NonEmptyParentheses);
   // For backward compatibility:
   CHECK_PARSE("SpaceAfterControlStatementKeyword: false", SpaceBeforeParens,
   FormatStyle::SBPO_Never);
Index: lib/Format/TokenAnnotator.h
===
--- lib/Format/TokenAnnotator.h
+++ lib/Format/TokenAnnotator.h
@@ -164,6 +164,8 @@
   unsigned splitPenalty(const AnnotatedLine , const FormatToken ,
 bool InFunctionDecl);
 
+  bool spaceRequiredBeforeParens(const FormatToken ) const;
+
   bool spaceRequiredBetween(const AnnotatedLine , const FormatToken ,
 const FormatToken );
 
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2403,6 +2403,12 @@
   return 3;
 }
 
+bool TokenAnnotator::spaceRequiredBeforeParens(const FormatToken ) const {
+  return Style.SpaceBeforeParens == FormatStyle::SBPO_Always ||
+ (Style.SpaceBeforeParens == FormatStyle::SBPO_NonEmptyParentheses &&
+  Right.ParameterCount > 0);
+}
+
 bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine ,
   const FormatToken ,
   const FormatToken ) {
@@ -2549,9 +2555,9 @@
  (Left.isOneOf(tok::kw_try, Keywords.kw___except, tok::kw_catch,
tok::kw_new, tok::kw_delete) &&
   (!Left.Previous || Left.Previous->isNot(tok::period) ||
-   (Style.SpaceBeforeParens == FormatStyle::SBPO_Always &&
+   (spaceRequiredBeforeParens(Right) &&
 (Left.is(tok::identifier) || 

[PATCH] D56323: [clang-tidy] Handle member variables in readability-simplify-boolean-expr

2019-03-20 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment.
Herald added a subscriber: jdoerfert.

In D56323#1350998 , @JonasToth wrote:

> @LegalizeAdulthood your stuck on the commit right things right? If you want I 
> can commit for you (maybe even later) as you said you are limited on time.


Yes, please.  I haven't figured out what the new commit procedure is or how I 
can submit.  If you could submit this for me, that would help me out, thanks.


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

https://reviews.llvm.org/D56323



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


[PATCH] D59394: [Sema] De-duplicate some availability checking logic

2019-03-20 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL356599: [Sema] Deduplicate some availability checking logic 
(authored by epilk, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59394?vs=190747=191557#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59394

Files:
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/include/clang/Sema/Sema.h
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/lib/Sema/SemaExprCXX.cpp
  cfe/trunk/lib/Sema/SemaInit.cpp
  cfe/trunk/lib/Sema/SemaOverload.cpp
  cfe/trunk/test/Sema/enable_if.c
  cfe/trunk/test/Sema/overloadable.c
  cfe/trunk/test/SemaCXX/attr-unavailable.cpp
  cfe/trunk/test/SemaCXX/coroutines.cpp
  cfe/trunk/test/SemaObjCXX/overload.mm
  cfe/trunk/test/SemaTemplate/instantiate-expr-4.cpp

Index: cfe/trunk/include/clang/Sema/Sema.h
===
--- cfe/trunk/include/clang/Sema/Sema.h
+++ cfe/trunk/include/clang/Sema/Sema.h
@@ -2634,13 +2634,6 @@
   bool IsOverload(FunctionDecl *New, FunctionDecl *Old, bool IsForUsingDecl,
   bool ConsiderCudaAttrs = true);
 
-  /// Checks availability of the function depending on the current
-  /// function context.Inside an unavailable function,unavailability is ignored.
-  ///
-  /// \returns true if \p FD is unavailable and current context is inside
-  /// an available function, false otherwise.
-  bool isFunctionConsideredUnavailable(FunctionDecl *FD);
-
   ImplicitConversionSequence
   TryImplicitConversion(Expr *From, QualType ToType,
 bool SuppressUserConversions,
@@ -4102,7 +4095,6 @@
  ObjCInterfaceDecl *ClassReciever = nullptr);
   void NoteDeletedFunction(FunctionDecl *FD);
   void NoteDeletedInheritingConstructor(CXXConstructorDecl *CD);
-  std::string getDeletedOrUnavailableSuffix(const FunctionDecl *FD);
   bool DiagnosePropertyAccessorMismatch(ObjCPropertyDecl *PD,
 ObjCMethodDecl *Getter,
 SourceLocation Loc);
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3605,12 +3605,11 @@
   "no matching member function for call to %0">;
 def err_ovl_ambiguous_call : Error<
   "call to %0 is ambiguous">;
-def err_ovl_deleted_call : Error<
-  "call to %select{unavailable|deleted}0 function %1%2">;
+def err_ovl_deleted_call : Error<"call to deleted function %0">;
 def err_ovl_ambiguous_member_call : Error<
   "call to member function %0 is ambiguous">;
 def err_ovl_deleted_member_call : Error<
-  "call to %select{unavailable|deleted}0 member function %1%2">;
+  "call to deleted member function %0">;
 def note_ovl_too_many_candidates : Note<
 "remaining %0 candidate%s0 omitted; "
 "pass -fshow-overloads=all to show them">;
@@ -3824,7 +3823,7 @@
 def err_ref_init_ambiguous : Error<
   "reference initialization of type %0 with initializer of type %1 is ambiguous">;
 def err_ovl_deleted_init : Error<
-  "call to %select{unavailable|deleted}0 constructor of %1">;
+  "call to deleted constructor of %0">;
 def err_ovl_deleted_special_init : Error<
   "call to implicitly-deleted %select{default constructor|copy constructor|"
   "move constructor|copy assignment operator|move assignment operator|"
@@ -3836,7 +3835,7 @@
 def err_ovl_no_viable_oper : Error<"no viable overloaded '%0'">;
 def note_assign_lhs_incomplete : Note<"type %0 is incomplete">;
 def err_ovl_deleted_oper : Error<
-  "overload resolution selected %select{unavailable|deleted}0 operator '%1'%2">;
+  "overload resolution selected deleted operator '%0'">;
 def err_ovl_deleted_special_oper : Error<
   "object of type %0 cannot be %select{constructed|copied|moved|assigned|"
   "assigned|destroyed}1 because its %sub{select_special_member_kind}1 is implicitly deleted">;
@@ -3857,7 +3856,7 @@
 def err_ovl_ambiguous_object_call : Error<
   "call to object of type %0 is ambiguous">;
 def err_ovl_deleted_object_call : Error<
-  "call to %select{unavailable|deleted}0 function call operator in type %1%2">;
+  "call to deleted function call operator in type %0">;
 def note_ovl_surrogate_cand : Note<"conversion candidate of type %0">;
 def err_member_call_without_object : Error<
   "call to non-static member function without an object argument">;
Index: cfe/trunk/test/SemaCXX/coroutines.cpp
===
--- cfe/trunk/test/SemaCXX/coroutines.cpp
+++ cfe/trunk/test/SemaCXX/coroutines.cpp
@@ -649,26 +649,26 @@
 
 struct bad_promise_base {
 private:
-  void return_void();
+  void return_void(); // expected-note 2 {{declared 

r356600 - Add a __has_extension check for '#pragma clang attribute' as an external-declaration

2019-03-20 Thread Erik Pilkington via cfe-commits
Author: epilk
Date: Wed Mar 20 12:26:37 2019
New Revision: 356600

URL: http://llvm.org/viewvc/llvm-project?rev=356600=rev
Log:
Add a __has_extension check for '#pragma clang attribute' as an 
external-declaration

This was added in r356075.

Modified:
cfe/trunk/include/clang/Basic/Features.def
cfe/trunk/test/Parser/pragma-attribute-context.cpp

Modified: cfe/trunk/include/clang/Basic/Features.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Features.def?rev=356600=356599=356600=diff
==
--- cfe/trunk/include/clang/Basic/Features.def (original)
+++ cfe/trunk/include/clang/Basic/Features.def Wed Mar 20 12:26:37 2019
@@ -247,6 +247,7 @@ EXTENSION(cxx_variable_templates, LangOp
 // Miscellaneous language extensions
 EXTENSION(overloadable_unmarked, true)
 EXTENSION(pragma_clang_attribute_namespaces, true)
+EXTENSION(pragma_clang_attribute_external_declaration, true)
 
 #undef EXTENSION
 #undef FEATURE

Modified: cfe/trunk/test/Parser/pragma-attribute-context.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/pragma-attribute-context.cpp?rev=356600=356599=356600=diff
==
--- cfe/trunk/test/Parser/pragma-attribute-context.cpp (original)
+++ cfe/trunk/test/Parser/pragma-attribute-context.cpp Wed Mar 20 12:26:37 2019
@@ -1,6 +1,10 @@
 // RUN: %clang_cc1 -triple x86_64-apple-darwin9.0.0 -verify -std=c++11 %s
 // RUN: %clang_cc1 -triple x86_64-apple-darwin9.0.0 -xobjective-c++ -verify 
-std=c++11 %s
 
+#if !__has_extension(pragma_clang_attribute_external_declaration)
+#error
+#endif
+
 #define BEGIN_PRAGMA _Pragma("clang attribute push 
(__attribute__((availability(macos, introduced=1000))), apply_to=function)")
 #define END_PRAGMA _Pragma("clang attribute pop")
 


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


r356599 - [Sema] Deduplicate some availability checking logic

2019-03-20 Thread Erik Pilkington via cfe-commits
Author: epilk
Date: Wed Mar 20 12:26:33 2019
New Revision: 356599

URL: http://llvm.org/viewvc/llvm-project?rev=356599=rev
Log:
[Sema] Deduplicate some availability checking logic

Before this commit, we emit unavailable errors for calls to functions during
overload resolution, and for references to all other declarations in
DiagnoseUseOfDecl. The early checks during overload resolution aren't as good as
the DiagnoseAvailabilityOfDecl based checks, as they error on the code from
PR40991. This commit fixes this by removing the early checking.

llvm.org/PR40991
rdar://48564179

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

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/test/Sema/enable_if.c
cfe/trunk/test/Sema/overloadable.c
cfe/trunk/test/SemaCXX/attr-unavailable.cpp
cfe/trunk/test/SemaCXX/coroutines.cpp
cfe/trunk/test/SemaObjCXX/overload.mm
cfe/trunk/test/SemaTemplate/instantiate-expr-4.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=356599=356598=356599=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Mar 20 12:26:33 
2019
@@ -3605,12 +3605,11 @@ def err_ovl_no_viable_member_function_in
   "no matching member function for call to %0">;
 def err_ovl_ambiguous_call : Error<
   "call to %0 is ambiguous">;
-def err_ovl_deleted_call : Error<
-  "call to %select{unavailable|deleted}0 function %1%2">;
+def err_ovl_deleted_call : Error<"call to deleted function %0">;
 def err_ovl_ambiguous_member_call : Error<
   "call to member function %0 is ambiguous">;
 def err_ovl_deleted_member_call : Error<
-  "call to %select{unavailable|deleted}0 member function %1%2">;
+  "call to deleted member function %0">;
 def note_ovl_too_many_candidates : Note<
 "remaining %0 candidate%s0 omitted; "
 "pass -fshow-overloads=all to show them">;
@@ -3824,7 +3823,7 @@ def err_ovl_ambiguous_init : Error<"call
 def err_ref_init_ambiguous : Error<
   "reference initialization of type %0 with initializer of type %1 is 
ambiguous">;
 def err_ovl_deleted_init : Error<
-  "call to %select{unavailable|deleted}0 constructor of %1">;
+  "call to deleted constructor of %0">;
 def err_ovl_deleted_special_init : Error<
   "call to implicitly-deleted %select{default constructor|copy constructor|"
   "move constructor|copy assignment operator|move assignment operator|"
@@ -3836,7 +3835,7 @@ def err_ovl_ambiguous_oper_binary : Erro
 def err_ovl_no_viable_oper : Error<"no viable overloaded '%0'">;
 def note_assign_lhs_incomplete : Note<"type %0 is incomplete">;
 def err_ovl_deleted_oper : Error<
-  "overload resolution selected %select{unavailable|deleted}0 operator 
'%1'%2">;
+  "overload resolution selected deleted operator '%0'">;
 def err_ovl_deleted_special_oper : Error<
   "object of type %0 cannot be %select{constructed|copied|moved|assigned|"
   "assigned|destroyed}1 because its %sub{select_special_member_kind}1 is 
implicitly deleted">;
@@ -3857,7 +3856,7 @@ def err_ovl_no_viable_object_call : Erro
 def err_ovl_ambiguous_object_call : Error<
   "call to object of type %0 is ambiguous">;
 def err_ovl_deleted_object_call : Error<
-  "call to %select{unavailable|deleted}0 function call operator in type %1%2">;
+  "call to deleted function call operator in type %0">;
 def note_ovl_surrogate_cand : Note<"conversion candidate of type %0">;
 def err_member_call_without_object : Error<
   "call to non-static member function without an object argument">;

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=356599=356598=356599=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Wed Mar 20 12:26:33 2019
@@ -2634,13 +2634,6 @@ public:
   bool IsOverload(FunctionDecl *New, FunctionDecl *Old, bool IsForUsingDecl,
   bool ConsiderCudaAttrs = true);
 
-  /// Checks availability of the function depending on the current
-  /// function context.Inside an unavailable function,unavailability is 
ignored.
-  ///
-  /// \returns true if \p FD is unavailable and current context is inside
-  /// an available function, false otherwise.
-  bool isFunctionConsideredUnavailable(FunctionDecl *FD);
-
   ImplicitConversionSequence
   TryImplicitConversion(Expr *From, QualType ToType,
 bool SuppressUserConversions,
@@ -4102,7 +4095,6 @@ public:
  ObjCInterfaceDecl *ClassReciever 

r356598 - Recommit "Support attribute used in member funcs of class templates"

2019-03-20 Thread Rafael Auler via cfe-commits
Author: rafauler
Date: Wed Mar 20 12:22:24 2019
New Revision: 356598

URL: http://llvm.org/viewvc/llvm-project?rev=356598=rev
Log:
Recommit "Support attribute used in member funcs of class templates"

This diff previously exposed a bug in LLVM's IRLinker, breaking
buildbots that tried to self-host LLVM with monolithic LTO.
The bug is now in LLVM by D59552

Original commit message:
As PR17480 describes, clang does not support the used attribute
for member functions of class templates. This means that if the member
function is not used, its definition is never instantiated. This patch
changes clang to emit the definition if it has the used attribute.

Test Plan: Added a testcase

Reviewed By: aaron.ballman

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

Added:

cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
Modified:
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp

Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=356598=356597=356598=diff
==
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Wed Mar 20 12:22:24 2019
@@ -2232,6 +2232,20 @@ TemplateDeclInstantiator::VisitCXXMethod
 Owner->addDecl(Method);
   }
 
+  // PR17480: Honor the used attribute to instantiate member function
+  // definitions
+  if (Method->hasAttr()) {
+if (const auto *A = dyn_cast(Owner)) {
+  SourceLocation Loc;
+  if (const MemberSpecializationInfo *MSInfo =
+  A->getMemberSpecializationInfo())
+Loc = MSInfo->getPointOfInstantiation();
+  else if (const auto *Spec = dyn_cast(A))
+Loc = Spec->getPointOfInstantiation();
+  SemaRef.MarkFunctionReferenced(Loc, Method);
+}
+  }
+
   return Method;
 }
 

Added: 
cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp?rev=356598=auto
==
--- 
cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp 
(added)
+++ 
cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp 
Wed Mar 20 12:22:24 2019
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s | 
FileCheck %s
+
+// Check that PR17480 is fixed: __attribute__((used)) ignored in templated
+// classes
+namespace InstantiateUsedMemberDefinition {
+template 
+struct S {
+  int __attribute__((used)) f() {
+return 0;
+  }
+};
+
+void test() {
+  // Check that InstantiateUsedMemberDefinition::S::f() is defined
+  // as a result of the S class template implicit instantiation
+  // CHECK: define linkonce_odr i32 
@_ZN31InstantiateUsedMemberDefinition1SIiE1fEv
+  S inst;
+}
+} // namespace InstantiateUsedMemberDefinition


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


RE: [clang-tools-extra] r356565 - Reland r356547 after fixing the tests for Linux.

2019-03-20 Thread via cfe-commits
Hi Zinovy,

The test you modified is still failing on the PS4 linux bot after your change:

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/45557

FAIL: Clang Tools :: clang-tidy/clang-tidy-diff.cpp (14410 of 47824)
 TEST 'Clang Tools :: clang-tidy/clang-tidy-diff.cpp' 
FAILED 
Script:
--
: 'RUN: at line 2';   sed 's/placeholder_for_f/f/' 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-tidy/clang-tidy-diff.cpp
 > 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/clang-tidy-diff.cpp.tmp.cpp
: 'RUN: at line 3';   clang-tidy -checks=-*,modernize-use-override 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/clang-tidy-diff.cpp.tmp.cpp
 -- -std=c++11 | FileCheck -check-prefix=CHECK-SANITY 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-tidy/clang-tidy-diff.cpp
: 'RUN: at line 4';   not diff -U0 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-tidy/clang-tidy-diff.cpp
 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/clang-tidy-diff.cpp.tmp.cpp
 | /usr/bin/python2.7 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/../test/../clang-tidy/tool/clang-tidy-diff.py
 -checks=-*,modernize-use-override -- -std=c++11 2>&1 | FileCheck 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-tidy/clang-tidy-diff.cpp
: 'RUN: at line 5';   not diff -U0 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-tidy/clang-tidy-diff.cpp
 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/clang-tidy-diff.cpp.tmp.cpp
 | /usr/bin/python2.7 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/../test/../clang-tidy/tool/clang-tidy-diff.py
 -checks=-*,modernize-use-override -quiet -- -std=c++11 2>&1 | FileCheck 
-check-prefix=CHECK-QUIET 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-tidy/clang-tidy-diff.cpp
: 'RUN: at line 6';   mkdir -p 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/compilation-database-test/
: 'RUN: at line 7';   echo '[{"directory": 
"/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output",
 "command": "clang++ -o test.o -std=c++11 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/clang-tidy-diff.cpp.tmp.cpp",
 "file": 
"/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/clang-tidy-diff.cpp.tmp.cpp"}]'
 > 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/compilation-database-test/compile_commands.json
: 'RUN: at line 8';   not diff -U0 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-tidy/clang-tidy-diff.cpp
 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/clang-tidy-diff.cpp.tmp.cpp
 | /usr/bin/python2.7 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/../test/../clang-tidy/tool/clang-tidy-diff.py
 -checks=-*,modernize-use-override -path 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/compilation-database-test
 2>&1 | FileCheck -check-prefix=CHECK 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-tidy/clang-tidy-diff.cpp
--
Exit Code: 1

Command Output (stderr):
--
2 warnings generated.
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-tidy/clang-tidy-diff.cpp:18:11:
 error: CHECK: expected string not found in input
// CHECK: [[@LINE-2]]:8: warning: annotate this
  ^
:1:1: note: scanning from here
Traceback (most recent call last):
^
:1:1: note: with expression "@LINE-2" equal to "16"
Traceback (most recent call last):
^

[PATCH] D59595: Remove the unused return value in ASTImporter::Imported [NFC]

2019-03-20 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL356592: Remove the unused return value in 
ASTImporter::Imported [NFC] (authored by teemperor, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59595?vs=191469=191551#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59595

Files:
  cfe/trunk/include/clang/AST/ASTImporter.h
  cfe/trunk/lib/AST/ExternalASTMerger.cpp
  lldb/trunk/include/lldb/Symbol/ClangASTImporter.h
  lldb/trunk/source/Symbol/ClangASTImporter.cpp


Index: lldb/trunk/include/lldb/Symbol/ClangASTImporter.h
===
--- lldb/trunk/include/lldb/Symbol/ClangASTImporter.h
+++ lldb/trunk/include/lldb/Symbol/ClangASTImporter.h
@@ -262,7 +262,7 @@
 
 void ImportDefinitionTo(clang::Decl *to, clang::Decl *from);
 
-clang::Decl *Imported(clang::Decl *from, clang::Decl *to) override;
+void Imported(clang::Decl *from, clang::Decl *to) override;
 
 clang::Decl *GetOriginalDecl(clang::Decl *To) override;
 
Index: lldb/trunk/source/Symbol/ClangASTImporter.cpp
===
--- lldb/trunk/source/Symbol/ClangASTImporter.cpp
+++ lldb/trunk/source/Symbol/ClangASTImporter.cpp
@@ -937,7 +937,7 @@
   }
 }
 
-clang::Decl *ClangASTImporter::Minion::Imported(clang::Decl *from,
+void ClangASTImporter::Minion::Imported(clang::Decl *from,
 clang::Decl *to) {
   ClangASTMetrics::RegisterClangImport();
 
@@ -1096,8 +1096,6 @@
   }
 }
   }
-
-  return clang::ASTImporter::Imported(from, to);
 }
 
 clang::Decl *ClangASTImporter::Minion::GetOriginalDecl(clang::Decl *To) {
Index: cfe/trunk/include/clang/AST/ASTImporter.h
===
--- cfe/trunk/include/clang/AST/ASTImporter.h
+++ cfe/trunk/include/clang/AST/ASTImporter.h
@@ -425,7 +425,7 @@
 
 /// Subclasses can override this function to observe all of the \c From ->
 /// \c To declaration mappings as they are imported.
-virtual Decl *Imported(Decl *From, Decl *To) { return To; }
+virtual void Imported(Decl *From, Decl *To) {}
 
 /// Store and assign the imported declaration to its counterpart.
 Decl *MapImported(Decl *From, Decl *To);
Index: cfe/trunk/lib/AST/ExternalASTMerger.cpp
===
--- cfe/trunk/lib/AST/ExternalASTMerger.cpp
+++ cfe/trunk/lib/AST/ExternalASTMerger.cpp
@@ -110,7 +110,7 @@
 
   /// Whenever a DeclContext is imported, ensure that ExternalASTSource's 
origin
   /// map is kept up to date.  Also set the appropriate flags.
-  Decl *Imported(Decl *From, Decl *To) override {
+  void Imported(Decl *From, Decl *To) override {
 if (auto *ToDC = dyn_cast(To)) {
   const bool LoggingEnabled = Parent.LoggingEnabled();
   if (LoggingEnabled)
@@ -153,7 +153,6 @@
   ToContainer->getPrimaryContext()->setMustBuildLookupTable();
   assert(Parent.CanComplete(ToContainer));
 }
-return To;
   }
   ASTImporter () { return Reverse; }
 };


Index: lldb/trunk/include/lldb/Symbol/ClangASTImporter.h
===
--- lldb/trunk/include/lldb/Symbol/ClangASTImporter.h
+++ lldb/trunk/include/lldb/Symbol/ClangASTImporter.h
@@ -262,7 +262,7 @@
 
 void ImportDefinitionTo(clang::Decl *to, clang::Decl *from);
 
-clang::Decl *Imported(clang::Decl *from, clang::Decl *to) override;
+void Imported(clang::Decl *from, clang::Decl *to) override;
 
 clang::Decl *GetOriginalDecl(clang::Decl *To) override;
 
Index: lldb/trunk/source/Symbol/ClangASTImporter.cpp
===
--- lldb/trunk/source/Symbol/ClangASTImporter.cpp
+++ lldb/trunk/source/Symbol/ClangASTImporter.cpp
@@ -937,7 +937,7 @@
   }
 }
 
-clang::Decl *ClangASTImporter::Minion::Imported(clang::Decl *from,
+void ClangASTImporter::Minion::Imported(clang::Decl *from,
 clang::Decl *to) {
   ClangASTMetrics::RegisterClangImport();
 
@@ -1096,8 +1096,6 @@
   }
 }
   }
-
-  return clang::ASTImporter::Imported(from, to);
 }
 
 clang::Decl *ClangASTImporter::Minion::GetOriginalDecl(clang::Decl *To) {
Index: cfe/trunk/include/clang/AST/ASTImporter.h
===
--- cfe/trunk/include/clang/AST/ASTImporter.h
+++ cfe/trunk/include/clang/AST/ASTImporter.h
@@ -425,7 +425,7 @@
 
 /// Subclasses can override this function to observe all of the \c From ->
 /// \c To declaration mappings as they are imported.
-virtual Decl *Imported(Decl *From, Decl *To) { return To; }
+virtual void Imported(Decl *From, Decl *To) {}
 
 /// Store and assign the imported 

r356592 - Remove the unused return value in ASTImporter::Imported [NFC]

2019-03-20 Thread Raphael Isemann via cfe-commits
Author: teemperor
Date: Wed Mar 20 12:00:25 2019
New Revision: 356592

URL: http://llvm.org/viewvc/llvm-project?rev=356592=rev
Log:
Remove the unused return value in ASTImporter::Imported [NFC]

Summary:
`ASTImporter::Imported` currently returns a Decl, but that return value is not 
used by the ASTImporter (or anywhere else)
nor is it documented.

Reviewers: balazske, martong, a.sidorin, shafik

Reviewed By: balazske, martong

Subscribers: rnkovacs, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/AST/ASTImporter.h
cfe/trunk/lib/AST/ExternalASTMerger.cpp

Modified: cfe/trunk/include/clang/AST/ASTImporter.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTImporter.h?rev=356592=356591=356592=diff
==
--- cfe/trunk/include/clang/AST/ASTImporter.h (original)
+++ cfe/trunk/include/clang/AST/ASTImporter.h Wed Mar 20 12:00:25 2019
@@ -425,7 +425,7 @@ class TypeSourceInfo;
 
 /// Subclasses can override this function to observe all of the \c From ->
 /// \c To declaration mappings as they are imported.
-virtual Decl *Imported(Decl *From, Decl *To) { return To; }
+virtual void Imported(Decl *From, Decl *To) {}
 
 /// Store and assign the imported declaration to its counterpart.
 Decl *MapImported(Decl *From, Decl *To);

Modified: cfe/trunk/lib/AST/ExternalASTMerger.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExternalASTMerger.cpp?rev=356592=356591=356592=diff
==
--- cfe/trunk/lib/AST/ExternalASTMerger.cpp (original)
+++ cfe/trunk/lib/AST/ExternalASTMerger.cpp Wed Mar 20 12:00:25 2019
@@ -110,7 +110,7 @@ public:
 
   /// Whenever a DeclContext is imported, ensure that ExternalASTSource's 
origin
   /// map is kept up to date.  Also set the appropriate flags.
-  Decl *Imported(Decl *From, Decl *To) override {
+  void Imported(Decl *From, Decl *To) override {
 if (auto *ToDC = dyn_cast(To)) {
   const bool LoggingEnabled = Parent.LoggingEnabled();
   if (LoggingEnabled)
@@ -153,7 +153,6 @@ public:
   ToContainer->getPrimaryContext()->setMustBuildLookupTable();
   assert(Parent.CanComplete(ToContainer));
 }
-return To;
   }
   ASTImporter () { return Reverse; }
 };


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


[PATCH] D59603: [PR40707][PR41011][OpenCL] Allow addr space spelling without double underscore in C++ mode

2019-03-20 Thread Ronan Keryell via Phabricator via cfe-commits
keryell added a comment.

I was a little bit worried about

  struct top_level { int i; };
  
  private ::top_level a;

but it should be fine because in that case we have a `tok::coloncolon`


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

https://reviews.llvm.org/D59603



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


[clang-tools-extra] r356589 - [clang-tidy] Fix redundant check breaking the test on many platforms.

2019-03-20 Thread Zinovy Nis via cfe-commits
Author: zinovy.nis
Date: Wed Mar 20 11:37:04 2019
New Revision: 356589

URL: http://llvm.org/viewvc/llvm-project?rev=356589=rev
Log:
[clang-tidy] Fix redundant check breaking the test on many platforms.

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


Modified:
clang-tools-extra/trunk/test/clang-tidy/clang-tidy-diff.cpp

Modified: clang-tools-extra/trunk/test/clang-tidy/clang-tidy-diff.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/clang-tidy-diff.cpp?rev=356589=356588=356589=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/clang-tidy-diff.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/clang-tidy-diff.cpp Wed Mar 20 
11:37:04 2019
@@ -23,5 +23,4 @@ struct B : public A {
 // CHECK-QUIET-NOT: warning:
 };
 // CHECK-SANITY-NOT: Suppressed
-// CHECK: Suppressed 1 warnings (1 due to line filter).
 // CHECK-QUIET-NOT: Suppressed


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


[PATCH] D59492: [OpenCL] Allow variadic macros as Clang feature

2019-03-20 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added inline comments.



Comment at: include/clang/Basic/DiagnosticLexKinds.td:397
+def ext_pp_opencl_variadic_macros : Extension<
+  "variadic macros not supported in OpenCL">;
 

Maybe rephrase the message now to say it's an extension? The other similar 
warnings seem to mention "GNU extension" or "Microsoft extensions" or  extension.

None of them seem to mention "clang" extensions however.


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

https://reviews.llvm.org/D59492



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


[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

2019-03-20 Thread Tolga Mizrak via Phabricator via cfe-commits
to-miz updated this revision to Diff 191540.
to-miz added a comment.

Made a new diff on the git repository, since the docs now recommend using git.
Rebased and created a new diff with `git show HEAD -U99`.
The patch applies cleanly on windows after `git reset --hard` so I hope there 
shouldn't be any merge conflicts now.


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

https://reviews.llvm.org/D52150

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2993,22 +2993,25 @@
 EXPECT_EQ(Expected, format(ToFormat, Style));
 EXPECT_EQ(Expected, format(Expected, Style));
   }
-  // Test with tabs.
-  Style.UseTab = FormatStyle::UT_Always;
-  Style.IndentWidth = 8;
-  Style.TabWidth = 8;
-  verifyFormat("#ifdef _WIN32\n"
-   "#\tdefine A 0\n"
-   "#\tifdef VAR2\n"
-   "#\t\tdefine B 1\n"
-   "#\t\tinclude \n"
-   "#\t\tdefine MACRO  \\\n"
-   "\t\t\tsome_very_long_func_aa();\n"
-   "#\tendif\n"
-   "#else\n"
-   "#\tdefine A 1\n"
-   "#endif",
-   Style);
+  // Test AfterHash with tabs.
+  {
+FormatStyle Tabbed = Style;
+Tabbed.UseTab = FormatStyle::UT_Always;
+Tabbed.IndentWidth = 8;
+Tabbed.TabWidth = 8;
+verifyFormat("#ifdef _WIN32\n"
+ "#\tdefine A 0\n"
+ "#\tifdef VAR2\n"
+ "#\t\tdefine B 1\n"
+ "#\t\tinclude \n"
+ "#\t\tdefine MACRO  \\\n"
+ "\t\t\tsome_very_long_func_aa();\n"
+ "#\tendif\n"
+ "#else\n"
+ "#\tdefine A 1\n"
+ "#endif",
+ Tabbed);
+  }
 
   // Regression test: Multiline-macro inside include guards.
   verifyFormat("#ifndef HEADER_H\n"
@@ -3018,6 +3021,102 @@
"  int j;\n"
"#endif // HEADER_H",
getLLVMStyleWithColumns(20));
+
+  Style.IndentPPDirectives = FormatStyle::PPDIS_BeforeHash;
+  // Basic before hash indent tests
+  verifyFormat("#ifdef _WIN32\n"
+   "  #define A 0\n"
+   "  #ifdef VAR2\n"
+   "#define B 1\n"
+   "#include \n"
+   "#define MACRO  \\\n"
+   "  some_very_long_func_aa();\n"
+   "  #endif\n"
+   "#else\n"
+   "  #define A 1\n"
+   "#endif",
+   Style);
+  verifyFormat("#if A\n"
+   "  #define MACRO\\\n"
+   "void a(int x) {\\\n"
+   "  b(); \\\n"
+   "  c(); \\\n"
+   "  d(); \\\n"
+   "  e(); \\\n"
+   "  f(); \\\n"
+   "}\n"
+   "#endif",
+   Style);
+  // Keep comments aligned with indented directives. These
+  // tests cannot use verifyFormat because messUp manipulates leading
+  // whitespace.
+  {
+const char *Expected = "void f() {\n"
+   "// Aligned to preprocessor.\n"
+   "#if 1\n"
+   "  // Aligned to code.\n"
+   "  int a;\n"
+   "  #if 1\n"
+   "// Aligned to preprocessor.\n"
+   "#define A 0\n"
+   "  // Aligned to code.\n"
+   "  int b;\n"
+   "  #endif\n"
+   "#endif\n"
+   "}";
+const char *ToFormat = "void f() {\n"
+   "// Aligned to preprocessor.\n"
+   "#if 1\n"
+   "// Aligned to code.\n"
+   "int a;\n"
+   "#if 1\n"
+   "// Aligned to preprocessor.\n"
+   "#define A 0\n"
+   "// Aligned to code.\n"
+   "int b;\n"
+   "#endif\n"
+   "#endif\n"
+   "}";
+EXPECT_EQ(Expected, format(ToFormat, Style));
+EXPECT_EQ(Expected, format(Expected, Style));
+  }
+  {
+const char *Expected = 

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

2019-03-20 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

> The cost is financial, as it's developer time, which costs real money to 
> companies. In the end, to support this, people like myself who are doing this 
> as part of their job spend time that they'd otherwise spend to make other 
> things better that might be more important.

Don't get me wrong I totally appreciate what you do,

But what you mean is it costs your employer.  That I understand, but not all of 
us are doing this on behalf of a company (more specially not yours), so you 
could also say that your employer benefits the other way from those 
contributors who give their time without them having to spend a dime.

I guess my question would be, should the cost to your employer really come into 
the decision about whether something goes in or not? Other than of course we 
are totally grateful to them for giving us so much of your time, but that 
shouldn't have impact on what is worthy to go in should it? or am I wrong?




Comment at: lib/Format/TokenAnnotator.cpp:2546-2560
 return Line.Type == LT_ObjCDecl || Left.is(tok::semi) ||
(Style.SpaceBeforeParens != FormatStyle::SBPO_Never &&
-(Left.isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for, tok::kw_while,
-  tok::kw_switch, tok::kw_case, TT_ForEachMacro,
-  TT_ObjCForIn) ||
- Left.endsSequence(tok::kw_constexpr, tok::kw_if) ||
- (Left.isOneOf(tok::kw_try, Keywords.kw___except, tok::kw_catch,
-   tok::kw_new, tok::kw_delete) &&
-  (!Left.Previous || Left.Previous->isNot(tok::period) ||
-   (Style.SpaceBeforeParens == FormatStyle::SBPO_Always &&
-(Left.is(tok::identifier) || Left.isFunctionLikeKeyword() ||
- Left.is(tok::r_paren) ||
- (Left.is(tok::r_square) && Left.MatchingParen &&
-  Left.MatchingParen->is(TT_LambdaLSquare))) &&
-Line.Type != LT_PreprocessorDirective);
+((Left.isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for, 
tok::kw_while,
+   tok::kw_switch, tok::kw_case, TT_ForEachMacro,
+   TT_ObjCForIn) ||
+  Left.endsSequence(tok::kw_constexpr, tok::kw_if) ||
+  (Left.isOneOf(tok::kw_try, Keywords.kw___except, tok::kw_catch,

klimek wrote:
> reuk wrote:
> > klimek wrote:
> > > I'm really confused about the changes os parens. It seems like this 
> > > should just change the spaceRequiredBeforeParens(...), but instead seems 
> > > to change parens in a way that completely changes the logic? (or 
> > > alternatively is phabricator showing this wrong?)
> > I think this looks like a bigger change than it really is because I added 
> > an open-parens on line 2548, which then affected the formatting of the 
> > following lines. I also added the `isSimpleTypeSpecifier` check, so that 
> > functional-style casts (`int (foo)`) are given the correct spacing.
> a) why did you add the one in 2548? you didn't change any of the logic, right?
> b) there are 3 extra closing parens in 2560, I see one more new opening in 
> 2556, but that also seems superfluous?
> One question is whether we should pull this apart into bools with names that 
> make sense :)
Isn't this just a classic example of where rewriting this as a series of static 
functions  e.g.

```
static bool someBehavior(Line, Left)
{
 if  (Line.Type == LT_ObjCDecl)
  return true;
 if  (Left.is(tok::semi)
  return true;
..

return false; 
}
```

would be so much easier to understand?


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

https://reviews.llvm.org/D55170



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


[PATCH] D59605: [clangd] Introduce background-indexer

2019-03-20 Thread Jan Korous via Phabricator via cfe-commits
jkorous added inline comments.



Comment at: clang-tools-extra/clangd/background-indexer/BackgroundIndexer.cpp:56
+  // non-interactive tools like this one.
+  24 * 60 * 60 * 1000);
+  llvm::SmallString<128> DummyFile(CompileCommandsDir);

Nit: maybe we should give this constant a name? Or maybe create a command line 
option for this?



Comment at: clang-tools-extra/clangd/background-indexer/BackgroundIndexer.cpp:60
+  llvm::sys::path::remove_dots(DummyFile, true);
+  llvm::sys::path::append(DummyFile, "dummy.cpp");
+  CDB.getCompileCommand(DummyFile);

Maybe we should add a comment about why we do this?



Comment at: clang-tools-extra/clangd/index/Background.cpp:605
 continue;
+  --UpToDateTUs;
   // FIXME: Currently, we simply schedule indexing on a TU whenever any of

It's not obvious to me that we don't underflow here given we are using 
`size_t`. Maybe add an `assert` or some other sanity check?



Comment at: clang-tools-extra/clangd/index/Background.h:149
+  // For logging
+  size_t EnqueuedTUs = 0;
+  size_t IndexedTUs = 0;

ilya-biryukov wrote:
> Maybe use `std::atomic` instead?
+1 for atomic


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59605



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


[PATCH] D56343: [clang-tidy] Refactor: Extract Class CheckRunner on check_clang_tidy.py

2019-03-20 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood marked 9 inline comments as done.
LegalizeAdulthood added inline comments.



Comment at: test/clang-tidy/check_clang_tidy.py:112
+process_output = e.output.decode()
+print('%s failed:\n%s' % (' '.join(args), process_output))
+if raise_error:

JonasToth wrote:
> Its better to use `.format()` instead of `%` syntax
That can be done in a later commit; it is not the point of this review.



Comment at: test/clang-tidy/check_clang_tidy.py:141
+   '-check-prefixes=' + ','.join(check_notes_prefixes),
+   '-implicit-check-not={{note|warning|error}}:'])
+  return

serge-sans-paille wrote:
> These three `check_*` function all use the same `'FileCheck', '-']` 
> pattern. Maybe it's worth syndicating that to a 
> try_run_filecheck(input_file0, input_file1, *extra_args)` function.
That can be done in a later commit; it is not the point of this review.



Comment at: test/clang-tidy/check_clang_tidy.py:178
+check_fixes_prefixes, check_messages_prefixes, check_notes_prefixes = \
+get_prefixes(args.check_suffix, input_text)
+

serge-sans-paille wrote:
> This is the verbose call site I was pointing to above.
Addressed by Extract Class instead of Extract Function



Comment at: test/clang-tidy/check_clang_tidy.py:206
 
-  if has_check_fixes:
-try:
-  subprocess.check_output(
-  ['FileCheck', '-input-file=' + temp_file_name, input_file_name,
-   '-check-prefixes=' + ','.join(check_fixes_prefixes),
-   '-strict-whitespace'],
-  stderr=subprocess.STDOUT)
-except subprocess.CalledProcessError as e:
-  print('FileCheck failed:\n' + e.output.decode())
-  raise
-
-  if has_check_messages:
-messages_file = temp_file_name + '.msg'
-write_file(messages_file, clang_tidy_output)
-try:
-  subprocess.check_output(
-  ['FileCheck', '-input-file=' + messages_file, input_file_name,
-   '-check-prefixes=' + ','.join(check_messages_prefixes),
-   '-implicit-check-not={{warning|error}}:'],
-  stderr=subprocess.STDOUT)
-except subprocess.CalledProcessError as e:
-  print('FileCheck failed:\n' + e.output.decode())
-  raise
-
-  if has_check_notes:
-notes_file = temp_file_name + '.notes'
-filtered_output = [line for line in clang_tidy_output.splitlines()
-   if not "note: FIX-IT applied" in line]
-write_file(notes_file, '\n'.join(filtered_output))
-try:
-  subprocess.check_output(
-  ['FileCheck', '-input-file=' + notes_file, input_file_name,
-   '-check-prefixes=' + ','.join(check_notes_prefixes),
-   '-implicit-check-not={{note|warning|error}}:'],
-  stderr=subprocess.STDOUT)
-except subprocess.CalledProcessError as e:
-  print('FileCheck failed:\n' + e.output.decode())
-  raise
+  check_fixes(check_fixes_prefixes, has_check_fixes, input_file_name, 
temp_file_name)
+  check_messages(check_messages_prefixes, has_check_messages, 
clang_tidy_output, input_file_name, temp_file_name)

JonasToth wrote:
> If would prefer keeping the `if check_notes` outside of the function call and 
> remove that one argument. Same for the other `check_...` functions
It's the wrong level of abstraction for the `if` check to be inside `run()`


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

https://reviews.llvm.org/D56343



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


[PATCH] D59605: [clangd] Introduce background-indexer

2019-03-20 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clangd/index/Background.cpp:29
 #include "llvm/Support/SHA1.h"
 
 #include 

Unnecessary empty line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59605



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


[PATCH] D59279: [Analyzer] Checker for non-determinism caused by iteration of unordered container of pointers

2019-03-20 Thread Mandeep Singh Grang via Phabricator via cfe-commits
mgrang added a comment.

Ping for reviews please.


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

https://reviews.llvm.org/D59279



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


[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

2019-03-20 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked 4 inline comments as done.
ymandel added inline comments.



Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:135
+// \endcode
+class RewriteRule {
+public:

ilya-biryukov wrote:
> ymandel wrote:
> > ilya-biryukov wrote:
> > > Maybe consider separating the fluent API to build the rewrite rule from 
> > > the rewrite rule itself?
> > > 
> > > Not opposed to having the fluent builder API, this does look nice and 
> > > seems to nicely align with the matcher APIs.
> > > However, it is somewhat hard to figure out what can `RewriteRule` do 
> > > **after** it was built when looking at the code right now.
> > > ```
> > > class RewriteRule {
> > > public:
> > >   RewriteRule(DynTypedMatcher, TextGenerator Replacement, TextGenerator 
> > > Explanation);
> > > 
> > >   DynTypedMatcher matcher() const;
> > >   Expected replacement() const;
> > >   Expected explanation() const;
> > > };
> > > 
> > > struct RewriteRuleBuilder { // Having a better name than 'Builder' would 
> > > be nice.
> > >   RewriteRule finish() &&; // produce the final RewriteRule.
> > > 
> > >   template 
> > >   RewriteRuleBuilder (const TypedNodeId ,
> > >   NodePart Part = NodePart::Node) &;
> > >   RewriteRuleBuilder (TextGenerator Replacement) &;
> > >   RewriteRuleBuilder (TextGenerator Explanation) &;
> > > private:
> > >   RewriteRule RuleInProgress;
> > > };
> > > RewriteRuleBuilder makeRewriteRule();
> > > ```
> > I see your point, but do you think it might be enough to improve the 
> > comments on the class? My concern with a builder is the mental burden on 
> > the user of another concept (the builder) and the need for an extra 
> > `.build()` everywhere. To a lesser extent, I also don't love the cost of an 
> > extra copy, although I doubt it matters and I suppose we could support 
> > moves in the build method.
> The issues with the builder interface is that it requires lots of 
> boilerplate, which is hard to throw away when reading the code of the class. 
> I agree that having a separate builder class is also annoying (more concepts, 
> etc).
> 
> Keeping them separate would be my personal preference, but if you'd prefer to 
> keep it in the same class than maybe move the non-builder pieces to the top 
> of the class and separate the builder methods with a comment. 
> WDYT? 
> 
> > To a lesser extent, I also don't love the cost of an extra copy, although I 
> > doubt it matters and I suppose we could support moves in the build method.
> I believe we can be as efficient (up to an extra move) with builders as 
> without them. If most usages are of the form `RewriteRule R = 
> rewriteRule(...).change(...).replaceWith(...).because(...);`
> Then we could make all builder functions return r-value reference to a 
> builder and have an implicit conversion operator that would consume the 
> builder, producing the final `RewriteRule`:
> ```
> class RewriteRuleBuilder {
>   operator RewriteRule () &&;
>   /// ...
> };
> RewriteRuleBuilder rewriteRule();
> 
> void addRule(RewriteRule R);
> void clientCode() {
>   addRule(rewriteRule().change(Matcher).replaceWith("foo").because("bar"));
> }
> ```
I didn't realize that implicit conversion functions are allowed. With that, I'm 
fine w/ splitting.   Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59376



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


[PATCH] D59605: [clangd] Introduce background-indexer

2019-03-20 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/background-indexer/BackgroundIndexer.cpp:13
+
+#include 
"/usr/local/google/home/kadircet/repos/llvm/clang-tools-extra/clangd/Context.h"
+#include 
"/usr/local/google/home/kadircet/repos/llvm/clang-tools-extra/clangd/Logger.h"

Eugene.Zelenko wrote:
> Please use relative path. Same below.
Heh, where do these come from?
Does our include insertion prefer to add global paths in some cases? Dynamic 
index?



Comment at: clang-tools-extra/clangd/index/Background.cpp:622
+IndexedTUs += UpToDateTUs;
+log("[{0}/{1}] Loaded shards from storage", IndexedTUs, EnqueuedTUs);
+  }

Everything else is `vlog` (or even `dlog`), what's the rationale of using `log` 
here?



Comment at: clang-tools-extra/clangd/index/Background.h:149
+  // For logging
+  size_t EnqueuedTUs = 0;
+  size_t IndexedTUs = 0;

Maybe use `std::atomic` instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59605



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


[PATCH] D59599: [clangd] Fix a crash while printing Template Arguments

2019-03-20 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments.



Comment at: clangd/AST.cpp:86
   else if (auto *Cls = llvm::dyn_cast()) {
-if (auto STL = Cls->getTypeAsWritten()
-   ->getTypeLoc()
-   .getAs()) {
-  llvm::SmallVector ArgLocs;
-  ArgLocs.reserve(STL.getNumArgs());
-  for (unsigned I = 0; I < STL.getNumArgs(); ++I)
-ArgLocs.push_back(STL.getArgLoc(I));
-  printTemplateArgumentList(OS, ArgLocs, Policy);
+if (auto *TSI = Cls->getTypeAsWritten()) {
+  if (auto STL = TSI->getTypeLoc().getAs()) 
{

Return type is not obvious, so auto should not be used.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59599



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


[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

2019-03-20 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

Thanks for changes. A few more comments.




Comment at: clangd/ClangdUnit.cpp:259
+// Generates a mapping from start of an include directive to its range.
+std::map
+positionToRangeMap(const std::vector ) {

Could we binary-search in a sorted `vector` with a custom comparator 
instead?



Comment at: clangd/ClangdUnit.cpp:411
+MaxDiagsPerIncludeDirective) {
+  ++NumberOfDiagsAtLine[D.IncludeDirective->line];
+  return true;

The function name suggests it does not have side-effects.
Maybe handle the update and query of `NumberOfDiagstAtLine` outside this 
function instead?



Comment at: clangd/Diagnostics.cpp:396
+  for (llvm::StringRef Inc : IncludeStack)
+OS << "In file included from: " << Inc << ":\n";
+}

NIT: could we reuse the function from clang that does the same thing?

The code here is pretty simple, though, so writing it here is fine. Just wanted 
to make sure we considered this option and found it's easier to redo this work 
ourselves.



Comment at: clangd/Diagnostics.cpp:419
-  else
-vlog("Dropped diagnostic outside main file: {0}: {1}", LastDiag->File,
- LastDiag->Message);

We can still drop diagnostics at some point, right?
Could we move the log statement in a different point?




Comment at: clangd/Diagnostics.h:87
+  /// directive, otherwise None.
+  llvm::Optional IncludeDirective = llvm::None;
 };

Could we avoid changing this interface?
We have enough information to fill the corresponding range and message when 
collecting diagnostics inside our implementation of clang's 
`DiagnosticConsumer`, there's really no point in carrying this information in 
this struct.

If that means intermediate structures stored in our consumer, so be it, it's a 
private implementation detail. `Diag` is a public interface, so keeping it 
simpler is valuable.



Comment at: unittests/clangd/DiagnosticsTests.cpp:773
+  UnorderedElementsAre(
+  Diag(Main.range(), "/clangd-test/a.h:2:44: C++ requires a "
+ "type specifier for all declarations")));

NIT: maybe use raw string literals to keep the message a one-liner?



Comment at: unittests/clangd/DiagnosticsTests.cpp:779
 } // namespace clangd
 } // namespace clang

Could we add a test for reaching the limit?
Could we mention that some diagnostics were dropped in that case? Something 
like:
```
In file included from a.h:20:1:
error: unresolved identifier 'foo'.

And 10 more diagnostics...
```


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59302



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


[PATCH] D59605: [clangd] Introduce background-indexer

2019-03-20 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clangd/background-indexer/BackgroundIndexer.cpp:13
+
+#include 
"/usr/local/google/home/kadircet/repos/llvm/clang-tools-extra/clangd/Context.h"
+#include 
"/usr/local/google/home/kadircet/repos/llvm/clang-tools-extra/clangd/Logger.h"

Please use relative path. Same below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59605



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


[PATCH] D59309: [clang-format] BreakAfterReturnType ignored on functions with numeric template parameters

2019-03-20 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 191536.
MyDeveloperDay added a comment.

Sorry for the spam:

- write this a little better


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

https://reviews.llvm.org/D59309

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5419,6 +5419,42 @@
"}\n"
"template  T *f(T );\n", // No break here.
Style);
+  verifyFormat("int\n"
+   "foo(A a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A<8> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, 8> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, bool> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, bool> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, 8> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
 }
 
 TEST_F(FormatTest, AlwaysBreakBeforeMultilineStrings) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2045,7 +2045,7 @@
 return true;
   for (const FormatToken *Tok = Next->Next; Tok && Tok != Next->MatchingParen;
Tok = Tok->Next) {
-if (Tok->is(tok::l_paren) && Tok->MatchingParen) {
+if (Tok->isOneOf(tok::l_paren, TT_TemplateOpener) && Tok->MatchingParen) {
   Tok = Tok->MatchingParen;
   continue;
 }


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5419,6 +5419,42 @@
"}\n"
"template  T *f(T );\n", // No break here.
Style);
+  verifyFormat("int\n"
+   "foo(A a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A<8> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, 8> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, bool> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, bool> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, 8> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
 }
 
 TEST_F(FormatTest, AlwaysBreakBeforeMultilineStrings) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2045,7 +2045,7 @@
 return true;
   for (const FormatToken *Tok = Next->Next; Tok && Tok != Next->MatchingParen;
Tok = Tok->Next) {
-if (Tok->is(tok::l_paren) && Tok->MatchingParen) {
+if (Tok->isOneOf(tok::l_paren, TT_TemplateOpener) && Tok->MatchingParen) {
   Tok = Tok->MatchingParen;
   continue;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D59605: [clangd] Introduce background-indexer

2019-03-20 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision.
kadircet added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, jdoerfert, erik.pilkington, arphaman, 
jkorous, MaskRay, ioeric, mgorny.
Herald added a project: clang.

Sample output:
I[18:37:50.083] BackgroundIndex: build symbol index periodically every 8640 
ms.
I[18:37:50.115] Enqueueing 3742 commands for indexing
I[18:37:50.280] [0/3742] Loaded shards from storage
I[18:37:51.026] [1/3742] Indexed 
/usr/local/google/home/kadircet/repos/llvm/llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
 (6422 symbols, 18236 refs, 161 files)
I[18:37:51.581] [2/3742] Indexed 
/usr/local/google/home/kadircet/repos/llvm/llvm/lib/Support/MD5.cpp (10204 
symbols, 28897 refs, 232 files)
I[18:37:51.607] [3/3742] Indexed 
/usr/local/google/home/kadircet/repos/llvm/llvm/lib/MC/MCSchedule.cpp (11461 
symbols, 32420 refs, 247 files)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59605

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/background-indexer/BackgroundIndexer.cpp
  clang-tools-extra/clangd/background-indexer/CMakeLists.txt
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/index/Background.h

Index: clang-tools-extra/clangd/index/Background.h
===
--- clang-tools-extra/clangd/index/Background.h
+++ clang-tools-extra/clangd/index/Background.h
@@ -144,6 +144,11 @@
   std::deque> Queue;
   std::vector ThreadPool; // FIXME: Abstract this away.
   GlobalCompilationDatabase::CommandChanged::Subscription CommandsChanged;
+
+  // For logging
+  size_t EnqueuedTUs = 0;
+  size_t IndexedTUs = 0;
+  std::mutex TUCounterMutex;
 };
 
 } // namespace clangd
Index: clang-tools-extra/clangd/index/Background.cpp
===
--- clang-tools-extra/clangd/index/Background.cpp
+++ clang-tools-extra/clangd/index/Background.cpp
@@ -29,6 +29,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -215,6 +216,10 @@
 // We're doing this asynchronously, because we'll read shards here too.
 log("Enqueueing {0} commands for indexing", ChangedFiles.size());
 SPAN_ATTACH(Tracer, "files", int64_t(ChangedFiles.size()));
+{
+  std::lock_guard Lock(TUCounterMutex);
+  EnqueuedTUs += ChangedFiles.size();
+}
 
 auto NeedsReIndexing = loadShards(std::move(ChangedFiles));
 // Run indexing for files that need to be updated.
@@ -447,9 +452,13 @@
   assert(Index.Symbols && Index.Refs && Index.Sources &&
  "Symbols, Refs and Sources must be set.");
 
-  log("Indexed {0} ({1} symbols, {2} refs, {3} files)",
-  Inputs.CompileCommand.Filename, Index.Symbols->size(),
-  Index.Refs->numRefs(), Index.Sources->size());
+  {
+std::lock_guard Lock(TUCounterMutex);
+++IndexedTUs;
+log("[{0}/{1}] Indexed {2} ({3} symbols, {4} refs, {5} files)", IndexedTUs,
+EnqueuedTUs, Inputs.CompileCommand.Filename, Index.Symbols->size(),
+Index.Refs->numRefs(), Index.Sources->size());
+  }
   SPAN_ATTACH(Tracer, "symbols", int(Index.Symbols->size()));
   SPAN_ATTACH(Tracer, "refs", int(Index.Refs->numRefs()));
   SPAN_ATTACH(Tracer, "sources", int(Index.Sources->size()));
@@ -581,20 +590,23 @@
   // Keeps track of the loaded shards to make sure we don't perform redundant
   // disk IO. Keys are absolute paths.
   llvm::StringSet<> LoadedShards;
+  size_t UpToDateTUs = 0;
   for (const auto  : ChangedFiles) {
 ProjectInfo PI;
 auto Cmd = CDB.getCompileCommand(File, );
 if (!Cmd)
   continue;
 BackgroundIndexStorage *IndexStorage = IndexStorageFactory(PI.SourceRoot);
+++UpToDateTUs;
 auto Dependencies = loadShard(*Cmd, IndexStorage, LoadedShards);
 for (const auto  : Dependencies) {
   if (!Dependency.NeedsReIndexing || FilesToIndex.count(Dependency.Path))
 continue;
+  --UpToDateTUs;
   // FIXME: Currently, we simply schedule indexing on a TU whenever any of
   // its dependencies needs re-indexing. We might do it smarter by figuring
   // out a minimal set of TUs that will cover all the stale dependencies.
-  vlog("Enqueueing TU {0} because its dependency {1} needs re-indexing.",
+  dlog("Enqueueing TU {0} because its dependency {1} needs re-indexing.",
Cmd->Filename, Dependency.Path);
   NeedsReIndexing.push_back({std::move(*Cmd), IndexStorage});
   // Mark all of this TU's dependencies as to-be-indexed so that we won't
@@ -604,7 +616,11 @@
   break;
 }
   }
-  vlog("Loaded all shards");
+  {
+std::lock_guard Lock(TUCounterMutex);
+IndexedTUs += UpToDateTUs;
+log("[{0}/{1}] Loaded shards from storage", IndexedTUs, EnqueuedTUs);
+  }
   reset(IndexedSymbols.buildIndex(IndexType::Heavy, DuplicateHandling::Merge));
   vlog("BackgroundIndex: built symbol index with estimated memory {0} "
"bytes.",
Index: 

[PATCH] D59309: [clang-format] BreakAfterReturnType ignored on functions with numeric template parameters

2019-03-20 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 191530.
MyDeveloperDay added a comment.

Addressing code review comment

  @klimek I think is what you meant, a much smaller and neater version of what 
I had before.


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

https://reviews.llvm.org/D59309

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5419,6 +5419,42 @@
"}\n"
"template  T *f(T );\n", // No break here.
Style);
+  verifyFormat("int\n"
+   "foo(A a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A<8> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, 8> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, bool> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, bool> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, 8> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
 }
 
 TEST_F(FormatTest, AlwaysBreakBeforeMultilineStrings) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2049,6 +2049,10 @@
   Tok = Tok->MatchingParen;
   continue;
 }
+if (Tok->is(TT_TemplateOpener) && Tok->MatchingParen) {
+  Tok = Tok->MatchingParen;
+  continue;
+}
 if (Tok->is(tok::kw_const) || Tok->isSimpleTypeSpecifier() ||
 Tok->isOneOf(TT_PointerOrReference, TT_StartOfName, tok::ellipsis))
   return true;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5419,6 +5419,42 @@
"}\n"
"template  T *f(T );\n", // No break here.
Style);
+  verifyFormat("int\n"
+   "foo(A a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A<8> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, 8> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, bool> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, bool> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, 8> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
 }
 
 TEST_F(FormatTest, AlwaysBreakBeforeMultilineStrings) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2049,6 +2049,10 @@
   Tok = Tok->MatchingParen;
   continue;
 }
+if (Tok->is(TT_TemplateOpener) && Tok->MatchingParen) {
+  Tok = Tok->MatchingParen;
+  continue;
+}
 if (Tok->is(tok::kw_const) || Tok->isSimpleTypeSpecifier() ||
 Tok->isOneOf(TT_PointerOrReference, TT_StartOfName, tok::ellipsis))
   return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

2019-03-20 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

In D52150#1436423 , @to-miz wrote:

> I checked out the git repository and applied the patch to head and reran all 
> tests.
>  The diff produced by git is basically the same, should I upload the new diff?


I think you might need a new diff, I tried to merge this and it rejected some 
of the patch


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

https://reviews.llvm.org/D52150



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


[PATCH] D59578: [X86] Remove getCPUKindCanonicalName which seems to be unused.

2019-03-20 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL356580: [X86] Remove getCPUKindCanonicalName which is 
unused. (authored by ctopper, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59578

Files:
  cfe/trunk/lib/Basic/Targets/X86.cpp
  cfe/trunk/lib/Basic/Targets/X86.h


Index: cfe/trunk/lib/Basic/Targets/X86.cpp
===
--- cfe/trunk/lib/Basic/Targets/X86.cpp
+++ cfe/trunk/lib/Basic/Targets/X86.cpp
@@ -1540,18 +1540,6 @@
   WholeList.split(Features, ',', /*MaxSplit=*/-1, /*KeepEmpty=*/false);
 }
 
-std::string X86TargetInfo::getCPUKindCanonicalName(CPUKind Kind) const {
-  switch (Kind) {
-  case CK_Generic:
-return "";
-#define PROC(ENUM, STRING, IS64BIT)
\
-  case CK_##ENUM:  
\
-return STRING;
-#include "clang/Basic/X86Target.def"
-  }
-  llvm_unreachable("Invalid CPUKind");
-}
-
 // We can't use a generic validation scheme for the cpus accepted here
 // versus subtarget cpus accepted in the target attribute because the
 // variables intitialized by the runtime only support the below currently
Index: cfe/trunk/lib/Basic/Targets/X86.h
===
--- cfe/trunk/lib/Basic/Targets/X86.h
+++ cfe/trunk/lib/Basic/Targets/X86.h
@@ -121,8 +121,6 @@
 
   CPUKind getCPUKind(StringRef CPU) const;
 
-  std::string getCPUKindCanonicalName(CPUKind Kind) const;
-
   enum FPMathKind { FP_Default, FP_SSE, FP_387 } FPMath = FP_Default;
 
 public:


Index: cfe/trunk/lib/Basic/Targets/X86.cpp
===
--- cfe/trunk/lib/Basic/Targets/X86.cpp
+++ cfe/trunk/lib/Basic/Targets/X86.cpp
@@ -1540,18 +1540,6 @@
   WholeList.split(Features, ',', /*MaxSplit=*/-1, /*KeepEmpty=*/false);
 }
 
-std::string X86TargetInfo::getCPUKindCanonicalName(CPUKind Kind) const {
-  switch (Kind) {
-  case CK_Generic:
-return "";
-#define PROC(ENUM, STRING, IS64BIT)\
-  case CK_##ENUM:  \
-return STRING;
-#include "clang/Basic/X86Target.def"
-  }
-  llvm_unreachable("Invalid CPUKind");
-}
-
 // We can't use a generic validation scheme for the cpus accepted here
 // versus subtarget cpus accepted in the target attribute because the
 // variables intitialized by the runtime only support the below currently
Index: cfe/trunk/lib/Basic/Targets/X86.h
===
--- cfe/trunk/lib/Basic/Targets/X86.h
+++ cfe/trunk/lib/Basic/Targets/X86.h
@@ -121,8 +121,6 @@
 
   CPUKind getCPUKind(StringRef CPU) const;
 
-  std::string getCPUKindCanonicalName(CPUKind Kind) const;
-
   enum FPMathKind { FP_Default, FP_SSE, FP_387 } FPMath = FP_Default;
 
 public:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r356580 - [X86] Remove getCPUKindCanonicalName which is unused.

2019-03-20 Thread Craig Topper via cfe-commits
Author: ctopper
Date: Wed Mar 20 10:26:51 2019
New Revision: 356580

URL: http://llvm.org/viewvc/llvm-project?rev=356580=rev
Log:
[X86] Remove getCPUKindCanonicalName which is unused.

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

Modified:
cfe/trunk/lib/Basic/Targets/X86.cpp
cfe/trunk/lib/Basic/Targets/X86.h

Modified: cfe/trunk/lib/Basic/Targets/X86.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.cpp?rev=356580=356579=356580=diff
==
--- cfe/trunk/lib/Basic/Targets/X86.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/X86.cpp Wed Mar 20 10:26:51 2019
@@ -1540,18 +1540,6 @@ void X86TargetInfo::getCPUSpecificCPUDis
   WholeList.split(Features, ',', /*MaxSplit=*/-1, /*KeepEmpty=*/false);
 }
 
-std::string X86TargetInfo::getCPUKindCanonicalName(CPUKind Kind) const {
-  switch (Kind) {
-  case CK_Generic:
-return "";
-#define PROC(ENUM, STRING, IS64BIT)
\
-  case CK_##ENUM:  
\
-return STRING;
-#include "clang/Basic/X86Target.def"
-  }
-  llvm_unreachable("Invalid CPUKind");
-}
-
 // We can't use a generic validation scheme for the cpus accepted here
 // versus subtarget cpus accepted in the target attribute because the
 // variables intitialized by the runtime only support the below currently

Modified: cfe/trunk/lib/Basic/Targets/X86.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.h?rev=356580=356579=356580=diff
==
--- cfe/trunk/lib/Basic/Targets/X86.h (original)
+++ cfe/trunk/lib/Basic/Targets/X86.h Wed Mar 20 10:26:51 2019
@@ -121,8 +121,6 @@ protected:
 
   CPUKind getCPUKind(StringRef CPU) const;
 
-  std::string getCPUKindCanonicalName(CPUKind Kind) const;
-
   enum FPMathKind { FP_Default, FP_SSE, FP_387 } FPMath = FP_Default;
 
 public:


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


[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

2019-03-20 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:135
+// \endcode
+class RewriteRule {
+public:

ymandel wrote:
> ilya-biryukov wrote:
> > Maybe consider separating the fluent API to build the rewrite rule from the 
> > rewrite rule itself?
> > 
> > Not opposed to having the fluent builder API, this does look nice and seems 
> > to nicely align with the matcher APIs.
> > However, it is somewhat hard to figure out what can `RewriteRule` do 
> > **after** it was built when looking at the code right now.
> > ```
> > class RewriteRule {
> > public:
> >   RewriteRule(DynTypedMatcher, TextGenerator Replacement, TextGenerator 
> > Explanation);
> > 
> >   DynTypedMatcher matcher() const;
> >   Expected replacement() const;
> >   Expected explanation() const;
> > };
> > 
> > struct RewriteRuleBuilder { // Having a better name than 'Builder' would be 
> > nice.
> >   RewriteRule finish() &&; // produce the final RewriteRule.
> > 
> >   template 
> >   RewriteRuleBuilder (const TypedNodeId ,
> >   NodePart Part = NodePart::Node) &;
> >   RewriteRuleBuilder (TextGenerator Replacement) &;
> >   RewriteRuleBuilder (TextGenerator Explanation) &;
> > private:
> >   RewriteRule RuleInProgress;
> > };
> > RewriteRuleBuilder makeRewriteRule();
> > ```
> I see your point, but do you think it might be enough to improve the comments 
> on the class? My concern with a builder is the mental burden on the user of 
> another concept (the builder) and the need for an extra `.build()` 
> everywhere. To a lesser extent, I also don't love the cost of an extra copy, 
> although I doubt it matters and I suppose we could support moves in the build 
> method.
The issues with the builder interface is that it requires lots of boilerplate, 
which is hard to throw away when reading the code of the class. I agree that 
having a separate builder class is also annoying (more concepts, etc).

Keeping them separate would be my personal preference, but if you'd prefer to 
keep it in the same class than maybe move the non-builder pieces to the top of 
the class and separate the builder methods with a comment. 
WDYT? 

> To a lesser extent, I also don't love the cost of an extra copy, although I 
> doubt it matters and I suppose we could support moves in the build method.
I believe we can be as efficient (up to an extra move) with builders as without 
them. If most usages are of the form `RewriteRule R = 
rewriteRule(...).change(...).replaceWith(...).because(...);`
Then we could make all builder functions return r-value reference to a builder 
and have an implicit conversion operator that would consume the builder, 
producing the final `RewriteRule`:
```
class RewriteRuleBuilder {
  operator RewriteRule () &&;
  /// ...
};
RewriteRuleBuilder rewriteRule();

void addRule(RewriteRule R);
void clientCode() {
  addRule(rewriteRule().change(Matcher).replaceWith("foo").because("bar"));
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59376



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


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

2019-03-20 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a comment.
Herald added a subscriber: ebevhan.
Herald added a project: clang.

Btw, I have created a patch that allows non-underscore prefixed spelling for 
address spaces in C++ for OpenCL:
https://reviews.llvm.org/D59603

I assume the error becomes less critical however it remains. I think it still 
makes sense to fix it considering that it isn't very costly?


Repository:
  rC Clang

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

https://reviews.llvm.org/D53705



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


r356578 - [NFC][ASTMatchers] Alphabetically sort REGISTER_MATCHER() macros in RegistryMaps::RegistryMaps()

2019-03-20 Thread Roman Lebedev via cfe-commits
Author: lebedevri
Date: Wed Mar 20 10:15:47 2019
New Revision: 356578

URL: http://llvm.org/viewvc/llvm-project?rev=356578=rev
Log:
[NFC][ASTMatchers] Alphabetically sort REGISTER_MATCHER() macros in 
RegistryMaps::RegistryMaps()

As noted in https://reviews.llvm.org/D59453#inline-526253

Modified:
cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp

Modified: cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp?rev=356578=356577=356578=diff
==
--- cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp Wed Mar 20 10:15:47 2019
@@ -90,6 +90,7 @@ void RegistryMaps::registerMatcher(
   } while (false)
 
 /// Generate a registry map with all the known matchers.
+/// Please keep sorted alphabetically!
 RegistryMaps::RegistryMaps() {
   // TODO: Here is the list of the missing matchers, grouped by reason.
   //
@@ -129,12 +130,12 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(argumentCountIs);
   REGISTER_MATCHER(arraySubscriptExpr);
   REGISTER_MATCHER(arrayType);
-  REGISTER_MATCHER(asmStmt);
   REGISTER_MATCHER(asString);
+  REGISTER_MATCHER(asmStmt);
   REGISTER_MATCHER(atomicExpr);
   REGISTER_MATCHER(atomicType);
-  REGISTER_MATCHER(autoreleasePoolStmt)
   REGISTER_MATCHER(autoType);
+  REGISTER_MATCHER(autoreleasePoolStmt)
   REGISTER_MATCHER(binaryConditionalOperator);
   REGISTER_MATCHER(binaryOperator);
   REGISTER_MATCHER(blockDecl);
@@ -143,6 +144,7 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(booleanType);
   REGISTER_MATCHER(breakStmt);
   REGISTER_MATCHER(builtinType);
+  REGISTER_MATCHER(cStyleCastExpr);
   REGISTER_MATCHER(callExpr);
   REGISTER_MATCHER(caseStmt);
   REGISTER_MATCHER(castExpr);
@@ -158,7 +160,6 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(constantExpr);
   REGISTER_MATCHER(containsDeclaration);
   REGISTER_MATCHER(continueStmt);
-  REGISTER_MATCHER(cStyleCastExpr);
   REGISTER_MATCHER(cudaKernelCallExpr);
   REGISTER_MATCHER(cxxBindTemporaryExpr);
   REGISTER_MATCHER(cxxBoolLiteral);
@@ -191,10 +192,10 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(cxxUnresolvedConstructExpr);
   REGISTER_MATCHER(decayedType);
   REGISTER_MATCHER(decl);
-  REGISTER_MATCHER(declaratorDecl);
   REGISTER_MATCHER(declCountIs);
   REGISTER_MATCHER(declRefExpr);
   REGISTER_MATCHER(declStmt);
+  REGISTER_MATCHER(declaratorDecl);
   REGISTER_MATCHER(decltypeType);
   REGISTER_MATCHER(defaultStmt);
   REGISTER_MATCHER(dependentSizedArrayType);
@@ -212,7 +213,6 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(expr);
   REGISTER_MATCHER(exprWithCleanups);
   REGISTER_MATCHER(fieldDecl);
-  REGISTER_MATCHER(indirectFieldDecl);
   REGISTER_MATCHER(floatLiteral);
   REGISTER_MATCHER(forEach);
   REGISTER_MATCHER(forEachArgumentWithParam);
@@ -255,8 +255,8 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(hasCondition);
   REGISTER_MATCHER(hasConditionVariableStatement);
   REGISTER_MATCHER(hasDecayedType);
-  REGISTER_MATCHER(hasDeclaration);
   REGISTER_MATCHER(hasDeclContext);
+  REGISTER_MATCHER(hasDeclaration);
   REGISTER_MATCHER(hasDeducedType);
   REGISTER_MATCHER(hasDefaultArgument);
   REGISTER_MATCHER(hasDefinition);
@@ -290,12 +290,12 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(hasParameter);
   REGISTER_MATCHER(hasParent);
   REGISTER_MATCHER(hasQualifier);
+  REGISTER_MATCHER(hasRHS);
   REGISTER_MATCHER(hasRangeInit);
   REGISTER_MATCHER(hasReceiver);
   REGISTER_MATCHER(hasReceiverType);
   REGISTER_MATCHER(hasReplacementType);
   REGISTER_MATCHER(hasReturnValue);
-  REGISTER_MATCHER(hasRHS);
   REGISTER_MATCHER(hasSelector);
   REGISTER_MATCHER(hasSingleDecl);
   REGISTER_MATCHER(hasSize);
@@ -326,6 +326,7 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(implicitCastExpr);
   REGISTER_MATCHER(implicitValueInitExpr);
   REGISTER_MATCHER(incompleteArrayType);
+  REGISTER_MATCHER(indirectFieldDecl);
   REGISTER_MATCHER(initListExpr);
   REGISTER_MATCHER(injectedClassNameType);
   REGISTER_MATCHER(innerType);
@@ -341,15 +342,15 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(isCatchAll);
   REGISTER_MATCHER(isClass);
   REGISTER_MATCHER(isConst);
-  REGISTER_MATCHER(isConstexpr);
   REGISTER_MATCHER(isConstQualified);
+  REGISTER_MATCHER(isConstexpr);
   REGISTER_MATCHER(isCopyAssignmentOperator);
   REGISTER_MATCHER(isCopyConstructor);
   REGISTER_MATCHER(isDefaultConstructor);
   REGISTER_MATCHER(isDefaulted);
   REGISTER_MATCHER(isDefinition);
-  REGISTER_MATCHER(isDeleted);
   REGISTER_MATCHER(isDelegatingConstructor);
+  REGISTER_MATCHER(isDeleted);
   REGISTER_MATCHER(isExceptionVariable);
   REGISTER_MATCHER(isExpansionInFileMatching);
   REGISTER_MATCHER(isExpansionInMainFile);
@@ -360,13 +361,13 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(isExternC);
   REGISTER_MATCHER(isFinal);
   

r356577 - [AST] Disable ast-dump-openmp-parallel-master-XFAIL.c test

2019-03-20 Thread Roman Lebedev via cfe-commits
Author: lebedevri
Date: Wed Mar 20 10:14:49 2019
New Revision: 356577

URL: http://llvm.org/viewvc/llvm-project?rev=356577=rev
Log:
[AST] Disable ast-dump-openmp-parallel-master-XFAIL.c test

Fails on MSVC buildbot (but not locally).
Not important as it is 'testing' something that isn't supported yet anyway:
https://bugs.llvm.org/show_bug.cgi?id=41022

Modified:
cfe/trunk/test/AST/ast-dump-openmp-parallel-master-XFAIL.c

Modified: cfe/trunk/test/AST/ast-dump-openmp-parallel-master-XFAIL.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/ast-dump-openmp-parallel-master-XFAIL.c?rev=356577=356576=356577=diff
==
--- cfe/trunk/test/AST/ast-dump-openmp-parallel-master-XFAIL.c (original)
+++ cfe/trunk/test/AST/ast-dump-openmp-parallel-master-XFAIL.c Wed Mar 20 
10:14:49 2019
@@ -1,5 +1,8 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -fopenmp -fopenmp-version=50 
-ast-dump %s 2>&1 | FileCheck --match-full-lines 
-implicit-check-not=openmp_structured_block %s
 
+// REQUIRES: broken-PR41022
+// https://bugs.llvm.org/show_bug.cgi?id=41022
+
 void test_zero() {
 #pragma omp parallel master
   ;


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


[PATCH] D59603: [PR40707][PR41011][OpenCL] Allow addr space spelling without double underscore in C++ mode

2019-03-20 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia created this revision.
Anastasia added reviewers: svenvh, rjmccall.
Herald added subscribers: jdoerfert, ebevhan, yaxunl.

For backwards compatibility we should allow alternative spelling of address 
spaces - `private`, `local`, `global`, `constant`, `generic`.

Note that in order to accept `private` correctly parsing has been changed to 
understand two different use cases - access specifier vs address space.

This fixes the issues reported in the bugs.

The header from libclc is compiled successfully.


https://reviews.llvm.org/D59603

Files:
  include/clang/Basic/TokenKinds.def
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Parse/ParseTentative.cpp
  test/Parser/opencl-cxx-keywords.cl

Index: test/Parser/opencl-cxx-keywords.cl
===
--- test/Parser/opencl-cxx-keywords.cl
+++ test/Parser/opencl-cxx-keywords.cl
@@ -19,32 +19,34 @@
 // Test that only __-prefixed address space qualifiers are accepted.
 struct test_address_space_qualifiers {
   global int *g;
-  // expected-error@-1 {{unknown type name 'global'}}
-  // expected-error@-2 {{expected member name or ';' after declaration specifiers}}
   __global int *uug;
-  int global; // should be fine in OpenCL C++
+  int global; // expected-warning{{declaration does not declare anything}}
 
   local int *l;
-  // expected-error@-1 {{unknown type name 'local'}}
-  // expected-error@-2 {{expected member name or ';' after declaration specifiers}}
   __local int *uul;
-  int local; // should be fine in OpenCL C++
+  int local; // expected-warning{{declaration does not declare anything}}
 
   private int *p;
-  // expected-error@-1 {{expected ':'}}
   __private int *uup;
-  int private; // 'private' is a keyword in C++14 and thus in OpenCL C++
-  // expected-error@-1 {{expected member name or ';' after declaration specifiers}}
+  int private; // expected-warning{{declaration does not declare anything}}
 
   constant int *c;
-  // expected-error@-1 {{unknown type name 'constant'}}
-  // expected-error@-2 {{expected member name or ';' after declaration specifiers}}
   __constant int *uuc;
-  int constant; // should be fine in OpenCL C++
+  int constant; // expected-warning{{declaration does not declare anything}}
 
   generic int *ge;
-  // expected-error@-1 {{unknown type name 'generic'}}
-  // expected-error@-2 {{expected member name or ';' after declaration specifiers}}
   __generic int *uuge;
-  int generic; // should be fine in OpenCL C++
+  int generic; // expected-warning{{declaration does not declare anything}}
 };
+
+// Test that 'private' can be parsed as an access qualifier and an address space too.
+class A{
+  private:
+  private int i; //expected-error{{field may not be qualified with an address space}}
+};
+
+private int i; //expected-error{{program scope variable must reside in global or constant address space}}
+
+void foo(private int i);
+
+private int bar(); //expected-error{{return value cannot be qualified with address space}}
Index: lib/Parse/ParseTentative.cpp
===
--- lib/Parse/ParseTentative.cpp
+++ lib/Parse/ParseTentative.cpp
@@ -1411,6 +1411,7 @@
   case tok::kw_const:
   case tok::kw_volatile:
 // OpenCL address space qualifiers
+  case tok::kw_private:
   case tok::kw___private:
   case tok::kw___local:
   case tok::kw___global:
Index: lib/Parse/ParseDeclCXX.cpp
===
--- lib/Parse/ParseDeclCXX.cpp
+++ lib/Parse/ParseDeclCXX.cpp
@@ -3047,9 +3047,14 @@
 DiagnoseUnexpectedNamespace(cast(TagDecl));
 return nullptr;
 
+  case tok::kw_private:
+// FIXME: We don't accept GNU attributes on access specifiers in OpenCL mode
+// yet.
+if (getLangOpts().OpenCL && !NextToken().is(tok::colon))
+  return ParseCXXClassMemberDeclaration(AS, AccessAttrs);
+LLVM_FALLTHROUGH;
   case tok::kw_public:
-  case tok::kw_protected:
-  case tok::kw_private: {
+  case tok::kw_protected: {
 AccessSpecifier NewAS = getAccessSpecifierIfPresent();
 assert(NewAS != AS_none);
 // Current token is a C++ access specifier.
Index: lib/Parse/ParseDecl.cpp
===
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -3824,6 +3824,7 @@
 break;
   };
   LLVM_FALLTHROUGH;
+case tok::kw_private:
 case tok::kw___private:
 case tok::kw___global:
 case tok::kw___local:
@@ -4790,6 +4791,7 @@
 
   case tok::kw___kindof:
 
+  case tok::kw_private:
   case tok::kw___private:
   case tok::kw___local:
   case tok::kw___global:
@@ -4980,6 +4982,7 @@
 
   case tok::kw___kindof:
 
+  case tok::kw_private:
   case tok::kw___private:
   case tok::kw___local:
   case tok::kw___global:
@@ -5192,6 +5195,7 @@
   break;
 
 // OpenCL qualifiers:
+case tok::kw_private:
 case tok::kw___private:
 case tok::kw___global:
 case 

[PATCH] D59546: [clang-format] structured binding in range for detected as Objective C

2019-03-20 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC356575: [clang-format] structured binding in range for 
detected as Objective C (authored by paulhoad, committed by ).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D59546?vs=191288=191524#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D59546

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


Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -410,7 +410,10 @@
  Parent->isUnaryOperator() ||
  // FIXME(bug 36976): ObjC return types shouldn't use TT_CastRParen.
  Parent->isOneOf(TT_ObjCForIn, TT_CastRParen) ||
- getBinOpPrecedence(Parent->Tok.getKind(), true, true) > 
prec::Unknown);
+ // for (auto && [A,B] : C)  && structure binding seen as 
ObjCMethodExpr
+ (Parent->isNot(tok::ampamp) &&
+  getBinOpPrecedence(Parent->Tok.getKind(), true, true) >
+  prec::Unknown));
 bool ColonFound = false;
 
 unsigned BindingIncrease = 1;
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12926,6 +12926,9 @@
 guessLanguage("foo.h", "[[using gsl: suppress(\"type\")]];"));
   EXPECT_EQ(
   FormatStyle::LK_Cpp,
+  guessLanguage("foo.h", "for (auto &&[endpoint, stream] : streams_)"));
+  EXPECT_EQ(
+  FormatStyle::LK_Cpp,
   guessLanguage("foo.h",
 "[[clang::callable_when(\"unconsumed\", \"unknown\")]]"));
   EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "[[foo::bar, ...]]"));


Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -410,7 +410,10 @@
  Parent->isUnaryOperator() ||
  // FIXME(bug 36976): ObjC return types shouldn't use TT_CastRParen.
  Parent->isOneOf(TT_ObjCForIn, TT_CastRParen) ||
- getBinOpPrecedence(Parent->Tok.getKind(), true, true) > prec::Unknown);
+ // for (auto && [A,B] : C)  && structure binding seen as ObjCMethodExpr
+ (Parent->isNot(tok::ampamp) &&
+  getBinOpPrecedence(Parent->Tok.getKind(), true, true) >
+  prec::Unknown));
 bool ColonFound = false;
 
 unsigned BindingIncrease = 1;
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12926,6 +12926,9 @@
 guessLanguage("foo.h", "[[using gsl: suppress(\"type\")]];"));
   EXPECT_EQ(
   FormatStyle::LK_Cpp,
+  guessLanguage("foo.h", "for (auto &&[endpoint, stream] : streams_)"));
+  EXPECT_EQ(
+  FormatStyle::LK_Cpp,
   guessLanguage("foo.h",
 "[[clang::callable_when(\"unconsumed\", \"unknown\")]]"));
   EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "[[foo::bar, ...]]"));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D47849: [OpenMP][Clang][NVPTX] Enable math functions called in an OpenMP NVPTX target device region to be resolved as device-native function calls

2019-03-20 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.



> This is, or is very similar to, the problem that the host/device overloading 
> addresses in CUDA.

IIRC the difference was that OpenMP didn't have explicit notion of host/device 
functions which made it hard to apply host/device overloading in practice.

> It is also the problem, or very similar to the problem, that the new OpenMP 5 
> `declare variant` directive is intended to address. Johannes and I discussed 
> this earlier today, and I suggest that we:

Interesting. `declare variant ` sounds (according to openmp-TR7 doc) like a 
`__device__` on steroids. That may indeed make things work. Actually, I would 
like __device__ eventually work like `device variant`, so we can have multiple 
device overloads specialized for particular GPU architecture without relying on 
preprocessor's `__CUDA_ARCH__`.

> 
> 
> 1. Add a math.h wrapper to clang/lib/Headers, which generally just does an 
> include_next of math.h, but provides us with the ability to customize this 
> behavior. Writing a header for OpenMP on NVIDIA GPUs which is essentially 
> identical to the math.h functions in __clang_cuda_device_functions.h would be 
> unfortunate, and as CUDA does provide the underlying execution environment 
> for OpenMP target offload on NVIDIA GPUs, duplicative even in principle. We 
> don't need to alter the default global namespace, however, but can include 
> this file from the wrapper math.h.

Using `__clang_cuda_device_functions.h` in addition to `math.h` wrapper should 
be fine. It gives us a path to provide device-side standard math library 
implementation and math.h wrapper provides convenient point to hook in the 
implementation for platforms other than CUDA.

> 2. We should allow host/device overloading in OpenMP mode. As an extension, 
> we could directly reuse the CUDA host/device overloading capability - this 
> also has the advantage of allowing us to directly reuse 
> __clang_cuda_device_functions.h (and perhaps do a similar thing to pick up 
> the device-side printf, etc. from __clang_cuda_runtime_wrapper.h). In the 
> future, we can extend these to provide overloading using OpenMP declare 
> variant, if desired, when in OpenMP mode.

Is OpenMP is still essentially C-based? host/device overloading relies on C++ 
machinery. I think it should work with `__attribute__((overloadable))` but it's 
not been tested.

We may need to restructure bits and pieces of CUDA-related headers to make them 
reusable by OpenMP.  I guess that with `declare variant` we may be able to 
reuse most of the headers as is by treating `__device__` as if the function was 
a variant for NVPTX back-end.

> Thoughts?

SGTM. Let me know if something in the CUDA-related headers gets in the way.


Repository:
  rC Clang

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

https://reviews.llvm.org/D47849



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


r356575 - [clang-format] structured binding in range for detected as Objective C

2019-03-20 Thread Paul Hoad via cfe-commits
Author: paulhoad
Date: Wed Mar 20 10:10:23 2019
New Revision: 356575

URL: http://llvm.org/viewvc/llvm-project?rev=356575=rev
Log:
[clang-format] structured binding in range for detected as Objective C

Summary:
Sometime after 6.0.0 and the current trunk 9.0.0 the following code would be 
considered as objective C and not C++

Reported by: https://twitter.com/mattgodbolt/status/1096188576503644160

$ clang-format.exe test.h
Configuration file(s) do(es) not support Objective-C: 
C:\clang\build\.clang-format

--- test.h --
```

std::vector> C;

void foo()
{
   for (auto && [A,B] : C)
   {
   std::string D = A + B;
   }
}
```
The following code fixes this issue of incorrect detection

Reviewers: djasper, klimek, JonasToth, reuk

Reviewed By: klimek

Subscribers: cfe-commits

Tags: #clang-tools-extra

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

Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=356575=356574=356575=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Wed Mar 20 10:10:23 2019
@@ -410,7 +410,10 @@ private:
  Parent->isUnaryOperator() ||
  // FIXME(bug 36976): ObjC return types shouldn't use TT_CastRParen.
  Parent->isOneOf(TT_ObjCForIn, TT_CastRParen) ||
- getBinOpPrecedence(Parent->Tok.getKind(), true, true) > 
prec::Unknown);
+ // for (auto && [A,B] : C)  && structure binding seen as 
ObjCMethodExpr
+ (Parent->isNot(tok::ampamp) &&
+  getBinOpPrecedence(Parent->Tok.getKind(), true, true) >
+  prec::Unknown));
 bool ColonFound = false;
 
 unsigned BindingIncrease = 1;

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=356575=356574=356575=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Mar 20 10:10:23 2019
@@ -12926,6 +12926,9 @@ TEST_F(FormatTest, GuessLanguageWithCpp1
 guessLanguage("foo.h", "[[using gsl: suppress(\"type\")]];"));
   EXPECT_EQ(
   FormatStyle::LK_Cpp,
+  guessLanguage("foo.h", "for (auto &&[endpoint, stream] : streams_)"));
+  EXPECT_EQ(
+  FormatStyle::LK_Cpp,
   guessLanguage("foo.h",
 "[[clang::callable_when(\"unconsumed\", \"unknown\")]]"));
   EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "[[foo::bar, ...]]"));


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


[PATCH] D59599: [clangd] Fix a crash while printing Template Arguments

2019-03-20 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: clangd/AST.cpp:95
+} else {
+  // FIXME: Cls->getTypeAsWritten might return null in some cases, e.g.
+  // clang sees first sees a friend declaration and then the 
specialization.

NIT: maybe shorten the comment?
Something like `// FIXME: Fix cases when getTypeAsWritten returns null, e.g. 
friend decls.`



Comment at: unittests/clangd/SymbolCollectorTests.cpp:1226
+TEST_F(SymbolCollectorTest, TemplateSpecForwardDecl) {
+  // FIXME: This should be fixed in AST to point at specialization. Exercised
+  // just to make sure we don't crash.

NIT: or something like `getTypeAsWritten` is missing for friend declaration, 
this should be fixed in the AST>


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59599



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


[PATCH] D58404: [clang-format] Add basic support for formatting C# files

2019-03-20 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay marked an inline comment as done.
MyDeveloperDay added inline comments.



Comment at: include/clang/Basic/TokenKinds.h:80
  K == tok::utf8_string_literal || K == tok::utf16_string_literal ||
- K == tok::utf32_string_literal;
 }

klimek wrote:
> This change looks pretty good, minus the changes outside Format/ :( Do you 
> see any chance to fix this by extending the token merging? It's unlikely that 
> the community will be convinced to add some minor parts of C# to clang proper 
> just to support clang-format.
I know I'm a little nervous about having this code out where it might break the 
actual compiler

Before I got stuck with how C# does its verbatim and interpolated strings 
especially when \ is in a string just before the "

let me go back and see if I can't do this with merging the tokens now I know a 
little more


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

https://reviews.llvm.org/D58404



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


[PATCH] D59408: [clang-format] [PR25010] Extend AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

2019-03-20 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay marked 2 inline comments as done.
MyDeveloperDay added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:401
+  * ``SIS_AlwaysNoElse`` (in configuration: ``AlwaysNoElse``)
+Allow short if/else if statements even if the else is a compound statement.
+

klimek wrote:
> I'd try to make this either unconditionally what we do, or decide against 
> doing it.
see note before, this is really about maintaining compatibility, I don't want 
to make the assumption that everyone likes


```
if (x) return 1;
else if (x) return 2;
else return 3;

```

There could easily be users who want this.

```
if (x) return 1;
else if (x) return 2;
else 
   return 3;
```

I don't think it over complicates it, but it keeps the flexibility. The name of 
the option may not be the best 'suggestions welcome'



Comment at: clang/include/clang/Format/Format.h:263-264
 SIS_WithoutElse,
-/// Always put short ifs on the same line if
-/// the else is not a compound statement or not.
+/// If Else statements have no braces don't put them
+/// on the same line.
+/// \code

klimek wrote:
> This seems weird - why would we want this?
This is a compatiblity case for the previous "true" as it was before

Before 


```
if (x) 
   return 1;
else
   return 2;
```

would be transformed to


```
if (x) return 1;
else
   return 2;
```

but


```
if (x) 
   return 1;
else {
   return 2;
}
```

would not be transformed at all



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

https://reviews.llvm.org/D59408



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


  1   2   >