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

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/docs/AutomaticReferenceCounting.rst:1748
+``objc_externally_retained`` can apply to local variables, or to parameters via
+a function definitions.
+

I think "can apply to parameters and local variables" is probably the clearest 
way to express this.

I would suggest turning this around a bit.  Don't name the section after the 
attribute; instead,
start by talking about externally-retained variables as a general concept, then 
begin a new
paragraph to discuss how they arise: (1) certain implicit places which will be 
discussed below
and (2) this attribute, which you can then discuss.  It will then be natural to 
add a third
paragraph to talk about the pragma or whatever it ends up being.

The first paragraph should talk about things like capture semantics like the 
attribute docs do.



Comment at: clang/include/clang/Basic/AttrDocs.td:3811
+will be retained as part of capturing it and released when the block is
+destroyed. It also affects C++ features such as lambda capture, ``decltype``,
+and template argument deduction.

Worth adding after this sentence: "(Note that the block capture field is not 
considered
externally retained.)"


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

https://reviews.llvm.org/D55865



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

[PATCH] D56066: [OpenCL] Address space for default class members

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaInit.cpp:4539
+  if (InitCategory.isPRValue() || InitCategory.isXValue())
+T1Quals.removeAddressSpace();
+

rjmccall wrote:
> I can understand why a pr-value wouldn't have an address space, but an 
> x-value's address space is surely important.
No, wait, I think this is more deeply wrong.  We're initializing a reference to 
type `cv1 T1`, and if we're initializing it with a temporary, we're dropping 
the address space from `cv1`?  That's definitely wrong.

If we're starting with a pr-value, reference-initialization normally has to 
start by initializing a temporary.  (The exception is it's a class type with a 
conversion operator to a reference type; C++ is abysmally complicated.  Let's 
ignore this for a moment.)  Temporaries are always in the default address space 
(the address space of local variables) because the language (neither OpenCL nor 
Embedded C) does not give us any facility for allocating temporary memory 
anywhere else.  So reference-initialization from a pr-value creates a temporary 
in the default address space and then attempts to initialize the destination 
reference type with that temporary.  That initialization should fail if there's 
no conversion from the default address space to the destination address space.  
For example, consider initializing a `const int __global &` from a pr-value: 
this should clearly not succeed, because we have no way of putting a temporary 
in the global address space.  That reference can only be initialized with a 
gl-value that's already in the `__global` address space.

On the other hand, we can successfully initialize a `const int &` with a 
gl-value of type `const int __global` not by direct reference initialization 
but by loading from the operand and then materializing the result into a new 
temporary.

I think what this means is:

- We need to be doing this checking as if pr-values were in `__private`.  This 
includes both (1) expressions that were originally pr-values and (2) 
expressions that have been converted to pr-values due to a failure to perform 
direct reference initialization.

- We need to make sure that reference compatibility correctly prevents 
references from being bound to gl-values in incompatible address spaces.


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

https://reviews.llvm.org/D56066



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

[PATCH] D56243: [coroutines] Experimenting with __builtin_coro_frame_max_size

[Brian Gesiak via Phabricator via cfe-commits]

modocache created this revision.
modocache added reviewers: GorNishanov, lewissbaker, tks2103.
Herald added subscribers: cfe-commits, kristina, EricWF.

This commit implements a proposed addendum to the C++ Coroutines TS that
would allow users to query, at compile time, the maximum potential size
of a coroutine frame, from within the frontend.

The __builtin_coro_frame_max_size allows users to access this size, for
coroutine lambdas, by passing in the type of the coroutine lambda into
the function from which __builtin_coro_frame_max_size is called.
https://reviews.llvm.org/P8124 provides an example of such a program.


Repository:
  rC Clang

https://reviews.llvm.org/D56243

Files:
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGCoroutine.cpp
  lib/CodeGen/CodeGenFunction.h

Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -2880,6 +2880,7 @@
  bool ignoreResult = false);
   LValue EmitCoyieldLValue(const CoyieldExpr *E);
   RValue EmitCoroutineIntrinsic(const CallExpr *E, unsigned int IID);
+  RValue EmitCoroutineFrameMaxSize(const CallExpr *E);
 
   void EnterCXXTryStmt(const CXXTryStmt , bool IsFnTryBlock = false);
   void ExitCXXTryStmt(const CXXTryStmt , bool IsFnTryBlock = false);
Index: lib/CodeGen/CGCoroutine.cpp
===
--- lib/CodeGen/CGCoroutine.cpp
+++ lib/CodeGen/CGCoroutine.cpp
@@ -14,8 +14,10 @@
 #include "CGCleanup.h"
 #include "CodeGenFunction.h"
 #include "llvm/ADT/ScopeExit.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtVisitor.h"
+#include "clang/Basic/TargetInfo.h"
 
 using namespace clang;
 using namespace CodeGen;
@@ -758,3 +760,172 @@
   }
   return RValue::get(Call);
 }
