[PATCH] D44882: [clangd] Implementation of workspace/symbol request

2018-04-01 Thread Marc-Andre Laperle via Phabricator via cfe-commits 
malaperle marked an inline comment as done.
malaperle added inline comments.



Comment at: clangd/FindSymbols.cpp:31
+
+  if (supportedSymbolKinds &&
+  std::find(supportedSymbolKinds->begin(), supportedSymbolKinds->end(),

MaskRay wrote:
> MaskRay wrote:
> > malaperle wrote:
> > > MaskRay wrote:
> > > > This std::find loop adds some overhead to each candidate... In my 
> > > > experience the client usually doesn't care about the returned symbol 
> > > > kinds, they are used to give a category name. You can always patch the 
> > > > upstream to add missing categories.
> > > > 
> > > > This is one instance where LSP is over specified. nvm I don't have 
> > > > strong opinion here
> > > I have a client that throws an exception when the symbolkind is not known 
> > > and the whole request fails, so I think it's worth checking. But if it's 
> > > too slow I can look at making it faster. Unfortunately, I cannot patch 
> > > any upstream project :)
> > https://github.com/gluon-lang/languageserver-types/blob/master/src/lib.rs#L2016
> > 
> > LanguageClient-neovim returns empty candidate list if one of the candidates 
> > has unknown SymbolKind. Apparently they should be more tolerant and there 
> > is an issue tracking it.
> If it was not an internal confidential client, I would like to know its name, 
> unless the confidentiality includes the existence of the client.
Sorry, I should have explained a bit more my thought. I see that an older-ish 
version of Monaco is not handling unknown symbols kinds well. I don't really 
know if it's our integration with Monaco or Monaco itself (the IDE is Theia). 
But my thought is that there are clients out there that follow the protocol 
"correctly" by choosing not to handle those unknown symbol kinds gracefully. I 
am not that concerned about a single client in particular, just general support 
of clients out there. As a server, I think it makes sense to support clients 
that follow the protocol, especially since 1.x and 2.x are not that old. I 
don't think it's the crux of the complexity of this patch.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44882



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

[PATCH] D44226: [clangd] Add -log-lsp-to-stderr option

2018-04-01 Thread Marc-Andre Laperle via Phabricator via cfe-commits 
malaperle added a comment.
Herald added a subscriber: MaskRay.

Gentle ping! It would be nice to have this so make Clangd less "verbose".




Comment at: clangd/ClangdLSPServer.cpp:412
+llvm::raw_string_ostream OS(Message);
+OS << "method not found (" << Method << ")";
+replyError(ErrorCode::MethodNotFound, OS.str());

ilya-biryukov wrote:
> Could we also `log` (i.e. not `vlog`) names of the methods that were handled 
> successfully? To have some context when something crashes and we only have 
> non-verbose logs.
Wouldn't that mean pretty much logging everything coming in? The idea of this 
patch it to make it that by default Clangd is not talkative unless there is an 
error and optionally make it verbose, for troubleshooting.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44226



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

Re: r328731 - [ObjC++] Make parameter passing and function return compatible with ObjC

2018-04-01 Thread Richard Smith via cfe-commits 
On 28 March 2018 at 14:13, Akira Hatanaka via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: ahatanak
> Date: Wed Mar 28 14:13:14 2018
> New Revision: 328731
>
> URL: http://llvm.org/viewvc/llvm-project?rev=328731=rev
> Log:
> [ObjC++] Make parameter passing and function return compatible with ObjC
>
> ObjC and ObjC++ pass non-trivial structs in a way that is incompatible
> with each other. For example:
>
> typedef struct {
>   id f0;
>   __weak id f1;
> } S;
>
> // this code is compiled in c++.
> extern "C" {
>   void foo(S s);
> }
>
> void caller() {
>   // the caller passes the parameter indirectly and destructs it.
>   foo(S());
> }
>
> // this function is compiled in c.
> // 'a' is passed directly and is destructed in the callee.
> void foo(S a) {
> }
>
> This patch fixes the incompatibility by passing and returning structs
> with __strong or weak fields using the C ABI in C++ mode. __strong and
> __weak fields in a struct do not cause the struct to be destructed in
> the caller and __strong fields do not cause the struct to be passed
> indirectly.
>
> Also, this patch fixes the microsoft ABI bug mentioned here:
>
> https://reviews.llvm.org/D41039?id=128767#inline-364710
>
> rdar://problem/38887866
>
> Differential Revision: https://reviews.llvm.org/D44908
>
> Added:
> cfe/trunk/test/CodeGenObjCXX/objc-struct-cxx-abi.mm
>   - copied, changed from r328730, cfe/trunk/test/CodeGenObjCXX/t
> rivial_abi.mm
> Removed:
> cfe/trunk/test/CodeGenObjCXX/trivial_abi.mm
> Modified:
> cfe/trunk/include/clang/AST/Decl.h
> cfe/trunk/include/clang/AST/DeclCXX.h
> cfe/trunk/include/clang/AST/Type.h
> cfe/trunk/include/clang/Basic/LangOptions.def
> cfe/trunk/include/clang/Basic/LangOptions.h
> cfe/trunk/include/clang/Basic/TargetInfo.h
> cfe/trunk/include/clang/Frontend/CodeGenOptions.def
> cfe/trunk/lib/AST/ASTContext.cpp
> cfe/trunk/lib/AST/Decl.cpp
> cfe/trunk/lib/AST/DeclCXX.cpp
> cfe/trunk/lib/AST/Type.cpp
> cfe/trunk/lib/Basic/TargetInfo.cpp
> cfe/trunk/lib/Basic/Targets/X86.h
> cfe/trunk/lib/CodeGen/CGCall.cpp
> cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
> cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
> cfe/trunk/lib/CodeGen/TargetInfo.cpp
> cfe/trunk/lib/Frontend/CompilerInvocation.cpp
> cfe/trunk/lib/Sema/SemaDecl.cpp
> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
> cfe/trunk/lib/Serialization/ASTWriter.cpp
> cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
> cfe/trunk/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
> cfe/trunk/test/CodeGenObjCXX/arc-special-member-functions.mm
> cfe/trunk/test/CodeGenObjCXX/property-dot-copy-elision.mm
>
> Modified: cfe/trunk/include/clang/AST/Decl.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/AST/Decl.h?rev=328731=328730=328731=diff
> 
> ==
> --- cfe/trunk/include/clang/AST/Decl.h (original)
> +++ cfe/trunk/include/clang/AST/Decl.h Wed Mar 28 14:13:14 2018
> @@ -3559,6 +3559,11 @@ class RecordDecl : public TagDecl {
>/// pass an object of this class.
>bool CanPassInRegisters : 1;
>
> +  /// Indicates whether this struct is destroyed in the callee. This flag
> is
> +  /// meaningless when Microsoft ABI is used since parameters are always
> +  /// destroyed in the callee.
> +  bool ParamDestroyedInCallee : 1;
> +
>  protected:
>RecordDecl(Kind DK, TagKind TK, const ASTContext , DeclContext *DC,
>   SourceLocation StartLoc, SourceLocation IdLoc,
> @@ -3654,6 +3659,14 @@ public:
>  CanPassInRegisters = CanPass;
>}
>
> +  bool isParamDestroyedInCallee() const {
> +return ParamDestroyedInCallee;
> +  }
> +
> +  void setParamDestroyedInCallee(bool V) {
> +ParamDestroyedInCallee = V;
> +  }
> +
>/// \brief Determines whether this declaration represents the
>/// injected class name.
>///
>
> Modified: cfe/trunk/include/clang/AST/DeclCXX.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/AST/DeclCXX.h?rev=328731=328730=328731=diff
> 
> ==
> --- cfe/trunk/include/clang/AST/DeclCXX.h (original)
> +++ cfe/trunk/include/clang/AST/DeclCXX.h Wed Mar 28 14:13:14 2018
> @@ -1468,13 +1468,6 @@ public:
>  return data().HasIrrelevantDestructor;
>}
>
> -  /// Determine whether the triviality for the purpose of calls for this
> class
> -  /// is overridden to be trivial because this class or the type of one
> of its
> -  /// subobjects has attribute "trivial_abi".
> -  bool hasTrivialABIOverride() const {
> -return canPassInRegisters() && hasNonTrivialDestructor();
> -  }
> -
>/// \brief Determine whether this class has a non-literal or/ volatile
> type
>/// non-static data member or base class.
>bool hasNonLiteralTypeFieldsOrBases() const {
>
> Modified:

r328951 - [Coroutines] Schedule coro-split before asan

2018-04-01 Thread Brian Gesiak via cfe-commits 
Author: modocache
Date: Sun Apr  1 16:55:21 2018
New Revision: 328951

URL: http://llvm.org/viewvc/llvm-project?rev=328951=rev
Log:
[Coroutines] Schedule coro-split before asan

Summary:
The docs for the LLVM coroutines intrinsic `@llvm.coro.id` state that
"The second argument, if not null, designates a particular alloca instruction
to be a coroutine promise."

However, if the address sanitizer pass is run before the `@llvm.coro.id`
intrinsic is lowered, the `alloca` instruction passed to the intrinsic as its
second argument is converted, as per the
https://github.com/google/sanitizers/wiki/AddressSanitizerAlgorithm docs, to
an `inttoptr` instruction that accesses the address of the promise.

On optimization levels `-O1` and above, the `-asan` pass is run after
`-coro-early`, `-coro-split`, and `-coro-elide`, and before
`-coro-cleanup`, and so there is no issue. At `-O0`, however, `-asan`
is run in between `-coro-early` and `-coro-split`, which causes an
assertion to be hit when the `inttoptr` instruction is forcibly cast to
an `alloca`.

Rearrange the passes such that the coroutine passes are registered
before the sanitizer passes.

Test Plan:
Compile a simple C++ program that uses coroutines in `-O0` with
`-fsanitize-address`, and confirm no assertion is hit:
`clang++ coro-example.cpp -fcoroutines-ts -g -fsanitize=address 
-fno-omit-frame-pointer`.

Reviewers: GorNishanov, lewissbaker, EricWF

Reviewed By: GorNishanov

Subscribers: cfe-commits

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

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

Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=328951=328950=328951=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Sun Apr  1 16:55:21 2018
@@ -541,6 +541,9 @@ void EmitAssemblyHelper::CreatePasses(le
addObjCARCOptPass);
   }
 
+  if (LangOpts.CoroutinesTS)
+addCoroutinePassesToExtensionPoints(PMBuilder);
+
   if (LangOpts.Sanitize.has(SanitizerKind::LocalBounds)) {
 PMBuilder.addExtension(PassManagerBuilder::EP_ScalarOptimizerLate,
addBoundsCheckingPass);
@@ -599,9 +602,6 @@ void EmitAssemblyHelper::CreatePasses(le
addDataFlowSanitizerPass);
   }
 
-  if (LangOpts.CoroutinesTS)
-addCoroutinePassesToExtensionPoints(PMBuilder);
-
   if (LangOpts.Sanitize.hasOneOf(SanitizerKind::Efficiency)) {
 PMBuilder.addExtension(PassManagerBuilder::EP_OptimizerLast,
addEfficiencySanitizerPass);


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

[PATCH] D43927: [Coroutines] Schedule coro-split before asan

2018-04-01 Thread Brian Gesiak via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rC328951: [Coroutines] Schedule coro-split before asan 
(authored by modocache, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D43927?vs=136463=140603#toc

Repository:
  rC Clang

https://reviews.llvm.org/D43927

Files:
  lib/CodeGen/BackendUtil.cpp


Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -541,6 +541,9 @@
addObjCARCOptPass);
   }
 
+  if (LangOpts.CoroutinesTS)
+addCoroutinePassesToExtensionPoints(PMBuilder);
+
   if (LangOpts.Sanitize.has(SanitizerKind::LocalBounds)) {
 PMBuilder.addExtension(PassManagerBuilder::EP_ScalarOptimizerLate,
addBoundsCheckingPass);
@@ -599,9 +602,6 @@
addDataFlowSanitizerPass);
   }
 