+
+namespace {
+class FrameSizeBuilder {
+  const ASTContext 
+  uint64_t FrameSize = 0;
+
+public:
+  FrameSizeBuilder(ASTContext ) : Context(Context) {}
+
+  uint64_t getFrameSize() const { return FrameSize; }
+
+  void addType(const QualType Ty) {
+if ((FrameSize & (Context.getTypeAlign(Ty) - 1)) != 0)
+  FrameSize = llvm::alignTo(FrameSize,
+Context.getTypeAlignInChars(Ty).getQuantity());
+FrameSize += Context.getTypeSizeInChars(Ty).getQuantity();
+  }
+
+  void addType(TargetInfo::IntType Ty) {
+const TargetInfo  = Context.getTargetInfo();
+if ((FrameSize & (TI.getTypeAlign(Ty) - 1)) != 0)
+  FrameSize = llvm::alignTo(
+  FrameSize,
+  Context.toCharUnitsFromBits(TI.getTypeAlign(Ty)).getQuantity());
+FrameSize += TI.getTypeWidth(Ty);
+  }
+};
+
+class PotentialSpillsVisitor
+: public RecursiveASTVisitor {
+  FrameSizeBuilder 
+  const QualType HandleTy;
+
+public:
+  PotentialSpillsVisitor(FrameSizeBuilder , QualType HandleTy)
+  : Builder(Builder), HandleTy(HandleTy) {}
+
+  bool shouldVisitImplicitCode() const { return true; }
+
+  // Add up the size of any variables explicitly declared within the
+  // coroutine, as well as the implicit __promise and __coro_gro variables.
+  bool VisitVarDecl(VarDecl *VD) {
+Builder.addType(VD->getType());
+return true;
+  }
+
+  // Add up the size of any temporaries materialized within the coroutine, as
+  // well as the implicit temporaries materialized when:
+  // 1. __promise.initial_suspend and .final_suspend are called to construct
+  //awaitables
+  // 2. Coroutine handles are materialized via .from_address and
+  //passed in as arguments to __builtin_coro_frame. Coroutine handles also
+  //get bitcast to void* in LLVM IR and the bitcast spills, so the bitcast
+  //also needs to be accounted for in the coroutine frame size
+  // 3. __promise.get_return_object is called to construct return objects
+  bool VisitMaterializeTemporaryExpr(MaterializeTemporaryExpr *E) {
+QualType ETy = E->GetTemporaryExpr()->getType();
+Builder.addType(ETy);
+
+// LLVM spills the bitcast for this type, causing an additional increase
+// in coroutine frame size.
+if (ETy == HandleTy)
+  Builder.addType(ETy);
+
+return true;
+  }
+};
+};
+
+static RValue emitFrameMaxSizeError(const CodeGenFunction ,
+const CallExpr *E, StringRef Err) {
+  CGF.CGM.Error(E->getExprLoc(), "__builtin_coro_frame_max_size " + Err.str());
+  return RValue::get(
+  llvm::Constant::getIntegerValue(CGF.Int64Ty, llvm::APInt(64, 0)));
+}
+
+RValue CodeGenFunction::EmitCoroutineFrameMaxSize(const CallExpr *E) {
+  auto *FD = dyn_cast_or_null(CurFuncDecl);
+  if (!FD)
+return emitFrameMaxSizeError(*this, E,
+ "must be used from within a function");
+
+  const TemplateArgumentList *Args = FD->getTemplateSpecializationArgs();
+  if (Args->size() != 1)
+return emitFrameMaxSizeError(
+*this, E,
+"must be 

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

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

Kader created this revision.
Kader added a project: clang.
Herald added a subscriber: cfe-commits.

While building openJDK11u, it seems that some of the code in the native core 
libraries make liberal use of integer to pointer comparisons. We currently have 
no flag to disabled this warning. This add such a flag.


Repository:
  rC Clang

https://reviews.llvm.org/D56241

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


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


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

[PATCH] D56109: [sanitizer_common] Define __sanitizer_FILE on NetBSD

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

In D56109#1344521 , @vitalybuka wrote:

> LGTM


Thanks! Any feedback regarding the raised comments in 
https://reviews.llvm.org/D56109#1341967 and 
https://reviews.llvm.org/D56109#1342059 ?


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

https://reviews.llvm.org/D56109



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

[PATCH] D56109: [sanitizer_common] Define __sanitizer_FILE on NetBSD

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

LGTM


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

https://reviews.llvm.org/D56109



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

[PATCH] D56160: [clang-tidy] modernize-use-trailing-return check

[Bernhard Manfred Gruber via Phabricator via cfe-commits]

bernhardmgruber updated this revision to Diff 179986.
bernhardmgruber added a comment.

rebased from release_70 onto master


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

https://reviews.llvm.org/D56160

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/UseTrailingReturnCheck.cpp
  clang-tidy/modernize/UseTrailingReturnCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-use-trailing-return.rst
  test/clang-tidy/modernize-use-trailing-return.cpp

Index: test/clang-tidy/modernize-use-trailing-return.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-trailing-return.cpp
@@ -0,0 +1,173 @@
+// RUN: %check_clang_tidy %s modernize-use-trailing-return %t -- -- --std=c++14
+
+namespace std {
+template 
+class vector;
+
+class string;
+}
+
+//
+// Samples triggering the check
+//
+
+int f();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto f() -> int;{{$}}
+int f(int);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto f(int) -> int;{{$}}
+int f(int arg);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto f(int arg) -> int;{{$}}
+int f(int arg1, int arg2, int arg3);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto f(int arg1, int arg2, int arg3) -> int;{{$}}
+int f(int arg1, int arg2, int arg3, ...);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto f(int arg1, int arg2, int arg3, ...) -> int;{{$}}
+template  int f(T t);
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}template  auto f(T t) -> int;{{$}}
+int a1() { return 42; }
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto a1() -> int { return 42; }{{$}}
+int a2() {
+return 42;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto a2() -> int {{{$}}
+int a3()
+{
+return 42;
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto a3() -> int{{$}}
+int b1(int   arg   )   ;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto b1(int   arg   ) -> int   ;{{$}}
+int b2
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto b2{{$}}
+(int arg);
+// CHECK-FIXES: {{^}}(int arg) -> int;{{$}}
+inline int d1(int arg);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}inline auto d1(int arg) -> int;{{$}}
+inline int d2(int arg) noexcept(true);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}inline auto d2(int arg) -> int noexcept(true);{{$}}
+inline int d3(int arg) try { } catch(...) { }
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}inline auto d3(int arg) -> int try { } catch(...) { }{{$}}
+namespace N {
+bool e1();
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:10: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto e1() -> bool;{{$}}
+inline volatile const std::vector e2();
+// CHECK-MESSAGES: :[[@LINE-1]]:48: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}inline auto e2() -> volatile const std::vector;{{$}}
+inline const std::vector volatile e2();
+// CHECK-MESSAGES: :[[@LINE-1]]:48: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}inline auto e2() -> const std::vector volatile;{{$}}
+inline std::vector const volatile e2();
+// CHECK-MESSAGES: :[[@LINE-1]]:48: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}inline auto e2() -> std::vector const volatile;{{$}}
+bool N::e1() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use a trailing return type for this function 

Re: r350282 - [libclang] CoroutineBody/Coreturn statements are UnexposedStmts and not Exprs

[Alex L via cfe-commits]

Oops, I committed wrong column number in the test due to reindentation.
Fixed in r350283.

~ Alex

On Wed, 2 Jan 2019 at 17:16, Alex Lorenz via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: arphaman
> Date: Wed Jan  2 17:13:33 2019
> New Revision: 350282
>
> URL: http://llvm.org/viewvc/llvm-project?rev=350282=rev
> Log:
> [libclang] CoroutineBody/Coreturn statements are UnexposedStmts and not
> Exprs
>
> This change ensures that the libclang CXCursor represents the CoroutineBody
> and the Coreturn statement using the appropriate CXCursor_UnexposedStmt
> kind
> instead of CXCursor_UnexposedExpr. The problem with CXCursor_UnexposedExpr
> is
> that the consumer functions assumed that CoroutineBody/Coreturn statements
> were valid expressions and performed an invalid downcast to Expr causing
> assertion failures or other crashes.
>
> rdar://40204290
>
> Added:
> cfe/trunk/test/Index/coroutines.cpp
> Modified:
> cfe/trunk/tools/libclang/CXCursor.cpp
>
> Added: cfe/trunk/test/Index/coroutines.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/coroutines.cpp?rev=350282=auto
>
> ==
> --- cfe/trunk/test/Index/coroutines.cpp (added)
> +++ cfe/trunk/test/Index/coroutines.cpp Wed Jan  2 17:13:33 2019
> @@ -0,0 +1,24 @@
> +// RUN: c-index-test -test-load-source all -c %s -fsyntax-only -target
> x86_64-apple-darwin9 -fcoroutines-ts -std=c++1z -I%S/../SemaCXX/Inputs |
> FileCheck %s
> +#include "std-coroutine.h"
> +
> +using std::experimental::suspend_always;
> +using std::experimental::suspend_never;
> +
> +struct promise_void {
> +  void get_return_object();
> +  suspend_always initial_suspend();
> +  suspend_always final_suspend();
> +  void return_void();
> +  void unhandled_exception();
> +};
> +
> +template <>
> +struct std::experimental::coroutine_traits { using promise_type =
> promise_void; };
> +
> +void CoroutineTestRet() {
> +  co_return;
> +}
> +// CHECK: [[@LINE-3]]:25: UnexposedStmt=
> +// CHECK-SAME: [[@LINE-4]]:25 - [[@LINE-2]]:2]
> +// CHECK: [[@LINE-4]]:5: UnexposedStmt=
> +// CHECK-SAME: [[@LINE-5]]:5 - [[@LINE-5]]:14]
>
> Modified: cfe/trunk/tools/libclang/CXCursor.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CXCursor.cpp?rev=350282=350281=350282=diff
>
> ==
> --- cfe/trunk/tools/libclang/CXCursor.cpp (original)
> +++ cfe/trunk/tools/libclang/CXCursor.cpp Wed Jan  2 17:13:33 2019
> @@ -241,16 +241,19 @@ CXCursor cxcursor::MakeCXCursor(const St
>case Stmt::SEHLeaveStmtClass:
>  K = CXCursor_SEHLeaveStmt;
>  break;
> -
> +
> +  case Stmt::CoroutineBodyStmtClass:
> +  case Stmt::CoreturnStmtClass:
> +K = CXCursor_UnexposedStmt;
> +break;
> +
>case Stmt::ArrayTypeTraitExprClass:
>case Stmt::AsTypeExprClass:
>case Stmt::AtomicExprClass:
>case Stmt::BinaryConditionalOperatorClass:
>case Stmt::TypeTraitExprClass:
> -  case Stmt::CoroutineBodyStmtClass:
>case Stmt::CoawaitExprClass:
>case Stmt::DependentCoawaitExprClass:
> -  case Stmt::CoreturnStmtClass:
>case Stmt::CoyieldExprClass:
>case Stmt::CXXBindTemporaryExprClass:
>case Stmt::CXXDefaultArgExprClass:
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r350283 - Fix incorrect column numbers in test from r350282.

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Wed Jan  2 17:30:50 2019
New Revision: 350283

URL: http://llvm.org/viewvc/llvm-project?rev=350283=rev
Log:
Fix incorrect column numbers in test from r350282.

After the test was reformatted using clang-format the numbers became invalid.

Modified:
cfe/trunk/test/Index/coroutines.cpp

Modified: cfe/trunk/test/Index/coroutines.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/coroutines.cpp?rev=350283=350282=350283=diff
==
--- cfe/trunk/test/Index/coroutines.cpp (original)
+++ cfe/trunk/test/Index/coroutines.cpp Wed Jan  2 17:30:50 2019
@@ -20,5 +20,5 @@ void CoroutineTestRet() {
 }
 // CHECK: [[@LINE-3]]:25: UnexposedStmt=
 // CHECK-SAME: [[@LINE-4]]:25 - [[@LINE-2]]:2]
-// CHECK: [[@LINE-4]]:5: UnexposedStmt=
-// CHECK-SAME: [[@LINE-5]]:5 - [[@LINE-5]]:14]
+// CHECK: [[@LINE-4]]:3: UnexposedStmt=
+// CHECK-SAME: [[@LINE-5]]:3 - [[@LINE-5]]:12]


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

r350282 - [libclang] CoroutineBody/Coreturn statements are UnexposedStmts and not Exprs

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Wed Jan  2 17:13:33 2019
New Revision: 350282

URL: http://llvm.org/viewvc/llvm-project?rev=350282=rev
Log:
[libclang] CoroutineBody/Coreturn statements are UnexposedStmts and not Exprs

This change ensures that the libclang CXCursor represents the CoroutineBody
and the Coreturn statement using the appropriate CXCursor_UnexposedStmt kind
instead of CXCursor_UnexposedExpr. The problem with CXCursor_UnexposedExpr is
that the consumer functions assumed that CoroutineBody/Coreturn statements
were valid expressions and performed an invalid downcast to Expr causing
assertion failures or other crashes.

rdar://40204290

Added:
cfe/trunk/test/Index/coroutines.cpp
Modified:
cfe/trunk/tools/libclang/CXCursor.cpp

Added: cfe/trunk/test/Index/coroutines.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/coroutines.cpp?rev=350282=auto
==
--- cfe/trunk/test/Index/coroutines.cpp (added)
+++ cfe/trunk/test/Index/coroutines.cpp Wed Jan  2 17:13:33 2019
@@ -0,0 +1,24 @@
+// RUN: c-index-test -test-load-source all -c %s -fsyntax-only -target 
x86_64-apple-darwin9 -fcoroutines-ts -std=c++1z -I%S/../SemaCXX/Inputs | 
FileCheck %s
+#include "std-coroutine.h"
+
+using std::experimental::suspend_always;
+using std::experimental::suspend_never;
+
+struct promise_void {
+  void get_return_object();
+  suspend_always initial_suspend();
+  suspend_always final_suspend();
+  void return_void();
+  void unhandled_exception();
+};
+
+template <>
+struct std::experimental::coroutine_traits { using promise_type = 
promise_void; };
+
+void CoroutineTestRet() {
+  co_return;
+}
+// CHECK: [[@LINE-3]]:25: UnexposedStmt=
+// CHECK-SAME: [[@LINE-4]]:25 - [[@LINE-2]]:2]
+// CHECK: [[@LINE-4]]:5: UnexposedStmt=
+// CHECK-SAME: [[@LINE-5]]:5 - [[@LINE-5]]:14]

Modified: cfe/trunk/tools/libclang/CXCursor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CXCursor.cpp?rev=350282=350281=350282=diff
==
--- cfe/trunk/tools/libclang/CXCursor.cpp (original)
+++ cfe/trunk/tools/libclang/CXCursor.cpp Wed Jan  2 17:13:33 2019
@@ -241,16 +241,19 @@ CXCursor cxcursor::MakeCXCursor(const St
   case Stmt::SEHLeaveStmtClass:
 K = CXCursor_SEHLeaveStmt;
 break;
-  
+
+  case Stmt::CoroutineBodyStmtClass:
+  case Stmt::CoreturnStmtClass:
+K = CXCursor_UnexposedStmt;
+break;
+
   case Stmt::ArrayTypeTraitExprClass:
   case Stmt::AsTypeExprClass:
   case Stmt::AtomicExprClass:
   case Stmt::BinaryConditionalOperatorClass:
   case Stmt::TypeTraitExprClass:
-  case Stmt::CoroutineBodyStmtClass:
   case Stmt::CoawaitExprClass:
   case Stmt::DependentCoawaitExprClass:
-  case Stmt::CoreturnStmtClass:
   case Stmt::CoyieldExprClass:
   case Stmt::CXXBindTemporaryExprClass:
   case Stmt::CXXDefaultArgExprClass:


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

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

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

I assume that we need to reimplement this in the context of lld:

  NetBSD::NetBSD(const Driver , const llvm::Triple , const ArgList 
)
  : Generic_ELF(D, Triple, Args) {
if (!Args.hasArg(options::OPT_nostdlib)) {
  // When targeting a 32-bit platform, try the special directory used on
  // 64-bit hosts, and only fall back to the main library directory if that
  // doesn't work.
  // FIXME: It'd be nicer to test if this directory exists, but I'm not sure
  // what all logic is needed to emulate the '=' prefix here.
  switch (Triple.getArch()) {
  case llvm::Triple::x86:
getFilePaths().push_back("=/usr/lib/i386");
break;
  case llvm::Triple::arm:
  case llvm::Triple::armeb:
  case llvm::Triple::thumb:
  case llvm::Triple::thumbeb:
switch (Triple.getEnvironment()) {
case llvm::Triple::EABI:
case llvm::Triple::GNUEABI:
  getFilePaths().push_back("=/usr/lib/eabi");
  break;
case llvm::Triple::EABIHF:
case llvm::Triple::GNUEABIHF:
  getFilePaths().push_back("=/usr/lib/eabihf");
  break;
default:
  getFilePaths().push_back("=/usr/lib/oabi");
  break;
}
break;
  case llvm::Triple::mips64:
  case llvm::Triple::mips64el:
if (tools::mips::hasMipsAbiArg(Args, "o32"))
  getFilePaths().push_back("=/usr/lib/o32");
else if (tools::mips::hasMipsAbiArg(Args, "64"))
  getFilePaths().push_back("=/usr/lib/64");
break;
  case llvm::Triple::ppc:
getFilePaths().push_back("=/usr/lib/powerpc");
break;
  case llvm::Triple::sparc:
getFilePaths().push_back("=/usr/lib/sparc");
break;
  default:
break;
  }
  
  getFilePaths().push_back("=/usr/lib");
}
  }

https://github.com/llvm-mirror/clang/blob/e030444510df2ffaf23eeae35692dc363bc28439/lib/Driver/ToolChains/NetBSD.cpp#L334-L379


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56215



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

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

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:3787
+variable goes out of scope. Parameters and variables annotated with this
+attribute are implicitly ``const``.
+

rjmccall wrote:
> You should add a paragraph contrasting this with ``__unsafe_unretained``.  
> For example:
> 
>   Unlike an `__unsafe_unretained` variable, an externally-retained variable 
> remains
>   semantically `__strong`.  This affects features like block capture which 
> copy the
>   current value of the variable into a capture field of the same type: the 
> block's
>   capture field will still be `__strong`, and so the value will be retained 
> as part of
>   capturing it and released when the block is destroyed.  It also affects C++ 
> features
>   such as lambda capture, `decltype`, and template argument deduction.
> 
> You should also describe this attribute in AutomaticReferenceCounting.rst.
> You can add it in a new `arc.misc.externally_retained` section immediately 
> above
> `arc.misc.self`.  You can borrow some of the existing wording from the two
> following sections, `arc.misc.self` and `arc.misc.enumeration`, and then make
> those sections refer back to the new concept.
Sure, I pretty much just copied that in verbatim. New patch updates ARC.rst as 
well.



Comment at: clang/test/SemaObjC/externally-retained.m:14
+
+  EXT_RET int (^d)() = ^{return 0;};
+  EXT_RET ObjCTy *e = 0;

aaron.ballman wrote:
> erik.pilkington wrote:
> > aaron.ballman wrote:
> > > Should this be useful for function pointer parameters as well? e.g.,
> > > ```
> > > typedef void (*fp)(EXT_RET __strong ObjCTy *);
> > > 
> > > void f(__strong ObjCTy *);
> > > 
> > > void g(EXT_RET ObjCTy *Ptr) {
> > >   fp Fn = f; // Good idea? Bad idea?
> > >   Fn(Ptr); // Which behavior "wins" in this call?
> > > }
> > > ```
> > The attribute doesn't have any effect on the caller side, so when used with 
> > a function pointer type the attribute doesn't really do anything (the 
> > function definition always "wins").
> Perhaps we should warn (and drop the attribute) if you try to write it on a 
> parameter in a function pointer type rather than a function declaration? That 
> way users won't think code like the above does anything useful. 
> Alternatively, we could warn if there was a mismatch between markings (so the 
> attribute is still a noop with function pointer types, but the consistent 
> markings make that benign).
In the new patch, applying the attribute to a parameter in a function pointer 
type is impossible.


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

https://reviews.llvm.org/D55865



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

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

[Erik Pilkington via Phabricator via cfe-commits]

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

Address review comments. Also, make the attribute apply to parameters via the 
function declaration instead to parameters directly. i.e.:

  __attribute__((objc_externally_retained(1)))
  void foo(Widget *w) { ... }

The rule being that when this is applied to a non-param variable, it behaves as 
before, but when applied to a function it takes zero or more indices that 
correspond to which parameters are pseudo-strong. If no indices are specified, 
then it applies to every parameter that isn't explicitly __strong. This has a 
couple of advantages:

1. It fixes a more "theoretical" ABI break when used with objective-c++ 
decltype:

  void foo(Widget *w, decltype(w) *) {}

Previously, adding an attribute to `w` would have caused the mangling of `foo` 
to change because `decltype(w)` was `Widget * __strong const`. Now, we wait 
until the `FunctionType` is constructed and the `decltype(w)` is resolved 
before tampering with the parameter type, so this gets the same mangling, but 
we still get the extra safety.

2. It makes it more obvious what this attribute can/can't be applied to. i.e. 
it now is impossible to annotate parameters of function types, which doesn't 
make any sense.

3. We don't have to add special-case hacks to `#pragma clang attribute` to make 
this work. i.e.:

  #pragma clang attribute push (__attribute__((objc_externally_retained)), 
apply_to=any(function,objc_method,block))
  void f(Widget *w, __strong Widget *other) {} // w is externally-retained, 
other is not
  #pragma clang attribute pop

Whereas previously we would have had to add something like 
`apply_to=is_objc_retainable_pointer_parameter_of_function_definition_that_is_implicitly_strong`
 in order to get these semantics.

Thanks for taking a look!


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

https://reviews.llvm.org/D55865

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

Index: clang/test/SemaObjC/externally-retained.m
===
--- /dev/null
+++ clang/test/SemaObjC/externally-retained.m
@@ -0,0 +1,127 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.13.0 -fobjc-runtime=macosx-10.13.0 -fblocks -fobjc-arc %s -verify
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.13.0 -fobjc-runtime=macosx-10.13.0 -fblocks -fobjc-arc -xobjective-c++ %s -verify
+
+#define EXT_RET __attribute__((objc_externally_retained))
+#define CALL_RET(...) __attribute__((objc_externally_retained(__VA_ARGS__)))
+
+@interface ObjCTy
+@end
+
+void test1() {
+  EXT_RET int a; // expected-warning{{'objc_externally_retained' can only be applied to}}
+  EXT_RET __weak ObjCTy *b; // expected-warning{{'objc_externally_retained' can only be applied to}}
+  EXT_RET __weak int (^c)(); // expected-warning{{'objc_externally_retained' can only be applied to}}
+
+  EXT_RET int (^d)() = ^{return 0;};
+  EXT_RET ObjCTy *e = 0;
+  EXT_RET __strong ObjCTy *f = 0;
+
+  e = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+  f = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+  d = ^{ return 0; }; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+}
+
+void test2(ObjCTy *a);
+
+void test2(ObjCTy *a) CALL_RET(1) {
+  a = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+}
+
+EXT_RET ObjCTy *test3; // expected-warning{{'objc_externally_retained' can only be applied to}}
+
+@interface X // expected-warning{{defined without specifying a base class}} expected-note{{add a super class}}
+-(void)m: (ObjCTy *) p;
+@end
+@implementation X
+-(void)m: (ObjCTy *) p CALL_RET(1) {
+  p = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+}
+@end
+
+void test4() {
+  __attribute__((objc_externally_retained(0))) ObjCTy *a; // expected-error{{'objc_externally_retained' attribute takes no arguments}}
+}
+
+void test5(ObjCTy *first, __strong ObjCTy *second) CALL_RET() {
+  first = 0; // expected-error{{variable declared with 'objc_externally_retained' cannot be modified in ARC}}
+  second = 0; // fine
+}
+
+void 

[PATCH] D56160: [clang-tidy] modernize-use-trailing-return check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/ReleaseNotes.rst:35
 
+- New :doc:`modernize-use-trailing-return
+  ` check.

Please rebase from trunk and use alphabetical order for new checks list.


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

https://reviews.llvm.org/D56160



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

[PATCH] D56067: Make test/Driver/darwin-sdk-version.c pass if the host triple is 32-bit

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL350278: Make test/Driver/darwin-sdk-version.c pass if the 
host triple is 32-bit (authored by nico, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D56067?vs=179479=179966#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D56067

Files:
  cfe/trunk/test/Driver/darwin-sdk-version.c


Index: cfe/trunk/test/Driver/darwin-sdk-version.c
===
--- cfe/trunk/test/Driver/darwin-sdk-version.c
+++ cfe/trunk/test/Driver/darwin-sdk-version.c
@@ -5,10 +5,10 @@
 //
 // RUN: rm -rf %t/SDKs/MacOSX10.10.sdk
 // RUN: mkdir -p %t/SDKs/MacOSX10.10.sdk
-// RUN: %clang -isysroot %t/SDKs/MacOSX10.10.sdk -c -### %s 2>&1 \
+// RUN: %clang -m64 -isysroot %t/SDKs/MacOSX10.10.sdk -c -### %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=INFER_SDK_VERSION %s
 // RUN: sed -e 's/10\.14/10\.8/g' %S/Inputs/MacOSX10.14.sdk/SDKSettings.json > 
%t/SDKs/MacOSX10.10.sdk/SDKSettings.json
-// RUN: %clang -isysroot %t/SDKs/MacOSX10.10.sdk -c -### %s 2>&1 \
+// RUN: %clang -m64 -isysroot %t/SDKs/MacOSX10.10.sdk -c -### %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=INFER_DEPLOYMENT_TARGET_VERSION %s
 // REQUIRES: system-darwin && native
 //


Index: cfe/trunk/test/Driver/darwin-sdk-version.c
===
--- cfe/trunk/test/Driver/darwin-sdk-version.c
+++ cfe/trunk/test/Driver/darwin-sdk-version.c
@@ -5,10 +5,10 @@
 //
 // RUN: rm -rf %t/SDKs/MacOSX10.10.sdk
 // RUN: mkdir -p %t/SDKs/MacOSX10.10.sdk
-// RUN: %clang -isysroot %t/SDKs/MacOSX10.10.sdk -c -### %s 2>&1 \
+// RUN: %clang -m64 -isysroot %t/SDKs/MacOSX10.10.sdk -c -### %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=INFER_SDK_VERSION %s
 // RUN: sed -e 's/10\.14/10\.8/g' %S/Inputs/MacOSX10.14.sdk/SDKSettings.json > %t/SDKs/MacOSX10.10.sdk/SDKSettings.json
-// RUN: %clang -isysroot %t/SDKs/MacOSX10.10.sdk -c -### %s 2>&1 \
+// RUN: %clang -m64 -isysroot %t/SDKs/MacOSX10.10.sdk -c -### %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=INFER_DEPLOYMENT_TARGET_VERSION %s
 // REQUIRES: system-darwin && native
 //
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r350278 - Make test/Driver/darwin-sdk-version.c pass if the host triple is 32-bit

[Nico Weber via cfe-commits]

Author: nico
Date: Wed Jan  2 16:17:02 2019
New Revision: 350278

URL: http://llvm.org/viewvc/llvm-project?rev=350278=rev
Log:
Make test/Driver/darwin-sdk-version.c pass if the host triple is 32-bit

For some reason, the cmake build on my macbook has
LLVM_HOST_TRIPLE:STRING=i386-apple-darwin16.7.0 .
test/Driver/darwin-sdk-version.c assumed that the host triple is 64-bit, so
make it resilient against 32-bit host triples.

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

Modified:
cfe/trunk/test/Driver/darwin-sdk-version.c

Modified: cfe/trunk/test/Driver/darwin-sdk-version.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/darwin-sdk-version.c?rev=350278=350277=350278=diff
==
--- cfe/trunk/test/Driver/darwin-sdk-version.c (original)
+++ cfe/trunk/test/Driver/darwin-sdk-version.c Wed Jan  2 16:17:02 2019
@@ -5,10 +5,10 @@
 //
 // RUN: rm -rf %t/SDKs/MacOSX10.10.sdk
 // RUN: mkdir -p %t/SDKs/MacOSX10.10.sdk
-// RUN: %clang -isysroot %t/SDKs/MacOSX10.10.sdk -c -### %s 2>&1 \
+// RUN: %clang -m64 -isysroot %t/SDKs/MacOSX10.10.sdk -c -### %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=INFER_SDK_VERSION %s
 // RUN: sed -e 's/10\.14/10\.8/g' %S/Inputs/MacOSX10.14.sdk/SDKSettings.json > 
%t/SDKs/MacOSX10.10.sdk/SDKSettings.json
-// RUN: %clang -isysroot %t/SDKs/MacOSX10.10.sdk -c -### %s 2>&1 \
+// RUN: %clang -m64 -isysroot %t/SDKs/MacOSX10.10.sdk -c -### %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=INFER_DEPLOYMENT_TARGET_VERSION %s
 // REQUIRES: system-darwin && native
 //


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

[PATCH] D56160: [clang-tidy] modernize-use-trailing-return check

[Bernhard Manfred Gruber via Phabricator via cfe-commits]

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



Comment at: test/clang-tidy/modernize-use-trailing-return.cpp:2
+// RUN: %check_clang_tidy %s modernize-use-trailing-return %t -- -- --std=c++14
+
+namespace std {

aaron.ballman wrote:
> lebedev.ri wrote:
> > Missing test coverage:
> > * macros
> > * is there tests for implicit functions?
> > * Out-of-line function with body.
> Also:
>   * functions with attributes in the type position `int f() [[]];`
>   * functions without attributes in the type position `[[]] int f();` and 
> `int f [[]] ();`
>   * lambdas?
>   * Special functions without return types, like constructors and destructors
>   * Conversion operators. `struct S {  operator int() const; };`
@lebedev.ri: I do not understand what kind of tests I should write for macros. 
Do you mean to rewrite functions inside macro definitions as well?

like rewriting
```
#define DEFINE_F int f();
```
into

```
#define DEFINE_F auto f() -> int;
```

Apart from that, I believe a lot of difficult trickery can be done with macros 
and I am fine without supporting those (i.e. omitting a possible rewrite, 
erroneus replacements should never happen). Can you come up with a particular 
example you would like to be rewritten?


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

https://reviews.llvm.org/D56160



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

[PATCH] D56160: [clang-tidy] modernize-use-trailing-return check

[Bernhard Manfred Gruber via Phabricator via cfe-commits]

bernhardmgruber updated this revision to Diff 179961.
bernhardmgruber added a comment.

added more test cases, allowing check to run on out-of-line definitions and 
updated docs.


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

https://reviews.llvm.org/D56160

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/UseTrailingReturnCheck.cpp
  clang-tidy/modernize/UseTrailingReturnCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-use-trailing-return.rst
  test/clang-tidy/modernize-use-trailing-return.cpp

Index: test/clang-tidy/modernize-use-trailing-return.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-trailing-return.cpp
@@ -0,0 +1,173 @@
+// RUN: %check_clang_tidy %s modernize-use-trailing-return %t -- -- --std=c++14
+
+namespace std {
+template 
+class vector;
+
+class string;
+}
+
+//
+// Samples triggering the check
+//
+
+int f();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto f() -> int;{{$}}
+int f(int);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto f(int) -> int;{{$}}
+int f(int arg);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto f(int arg) -> int;{{$}}
+int f(int arg1, int arg2, int arg3);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto f(int arg1, int arg2, int arg3) -> int;{{$}}
+int f(int arg1, int arg2, int arg3, ...);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto f(int arg1, int arg2, int arg3, ...) -> int;{{$}}
+template  int f(T t);
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}template  auto f(T t) -> int;{{$}}
+int a1() { return 42; }
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto a1() -> int { return 42; }{{$}}
+int a2() {
+return 42;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto a2() -> int {{{$}}
+int a3()
+{
+return 42;
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto a3() -> int{{$}}
+int b1(int   arg   )   ;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto b1(int   arg   ) -> int   ;{{$}}
+int b2
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto b2{{$}}
+(int arg);
+// CHECK-FIXES: {{^}}(int arg) -> int;{{$}}
+inline int d1(int arg);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}inline auto d1(int arg) -> int;{{$}}
+inline int d2(int arg) noexcept(true);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}inline auto d2(int arg) -> int noexcept(true);{{$}}
+inline int d3(int arg) try { } catch(...) { }
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}inline auto d3(int arg) -> int try { } catch(...) { }{{$}}
+namespace N {
+bool e1();
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:10: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto e1() -> bool;{{$}}
+inline volatile const std::vector e2();
+// CHECK-MESSAGES: :[[@LINE-1]]:48: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}inline auto e2() -> volatile const std::vector;{{$}}
+inline const std::vector volatile e2();
+// CHECK-MESSAGES: :[[@LINE-1]]:48: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}inline auto e2() -> const std::vector volatile;{{$}}
+inline std::vector const volatile e2();
+// CHECK-MESSAGES: :[[@LINE-1]]:48: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}inline auto e2() -> std::vector const volatile;{{$}}
+bool N::e1() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use a 

[clang-tools-extra] r350273 - [Documentation] Alphabetical order in Clang-tidy checks changes list.

[Eugene Zelenko via cfe-commits]

Author: eugenezelenko
Date: Wed Jan  2 15:35:57 2019
New Revision: 350273

URL: http://llvm.org/viewvc/llvm-project?rev=350273=rev
Log:
[Documentation] Alphabetical order in Clang-tidy checks changes list.

Modified:
clang-tools-extra/trunk/docs/ReleaseNotes.rst

Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=350273=350272=350273=diff
==
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Wed Jan  2 15:35:57 2019
@@ -232,14 +232,6 @@ Improvements to clang-tidy
   `
   added.
 
-- The :doc:`readability-redundant-smartptr-get
-  ` check does not warn
-  about calls inside macros anymore by default.
-
-- The :doc:`readability-uppercase-literal-suffix
-  ` check does not warn
-  about literal suffixes inside macros anymore by default.
-
 - The :doc:`cppcoreguidelines-narrowing-conversions
   ` check now
   detects more narrowing conversions:
@@ -252,6 +244,14 @@ Improvements to clang-tidy
   ` check now ignores the
   `Acronyms` and `IncludeDefaultAcronyms` options.
 
+- The :doc:`readability-redundant-smartptr-get
+  ` check does not warn
+  about calls inside macros anymore by default.
+
+- The :doc:`readability-uppercase-literal-suffix
+  ` check does not warn
+  about literal suffixes inside macros anymore by default.
+
 Improvements to include-fixer
 -
 


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

[PATCH] D54408: [ASTMatchers] Add matchers available through casting to derived

[Stephen Kelly via Phabricator via cfe-commits]

steveire updated this revision to Diff 179953.
steveire added a comment.

Case


Repository:
  rC Clang

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

https://reviews.llvm.org/D54408

Files:
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/ASTMatchers/Dynamic/RegistryTest.cpp

Index: unittests/ASTMatchers/Dynamic/RegistryTest.cpp
===
--- unittests/ASTMatchers/Dynamic/RegistryTest.cpp
+++ unittests/ASTMatchers/Dynamic/RegistryTest.cpp
@@ -461,6 +461,16 @@
   "Matcher "
   "hasDescendant(Matcher)"));
 
+  CompVector FuncDeclComps = getCompletions("functionDecl", 0);
+
+  EXPECT_TRUE(!FuncDeclComps.empty());
+
+  // Exclude bases
+  EXPECT_TRUE(!hasCompletion(FuncDeclComps, "namedDecl("));
+  EXPECT_TRUE(!hasCompletion(FuncDeclComps, "valueDecl("));
+  EXPECT_TRUE(!hasCompletion(FuncDeclComps, "declaratorDecl("));
+  EXPECT_TRUE(!hasCompletion(FuncDeclComps, "functionDecl("));
+
   CompVector WhileComps = getCompletions("whileStmt", 0);
 
   EXPECT_TRUE(hasCompletion(WhileComps, "hasBody(",
@@ -569,13 +579,13 @@
   EXPECT_TRUE(Contains(Matchers, "isPublic()"));
   EXPECT_TRUE(Contains(Matchers, "hasName()"));
   EXPECT_TRUE(Contains(Matchers, "parameterCountIs()"));
+  EXPECT_TRUE(Contains(Matchers, "cxxMethodDecl(isOverride())"));
 
   EXPECT_FALSE(Contains(Matchers, "namedDecl()"));
   EXPECT_FALSE(Contains(Matchers, "valueDecl()"));
   EXPECT_FALSE(Contains(Matchers, "declaratorDecl()"));
   EXPECT_FALSE(Contains(Matchers, "functionDecl()"));
-
-  EXPECT_TRUE(Contains(Matchers, "cxxMethodDecl()"));
+  EXPECT_FALSE(Contains(Matchers, "cxxMethodDecl()"));
 }
 
 } // end anonymous namespace
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -647,17 +647,77 @@
   }
 }
 
+llvm::Optional>
+getNodeConstructorType(MatcherCtor TargetCtor) {
+  const auto  = RegistryData->nodeConstructors();
+  auto It = llvm::find_if(
+  Ctors, [TargetCtor](const NodeConstructorMap::value_type ) {
+return Ctor.second.second == TargetCtor;
+  });
+  if (It == Ctors.end())
+return llvm::None;
+  return std::make_pair(It->first, It->second.first);
+}
+
 std::vector
-Registry::getMatchingMatchers(ast_type_traits::ASTNodeKind StaticType) {
+getDerivedResults(ast_type_traits::ASTNodeKind StaticType, StringRef Name);
+
+enum MatchScope { ExactOnly, IncludeDerived };
+
+std::vector
+getMatchingMatchersImpl(ast_type_traits::ASTNodeKind StaticType,
+MatchScope Scope) {
+
   std::vector Result;
 
   std::vector AcceptedTypes{StaticType};
 
   processAcceptableMatchers(
-  AcceptedTypes,
-  [](StringRef Name, const MatcherDescriptor &,
-std::set &, std::vector>,
-unsigned) { Result.emplace_back((Name + "()").str()); });
+  AcceptedTypes, [&](StringRef Name, const MatcherDescriptor ,
+ std::set &,
+ std::vector>, unsigned) {
+unsigned Specificity;
+ASTNodeKind LeastDerivedKind;
+if (Scope == ExactOnly &&
+Matcher.isConvertibleTo(StaticType, ,
+) &&
+!LeastDerivedKind.isSame(StaticType))
+  return;
+
+auto TypeForMatcherOpt = getNodeConstructorType();
+if (TypeForMatcherOpt &&
+StaticType.isBaseOf(TypeForMatcherOpt->first)) {
+  auto Derived = getDerivedResults(TypeForMatcherOpt->first, Name);
+  Result.insert(Result.end(), Derived.begin(), Derived.end());
+  return;
+}
+
+Result.emplace_back((Name + "()").str());
+  });
+
+  return Result;
+}
+
+std::vector
+getDerivedResults(ast_type_traits::ASTNodeKind StaticType, StringRef Name) {
+  auto NestedResult = getMatchingMatchersImpl(StaticType, ExactOnly);
+
+  std::vector Result;
+
+  Diagnostics DiagnosticIgnorer;
+
+  for (const auto  : NestedResult) {
+Result.emplace_back((Name + "(" + Item.MatcherString + ")").str());
+  }
+
+  return Result;
+}
+
+std::vector
+Registry::getMatchingMatchers(ast_type_traits::ASTNodeKind StaticType) {
+
+  std::vector Result =
+  getMatchingMatchersImpl(StaticType, IncludeDerived);
 
   return Result;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

In D56215#1344317 , @joerg wrote:

> In D56215#1344279 , @krytarowski 
> wrote:
>
> > In D56215#1344183 , @joerg wrote:
> >
> > > This doesn't seem a reasonable approach at all:
> > >
> > > (1) It breaks cross-linking.
> > >  (2) It is not correct for any target architecture, e.g. /usr/local/lib 
> > > certainly doesn't belong on this list and /lib doesn't either.
> > >  (3) The correct list depends not only on the target architecture, but 
> > > also the active emulation.
> >
> >
> > Is it acceptable to pass all the paths through configure/build phase of 
> > lld? It's done this way in GNU ld in the NetBSD distribution. If we need or 
> > want to hardcode all the specific paths it will be harder to maintain the 
> > proper list and behavior inside lld.
>
>
> I don't think that would be better either. The main point is that it needs a 
> lot more architectural knowledge than shown in the path. I would expect e.g. 
> Linux distros have a similar problem nowadays.


What's the expected solution?

This is a blocker to move on.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56215



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

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

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

In D56215#1344279 , @krytarowski wrote:

> In D56215#1344183 , @joerg wrote:
>
> > This doesn't seem a reasonable approach at all:
> >
> > (1) It breaks cross-linking.
> >  (2) It is not correct for any target architecture, e.g. /usr/local/lib 
> > certainly doesn't belong on this list and /lib doesn't either.
> >  (3) The correct list depends not only on the target architecture, but also 
> > the active emulation.
>
>
> Is it acceptable to pass all the paths through configure/build phase of lld? 
> It's done this way in GNU ld in the NetBSD distribution. If we need or want 
> to hardcode all the specific paths it will be harder to maintain the proper 
> list and behavior inside lld.


I don't think that would be better either. The main point is that it needs a 
lot more architectural knowledge than shown in the path. I would expect e.g. 
Linux distros have a similar problem nowadays.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56215



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

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

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.
arphaman added reviewers: benhamilton, krasimir, djasper.
Herald added subscribers: dexonsmith, jkorous.

The commit r322690 introduced support for ObjC detection in header files. 
Unfortunately some C headers that use designated initializers are now 
incorrectly detected as Objective-C.
This patch fixes it by ensuring `[ ... ]` is not annotated as an Objective-C 
message send when the expression is followed by `=` as it's not a commonly used 
Objective-C pattern.

rdar://45504376


Repository:
  rC Clang

https://reviews.llvm.org/D56226

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


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


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

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

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

In D56215#1344233 , @ruiu wrote:

> lld's driver is intentionally designed to be agnostic of the host that the 
> linker is running for the reasons described at the beginning of Driver.cpp: 
> https://github.com/llvm-project/lld/blob/master/ELF/Driver.cpp#L13 I think I 
> like that approach. If you need to do this, you can do this in the compiler 
> driver rather than in the linker itself. Is there any reason you need to do 
> this in the linker?


This breaks compat with GNU ld here and the linker is intended to be used 
standalone.

In D56215#1344183 , @joerg wrote:

> This doesn't seem a reasonable approach at all:
>
> (1) It breaks cross-linking.
>  (2) It is not correct for any target architecture, e.g. /usr/local/lib 
> certainly doesn't belong on this list and /lib doesn't either.
>  (3) The correct list depends not only on the target architecture, but also 
> the active emulation.


Is it acceptable to pass all the paths through configure/build phase of lld? 
It's done this way in GNU ld in the NetBSD distribution. If we need or want to 
hardcode all the specific paths it will be harder to maintain the proper list 
and behavior inside lld.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56215



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

[PATCH] D56160: [clang-tidy] modernize-use-trailing-return check

[Bernhard Manfred Gruber via Phabricator via cfe-commits]

bernhardmgruber marked 10 inline comments as done.
bernhardmgruber added a comment.

I fixed most of the stylistic issues. Regarding the missing test cases, I will 
add those and do the necessary code changes. Thank you very much for pointing 
them out to me!




Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:21-22
+
+// very similar to UseOverrideCheck
+SourceLocation findTrailingReturnTypeLocation(const CharSourceRange ,
+  const ASTContext ,

lebedev.ri wrote:
> This function looks quite fragile.
> Is there no existing function elsewhere [in the support libraries] that does 
> this?
> There is no way to extract this from AST?
> 
I have not found one by browsing the doxygen documentation. I got inspired by 
ParseTokens in UseOverrideCheck.cpp. But I am an absolute beginner on the LLVM 
codebase, so please point me in a direction and I try to find something better!

There is also this in the documentation of [[ 
https://clang.llvm.org/doxygen/classclang_1_1FunctionDecl.html#a1ac2b87b937cc44d8980a898851c0c75
 | FunctionDecl::getReturnTypeSourceRange()]]: "This may omit qualifiers and 
other information with limited representation in the AST."
So maybe the AST really does not contain the locations of const/volatile and 
const/ref qualifiers



Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:91
+  Token t;
+  std::vector tokens;
+  while (!lexer.LexFromRawLexer(t)) {

lebedev.ri wrote:
> Wouldn't `SmallVector` be sufficient in most cases?
I believe return types with namespaces or template arguments produce more 
tokens, so I will go with SmallVector. But i did not do any 
measurements.



Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:161-163
+  const auto  = *Result.Context;
+  const auto  = *Result.SourceManager;
+  const auto  = getLangOpts();

lebedev.ri wrote:
> 1. All the variables should be WithThisCase
> 2. Can't tell if the `auto` is ok here? 
> 
1. done
2. Is it important to know what types Result.Context and the others are? I am 
just passing them on to further functions because I have to, they are not 
relevant for the logic I am coding.


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

https://reviews.llvm.org/D56160



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

[PATCH] D56160: [clang-tidy] modernize-use-trailing-return check

[Bernhard Manfred Gruber via Phabricator via cfe-commits]

bernhardmgruber updated this revision to Diff 179945.
bernhardmgruber marked 2 inline comments as done.

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

https://reviews.llvm.org/D56160

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/UseTrailingReturnCheck.cpp
  clang-tidy/modernize/UseTrailingReturnCheck.h
  docs/clang-tidy/checks/modernize-use-trailing-return.rst
  test/clang-tidy/modernize-use-trailing-return.cpp

Index: test/clang-tidy/modernize-use-trailing-return.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-trailing-return.cpp
@@ -0,0 +1,144 @@
+// RUN: %check_clang_tidy %s modernize-use-trailing-return %t -- -- --std=c++14
+
+namespace std {
+template 
+class vector;
+
+class string;
+}
+
+//
+// Samples triggering the check
+//
+
+int f();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto f() -> int;{{$}}
+int f(int);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto f(int) -> int;{{$}}
+int f(int arg);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto f(int arg) -> int;{{$}}
+int f(int arg1, int arg2, int arg3);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto f(int arg1, int arg2, int arg3) -> int;{{$}}
+int f(int arg1, int arg2, int arg3, ...);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto f(int arg1, int arg2, int arg3, ...) -> int;{{$}}
+template  int f(T t);
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}template  auto f(T t) -> int;{{$}}
+int a1() { return 42; }
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto a1() -> int { return 42; }{{$}}
+int a2() {
+return 42;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto a2() -> int {{{$}}
+int a3()
+{
+return 42;
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto a3() -> int{{$}}
+int b(int   arg   )   ;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto b(int   arg   ) -> int   ;{{$}}
+inline int d1(int arg);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}inline auto d1(int arg) -> int;{{$}}
+inline int d2(int arg) noexcept(true);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}inline auto d2(int arg) -> int noexcept(true);{{$}}
+inline int d3(int arg) try { } catch(...) { }
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}inline auto d3(int arg) -> int try { } catch(...) { }{{$}}
+namespace N {
+bool e1();
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:10: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto e1() -> bool;{{$}}
+inline volatile const std::vector e2();
+// CHECK-MESSAGES: :[[@LINE-1]]:48: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}inline auto e2() -> volatile const std::vector;{{$}}
+inline const std::vector volatile e2();
+// CHECK-MESSAGES: :[[@LINE-1]]:48: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}inline auto e2() -> const std::vector volatile;{{$}}
+inline std::vector const volatile e2();
+// CHECK-MESSAGES: :[[@LINE-1]]:48: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}inline auto e2() -> std::vector const volatile;{{$}}
+bool N::e1() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use a trailing return type for this function [modernize-use-trailing-return]
+// CHECK-FIXES: {{^}}auto N::e1() -> bool {}{{$}}
+int (*e3())(double);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use a trailing return type for this function (FixIt not implemented) [modernize-use-trailing-return]
+// TODO: not matched by the AST matcher
+//decltype(auto) e4();

[PATCH] D56225: [HIP] Use nul instead of /dev/null when running on windows

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added a reviewer: tra.

When clang is running on windows, /dev/null is not available. Use nul as empty 
input file instead.


https://reviews.llvm.org/D56225

Files:
  lib/Driver/ToolChains/HIP.cpp


Index: lib/Driver/ToolChains/HIP.cpp
===
--- lib/Driver/ToolChains/HIP.cpp
+++ lib/Driver/ToolChains/HIP.cpp
@@ -24,6 +24,12 @@
 using namespace clang;
 using namespace llvm::opt;
 
+#if _WIN32 || _WIN64
+#define NULL_FILE "nul"
+#else
+#define NULL_FILE "/dev/null"
+#endif
+
 namespace {
 
 static void addBCLib(Compilation , const ArgList ,
@@ -201,7 +207,7 @@
   // ToDo: Remove the dummy host binary entry which is required by
   // clang-offload-bundler.
   std::string BundlerTargetArg = "-targets=host-x86_64-unknown-linux";
-  std::string BundlerInputArg = "-inputs=/dev/null";
+  std::string BundlerInputArg = "-inputs=" NULL_FILE;
 
   for (const auto  : Inputs) {
 const auto* A = II.getAction();


Index: lib/Driver/ToolChains/HIP.cpp
===
--- lib/Driver/ToolChains/HIP.cpp
+++ lib/Driver/ToolChains/HIP.cpp
@@ -24,6 +24,12 @@
 using namespace clang;
 using namespace llvm::opt;
 
+#if _WIN32 || _WIN64
+#define NULL_FILE "nul"
+#else
+#define NULL_FILE "/dev/null"
+#endif
+
 namespace {
 
 static void addBCLib(Compilation , const ArgList ,
@@ -201,7 +207,7 @@
   // ToDo: Remove the dummy host binary entry which is required by
   // clang-offload-bundler.
   std::string BundlerTargetArg = "-targets=host-x86_64-unknown-linux";
-  std::string BundlerInputArg = "-inputs=/dev/null";
+  std::string BundlerInputArg = "-inputs=" NULL_FILE;
 
   for (const auto  : Inputs) {
 const auto* A = II.getAction();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Rui Ueyama via Phabricator via cfe-commits]

ruiu added a comment.

lld's driver is intentionally designed to be agnostic of the host that the 
linker is running for the reasons described at the beginning of Driver.cpp: 
https://github.com/llvm-project/lld/blob/master/ELF/Driver.cpp#L13 I think I 
like that approach. If you need to do this, you can do this in the compiler 
driver rather than in the linker itself. Is there any reason you need to do 
this in the linker?


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56215



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

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

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

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

In D56113#1342879 , @ABataev wrote:

> But you will need another serie of patches for `reduction` and `linear` 
> clauses to update their error messages.


Those already had their own checks for const.  I'm not aware of any cases not 
handled, and the test suite does pass after my patch.

By the way, is there any value to keeping the predetermined shared for const if 
-openmp-version=3.1 or earlier?

>> Sorry, I misunderstood your first response.  When I try an atomic write to 
>> the shared variable, the difference between the uncombined and combined 
>> target teams directive affects functionality when targeting multicore.  Does 
>> that matter?
> 
> Yes, I'm aware if this problem. But I don't think it is very important and 
> can cause serious problems in user code. If you think that it is worth to fix 
> this prblem, you can go ahead and fix it.

No, not worthwhile for me right now.  I just wanted to point it out in passing 
in case it might matter to someone.  Does a bugzilla seem worthwhile to you?


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

https://reviews.llvm.org/D56113



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

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

[Alexey Lapshin via Phabricator via cfe-commits]

alexey.lapshin updated this revision to Diff 179933.
alexey.lapshin added a comment.

Thank you. I updated doc, splitted Parser test, added Sema tests. Please check 
it once more.


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

https://reviews.llvm.org/D55710

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticParseKinds.td
  lib/CodeGen/CGLoopInfo.cpp
  lib/CodeGen/CGLoopInfo.h
  lib/Parse/ParsePragma.cpp
  lib/Sema/SemaStmtAttr.cpp
  test/CodeGenCXX/pragma-pipeline.cpp
  test/Parser/pragma-loop.cpp
  test/Parser/pragma-pipeline.cpp
  test/Parser/pragma-unroll-and-jam.cpp
  test/Sema/pragma-pipeline.cpp

Index: test/Sema/pragma-pipeline.cpp
===
--- /dev/null
+++ test/Sema/pragma-pipeline.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+
+#pragma clang loop pipeline(disable) /* expected-error {{expected unqualified-id}} */
+int main() {
+  for (int i = 0; i < 10; ++i)
+;
+}
+
+void test(int *List, int Length, int Value) {
+  int i = 0;
+
+/* expected-error {{invalid argument of type 'double'; expected an integer type}} */ #pragma clang loop pipeline_initiation_interval(1.0)
+/* expected-error {{invalid value '0'; must be positive}} */ #pragma clang loop pipeline_initiation_interval(0)
+/* expected-error {{invalid value '-1'; must be positive}} */ #pragma clang loop pipeline_initiation_interval(-1)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma clang loop pipeline(disable) 
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma clang loop'}} */ int j = Length;
+#pragma clang loop pipeline_initiation_interval(4)
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma clang loop'}} */ int k = Length;
+
+#pragma clang loop pipeline(disable)
+#pragma clang loop pipeline_initiation_interval(4) /* expected-error {{incompatible directives 'pipeline(disable)' and 'pipeline_initiation_interval(4)'}} */
+  for (int i = 0; i < Length; i++) {
+List[i] = Value;
+  }
+
+#pragma clang loop pipeline(disable)
+/* expected-error {{expected statement}} */ }
+
Index: test/Parser/pragma-unroll-and-jam.cpp
===
--- test/Parser/pragma-unroll-and-jam.cpp
+++ test/Parser/pragma-unroll-and-jam.cpp
@@ -67,7 +67,7 @@
   }
 
 // pragma clang unroll_and_jam is disabled for the moment
-/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, or distribute}} */ #pragma clang loop unroll_and_jam(4)
+/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval, or distribute}} */ #pragma clang loop unroll_and_jam(4)
   for (int i = 0; i < Length; i++) {
 for (int j = 0; j < Length; j++) {
   List[i * Length + j] = Value;
Index: test/Parser/pragma-pipeline.cpp
===
--- /dev/null
+++ test/Parser/pragma-pipeline.cpp
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+
+// Note that this puts the expected lines before the directives to work around
+// limitations in the -verify mode.
+
+void test(int *List, int Length, int Value) {
+  int i = 0;
+
+#pragma clang loop pipeline(disable)
+  for (int i = 0; i < Length; i++) {
+List[i] = Value;
+  }
+
+#pragma clang loop pipeline_initiation_interval(10)
+  for (int i = 0; i < Length; i++) {
+List[i] = Value;
+  }
+
+/* expected-error {{expected ')'}} */ #pragma clang loop pipeline(disable
+/* expected-error {{invalid argument; expected 'disable'}} */ #pragma clang loop pipeline(enable)
+/* expected-error {{invalid argument; expected 'disable'}} */ #pragma clang loop pipeline(error)
+/* expected-error {{expected '('}} */ #pragma clang loop pipeline disable
+/* expected-error {{missing argument; expected an integer value}} */ #pragma clang loop pipeline_initiation_interval()
+/* expected-error {{use of undeclared identifier 'error'}} */ #pragma clang loop pipeline_initiation_interval(error)
+/* expected-error {{expected '('}} */ #pragma clang loop pipeline_initiation_interval 1 2
+/* expected-error {{expected ')'}} */ #pragma clang loop pipeline_initiation_interval(1
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+}
Index: test/Parser/pragma-loop.cpp
===
--- test/Parser/pragma-loop.cpp
+++ test/Parser/pragma-loop.cpp
@@ -147,7 +147,7 @@
 /* expected-error {{missing argument; expected 'enable', 'full' or 'disable'}} */ #pragma clang loop unroll()
 /* expected-error {{missing argument; expected 'enable' 

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

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

This doesn't seem a reasonable approach at all:

(1) It breaks cross-linking.
(2) It is not correct for any target architecture, e.g. /usr/local/lib 
certainly doesn't belong on this list and /lib doesn't either.
(3) The correct list depends not only on the target architecture, but also the 
active emulation.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56215



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

[PATCH] D56067: Make test/Driver/darwin-sdk-version.c pass if the host triple is 32-bit

[Alex Lorenz via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D56067



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

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

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

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

The NetBSD clang driver relies on NetBSD ld knowing the default search
paths and never passes those paths explicitly.  As a result of this
design requirement, implement appending default search paths within lld
as well.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D56215

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


Index: ELF/Driver.h
===
--- ELF/Driver.h
+++ ELF/Driver.h
@@ -32,6 +32,7 @@
 
 private:
   void readConfigs(llvm::opt::InputArgList );
+  void appendDefaultSearchPaths();
   void createFiles(llvm::opt::InputArgList );
   void inferMachineType();
   template  void link(llvm::opt::InputArgList );
Index: ELF/Driver.cpp
===
--- ELF/Driver.cpp
+++ ELF/Driver.cpp
@@ -365,6 +365,15 @@
   error("unknown -z value: " + StringRef(Arg->getValue()));
 }
 
+void LinkerDriver::appendDefaultSearchPaths() {
+#if defined(__NetBSD__)
+  // NetBSD driver relies on the linker knowing the default search paths.
+  Config->SearchPaths.push_back("/usr/local/lib");
+  Config->SearchPaths.push_back("/lib");
+  Config->SearchPaths.push_back("/usr/lib");
+#endif
+}
+
 void LinkerDriver::main(ArrayRef ArgsArr) {
   ELFOptTable Parser;
   opt::InputArgList Args = Parser.parse(ArgsArr.slice(1));
@@ -412,6 +421,7 @@
 
   readConfigs(Args);
   checkZOptions(Args);
+  appendDefaultSearchPaths();
 
   // The behavior of -v or --version is a bit strange, but this is
   // needed for compatibility with GNU linkers.


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

[PATCH] D55949: Correctly handle function pointers returning a type marked nodiscard

[Erik Pilkington via Phabricator via cfe-commits]

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

Still LG, sorry for the delay!


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

https://reviews.llvm.org/D55949



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

[PATCH] D55949: Correctly handle function pointers returning a type marked nodiscard

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Ping


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

https://reviews.llvm.org/D55949



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

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman marked 2 inline comments as done.
aaron.ballman added inline comments.



Comment at: include/clang/Sema/Sema.h:5337
   ExprResult ActOnFinishFullExpr(Expr *Expr, SourceLocation CC,
- bool DiscardedValue = false,
+ bool WarnOnDiscardedValue = false,
  bool IsConstexpr = false);

rsmith wrote:
> Quuxplusone wrote:
> > aaron.ballman wrote:
> > > rsmith wrote:
> > > > Why "WarnOn"? Shouldn't this flag simply indicate whether the 
> > > > expression is a discarded-value expression?
> > > It probably can; but then it feels like the logic is backwards from the 
> > > suggested changes as I understood them. If it's a discarded value 
> > > expression, then the value being unused should *not* be diagnosed because 
> > > the expression only exists for its side effects (not its value 
> > > computations), correct?
> > Peanut gallery says: There are at least three things that need to be 
> > computed somewhere: (1) Is this expression's value discarded? (2) Is this 
> > expression the result of a `[[nodiscard]]` function? (3) Is the diagnostic 
> > enabled? It is unclear to me who's responsible for computing which of these 
> > things. I.e., it is unclear to me whether `WarnOnDiscardedValue=true` means 
> > "Hey `ActOnFinishFullExpr`, please give a warning //because// this value is 
> > being discarded" (conjunction of 1,2, and maybe 3) or "Hey 
> > `ActOnFinishFullExpr`, please give a warning //if// this value is being 
> > discarded" (conjunction of 2 and maybe 3).
> > 
> > I also think it is needlessly confusing that `ActOnFinishFullExpr` gives 
> > `WarnOnDiscardedValue` a defaulted value of `false` but `ActOnExprStmt` 
> > gives `WarnOnDiscardedValue` a defaulted value of `true`. Defaulted values 
> > (especially of boolean type) are horrible, but context-dependent defaulted 
> > values are even worse.
> I don't think it makes sense for `ActOnFinishFullExpr` to have a default 
> argument for `DiscardedValue`, because there's really no reason to assume one 
> way or the other -- the values of some full-expressions are used, and the 
> values of others are not. A default of `false` certainly seems wrong.
> 
> For `ActOnExprStmt`, the default argument makes sense to me: the expression 
> in an expression-statement is by definition a discarded-value expression 
> (http://eel.is/c++draft/stmt.stmt#stmt.expr-1.sentence-2) -- it's only the 
> weird special case for a final expression-statement in an 
> statement-expression that bucks the trend here.
> 
> > If it's a discarded value expression, then the value being unused should 
> > *not* be diagnosed because the expression only exists for its side effects 
> > (not its value computations), correct?
> 
> No. If it's a discarded-value expression, that means the value of the 
> full-expression is not being used, so it should be diagnosed. If it's not a 
> discarded-value expression, then the value of the full-expression is used for 
> something (eg, it's a condition or an array bound or a template argument) and 
> so we should not warn. Indeed, the wording for `[[nodiscard]]` suggests to 
> warn (only) on potentially-evaluated discarded-value expressions.
> 
> Discarded-value expressions are things like expression-statements, the 
> left-hand-side of a comma operator, and the operands of casts to void. (Note 
> in the cast-to-void case is explicitly called out by the `[[nodiscard]]` 
> wording as a discarded-value expression that should not warn: 
> http://eel.is/c++draft/dcl.attr.nodiscard#2.sentence-2)
Thank you for the explanation @rsmith, my mental model was exactly backwards. 
:-D I was thrown off by 
http://eel.is/c++draft/expr.prop#expr.context-2.sentence-1 because I read it as 
saying the discarded nature of the results are *desired* rather than 
problematic. e.g., some statements only appear for their side effects, do not 
warn about such statements because the context makes it obvious that this is 
expected. I should have looked it it in the context of the nodiscard attribute 
wording rather than in isolation.


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

https://reviews.llvm.org/D55955



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

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 179920.
aaron.ballman marked an inline comment as done.
aaron.ballman added a comment.

Updated based on review feedback. The vast majority of the changes are from the 
removal of a default argument to `ActOnFullExpr()`.


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

https://reviews.llvm.org/D55955

Files:
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseObjc.cpp
  lib/Parse/ParseOpenMP.cpp
  lib/Parse/ParseStmt.cpp
  lib/Sema/SemaCoroutine.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprCXX.cpp
  lib/Sema/SemaLambda.cpp
  lib/Sema/SemaOpenMP.cpp
  lib/Sema/SemaStmt.cpp
  lib/Sema/TreeTransform.h
  test/CXX/stmt.stmt/stmt.select/p3.cpp
  test/CodeCompletion/pragma-macro-token-caching.c
  test/Parser/cxx1z-init-statement.cpp
  test/Parser/switch-recovery.cpp
  test/SemaCXX/cxx1z-init-statement.cpp
  test/SemaCXX/for-range-examples.cpp
  test/SemaCXX/warn-unused-result.cpp
  test/SemaObjCXX/foreach.mm

Index: test/SemaObjCXX/foreach.mm
===
--- test/SemaObjCXX/foreach.mm
+++ test/SemaObjCXX/foreach.mm
@@ -6,8 +6,8 @@
 void f(NSArray *a) {
 id keys;
 for (int i : a); // expected-error{{selector element type 'int' is not a valid object}} 
-for ((id)2 : a);  // expected-error {{for range declaration must declare a variable}}
-for (2 : a); // expected-error {{for range declaration must declare a variable}}
+for ((id)2 : a);  // expected-error {{for range declaration must declare a variable}} expected-warning {{expression result unused}}
+for (2 : a); // expected-error {{for range declaration must declare a variable}} expected-warning {{expression result unused}}
   
   for (id thisKey : keys);
 
@@ -71,7 +71,7 @@
 @end
 void test2(NSObject *collection) {
   Test2 *obj;
-  for (obj.prop : collection) { // expected-error {{for range declaration must declare a variable}}
+  for (obj.prop : collection) { // expected-error {{for range declaration must declare a variable}} expected-warning {{property access result unused}}
   }
 }
 
Index: test/SemaCXX/warn-unused-result.cpp
===
--- test/SemaCXX/warn-unused-result.cpp
+++ test/SemaCXX/warn-unused-result.cpp
@@ -33,6 +33,36 @@
   const S  = g1();
 }
 
+void testSubstmts(int i) {
+  switch (i) {
+  case 0:
+f(); // expected-warning {{ignoring return value}}
+  default:
+f(); // expected-warning {{ignoring return value}}
+  }
+
+  if (i)
+f(); // expected-warning {{ignoring return value}}
+  else
+f(); // expected-warning {{ignoring return value}}
+
+  while (i)
+f(); // expected-warning {{ignoring return value}}
+
+  do
+f(); // expected-warning {{ignoring return value}}
+  while (i);
+
+  for (f(); // expected-warning {{ignoring return value}}
+   ;
+   f() // expected-warning {{ignoring return value}}
+  )
+f(); // expected-warning {{ignoring return value}}
+
+  f(),  // expected-warning {{ignoring return value}}
+  (void)f();
+}
+
 struct X {
  int foo() __attribute__((warn_unused_result));
 };
@@ -206,3 +236,13 @@
   (void)++p;
 }
 } // namespace
+
+namespace PR39837 {
+[[clang::warn_unused_result]] int f(int);
+
+void g() {
+  int a[2];
+  for (int b : a)
+f(b); // expected-warning {{ignoring return value}}
+}
+} // namespace PR39837
Index: test/SemaCXX/for-range-examples.cpp
===
--- test/SemaCXX/for-range-examples.cpp
+++ test/SemaCXX/for-range-examples.cpp
@@ -176,9 +176,9 @@
 
 // Make sure these don't crash. Better diagnostics would be nice.
 for (: {1, 2, 3}) {} // expected-error {{expected expression}} expected-error {{expected ';'}}
-for (1 : {1, 2, 3}) {} // expected-error {{must declare a variable}}
+for (1 : {1, 2, 3}) {} // expected-error {{must declare a variable}} expected-warning {{expression result unused}}
 for (+x : {1, 2, 3}) {} // expected-error {{undeclared identifier}} expected-error {{expected ';'}}
-for (+y : {1, 2, 3}) {} // expected-error {{must declare a variable}}
+for (+y : {1, 2, 3}) {} // expected-error {{must declare a variable}} expected-warning {{expression result unused}}
   }
 }
 
@@ -244,7 +244,7 @@
 { 
   int b = 1, a[b];
   a[0] = 0;
-  [&] { for (int c : a) 0; } ();
+  [&] { for (int c : a) 0; } (); // expected-warning {{expression result unused}}
 }
 
 
Index: test/SemaCXX/cxx1z-init-statement.cpp
===
--- test/SemaCXX/cxx1z-init-statement.cpp
+++ test/SemaCXX/cxx1z-init-statement.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -std=c++1z -verify %s
-// RUN: %clang_cc1 -std=c++17 -verify %s
+// RUN: %clang_cc1 -std=c++1z -Wno-unused-value -verify %s
+// RUN: %clang_cc1 -std=c++17 -Wno-unused-value -verify %s
 
 void testIf() {
   int x = 0;
@@ -12,7 

r350252 - [OpenMP] Added support for explicit mapping of classes using 'this' pointer. Differential revision: https://reviews.llvm.org/D55982

[Patrick Lyster via cfe-commits]

Author: plyster
Date: Wed Jan  2 11:28:48 2019
New Revision: 350252

URL: http://llvm.org/viewvc/llvm-project?rev=350252=rev
Log:
[OpenMP] Added support for explicit mapping of classes using 'this' pointer. 
Differential revision: https://reviews.llvm.org/D55982

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/target_ast_print.cpp
cfe/trunk/test/OpenMP/target_codegen.cpp
cfe/trunk/test/OpenMP/target_messages.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=350252=350251=350252=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Jan  2 11:28:48 
2019
@@ -9076,6 +9076,14 @@ def note_omp_requires_previous_clause :
   "%0 clause previously used here">;
 def err_omp_invalid_scope : Error <
   "'#pragma omp %0' directive must appear only in file scope">;
+def note_omp_invalid_length_on_this_ptr_mapping : Note <
+  "expected length on mapping of 'this' array section expression to be '1'">;
+def note_omp_invalid_lower_bound_on_this_ptr_mapping : Note <
+  "expected lower bound on mapping of 'this' array section expression to be 
'0' or not specified">;
+def note_omp_invalid_subscript_on_this_ptr_map : Note <
+  "expected 'this' subscript expression on map clause to be 'this[0]'">;
+def err_omp_invalid_map_this_expr : Error <
+  "invalid 'this' expression on 'map' clause">;
 } // end of OpenMP category
 
 let CategoryName = "Related Result Type Issue" in {

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=350252=350251=350252=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Wed Jan  2 11:28:48 2019
@@ -6992,15 +6992,22 @@ private:
 // components.
 bool IsExpressionFirstInfo = true;
 Address BP = Address::invalid();
+const Expr *AssocExpr = I->getAssociatedExpression();
+const auto *AE = dyn_cast(AssocExpr);
+const auto *OASE = dyn_cast(AssocExpr);
 
-if (isa(I->getAssociatedExpression())) {
+if (isa(AssocExpr)) {
   // The base is the 'this' pointer. The content of the pointer is going
   // to be the base of the field being mapped.
   BP = CGF.LoadCXXThisAddress();
+} else if ((AE && isa(AE->getBase()->IgnoreParenImpCasts())) 
||
+   (OASE &&
+isa(OASE->getBase()->IgnoreParenImpCasts( {
+  BP = CGF.EmitOMPSharedLValue(AssocExpr).getAddress();
 } else {
   // The base is the reference to the variable.
   // BP = 
-  BP = CGF.EmitOMPSharedLValue(I->getAssociatedExpression()).getAddress();
+  BP = CGF.EmitOMPSharedLValue(AssocExpr).getAddress();
   if (const auto *VD =
   dyn_cast_or_null(I->getAssociatedDeclaration())) {
 if (llvm::Optional Res =

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=350252=350251=350252=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Wed Jan  2 11:28:48 2019
@@ -22,6 +22,7 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtOpenMP.h"
 #include "clang/AST/StmtVisitor.h"
+#include "clang/AST/TypeOrdering.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
@@ -146,6 +147,7 @@ private:
 SourceLocation InnerTeamsRegionLoc;
 /// Reference to the taskgroup task_reduction reference expression.
 Expr *TaskgroupReductionRef = nullptr;
+llvm::DenseSet MappedClassesQualTypes;
 SharingMapTy(OpenMPDirectiveKind DKind, DeclarationNameInfo Name,
  Scope *CurScope, SourceLocation Loc)
 : Directive(DKind), DirectiveName(Name), CurScope(CurScope),
@@ -660,6 +662,19 @@ public:
 return llvm::make_range(StackElem.DoacrossDepends.end(),
 StackElem.DoacrossDepends.end());
   }
+
+  // Store types of classes which have been explicitly mapped
+  void addMappedClassesQualTypes(QualType QT) {
+SharingMapTy  = Stack.back().first.back();
+StackElem.MappedClassesQualTypes.insert(QT);
+  }
+
+  // Return set of mapped classes types
+  bool isClassPreviouslyMapped(QualType QT) const {
+const SharingMapTy  = Stack.back().first.back();
+return StackElem.MappedClassesQualTypes.count(QT) != 0;
+  }
+
 };
 bool isParallelOrTaskRegion(OpenMPDirectiveKind DKind) {
   return isOpenMPParallelDirective(DKind) || 

[PATCH] D56198: [Basic] Extend DiagnosticEngine to store and format Qualifiers

[John McCall via Phabricator via cfe-commits]

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

Thanks, LGTM.


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

https://reviews.llvm.org/D56198



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

[PATCH] D55948: Modify DeclaratorChuck::getFunction to use DeclSpec for qualifiers

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/DeclSpec.cpp:220
+
+  I.Fun.QualAttrFactory = nullptr;
+

Anastasia wrote:
> rjmccall wrote:
> > Spacing?
> I am keeping  the old formatting from the lines 195-204. But obviously it 
> doesn't match with the current clang-format rules. Do you prefer that I 
> reformat the whole block or just this line?
Oh, I see.  Could you hoist the null-initialization of these two fields above 
this block, then?  They'll look more consistent, and it'll make it clearer that 
you're establishing the basic invariants on these fields before modifying them.



Comment at: lib/Sema/SemaDeclCXX.cpp:8175
   DeclaratorChunk::FunctionTypeInfo  = D.getFunctionTypeInfo();
-  if (FTI.TypeQuals != 0) {
-if (FTI.TypeQuals & Qualifiers::Const)
-  Diag(D.getIdentifierLoc(), diag::err_invalid_qualified_constructor)
-<< "const" << SourceRange(D.getIdentifierLoc());
-if (FTI.TypeQuals & Qualifiers::Volatile)
-  Diag(D.getIdentifierLoc(), diag::err_invalid_qualified_constructor)
-<< "volatile" << SourceRange(D.getIdentifierLoc());
-if (FTI.TypeQuals & Qualifiers::Restrict)
-  Diag(D.getIdentifierLoc(), diag::err_invalid_qualified_constructor)
-<< "restrict" << SourceRange(D.getIdentifierLoc());
+  if (FTI.MethodQualifiers && FTI.MethodQualifiers->getTypeQualifiers() != 0) {
+auto DiagQual = [&](DeclSpec::TQ TypeQual, StringRef QualName,

Anastasia wrote:
> rjmccall wrote:
> > I think you should add a `hasMethodQualifiers` method to FTI that does this 
> > check.  Note that it needs to check for attributes, too, and I think you 
> > need to figure out some way to generalize `forEachCVRUQual` to cover those.
> Are there any attributes I should handle currently?
> 
> Also are you suggesting to add another `forEach...` method or extend 
> existing? If the latter, I might not be able to use it in all places I use it 
> now.
Adding another method might be easier.  How many clients actually use the TQ?


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

https://reviews.llvm.org/D55948



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

[PATCH] D56066: [OpenCL] Address space for default class members

[Anastasia Stulova via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGCall.cpp:78
+RecTy = Context.getAddrSpaceQualType(
+RecTy, MD->getTypeQualifiers().getAddressSpace());
   return Context.getPointerType(CanQualType::CreateUnsafe(RecTy));

rjmccall wrote:
> Would you mind making a patch to rename this (and the method on 
> `FunctionProtoType`) to something like `getMethodQualifiers`?  It can be a 
> follow-up instead of blocking this.
Sure. I will prepare a separate patch for this.


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

https://reviews.llvm.org/D56066



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

[PATCH] D56066: [OpenCL] Address space for default class members

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 179899.
Anastasia added a comment.

Addressed review comments.


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

https://reviews.llvm.org/D56066

Files:
  include/clang/Sema/Sema.h
  lib/CodeGen/CGCall.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaInit.cpp
  lib/Sema/SemaType.cpp
  test/CodeGenOpenCLCXX/addrspace-of-this.cl
  test/SemaOpenCLCXX/address-space-templates.cl

Index: test/SemaOpenCLCXX/address-space-templates.cl
===
--- test/SemaOpenCLCXX/address-space-templates.cl
+++ test/SemaOpenCLCXX/address-space-templates.cl
@@ -5,7 +5,7 @@
   T a;// expected-error{{field may not be qualified with an address space}}
   T f1(); // expected-error{{function type may not be qualified with an address space}}
   // FIXME: Should only get the error message once.
-  void f2(T); // expected-error{{parameter may not be qualified with an address space}} expected-error{{parameter may not be qualified with an address space}}
+  void f2(T); // expected-error{{parameter may not be qualified with an address space}}
 
 };
 
Index: test/CodeGenOpenCLCXX/addrspace-of-this.cl
===
--- test/CodeGenOpenCLCXX/addrspace-of-this.cl
+++ test/CodeGenOpenCLCXX/addrspace-of-this.cl
@@ -9,18 +9,21 @@
 public:
   int v;
   C() { v = 2; }
-  // FIXME: Does not work yet.
-  // C(C &) { v = c.v; }
+  C(C &) { v = c.v; }
   C(const C ) { v = c.v; }
   C =(const C ) {
 v = c.v;
 return *this;
   }
-  // FIXME: Does not work yet.
-  //C =(C&& c) & {
-  //  v = c.v;
-  //  return *this;
-  //}
+  C =(C &) & {
+v = c.v;
+return *this;
+  }
+
+  C operator+(const C& c) {
+v += c.v;
+return *this;
+  }
 
   int get() { return v; }
 
@@ -41,15 +44,13 @@
   C c1(c);
   C c2;
   c2 = c1;
-  // FIXME: Does not work yet.
-  // C c3 = c1 + c2;
-  // C c4(foo());
-  // C c5 = foo();
-
+  C c3 = c1 + c2;
+  C c4(foo());
+  C c5 = foo();
 }
 
 // CHECK-LABEL: @__cxx_global_var_init()
-// CHECK: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast (%class.C addrspace(1)* @c to %class.C addrspace(4)*)) #4
+// CHECK: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast (%class.C addrspace(1)* @c to %class.C addrspace(4)*))
 
 // Test that the address space is __generic for the constructor
 // CHECK-LABEL: @_ZNU3AS41CC1Ev(%class.C addrspace(4)* %this)
@@ -57,7 +58,7 @@
 // CHECK:   %this.addr = alloca %class.C addrspace(4)*, align 4
 // CHECK:   store %class.C addrspace(4)* %this, %class.C addrspace(4)** %this.addr, align 4
 // CHECK:   %this1 = load %class.C addrspace(4)*, %class.C addrspace(4)** %this.addr, align 4
-// CHECK:   call void @_ZNU3AS41CC2Ev(%class.C addrspace(4)* %this1) #4
+// CHECK:   call void @_ZNU3AS41CC2Ev(%class.C addrspace(4)* %this1)
 // CHECK:   ret void
 
 // CHECK-LABEL: @_Z12test__globalv()
@@ -74,13 +75,29 @@
 
 // Test the address space of 'this' when invoking a constructor.
 // CHECK:   %1 = addrspacecast %class.C* %c2 to %class.C addrspace(4)*
-// CHECK:   call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* %1) #4
+// CHECK:   call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* %1)
 
 // Test the address space of 'this' when invoking assignment operator.
 // CHECK:   %2 = addrspacecast %class.C* %c1 to %class.C addrspace(4)*
 // CHECK:   %3 = addrspacecast %class.C* %c2 to %class.C addrspace(4)*
 // CHECK:   %call2 = call dereferenceable(4) %class.C addrspace(4)* @_ZNU3AS41CaSERU3AS4KS_(%class.C addrspace(4)* %3, %class.C addrspace(4)* dereferenceable(4) %2)
 
+// Test the address space of 'this' when invoking the operator+
+// CHECK:   %4 = addrspacecast %class.C* %c1 to %class.C addrspace(4)*
+// CHECK:   %5 = addrspacecast %class.C* %c2 to %class.C addrspace(4)*
+// CHECK:   call void @_ZNU3AS41CplERU3AS4KS_(%class.C* sret %c3, %class.C addrspace(4)* %4, %class.C addrspace(4)* dereferenceable(4) %5)
+
+// Test the address space of 'this' when invoking the move constructor
+// CHECK:   %6 = addrspacecast %class.C* %c4 to %class.C addrspace(4)*
+// CHECK:   %call3 = call spir_func dereferenceable(4) %class.C addrspace(4)* @_Z3foov()
+// CHECK:   call void @_ZNU3AS41CC1EOU3AS4S_(%class.C addrspace(4)* %6, %class.C addrspace(4)* dereferenceable(4) %call3)
+
+// Test the address space of 'this' when invoking the move assignment
+// CHECK:   %7 = addrspacecast %class.C* %c5 to %class.C addrspace(4)*
+// CHECK:   %call4 = call spir_func dereferenceable(4) %class.C addrspace(4)* @_Z3foov() #5
+// CHECK:   call void @_ZNU3AS41CC1EOU3AS4S_(%class.C addrspace(4)* %7, %class.C addrspace(4)* dereferenceable(4) %call4)
+
+
 #define TEST(AS) \
   __kernel void test##AS() { \
 AS C c;  \
@@ -137,7 +154,7 @@
 // CHECK-LABEL: @_Z4testv()
 // Test the address space of 'this' when invoking a method.
 // CHECK: %1 = addrspacecast %class.C* %c to %class.C 

[PATCH] D55948: Modify DeclaratorChuck::getFunction to use DeclSpec for qualifiers

[Anastasia Stulova via Phabricator via cfe-commits]

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



Comment at: lib/Sema/DeclSpec.cpp:220
+
+  I.Fun.QualAttrFactory = nullptr;
+

rjmccall wrote:
> Spacing?
I am keeping  the old formatting from the lines 195-204. But obviously it 
doesn't match with the current clang-format rules. Do you prefer that I 
reformat the whole block or just this line?



Comment at: lib/Sema/SemaDeclCXX.cpp:8175
   DeclaratorChunk::FunctionTypeInfo  = D.getFunctionTypeInfo();
-  if (FTI.TypeQuals != 0) {
-if (FTI.TypeQuals & Qualifiers::Const)
-  Diag(D.getIdentifierLoc(), diag::err_invalid_qualified_constructor)
-<< "const" << SourceRange(D.getIdentifierLoc());
-if (FTI.TypeQuals & Qualifiers::Volatile)
-  Diag(D.getIdentifierLoc(), diag::err_invalid_qualified_constructor)
-<< "volatile" << SourceRange(D.getIdentifierLoc());
-if (FTI.TypeQuals & Qualifiers::Restrict)
-  Diag(D.getIdentifierLoc(), diag::err_invalid_qualified_constructor)
-<< "restrict" << SourceRange(D.getIdentifierLoc());
+  if (FTI.MethodQualifiers && FTI.MethodQualifiers->getTypeQualifiers() != 0) {
+auto DiagQual = [&](DeclSpec::TQ TypeQual, StringRef QualName,

rjmccall wrote:
> I think you should add a `hasMethodQualifiers` method to FTI that does this 
> check.  Note that it needs to check for attributes, too, and I think you need 
> to figure out some way to generalize `forEachCVRUQual` to cover those.
Are there any attributes I should handle currently?

Also are you suggesting to add another `forEach...` method or extend existing? 
If the latter, I might not be able to use it in all places I use it now.


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

https://reviews.llvm.org/D55948



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

[PATCH] D56198: [Basic] Extend DiagnosticEngine to store and format Qualifiers

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 179887.
Anastasia added a comment.

Addressed review comments.


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

https://reviews.llvm.org/D56198

Files:
  include/clang/AST/Type.h
  include/clang/Basic/Diagnostic.h
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/AST/ASTDiagnostic.cpp
  lib/Basic/Diagnostic.cpp
  lib/Sema/SemaOverload.cpp
  test/SemaCXX/addr-of-overloaded-function.cpp
  test/SemaCXX/warn-overloaded-virtual.cpp

Index: test/SemaCXX/warn-overloaded-virtual.cpp
===
--- test/SemaCXX/warn-overloaded-virtual.cpp
+++ test/SemaCXX/warn-overloaded-virtual.cpp
@@ -130,7 +130,7 @@
 virtual int foo(int*) const;
 // expected-note@-1{{type mismatch at 1st parameter ('int *' vs 'int')}}
 virtual int foo(int) volatile;
-// expected-note@-1{{different qualifiers (volatile vs const)}}
+// expected-note@-1{{different qualifiers ('volatile' vs 'const')}}
   };
 
   class B : public A {
Index: test/SemaCXX/addr-of-overloaded-function.cpp
===
--- test/SemaCXX/addr-of-overloaded-function.cpp
+++ test/SemaCXX/addr-of-overloaded-function.cpp
@@ -221,13 +221,13 @@
 
   void QualifierTest() {
 void (Qualifiers::*X)();
-X = ::C; // expected-error-re {{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} const': different qualifiers (none vs const)}}
-X = ::V; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} volatile': different qualifiers (none vs volatile)}}
-X = ::R; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} __restrict': different qualifiers (none vs restrict)}}
-X = ::CV; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} const volatile': different qualifiers (none vs const and volatile)}}
-X = ::CR; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} const __restrict': different qualifiers (none vs const and restrict)}}
-X = ::VR; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} volatile __restrict': different qualifiers (none vs volatile and restrict)}}
-X = ::CVR; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} const volatile __restrict': different qualifiers (none vs const, volatile, and restrict)}}
+X = ::C; // expected-error-re {{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} const': different qualifiers (unqualified vs 'const')}}
+X = ::V; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} volatile': different qualifiers (unqualified vs 'volatile')}}
+X = ::R; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} __restrict': different qualifiers (unqualified vs '__restrict')}}
+X = ::CV; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} const volatile': different qualifiers (unqualified vs 'const volatile')}}
+X = ::CR; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} const __restrict': different qualifiers (unqualified vs 'const __restrict')}}
+X = ::VR; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} volatile __restrict': different qualifiers (unqualified vs 'volatile __restrict')}}
+X = ::CVR; // expected-error-re{{assigning to 

[PATCH] D55948: Modify DeclaratorChuck::getFunction to use DeclSpec for qualifiers

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Parse/ParseDeclCXX.cpp:2368
 }
   };
+  DeclSpecCheck(DeclSpec::TQ_const, "const", DS.getConstSpecLoc());

Anastasia wrote:
> rjmccall wrote:
> > Please remove this dead semicolon since you're touching this code anyway.
> Here the semicolon is at the end of a lambda declaration statement.
Oh, yes it is, sorry.



Comment at: lib/Parse/ParseExprCXX.cpp:1264
 SourceLocation NoLoc;
+DeclSpec NoDS(AttrFactory);
 D.AddTypeInfo(DeclaratorChunk::getFunction(

This is dead now.



Comment at: lib/Sema/DeclSpec.cpp:220
+
+  I.Fun.QualAttrFactory = nullptr;
+

Spacing?



Comment at: lib/Sema/SemaDeclCXX.cpp:8175
   DeclaratorChunk::FunctionTypeInfo  = D.getFunctionTypeInfo();
-  if (FTI.TypeQuals != 0) {
-if (FTI.TypeQuals & Qualifiers::Const)
-  Diag(D.getIdentifierLoc(), diag::err_invalid_qualified_constructor)
-<< "const" << SourceRange(D.getIdentifierLoc());
-if (FTI.TypeQuals & Qualifiers::Volatile)
-  Diag(D.getIdentifierLoc(), diag::err_invalid_qualified_constructor)
-<< "volatile" << SourceRange(D.getIdentifierLoc());
-if (FTI.TypeQuals & Qualifiers::Restrict)
-  Diag(D.getIdentifierLoc(), diag::err_invalid_qualified_constructor)
-<< "restrict" << SourceRange(D.getIdentifierLoc());
+  if (FTI.MethodQualifiers && FTI.MethodQualifiers->getTypeQualifiers() != 0) {
+auto DiagQual = [&](DeclSpec::TQ TypeQual, StringRef QualName,

I think you should add a `hasMethodQualifiers` method to FTI that does this 
check.  Note that it needs to check for attributes, too, and I think you need 
to figure out some way to generalize `forEachCVRUQual` to cover those.



Comment at: lib/Sema/SemaDeclCXX.cpp:8181
+};
+FTI.MethodQualifiers->forEachCVRUQual(DiagQual);
 D.setInvalidType();

I'd write this lambda inline as the argument, but if you think it's better 
separate, that's okay.



Comment at: lib/Sema/SemaDeclCXX.cpp:8361
   DeclaratorChunk::FunctionTypeInfo  = D.getFunctionTypeInfo();
-  if (FTI.TypeQuals != 0 && !D.isInvalidType()) {
-if (FTI.TypeQuals & Qualifiers::Const)
-  Diag(D.getIdentifierLoc(), diag::err_invalid_qualified_destructor)
-<< "const" << SourceRange(D.getIdentifierLoc());
-if (FTI.TypeQuals & Qualifiers::Volatile)
-  Diag(D.getIdentifierLoc(), diag::err_invalid_qualified_destructor)
-<< "volatile" << SourceRange(D.getIdentifierLoc());
-if (FTI.TypeQuals & Qualifiers::Restrict)
-  Diag(D.getIdentifierLoc(), diag::err_invalid_qualified_destructor)
-<< "restrict" << SourceRange(D.getIdentifierLoc());
+  if (FTI.MethodQualifiers && FTI.MethodQualifiers->getTypeQualifiers() != 0 &&
+  !D.isInvalidType()) {

Another place for `hasMethodQualifiers`.



Comment at: lib/Sema/SemaType.cpp:719
   SourceLocation NoLoc;
+  AttributeFactory attrFactory;
   declarator.AddInnermostTypeInfo(DeclaratorChunk::getFunction(

This is dead now.



Comment at: lib/Sema/SemaType.cpp:4460
+  IsQualifiedFunction =
+  (FTI.MethodQualifiers && FTI.MethodQualifiers->getTypeQualifiers()) 
||
+  FTI.hasRefQualifier();

This would be a good place to use `hasMethodQualifiers` (unless there's a 
reason to ignore attribute qualifiers here).



Comment at: lib/Sema/SemaType.cpp:5035
+RemovalLocs.push_back(SL);
+  };
+  Chunk.Fun.MethodQualifiers->forEachCVRUQual(AddToRemovals);

A lambda this simple should definitely be written inline.


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

https://reviews.llvm.org/D55948



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

[PATCH] D55948: Modify DeclaratorChuck::getFunction to use DeclSpec for qualifiers

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 179877.
Anastasia marked an inline comment as done.
Anastasia added a comment.

Addressed review comments.


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

https://reviews.llvm.org/D55948

Files:
  include/clang/Sema/DeclSpec.h
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Parse/ParseExpr.cpp
  lib/Parse/ParseExprCXX.cpp
  lib/Sema/DeclSpec.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaLambda.cpp
  lib/Sema/SemaType.cpp

Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -716,6 +716,7 @@
 
   // ...and *prepend* it to the declarator.
   SourceLocation NoLoc;
+  AttributeFactory attrFactory;
   declarator.AddInnermostTypeInfo(DeclaratorChunk::getFunction(
   /*HasProto=*/true,
   /*IsAmbiguous=*/false,
@@ -724,12 +725,8 @@
   /*NumArgs=*/0,
   /*EllipsisLoc=*/NoLoc,
   /*RParenLoc=*/NoLoc,
-  /*TypeQuals=*/0,
   /*RefQualifierIsLvalueRef=*/true,
   /*RefQualifierLoc=*/NoLoc,
-  /*ConstQualifierLoc=*/NoLoc,
-  /*VolatileQualifierLoc=*/NoLoc,
-  /*RestrictQualifierLoc=*/NoLoc,
   /*MutableLoc=*/NoLoc, EST_None,
   /*ESpecRange=*/SourceRange(),
   /*Exceptions=*/nullptr,
@@ -737,8 +734,7 @@
   /*NumExceptions=*/0,
   /*NoexceptExpr=*/nullptr,
   /*ExceptionSpecTokens=*/nullptr,
-  /*DeclsInPrototype=*/None,
-  loc, loc, declarator));
+  /*DeclsInPrototype=*/None, loc, loc, declarator));
 
   // For consistency, make sure the state still has us as processing
   // the decl spec.
@@ -4460,7 +4456,9 @@
   // does not have a K identifier list), then the arguments are part
   // of the type, otherwise the argument list is ().
   const DeclaratorChunk::FunctionTypeInfo  = DeclType.Fun;
-  IsQualifiedFunction = FTI.TypeQuals || FTI.hasRefQualifier();
+  IsQualifiedFunction =
+  (FTI.MethodQualifiers && FTI.MethodQualifiers->getTypeQualifiers()) ||
+  FTI.hasRefQualifier();
 
   // Check for auto functions and trailing return type and adjust the
   // return type accordingly.
@@ -4698,7 +4696,9 @@
 EPI.ExtInfo = EI;
 EPI.Variadic = FTI.isVariadic;
 EPI.HasTrailingReturn = FTI.hasTrailingReturnType();
-EPI.TypeQuals.addCVRUQualifiers(FTI.TypeQuals);
+EPI.TypeQuals.addCVRUQualifiers(
+FTI.MethodQualifiers ? FTI.MethodQualifiers->getTypeQualifiers()
+ : 0);
 EPI.RefQualifier = !FTI.hasRefQualifier()? RQ_None
 : FTI.RefQualifierIsLValueRef? RQ_LValue
 : RQ_RValue;
@@ -5024,14 +5024,18 @@
 SmallVector RemovalLocs;
 const DeclaratorChunk  = D.getTypeObject(I);
 assert(Chunk.Kind == DeclaratorChunk::Function);
+
 if (Chunk.Fun.hasRefQualifier())
   RemovalLocs.push_back(Chunk.Fun.getRefQualifierLoc());
-if (Chunk.Fun.TypeQuals & Qualifiers::Const)
-  RemovalLocs.push_back(Chunk.Fun.getConstQualifierLoc());
-if (Chunk.Fun.TypeQuals & Qualifiers::Volatile)
-  RemovalLocs.push_back(Chunk.Fun.getVolatileQualifierLoc());
-if (Chunk.Fun.TypeQuals & Qualifiers::Restrict)
-  RemovalLocs.push_back(Chunk.Fun.getRestrictQualifierLoc());
+
+if (Chunk.Fun.MethodQualifiers) {
+  auto AddToRemovals = [&](DeclSpec::TQ TypeQual, StringRef QualName,
+   SourceLocation SL) {
+RemovalLocs.push_back(SL);
+  };
+  Chunk.Fun.MethodQualifiers->forEachCVRUQual(AddToRemovals);
+}
+
 if (!RemovalLocs.empty()) {
   llvm::sort(RemovalLocs,
  BeforeThanCompare(S.getSourceManager()));
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -884,8 +884,10 @@
 //   This function call operator is declared const (9.3.1) if and only if
 //   the lambda-expression's parameter-declaration-clause is not followed
 //   by mutable. It is neither virtual nor declared volatile. [...]
-if (!FTI.hasMutableQualifier())
-  FTI.TypeQuals |= DeclSpec::TQ_const;
+if (!FTI.hasMutableQualifier()) {
+  FTI.getOrCreateMethodQualifiers().SetTypeQual(DeclSpec::TQ_const,
+SourceLocation());
+}
 
 MethodTyInfo = GetTypeForDeclarator(ParamInfo, CurScope);
 assert(MethodTyInfo && "no type from lambda-declarator");
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -8172,16 +8172,13 @@
   }
 
   DeclaratorChunk::FunctionTypeInfo  = D.getFunctionTypeInfo();
-  if (FTI.TypeQuals != 0) {
-if (FTI.TypeQuals & Qualifiers::Const)
-  

[PATCH] D55948: Modify DeclaratorChuck::getFunction to use DeclSpec for qualifiers

[Anastasia Stulova via Phabricator via cfe-commits]

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



Comment at: lib/Parse/ParseDeclCXX.cpp:2368
 }
   };
+  DeclSpecCheck(DeclSpec::TQ_const, "const", DS.getConstSpecLoc());

rjmccall wrote:
> Please remove this dead semicolon since you're touching this code anyway.
Here the semicolon is at the end of a lambda declaration statement.


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

https://reviews.llvm.org/D55948



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

[PATCH] D56158: [sanitizer_common] Implement funopen*() interceptors for NetBSD

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL350233: [sanitizer_common] Implement funopen*() interceptors 
for NetBSD (authored by mgorny, committed by ).
Herald added a subscriber: delcypher.

Changed prior to commit:
  https://reviews.llvm.org/D56158?vs=179728=179873#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D56158

Files:
  compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc
  compiler-rt/trunk/lib/sanitizer_common/sanitizer_platform_interceptors.h

Index: compiler-rt/trunk/lib/sanitizer_common/sanitizer_platform_interceptors.h
===
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_platform_interceptors.h
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_platform_interceptors.h
@@ -551,5 +551,7 @@
 #define SANITIZER_INTERCEPT_POPEN SI_POSIX
 #define SANITIZER_INTERCEPT_POPENVE SI_NETBSD
 #define SANITIZER_INTERCEPT_PCLOSE SI_POSIX
+#define SANITIZER_INTERCEPT_FUNOPEN SI_NETBSD
+#define SANITIZER_INTERCEPT_FUNOPEN2 SI_NETBSD
 
 #endif  // #ifndef SANITIZER_PLATFORM_INTERCEPTORS_H
Index: compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc
===
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc
@@ -9187,6 +9187,166 @@
 #define INIT_PCLOSE
 #endif
 
+#if SANITIZER_INTERCEPT_FUNOPEN
+typedef int (*funopen_readfn)(void *cookie, char *buf, int len);
+typedef int (*funopen_writefn)(void *cookie, const char *buf, int len);
+typedef OFF_T (*funopen_seekfn)(void *cookie, OFF_T offset, int whence);
+typedef int (*funopen_closefn)(void *cookie);
+
+struct WrappedFunopenCookie {
+  void *real_cookie;
+  funopen_readfn real_read;
+  funopen_writefn real_write;
+  funopen_seekfn real_seek;
+  funopen_closefn real_close;
+};
+
+static int wrapped_funopen_read(void *cookie, char *buf, int len) {
+  COMMON_INTERCEPTOR_UNPOISON_PARAM(3);
+  WrappedFunopenCookie *wrapped_cookie = (WrappedFunopenCookie *)cookie;
+  funopen_readfn real_read = wrapped_cookie->real_read;
+  return real_read(wrapped_cookie->real_cookie, buf, len);
+}
+
+static int wrapped_funopen_write(void *cookie, const char *buf, int len) {
+  COMMON_INTERCEPTOR_UNPOISON_PARAM(3);
+  WrappedFunopenCookie *wrapped_cookie = (WrappedFunopenCookie *)cookie;
+  funopen_writefn real_write = wrapped_cookie->real_write;
+  return real_write(wrapped_cookie->real_cookie, buf, len);
+}
+
+static OFF_T wrapped_funopen_seek(void *cookie, OFF_T offset, int whence) {
+  COMMON_INTERCEPTOR_UNPOISON_PARAM(3);
+  WrappedFunopenCookie *wrapped_cookie = (WrappedFunopenCookie *)cookie;
+  funopen_seekfn real_seek = wrapped_cookie->real_seek;
+  return real_seek(wrapped_cookie->real_cookie, offset, whence);
+}
+
+static int wrapped_funopen_close(void *cookie) {
+  COMMON_INTERCEPTOR_UNPOISON_PARAM(1);
+  WrappedFunopenCookie *wrapped_cookie = (WrappedFunopenCookie *)cookie;
+  funopen_closefn real_close = wrapped_cookie->real_close;
+  int res = real_close(wrapped_cookie->real_cookie);
+  InternalFree(wrapped_cookie);
+  return res;
+}
+
+INTERCEPTOR(__sanitizer_FILE *, funopen, void *cookie, funopen_readfn readfn,
+funopen_writefn writefn, funopen_seekfn seekfn,
+funopen_closefn closefn) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, funopen, cookie, readfn, writefn, seekfn,
+   closefn);
+
+  WrappedFunopenCookie *wrapped_cookie =
+  (WrappedFunopenCookie *)InternalAlloc(sizeof(WrappedFunopenCookie));
+  wrapped_cookie->real_cookie = cookie;
+  wrapped_cookie->real_read = readfn;
+  wrapped_cookie->real_write = writefn;
+  wrapped_cookie->real_seek = seekfn;
+  wrapped_cookie->real_close = closefn;
+
+  __sanitizer_FILE *res =
+  REAL(funopen)(wrapped_cookie,
+readfn  ? wrapped_funopen_read  : nullptr,
+writefn ? wrapped_funopen_write : nullptr,
+seekfn  ? wrapped_funopen_seek  : nullptr,
+closefn ? wrapped_funopen_close : nullptr);
+  if (res)
+unpoison_file(res);
+  return res;
+}
+#define INIT_FUNOPEN COMMON_INTERCEPT_FUNCTION(funopen)
+#else
+#define INIT_FUNOPEN
+#endif
+
+#if SANITIZER_INTERCEPT_FUNOPEN2
+typedef SSIZE_T (*funopen2_readfn)(void *cookie, void *buf, SIZE_T len);
+typedef SSIZE_T (*funopen2_writefn)(void *cookie, const void *buf, SIZE_T len);
+typedef OFF_T (*funopen2_seekfn)(void *cookie, OFF_T offset, int whence);
+typedef int (*funopen2_flushfn)(void *cookie);
+typedef int (*funopen2_closefn)(void *cookie);
+
+struct WrappedFunopen2Cookie {
+  void *real_cookie;
+  funopen2_readfn real_read;
+  funopen2_writefn real_write;
+  funopen2_seekfn real_seek;
+  funopen2_flushfn real_flush;
+  funopen2_closefn real_close;
+};
+

[PATCH] D56157: [sanitizer_common] Implement popen, popenve, pclose interceptors

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL350232: [sanitizer_common] Implement popen, popenve, pclose 
interceptors (authored by mgorny, committed by ).
Herald added a subscriber: delcypher.

Changed prior to commit:
  https://reviews.llvm.org/D56157?vs=179842=179872#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D56157

Files:
  compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc
  compiler-rt/trunk/lib/sanitizer_common/sanitizer_platform_interceptors.h
  compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc

Index: compiler-rt/trunk/lib/sanitizer_common/sanitizer_platform_interceptors.h
===
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_platform_interceptors.h
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_platform_interceptors.h
@@ -548,4 +548,8 @@
 #define SANITIZER_INTERCEPT_GETFSENT (SI_FREEBSD || SI_NETBSD || SI_MAC)
 #define SANITIZER_INTERCEPT_ARC4RANDOM (SI_FREEBSD || SI_NETBSD)
 
+#define SANITIZER_INTERCEPT_POPEN SI_POSIX
+#define SANITIZER_INTERCEPT_POPENVE SI_NETBSD
+#define SANITIZER_INTERCEPT_PCLOSE SI_POSIX
+
 #endif  // #ifndef SANITIZER_PLATFORM_INTERCEPTORS_H
Index: compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc
===
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc
@@ -9116,6 +9116,77 @@
 #define INIT_ARC4RANDOM
 #endif
 
+#if SANITIZER_INTERCEPT_POPEN
+INTERCEPTOR(__sanitizer_FILE *, popen, const char *command, const char *type) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, popen, command, type);
+  if (command)
+COMMON_INTERCEPTOR_READ_RANGE(ctx, command, REAL(strlen)(command) + 1);
+  if (type)
+COMMON_INTERCEPTOR_READ_RANGE(ctx, type, REAL(strlen)(type) + 1);
+  __sanitizer_FILE *res = REAL(popen)(command, type);
+  COMMON_INTERCEPTOR_FILE_OPEN(ctx, res, nullptr);
+  if (res) unpoison_file(res);
+  return res;
+}
+#define INIT_POPEN COMMON_INTERCEPT_FUNCTION(popen)
+#else
+#define INIT_POPEN
+#endif
+
+#if SANITIZER_INTERCEPT_POPENVE
+INTERCEPTOR(__sanitizer_FILE *, popenve, const char *path,
+char *const *argv, char *const *envp, const char *type) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, popenve, path, argv, envp, type);
+  if (path)
+COMMON_INTERCEPTOR_READ_RANGE(ctx, path, REAL(strlen)(path) + 1);
+  if (argv) {
+for (char *const *pa = argv; ; ++pa) {
+  COMMON_INTERCEPTOR_READ_RANGE(ctx, pa, sizeof(char **));
+  if (!*pa)
+break;
+  COMMON_INTERCEPTOR_READ_RANGE(ctx, *pa, REAL(strlen)(*pa) + 1);
+}
+  }
+  if (envp) {
+for (char *const *pa = envp; ; ++pa) {
+  COMMON_INTERCEPTOR_READ_RANGE(ctx, pa, sizeof(char **));
+  if (!*pa)
+break;
+  COMMON_INTERCEPTOR_READ_RANGE(ctx, *pa, REAL(strlen)(*pa) + 1);
+}
+  }
+  if (type)
+COMMON_INTERCEPTOR_READ_RANGE(ctx, type, REAL(strlen)(type) + 1);
+  __sanitizer_FILE *res = REAL(popenve)(path, argv, envp, type);
+  COMMON_INTERCEPTOR_FILE_OPEN(ctx, res, nullptr);
+  if (res) unpoison_file(res);
+  return res;
+}
+#define INIT_POPENVE COMMON_INTERCEPT_FUNCTION(popenve)
+#else
+#define INIT_POPENVE
+#endif
+
+#if SANITIZER_INTERCEPT_PCLOSE
+INTERCEPTOR(int, pclose, __sanitizer_FILE *fp) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, pclose, fp);
+  COMMON_INTERCEPTOR_FILE_CLOSE(ctx, fp);
+  const FileMetadata *m = GetInterceptorMetadata(fp);
+  int res = REAL(pclose)(fp);
+  if (m) {
+COMMON_INTERCEPTOR_INITIALIZE_RANGE(*m->addr, *m->size);
+DeleteInterceptorMetadata(fp);
+  }
+  return res;
+}
+#define INIT_PCLOSE COMMON_INTERCEPT_FUNCTION(pclose);
+#else
+#define INIT_PCLOSE
+#endif
+
 static void InitializeCommonInterceptors() {
   static u64 metadata_mem[sizeof(MetadataHashMap) / sizeof(u64) + 1];
   interceptor_metadata_map =
@@ -9398,6 +9469,9 @@
   INIT_CDB;
   INIT_GETFSENT;
   INIT_ARC4RANDOM;
+  INIT_POPEN;
+  INIT_POPENVE;
+  INIT_PCLOSE;
 
   INIT___PRINTF_CHK;
 }
Index: compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc
===
--- compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc
+++ compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc
@@ -2254,7 +2254,8 @@
   (void) ctx;
 
 #define COMMON_INTERCEPTOR_FILE_OPEN(ctx, file, path) \
-  Acquire(thr, pc, File2addr(path));  \
+  if (path)   \
+Acquire(thr, pc, File2addr(path));\
   if (file) { \
 int fd = fileno_unlocked(file);   \
 if (fd >= 0) FdFileCreate(thr, pc, fd);   \
___
cfe-commits mailing list

[PATCH] D56153: [sanitizer_common] Add test for popen()

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL350230: [sanitizer_common] Add test for popen() (authored by 
mgorny, committed by ).
Herald added a subscriber: delcypher.

Changed prior to commit:
  https://reviews.llvm.org/D56153?vs=179712=179870#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D56153

Files:
  compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/popen.cc


Index: compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/popen.cc
===
--- compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/popen.cc
+++ compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/popen.cc
@@ -0,0 +1,23 @@
+// RUN: %clangxx -g %s -o %t && %run %t | FileCheck %s
+// CHECK: 1
+// CHECK-NEXT: 2
+
+#include 
+#include 
+
+int main(void) {
+  // use a tool that produces different output than input to verify
+  // that everything worked correctly
+  FILE *fp = popen("sort", "w");
+  assert(fp);
+
+  // verify that fileno() returns a meaningful descriptor (needed
+  // for the implementation of TSan)
+  assert(fileno(fp) != -1);
+
+  assert(fputs("2\n", fp) >= 0);
+  assert(fputs("1\n", fp) >= 0);
+  assert(pclose(fp) == 0);
+
+  return 0;
+}


Index: compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/popen.cc
===
--- compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/popen.cc
+++ compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/popen.cc
@@ -0,0 +1,23 @@
+// RUN: %clangxx -g %s -o %t && %run %t | FileCheck %s
+// CHECK: 1
+// CHECK-NEXT: 2
+
+#include 
+#include 
+
+int main(void) {
+  // use a tool that produces different output than input to verify
+  // that everything worked correctly
+  FILE *fp = popen("sort", "w");
+  assert(fp);
+
+  // verify that fileno() returns a meaningful descriptor (needed
+  // for the implementation of TSan)
+  assert(fileno(fp) != -1);
+
+  assert(fputs("2\n", fp) >= 0);
+  assert(fputs("1\n", fp) >= 0);
+  assert(pclose(fp) == 0);
+
+  return 0;
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56154: [sanitizer_common] Add tests for NetBSD funopen*()

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL350231: [sanitizer_common] Add tests for NetBSD funopen*() 
functions (authored by mgorny, committed by ).
Herald added a subscriber: delcypher.

Changed prior to commit:
  https://reviews.llvm.org/D56154?vs=179713=179871#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D56154

Files:
  compiler-rt/trunk/test/sanitizer_common/TestCases/NetBSD/funopen.cc
  compiler-rt/trunk/test/sanitizer_common/TestCases/NetBSD/funopen2.cc

Index: compiler-rt/trunk/test/sanitizer_common/TestCases/NetBSD/funopen2.cc
===
--- compiler-rt/trunk/test/sanitizer_common/TestCases/NetBSD/funopen2.cc
+++ compiler-rt/trunk/test/sanitizer_common/TestCases/NetBSD/funopen2.cc
@@ -0,0 +1,110 @@
+// RUN: %clangxx -g %s -o %t && %run %t | FileCheck %s
+
+// CHECK: READ CALLED; len={{[0-9]*}}
+// CHECK-NEXT: READ: test
+// CHECK-NEXT: WRITE CALLED: test
+// CHECK-NEXT: READ CALLED; len={{[0-9]*}}
+// CHECK-NEXT: READ: test
+// CHECK-NEXT: WRITE CALLED: test
+// CHECK-NEXT: CLOSE CALLED
+// CHECK-NEXT: SEEK CALLED; off=100, whence=0
+// CHECK-NEXT: READ CALLED; len={{[0-9]*}}
+// CHECK-NEXT: READ: test
+// CHECK-NEXT: WRITE CALLED: test
+// CHECK-NEXT: FLUSH CALLED
+// CHECK-NEXT: WRITE CALLED: test
+// CHECK-NEXT: FLUSH CALLED
+
+#include 
+#include 
+#include 
+#include 
+
+int cookie_var;
+
+ssize_t f_read(void *cookie, void *buf, size_t len) {
+  assert(cookie == _var);
+  assert(len >= 6);
+  printf("READ CALLED; len=%zd\n", len);
+  return strlcpy((char*)buf, "test\n", len);
+}
+
+ssize_t f_write(void *cookie, const void *buf, size_t len) {
+  assert(cookie == _var);
+  char *data = strndup((char*)buf, len);
+  assert(data);
+  printf("WRITE CALLED: %s\n", data);
+  free(data);
+  return len;
+}
+
+off_t f_seek(void *cookie, off_t off, int whence) {
+  assert(cookie == _var);
+  assert(whence == SEEK_SET);
+  printf("SEEK CALLED; off=%d, whence=%d\n", (int)off, whence);
+  return off;
+}
+
+int f_flush(void *cookie) {
+  assert(cookie == _var);
+  printf("FLUSH CALLED\n");
+  return 0;
+}
+
+int f_close(void *cookie) {
+  assert(cookie == _var);
+  printf("CLOSE CALLED\n");
+  return 0;
+}
+
+int main(void) {
+  FILE *fp;
+  char buf[10];
+
+  // 1. read-only variant
+  fp = fropen2(_var, f_read);
+  assert(fp);
+  // verify that fileno() does not crash or report nonsense
+  assert(fileno(fp) == -1);
+  assert(fgets(buf, sizeof(buf), fp));
+  printf("READ: %s", buf);
+  assert(!fclose(fp));
+
+  // 2. write-only variant
+  fp = fwopen2(_var, f_write);
+  assert(fp);
+  assert(fileno(fp) == -1);
+  assert(fputs("test", fp) >= 0);
+  assert(!fclose(fp));
+
+  // 3. read+write+close
+  fp = funopen2(_var, f_read, f_write, NULL, NULL, f_close);
+  assert(fp);
+  assert(fileno(fp) == -1);
+  assert(fgets(buf, sizeof(buf), fp));
+  printf("READ: %s", buf);
+  assert(fputs("test", fp) >= 0);
+  assert(!fflush(fp));
+  assert(!fclose(fp));
+
+  // 4. read+seek
+  fp = funopen2(_var, f_read, NULL, f_seek, NULL, NULL);
+  assert(fp);
+  assert(fileno(fp) == -1);
+  assert(fseek(fp, 100, SEEK_SET) == 0);
+  assert(fgets(buf, sizeof(buf), fp));
+  printf("READ: %s", buf);
+  assert(!fclose(fp));
+
+  // 5. write+flush
+  fp = funopen2(_var, NULL, f_write, NULL, f_flush, NULL);
+  assert(fp);
+  assert(fileno(fp) == -1);
+  assert(fputs("test", fp) >= 0);
+  assert(!fflush(fp));
+  assert(fputs("test", fp) >= 0);
+  // NB: fclose() also implicitly calls flush
+  assert(!fclose(fp));
+
+  return 0;
+}
Index: compiler-rt/trunk/test/sanitizer_common/TestCases/NetBSD/funopen.cc
===
--- compiler-rt/trunk/test/sanitizer_common/TestCases/NetBSD/funopen.cc
+++ compiler-rt/trunk/test/sanitizer_common/TestCases/NetBSD/funopen.cc
@@ -0,0 +1,89 @@
+// RUN: %clangxx -g %s -o %t && %run %t | FileCheck %s
+
+// CHECK: READ CALLED; len={{[0-9]*}}
+// CHECK-NEXT: READ: test
+// CHECK-NEXT: WRITE CALLED: test
+// CHECK-NEXT: READ CALLED; len={{[0-9]*}}
+// CHECK-NEXT: READ: test
+// CHECK-NEXT: WRITE CALLED: test
+// CHECK-NEXT: CLOSE CALLED
+// CHECK-NEXT: SEEK CALLED; off=100, whence=0
+// CHECK-NEXT: READ CALLED; len={{[0-9]*}}
+// CHECK-NEXT: READ: test
+
+#include 
+#include 
+#include 
+#include 
+
+int cookie_var;
+
+int f_read(void *cookie, char *buf, int len) {
+  assert(cookie == _var);
+  assert(len >= 6);
+  printf("READ CALLED; len=%d\n", len);
+  return strlcpy(buf, "test\n", len);
+}
+
+int f_write(void *cookie, const char *buf, int len) {
+  assert(cookie == _var);
+  char *data = strndup(buf, len);
+  assert(data);
+  printf("WRITE CALLED: %s\n", data);
+  free(data);
+  return len;
+}
+
+off_t f_seek(void *cookie, off_t off, int whence) {
+  assert(cookie == _var);
+  assert(whence == SEEK_SET);
+  printf("SEEK CALLED; off=%d, whence=%d\n", (int)off, whence);
+  

[PATCH] D56150: [sanitizer_common] Fix devname_r() return type on !NetBSD

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL350228: [sanitizer_common] Fix devname_r() return type on 
!NetBSD (authored by mgorny, committed by ).
Herald added a subscriber: delcypher.

Changed prior to commit:
  https://reviews.llvm.org/D56150?vs=179707=179869#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D56150

Files:
  compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc


Index: compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc
===
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc
@@ -7061,12 +7061,19 @@
 #endif
 
 #if SANITIZER_INTERCEPT_DEVNAME_R
-INTERCEPTOR(int, devname_r, u64 dev, u32 type, char *path, uptr len) {
+#if SANITIZER_NETBSD
+#define DEVNAME_R_RETTYPE int
+#define DEVNAME_R_SUCCESS(x) (!(x))
+#else
+#define DEVNAME_R_RETTYPE char*
+#define DEVNAME_R_SUCCESS(x) (x)
+#endif
+INTERCEPTOR(DEVNAME_R_RETTYPE, devname_r, u64 dev, u32 type, char *path,
+uptr len) {
   void *ctx;
-  int res;
   COMMON_INTERCEPTOR_ENTER(ctx, devname_r, dev, type, path, len);
-  res = REAL(devname_r)(dev, type, path, len);
-  if (!res)
+  DEVNAME_R_RETTYPE res = REAL(devname_r)(dev, type, path, len);
+  if (DEVNAME_R_SUCCESS(res))
 COMMON_INTERCEPTOR_WRITE_RANGE(ctx, path, REAL(strlen)(path) + 1);
   return res;
 }


Index: compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc
===
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc
@@ -7061,12 +7061,19 @@
 #endif
 
 #if SANITIZER_INTERCEPT_DEVNAME_R
-INTERCEPTOR(int, devname_r, u64 dev, u32 type, char *path, uptr len) {
+#if SANITIZER_NETBSD
+#define DEVNAME_R_RETTYPE int
+#define DEVNAME_R_SUCCESS(x) (!(x))
+#else
+#define DEVNAME_R_RETTYPE char*
+#define DEVNAME_R_SUCCESS(x) (x)
+#endif
+INTERCEPTOR(DEVNAME_R_RETTYPE, devname_r, u64 dev, u32 type, char *path,
+uptr len) {
   void *ctx;
-  int res;
   COMMON_INTERCEPTOR_ENTER(ctx, devname_r, dev, type, path, len);
-  res = REAL(devname_r)(dev, type, path, len);
-  if (!res)
+  DEVNAME_R_RETTYPE res = REAL(devname_r)(dev, type, path, len);
+  if (DEVNAME_R_SUCCESS(res))
 COMMON_INTERCEPTOR_WRITE_RANGE(ctx, path, REAL(strlen)(path) + 1);
   return res;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56152: [sanitizer_common] Add tests for more *putc and *getc variants

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL350229: [sanitizer_common] Add tests for more *putc and 
*getc variants (authored by mgorny, committed by ).
Herald added a subscriber: delcypher.

Changed prior to commit:
  https://reviews.llvm.org/D56152?vs=179711=179868#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D56152

Files:
  compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/fputc_putc_putchar.cc
  compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/getc_unlocked.cc
  
compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/putc_putchar_unlocked.cc


Index: 
compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/fputc_putc_putchar.cc
===
--- 
compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/fputc_putc_putchar.cc
+++ 
compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/fputc_putc_putchar.cc
@@ -0,0 +1,13 @@
+// RUN: %clangxx -g %s -o %t && %run %t | FileCheck %s
+// CHECK: abc
+
+#include 
+#include 
+
+int main(void) {
+  assert(fputc('a', stdout) != EOF);
+  assert(putc('b', stdout) != EOF);
+  assert(putchar('c') != EOF);
+
+  return 0;
+}
Index: 
compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/putc_putchar_unlocked.cc
===
--- 
compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/putc_putchar_unlocked.cc
+++ 
compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/putc_putchar_unlocked.cc
@@ -0,0 +1,12 @@
+// RUN: %clangxx -g %s -o %t && %run %t | FileCheck %s
+// CHECK: bc
+
+#include 
+#include 
+
+int main(void) {
+  assert(putc_unlocked('b', stdout) != EOF);
+  assert(putchar_unlocked('c') != EOF);
+
+  return 0;
+}
Index: compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/getc_unlocked.cc
===
--- compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/getc_unlocked.cc
+++ compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/getc_unlocked.cc
@@ -0,0 +1,20 @@
+// RUN: %clangxx -g %s -o %t && %run %t
+
+#include 
+#include 
+
+int main(int argc, char **argv) {
+  FILE *fp = fopen(argv[0], "r");
+  assert(fp);
+
+  // the file should be at least one character long, always
+  assert(getc_unlocked(fp) != EOF);
+  // POSIX guarantees being able to ungetc() at least one character
+  // NB: ungetc_unlocked is apparently not present
+  assert(ungetc('X', fp) != EOF);
+  // check whether ungetc() works with getc_unlocked()
+  assert(getc_unlocked(fp) == 'X');
+
+  assert(!fclose(fp));
+  return 0;
+}


Index: compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/fputc_putc_putchar.cc
===
--- compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/fputc_putc_putchar.cc
+++ compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/fputc_putc_putchar.cc
@@ -0,0 +1,13 @@
+// RUN: %clangxx -g %s -o %t && %run %t | FileCheck %s
+// CHECK: abc
+
+#include 
+#include 
+
+int main(void) {
+  assert(fputc('a', stdout) != EOF);
+  assert(putc('b', stdout) != EOF);
+  assert(putchar('c') != EOF);
+
+  return 0;
+}
Index: compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/putc_putchar_unlocked.cc
===
--- compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/putc_putchar_unlocked.cc
+++ compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/putc_putchar_unlocked.cc
@@ -0,0 +1,12 @@
+// RUN: %clangxx -g %s -o %t && %run %t | FileCheck %s
+// CHECK: bc
+
+#include 
+#include 
+
+int main(void) {
+  assert(putc_unlocked('b', stdout) != EOF);
+  assert(putchar_unlocked('c') != EOF);
+
+  return 0;
+}
Index: compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/getc_unlocked.cc
===
--- compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/getc_unlocked.cc
+++ compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/getc_unlocked.cc
@@ -0,0 +1,20 @@
+// RUN: %clangxx -g %s -o %t && %run %t
+
+#include 
+#include 
+
+int main(int argc, char **argv) {
+  FILE *fp = fopen(argv[0], "r");
+  assert(fp);
+
+  // the file should be at least one character long, always
+  assert(getc_unlocked(fp) != EOF);
+  // POSIX guarantees being able to ungetc() at least one character
+  // NB: ungetc_unlocked is apparently not present
+  assert(ungetc('X', fp) != EOF);
+  // check whether ungetc() works with getc_unlocked()
+  assert(getc_unlocked(fp) == 'X');
+
+  assert(!fclose(fp));
+  return 0;
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56149: [sanitizer_common] Rewrite more Posix tests to use asserts

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL350227: [sanitizer_common] Rewrite more Posix tests to use 
asserts (authored by mgorny, committed by ).
Herald added a subscriber: delcypher.

Changed prior to commit:
  https://reviews.llvm.org/D56149?vs=179693=179867#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D56149

Files:
  compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/devname.cc
  compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/devname_r.cc
  compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/fgetln.cc
  compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/fgets.cc
  compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/fputs_puts.cc
  compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/lstat.cc

Index: compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/devname.cc
===
--- compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/devname.cc
+++ compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/devname.cc
@@ -1,6 +1,7 @@
 // RUN: %clangxx -O0 -g %s -o %t && %run %t 2>&1 | FileCheck %s
 // UNSUPPORTED: linux, solaris
 
+#include 
 #include 
 #include 
 #include 
@@ -9,11 +10,8 @@
   struct stat st;
   char *name;
 
-  if (stat("/dev/null", ))
-exit(1);
-
-  if (!(name = devname(st.st_rdev, S_ISCHR(st.st_mode) ? S_IFCHR : S_IFBLK)))
-exit(1);
+  assert(!stat("/dev/null", ));
+  assert((name = devname(st.st_rdev, S_ISCHR(st.st_mode) ? S_IFCHR : S_IFBLK)));
 
   printf("%s\n", name);
 
Index: compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/fputs_puts.cc
===
--- compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/fputs_puts.cc
+++ compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/fputs_puts.cc
@@ -1,18 +1,12 @@
 // RUN: %clangxx -g %s -o %t && %run %t | FileCheck %s
 // CHECK: {{^foobar$}}
 
+#include 
 #include 
 
 int main(void) {
-  int r;
-
-  r = fputs("foo", stdout);
-  if (r < 0)
-return 1;
-
-  r = puts("bar");
-  if (r < 0)
-return 1;
+  assert(fputs("foo", stdout) >= 0);
+  assert(puts("bar") >= 0);
 
   return 0;
 }
Index: compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/fgets.cc
===
--- compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/fgets.cc
+++ compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/fgets.cc
@@ -1,20 +1,16 @@
 // RUN: %clangxx -g %s -o %t && %run %t
 
+#include 
 #include 
 
 int main(int argc, char **argv) {
-  FILE *fp;
-  char buf[2];
-  char *s;
-
-  fp = fopen(argv[0], "r");
-  if (!fp)
-return 1;
+  FILE *fp = fopen(argv[0], "r");
+  assert(fp);
 
-  s = fgets(buf, sizeof(buf), fp);
-  if (!s)
-return 2;
+  char buf[2];
+  char *s = fgets(buf, sizeof(buf), fp);
+  assert(s);
 
-  fclose(fp);
+  assert(!fclose(fp));
   return 0;
 }
Index: compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/fgetln.cc
===
--- compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/fgetln.cc
+++ compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/fgetln.cc
@@ -1,24 +1,20 @@
 // RUN: %clangxx -O0 -g %s -o %t && %run %t
 // UNSUPPORTED: linux
 
+#include 
 #include 
 #include 
 
 int main(void) {
-  FILE *fp;
-  size_t len;
-  char *s;
-
-  fp = fopen("/etc/hosts", "r");
-  if (!fp)
-exit(1);
+  FILE *fp = fopen("/etc/hosts", "r");
+  assert(fp);
 
-  s = fgetln(fp, );
+  size_t len;
+  char *s = fgetln(fp, );
 
   printf("%.*s\n", (int)len, s);
 
-  if (fclose(fp) == EOF)
-exit(1);
+  assert(!fclose(fp));
 
   return 0;
 }
Index: compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/devname_r.cc
===
--- compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/devname_r.cc
+++ compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/devname_r.cc
@@ -4,6 +4,7 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 
@@ -12,17 +13,14 @@
   char name[100];
   mode_t type;
 
-  if (stat("/dev/null", ))
-exit(1);
+  assert(!stat("/dev/null", ));
 
   type = S_ISCHR(st.st_mode) ? S_IFCHR : S_IFBLK;
 
 #if defined(__NetBSD__)
-  if (devname_r(st.st_rdev, type, name, sizeof(name)))
-exit(1);
+  assert(!devname_r(st.st_rdev, type, name, sizeof(name)));
 #else
-  if (!devname_r(st.st_rdev, type, name, sizeof(name)))
-exit(1);
+  assert(devname_r(st.st_rdev, type, name, sizeof(name)));
 #endif
 
   printf("%s\n", name);
Index: compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/lstat.cc
===
--- compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/lstat.cc
+++ compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/lstat.cc
@@ -1,16 +1,14 @@
 // RUN: %clangxx -O0 -g %s -o %t && 

[PATCH] D56136: [compiler-rt] [sanitizer_common] Add tests for more stdio.h functions

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL350225: [sanitizer_common] Add tests for more stdio.h 
functions (authored by mgorny, committed by ).
Herald added a subscriber: delcypher.

Changed prior to commit:
  https://reviews.llvm.org/D56136?vs=179691=179865#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D56136

Files:
  compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/feof_fileno_ferror.cc
  compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/fgetc_ungetc_getc.cc


Index: 
compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/fgetc_ungetc_getc.cc
===
--- compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/fgetc_ungetc_getc.cc
+++ compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/fgetc_ungetc_getc.cc
@@ -0,0 +1,19 @@
+// RUN: %clangxx -g %s -o %t && %run %t
+
+#include 
+#include 
+
+int main(int argc, char **argv) {
+  FILE *fp = fopen(argv[0], "r");
+  assert(fp);
+
+  // the file should be at least one character long, always
+  assert(fgetc(fp) != EOF);
+  // POSIX guarantees being able to ungetc() at least one character
+  assert(ungetc('X', fp) != EOF);
+  // check whether ungetc() worked
+  assert(getc(fp) == 'X');
+
+  assert(!fclose(fp));
+  return 0;
+}
Index: 
compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/feof_fileno_ferror.cc
===
--- 
compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/feof_fileno_ferror.cc
+++ 
compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/feof_fileno_ferror.cc
@@ -0,0 +1,41 @@
+// RUN: %clangxx -g %s -o %t && %run %t
+
+#include 
+#include 
+#include 
+
+int main(int argc, char **argv) {
+  FILE *fp = fopen(argv[0], "r");
+  assert(fp);
+
+  // file should be good upon opening
+  assert(!feof(fp) && !ferror(fp));
+
+  // read until EOF
+  char buf[BUFSIZ];
+  while (fread(buf, 1, sizeof buf, fp) != 0) {}
+  assert(feof(fp));
+
+  // clear EOF
+  clearerr(fp);
+  assert(!feof(fp) && !ferror(fp));
+
+  // get file descriptor
+  int fd = fileno(fp);
+  assert(fd != -1);
+
+  // break the file by closing underlying descriptor
+  assert(close(fd) != -1);
+
+  // verify that an error is signalled
+  assert(fread(buf, 1, sizeof buf, fp) == 0);
+  assert(ferror(fp));
+
+  // clear error
+  clearerr(fp);
+  assert(!feof(fp) && !ferror(fp));
+
+  // fclose() will return EBADF because of closed fd
+  assert(fclose(fp) == -1);
+  return 0;
+}


Index: compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/fgetc_ungetc_getc.cc
===
--- compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/fgetc_ungetc_getc.cc
+++ compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/fgetc_ungetc_getc.cc
@@ -0,0 +1,19 @@
+// RUN: %clangxx -g %s -o %t && %run %t
+
+#include 
+#include 
+
+int main(int argc, char **argv) {
+  FILE *fp = fopen(argv[0], "r");
+  assert(fp);
+
+  // the file should be at least one character long, always
+  assert(fgetc(fp) != EOF);
+  // POSIX guarantees being able to ungetc() at least one character
+  assert(ungetc('X', fp) != EOF);
+  // check whether ungetc() worked
+  assert(getc(fp) == 'X');
+
+  assert(!fclose(fp));
+  return 0;
+}
Index: compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/feof_fileno_ferror.cc
===
--- compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/feof_fileno_ferror.cc
+++ compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/feof_fileno_ferror.cc
@@ -0,0 +1,41 @@
+// RUN: %clangxx -g %s -o %t && %run %t
+
+#include 
+#include 
+#include 
+
+int main(int argc, char **argv) {
+  FILE *fp = fopen(argv[0], "r");
+  assert(fp);
+
+  // file should be good upon opening
+  assert(!feof(fp) && !ferror(fp));
+
+  // read until EOF
+  char buf[BUFSIZ];
+  while (fread(buf, 1, sizeof buf, fp) != 0) {}
+  assert(feof(fp));
+
+  // clear EOF
+  clearerr(fp);
+  assert(!feof(fp) && !ferror(fp));
+
+  // get file descriptor
+  int fd = fileno(fp);
+  assert(fd != -1);
+
+  // break the file by closing underlying descriptor
+  assert(close(fd) != -1);
+
+  // verify that an error is signalled
+  assert(fread(buf, 1, sizeof buf, fp) == 0);
+  assert(ferror(fp));
+
+  // clear error
+  clearerr(fp);
+  assert(!feof(fp) && !ferror(fp));
+
+  // fclose() will return EBADF because of closed fd
+  assert(fclose(fp) == -1);
+  return 0;
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56067: Make test/Driver/darwin-sdk-version.c pass if the host triple is 32-bit

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

arphaman: 2019 ping :-)


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

https://reviews.llvm.org/D56067



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

r350224 - Only convert objc messages to alloc to objc_alloc if the receiver is a class.

[Pete Cooper via cfe-commits]

Author: pete
Date: Wed Jan  2 09:25:30 2019
New Revision: 350224

URL: http://llvm.org/viewvc/llvm-project?rev=350224=rev
Log:
Only convert objc messages to alloc to objc_alloc if the receiver is a class.

r348687 converted [Foo alloc] to objc_alloc(Foo).  However the objc runtime 
method only takes a Class, not an arbitrary pointer.

This makes sure we are messaging a class before we convert these messages.

rdar://problem/46943703

Modified:
cfe/trunk/lib/CodeGen/CGObjC.cpp
cfe/trunk/test/CodeGenObjC/convert-messages-to-runtime-calls.m

Modified: cfe/trunk/lib/CodeGen/CGObjC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjC.cpp?rev=350224=350223=350224=diff
==
--- cfe/trunk/lib/CodeGen/CGObjC.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGObjC.cpp Wed Jan  2 09:25:30 2019
@@ -370,7 +370,8 @@ static Optional
 tryGenerateSpecializedMessageSend(CodeGenFunction , QualType ResultType,
   llvm::Value *Receiver,
   const CallArgList& Args, Selector Sel,
-  const ObjCMethodDecl *method) {
+  const ObjCMethodDecl *method,
+  bool isClassMessage) {
   auto  = CGF.CGM;
   if (!CGM.getCodeGenOpts().ObjCConvertMessagesToRuntimeCalls)
 return None;
@@ -378,7 +379,8 @@ tryGenerateSpecializedMessageSend(CodeGe
   auto  = CGM.getLangOpts().ObjCRuntime;
   switch (Sel.getMethodFamily()) {
   case OMF_alloc:
-if (Runtime.shouldUseRuntimeFunctionsForAlloc() &&
+if (isClassMessage &&
+Runtime.shouldUseRuntimeFunctionsForAlloc() &&
 ResultType->isObjCObjectPointerType()) {
 // [Foo alloc] -> objc_alloc(Foo)
 if (Sel.isUnarySelector() && Sel.getNameForSlot(0) == "alloc")
@@ -550,7 +552,8 @@ RValue CodeGenFunction::EmitObjCMessageE
 // Call runtime methods directly if we can.
 if (Optional SpecializedResult =
 tryGenerateSpecializedMessageSend(*this, ResultType, Receiver, 
Args,
-  E->getSelector(), method)) {
+  E->getSelector(), method,
+  isClassMessage)) {
   result = RValue::get(SpecializedResult.getValue());
 } else {
   result = Runtime.GenerateMessageSend(*this, Return, ResultType,

Modified: cfe/trunk/test/CodeGenObjC/convert-messages-to-runtime-calls.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/convert-messages-to-runtime-calls.m?rev=350224=350223=350224=diff
==
--- cfe/trunk/test/CodeGenObjC/convert-messages-to-runtime-calls.m (original)
+++ cfe/trunk/test/CodeGenObjC/convert-messages-to-runtime-calls.m Wed Jan  2 
09:25:30 2019
@@ -54,7 +54,9 @@ void test2(void* x) {
 @class A;
 @interface B
 + (A*) alloc;
-+ (A*)allocWithZone:(void*)zone;
++ (A*) allocWithZone:(void*)zone;
+- (A*) alloc;
+- (A*) allocWithZone:(void*)zone;
 - (A*) retain;
 - (A*) autorelease;
 @end
@@ -79,6 +81,19 @@ A* test_allocWithZone_class_ptr() {
   return [B allocWithZone:nil];
 }
 
+// Only call objc_alloc on a Class, not an instance
+// CHECK-LABEL: define {{.*}}void @test_alloc_instance
+void test_alloc_instance(A *a) {
+  // CALLS: {{call.*@objc_alloc}}
+  // CALLS: {{call.*@objc_allocWithZone}}
+  // CALLS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_msgSend}}
+  [A alloc];
+  [A allocWithZone:nil];
+  [a alloc];
+  [a allocWithZone:nil];
+}
+
 // Make sure we get a bitcast on the return type as the
 // call will return i8* which we have to cast to A*
 // CHECK-LABEL: define {{.*}}void @test_retain_class_ptr


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

[PATCH] D56198: [Basic] Extend DiagnosticEngine to store and format Qualifiers

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: include/clang/AST/Type.h:6705
+/// Insertion operator for diagnostics. This allows sending Qualifiers' into a
+/// diagnostic with <<.
+inline const DiagnosticBuilder <<(const DiagnosticBuilder ,

Unpaired apostrophe, here and below.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:1812
   "|: different qualifiers ("
-  "%select{none|const|restrict|const and restrict|volatile|const and volatile|"
-  "volatile and restrict|const, volatile, and restrict}5 vs "
-  "%select{none|const|restrict|const and restrict|volatile|const and volatile|"
-  "volatile and restrict|const, volatile, and restrict}6)"
+  "%5 vs %6)"
   "|: different exception specifications}4">;

The line break is no longer necessary.



Comment at: test/SemaCXX/addr-of-overloaded-function.cpp:224
 void (Qualifiers::*X)();
-X = ::C; // expected-error-re {{assigning to 'void 
(test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from 
incompatible type 'void (test1::Qualifiers::*)(){{( 
__attribute__\(\(thiscall\)\))?}} const': different qualifiers (none vs const)}}
-X = ::V; // expected-error-re{{assigning to 'void 
(test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from 
incompatible type 'void (test1::Qualifiers::*)(){{( 
__attribute__\(\(thiscall\)\))?}} volatile': different qualifiers (none vs 
volatile)}}
-X = ::R; // expected-error-re{{assigning to 'void 
(test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from 
incompatible type 'void (test1::Qualifiers::*)(){{( 
__attribute__\(\(thiscall\)\))?}} __restrict': different qualifiers (none vs 
restrict)}}
-X = ::CV; // expected-error-re{{assigning to 'void 
(test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from 
incompatible type 'void (test1::Qualifiers::*)(){{( 
__attribute__\(\(thiscall\)\))?}} const volatile': different qualifiers (none 
vs const and volatile)}}
-X = ::CR; // expected-error-re{{assigning to 'void 
(test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from 
incompatible type 'void (test1::Qualifiers::*)(){{( 
__attribute__\(\(thiscall\)\))?}} const __restrict': different qualifiers (none 
vs const and restrict)}}
-X = ::VR; // expected-error-re{{assigning to 'void 
(test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from 
incompatible type 'void (test1::Qualifiers::*)(){{( 
__attribute__\(\(thiscall\)\))?}} volatile __restrict': different qualifiers 
(none vs volatile and restrict)}}
-X = ::CVR; // expected-error-re{{assigning to 'void 
(test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from 
incompatible type 'void (test1::Qualifiers::*)(){{( 
__attribute__\(\(thiscall\)\))?}} const volatile __restrict': different 
qualifiers (none vs const, volatile, and restrict)}}
+X = ::C; // expected-error-re {{assigning to 'void 
(test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from 
incompatible type 'void (test1::Qualifiers::*)(){{( 
__attribute__\(\(thiscall\)\))?}} const': different qualifiers ('' vs 'const')}}
+X = ::V; // expected-error-re{{assigning to 'void 
(test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from 
incompatible type 'void (test1::Qualifiers::*)(){{( 
__attribute__\(\(thiscall\)\))?}} volatile': different qualifiers ('' vs 
'volatile')}}