-  if (LangOpts.CoroutinesTS)
-addCoroutinePassesToExtensionPoints(PMBuilder);
-
   if (LangOpts.Sanitize.hasOneOf(SanitizerKind::Efficiency)) {
 PMBuilder.addExtension(PassManagerBuilder::EP_OptimizerLast,
addEfficiencySanitizerPass);


Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -541,6 +541,9 @@
addObjCARCOptPass);
   }
 
+  if (LangOpts.CoroutinesTS)
+addCoroutinePassesToExtensionPoints(PMBuilder);
+
   if (LangOpts.Sanitize.has(SanitizerKind::LocalBounds)) {
 PMBuilder.addExtension(PassManagerBuilder::EP_ScalarOptimizerLate,
addBoundsCheckingPass);
@@ -599,9 +602,6 @@
addDataFlowSanitizerPass);
   }
 
-  if (LangOpts.CoroutinesTS)
-addCoroutinePassesToExtensionPoints(PMBuilder);
-
   if (LangOpts.Sanitize.hasOneOf(SanitizerKind::Efficiency)) {
 PMBuilder.addExtension(PassManagerBuilder::EP_OptimizerLast,
addEfficiencySanitizerPass);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44552: [Coroutines] Find custom allocators in class scope

2018-04-01 Thread Brian Gesiak via Phabricator via cfe-commits 
modocache marked 2 inline comments as done.
modocache added a comment.

Thanks for the review, @GorNishanov! I adopted your suggestions and made the 
commit.


Repository:
  rC Clang

https://reviews.llvm.org/D44552



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

[PATCH] D44552: [Coroutines] Find custom allocators in class scope

2018-04-01 Thread Brian Gesiak via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rC328949: [Coroutines] Find custom allocators in class scope 
(authored by modocache, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D44552?vs=138663=140600#toc

Repository:
  rC Clang

https://reviews.llvm.org/D44552

Files:
  include/clang/Sema/Sema.h
  lib/Sema/SemaCoroutine.cpp
  lib/Sema/SemaExprCXX.cpp
  test/CodeGenCoroutines/coro-alloc.cpp
  test/SemaCXX/coroutines.cpp

Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -5140,8 +5140,25 @@
 
   bool CheckAllocatedType(QualType AllocType, SourceLocation Loc,
   SourceRange R);
+
+  /// \brief The scope in which to find allocation functions.
+  enum AllocationFunctionScope {
+/// \brief Only look for allocation functions in the global scope.
+AFS_Global,
+/// \brief Only look for allocation functions in the scope of the
+/// allocated class.
+AFS_Class,
+/// \brief Look for allocation functions in both the global scope
+/// and in the scope of the allocated class.
+AFS_Both
+  };
+
+  /// \brief Finds the overloads of operator new and delete that are appropriate
+  /// for the allocation.
   bool FindAllocationFunctions(SourceLocation StartLoc, SourceRange Range,
-   bool UseGlobal, QualType AllocType, bool IsArray,
+   AllocationFunctionScope NewScope,
+   AllocationFunctionScope DeleteScope,
+   QualType AllocType, bool IsArray,
bool , MultiExprArg PlaceArgs,
FunctionDecl *,
FunctionDecl *,
Index: test/CodeGenCoroutines/coro-alloc.cpp
===
--- test/CodeGenCoroutines/coro-alloc.cpp
+++ test/CodeGenCoroutines/coro-alloc.cpp
@@ -134,6 +134,32 @@
   co_return;
 }
 
+// Declare a placement form operator new, such as the one described in
+// C++ 18.6.1.3.1, which takes a void* argument.
+void* operator new(SizeT __sz, void *__p) noexcept;
+
+struct promise_matching_global_placement_new_tag {};
+struct dummy {};
+template<>
+struct std::experimental::coroutine_traits {
+  struct promise_type {
+void get_return_object() {}
+suspend_always initial_suspend() { return {}; }
+suspend_always final_suspend() { return {}; }
+void return_void() {}
+  };
+};
+
+// A coroutine that takes a single pointer argument should not invoke this
+// placement form operator. [dcl.fct.def.coroutine]/7 dictates that lookup for
+// allocation functions matching the coroutine function's signature be done
+// within the scope of the promise type's class.
+// CHECK-LABEL: f1b(
+extern "C" void f1b(promise_matching_global_placement_new_tag, dummy *) {
+  // CHECK: call i8* @_Znwm(i64
+  co_return;
+}
+
 struct promise_delete_tag {};
 
 template<>
Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -804,62 +804,6 @@
   void *operator new(SizeT, coroutine_nonstatic_member_struct &, double);
 };
 
-struct bad_promise_nonstatic_member_mismatched_custom_new_operator {
-  coro get_return_object();
-  suspend_always initial_suspend();
-  suspend_always final_suspend();
-  void return_void();
-  void unhandled_exception();
-  // expected-note@+1 {{candidate function not viable: requires 2 arguments, but 1 was provided}}
-  void *operator new(SizeT, double);
-};
-
-struct coroutine_nonstatic_member_struct {
-  coro
-  good_coroutine_calls_nonstatic_member_custom_new_operator(double) {
-co_return;
-  }
-
-  coro
-  bad_coroutine_calls_nonstatic_member_mistmatched_custom_new_operator(double) {
-// expected-error@-1 {{no matching function for call to 'operator new'}}
-co_return;
-  }
-};
-
-struct bad_promise_mismatched_custom_new_operator {
-  coro get_return_object();
-  suspend_always initial_suspend();
-  suspend_always final_suspend();
-  void return_void();
-  void unhandled_exception();
-  // expected-note@+1 {{candidate function not viable: requires 4 arguments, but 1 was provided}}
-  void *operator new(SizeT, double, float, int);
-};
-
-coro
-bad_coroutine_calls_mismatched_custom_new_operator(double) {
-  // expected-error@-1 {{no matching function for call to 'operator new'}}
-  co_return;
-}
-
-struct bad_promise_throwing_custom_new_operator {
-  static coro get_return_object_on_allocation_failure();
-  coro get_return_object();
-  suspend_always initial_suspend();
-  suspend_always final_suspend();
-  void return_void();
-  void unhandled_exception();
-  // expected-error@+1 {{'operator new' is required to have a non-throwing noexcept

r328949 - [Coroutines] Find custom allocators in class scope

2018-04-01 Thread Brian Gesiak via cfe-commits 
Author: modocache
Date: Sun Apr  1 15:59:22 2018
New Revision: 328949

URL: http://llvm.org/viewvc/llvm-project?rev=328949=rev
Log:
[Coroutines] Find custom allocators in class scope

Summary:
https://reviews.llvm.org/rL325291 implemented Coroutines TS N4723
section [dcl.fct.def.coroutine]/7, but it performed lookup of allocator
functions within both the global and class scope, whereas the specified
behavior is to perform lookup for custom allocators within just the
class scope.

To fix, add parameters to the `Sema::FindAllocationFunctions` function
such that it can be used to lookup allocators in global scope,
class scope, or both (instead of just being able to look up in just global
scope or in both global and class scope). Then, use those parameters
from within the coroutine Sema.

This incorrect behavior had the unfortunate side-effect of causing the
bug https://bugs.llvm.org/show_bug.cgi?id=36578 (or at least the reports
of that bug in C++ programs). That bug would occur for any C++ user with
a coroutine frame that took a single pointer argument, since it would
then find the global placement form `operator new`, described in the
C++ standard 18.6.1.3.1. This patch prevents Clang from generating code
that triggers the LLVM assert described in that bug report.

Test Plan: `check-clang`

Reviewers: GorNishanov, eric_niebler, lewissbaker

Reviewed By: GorNishanov

Subscribers: EricWF, cfe-commits

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

Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaCoroutine.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/test/CodeGenCoroutines/coro-alloc.cpp
cfe/trunk/test/SemaCXX/coroutines.cpp

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=328949=328948=328949=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Sun Apr  1 15:59:22 2018
@@ -5140,8 +5140,25 @@ public:
 
   bool CheckAllocatedType(QualType AllocType, SourceLocation Loc,
   SourceRange R);
+
+  /// \brief The scope in which to find allocation functions.
+  enum AllocationFunctionScope {
+/// \brief Only look for allocation functions in the global scope.
+AFS_Global,
+/// \brief Only look for allocation functions in the scope of the
+/// allocated class.
+AFS_Class,
+/// \brief Look for allocation functions in both the global scope
+/// and in the scope of the allocated class.
+AFS_Both
+  };
+
+  /// \brief Finds the overloads of operator new and delete that are 
appropriate
+  /// for the allocation.
   bool FindAllocationFunctions(SourceLocation StartLoc, SourceRange Range,
-   bool UseGlobal, QualType AllocType, bool 
IsArray,
+   AllocationFunctionScope NewScope,
+   AllocationFunctionScope DeleteScope,
+   QualType AllocType, bool IsArray,
bool , MultiExprArg PlaceArgs,
FunctionDecl *,
FunctionDecl *,

Modified: cfe/trunk/lib/Sema/SemaCoroutine.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCoroutine.cpp?rev=328949=328948=328949=diff
==
--- cfe/trunk/lib/Sema/SemaCoroutine.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCoroutine.cpp Sun Apr  1 15:59:22 2018
@@ -1110,8 +1110,8 @@ bool CoroutineStmtBuilder::makeNewAndDel
 
 PlacementArgs.push_back(PDRefExpr.get());
   }
-  S.FindAllocationFunctions(Loc, SourceRange(),
-/*UseGlobal*/ false, PromiseType,
+  S.FindAllocationFunctions(Loc, SourceRange(), /*NewScope*/ Sema::AFS_Class,
+/*DeleteScope*/ Sema::AFS_Both, PromiseType,
 /*isArray*/ false, PassAlignment, PlacementArgs,
 OperatorNew, UnusedResult, /*Diagnose*/ false);
 
@@ -1121,10 +1121,21 @@ bool CoroutineStmtBuilder::makeNewAndDel
   // an argument of type std::size_t."
   if (!OperatorNew && !PlacementArgs.empty()) {
 PlacementArgs.clear();
-S.FindAllocationFunctions(Loc, SourceRange(),
-  /*UseGlobal*/ false, PromiseType,
-  /*isArray*/ false, PassAlignment,
-  PlacementArgs, OperatorNew, UnusedResult);
+S.FindAllocationFunctions(Loc, SourceRange(), /*NewScope*/ Sema::AFS_Class,
+  /*DeleteScope*/ Sema::AFS_Both, PromiseType,
+  /*isArray*/ false, PassAlignment, PlacementArgs,
+  OperatorNew, UnusedResult, /*Diagnose*/ false);
+  }
+
+  // [dcl.fct.def.coroutine]/7
+  // "The allocation function’s name is

[PATCH] D45153: test

2018-04-01 Thread Shuai Wang via Phabricator via cfe-commits 
shuaiwang created this revision.
Herald added subscribers: cfe-commits, klimek.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45153

Files:
  clang-tidy/utils/ASTUtils.cpp
  clang-tidy/utils/ASTUtils.h

Index: clang-tidy/utils/ASTUtils.h
===
--- clang-tidy/utils/ASTUtils.h
+++ clang-tidy/utils/ASTUtils.h
@@ -27,6 +27,10 @@
 bool exprHasBitFlagWithSpelling(const Expr *Flags, const SourceManager ,
 const LangOptions ,
 StringRef FlagName);
+
+// Checks whether `expr` is (potentially) modified within `stmt`.
+bool isModified(const Expr& expr, const Stmt& stmt, ASTContext* context);
+
 } // namespace utils
 } // namespace tidy
 } // namespace clang