Hmm.  I think `''` is acceptable for the unqualified case in this diagnostic 
because at least one of the operands has to be non-empty, but it might be 
better overall to say something like `unqualified` (not in quotes, of course).

I like the general effect of this, though; this is definitely a more readable 
diagnostic in the complex cases than it used to be.


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

https://reviews.llvm.org/D56198



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

[PATCH] D55994: [clangd] Check preceding char when completion triggers on ':' or '>'

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

This looks like a work around LSP imperfection indeed.

Are you going to push for change in LSP? Something like 
`CompletionOptions/triggerCharacters` -> `CompletionOptions/triggerStrings`?

  export interface CompletionOptions {
/**
 * The server provides support to resolve additional
 * information for a completion item.
 */
resolveProvider?: boolean;
  
/**
 * The characters that trigger completion automatically.
 */
triggerCharacters?: string[];
  }

https://microsoft.github.io/language-server-protocol/specification


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55994



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

[PATCH] D56188: Adopt SwiftABIInfo for WebAssembly.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

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


Repository:
  rC Clang

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

https://reviews.llvm.org/D56188



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

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



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

courbet wrote:
> aaron.ballman wrote:
> > courbet wrote:
> > > aaron.ballman wrote:
> > > > Any idea why the `std::` was dropped here?
> > > `NestedNameSpecifier::print()` explicitly does:
> > > 
> > > ```
> > >  PrintingPolicy InnerPolicy(Policy);
> > >  InnerPolicy.SuppressScope = true;
> > > ```
> > > 
> > Ah, good point, but is that a good behavioral change? I slightly prefer 
> > printing the namespace name there -- it will likely be redundant 
> > information most of the time, but when the namespace actually matters, 
> > having it printed could save someone a lot of head scratching.
> > I slightly prefer printing the namespace name there
> 
> I tend to agree, so it's more a trade-off of code complexity vs better 
> diagnostic - I tend to err on the side of simplifying the code :)
> 
> Another option is to add yet another boolean to PrintingPolicy, but I htink 
> this is too narrow a use case.
Heh, I tend to err on the side of helping the user unless the code will be 
truly awful. I agree that another option on PrintingPolicy may not be the best 
approach. Do you know why the namespace is being suppressed in the first place? 
Another option would be to always print the namespace, but I don't know what 
that might regress (if anything).


Repository:
  rC Clang

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

https://reviews.llvm.org/D55932



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

[PATCH] D56188: Adopt SwiftABIInfo for WebAssembly.

[John McCall via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56188



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

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

[Clement Courbet via Phabricator via cfe-commits]

courbet added inline comments.



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

aaron.ballman wrote:
> courbet wrote:
> > aaron.ballman wrote:
> > > Any idea why the `std::` was dropped here?
> > `NestedNameSpecifier::print()` explicitly does:
> > 
> > ```
> >  PrintingPolicy InnerPolicy(Policy);
> >  InnerPolicy.SuppressScope = true;
> > ```
> > 
> Ah, good point, but is that a good behavioral change? I slightly prefer 
> printing the namespace name there -- it will likely be redundant information 
> most of the time, but when the namespace actually matters, having it printed 
> could save someone a lot of head scratching.
> I slightly prefer printing the namespace name there

I tend to agree, so it's more a trade-off of code complexity vs better 
diagnostic - I tend to err on the side of simplifying the code :)

Another option is to add yet another boolean to PrintingPolicy, but I htink 
this is too narrow a use case.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55932



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

[PATCH] D56160: [clang-tidy] modernize-use-trailing-return check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: test/clang-tidy/modernize-use-trailing-return.cpp:2
+// RUN: %check_clang_tidy %s modernize-use-trailing-return %t -- -- --std=c++14
+
+namespace std {

lebedev.ri wrote:
> Missing test coverage:
> * macros
> * is there tests for implicit functions?
> * Out-of-line function with body.
Also:
  * functions with attributes in the type position `int f() [[]];`
  * functions without attributes in the type position `[[]] int f();` and `int 
f [[]] ();`
  * lambdas?
  * Special functions without return types, like constructors and destructors
  * Conversion operators. `struct S {  operator int() const; };`


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56160



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

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Basic/AttrDocs.td:2582
 specified for the subsequent loop. The directive allows vectorization,
-interleaving, and unrolling to be enabled or disabled. Vector width as well
-as interleave and unrolling count can be manually specified. See
-`language extensions
+interleaving, and unrolling to be enabled or disabled. Pipelining could be 
disabled.
+Vector width, interleave count, unrolling count, and the initiation interval 
for pipelining

alexey.lapshin wrote:
> aaron.ballman wrote:
> > Can you combine the new sentence with the previous one? `The directive 
> > allows vectorization, interleaving, pipelining, and unrolling to be enabled 
> > or disabled.`
> In that case it would change a meaning and become incorrect. Pipelining could 
> only be disabled. It could not be enabled.
Ah, yes, that would change the meaning then. I'd go with: `The directive allows 
pipelining to be disabled, or vectorization, interleaving, and unrolling to be 
enabled or disabled.`



Comment at: include/clang/Basic/AttrDocs.td:2663
+  could be used as hints for the software pipelining optimization. The pragma 
is
+  placed immediately before a for, while, do-while, or a C++11 range-based for
+  loop.

I'd like to see a Sema test where the pragma is put at global scope to see if 
the diagnostic is reasonable or not. e.g.,
```
#pragma clang loop pipeline(disable)

int main() {
  for (int i = 0; i < 10; ++i)
;
}
```



Comment at: include/clang/Basic/AttrDocs.td:2680-2681
+  the specified cycle count. If a schedule was not found then loop
+  remains unchanged. The initiation interval must be a positive number
+  greater than zero:
+

Please add a Sema test with a negative number as the initiation interval.



Comment at: include/clang/Basic/DiagnosticParseKinds.td:1188
+  "vectorize_width, interleave, interleave_count, unroll, unroll_count, "
+  "pipeline, pipeline_initiation_interval or distribute">;
 

pipeline_initiation_interval or distribute -> pipeline_initiation_interval, or 
distribute

(Keeps the Oxford comma as in the original diagnostic.)



Comment at: test/Parser/pragma-pipeline.cpp:28
+/* expected-error {{invalid argument of type 'double'; expected an integer 
type}} */ #pragma clang loop pipeline_initiation_interval(1.0)
+/* expected-error {{invalid value '0'; must be positive}} */ #pragma clang 
loop pipeline_initiation_interval(0)
+  for (int i = 0; i < Length; i++) {

This isn't really a parser test -- it should probably be in Sema instead.



Comment at: test/Parser/pragma-pipeline.cpp:35-47
+#pragma clang loop pipeline(disable) 
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma 
clang loop'}} */ int j = Length;
+#pragma clang loop pipeline_initiation_interval(4)
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma 
clang loop'}} */ int k = Length;
+
+#pragma clang loop pipeline(disable)
+#pragma clang loop pipeline_initiation_interval(4) /* expected-error 
{{incompatible directives 'pipeline(disable)' and 
'pipeline_initiation_interval(4)'}} */

These should also move to Sema.


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

https://reviews.llvm.org/D55710



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

[PATCH] D55994: [clangd] Check preceding char when completion triggers on ':' or '>'

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: clangd/ClangdLSPServer.cpp:811
+  if (Trigger == ">")
+return (*Code)[*Offset - 2] != '-'; // trigger only on '->'.
+  if (Trigger == ":")

ilya-biryukov wrote:
> hokein wrote:
> > Checking `Offset` is not always right for rare cases like 
> > (`bar:/*commment*/:`), a robust way is to use lexer and get the token at 
> > the current position, but I don't think it is worth (these rare cases 
> > should not happen when people write code). Maybe add a comment documenting 
> > the limitation?
> Done. Note that `bar:/*comment*/:` is not actually a qualifier, i.e. a single 
> token cannot be split in half by the comment.
> But the comment is totally valid: use the lexer would allow to filter out 
> more things, e.g. we don't want to auto-trigger inside a comment, etc.
Yeah, you are right `a single token cannot be split in half by the comment`. 
Thanks for the clarification.



Comment at: test/clangd/completion-auto-trigger.test:13
+---
+{"jsonrpc":"2.0","id":2,"method":"textDocument/completion","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":3,"character":5},"context":{"triggerKind":2,"triggerCharacter":">"}}}
+#  CHECK:  "id": 2,

nit: would be nicer if we add a comment illustrating what this request tests 
for. For this case, it is `a->^`, IIUC.



Comment at: test/clangd/completion-auto-trigger.test:26
+# CHECK-NEXT:"label": " size",
+# CHECK-NEXT:"sortText": "3eacsize",
+# CHECK-NEXT:"textEdit": {

`sortText` is implementation details, maybe use `{{.*}}size`, the same below.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55994



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

[PATCH] D55363: [clangd] Expose FileStatus in LSP.

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

In D55363#1337376 , @hokein wrote:

> In D55363#1336315 , @ilya-biryukov 
> wrote:
>
> > Maybe consider sending an update after some period of time, e.g. `0.5s`? (I 
> > expect this particular strategy to be a bit complicated and out of scope of 
> > this patch, so another alternative is to simply not send them to the 
> > clients in the first version). WDYT?
>
>
> Sounds fair, keeping status bar  `blinking` seems annony to users. Added a 
> FIXME.


I am a bit late to the discussion but isn't this something to be dealt with on 
the client's side?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55363



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

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



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

courbet wrote:
> aaron.ballman wrote:
> > Any idea why the `std::` was dropped here?
> `NestedNameSpecifier::print()` explicitly does:
> 
> ```
>  PrintingPolicy InnerPolicy(Policy);
>  InnerPolicy.SuppressScope = true;
> ```
> 
Ah, good point, but is that a good behavioral change? I slightly prefer 
printing the namespace name there -- it will likely be redundant information 
most of the time, but when the namespace actually matters, having it printed 
could save someone a lot of head scratching.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55932



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

[PATCH] D55994: [clangd] Check preceding char when completion triggers on ':' or '>'

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 179844.
ilya-biryukov marked 3 inline comments as done.
ilya-biryukov added a comment.

- Address comments
- Added a test


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55994

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  test/clangd/completion-auto-trigger.test

Index: test/clangd/completion-auto-trigger.test
===
--- /dev/null
+++ test/clangd/completion-auto-trigger.test
@@ -0,0 +1,106 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"namespace ns { int ns_member; } struct vector { int size; static int default_capacity; };\nvoid test(vector *a, vector *b) {\n  if (a > b) {} \n  a->size = 10;\n\n  a ? a : b;\n  ns::ns_member = 10;\n}"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":2,"character":9},"context":{"triggerKind":2,"triggerCharacter":">"}}}
+#  CHECK:  "error": {
+# CHECK-NEXT:"code": -32001,
+# CHECK-NEXT:"message": "ignored auto-triggered completion, preceding char did not match"
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "id": 1,
+---
+{"jsonrpc":"2.0","id":2,"method":"textDocument/completion","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":3,"character":5},"context":{"triggerKind":2,"triggerCharacter":">"}}}
+#  CHECK:  "id": 2,
+# CHECK-NEXT:  "jsonrpc": "2.0"
+# CHECK-NEXT:  "result": {
+# CHECK-NEXT:"isIncomplete": false,
+# CHECK-NEXT:"items": [
+# CHECK-NEXT:   {
+# CHECK-NEXT:"detail": "int",
+# CHECK-NEXT:"filterText": "size",
+# CHECK-NEXT:"insertText": "size",
+# CHECK-NEXT:"insertTextFormat": 1,
+# CHECK-NEXT:"kind": 5,
+# CHECK-NEXT:"label": " size",
+# CHECK-NEXT:"sortText": "3eacsize",
+# CHECK-NEXT:"textEdit": {
+# CHECK-NEXT:  "newText": "size",
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 5,
+# CHECK-NEXT:  "line": 3
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT:  "character": 5,
+# CHECK-NEXT:  "line": 3
+# CHECK-NEXT:}
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  {
+# CHECK-NEXT: "detail": "int",
+# CHECK-NEXT: "filterText": "default_capacity",
+# CHECK-NEXT: "insertText": "default_capacity",
+# CHECK-NEXT: "insertTextFormat": 1,
+# CHECK-NEXT: "kind": 10,
+# CHECK-NEXT: "label": " default_capacity",
+# CHECK-NEXT: "sortText": "3fd70a3ddefault_capacity",
+# CHECK-NEXT: "textEdit": {
+# CHECK-NEXT:   "newText": "default_capacity",
+# CHECK-NEXT:   "range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT:   "character": 5,
+# CHECK-NEXT:   "line": 3
+# CHECK-NEXT: },
+# CHECK-NEXT: "start": {
+# CHECK-NEXT:   "character": 5,
+# CHECK-NEXT:   "line": 3
+# CHECK-NEXT: }
+# CHECK-NEXT:   }
+# CHECK-NEXT: }
+# CHECK-NEXT:   }
+# CHECK-NEXT: ]
+# CHECK-NEXT:   }
+---
+{"jsonrpc":"2.0","id":3,"method":"textDocument/completion","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":5,"character":9},"context":{"triggerKind":2,"triggerCharacter":":"}}}
+#  CHECK:  "error": {
+# CHECK-NEXT:"code": -32001,
+# CHECK-NEXT:"message": "ignored auto-triggered completion, preceding char did not match"
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "id": 3,
+---
+{"jsonrpc":"2.0","id":4,"method":"textDocument/completion","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":6,"character":6},"context":{"triggerKind":2,"triggerCharacter":":"}}}
+---
+#  CHECK:  "id": 4,
+# CHECK-NEXT:  "jsonrpc": "2.0"
+# CHECK-NEXT:  "result": {
+# CHECK-NEXT:"isIncomplete": false,
+# CHECK-NEXT:"items": [
+# CHECK-NEXT:  {
+# CHECK-NEXT:"detail": "int",
+# CHECK-NEXT:"filterText": "ns_member",
+# CHECK-NEXT:"insertText": "ns_member",
+# CHECK-NEXT:"insertTextFormat": 1,
+# CHECK-NEXT:"kind": 6,
+# CHECK-NEXT:"label": " ns_member",
+# CHECK-NEXT:"sortText": "3f2cns_member",
+# CHECK-NEXT:"textEdit": {
+# CHECK-NEXT:  "newText": "ns_member",
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 6,
+# CHECK-NEXT:

[PATCH] D55994: [clangd] Check preceding char when completion triggers on ':' or '>'

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/ClangdLSPServer.cpp:811
+  if (Trigger == ">")
+return (*Code)[*Offset - 2] != '-'; // trigger only on '->'.
+  if (Trigger == ":")

hokein wrote:
> Checking `Offset` is not always right for rare cases like 
> (`bar:/*commment*/:`), a robust way is to use lexer and get the token at the 
> current position, but I don't think it is worth (these rare cases should not 
> happen when people write code). Maybe add a comment documenting the 
> limitation?
Done. Note that `bar:/*comment*/:` is not actually a qualifier, i.e. a single 
token cannot be split in half by the comment.
But the comment is totally valid: use the lexer would allow to filter out more 
things, e.g. we don't want to auto-trigger inside a comment, etc.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55994



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

[PATCH] D53023: Prototype OpenCL BIFs using Tablegen

[Nicola Zaghen via Phabricator via cfe-commits]

Nicola added a comment.

Since this review is just for a prototype, I'll leave some feedback after 
having played with it for a bit.
It seems to be quite trivial to add new builtins and fix existing ones 
(conversion builtins sat/rounding modes). One builtin that might have a small 
issue is the as_type builtin in which source and destination type need to have 
the same bitwidth. To get that to work I ended up patching TableGen to add !mul 
support for it (and set VectorSize to 1 for scalars).


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

https://reviews.llvm.org/D53023



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

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

[Alexey Lapshin via Phabricator via cfe-commits]

alexey.lapshin updated this revision to Diff 179843.
alexey.lapshin added a comment.

Thank you. addressed all grammar things. please check it once more.


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

https://reviews.llvm.org/D55710

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticParseKinds.td
  lib/CodeGen/CGLoopInfo.cpp
  lib/CodeGen/CGLoopInfo.h
  lib/Parse/ParsePragma.cpp
  lib/Sema/SemaStmtAttr.cpp
  test/CodeGenCXX/pragma-pipeline.cpp
  test/Parser/pragma-loop.cpp
  test/Parser/pragma-pipeline.cpp
  test/Parser/pragma-unroll-and-jam.cpp

Index: test/Parser/pragma-unroll-and-jam.cpp
===
--- test/Parser/pragma-unroll-and-jam.cpp
+++ test/Parser/pragma-unroll-and-jam.cpp
@@ -67,7 +67,7 @@
   }
 
 // pragma clang unroll_and_jam is disabled for the moment
-/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, or distribute}} */ #pragma clang loop unroll_and_jam(4)
+/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval or distribute}} */ #pragma clang loop unroll_and_jam(4)
   for (int i = 0; i < Length; i++) {
 for (int j = 0; j < Length; j++) {
   List[i * Length + j] = Value;
Index: test/Parser/pragma-pipeline.cpp
===
--- /dev/null
+++ test/Parser/pragma-pipeline.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+
+// Note that this puts the expected lines before the directives to work around
+// limitations in the -verify mode.
+
+void test(int *List, int Length, int Value) {
+  int i = 0;
+
+#pragma clang loop pipeline(disable)
+  for (int i = 0; i < Length; i++) {
+List[i] = Value;
+  }
+
+#pragma clang loop pipeline_initiation_interval(10)
+  for (int i = 0; i < Length; i++) {
+List[i] = Value;
+  }
+
+/* expected-error {{expected ')'}} */ #pragma clang loop pipeline(disable
+/* expected-error {{invalid argument; expected 'disable'}} */ #pragma clang loop pipeline(enable)
+/* expected-error {{invalid argument; expected 'disable'}} */ #pragma clang loop pipeline(error)
+/* expected-error {{expected '('}} */ #pragma clang loop pipeline disable
+/* expected-error {{missing argument; expected an integer value}} */ #pragma clang loop pipeline_initiation_interval()
+/* expected-error {{use of undeclared identifier 'error'}} */ #pragma clang loop pipeline_initiation_interval(error)
+/* expected-error {{expected '('}} */ #pragma clang loop pipeline_initiation_interval 1 2
+/* expected-error {{expected ')'}} */ #pragma clang loop pipeline_initiation_interval(1
+/* expected-error {{invalid argument of type 'double'; expected an integer type}} */ #pragma clang loop pipeline_initiation_interval(1.0)
+/* expected-error {{invalid value '0'; must be positive}} */ #pragma clang loop pipeline_initiation_interval(0)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma clang loop pipeline(disable) 
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma clang loop'}} */ int j = Length;
+#pragma clang loop pipeline_initiation_interval(4)
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma clang loop'}} */ int k = Length;
+
+#pragma clang loop pipeline(disable)
+#pragma clang loop pipeline_initiation_interval(4) /* expected-error {{incompatible directives 'pipeline(disable)' and 'pipeline_initiation_interval(4)'}} */
+  for (int i = 0; i < Length; i++) {
+List[i] = Value;
+  }
+
+#pragma clang loop pipeline(disable)
+/* expected-error {{expected statement}} */ }
Index: test/Parser/pragma-loop.cpp
===
--- test/Parser/pragma-loop.cpp
+++ test/Parser/pragma-loop.cpp
@@ -147,7 +147,7 @@
 /* expected-error {{missing argument; expected 'enable', 'full' or 'disable'}} */ #pragma clang loop unroll()
 /* expected-error {{missing argument; expected 'enable' or 'disable'}} */ #pragma clang loop distribute()
 
-/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, or distribute}} */ #pragma clang loop
+/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval or distribute}} */ #pragma clang loop
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword(enable)
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop vectorize(enable) badkeyword(4)
Index: test/CodeGenCXX/pragma-pipeline.cpp

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

[Alexey Lapshin via Phabricator via cfe-commits]

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



Comment at: include/clang/Basic/AttrDocs.td:2582
 specified for the subsequent loop. The directive allows vectorization,
-interleaving, and unrolling to be enabled or disabled. Vector width as well
-as interleave and unrolling count can be manually specified. See
-`language extensions
+interleaving, and unrolling to be enabled or disabled. Pipelining could be 
disabled.
+Vector width, interleave count, unrolling count, and the initiation interval 
for pipelining

aaron.ballman wrote:
> Can you combine the new sentence with the previous one? `The directive allows 
> vectorization, interleaving, pipelining, and unrolling to be enabled or 
> disabled.`
In that case it would change a meaning and become incorrect. Pipelining could 
only be disabled. It could not be enabled.


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

https://reviews.llvm.org/D55710



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

[PATCH] D56157: [sanitizer_common] Implement popen, popenve, pclose interceptors

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

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



Comment at: lib/esan/esan_interceptors.cpp:90
   } while (false)
+#define COMMON_INTERCEPTOR_PIPE_OPEN(ctx, file)
\
+  do { 
\

krytarowski wrote:
> dvyukov wrote:
> > The idea behind defining a no-op version in 
> > sanitizer_common_interceptors.inc was exactly that tools that are not 
> > interested in it does not need to be changed. So please remove this.
> I would add an intermediate revision to drop redefinitions in this file.
@krytarowski, apparently it's not that simple. I've tried removing all the 
empty definitions and now I get a lot of build errors ;-). It would probably be 
necessary to take them one by one to establish which one are unnecessary after 
all, and I don't think we have time for this right now.


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

https://reviews.llvm.org/D56157



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

[PATCH] D56157: [sanitizer_common] Implement popen, popenve, pclose interceptors

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

mgorny updated this revision to Diff 179842.
mgorny added a comment.

Updated to use `COMMON_INTERCEPTOR_FILE_OPEN` with `nullptr` argument.


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

https://reviews.llvm.org/D56157

Files:
  lib/sanitizer_common/sanitizer_common_interceptors.inc
  lib/sanitizer_common/sanitizer_platform_interceptors.h
  lib/tsan/rtl/tsan_interceptors.cc

Index: lib/tsan/rtl/tsan_interceptors.cc
===
--- lib/tsan/rtl/tsan_interceptors.cc
+++ lib/tsan/rtl/tsan_interceptors.cc
@@ -2250,7 +2250,8 @@
   (void) ctx;
 
 #define COMMON_INTERCEPTOR_FILE_OPEN(ctx, file, path) \
-  Acquire(thr, pc, File2addr(path));  \
+  if (path)   \
+Acquire(thr, pc, File2addr(path));\
   if (file) { \
 int fd = fileno_unlocked(file);   \
 if (fd >= 0) FdFileCreate(thr, pc, fd);   \
Index: lib/sanitizer_common/sanitizer_platform_interceptors.h
===
--- lib/sanitizer_common/sanitizer_platform_interceptors.h
+++ lib/sanitizer_common/sanitizer_platform_interceptors.h
@@ -546,4 +546,8 @@
 #define SANITIZER_INTERCEPT_CDB SI_NETBSD
 #define SANITIZER_INTERCEPT_VIS (SI_NETBSD || SI_FREEBSD)
 
+#define SANITIZER_INTERCEPT_POPEN SI_POSIX
+#define SANITIZER_INTERCEPT_POPENVE SI_NETBSD
+#define SANITIZER_INTERCEPT_PCLOSE SI_POSIX
+
 #endif  // #ifndef SANITIZER_PLATFORM_INTERCEPTORS_H
Index: lib/sanitizer_common/sanitizer_common_interceptors.inc
===
--- lib/sanitizer_common/sanitizer_common_interceptors.inc
+++ lib/sanitizer_common/sanitizer_common_interceptors.inc
@@ -9058,6 +9058,77 @@
 #define INIT_CDB
 #endif
 
+#if SANITIZER_INTERCEPT_POPEN
+INTERCEPTOR(__sanitizer_FILE *, popen, const char *command, const char *type) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, popen, command, type);
+  if (command)
+COMMON_INTERCEPTOR_READ_RANGE(ctx, command, REAL(strlen)(command) + 1);
+  if (type)
+COMMON_INTERCEPTOR_READ_RANGE(ctx, type, REAL(strlen)(type) + 1);
+  __sanitizer_FILE *res = REAL(popen)(command, type);
+  COMMON_INTERCEPTOR_FILE_OPEN(ctx, res, nullptr);
+  if (res) unpoison_file(res);
+  return res;
+}
+#define INIT_POPEN COMMON_INTERCEPT_FUNCTION(popen)
+#else
+#define INIT_POPEN
+#endif
+
+#if SANITIZER_INTERCEPT_POPENVE
+INTERCEPTOR(__sanitizer_FILE *, popenve, const char *path,
+char *const *argv, char *const *envp, const char *type) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, popenve, path, argv, envp, type);
+  if (path)
+COMMON_INTERCEPTOR_READ_RANGE(ctx, path, REAL(strlen)(path) + 1);
+  if (argv) {
+for (char *const *pa = argv; ; ++pa) {
+  COMMON_INTERCEPTOR_READ_RANGE(ctx, pa, sizeof(char **));
+  if (!*pa)
+break;
+  COMMON_INTERCEPTOR_READ_RANGE(ctx, *pa, REAL(strlen)(*pa) + 1);
+}
+  }
+  if (envp) {
+for (char *const *pa = envp; ; ++pa) {
+  COMMON_INTERCEPTOR_READ_RANGE(ctx, pa, sizeof(char **));
+  if (!*pa)
+break;
+  COMMON_INTERCEPTOR_READ_RANGE(ctx, *pa, REAL(strlen)(*pa) + 1);
+}
+  }
+  if (type)
+COMMON_INTERCEPTOR_READ_RANGE(ctx, type, REAL(strlen)(type) + 1);
+  __sanitizer_FILE *res = REAL(popenve)(path, argv, envp, type);
+  COMMON_INTERCEPTOR_FILE_OPEN(ctx, res, nullptr);
+  if (res) unpoison_file(res);
+  return res;
+}
+#define INIT_POPENVE COMMON_INTERCEPT_FUNCTION(popenve)
+#else
+#define INIT_POPENVE
+#endif
+
+#if SANITIZER_INTERCEPT_PCLOSE
+INTERCEPTOR(int, pclose, __sanitizer_FILE *fp) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, pclose, fp);
+  COMMON_INTERCEPTOR_FILE_CLOSE(ctx, fp);
+  const FileMetadata *m = GetInterceptorMetadata(fp);
+  int res = REAL(pclose)(fp);
+  if (m) {
+COMMON_INTERCEPTOR_INITIALIZE_RANGE(*m->addr, *m->size);
+DeleteInterceptorMetadata(fp);
+  }
+  return res;
+}
+#define INIT_PCLOSE COMMON_INTERCEPT_FUNCTION(pclose);
+#else
+#define INIT_PCLOSE
+#endif
+
 static void InitializeCommonInterceptors() {
   static u64 metadata_mem[sizeof(MetadataHashMap) / sizeof(u64) + 1];
   interceptor_metadata_map =
@@ -9338,6 +9409,9 @@
   INIT_SHA2;
   INIT_VIS;
   INIT_CDB;
+  INIT_POPEN;
+  INIT_POPENVE;
+  INIT_PCLOSE;
 
   INIT___PRINTF_CHK;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56157: [sanitizer_common] Implement popen, popenve, pclose interceptors

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

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

The tests were submitted earlier as D56153 . 
I'll implement the remaining suggestions.


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D56157



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

[PATCH] D56157: [sanitizer_common] Implement popen, popenve, pclose interceptors

[Dmitry Vyukov via Phabricator via cfe-commits]

dvyukov added inline comments.



Comment at: lib/tsan/rtl/tsan_interceptors.cc:2259
 
+#define COMMON_INTERCEPTOR_PIPE_OPEN(ctx, file) \
+  if (file) {   \

krytarowski wrote:
> dvyukov wrote:
> > An alternative would be to pass NULL to COMMON_INTERCEPTOR_FILE_OPEN and 
> > then do:
> > 
> >   if (path)
> > Acquire(thr, pc, File2addr(path))
> > 
> > Just to not multiply entities. But neither approach looks strictly better 
> > to me, so I am not too strong about it.
> Is `File2addr()` producing any useful result?
> 
> Reusing `COMMON_INTERCEPTOR_FILE_OPEN` looks fine.
> Is File2addr() producing any useful result?

What exactly do you mean?
It produces a degenerate, but useful result. The original idea was that it can 
return different results for different files, depending on io_sync flag value. 
E.g. not synchronizing deletion of one file with open of another file. But this 
gets tricky with links, bind mounts, etc and was never implemented.
But at least if a file is not involved at all, then we need to skip the Acquire.


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D56157



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

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

[Anastasia Stulova via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaOverload.cpp:2828
 
   // FIXME: OpenCL: Need to consider address spaces
   unsigned FromQuals = FromFunction->getTypeQuals().getCVRUQualifiers();

rjmccall wrote:
> Anastasia wrote:
> > I am still missing something here.
> Well, at least the failure here is just to fall into the generic diagnostic.
> 
> Getting this diagnostic right probably requires some minor work to the 
> diagnostics engine.  If you look at `err_init_conversion_failed`, which is (I 
> think) the diagnostic that's always being used here, it matches every 
> possible CVR mask so that it can pretty-print them.  This is already a 
> problem because the input is actually a CVRU mask!  A better option would be 
> to teach `DiagnosticEngine` how to store and format a `Qualifiers` value, and 
> then you can just stream the original `Qualifiers` into the diagnostic here.
> 
> But that's obviously work for a separate patch.
I created a patch for this in https://reviews.llvm.org/D56198.


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

https://reviews.llvm.org/D55850



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

[PATCH] D56198: [Basic] Extend DiagnosticEngine to store and format Qualifiers

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.
Anastasia added a reviewer: rjmccall.

This is a follow up patch from the following review comment: 
https://reviews.llvm.org/D55850?id=178767#inline-495051


https://reviews.llvm.org/D56198

Files:
  include/clang/AST/Type.h
  include/clang/Basic/Diagnostic.h
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/AST/ASTDiagnostic.cpp
  lib/Basic/Diagnostic.cpp
  lib/Sema/SemaOverload.cpp
  test/SemaCXX/addr-of-overloaded-function.cpp
  test/SemaCXX/warn-overloaded-virtual.cpp

Index: test/SemaCXX/warn-overloaded-virtual.cpp
===
--- test/SemaCXX/warn-overloaded-virtual.cpp
+++ test/SemaCXX/warn-overloaded-virtual.cpp
@@ -130,7 +130,7 @@
 virtual int foo(int*) const;
 // expected-note@-1{{type mismatch at 1st parameter ('int *' vs 'int')}}
 virtual int foo(int) volatile;
-// expected-note@-1{{different qualifiers (volatile vs const)}}
+// expected-note@-1{{different qualifiers ('volatile' vs 'const')}}
   };
 
   class B : public A {
Index: test/SemaCXX/addr-of-overloaded-function.cpp
===
--- test/SemaCXX/addr-of-overloaded-function.cpp
+++ test/SemaCXX/addr-of-overloaded-function.cpp
@@ -221,13 +221,13 @@
 
   void QualifierTest() {
 void (Qualifiers::*X)();
-X = ::C; // expected-error-re {{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} const': different qualifiers (none vs const)}}
-X = ::V; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} volatile': different qualifiers (none vs volatile)}}
-X = ::R; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} __restrict': different qualifiers (none vs restrict)}}
-X = ::CV; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} const volatile': different qualifiers (none vs const and volatile)}}
-X = ::CR; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} const __restrict': different qualifiers (none vs const and restrict)}}
-X = ::VR; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} volatile __restrict': different qualifiers (none vs volatile and restrict)}}
-X = ::CVR; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} const volatile __restrict': different qualifiers (none vs const, volatile, and restrict)}}
+X = ::C; // expected-error-re {{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} const': different qualifiers ('' vs 'const')}}
+X = ::V; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} volatile': different qualifiers ('' vs 'volatile')}}
+X = ::R; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} __restrict': different qualifiers ('' vs '__restrict')}}
+X = ::CV; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} const volatile': different qualifiers ('' vs 'const volatile')}}
+X = ::CR; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} const __restrict': different qualifiers ('' vs 'const __restrict')}}
+X = ::VR; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} volatile __restrict': different qualifiers ('' vs 'volatile __restrict')}}
+X = ::CVR; // expected-error-re{{assigning to 'void 

[PATCH] D55374: [clangd] Show FileStatus in vscode-clangd.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL350210: [clangd] Show FileStatus in vscode-clangd. (authored 
by hokein, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55374

Files:
  clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts


Index: clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
===
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
@@ -18,6 +18,33 @@
  void, void>('textDocument/switchSourceHeader');
 }
 
+class FileStatus {
+private statuses = new Map();
+private readonly statusBarItem =
+vscode.window.createStatusBarItem(vscode.StatusBarAlignment.Left, 10);
+
+onFileUpdated(fileStatus: any) {
+const filePath = vscode.Uri.parse(fileStatus.uri);
+this.statuses.set(filePath.fsPath, fileStatus);
+this.updateStatus();
+}
+
+updateStatus() {
+const path = vscode.window.activeTextEditor.document.fileName;
+const status = this.statuses.get(path);
+if (!status) {
+  this.statusBarItem.hide();
+  return;
+}
+this.statusBarItem.text = `clangd: ` + status.state;
+this.statusBarItem.show();
+}
+
+dispose() {
+this.statusBarItem.dispose();
+}
+}
+
 /**
  *  this method is called when your extension is activate
  *  your extension is activated the very first time the command is executed
@@ -44,6 +71,7 @@
 synchronize: !syncFileEvents ? undefined : {
 fileEvents: vscode.workspace.createFileSystemWatcher(filePattern)
 },
+initializationOptions: { clangdFileStatus: true },
 // Resolve symlinks for all files provided by clangd.
 // This is a workaround for a bazel + clangd issue - bazel produces a 
symlink tree to build in,
 // and when navigating to the included file, clangd passes its path 
inside the symlink tree
@@ -80,4 +108,13 @@
 vscode.Uri.parse(sourceUri));
 vscode.window.showTextDocument(doc);
   }));
+const status = new FileStatus();
+context.subscriptions.push(vscode.window.onDidChangeActiveTextEditor(() => 
{
+status.updateStatus();
+}));
+clangdClient.onReady().then(() => {
+clangdClient.onNotification(
+'textDocument/clangd.fileStatus',
+(fileStatus) => { status.onFileUpdated(fileStatus); });
+})
 }


Index: clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
===
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
@@ -18,6 +18,33 @@
  void, void>('textDocument/switchSourceHeader');
 }
 
+class FileStatus {
+private statuses = new Map();
+private readonly statusBarItem =
+vscode.window.createStatusBarItem(vscode.StatusBarAlignment.Left, 10);
+
+onFileUpdated(fileStatus: any) {
+const filePath = vscode.Uri.parse(fileStatus.uri);
+this.statuses.set(filePath.fsPath, fileStatus);
+this.updateStatus();
+}
+
+updateStatus() {
+const path = vscode.window.activeTextEditor.document.fileName;
+const status = this.statuses.get(path);
+if (!status) {
+  this.statusBarItem.hide();
+  return;
+}
+this.statusBarItem.text = `clangd: ` + status.state;
+this.statusBarItem.show();
+}
+
+dispose() {
+this.statusBarItem.dispose();
+}
+}
+
 /**
  *  this method is called when your extension is activate
  *  your extension is activated the very first time the command is executed
@@ -44,6 +71,7 @@
 synchronize: !syncFileEvents ? undefined : {
 fileEvents: vscode.workspace.createFileSystemWatcher(filePattern)
 },
+initializationOptions: { clangdFileStatus: true },
 // Resolve symlinks for all files provided by clangd.
 // This is a workaround for a bazel + clangd issue - bazel produces a symlink tree to build in,
 // and when navigating to the included file, clangd passes its path inside the symlink tree
@@ -80,4 +108,13 @@
 vscode.Uri.parse(sourceUri));
 vscode.window.showTextDocument(doc);
   }));
+const status = new FileStatus();
+context.subscriptions.push(vscode.window.onDidChangeActiveTextEditor(() => {
+status.updateStatus();
+}));
+clangdClient.onReady().then(() => {
+clangdClient.onNotification(
+'textDocument/clangd.fileStatus',
+(fileStatus) => { 

[clang-tools-extra] r350210 - [clangd] Show FileStatus in vscode-clangd.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Wed Jan  2 03:25:37 2019
New Revision: 350210

URL: http://llvm.org/viewvc/llvm-project?rev=350210=rev
Log:
[clangd] Show FileStatus in vscode-clangd.

Summary:
The file status will be shown in the status bar.
Depends on D55363.

Reviewers: ilya-biryukov

Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts

Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts?rev=350210=350209=350210=diff
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts 
(original)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts Wed 
Jan  2 03:25:37 2019
@@ -18,6 +18,33 @@ export const type =
  void, void>('textDocument/switchSourceHeader');
 }
 
+class FileStatus {
+private statuses = new Map();
+private readonly statusBarItem =
+vscode.window.createStatusBarItem(vscode.StatusBarAlignment.Left, 10);
+
+onFileUpdated(fileStatus: any) {
+const filePath = vscode.Uri.parse(fileStatus.uri);
+this.statuses.set(filePath.fsPath, fileStatus);
+this.updateStatus();
+}
+
+updateStatus() {
+const path = vscode.window.activeTextEditor.document.fileName;
+const status = this.statuses.get(path);
+if (!status) {
+  this.statusBarItem.hide();
+  return;
+}
+this.statusBarItem.text = `clangd: ` + status.state;
+this.statusBarItem.show();
+}
+
+dispose() {
+this.statusBarItem.dispose();
+}
+}
+
 /**
  *  this method is called when your extension is activate
  *  your extension is activated the very first time the command is executed
@@ -44,6 +71,7 @@ export function activate(context: vscode
 synchronize: !syncFileEvents ? undefined : {
 fileEvents: vscode.workspace.createFileSystemWatcher(filePattern)
 },
+initializationOptions: { clangdFileStatus: true },
 // Resolve symlinks for all files provided by clangd.
 // This is a workaround for a bazel + clangd issue - bazel produces a 
symlink tree to build in,
 // and when navigating to the included file, clangd passes its path 
inside the symlink tree
@@ -80,4 +108,13 @@ export function activate(context: vscode
 vscode.Uri.parse(sourceUri));
 vscode.window.showTextDocument(doc);
   }));
+const status = new FileStatus();
+context.subscriptions.push(vscode.window.onDidChangeActiveTextEditor(() => 
{
+status.updateStatus();
+}));
+clangdClient.onReady().then(() => {
+clangdClient.onNotification(
+'textDocument/clangd.fileStatus',
+(fileStatus) => { status.onFileUpdated(fileStatus); });
+})
 }


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

[PATCH] D55994: [clangd] Check preceding char when completion triggers on ':' or '>'

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

The code looks mostly good to me.




Comment at: clangd/ClangdLSPServer.cpp:789
 
+bool ClangdLSPServer::isSporadicCompletion(
+const CompletionParams ) const {

The method name is a bit confusing to me, I didn't get its meaning at the first 
glance and without reading its comment. Maybe `shouldRunCompletion`?



Comment at: clangd/ClangdLSPServer.cpp:811
+  if (Trigger == ">")
+return (*Code)[*Offset - 2] != '-'; // trigger only on '->'.
+  if (Trigger == ":")

Checking `Offset` is not always right for rare cases like 
(`bar:/*commment*/:`), a robust way is to use lexer and get the token at the 
current position, but I don't think it is worth (these rare cases should not 
happen when people write code). Maybe add a comment documenting the limitation?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55994



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

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

[Clement Courbet via Phabricator via cfe-commits]

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



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

aaron.ballman wrote:
> Any idea why the `std::` was dropped here?
`NestedNameSpecifier::print()` explicitly does:

```
 PrintingPolicy InnerPolicy(Policy);
 InnerPolicy.SuppressScope = true;
```



Repository:
  rC Clang

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

https://reviews.llvm.org/D55932



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