Index: clang-tidy/utils/ASTUtils.cpp
===
--- clang-tidy/utils/ASTUtils.cpp
+++ clang-tidy/utils/ASTUtils.cpp
@@ -67,6 +67,83 @@
   return true;
 }
 
+bool isModified(const Expr& expr, const Stmt& stmt, ASTContext* context) {
+  // LHS of any assignment operators.
+  const auto as_assignment_lhs = binaryOperator(
+  anyOf(hasOperatorName("="), hasOperatorName("+="), hasOperatorName("-="),
+hasOperatorName("*="), hasOperatorName("/="), hasOperatorName("%="),
+hasOperatorName("^="), hasOperatorName("&="), hasOperatorName("|="),
+hasOperatorName(">>="), hasOperatorName("<<=")),
+  hasLHS(equalsNode()));
+
+  // Operand of increment/decrement operators.
+  const auto as_inc_dec_operand =
+  unaryOperator(anyOf(hasOperatorName("++"), hasOperatorName("--")),
+hasUnaryOperand(equalsNode()));
+
+  // Invoking non-const member function.
+  const auto non_const_method = cxxMethodDecl(unless(isConst()));
+  const auto as_non_const_this = ast_matchers::expr(
+  anyOf(cxxMemberCallExpr(callee(non_const_method), on(equalsNode())),
+cxxOperatorCallExpr(callee(non_const_method),
+hasArgument(0, equalsNode();
+
+  // Used as non-const-ref argument when calling a function.
+  const auto non_const_ref_type =
+  referenceType(pointee(unless(isConstQualified(;
+  const auto non_const_ref_param = forEachArgumentWithParam(
+  equalsNode(), parmVarDecl(hasType(qualType(non_const_ref_type;
+  const auto as_non_const_ref_arg = anyOf(
+  callExpr(non_const_ref_param), cxxConstructExpr(non_const_ref_param));
+
+  // Taking address of 'exp'.
+  const auto as_ampersand_operand =
+  unaryOperator(hasOperatorName("&"), hasUnaryOperand(equalsNode()));
+  const auto as_pointer_from_array_decay =
+  castExpr(hasCastKind(CK_ArrayToPointerDecay), has(equalsNode()));
+
+  // Check whether 'exp' is directly modified as a whole.
+  MatchFinder finder;
+  bool has_match = false;
+  MatchCallbackAdaptor callback(
+  [_match](const MatchFinder::MatchResult&) { has_match = true; });
+  finder.addMatcher(findAll(ast_matchers::expr(anyOf(
+as_assignment_lhs, as_inc_dec_operand,
+as_non_const_this, as_non_const_ref_arg,
+as_ampersand_operand, as_pointer_from_array_decay))),
+);
+  finder.match(stmt, *context);
+  if (has_match) return true;
+
+  // Check whether any member of 'exp' is modified.
+  const auto member_exprs = match(
+  findAll(memberExpr(hasObjectExpression(equalsNode())).bind("expr")),
+  stmt, *context);
+  for (const auto& node : member_exprs) {
+if (isModified(*node.getNodeAs("expr"), stmt, context)) return true;
+  }
+
+  // If 'exp' is bound to a non-const reference, check all declRefExpr to that.
+  const auto decls = match(
+  ast_matchers::stmt(forEachDescendant(
+  varDecl(hasType(referenceType(pointee(unless(isConstQualified(),
+  hasInitializer(equalsNode()))
+  .bind("decl"))),
+  stmt, *context);
+  for (const auto& decl_node : decls) {
+const auto exprs = match(
+findAll(declRefExpr(to(equalsNode(decl_node.getNodeAs("decl"
+.bind("expr")),
+stmt, *context);
+for (const auto& expr_node : exprs) {
+  if (isModified(*expr_node.getNodeAs("expr"), stmt, context))
+return true;
+}
+  }
+
+  return false;
+}
+
 } // namespace utils
 } // namespace tidy
 } // namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45120: [coroutines] Add __builtin_coro_noop => llvm.coro.noop

2018-04-01 Thread Brian Gesiak via Phabricator via cfe-commits 
modocache accepted this revision.
modocache added a comment.
This revision is now accepted and ready to land.

Great, thanks!


https://reviews.llvm.org/D45120



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

[PATCH] D45152: [MinGW] Add option for disabling looking for a mingw gcc in PATH

2018-04-01 Thread Martin Storsjö via Phabricator via cfe-commits 
mstorsjo created this revision.
mstorsjo added reviewers: martell, compnerd, rnk, mati865, ismail, yaron.keren.

This avoids looking for a mingw gcc to use as sysroot, preferring the clang 
installation itself over a similar gcc found in the path.

The option name, `--ignore-gcc`, might not be ideal - any suggestions for a 
better name?

Currently, I'm injecting a --sysroot parameter via a wrapper script, forcing 
the clang invocation to use `clang_bin/../triple` as sysroot, even if there's a 
similar named cross gcc elsewhere in PATH. With this option, I wouldn't need to 
use a wrapper script to manually set the sysroot.


Repository:
  rC Clang

https://reviews.llvm.org/D45152

Files:
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/MinGW.cpp
  test/Driver/Inputs/mingw_gcc/bin/x86_64-w64-mingw32-gcc
  test/Driver/Inputs/mingw_gcc/x86_64-w64-mingw32/include/.keep
  test/Driver/mingw-gcc.c


Index: test/Driver/mingw-gcc.c
===
--- /dev/null
+++ test/Driver/mingw-gcc.c
@@ -0,0 +1,7 @@
+// RUN: env PATH=%S/Inputs/mingw_gcc/bin \
+// RUN:%clang -target x86_64-w64-mingw32 -c -### %s 2>&1 | FileCheck 
-check-prefix=CHECK_MINGW_GCC %s
+// CHECK_MINGW_GCC: 
"{{.*}}{{/|}}Inputs{{/|}}mingw_gcc{{/|}}x86_64-w64-mingw32{{/|}}include"
+
+// RUN: env PATH=%S/Inputs/mingw_gcc/bin \
+// RUN:%clang -target x86_64-w64-mingw32 -c -### %s --ignore-gcc 2>&1 | 
FileCheck -check-prefix=CHECK_NOT_MINGW_GCC %s
+// CHECK_NOT_MINGW_GCC-NOT: 
"{{.*}}{{/|}}Inputs{{/|}}mingw_gcc{{/|}}x86_64-w64-mingw32{{/|}}include"
Index: lib/Driver/ToolChains/MinGW.cpp
===
--- lib/Driver/ToolChains/MinGW.cpp
+++ lib/Driver/ToolChains/MinGW.cpp
@@ -309,11 +309,14 @@
 
   if (getDriver().SysRoot.size())
 Base = getDriver().SysRoot;
-  else if (llvm::ErrorOr GPPName = findGcc())
-Base = llvm::sys::path::parent_path(
-llvm::sys::path::parent_path(GPPName.get()));
-  else
-Base = llvm::sys::path::parent_path(getDriver().getInstalledDir());
+  else {
+llvm::ErrorOr GPPName("");
+if (!Args.hasArg(options::OPT_ignore_gcc) && (GPPName = findGcc()))
+  Base = llvm::sys::path::parent_path(
+  llvm::sys::path::parent_path(GPPName.get()));
+else
+  Base = llvm::sys::path::parent_path(getDriver().getInstalledDir());
+  }
 
   Base += llvm::sys::path::get_separator();
   findGccLibDir();
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1732,6 +1732,8 @@
   HelpText<"Add directory to SYSTEM framework search path, "
"absolute paths are relative to -isysroot">,
   MetaVarName<"">, Flags<[CC1Option]>;
+def ignore_gcc : Joined<["--"], "ignore-gcc">, Flags<[DriverOption]>,
+  HelpText<"Don't look for gcc for finding a suitable sysroot">;
 def imacros : JoinedOrSeparate<["-", "--"], "imacros">, Group, 
Flags<[CC1Option]>,
   HelpText<"Include macros from file before parsing">, MetaVarName<"">;
 def image__base : Separate<["-"], "image_base">;


Index: test/Driver/mingw-gcc.c
===
--- /dev/null
+++ test/Driver/mingw-gcc.c
@@ -0,0 +1,7 @@
+// RUN: env PATH=%S/Inputs/mingw_gcc/bin \
+// RUN:%clang -target x86_64-w64-mingw32 -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_MINGW_GCC %s
+// CHECK_MINGW_GCC: "{{.*}}{{/|}}Inputs{{/|}}mingw_gcc{{/|}}x86_64-w64-mingw32{{/|}}include"
+
+// RUN: env PATH=%S/Inputs/mingw_gcc/bin \
+// RUN:%clang -target x86_64-w64-mingw32 -c -### %s --ignore-gcc 2>&1 | FileCheck -check-prefix=CHECK_NOT_MINGW_GCC %s
+// CHECK_NOT_MINGW_GCC-NOT: "{{.*}}{{/|}}Inputs{{/|}}mingw_gcc{{/|}}x86_64-w64-mingw32{{/|}}include"
Index: lib/Driver/ToolChains/MinGW.cpp
===
--- lib/Driver/ToolChains/MinGW.cpp
+++ lib/Driver/ToolChains/MinGW.cpp
@@ -309,11 +309,14 @@
 
   if (getDriver().SysRoot.size())
 Base = getDriver().SysRoot;
-  else if (llvm::ErrorOr GPPName = findGcc())
-Base = llvm::sys::path::parent_path(
-llvm::sys::path::parent_path(GPPName.get()));
-  else
-Base = llvm::sys::path::parent_path(getDriver().getInstalledDir());
+  else {
+llvm::ErrorOr GPPName("");
+if (!Args.hasArg(options::OPT_ignore_gcc) && (GPPName = findGcc()))
+  Base = llvm::sys::path::parent_path(
+  llvm::sys::path::parent_path(GPPName.get()));
+else
+  Base = llvm::sys::path::parent_path(getDriver().getInstalledDir());
+  }
 
   Base += llvm::sys::path::get_separator();
   findGccLibDir();
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1732,6 +1732,8 @@
   HelpText<"Add

[PATCH] D45149: MallocChecker, adding specific BSD calls

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

Nice, thanks.




Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:662-676
 if (Family == AF_Malloc && CheckAlloc) {
   if (FunI == II_malloc || FunI == II_realloc || FunI == II_reallocf ||
   FunI == II_calloc || FunI == II_valloc || FunI == II_strdup ||
   FunI == II_win_strdup || FunI == II_strndup || FunI == II_wcsdup ||
   FunI == II_win_wcsdup || FunI == II_kmalloc ||
   FunI == II_g_malloc || FunI == II_g_malloc0 || 
   FunI == II_g_realloc || FunI == II_g_try_malloc || 

These lists are getting long, i guess they should be refactored into a simple 
`II` -> `Kind` pointer map lookup eventually.



Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:890-891
   State = ProcessZeroAllocation(C, CE, 1, State);
-} else if (FunI == II_free || FunI == II_g_free) {
+} else if (FunI == II_recallocarray) {
+  State = CallocMem(C, CE, State, true);
+  State = ProcessZeroAllocation(C, CE, 0, State);

The moved array is not all zeros, just the new part, right? It should be more 
accurate to realloc() here. Not sure if we actually model realloc() by moving 
memory contents (at least, i'm sure we're not modeling it perfectly). If we 
simply invalidate the newly allocated region, it should be fine to simply 
re-use `ReallocMemAux()` here. If we try to mark the newly added bytes as 
uninitialized, then you might need to pass a flag to zero-initialize them 
instead.


Repository:
  rC Clang

https://reviews.llvm.org/D45149



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

r328942 - Fix a major swiftcall ABI bug with trivial C++ class types.

2018-04-01 Thread John McCall via cfe-commits 
Author: rjmccall
Date: Sun Apr  1 14:04:30 2018
New Revision: 328942

URL: http://llvm.org/viewvc/llvm-project?rev=328942=rev
Log:
Fix a major swiftcall ABI bug with trivial C++ class types.

The problem with the previous logic was that there might not be any
explicit copy/move constructor declarations, e.g. if the type is
trivial and we've never type-checked a copy of it.  Relying on Sema's
computation seems much more reliable.

Also, I believe Richard's recommendation is exactly the rule we use
now on the Itanium ABI, modulo the trivial_abi attribute (which this
change of course fixes our handling of in Swift).

This does mean that we have a less portable rule for deciding
indirectness for swiftcall.  I would prefer it if we just applied the
Itanium rule universally under swiftcall, but in the meantime, I need
to fix this bug.

This only arises when defining functions with class-type arguments
in C++, as we do in the Swift runtime.  It doesn't affect normal Swift
operation because we don't import code as C++.

Modified:
cfe/trunk/lib/CodeGen/SwiftCallingConv.cpp
cfe/trunk/test/CodeGenCXX/arm-swiftcall.cpp

Modified: cfe/trunk/lib/CodeGen/SwiftCallingConv.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/SwiftCallingConv.cpp?rev=328942=328941=328942=diff
==
--- cfe/trunk/lib/CodeGen/SwiftCallingConv.cpp (original)
+++ cfe/trunk/lib/CodeGen/SwiftCallingConv.cpp Sun Apr  1 14:04:30 2018
@@ -742,22 +742,10 @@ void swiftcall::legalizeVectorType(CodeG
 
 bool swiftcall::shouldPassCXXRecordIndirectly(CodeGenModule ,
   const CXXRecordDecl *record) {
-  // Following a recommendation from Richard Smith, pass a C++ type
-  // indirectly only if the destructor is non-trivial or *all* of the
-  // copy/move constructors are deleted or non-trivial.
-
-  if (record->hasNonTrivialDestructor())
-return true;
-
-  // It would be nice if this were summarized on the CXXRecordDecl.
-  for (auto ctor : record->ctors()) {
-if (ctor->isCopyOrMoveConstructor() && !ctor->isDeleted() &&
-ctor->isTrivial()) {
-  return false;
-}
-  }
-
-  return true;
+  // FIXME: should we not rely on the standard computation in Sema, just in
+  // case we want to diverge from the platform ABI (e.g. on targets where
+  // that uses the MSVC rule)?
+  return !record->canPassInRegisters();
 }
 
 static ABIArgInfo classifyExpandedType(SwiftAggLowering ,

Modified: cfe/trunk/test/CodeGenCXX/arm-swiftcall.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/arm-swiftcall.cpp?rev=328942=328941=328942=diff
==
--- cfe/trunk/test/CodeGenCXX/arm-swiftcall.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/arm-swiftcall.cpp Sun Apr  1 14:04:30 2018
@@ -113,3 +113,13 @@ TEST(struct_indirect_1)
 
 // Should not be byval.
 // CHECK-LABEL: define {{.*}} void @take_struct_indirect_1({{.*}}*{{( %.*)?}})
+
+// Do a simple standalone test here of a function definition to ensure that
+// we don't have problems due to failure to eagerly synthesize a copy
+// constructor declaration.
+class struct_trivial {
+  int x;
+};
+// CHECK-LABEL define void @test_struct_trivial(i32{{( %.*)?}})
+extern "C" SWIFTCALL
+void test_struct_trivial(struct_trivial triv) {}


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

[PATCH] D45149: MallocChecker, adding specific BSD calls

2018-04-01 Thread David CARLIER via Phabricator via cfe-commits 
devnexen created this revision.
devnexen added reviewers: NoQ, jdenny.
Herald added a subscriber: cfe-commits.

- reallocarray/recallocarray
- freezero


Repository:
  rC Clang

https://reviews.llvm.org/D45149

Files:
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  test/Analysis/bsd-malloc.c

Index: test/Analysis/bsd-malloc.c
===
--- /dev/null
+++ test/Analysis/bsd-malloc.c
@@ -0,0 +1,28 @@
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-openbsd -analyzer-checker=unix.Malloc -verify %s
+
+#define NULL  ((void *) 0)
+
+typedef __typeof(sizeof(int)) size_t;
+
+void *reallocarray(void *ptr, size_t nmemb, size_t size);
+void *recallocarray(void *ptr, size_t onmemb, size_t nmemb, size_t size);
+void freezero(void *ptr, size_t size);
+
+void f1() {
+  int *parr = NULL;
+  parr = reallocarray(NULL, 10, sizeof(*parr));
+  return; // expected-warning{{Potential leak of memory pointed to by 'parr'}}
+}
+
+void f2() {
+  int *parr = NULL;
+  parr = recallocarray(NULL, 10, 20, sizeof(*parr));
+  return; // expected-warning{{Potential leak of memory pointed to by 'parr'}}
+}
+
+void f3() {
+  int *parr = NULL;
+  parr = reallocarray(NULL, 10, sizeof(*parr));
+  freezero(parr, 10 * sizeof(*parr)); // expected-no-warning
+  freezero(parr, 10 * sizeof(*parr)); // expected-warning{{Attempt to free released memory}}
+}
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -182,7 +182,8 @@
 II_g_free(nullptr), II_g_memdup(nullptr), II_g_malloc_n(nullptr), 
 II_g_malloc0_n(nullptr), II_g_realloc_n(nullptr), 
 II_g_try_malloc_n(nullptr), II_g_try_malloc0_n(nullptr), 
-II_g_try_realloc_n(nullptr) {}
+II_g_try_realloc_n(nullptr), II_reallocarray(nullptr),
+II_recallocarray(nullptr), II_freezero(nullptr) {}
 
   /// In pessimistic mode, the checker assumes that it does not know which
   /// functions might free the memory.
@@ -251,7 +252,8 @@
  *II_g_try_realloc, *II_g_free, *II_g_memdup, 
  *II_g_malloc_n, *II_g_malloc0_n, *II_g_realloc_n, 
  *II_g_try_malloc_n, *II_g_try_malloc0_n, 
- *II_g_try_realloc_n;
+ *II_g_try_realloc_n, *II_reallocarray,
+ *II_recallocarray, *II_freezero;
   mutable Optional KernelZeroFlagVal;
 
   void initIdentifierInfo(ASTContext ) const;
@@ -349,7 +351,8 @@
   static SVal evalMulForBufferSize(CheckerContext , const Expr *Blocks,
const Expr *BlockBytes);
   static ProgramStateRef CallocMem(CheckerContext , const CallExpr *CE,
-   ProgramStateRef State);
+   ProgramStateRef State,
+   bool SuffixWithArray = false);
 
   ///\brief Check if the memory associated with this symbol was released.
   bool isReleased(SymbolRef Sym, CheckerContext ) const;
@@ -611,6 +614,11 @@
   II_g_try_malloc_n = ("g_try_malloc_n");
   II_g_try_malloc0_n = ("g_try_malloc0_n");
   II_g_try_realloc_n = ("g_try_realloc_n");
+
+  // BSD
+  II_reallocarray = ("reallocarray");
+  II_recallocarray = ("recallocarray");
+  II_freezero = ("freezero");
 }
 
 bool MallocChecker::isMemFunction(const FunctionDecl *FD, ASTContext ) const {
@@ -647,7 +655,7 @@
 
 if (Family == AF_Malloc && CheckFree) {
   if (FunI == II_free || FunI == II_realloc || FunI == II_reallocf || 
-  FunI == II_g_free)
+  FunI == II_g_free || FunI == II_freezero)
 return true;
 }
 
@@ -662,7 +670,8 @@
   FunI == II_g_memdup || FunI == II_g_malloc_n || 
   FunI == II_g_malloc0_n || FunI == II_g_realloc_n || 
   FunI == II_g_try_malloc_n || FunI == II_g_try_malloc0_n || 
-  FunI == II_g_try_realloc_n)
+  FunI == II_g_try_realloc_n || FunI == II_reallocarray ||
+  FunI == II_recallocarray)
 return true;
 }
 
@@ -878,7 +887,11 @@
   State = CallocMem(C, CE, State);
   State = ProcessZeroAllocation(C, CE, 0, State);
   State = ProcessZeroAllocation(C, CE, 1, State);
-} else if (FunI == II_free || FunI == II_g_free) {
+} else if (FunI == II_recallocarray) {
+  State = CallocMem(C, CE, State, true);
+  State = ProcessZeroAllocation(C, CE, 0, State);
+  State = ProcessZeroAllocation(C, CE, 1, State);
+} else if (FunI == II_free || FunI == II_g_free || FunI == II_freezero) {
   State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory);
 } else if (FunI == II_strdup || FunI == II_win_strdup ||
FunI == II_wcsdup || FunI == II_win_wcsdup) {
@@ -943,7 +956,8 @@
   State = MallocMemAux(C, CE, TotalSize, Init, State);
   State = ProcessZeroAllocation(C,

[PATCH] D45147: [clang] Implement Intercal compatibility feature

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

testcase 
Please add docs, release notes entry, and most importantly, tests!


Repository:
  rC Clang

https://reviews.llvm.org/D45147



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

[PATCH] D45147: [clang] Implement Intercal compatibility feature

2018-04-01 Thread Leandro A. F. Pereira via Phabricator via cfe-commits 
lpereira created this revision.
Herald added a subscriber: cfe-commits.

This commit is part of a grand series of changes that aims to reduce
the amount of evil in the world.  The "goto" statement is one of such
evils that must be fought; however, due to both compatibility reasons,
and lack of support in mainstream compilers, it can't be removed just
now.

This patch implements Intercal's "comefrom" statement in both C and C++
modes.

When comparing to the original implementation, the astute reader will
note that behavior such as producing an error when more than one line
comes from another label or not properly handling when a line comes
from a label and goes to a label.  This is to keep the tone of both C
and C++ languages, with regards to undefined behavior.


Repository:
  rC Clang

https://reviews.llvm.org/D45147

Files:
  include/clang-c/Index.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/AST/Stmt.h
  include/clang/Basic/StmtNodes.td
  include/clang/Basic/TokenKinds.def
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/AST/StmtPrinter.cpp
  lib/CodeGen/CGStmt.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Parse/ParseStmt.cpp
  lib/Sema/SemaStmt.cpp
  lib/Sema/TreeTransform.h
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  tools/libclang/CXCursor.cpp

Index: tools/libclang/CXCursor.cpp
===
--- tools/libclang/CXCursor.cpp
+++ tools/libclang/CXCursor.cpp
@@ -140,6 +140,10 @@
 K = CXCursor_ForStmt;
 break;
   
+  case Stmt::ComeFromStmtClass:
+K = CXCursor_ComeFromStmt;
+break;
+
   case Stmt::GotoStmtClass:
 K = CXCursor_GotoStmt;
 break;
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1358,6 +1358,7 @@
 case Stmt::DoStmtClass:
 case Stmt::ForStmtClass:
 case Stmt::GotoStmtClass:
+case Stmt::ComeFromStmtClass:
 case Stmt::IfStmtClass:
 case Stmt::IndirectGotoStmtClass:
 case Stmt::LabelStmtClass:
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -6752,6 +6752,12 @@
  S->getRParenLoc(), Body.get());
 }
 
+template
+StmtResult
+TreeTransform::TransformComeFromStmt(ComeFromStmt *S) {
+  return S;
+}
+
 template
 StmtResult
 TreeTransform::TransformGotoStmt(GotoStmt *S) {
Index: lib/Sema/SemaStmt.cpp
===
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -1442,6 +1442,10 @@
   FoundDecl = true;
 }
 
+void VisitComeFromStmt(ComeFromStmt *S) {
+  FoundDecl = true;
+}
+
 void VisitCastExpr(CastExpr *E) {
   if (E->getCastKind() == CK_LValueToRValue)
 CheckLValueToRValueCast(E->getSubExpr());
@@ -2789,6 +2793,14 @@
   return new (Context) GotoStmt(TheDecl, GotoLoc, LabelLoc);
 }
 
+StmtResult Sema::ActOnComeFromStmt(SourceLocation ComeFromLoc,
+   SourceLocation LabelLoc,
+   LabelDecl *TheDecl) {
+  getCurFunction()->setHasBranchIntoScope();
+  TheDecl->markUsed(Context);
+  return new (Context) ComeFromStmt(TheDecl, ComeFromLoc, LabelLoc);
+}
+
 StmtResult
 Sema::ActOnIndirectGotoStmt(SourceLocation GotoLoc, SourceLocation StarLoc,
 Expr *E) {
Index: lib/Parse/ParseStmt.cpp
===
--- lib/Parse/ParseStmt.cpp
+++ lib/Parse/ParseStmt.cpp
@@ -254,6 +254,10 @@
 Res = ParseGotoStatement();
 SemiError = "goto";
 break;
+  case tok::kw_comefrom:// Intercal compatibility
+Res = ParseComeFromStatement();
+SemiError = "comefrom";
+break;
   case tok::kw_continue:// C99 6.8.6.2: continue-statement
 Res = ParseContinueStatement();
 SemiError = "continue";
@@ -1861,6 +1865,31 @@
   return Res;
 }
 
+/// ParseComeFromStatement
+///   jump-statement:
+/// 'comefrom' identifier ';'
+///
+/// Note: this lets the caller parse the end ';'.
+///
+StmtResult Parser::ParseComeFromStatement() {
+  assert(Tok.is(tok::kw_comefrom) && "Not a comefrom stmt!");
+  SourceLocation ComeFromLoc = ConsumeToken();  // eat the 'comefrom'.
+
+  StmtResult Res;
+  if (Tok.is(tok::identifier)) {
+LabelDecl *LD = Actions.LookupOrCreateLabel(Tok.getIdentifierInfo(),
+Tok.getLocation());
+Res = Actions.ActOnComeFromStmt(ComeFromLoc, Tok.getLocation(), LD);
+ConsumeToken();
+  } else {
+Diag(Tok, diag::err_expected) << tok::identifier;
+return StmtError();
+  }
+
+  return Res;
+}
+
+
 /// ParseContinueStatement
 ///   jump-statement:
 /// 'continue' ';'
Index: lib/CodeGen/CodeGenFunction.h

[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

2018-04-01 Thread Zinovy Nis via Phabricator via cfe-commits 
zinovy.nis updated this revision to Diff 140579.
zinovy.nis added a comment.

- Minor cosmetic fix: use lexical representation instead of semanic for 
printing callee string. It helps to print class names hidden behind 'typedef' 
and 'using':



  using A1 = A;
  typedef A A2;
  
  class C2 : public B {
  public:
int virt_1() override { return A1::virt_1() + A2::virt_1(); }
// BEFORE: warning: qualified name 'A::virt_1'...
// BEFORE: warning: qualified name 'A::virt_1'...
  
//  AFTER: warning: qualified name 'A1::virt_1'...
//  AFTER: warning: qualified name 'A2::virt_1'...
  };


https://reviews.llvm.org/D44295

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/ParentVirtualCallCheck.cpp
  clang-tidy/bugprone/ParentVirtualCallCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-parent-virtual-call.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-parent-virtual-call.cpp

Index: test/clang-tidy/bugprone-parent-virtual-call.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-parent-virtual-call.cpp
@@ -0,0 +1,177 @@
+// RUN: %check_clang_tidy %s bugprone-parent-virtual-call %t
+
+extern int foo();
+
+class A {
+public:
+  A() = default;
+  virtual ~A() = default;
+
+  virtual int virt_1() { return foo() + 1; }
+  virtual int virt_2() { return foo() + 2; }
+
+  int non_virt() { return foo() + 3; }
+  static int stat() { return foo() + 4; }
+};
+
+class B : public A {
+public:
+  B() = default;
+
+  // Nothing to fix: calls to direct parent.
+  int virt_1() override { return A::virt_1() + 3; }
+  int virt_2() override { return A::virt_2() + 4; }
+};
+
+class C : public B {
+public:
+  int virt_1() override { return A::virt_1() + B::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' refers to a member overridden in subclass; did you mean 'B'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return B::virt_1() + B::virt_1(); }
+  int virt_2() override { return A::virt_1() + B::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' {{.*}}; did you mean 'B'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_2() override { return B::virt_1() + B::virt_1(); }
+
+  // Test that non-virtual and static methods are not affected by this cherker.
+  int method_c() { return A::stat() + A::non_virt(); }
+};
+
+// Check aliased type names
+using A1 = A;
+typedef A A2;
+
+class C2 : public B {
+public:
+  int virt_1() override { return A1::virt_1() + A2::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A1::virt_1' {{.*}}; did you mean 'B'? [bugprone-parent-virtual-call]
+  // CHECK-MESSAGES: :[[@LINE-2]]:49: warning: qualified name 'A2::virt_1' {{.*}}; did you mean 'B'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return B::virt_1() + B::virt_1(); }
+};
+
+// Test that the check affects grand-grand..-parent calls too.
+class D : public C {
+public:
+  int virt_1() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' {{.*}}; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: qualified name 'B::virt_1' {{.*}}; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+  int virt_2() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' {{.*}}; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: qualified name 'B::virt_1' {{.*}}; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_2() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+};
+
+// Test classes in namespaces.
+namespace {
+class BN : public A {
+public:
+  int virt_1() override { return A::virt_1() + 3; }
+  int virt_2() override { return A::virt_2() + 4; }
+};
+} // namespace
+
+namespace N1 {
+class A {
+public:
+  A() = default;
+  virtual int virt_1() { return foo() + 1; }
+  virtual int virt_2() { return foo() + 2; }
+};
+} // namespace N1
+
+namespace N2 {
+class BN : public N1::A {
+public:
+  int virt_1() override { return A::virt_1() + 3; }
+  int virt_2() override { return A::virt_2() + 4; }
+};
+} // namespace N2
+
+class CN : public BN {
+public:
+  int virt_1() override { return A::virt_1() + BN::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' {{.*}}; did you mean 'BN'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return BN::virt_1() + BN::virt_1(); }
+  int virt_2() override { return A::virt_1() + BN::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name

[PATCH] D44906: [clang-tidy] Define __clang_analyzer__ macro for clang-tidy for compatibility with clang static analyzer

2018-04-01 Thread Zinovy Nis via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL328932: [clang-tidy] Define __clang_analyzer__ macro for 
clang-tidy for compatibility… (authored by zinovy.nis, committed by ).
Herald added subscribers: llvm-commits, klimek.

Changed prior to commit:
  https://reviews.llvm.org/D44906?vs=139843=140577#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D44906

Files:
  clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
  clang-tools-extra/trunk/test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp


Index: 
clang-tools-extra/trunk/test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp
===
--- 
clang-tools-extra/trunk/test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp
+++ 
clang-tools-extra/trunk/test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp
@@ -0,0 +1,8 @@
+// RUN: %check_clang_tidy %s * %t
+
+#if defined(__clang_analyzer__)
+#warning __clang_analyzer__ is defined
+#endif
+// CHECK-MESSAGES: :[[@LINE-2]]:2: warning: __clang_analyzer__ is defined 
[clang-diagnostic-#warnings]
+
+
Index: clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
===
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
@@ -481,6 +481,16 @@
   ClangTool Tool(Compilations, InputFiles,
  std::make_shared(), BaseFS);
 
+  // Add __clang_analyzer__ macro definition for compatibility with the clang
+  // static analyzer.
+  ArgumentsAdjuster ClangTidyMacroDefinitionInserter =
+  [](const CommandLineArguments , StringRef Filename) {
+ClangTidyOptions Opts = Context.getOptionsForFile(Filename);
+CommandLineArguments AdjustedArgs = Args;
+AdjustedArgs.emplace_back("-D__clang_analyzer__");
+return AdjustedArgs;
+  };
+
   // Add extra arguments passed by the clang-tidy command-line.
   ArgumentsAdjuster PerFileExtraArgumentsInserter =
   [](const CommandLineArguments , StringRef Filename) {
@@ -515,6 +525,7 @@
 return AdjustedArgs;
   };
 
+  Tool.appendArgumentsAdjuster(ClangTidyMacroDefinitionInserter);
   Tool.appendArgumentsAdjuster(PerFileExtraArgumentsInserter);
   Tool.appendArgumentsAdjuster(PluginArgumentsRemover);
   if (Profile)


Index: clang-tools-extra/trunk/test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp
@@ -0,0 +1,8 @@
+// RUN: %check_clang_tidy %s * %t
+
+#if defined(__clang_analyzer__)
+#warning __clang_analyzer__ is defined
+#endif
+// CHECK-MESSAGES: :[[@LINE-2]]:2: warning: __clang_analyzer__ is defined [clang-diagnostic-#warnings]
+
+
Index: clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
===
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
@@ -481,6 +481,16 @@
   ClangTool Tool(Compilations, InputFiles,
  std::make_shared(), BaseFS);
 
+  // Add __clang_analyzer__ macro definition for compatibility with the clang
+  // static analyzer.
+  ArgumentsAdjuster ClangTidyMacroDefinitionInserter =
+  [](const CommandLineArguments , StringRef Filename) {
+ClangTidyOptions Opts = Context.getOptionsForFile(Filename);
+CommandLineArguments AdjustedArgs = Args;
+AdjustedArgs.emplace_back("-D__clang_analyzer__");
+return AdjustedArgs;
+  };
+
   // Add extra arguments passed by the clang-tidy command-line.
   ArgumentsAdjuster PerFileExtraArgumentsInserter =
   [](const CommandLineArguments , StringRef Filename) {
@@ -515,6 +525,7 @@
 return AdjustedArgs;
   };
 
+  Tool.appendArgumentsAdjuster(ClangTidyMacroDefinitionInserter);
   Tool.appendArgumentsAdjuster(PerFileExtraArgumentsInserter);
   Tool.appendArgumentsAdjuster(PluginArgumentsRemover);
   if (Profile)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits