[PATCH] D75453: [Driver][ARM] parse version of arm/thumb architecture correctly

[Daniel Kiss via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG070acb1d1e51: [Driver][ARM] parse version of arm/thumb 
architecture correctly (authored by danielkiss).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75453

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/windows-thumbv7em.cpp


Index: clang/test/Driver/windows-thumbv7em.cpp
===
--- /dev/null
+++ clang/test/Driver/windows-thumbv7em.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang -target thumb-none-windows-eabi-coff -mcpu=cortex-m7 -### -c %s 
2>&1 \
+// RUN: | FileCheck %s --check-prefix CHECK-V7
+// CHECK-V7-NOT: error: the target architecture 'thumbv7em' is not supported 
by the target 'thumbv7em-none-windows-eabi'
+
+// RUN: %clang -target thumb-none-windows-eabi-coff -mcpu=cortex-m1 -### -c %s 
2>&1 \
+// RUN: | FileCheck %s --check-prefix CHECK-V6
+// CHECK-V6: error: the target architecture 'thumbv6m' is not supported by the 
target 'thumbv6m-none-windows-eabi'
+
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4041,9 +4041,10 @@
   if (Triple.isOSWindows() && (Triple.getArch() == llvm::Triple::arm ||
Triple.getArch() == llvm::Triple::thumb)) {
 unsigned Offset = Triple.getArch() == llvm::Triple::arm ? 4 : 6;
-unsigned Version;
-Triple.getArchName().substr(Offset).getAsInteger(10, Version);
-if (Version < 7)
+unsigned Version = 0;
+bool Failure =
+Triple.getArchName().substr(Offset).consumeInteger(10, Version);
+if (Failure || Version < 7)
   D.Diag(diag::err_target_unsupported_arch) << Triple.getArchName()
 << TripleStr;
   }


Index: clang/test/Driver/windows-thumbv7em.cpp
===
--- /dev/null
+++ clang/test/Driver/windows-thumbv7em.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang -target thumb-none-windows-eabi-coff -mcpu=cortex-m7 -### -c %s 2>&1 \
+// RUN: | FileCheck %s --check-prefix CHECK-V7
+// CHECK-V7-NOT: error: the target architecture 'thumbv7em' is not supported by the target 'thumbv7em-none-windows-eabi'
+
+// RUN: %clang -target thumb-none-windows-eabi-coff -mcpu=cortex-m1 -### -c %s 2>&1 \
+// RUN: | FileCheck %s --check-prefix CHECK-V6
+// CHECK-V6: error: the target architecture 'thumbv6m' is not supported by the target 'thumbv6m-none-windows-eabi'
+
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4041,9 +4041,10 @@
   if (Triple.isOSWindows() && (Triple.getArch() == llvm::Triple::arm ||
Triple.getArch() == llvm::Triple::thumb)) {
 unsigned Offset = Triple.getArch() == llvm::Triple::arm ? 4 : 6;
-unsigned Version;
-Triple.getArchName().substr(Offset).getAsInteger(10, Version);
-if (Version < 7)
+unsigned Version = 0;
+bool Failure =
+Triple.getArchName().substr(Offset).consumeInteger(10, Version);
+if (Failure || Version < 7)
   D.Diag(diag::err_target_unsupported_arch) << Triple.getArchName()
 << TripleStr;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82940: [ASTReader][ASTWriter][PCH][Modules] Fix (de)serialization of SwitchCases

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

In D82940#2124955 , @aaron.ballman 
wrote:

> LGTM, thank you for the fix!


Wait, I am not sure that it is the right fix. The body of the lambda should not 
be different from the body of the `CXXMethodDecl`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82940



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

[PATCH] D82844: [clangd] Set gRPC deadlines to all remote index requests

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 274744.
kbobyrev marked 2 inline comments as done.
kbobyrev added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82844

Files:
  clang-tools-extra/clangd/index/remote/Client.cpp


Index: clang-tools-extra/clangd/index/remote/Client.cpp
===
--- clang-tools-extra/clangd/index/remote/Client.cpp
+++ clang-tools-extra/clangd/index/remote/Client.cpp
@@ -15,6 +15,8 @@
 #include "support/Logger.h"
 #include "support/Trace.h"
 
+#include 
+
 namespace clang {
 namespace clangd {
 namespace remote {
@@ -25,7 +27,6 @@
   using StreamingCall = std::unique_ptr> (
   remote::SymbolIndex::Stub::*)(grpc::ClientContext *, const RequestT &);
 
-  // FIXME(kirillbobyrev): Set deadlines for requests.
   template 
   bool streamRPC(ClangdRequestT Request,
@@ -35,6 +36,9 @@
 trace::Span Tracer(RequestT::descriptor()->name());
 const auto RPCRequest = toProtobuf(Request);
 grpc::ClientContext Context;
+std::chrono::system_clock::time_point Deadline =
+std::chrono::system_clock::now() + DeadlineWaitingTime;
+Context.set_deadline(Deadline);
 auto Reader = (Stub.get()->*RPCCall)(&Context, RPCRequest);
 llvm::BumpPtrAllocator Arena;
 llvm::UniqueStringSaver Strings(Arena);
@@ -54,8 +58,11 @@
   }
 
 public:
-  IndexClient(std::shared_ptr Channel)
-  : Stub(remote::SymbolIndex::NewStub(Channel)) {}
+  IndexClient(
+  std::shared_ptr Channel,
+  std::chrono::milliseconds DeadlineTime = std::chrono::milliseconds(1000))
+  : Stub(remote::SymbolIndex::NewStub(Channel)),
+DeadlineWaitingTime(DeadlineTime) {}
 
   void lookup(const clangd::LookupRequest &Request,
   llvm::function_ref Callback) const 
{
@@ -84,6 +91,8 @@
 
 private:
   std::unique_ptr Stub;
+  // Each request will be terminated if it takes too long.
+  std::chrono::milliseconds DeadlineWaitingTime;
 };
 
 } // namespace


Index: clang-tools-extra/clangd/index/remote/Client.cpp
===
--- clang-tools-extra/clangd/index/remote/Client.cpp
+++ clang-tools-extra/clangd/index/remote/Client.cpp
@@ -15,6 +15,8 @@
 #include "support/Logger.h"
 #include "support/Trace.h"
 
+#include 
+
 namespace clang {
 namespace clangd {
 namespace remote {
@@ -25,7 +27,6 @@
   using StreamingCall = std::unique_ptr> (
   remote::SymbolIndex::Stub::*)(grpc::ClientContext *, const RequestT &);
 
-  // FIXME(kirillbobyrev): Set deadlines for requests.
   template 
   bool streamRPC(ClangdRequestT Request,
@@ -35,6 +36,9 @@
 trace::Span Tracer(RequestT::descriptor()->name());
 const auto RPCRequest = toProtobuf(Request);
 grpc::ClientContext Context;
+std::chrono::system_clock::time_point Deadline =
+std::chrono::system_clock::now() + DeadlineWaitingTime;
+Context.set_deadline(Deadline);
 auto Reader = (Stub.get()->*RPCCall)(&Context, RPCRequest);
 llvm::BumpPtrAllocator Arena;
 llvm::UniqueStringSaver Strings(Arena);
@@ -54,8 +58,11 @@
   }
 
 public:
-  IndexClient(std::shared_ptr Channel)
-  : Stub(remote::SymbolIndex::NewStub(Channel)) {}
+  IndexClient(
+  std::shared_ptr Channel,
+  std::chrono::milliseconds DeadlineTime = std::chrono::milliseconds(1000))
+  : Stub(remote::SymbolIndex::NewStub(Channel)),
+DeadlineWaitingTime(DeadlineTime) {}
 
   void lookup(const clangd::LookupRequest &Request,
   llvm::function_ref Callback) const {
@@ -84,6 +91,8 @@
 
 private:
   std::unique_ptr Stub;
+  // Each request will be terminated if it takes too long.
+  std::chrono::milliseconds DeadlineWaitingTime;
 };
 
 } // namespace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 22a3e40 - [clangd] Set gRPC deadlines to all remote index requests

[Kirill Bobyrev via cfe-commits]

Author: Kirill Bobyrev
Date: 2020-07-01T12:46:29+02:00
New Revision: 22a3e4055f4382e8ebbf67705501e6969c6b398b

URL: 
https://github.com/llvm/llvm-project/commit/22a3e4055f4382e8ebbf67705501e6969c6b398b
DIFF: 
https://github.com/llvm/llvm-project/commit/22a3e4055f4382e8ebbf67705501e6969c6b398b.diff

LOG: [clangd] Set gRPC deadlines to all remote index requests

Summary: "TL;DR: Always set a deadline.", https://grpc.io/blog/deadlines/

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, 
cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/index/remote/Client.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/index/remote/Client.cpp 
b/clang-tools-extra/clangd/index/remote/Client.cpp
index 366a37a135a4..c43afd2573a9 100644
--- a/clang-tools-extra/clangd/index/remote/Client.cpp
+++ b/clang-tools-extra/clangd/index/remote/Client.cpp
@@ -15,6 +15,8 @@
 #include "support/Logger.h"
 #include "support/Trace.h"
 
+#include 
+
 namespace clang {
 namespace clangd {
 namespace remote {
@@ -25,7 +27,6 @@ class IndexClient : public clangd::SymbolIndex {
   using StreamingCall = std::unique_ptr> (
   remote::SymbolIndex::Stub::*)(grpc::ClientContext *, const RequestT &);
 
-  // FIXME(kirillbobyrev): Set deadlines for requests.
   template 
   bool streamRPC(ClangdRequestT Request,
@@ -35,6 +36,9 @@ class IndexClient : public clangd::SymbolIndex {
 trace::Span Tracer(RequestT::descriptor()->name());
 const auto RPCRequest = toProtobuf(Request);
 grpc::ClientContext Context;
+std::chrono::system_clock::time_point Deadline =
+std::chrono::system_clock::now() + DeadlineWaitingTime;
+Context.set_deadline(Deadline);
 auto Reader = (Stub.get()->*RPCCall)(&Context, RPCRequest);
 llvm::BumpPtrAllocator Arena;
 llvm::UniqueStringSaver Strings(Arena);
@@ -54,8 +58,11 @@ class IndexClient : public clangd::SymbolIndex {
   }
 
 public:
-  IndexClient(std::shared_ptr Channel)
-  : Stub(remote::SymbolIndex::NewStub(Channel)) {}
+  IndexClient(
+  std::shared_ptr Channel,
+  std::chrono::milliseconds DeadlineTime = std::chrono::milliseconds(1000))
+  : Stub(remote::SymbolIndex::NewStub(Channel)),
+DeadlineWaitingTime(DeadlineTime) {}
 
   void lookup(const clangd::LookupRequest &Request,
   llvm::function_ref Callback) const 
{
@@ -84,6 +91,8 @@ class IndexClient : public clangd::SymbolIndex {
 
 private:
   std::unique_ptr Stub;
+  // Each request will be terminated if it takes too long.
+  std::chrono::milliseconds DeadlineWaitingTime;
 };
 
 } // namespace



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

[clang] c79745e - [Analyzer] Quick fix for broken tests on Windows

[Adam Balogh via cfe-commits]

Author: Adam Balogh
Date: 2020-07-01T12:52:47+02:00
New Revision: c79745ed48f3e22e9c5fdfa070bceecf7590896c

URL: 
https://github.com/llvm/llvm-project/commit/c79745ed48f3e22e9c5fdfa070bceecf7590896c
DIFF: 
https://github.com/llvm/llvm-project/commit/c79745ed48f3e22e9c5fdfa070bceecf7590896c.diff

LOG: [Analyzer] Quick fix for broken tests on Windows

Added: 


Modified: 
clang/test/Analysis/iterator-modeling.cpp

Removed: 




diff  --git a/clang/test/Analysis/iterator-modeling.cpp 
b/clang/test/Analysis/iterator-modeling.cpp
index f522dded37a6..f19848b8dc93 100644
--- a/clang/test/Analysis/iterator-modeling.cpp
+++ b/clang/test/Analysis/iterator-modeling.cpp
@@ -1976,26 +1976,23 @@ void clang_analyzer_printState();
 
 void print_state(std::vector &V) {
   const auto i0 = V.cbegin();
-  const auto i1 = V.cbegin() + 1;
   clang_analyzer_printState();
 
   // CHECK:  "checker_messages": [
   // CHECK:   { "checker": "alpha.cplusplus.IteratorModeling", "messages": [
   // CHECK-NEXT: "Iterator Positions :",
   // CHECK-NEXT: "i0 : Valid ; Container == 
SymRegion{reg_$[[#]] & V>} ; Offset == conj_$[[#]]{long, 
LC[[#]], S[[#]], #[[#]]}"
-  // CHECK-NEXT: "i1 : Valid ; Container == 
SymRegion{reg_$[[#]] & V>} ; Offset == (conj_$[[#]]{long, 
LC[[#]], S[[#]], #[[#]]}) + 1"
   // CHECK-NEXT:   ]}
 
   *i0;
-  *i1;
-  const auto i2 = V.cend();
+  const auto i1 = V.cend();
   clang_analyzer_printState();
 
   // CHECK:  "checker_messages": [
   // CHECK:   { "checker": "alpha.cplusplus.IteratorModeling", "messages": [
   // CHECK-NEXT: "Iterator Positions :",
-  // CHECK-NEXT: "i2 : Valid ; Container == 
SymRegion{reg_$[[#]] & V>} ; Offset == conj_$[[#]]{long, 
LC[[#]], S[[#]], #[[#]]}"
+  // CHECK-NEXT: "i1 : Valid ; Container == 
SymRegion{reg_$[[#]] & V>} ; Offset == conj_$[[#]]{long, 
LC[[#]], S[[#]], #[[#]]}"
   // CHECK-NEXT:   ]}
 
-  *i2;
+  *i1;
 }



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

[clang] 36aaffb - Fix Wdocumentation warnings due to outdated parameter list. NFC.

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2020-07-01T12:01:18+01:00
New Revision: 36aaffbf56913ebe1e3987d7d0ac76573be65cbc

URL: 
https://github.com/llvm/llvm-project/commit/36aaffbf56913ebe1e3987d7d0ac76573be65cbc
DIFF: 
https://github.com/llvm/llvm-project/commit/36aaffbf56913ebe1e3987d7d0ac76573be65cbc.diff

LOG: Fix Wdocumentation warnings due to outdated parameter list. NFC.

Added: 


Modified: 
clang/lib/CodeGen/CGDecl.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index e4cb849e79d1..09593531af83 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/lib/CodeGen/CGDecl.cpp
@@ -1880,9 +1880,7 @@ void CodeGenFunction::EmitAutoVarInit(const 
AutoVarEmission &emission) {
 ///
 /// \param init the initializing expression
 /// \param D the object to act as if we're initializing
-/// \param loc the address to initialize; its type is a pointer
-///   to the LLVM mapping of the object's type
-/// \param alignment the alignment of the address
+/// \param lvalue the lvalue to initialize
 /// \param capturedByInit true if \p D is a __block variable
 ///   whose address is potentially changed by the initializer
 void CodeGenFunction::EmitExprAsInit(const Expr *init, const ValueDecl *D,



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

[PATCH] D82185: [Analyzer] Handle pointer implemented as iterators in iterator checkers

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In D82185#2124948 , @thakis wrote:

> This (or a follow-up) broke tests on windows: 
> http://45.33.8.238/win/18877/step_7.txt
>
> Please take a look and revert for now if it takes a while to fix.


It was this one: D82385 . I reverted that test 
function so one of the fixes now remains untested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82185



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

[PATCH] D82940: [ASTReader][ASTWriter][PCH][Modules] Fix (de)serialization of SwitchCases

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman requested changes to this revision.
aaron.ballman added a comment.
This revision now requires changes to proceed.

In D82940#2125005 , @riccibruno wrote:

> In D82940#2124955 , @aaron.ballman 
> wrote:
>
> > LGTM, thank you for the fix!
>
>
> Wait, I am not sure that it is the right fix. The body of the lambda should 
> not be different from the body of the `CXXMethodDecl`.


I'm not certain I understand, but I'm removing my LG while we figure it out. 
Can you expound a bit further?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82940



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay marked 3 inline comments as done.
MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:13540
   CHECK_PARSE_BOOL(AlignConsecutiveMacros);
+  CHECK_PARSE_BOOL(AlwaysBreakBeforeConceptDeclarations);
   CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);

curdeius wrote:
> Format: leading whitespace. Should it be in the same "group" as 
> `AlignConsecutiveMacros`?
I might be misunderstanding your comment, but the list should alphabetic but 
now I realize it should be after the `Allows`



Comment at: clang/unittests/Format/FormatTest.cpp:16745
+   "::std::is_copy_constructable and "
+   "::std::is_move_constructable and\n"
+   "requires (T c) {\n"

curdeius wrote:
> Isn't it a strange indentation?
there is no newline on the line above, but I clang-format the tests so it wraps 
it, I know visually its confusing


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

https://reviews.llvm.org/D79773



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

[PATCH] D82706: [ASTMatchers] Enhanced support for matchers taking Regex arguments

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/ASTMatchers/Dynamic/Marshallers.cpp:124
+llvm::Optional getRegexFlag(llvm::StringRef Flag) {
+  for (auto &StringFlag : RegexMap) {
+if (Flag == StringFlag.first)

`const auto &` (same below)



Comment at: clang/lib/ASTMatchers/Dynamic/Marshallers.cpp:158
+  }
+  if (!Any)
+return None; // Without this an empty string would return "NoFlags",

I would probably simplify this (untested):
```
SmallVector Split;
llvm::Optional Flag;

Flags.split(Split, '|', -1, false);
for (StringRef OrFlag : Split) {
  if (llvm::Optional NextFlag =
getRegexFlag(OrFlag.trim())) {
Flag = Flag.getValueOr(llvm::Regex::NoFlags) | *NextFlag;
  }
}
return Flag;
```
Similar below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82706



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

[PATCH] D77150: [Analyzer] New Option for ContainerModeling: AggressiveEraseModeling

[Balogh , Ádám via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:647-653
 CmdLineOption
   ]>,

Szelethus wrote:
> Szelethus wrote:
> > NoQ wrote:
> > > baloghadamsoftware wrote:
> > > > Szelethus wrote:
> > > > > Ah, okay, I see which one you refer to. We should totally make this 
> > > > > non-user facing as well. 
> > > > The option is not about state split! It is for choosing between the 
> > > > (default) conservative approach and a more aggressive one. It is 
> > > > absolutely for the user to set. Some users prefer less false positive 
> > > > for the price of losing true positives. However, some other users 
> > > > prefer more true positives for the price of additional false positives. 
> > > > This is why we have checker options to be able to serve both groups.
> > > > 
> > > > This feature was explicitly requested by our users who first disabled 
> > > > iterator checkers because of the too many false positives but later 
> > > > re-enabled them because they run into a bug in a production system 
> > > > which could have been prevented by enabling them. However, they run 
> > > > into another bug that our checker did not find because of its 
> > > > conservative behavior. They requested a more aggressive one but we must 
> > > > not do it for everyone. The concept of the Analyzer is that we apply 
> > > > the conservative approach by default and the aggressive can be enabled 
> > > > by analyzer and checker options.
> > > These options are unlikely to be on-by-default in vanilla clang because 
> > > of having a fundamentally unfixable class of false positives associated 
> > > with them. Honestly, i've never seen users who are actually ok with false 
> > > positives. I've regularly seen users say that they are ok with false 
> > > positives when they wanted their false negatives fixed, but it always 
> > > turned out that they don't understand what they're asking for and they 
> > > quickly changed their mind when they saw the result. But you guys 
> > > basically vend your own tool to your own customers and bear your own 
> > > responsibility and i therefore can't tell you what to do in your realm, 
> > > and i'm ok with having options that allow you to disagree with my choices.
> > > 
> > > inb4, "//`-analyzer-config` is not user-facing!!!11//"
> > >inb4, "-analyzer-config is not user-facing!!!11"
> > I was starting to breath really heavy and type really fast :D
> > 
> > Jokes aside, I'll spill the beans, we don't have a really convenient way to 
> > tweak these options through CodeChecker just yet, so for the time being, 
> > they really aren't, but maybe one day.
> > 
> > With that said, I also agree with you regarding options that 
> > overapproximate the analysis (greater code coverage with more false and 
> > true positives). User facing options should be the ones where the nature of 
> > the bug itself if subjective, like whether we want to check the 
> > initializedness of pointees. We sure can regard them as unwanted, or we 
> > could regard them as totally fine, so an option is a great compromise.
> > 
> > >[...] it always turned out that they don't understand what they're asking 
> > >for [...]
> > 
> > This is why I think this shouldn't be user-facing. You need to be quite 
> > knowledgeable about the analyzer, and IteratorChecker in particular to make 
> > good judgement about enabling the option added in this patch. That doesn't 
> > mean you shouldn't experiment with it, but I'd prefer to not expose it too 
> > much.
> > 
> > I just don't see this feature being that desired by many if they knew the 
> > costs, even if one user really wants it. We could however, in that case, say
> > "Hey, there is this option you can enable that might help you hunt down 
> > more bugs. It does change the actual analysis itself, and experience showed 
> > that it might result in more false positives and could impact performance, 
> > so we opted not to make it user-facing, but it is there, if you feel 
> > adventurous."
> Granted, I might be demonizing the cons of these options too much, but the 
> description doesn't help on understanding it that much either O:)
So you both are saying that I should tell the user that we cannot help, the 
community does not support that he catches this bug in early phase with the 
analyzer. He must catch it in the testing phase for higher costs. This is not 
the best way to build confidence in the analyzer.

In my perception options are used to serve one users need without harming 
others. I never understand why there is so much resistance against introducing 
an option that does not change the default behavior at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77150



___
cfe-commits mailing list

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-assignment.cpp:29
+public:
+  Simple2() : n (0) {
+x = 0.0;

baloghadamsoftware wrote:
> aaron.ballman wrote:
> > baloghadamsoftware wrote:
> > > aaron.ballman wrote:
> > > > By my reading of the core guideline 
> > > > (https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c48-prefer-in-class-initializers-to-member-initializers-in-constructors-for-constant-initializers),
> > > >  it looks like `n` should also be diagnosed because all of the 
> > > > constructors in the class initialize the member to the same constant 
> > > > value. Is there a reason to deviate from the rule (or have I missed 
> > > > something)?
> > > > 
> > > > Also, I'd like to see a test case like:
> > > > ```
> > > > class C {
> > > >   int n;
> > > > public:
> > > >   C() { n = 0; }
> > > >   explicit C(int) { n = 12; }
> > > > };
> > > > ```
> > > This check only cares for initializations inside the body (rule `C.49`, 
> > > but if the proper fix is to convert them to default member initializer 
> > > according to rule `C.48` then we follow that rule in the fix). For 
> > > initializations implemented as constructor member initializers but 
> > > according to `C.48` they should have been implemented as default member 
> > > initializers we already have check `modernize-use-default-member-init`.
> > Thank you for the explanation. I have the feeling that these two rules are 
> > going to have some really weird interactions together. For instance, the 
> > example you added at my request shows behavior I can't easily explain as a 
> > user:
> > ```
> > class Complex19 {
> >   int n;
> >   // CHECK-FIXES: int n{0};
> > public:
> >   Complex19() {
> > n = 0;
> > // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized 
> > in an in-class default member initializer 
> > [cppcoreguidelines-prefer-member-initializer]
> > // CHECK-FIXES: {{^\ *$}}
> >   }
> > 
> >   explicit Complex19(int) {
> > // CHECK-FIXES: Complex19(int) : n(12) {
> > n = 12;
> > // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized 
> > in a member initializer of the constructor 
> > [cppcoreguidelines-prefer-member-initializer]
> > // CHECK-FIXES: {{^\ *$}}
> >   }
> > 
> >   ~Complex19() = default;
> > };
> > ```
> > Despite both constructors having the assignment expression `n = ;` 
> > one gets one diagnostic + fixit and the other gets a different diagnostic + 
> > fixit.
> > 
> > Also, from reading C.49, I don't see anything about using in-class 
> > initialization. I see C.49 as being about avoiding the situation where the 
> > ctor runs a constructor for a data member only to then immediately in the 
> > ctor body make an assignment to that member. You don't need to initialize 
> > then reinitialize when you can use a member init list to construct the 
> > object properly initially.
> Thank you for your comment. While I can agree with you, I think that this 
> check should be fully consistent with `modernize-use-default-member-init`. 
> Thus I tried it on the following example:
> ```
> class C {
>   int n;
> public:
>   C() : n(1) {}
>   C(int nn) : n(nn) {}
> 
>   int get_n() { return n; }
> };
> ```
> I got the following message from Clang-Tidy:
> ```
> warning: use default member initializer for 'n' 
> [modernize-use-default-member-init]
>   int n;
>   ^
>{1}
> ```
> This is no surprise, however. If you take a look on the example in [[ 
> https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c48-prefer-in-class-initializers-to-member-initializers-in-constructors-for-constant-initializers
>  | C.48: Prefer in-class initializers to member initializers in constructors 
> for constant initializers ]] you can see that our code is almost the same as 
> the example considered bad there. So rule C.49 does not speak anything about 
> in-class initialization, but C.48 does, our check enforcing C.49 must not 
> suggest a fixit the the check enforcing C.48 fixes again. We should not force 
> the user to run Clang-Tidy in multiple passes. Maybe we can mention the 
> numbers of the rules in the error messages to make them clearer.
> This is no surprise, however.

Agreed, but it's also a different example from the one I posted where the 
behavior is mysterious.

> We should not force the user to run Clang-Tidy in multiple passes.

Agreed, but the checks are also independent of one another (e.g., I can run one 
but not the other) and that use case needs to be considered as well as the 
combined use case.

I personally don't use the C++ Core Guidelines and so I'm not as well-versed in 
their nuances -- I'm happy to let someone more familiar with the standard sway 
my opinion, but this smells a bit like the two rules are less orthogonal than 
the rule authors may believe. Both rules say to 

[PATCH] D82787: Make RecursiveASTVisitor call WalkUpFrom for unary and binary operators in post-order traversal mode

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:649
+  case BO_##NAME:  
\
+if (isSameMethod(&RecursiveASTVisitor::TraverseBin##NAME,  
\
+ &Derived::TraverseBin##NAME)) {   
\

eduucaldas wrote:
> There's only one place that call PostVisitStmt. Could we pull this 
> conditional there? It is repeated in every macro. It would make this function 
> much simpler, and make clear at call site what is being done
Unfortunately, we can't, because every call to `isSameMethod` has different 
arguments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82787



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

[PATCH] D82844: [clangd] Set gRPC deadlines to all remote index requests

[Kirill Bobyrev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG22a3e4055f43: [clangd] Set gRPC deadlines to all remote 
index requests (authored by kbobyrev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82844

Files:
  clang-tools-extra/clangd/index/remote/Client.cpp


Index: clang-tools-extra/clangd/index/remote/Client.cpp
===
--- clang-tools-extra/clangd/index/remote/Client.cpp
+++ clang-tools-extra/clangd/index/remote/Client.cpp
@@ -15,6 +15,8 @@
 #include "support/Logger.h"
 #include "support/Trace.h"
 
+#include 
+
 namespace clang {
 namespace clangd {
 namespace remote {
@@ -25,7 +27,6 @@
   using StreamingCall = std::unique_ptr> (
   remote::SymbolIndex::Stub::*)(grpc::ClientContext *, const RequestT &);
 
-  // FIXME(kirillbobyrev): Set deadlines for requests.
   template 
   bool streamRPC(ClangdRequestT Request,
@@ -35,6 +36,9 @@
 trace::Span Tracer(RequestT::descriptor()->name());
 const auto RPCRequest = toProtobuf(Request);
 grpc::ClientContext Context;
+std::chrono::system_clock::time_point Deadline =
+std::chrono::system_clock::now() + DeadlineWaitingTime;
+Context.set_deadline(Deadline);
 auto Reader = (Stub.get()->*RPCCall)(&Context, RPCRequest);
 llvm::BumpPtrAllocator Arena;
 llvm::UniqueStringSaver Strings(Arena);
@@ -54,8 +58,11 @@
   }
 
 public:
-  IndexClient(std::shared_ptr Channel)
-  : Stub(remote::SymbolIndex::NewStub(Channel)) {}
+  IndexClient(
+  std::shared_ptr Channel,
+  std::chrono::milliseconds DeadlineTime = std::chrono::milliseconds(1000))
+  : Stub(remote::SymbolIndex::NewStub(Channel)),
+DeadlineWaitingTime(DeadlineTime) {}
 
   void lookup(const clangd::LookupRequest &Request,
   llvm::function_ref Callback) const 
{
@@ -84,6 +91,8 @@
 
 private:
   std::unique_ptr Stub;
+  // Each request will be terminated if it takes too long.
+  std::chrono::milliseconds DeadlineWaitingTime;
 };
 
 } // namespace


Index: clang-tools-extra/clangd/index/remote/Client.cpp
===
--- clang-tools-extra/clangd/index/remote/Client.cpp
+++ clang-tools-extra/clangd/index/remote/Client.cpp
@@ -15,6 +15,8 @@
 #include "support/Logger.h"
 #include "support/Trace.h"
 
+#include 
+
 namespace clang {
 namespace clangd {
 namespace remote {
@@ -25,7 +27,6 @@
   using StreamingCall = std::unique_ptr> (
   remote::SymbolIndex::Stub::*)(grpc::ClientContext *, const RequestT &);
 
-  // FIXME(kirillbobyrev): Set deadlines for requests.
   template 
   bool streamRPC(ClangdRequestT Request,
@@ -35,6 +36,9 @@
 trace::Span Tracer(RequestT::descriptor()->name());
 const auto RPCRequest = toProtobuf(Request);
 grpc::ClientContext Context;
+std::chrono::system_clock::time_point Deadline =
+std::chrono::system_clock::now() + DeadlineWaitingTime;
+Context.set_deadline(Deadline);
 auto Reader = (Stub.get()->*RPCCall)(&Context, RPCRequest);
 llvm::BumpPtrAllocator Arena;
 llvm::UniqueStringSaver Strings(Arena);
@@ -54,8 +58,11 @@
   }
 
 public:
-  IndexClient(std::shared_ptr Channel)
-  : Stub(remote::SymbolIndex::NewStub(Channel)) {}
+  IndexClient(
+  std::shared_ptr Channel,
+  std::chrono::milliseconds DeadlineTime = std::chrono::milliseconds(1000))
+  : Stub(remote::SymbolIndex::NewStub(Channel)),
+DeadlineWaitingTime(DeadlineTime) {}
 
   void lookup(const clangd::LookupRequest &Request,
   llvm::function_ref Callback) const {
@@ -84,6 +91,8 @@
 
 private:
   std::unique_ptr Stub;
+  // Each request will be terminated if it takes too long.
+  std::chrono::milliseconds DeadlineWaitingTime;
 };
 
 } // namespace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay marked an inline comment as done.
MyDeveloperDay added a comment.

In D79773#2124231 , @JohelEGP wrote:

>   constexpr plane(const plane& other) noexcept(
>   (detail::default_allocator_is_nothrow &&
>std::is_nothrow_copy_constructible_v)) requires(std::copyable)
> : plane{to1d(other), other.sz.w}
>   {
>   }
>
>
> Without the parentheses in trailing requires-clauses, all code following the 
> constructor initializer list is further indented.


Just so I'm clear for this and your Allman coment would you show me what you 
are seeing and what you expect to see? just so I understand.


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

https://reviews.llvm.org/D79773



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

[PATCH] D82940: [ASTReader][ASTWriter][PCH][Modules] Fix (de)serialization of SwitchCases

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers added a comment.

This patch addresses a crash we started seeing in our CTU analysis runs as a 
result of 05843dc6ab97a00cbde7aa4f08bf3692eb83109d 
. Gabor, 
maybe I can generate a regression test for the problems that 
05843dc6ab97a00cbde7aa4f08bf3692eb83109d 
 caused as 
a follow up for whatever is committed here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82940



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

[PATCH] D82947: [Driver][ARM] Disable unsupported features when nofp arch extension is used

[Victor Campos via Phabricator via cfe-commits]

vhscampos created this revision.
Herald added subscribers: cfe-commits, danielkiss, kristof.beyls.
Herald added a project: clang.
vhscampos abandoned this revision.

A list of target features is disabled when there is no hardware
floating-point support. This is the case when one of the following
options is passed to clang:

- -mfloat-abi=soft
- -mfpu=none

This option list is missing, however, the extension "+nofp" that can be
specified in -march flags, such as "-march=armv8-a+nofp".

This patch also disables unsupported target features when nofp is passed
to -march.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82947

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp


Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -285,6 +285,35 @@
  (NoMVE == F.rend() || std::distance(MVE, NoMVE) > 0);
 }
 
+static void appendNoFPUnsupportedFeatures(const arm::FloatABI ABI,
+  const unsigned FPUID,
+  const StringRef &ArchName,
+  std::vector &Features) {
+  auto checkFPDisabledInArchName = [](const StringRef &ArchName) {
+SmallVector Split;
+ArchName.split(Split, '+', -1, false);
+return llvm::any_of(
+Split, [](const StringRef &Extension) { return Extension == "nofp"; });
+  };
+
+  if (ABI == arm::FloatABI::Soft) {
+llvm::ARM::getFPUFeatures(llvm::ARM::FK_NONE, Features);
+
+// Disable all features relating to hardware FP, not already disabled by 
the
+// above call.
+Features.insert(Features.end(),
+{"-dotprod", "-fp16fml", "-mve", "-mve.fp", "-fpregs"});
+  } else if (FPUID == llvm::ARM::FK_NONE ||
+ checkFPDisabledInArchName(ArchName)) {
+// -mfpu=none or -march=armvX+nofp is *very* similar to -mfloat-abi=soft,
+// only that it should not disable MVE-I.
+Features.insert(Features.end(), {"-dotprod", "-fp16fml", "-mve.fp"});
+if (!hasIntegerMVE(Features)) {
+  Features.emplace_back("-fpregs");
+}
+  }
+}
+
 void arm::getARMTargetFeatures(const Driver &D, const llvm::Triple &Triple,
const ArgList &Args, ArgStringList &CmdArgs,
std::vector &Features, bool ForAS) {
@@ -455,23 +484,10 @@
   Features.push_back("+fullfp16");
   }
 
-  // Setting -msoft-float/-mfloat-abi=soft effectively disables the FPU (GCC
-  // ignores the -mfpu options in this case).
-  // Note that the ABI can also be set implicitly by the target selected.
-  if (ABI == arm::FloatABI::Soft) {
-llvm::ARM::getFPUFeatures(llvm::ARM::FK_NONE, Features);
-
-// Disable all features relating to hardware FP, not already disabled by 
the
-// above call.
-Features.insert(Features.end(),
-{"-dotprod", "-fp16fml", "-mve", "-mve.fp", "-fpregs"});
-  } else if (FPUID == llvm::ARM::FK_NONE) {
-// -mfpu=none is *very* similar to -mfloat-abi=soft, only that it should 
not
-// disable MVE-I.
-Features.insert(Features.end(), {"-dotprod", "-fp16fml", "-mve.fp"});
-if (!hasIntegerMVE(Features))
-  Features.emplace_back("-fpregs");
-  }
+  // Setting -msoft-float/-mfloat-abi=soft, -mfpu=none, or adding +nofp to
+  // -march effectively disables the FPU (GCC ignores the -mfpu options in this
+  // case). Note that the ABI can also be set implicitly by the target 
selected.
+  appendNoFPUnsupportedFeatures(ABI, FPUID, ArchName, Features);
 
   // En/disable crc code generation.
   if (Arg *A = Args.getLastArg(options::OPT_mcrc, options::OPT_mnocrc)) {


Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -285,6 +285,35 @@
  (NoMVE == F.rend() || std::distance(MVE, NoMVE) > 0);
 }
 
+static void appendNoFPUnsupportedFeatures(const arm::FloatABI ABI,
+  const unsigned FPUID,
+  const StringRef &ArchName,
+  std::vector &Features) {
+  auto checkFPDisabledInArchName = [](const StringRef &ArchName) {
+SmallVector Split;
+ArchName.split(Split, '+', -1, false);
+return llvm::any_of(
+Split, [](const StringRef &Extension) { return Extension == "nofp"; });
+  };
+
+  if (ABI == arm::FloatABI::Soft) {
+llvm::ARM::getFPUFeatures(llvm::ARM::FK_NONE, Features);
+
+// Disable all features relating to hardware FP, not already disabled by the
+// above call.
+Features.insert(Features.end(),
+{"-dotprod", "-fp16fml", "-mve", "-mve.fp", "-fpregs"});
+  } else if (FPUID == llvm::ARM::FK_NONE ||
+ checkFPDisabledInArchName(ArchNa

[PATCH] D82949: [Driver][ARM] Disable bf16 when hardware FP support is missing

[Victor Campos via Phabricator via cfe-commits]

vhscampos created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
vhscampos retitled this revision from "Disable bf16 when hardware FP support is 
missing" to "[Driver][ARM] Disable bf16 when hardware FP support is missing".
vhscampos edited the summary of this revision.
Herald added subscribers: danielkiss, kristof.beyls.
vhscampos added a reviewer: chill.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82949

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/CodeGen/arm-bf16-softfloat.c


Index: clang/test/CodeGen/arm-bf16-softfloat.c
===
--- clang/test/CodeGen/arm-bf16-softfloat.c
+++ clang/test/CodeGen/arm-bf16-softfloat.c
@@ -1,4 +1,6 @@
-// RUN: not %clang -o %t.out -target arm-arm-eabi -march=armv8-a+bf16 
-mfloat-abi=soft -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target arm-arm-none-eabi -march=armv8-a+bf16 
-mfloat-abi=soft -c %s -o %t 2>&1 | FileCheck %s
+// RUN: not %clang -target arm-arm-none-eabi -march=armv8-a+bf16 -mfpu=none -c 
%s -o %t 2>&1 | FileCheck %s
+// RUN: not %clang -target arm-arm-none-eabi -march=armv8-a+bf16+nofp -c %s -o 
%t 2>&1 | FileCheck %s
 
 // CHECK: error: __bf16 is not supported on this target
 extern __bf16 var;
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -301,13 +301,14 @@
 
 // Disable all features relating to hardware FP, not already disabled by 
the
 // above call.
-Features.insert(Features.end(),
-{"-dotprod", "-fp16fml", "-mve", "-mve.fp", "-fpregs"});
+Features.insert(Features.end(), {"-dotprod", "-fp16fml", "-bf16", "-mve",
+ "-mve.fp", "-fpregs"});
   } else if (FPUID == llvm::ARM::FK_NONE ||
  checkFPDisabledInArchName(ArchName)) {
 // -mfpu=none or -march=armvX+nofp is *very* similar to -mfloat-abi=soft,
 // only that it should not disable MVE-I.
-Features.insert(Features.end(), {"-dotprod", "-fp16fml", "-mve.fp"});
+Features.insert(Features.end(),
+{"-dotprod", "-fp16fml", "-bf16", "-mve.fp"});
 if (!hasIntegerMVE(Features)) {
   Features.emplace_back("-fpregs");
 }


Index: clang/test/CodeGen/arm-bf16-softfloat.c
===
--- clang/test/CodeGen/arm-bf16-softfloat.c
+++ clang/test/CodeGen/arm-bf16-softfloat.c
@@ -1,4 +1,6 @@
-// RUN: not %clang -o %t.out -target arm-arm-eabi -march=armv8-a+bf16 -mfloat-abi=soft -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target arm-arm-none-eabi -march=armv8-a+bf16 -mfloat-abi=soft -c %s -o %t 2>&1 | FileCheck %s
+// RUN: not %clang -target arm-arm-none-eabi -march=armv8-a+bf16 -mfpu=none -c %s -o %t 2>&1 | FileCheck %s
+// RUN: not %clang -target arm-arm-none-eabi -march=armv8-a+bf16+nofp -c %s -o %t 2>&1 | FileCheck %s
 
 // CHECK: error: __bf16 is not supported on this target
 extern __bf16 var;
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -301,13 +301,14 @@
 
 // Disable all features relating to hardware FP, not already disabled by the
 // above call.
-Features.insert(Features.end(),
-{"-dotprod", "-fp16fml", "-mve", "-mve.fp", "-fpregs"});
+Features.insert(Features.end(), {"-dotprod", "-fp16fml", "-bf16", "-mve",
+ "-mve.fp", "-fpregs"});
   } else if (FPUID == llvm::ARM::FK_NONE ||
  checkFPDisabledInArchName(ArchName)) {
 // -mfpu=none or -march=armvX+nofp is *very* similar to -mfloat-abi=soft,
 // only that it should not disable MVE-I.
-Features.insert(Features.end(), {"-dotprod", "-fp16fml", "-mve.fp"});
+Features.insert(Features.end(),
+{"-dotprod", "-fp16fml", "-bf16", "-mve.fp"});
 if (!hasIntegerMVE(Features)) {
   Features.emplace_back("-fpregs");
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82948: [Driver][ARM] Disable unsupported features when nofp arch extension is used

[Victor Campos via Phabricator via cfe-commits]

vhscampos created this revision.
Herald added subscribers: cfe-commits, danielkiss, kristof.beyls.
Herald added a project: clang.
vhscampos added a child revision: D82949: [Driver][ARM] Disable bf16 when 
hardware FP support is missing.
vhscampos added a reviewer: chill.

A list of target features is disabled when there is no hardware
floating-point support. This is the case when one of the following
options is passed to clang:

- -mfloat-abi=soft
- -mfpu=none

This option list is missing, however, the extension "+nofp" that can be
specified in -march flags, such as "-march=armv8-a+nofp".

This patch also disables unsupported target features when nofp is passed
to -march.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82948

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp


Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -285,6 +285,35 @@
  (NoMVE == F.rend() || std::distance(MVE, NoMVE) > 0);
 }
 
+static void appendNoFPUnsupportedFeatures(const arm::FloatABI ABI,
+  const unsigned FPUID,
+  const StringRef &ArchName,
+  std::vector &Features) {
+  auto checkFPDisabledInArchName = [](const StringRef &ArchName) {
+SmallVector Split;
+ArchName.split(Split, '+', -1, false);
+return llvm::any_of(
+Split, [](const StringRef &Extension) { return Extension == "nofp"; });
+  };
+
+  if (ABI == arm::FloatABI::Soft) {
+llvm::ARM::getFPUFeatures(llvm::ARM::FK_NONE, Features);
+
+// Disable all features relating to hardware FP, not already disabled by 
the
+// above call.
+Features.insert(Features.end(),
+{"-dotprod", "-fp16fml", "-mve", "-mve.fp", "-fpregs"});
+  } else if (FPUID == llvm::ARM::FK_NONE ||
+ checkFPDisabledInArchName(ArchName)) {
+// -mfpu=none or -march=armvX+nofp is *very* similar to -mfloat-abi=soft,
+// only that it should not disable MVE-I.
+Features.insert(Features.end(), {"-dotprod", "-fp16fml", "-mve.fp"});
+if (!hasIntegerMVE(Features)) {
+  Features.emplace_back("-fpregs");
+}
+  }
+}
+
 void arm::getARMTargetFeatures(const Driver &D, const llvm::Triple &Triple,
const ArgList &Args, ArgStringList &CmdArgs,
std::vector &Features, bool ForAS) {
@@ -455,23 +484,10 @@
   Features.push_back("+fullfp16");
   }
 
-  // Setting -msoft-float/-mfloat-abi=soft effectively disables the FPU (GCC
-  // ignores the -mfpu options in this case).
-  // Note that the ABI can also be set implicitly by the target selected.
-  if (ABI == arm::FloatABI::Soft) {
-llvm::ARM::getFPUFeatures(llvm::ARM::FK_NONE, Features);
-
-// Disable all features relating to hardware FP, not already disabled by 
the
-// above call.
-Features.insert(Features.end(),
-{"-dotprod", "-fp16fml", "-mve", "-mve.fp", "-fpregs"});
-  } else if (FPUID == llvm::ARM::FK_NONE) {
-// -mfpu=none is *very* similar to -mfloat-abi=soft, only that it should 
not
-// disable MVE-I.
-Features.insert(Features.end(), {"-dotprod", "-fp16fml", "-mve.fp"});
-if (!hasIntegerMVE(Features))
-  Features.emplace_back("-fpregs");
-  }
+  // Setting -msoft-float/-mfloat-abi=soft, -mfpu=none, or adding +nofp to
+  // -march effectively disables the FPU (GCC ignores the -mfpu options in this
+  // case). Note that the ABI can also be set implicitly by the target 
selected.
+  appendNoFPUnsupportedFeatures(ABI, FPUID, ArchName, Features);
 
   // En/disable crc code generation.
   if (Arg *A = Args.getLastArg(options::OPT_mcrc, options::OPT_mnocrc)) {


Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -285,6 +285,35 @@
  (NoMVE == F.rend() || std::distance(MVE, NoMVE) > 0);
 }
 
+static void appendNoFPUnsupportedFeatures(const arm::FloatABI ABI,
+  const unsigned FPUID,
+  const StringRef &ArchName,
+  std::vector &Features) {
+  auto checkFPDisabledInArchName = [](const StringRef &ArchName) {
+SmallVector Split;
+ArchName.split(Split, '+', -1, false);
+return llvm::any_of(
+Split, [](const StringRef &Extension) { return Extension == "nofp"; });
+  };
+
+  if (ABI == arm::FloatABI::Soft) {
+llvm::ARM::getFPUFeatures(llvm::ARM::FK_NONE, Features);
+
+// Disable all features relating to hardware FP, not already disabled by the
+// above call.
+Features.insert(Features.end(),
+{"-dotprod", "-fp16fml", "-mve", "-mve.fp

[PATCH] D82921: Removed a RecursiveASTVisitor feature to visit operator kinds with different methods

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM, I think the complexity argument is reasonable (and I appreciate the 
information showing this also reduces binary size at the same time).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82921



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

[PATCH] D82946: [Driver][ARM] Disable bf16 when hardware FP support is missing

[Victor Campos via Phabricator via cfe-commits]

vhscampos created this revision.
Herald added subscribers: cfe-commits, danielkiss, kristof.beyls.
Herald added a project: clang.
vhscampos abandoned this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82946

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/CodeGen/arm-bf16-softfloat.c


Index: clang/test/CodeGen/arm-bf16-softfloat.c
===
--- clang/test/CodeGen/arm-bf16-softfloat.c
+++ clang/test/CodeGen/arm-bf16-softfloat.c
@@ -1,4 +1,6 @@
-// RUN: not %clang -o %t.out -target arm-arm-eabi -march=armv8-a+bf16 
-mfloat-abi=soft -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target arm-arm-none-eabi -march=armv8-a+bf16 
-mfloat-abi=soft -c %s -o %t 2>&1 | FileCheck %s
+// RUN: not %clang -target arm-arm-none-eabi -march=armv8-a+bf16 -mfpu=none -c 
%s -o %t 2>&1 | FileCheck %s
+// RUN: not %clang -target arm-arm-none-eabi -march=armv8-a+bf16+nofp -c %s -o 
%t 2>&1 | FileCheck %s
 
 // CHECK: error: __bf16 is not supported on this target
 extern __bf16 var;
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -301,13 +301,14 @@
 
 // Disable all features relating to hardware FP, not already disabled by 
the
 // above call.
-Features.insert(Features.end(),
-{"-dotprod", "-fp16fml", "-mve", "-mve.fp", "-fpregs"});
+Features.insert(Features.end(), {"-dotprod", "-fp16fml", "-bf16", "-mve",
+ "-mve.fp", "-fpregs"});
   } else if (FPUID == llvm::ARM::FK_NONE ||
  checkFPDisabledInArchName(ArchName)) {
 // -mfpu=none or -march=armvX+nofp is *very* similar to -mfloat-abi=soft,
 // only that it should not disable MVE-I.
-Features.insert(Features.end(), {"-dotprod", "-fp16fml", "-mve.fp"});
+Features.insert(Features.end(),
+{"-dotprod", "-fp16fml", "-bf16", "-mve.fp"});
 if (!hasIntegerMVE(Features)) {
   Features.emplace_back("-fpregs");
 }


Index: clang/test/CodeGen/arm-bf16-softfloat.c
===
--- clang/test/CodeGen/arm-bf16-softfloat.c
+++ clang/test/CodeGen/arm-bf16-softfloat.c
@@ -1,4 +1,6 @@
-// RUN: not %clang -o %t.out -target arm-arm-eabi -march=armv8-a+bf16 -mfloat-abi=soft -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target arm-arm-none-eabi -march=armv8-a+bf16 -mfloat-abi=soft -c %s -o %t 2>&1 | FileCheck %s
+// RUN: not %clang -target arm-arm-none-eabi -march=armv8-a+bf16 -mfpu=none -c %s -o %t 2>&1 | FileCheck %s
+// RUN: not %clang -target arm-arm-none-eabi -march=armv8-a+bf16+nofp -c %s -o %t 2>&1 | FileCheck %s
 
 // CHECK: error: __bf16 is not supported on this target
 extern __bf16 var;
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -301,13 +301,14 @@
 
 // Disable all features relating to hardware FP, not already disabled by the
 // above call.
-Features.insert(Features.end(),
-{"-dotprod", "-fp16fml", "-mve", "-mve.fp", "-fpregs"});
+Features.insert(Features.end(), {"-dotprod", "-fp16fml", "-bf16", "-mve",
+ "-mve.fp", "-fpregs"});
   } else if (FPUID == llvm::ARM::FK_NONE ||
  checkFPDisabledInArchName(ArchName)) {
 // -mfpu=none or -march=armvX+nofp is *very* similar to -mfloat-abi=soft,
 // only that it should not disable MVE-I.
-Features.insert(Features.end(), {"-dotprod", "-fp16fml", "-mve.fp"});
+Features.insert(Features.end(),
+{"-dotprod", "-fp16fml", "-bf16", "-mve.fp"});
 if (!hasIntegerMVE(Features)) {
   Features.emplace_back("-fpregs");
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82729: [clangd] Make sure headers are available in FS inside FindSymbolsTests

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet abandoned this revision.
kadircet added a comment.

abandoning in favor of D82944 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82729



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

[PATCH] D82875: Added tests for RecursiveASTVisitor for AST nodes that are special cased

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp:501
   WalkUpFromStmt IntegerLiteral(1)
-WalkUpFromBinaryOperator BinaryOperator(+)
-  WalkUpFromExpr BinaryOperator(+)
-WalkUpFromStmt BinaryOperator(+)
+WalkUpFromUnaryOperator UnaryOperator(-)
+  WalkUpFromExpr UnaryOperator(-)

eduucaldas wrote:
> A question came to mind: " Does WalkUpFromUnaryMinus walk up to 
> WalkUpFromUnaryOperator? " Perhaps that could be included in a test.
Good question -- but it is already covered by the 
`StmtCallbacks_WalkUpFromUnaryMinus` test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82875



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

[PATCH] D82924: [clang-tidy] fix cppcoreguidelines-init-variables with catch variables

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82924



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

[PATCH] D80802: [RISCV] Upgrade RVV MC to v0.9.

[Ferran Pallarès Roca via Phabricator via cfe-commits]

fpallares added inline comments.



Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:2362
+  }
+  if (TargetFlags & RISCV::VMConstraint) {
+// vadc, vsbc are special cases.

Given that this constraint has no effect when `DestReg != RISCV::V0`, we could 
simplify the logic by adding this to the condition:

```
​  if (TargetFlags & RISCV::VMConstraint && DestReg == RISCV::V0) {
```

Then the `DestReg` checks within the block can go away.



Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:2386
+CheckReg = Inst.getOperand(3).getReg();
 }
+if (DestReg == CheckReg)

With the suggestion above, this could be further simplified to:

```
​if ((TargetFlags & RISCV::OneInput && Inst.getNumOperands() == 3) || 
Inst.getNumOperands() == 4)
​  return Error(Loc, "The destination vector register group cannot overlap"
​" the mask register.");
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80802



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

[PATCH] D82937: Fix `isInfixBinaryOp` that returned true for postfix ++

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas updated this revision to Diff 274769.
eduucaldas added a comment.

better comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82937

Files:
  clang/lib/AST/ExprCXX.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/CXXOperatorCallExprTest.cpp

Index: clang/unittests/Tooling/CXXOperatorCallExprTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/CXXOperatorCallExprTest.cpp
@@ -0,0 +1,77 @@
+//===- unittests/Tooling/CXXOperatorCallExprTest.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Unit tests for the predicates in CXXOperatorCallExpr.
+//
+//===--===//
+
+#include "TestVisitor.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace {
+
+TEST(CXXOperatorPredicatesTest, InfixBinaryOp) {
+  const std::string Code = R"cpp(
+  struct X{
+friend X operator+(X, X);
+  };
+  void test(X x){
+x + x;
+  }
+  )cpp";
+
+  struct Visitor : TestVisitor {
+bool VisitCXXOperatorCallExpr(CXXOperatorCallExpr *E) {
+  EXPECT_TRUE(E->isInfixBinaryOp());
+  return true;
+}
+  } visitor;
+  visitor.runOver(Code);
+}
+
+TEST(CXXOperatorPredicatesTest, CallLikeOp) {
+  const std::string Code = R"cpp(
+  struct X{
+int operator[](int idx);
+  };
+  void test(X x){
+x[1];
+  }
+  )cpp";
+  struct Visitor : TestVisitor {
+bool VisitCXXOperatorCallExpr(CXXOperatorCallExpr *E) {
+  EXPECT_FALSE(E->isInfixBinaryOp());
+  return true;
+}
+  } visitor;
+
+  visitor.runOver(Code);
+}
+
+TEST(CXXOperatorPredicatesTest, PostfixUnaryOp) {
+  const std::string Code = R"cpp(
+  struct X{
+X operator++(int);
+  };
+  void test(X x){
+x++;
+  }
+  )cpp";
+
+  struct Visitor : TestVisitor {
+bool VisitCXXOperatorCallExpr(CXXOperatorCallExpr *E) {
+  EXPECT_FALSE(E->isInfixBinaryOp());
+  return true;
+}
+  } visitor;
+
+  visitor.runOver(Code);
+}
+} // namespace
+} // namespace clang
Index: clang/unittests/Tooling/CMakeLists.txt
===
--- clang/unittests/Tooling/CMakeLists.txt
+++ clang/unittests/Tooling/CMakeLists.txt
@@ -18,6 +18,7 @@
   CastExprTest.cpp
   CommentHandlerTest.cpp
   CompilationDatabaseTest.cpp
+  CXXOperatorCallExprTest.cpp
   DependencyScannerTest.cpp
   DiagnosticsYamlTest.cpp
   ExecutionTest.cpp
Index: clang/lib/AST/ExprCXX.cpp
===
--- clang/lib/AST/ExprCXX.cpp
+++ clang/lib/AST/ExprCXX.cpp
@@ -46,15 +46,18 @@
 //===--===//
 
 bool CXXOperatorCallExpr::isInfixBinaryOp() const {
-  // An infix binary operator is any operator with two arguments other than
-  // operator() and operator[]. Note that none of these operators can have
-  // default arguments, so it suffices to check the number of argument
-  // expressions.
   if (getNumArgs() != 2)
 return false;
 
   switch (getOperator()) {
-  case OO_Call: case OO_Subscript:
+  // operator() may have two arguments, but it's not a binary operator
+  case OO_Call:
+  // operator[] takes two arguments but it's not infix
+  case OO_Subscript:
+  // Postfix unary operators (++ and --) take 2 arguments to differ from their
+  // prefix counterparts
+  case OO_PlusPlus:
+  case OO_MinusMinus:
 return false;
   default:
 return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82880: Fix PR35677: UB on __int128_t or __uint128_t template parameters.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added inline comments.



Comment at: clang/lib/AST/StmtPrinter.cpp:1159
+  case BuiltinType::UInt128:
+OS << "Ui128";
+break;

`i128` and `Ui128` are not valid integer literal suffix. The output of 
`StmtPrinter` is intended to be valid C++. Unfortunately here I think that your 
only choice is to print the high and low parts separately. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82880



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

[PATCH] D82940: [ASTReader][ASTWriter][PCH][Modules] Fix (de)serialization of SwitchCases

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

> I'm not certain I understand, but I'm removing my LG while we figure it out. 
> Can you expound a bit further?

Take a look at the AST dump of a small modification of the reduced test case 
which doesn't trigger the assertion:

  void should_not_crash_with_switch_in_lambda() {
switch (1)
default:;
auto f = [] {
  switch (1)
  default:;
};
  }

Before the serialization of `LambdaExpr` for the `LambdaExpr *E`, we have 
`E->getBody() == E->getCallOperator()->getBody()`.
After serialization this is not true anymore because we are writing and reading 
the body directly when visiting the `LambdaExpr`
(the captures have the same problem).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82940



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

[PATCH] D78655: [CUDA][HIP] Let lambda be host device by default

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 274774.
yaxunl marked 4 inline comments as done.
yaxunl added a comment.

revised by Artem's and Paul's comments


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

https://reviews.llvm.org/D78655

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaCUDA.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/test/CodeGenCUDA/lambda.cu
  clang/test/SemaCUDA/Inputs/cuda.h
  clang/test/SemaCUDA/lambda.cu

Index: clang/test/SemaCUDA/lambda.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/lambda.cu
@@ -0,0 +1,73 @@
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify=com %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -fcuda-is-device -verify=com,dev %s
+
+#include "Inputs/cuda.h"
+
+auto global_lambda = [] () { return 123; };
+
+template
+__global__ void kernel(F f) { f(); }
+// dev-note@-1 7{{called by 'kernel<(lambda}}
+
+__host__ __device__ void hd(int x);
+
+class A {
+  int b;
+public:
+  void test() {
+[=](){ hd(b); }();
+
+[&](){ hd(b); }();
+
+kernel<<<1,1>>>([](){ hd(0); });
+
+kernel<<<1,1>>>([=](){ hd(b); });
+// dev-error@-1 {{capture host side class data member by this pointer in device or host device lambda function}}
+
+kernel<<<1,1>>>([&](){ hd(b); });
+// dev-error@-1 {{capture host side class data member by this pointer in device or host device lambda function}}
+
+kernel<<<1,1>>>([&] __device__ (){ hd(b); });
+// dev-error@-1 {{capture host side class data member by this pointer in device or host device lambda function}}
+
+kernel<<<1,1>>>([&](){
+  auto f = [&]{ hd(b); };
+  // dev-error@-1 {{capture host side class data member by this pointer in device or host device lambda function}}
+  f();
+});
+  }
+};
+
+int main(void) {
+  auto lambda_kernel = [&]__global__(){};
+  // com-error@-1 {{kernel function 'operator()' must be a free function or static member function}}
+
+  int b;
+  [&](){ hd(b); }();
+
+  [=, &b](){ hd(b); }();
+
+  kernel<<<1,1>>>(global_lambda);
+
+  kernel<<<1,1>>>([](){ hd(0); });
+
+  kernel<<<1,1>>>([=](){ hd(b); });
+
+  kernel<<<1,1>>>([b](){ hd(b); });
+
+  kernel<<<1,1>>>([&](){ hd(b); });
+  // dev-error@-1 {{capture host variable 'b' by reference in device or host device lambda function}}
+
+  kernel<<<1,1>>>([=, &b](){ hd(b); });
+  // dev-error@-1 {{capture host variable 'b' by reference in device or host device lambda function}}
+
+  kernel<<<1,1>>>([&, b](){ hd(b); });
+
+  kernel<<<1,1>>>([&](){
+  auto f = [&]{ hd(b); };
+  // dev-error@-1 {{capture host variable 'b' by reference in device or host device lambda function}}
+  f();
+  });
+
+  return 0;
+}
Index: clang/test/SemaCUDA/Inputs/cuda.h
===
--- clang/test/SemaCUDA/Inputs/cuda.h
+++ clang/test/SemaCUDA/Inputs/cuda.h
@@ -17,6 +17,19 @@
   __host__ __device__ dim3(unsigned x, unsigned y = 1, unsigned z = 1) : x(x), y(y), z(z) {}
 };
 
+#ifdef __HIP__
+typedef struct hipStream *hipStream_t;
+typedef enum hipError {} hipError_t;
+int hipConfigureCall(dim3 gridSize, dim3 blockSize, size_t sharedSize = 0,
+ hipStream_t stream = 0);
+extern "C" hipError_t __hipPushCallConfiguration(dim3 gridSize, dim3 blockSize,
+ size_t sharedSize = 0,
+ hipStream_t stream = 0);
+extern "C" hipError_t hipLaunchKernel(const void *func, dim3 gridDim,
+  dim3 blockDim, void **args,
+  size_t sharedMem,
+  hipStream_t stream);
+#else
 typedef struct cudaStream *cudaStream_t;
 typedef enum cudaError {} cudaError_t;
 
@@ -29,6 +42,7 @@
 extern "C" cudaError_t cudaLaunchKernel(const void *func, dim3 gridDim,
 dim3 blockDim, void **args,
 size_t sharedMem, cudaStream_t stream);
+#endif
 
 // Host- and device-side placement new overloads.
 void *operator new(__SIZE_TYPE__, void *p) { return p; }
Index: clang/test/CodeGenCUDA/lambda.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/lambda.cu
@@ -0,0 +1,85 @@
+// RUN: %clang_cc1 -x hip -emit-llvm -std=c++11 %s -o - \
+// RUN:   -triple x86_64-linux-gnu \
+// RUN:   | FileCheck -check-prefix=HOST %s
+// RUN: %clang_cc1 -x hip -emit-llvm -std=c++11 %s -o - \
+// RUN:   -triple amdgcn-amd-amdhsa -fcuda-is-device \
+// RUN:   | FileCheck -check-prefix=DEV %s
+
+#include "Inputs/cuda.h"
+
+// Device side kernel name.
+// HOST: @[[KERN_CAPTURE:[0-9]+]] = {{.*}} c"_Z1gIZ12test_capturevEUlvE_EvT_\00"
+// HOST: @[[KERN_RESOLVE:[0-9]+]] = {{.*}} c"_Z1gIZ12test_resolvevEUlvE_EvT_\00"
+
+// Check functions emitted for test_capture in host compi

[PATCH] D82924: [clang-tidy] fix cppcoreguidelines-init-variables with catch variables

[Nathan James via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG669494e9c06c: [clang-tidy] fix 
cppcoreguidelines-init-variables with catch variables (authored by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82924

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables %t -- -- 
-fno-delayed-template-parsing
+// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables %t -- -- 
-fno-delayed-template-parsing -fexceptions
 
 // Ensure that function declarations are not changed.
 void some_func(int x, double d, bool b, const char *p);
@@ -84,3 +84,10 @@
   for (char c : r) {
   }
 }
+
+void catch_variable_decl() {
+  // Expect no warning given here.
+  try {
+  } catch (int X) {
+  }
+}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -42,6 +42,7 @@
   Finder->addMatcher(
   varDecl(unless(hasInitializer(anything())), unless(isInstantiated()),
   isLocalVarDecl(), unless(isStaticLocal()), isDefinition(),
+  unless(hasParent(cxxCatchStmt())),
   optionally(hasParent(declStmt(hasParent(
   
cxxForRangeStmt(hasLoopVariable(varDecl().bind(BadDecl))),
   unless(equalsBoundNode(BadDecl)))


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables %t -- -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables %t -- -- -fno-delayed-template-parsing -fexceptions
 
 // Ensure that function declarations are not changed.
 void some_func(int x, double d, bool b, const char *p);
@@ -84,3 +84,10 @@
   for (char c : r) {
   }
 }
+
+void catch_variable_decl() {
+  // Expect no warning given here.
+  try {
+  } catch (int X) {
+  }
+}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -42,6 +42,7 @@
   Finder->addMatcher(
   varDecl(unless(hasInitializer(anything())), unless(isInstantiated()),
   isLocalVarDecl(), unless(isStaticLocal()), isDefinition(),
+  unless(hasParent(cxxCatchStmt())),
   optionally(hasParent(declStmt(hasParent(
   cxxForRangeStmt(hasLoopVariable(varDecl().bind(BadDecl))),
   unless(equalsBoundNode(BadDecl)))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78655: [CUDA][HIP] Let lambda be host device by default

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 10 inline comments as done.
yaxunl added inline comments.



Comment at: clang/lib/Sema/SemaCUDA.cpp:750
 
+bool Sema::CUDACheckLambdaCapture(CXXMethodDecl *Callee,
+  const sema::Capture &Capture) {

tra wrote:
> What does the return value mean? We don't seem to check it anyways.  If we 
> don't care about the result, perhaps the function should be void.
> If we do, then it would be good to document its purpose and returned values 
> and, probably, rename it to better indicate what is it it's supposed to check.
it should return void. fixed.



Comment at: clang/lib/Sema/SemaLambda.cpp:1783
+
+  if (LangOpts.CUDA && LangOpts.CUDAIsDevice)
+CUDACheckLambdaCapture(CallOperator, From);

tra wrote:
> I would expect Sema-level diags to be produced during both host and device 
> compilation. Some of the diags for HD may need to be postponed until after 
> things have been CodeGen'ed, but the checks should happen nevertheless.
> 
> 
A host device function could be emitted in both host and device compilation. 
The way the deferred diags works is that they are only emitted when the 
function is sure to be emitted in a specific compilation. In host compilation, 
when a host device function is sure to be emitted, it is emitted as host 
function, therefore diags for host compilation and only diags for host 
compilation should be emitted. The same with device compilation.

This makes sense since we do not know if a host device function will be emitted 
in device compilation when we are doing host compilation, since to do that we 
have to go through the whole device compilation whereas currently device 
compilation and host compilation are separate process.

That said, when we emit diags for captures by reference, we should only emit 
them when the lambdas are emitted as device functions. When they are emitted as 
host functions in host compilation, these captures are valid and should not be 
diagnosed.



Comment at: clang/test/CodeGenCUDA/lambda.cu:53
+// DEV:  call void @_ZZ12test_resolvevENKUlvE_clEv
+// DEV-LABE: define internal void @_ZZ12test_resolvevENKUlvE_clEv
+// DEV:  call i32 @_Z10overloadedIiET_v

ashi1 wrote:
> There is a typo here, DEV-LABEL
fixed. thanks



Comment at: clang/test/SemaCUDA/lambda.cu:27
+
+kernel<<<1,1>>>([&](){ hd(b); });
+// dev-error@-1 {{capture host side class data member by this pointer in 
device or host device lambda function}}

pfultz2 wrote:
> Will this still produce diagnostics when the lambda is explicitly 
> `__device__`? Maybe you could add a test case for that.
> 
> ```
> kernel<<<1,1>>>([&]() __device__ { hd(b); });
> ```
yes. added a test


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

https://reviews.llvm.org/D78655



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

[PATCH] D82954: Fix crash on overloaded postfix unary operators due to invalid sloc

[Eduardo Caldas via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82954

Files:
  clang/include/clang/AST/ExprCXX.h
  clang/lib/AST/ExprCXX.cpp
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -2268,6 +2268,64 @@
 )txt"));
 }
 
+TEST_P(SyntaxTreeTest, UserDefinedUnaryPostfixOperator) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqual(
+  R"cpp(
+struct X {
+  X operator++(int);
+};
+void test(X x) {
+  x++;
+}
+)cpp",
+  R"txt(
+*: TranslationUnit
+|-SimpleDeclaration
+| |-struct
+| |-X
+| |-{
+| |-SimpleDeclaration
+| | |-X
+| | |-SimpleDeclarator
+| | | |-operator
+| | | |-++
+| | | `-ParametersAndQualifiers
+| | |   |-(
+| | |   |-SimpleDeclaration
+| | |   | `-int
+| | |   `-)
+| | `-;
+| |-}
+| `-;
+`-SimpleDeclaration
+  |-void
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   |-SimpleDeclaration
+  |   | |-X
+  |   | `-SimpleDeclarator
+  |   |   `-x
+  |   `-)
+  `-CompoundStatement
+|-{
+|-ExpressionStatement
+| |-PostfixUnaryOperatorExpression
+| | |-IdExpression
+| | | `-UnqualifiedId
+| | |   `-x
+| | `-IdExpression
+| |   `-UnqualifiedId
+| | `-++
+| `-;
+`-}
+)txt"));
+}
+
 TEST_P(SyntaxTreeTest, MultipleDeclaratorsGrouping) {
   EXPECT_TRUE(treeDumpEqual(
   R"cpp(
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -648,6 +648,8 @@
   }
 
   bool WalkUpFromIntegerLiteral(IntegerLiteral *S) {
+if (S->getLocation().isInvalid())
+  return true;
 Builder.markChildToken(S->getLocation(), syntax::NodeRole::LiteralToken);
 Builder.foldNode(Builder.getExprRange(S),
  new (allocator()) syntax::IntegerLiteralExpression, S);
@@ -733,8 +735,23 @@
   Builder.foldNode(Builder.getExprRange(S),
new (allocator()) syntax::BinaryOperatorExpression, S);
   return true;
-}
-return RecursiveASTVisitor::WalkUpFromCXXOperatorCallExpr(S);
+} else if (S->isUnaryOp()) {
+  Builder.markChildToken(
+  S->getOperatorLoc(),
+  syntax::NodeRole::OperatorExpression_operatorToken);
+  Builder.markExprChild(S->getArg(0),
+syntax::NodeRole::UnaryOperatorExpression_operand);
+  if (S->isPostfixUnaryOp())
+Builder.foldNode(
+Builder.getExprRange(S),
+new (allocator()) syntax::PostfixUnaryOperatorExpression, S);
+  else
+Builder.foldNode(
+Builder.getExprRange(S),
+new (allocator()) syntax::PrefixUnaryOperatorExpression, S);
+  return true;
+} else
+  return RecursiveASTVisitor::WalkUpFromCXXOperatorCallExpr(S);
   }
 
   bool WalkUpFromNamespaceDecl(NamespaceDecl *S) {
Index: clang/lib/AST/ExprCXX.cpp
===
--- clang/lib/AST/ExprCXX.cpp
+++ clang/lib/AST/ExprCXX.cpp
@@ -48,7 +48,6 @@
 bool CXXOperatorCallExpr::isInfixBinaryOp() const {
   if (getNumArgs() != 2)
 return false;
-
   switch (getOperator()) {
   // operator() may have two arguments, but it's not a binary operator
   case OO_Call:
@@ -64,6 +63,30 @@
   }
 }
 
+bool CXXOperatorCallExpr::isPostfixUnaryOp() const {
+  switch (getOperator()) {
+  case OO_PlusPlus:
+  case OO_MinusMinus:
+return getNumArgs() == 2;
+  default:
+return false;
+  }
+}
+
+bool CXXOperatorCallExpr::isPrefixUnaryOp() const {
+  switch (getOperator()) {
+  case OO_Call:
+  case OO_Subscript:
+return false;
+  default:
+return getNumArgs() == 1;
+  }
+}
+
+bool CXXOperatorCallExpr::isUnaryOp() const {
+  return isPrefixUnaryOp() || isPostfixUnaryOp();
+}
+
 CXXRewrittenBinaryOperator::DecomposedForm
 CXXRewrittenBinaryOperator::getDecomposedForm() const {
   DecomposedForm Result = {};
Index: clang/include/clang/AST/ExprCXX.h
===
--- clang/include/clang/AST/ExprCXX.h
+++ clang/include/clang/AST/ExprCXX.h
@@ -121,6 +121,7 @@
Opc == OO_GreaterGreaterEqual || Opc == OO_AmpEqual ||
Opc == OO_CaretEqual || Opc == OO_PipeEqual;
   }
+
   bool isAssignmentOp() const { return isAssignmentOp(getOperator()); }
 
   static bool isComparisonOp(OverloadedOperatorKind Opc) {
@@ -139,7 +140,12 @@
   }
   bool isComparisonOp() const { return isComparisonOp(getOperator()); }
 
-  /// Is this written as an infix binary operator?
+  bool isPostfixUnaryOp() const;
+
+  bool isPrefixUnaryOp() const;
+
+  bool isUnar

[PATCH] D82467: [PowerPC][Power10] Implement Truncate and Store VSX Vector Builtins

[Lei Huang via Phabricator via cfe-commits]

lei added inline comments.



Comment at: clang/test/CodeGen/builtins-ppc-p10vector.c:108
+   signed char *__c) {
+  // CHECK-BE: store i8 %{{.+}}, i8* %{{.+}}, align 1
+  // CHECK-LE: store i8 %{{.+}}, i8* %{{.+}}, align 1

amyk wrote:
> lei wrote:
> > I don't see a run line above that uses `CHECK-BE|CHECK-LE`.  Since all 
> > these look the same, did you mean to use the default `CHECK`?
> You're right - unfortunately in the previous tests that use 
> `CHECK-BE`/`CHECK-LE` that were committed, I did not realize that I forgot to 
> upstream the appropriate RUN lines... If it is okay, I can add them into this 
> patch when I update it. 
Not sure what you mean by appropriate RUN lines.  From the checks you are 
adding seems there is no diff between BE and LE so both can use the default 
`CHECK`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82467



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D79972#2124108 , @cchen wrote:

> @ABataev , I'm considering emitting an extra dimension for a non-contiguous 
> descriptor to support stride in this patch (stride = 1 in array section is 
> just a special case for computing stride, however, the formula computing 
> stride do not change). Do you think I should do it in this patch?
>
> Computing of stride after support stride in array section:
>
>   int arr[5][5][5];
>   #pragma omp target update to(arr[0:2:2][1:2:1][0:2:2]
>  
>   D0: { offset = 0, count = 1, stride = 4 }   
> // offset, count, dimension size always be 0, 1, 1 for this extra 
> dimension, stride is the unit size
>   D1: { offset = 0, count = 2, stride = 4 * 1 * 2 = 8 }   
>  // stride = unit size * (production of dimension size of D0) * D1.stride = 4 
> * 1 * 2 = 8
>   D2: { offset = 0, count = 1, stride = 4 * (1 * 5) * 1 = 20  } 
> // stride = unit size * (production of dimension size of D0, D1) * D2.stride 
> = 4 * 5 * 1 = 20
>   D3: { offset = 0, count = 2, stride = 4 * (1 * 5 * 5) * 2 = 200 }  // 
> stride = unit size * (production of dimension size of D0, D1, D2) * D3.stride 
> = 4 * 25 * 2 = 200
>
>
> For the case in this patch (stride = 1), we can use the same formula for 
> computing stride with extra dimension:
>
>   int arr[5][5][5];
>   #pragma omp target update to(arr[0:2][1:2][0:2]
>  
>   D0: { offset = 0, count = 1, stride = 4 }   
>// offset, count, dimension size always be 0, 1, 1 for this extra 
> dimension, stride is the unit size
>   D1: { offset = 0, count = 2, stride = 4 * 1 * 1 = 4 }   
>  // stride = unit size * (production of dimension size of D0) * D1.stride = 4 
> * 1 * 1 = 4
>   D2: { offset = 0, count = 1, stride = 4 * (1 * 5) * 1 = 20  }// 
> stride = unit size * (production of dimension size of D0, D1) * D2.stride = 4 
> * 5 * 1 = 20
>   D3: { offset = 0, count = 2, stride = 4 * (1 * 5 * 5) * 1 = 100 } // 
> stride = unit size * (production of dimension size of D0, D1, D2) * D3.stride 
> = 4 * 25 * 1 = 100
>
>
> The extra dimension does not affect the runtime implementation at all since 
> runtime will try to merge inner dimensions if they are contiguous. Take the 
> above case for example (arr[0:2][1:2][0:2]):
>  The product of count and stride for D0 is 4 which is the same as the stride 
> of D1 , therefore, runtime just ignores D0.


You can do this patch. But at first, you need to commit the runtime part of the 
patch that supports it, and the part that introduces stride support.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D82921: Removed a RecursiveASTVisitor feature to visit operator kinds with different methods

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel accepted this revision.
ymandel added a comment.

+1 Especially given how trivial it was to fix 
clang/lib/ARCMigrate/TransProperties.cpp, this functionality seems unjustified.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82921



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

[PATCH] D82954: Fix crash on overloaded postfix unary operators due to invalid sloc

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas added a reviewer: gribozavr2.
eduucaldas marked an inline comment as done.
eduucaldas added a comment.

Code that reproduces the crash 
Notice that when using a postfix unary operator ++ one argument is introduced 
to differ it from its prefix counterpart, but that argument is not used. This 
phantom argument shows up in the ClangAST in the form of an `IntegerLiteral` 
with `invalid sloc`. This invalid sloc in a valid AST node was causing the 
SyntaxTree generation to crash.
We can address this problems at two different levels:

1. At the `TraverseCXXOperatorCallExpr`, by overriding it and replacing the 
children iteration to not iterate over this phantom argument.
2. At the `WalkUpFromIntegerLiteral`, by skipping the visitation of the phantom 
node.

We preferred the latter.

1. Cons: We would have to duplicate the implementation of RecursiveASTVisitor 
in BuildTree, to handle this subtle change in traversal. That would add code 
complexity.
2. Cons: We are handling a problem of `CXXOperatorCallExpr` in `IntegerLiteral`.

We chose the latter as, anyways, if an AST node has an invalid sloc, it was 
either a problem with parsing or the node is supposed to be ignored




Comment at: clang/include/clang/AST/ExprCXX.h:143-148
+  bool isPostfixUnaryOp() const;
+
+  bool isPrefixUnaryOp() const;
+
+  bool isUnaryOp() const;
+

Should new additions to CXXOperatorCallExpr go on a different patch?
I'll also add unit tests for those. I'm waiting for review on 
https://reviews.llvm.org/D82937, to do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82954



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

[PATCH] D82940: [ASTReader][ASTWriter][PCH][Modules] Fix (de)serialization of SwitchCases

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D82940#2125183 , @riccibruno wrote:

> > I'm not certain I understand, but I'm removing my LG while we figure it 
> > out. Can you expound a bit further?
>
> Take a look at the AST dump of a small modification of the reduced test case 
> which doesn't trigger the assertion:
>
>   void should_not_crash_with_switch_in_lambda() {
> switch (1)
> default:;
> auto f = [] {
>   switch (1)
>   default:;
> };
>   }
>
>
> Before the serialization of `LambdaExpr` for the `LambdaExpr *E`, we have 
> `E->getBody() == E->getCallOperator()->getBody()`.
>  After serialization this is not true anymore because we are writing and 
> reading the body directly when visiting the `LambdaExpr`
>  (the captures have the same problem).


Ah, thank you for the explanation! That does seem like an issue, good catch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82940



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

[PATCH] D80802: [RISCV] Upgrade RVV MC to v0.9.

[Ferran Pallarès Roca via Phabricator via cfe-commits]

fpallares added inline comments.



Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:2367
+Opcode == RISCV::VADC_VIM || Opcode == RISCV::VSBC_VVM ||
+Opcode == RISCV::VSBC_VXM) {
+  if (DestReg == RISCV::V0)

I think we might not need to treat `vadc` and `vsbc` specially here. Since 
those have 4 operands they should fall on the `else` branch of the next 
`if-else` block:

```
if (Inst.getNumOperands() == 4)
  CheckReg = Inst.getOperand(3).getReg();
```

And that should produce the expected result. Did I miss some other reason why 
those should be treated separately?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80802



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

[PATCH] D82288: [analyzer][StdLibraryFunctionsChecker] Add POSIX file handling functions

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus accepted this revision.
Szelethus added a comment.

The changes proposed in mailing list thread 
(http://lists.llvm.org/pipermail/cfe-dev/2020-June/066017.html) seem ortogonal 
to this change. Sorry for the slack today, I just peeked around in 
MallocChecker to look for conflicts, but it seems like there isn't any :) LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82288



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

[PATCH] D82568: [clang][CrossTU] Invalidate parent map after get cross TU definition.

[Balázs Kéri via Phabricator via cfe-commits]

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



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:723
+  // Parent map is invalidated after changing the AST.
+  ToDecl->getASTContext().getParentMapContext().clear();
+

balazske wrote:
> balazske wrote:
> > martong wrote:
> > > > ... the TU is modified by getCrossTUDefinition the parent map does not
> > > contain the newly imported objects and has to be re-created.
> > > 
> > > I see we clear it here, but when/where will be the parent map re-created?
> > It should be created at any access to it. (Actually I did not check how 
> > this works after the test showed that the fix works.)
> `ASTContext::getParentMapContext` does the job.
More precisely, `ParentMapContext::getParents` re-creates the parent map after 
it was cleared. The parent maps in the `ParentMapContext` are cleared, not the 
parent map pointer in `ASTContext`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82568



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

[PATCH] D82954: Fix crash on overloaded postfix unary operators due to invalid sloc

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added inline comments.



Comment at: clang/lib/AST/ExprCXX.cpp:82
+  default:
+return getNumArgs() == 1;
+  }

This will be true for `operator->`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82954



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

[PATCH] D82940: [ASTReader][ASTWriter][PCH][Modules] Fix (de)serialization of SwitchCases

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In case of the reproducer the problem is that we clear the switch ids in 
GetExternalDeclStmt. But we read the SwitchCase with the same ID twice from the 
same call of `GetExternalDeclStmt` (**//bold-italic//** in the below call 
stacks):

- 1)

clang::ASTReader::RecordSwitchCaseID (this=0x4d5710, SC=0x5022f0, ID=0) at 
../../git/llvm-project/clang/lib/Serialization/ASTReader.cpp:9070
0x70313097 in clang::ASTRecordReader::recordSwitchCaseID 
(this=0x7fff7ca8, SC=0x5022f0, ID=0) at 
../../git/llvm-project/clang/include/clang/Serialization/ASTRecordReader.h:331
0x70300ebc in clang::ASTStmtReader::VisitSwitchCase 
(this=0x7fff7c98, S=0x5022f0) at 
../../git/llvm-project/clang/lib/Serialization/ASTReaderStmt.cpp:165
0x70300f3f in clang::ASTStmtReader::VisitCaseStmt (this=0x7fff7c98, 
S=0x5022f0) at 
../../git/llvm-project/clang/lib/Serialization/ASTReaderStmt.cpp:171
0x7031e768 in clang::StmtVisitorBase::Visit (this=0x7fff7c98, S=0x5022f0) at 
tools/clang/include/clang/AST/StmtNodes.inc:561
0x70312b7c in clang::ASTReader::ReadStmtFromStream (this=0x4d5710, 
F=...) at ../../git/llvm-project/clang/lib/Serialization/ASTReaderStmt.cpp:3896
0x7030eb2e in clang::ASTReader::ReadStmt (this=0x4d5710, F=...) at 
../../git/llvm-project/clang/lib/Serialization/ASTReaderStmt.cpp:2696
0x70312e2d in clang::ASTReader::ReadExpr (this=0x4d5710, F=...) at 
../../git/llvm-project/clang/lib/Serialization/ASTReaderStmt.cpp:2705
0x701cbb10 in clang::ASTRecordReader::readExpr (this=0x7fff8450) at 
../../git/llvm-project/clang/include/clang/Serialization/ASTRecordReader.h:131
0x70293b23 in clang::ASTDeclReader::VisitVarDeclImpl 
(this=0x7fff8408, VD=0x5010d0) at 
../../git/llvm-project/clang/lib/Serialization/ASTReaderDecl.cpp:1419
0x702c40ad in clang::ASTDeclReader::VisitVarDecl (this=0x7fff8408, 
VD=0x5010d0) at 
../../git/llvm-project/clang/lib/Serialization/ASTReaderDecl.cpp:371
0x702be24c in clang::declvisitor::Base::Visit (this=0x7fff8408, D=0x5010d0) at 
tools/clang/include/clang/AST/DeclNodes.inc:453
0x7028e5ca in clang::ASTDeclReader::Visit (this=0x7fff8408, 
D=0x5010d0) at 
../../git/llvm-project/clang/lib/Serialization/ASTReaderDecl.cpp:519
0x702b938f in clang::ASTReader::ReadDeclRecord (this=0x4d5710, ID=29) 
at ../../git/llvm-project/clang/lib/Serialization/ASTReaderDecl.cpp:4044
0x7018c714 in clang::ASTReader::GetDecl (this=0x4d5710, ID=29) at 
../../git/llvm-project/clang/lib/Serialization/ASTReader.cpp:7422
0x7021f9fa in clang::ASTReader::ReadDecl (this=0x4d5710, F=..., 
R=llvm::SmallVector of Size 3, Capacity 64 = {...}, I=@0x7fff9830: 3) at 
../../git/llvm-project/clang/include/clang/Serialization/ASTReader.h:1863
0x7021f9a6 in clang::ASTRecordReader::readDecl (this=0x7fff9818) at 
../../git/llvm-project/clang/include/clang/Serialization/ASTRecordReader.h:192
0x70313d88 in clang::ASTStmtReader::readDecl (this=0x7fff9808) at 
../../git/llvm-project/clang/lib/Serialization/ASTReaderStmt.cpp:92
0x70301d6a in clang::ASTStmtReader::VisitDeclStmt (this=0x7fff9808, 
S=0x5010b0) at 
../../git/llvm-project/clang/lib/Serialization/ASTReaderStmt.cpp:340
0x7031e078 in clang::StmtVisitorBase::Visit (this=0x7fff9808, S=0x5010b0) at 
tools/clang/include/clang/AST/StmtNodes.inc:97
0x70312b7c in clang::ASTReader::ReadStmtFromStream (this=0x4d5710, 
F=...) at ../../git/llvm-project/clang/lib/Serialization/ASTReaderStmt.cpp:3896
**//0x7019a07c in clang::ASTReader::GetExternalDeclStmt (this=0x4d5710, 
Offset=107033) at 
../../git/llvm-project/clang/lib/Serialization/ASTReader.cpp:7476//**

- 2)

clang::ASTReader::RecordSwitchCaseID (this=0x4d5710, SC=0x502440, ID=0) at 
../../git/llvm-project/clang/lib/Serialization/ASTReader.cpp:9070
0x70313097 in clang::ASTRecordReader::recordSwitchCaseID 
(this=0x7fff9818, SC=0x502440, ID=0) at 
../../git/llvm-project/clang/include/clang/Serialization/ASTRecordReader.h:331
0x70300ebc in clang::ASTStmtReader::VisitSwitchCase 
(this=0x7fff9808, S=0x502440) at 
../../git/llvm-project/clang/lib/Serialization/ASTReaderStmt.cpp:165
0x7030102f in clang::ASTStmtReader::VisitDefaultStmt 
(this=0x7fff9808, S=0x502440) at 
../../git/llvm-project/clang/lib/Serialization/ASTReaderStmt.cpp:182
0x7031e780 in clang::StmtVisitorBase::Visit (this=0x7fff9808, S=0x502440) at 
tools/clang/include/clang/AST/StmtNodes.inc:567
0x70312b7c in clang::ASTReader::ReadStmtFromStream (this=0x4d5710, 
F=...) at ../../git/llvm-project/clang/lib/Serialization/ASTReaderStmt.cpp:3896
**//0x7019a07c in clang::ASTReader::GetExternalDeclStmt (this=0x4d5710, 
Offset=107033) at 
../../git/llvm-project/clang/lib/Serialization/ASTReader.cpp:7476
//**

(In Bruno's modified example we always see the 2) call stack, meaning that the 
ids 

[PATCH] D82887: [ARM] Add Cortex-A77 Support for Clang and LLVM

[Luke Geeson via Phabricator via cfe-commits]

LukeGeeson updated this revision to Diff 274791.
LukeGeeson marked 5 inline comments as done.
LukeGeeson added a comment.

- Added A77 CPU Part Number to Host.cpp
- added case to AArch64Subtarget::initializeProperties
- moved  a77 test up in AArch64-cpu
- formatted line for AArch64 Target Parser
- removed whitespace in ARM Target Parser
- refactored ProcessorModel  + sorted next to A76
- Added ProcA77

This should address everything David, please let me know if there is anything 
else


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

https://reviews.llvm.org/D82887

Files:
  clang/test/Driver/aarch64-cpus.c
  clang/test/Driver/arm-cortex-cpus.c
  llvm/include/llvm/Support/AArch64TargetParser.def
  llvm/include/llvm/Support/ARMTargetParser.def
  llvm/lib/Support/Host.cpp
  llvm/lib/Target/AArch64/AArch64.td
  llvm/lib/Target/AArch64/AArch64Subtarget.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/ARM/ARM.td
  llvm/lib/Target/ARM/ARMSubtarget.cpp
  llvm/lib/Target/ARM/ARMSubtarget.h
  llvm/test/CodeGen/AArch64/cpus.ll
  llvm/test/CodeGen/AArch64/remat.ll
  llvm/test/MC/AArch64/armv8.2a-dotprod.s
  llvm/test/MC/ARM/armv8.2a-dotprod-a32.s
  llvm/test/MC/ARM/armv8.2a-dotprod-t32.s
  llvm/test/MC/Disassembler/AArch64/armv8.3a-rcpc.txt
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -256,6 +256,12 @@
  ARM::AEK_HWDIVTHUMB | ARM::AEK_DSP | ARM::AEK_FP16 |
  ARM::AEK_RAS | ARM::AEK_DOTPROD,
  "8.2-A"));
+  EXPECT_TRUE(testARMCPU("cortex-a77", "armv8.2-a", "crypto-neon-fp-armv8",
+ ARM::AEK_CRC | ARM::AEK_SEC | ARM::AEK_MP |
+ ARM::AEK_VIRT | ARM::AEK_HWDIVARM |
+ ARM::AEK_HWDIVTHUMB | ARM::AEK_DSP | ARM::AEK_FP16 |
+ ARM::AEK_RAS | ARM::AEK_DOTPROD,
+ "8.2-A"));
   EXPECT_TRUE(testARMCPU("neoverse-n1", "armv8.2-a", "crypto-neon-fp-armv8",
 ARM::AEK_CRC | ARM::AEK_SEC | ARM::AEK_MP |
 ARM::AEK_VIRT | ARM::AEK_HWDIVARM |
@@ -304,7 +310,7 @@
  "7-S"));
 }
 
-static constexpr unsigned NumARMCPUArchs = 86;
+static constexpr unsigned NumARMCPUArchs = 87;
 
 TEST(TargetParserTest, testARMCPUArchList) {
   SmallVector List;
@@ -853,6 +859,12 @@
   AArch64::AEK_LSE | AArch64::AEK_FP16 | AArch64::AEK_DOTPROD |
   AArch64::AEK_RCPC| AArch64::AEK_SSBS, "8.2-A"));
   EXPECT_TRUE(testAArch64CPU(
+  "cortex-a77", "armv8.2-a", "crypto-neon-fp-armv8",
+  AArch64::AEK_CRC | AArch64::AEK_CRYPTO | AArch64::AEK_FP |
+  AArch64::AEK_RDM | AArch64::AEK_SIMD | AArch64::AEK_RAS |
+  AArch64::AEK_LSE | AArch64::AEK_FP16 | AArch64::AEK_DOTPROD |
+  AArch64::AEK_RCPC | AArch64::AEK_SSBS, "8.2-A"));
+  EXPECT_TRUE(testAArch64CPU(
   "cyclone", "armv8-a", "crypto-neon-fp-armv8",
   AArch64::AEK_CRYPTO | AArch64::AEK_FP | AArch64::AEK_SIMD, "8-A"));
   EXPECT_TRUE(testAArch64CPU(
@@ -990,7 +1002,7 @@
   "8.2-A"));
 }
 
-static constexpr unsigned NumAArch64CPUArchs = 39;
+static constexpr unsigned NumAArch64CPUArchs = 40;
 
 TEST(TargetParserTest, testAArch64CPUArchList) {
   SmallVector List;
Index: llvm/test/MC/Disassembler/AArch64/armv8.3a-rcpc.txt
===
--- llvm/test/MC/Disassembler/AArch64/armv8.3a-rcpc.txt
+++ llvm/test/MC/Disassembler/AArch64/armv8.3a-rcpc.txt
@@ -4,6 +4,7 @@
 # RUN: llvm-mc -triple aarch64-none-linux-gnu -mcpu=cortex-a65 --disassemble < %s | FileCheck %s
 # RUN: llvm-mc -triple aarch64-none-linux-gnu -mcpu=cortex-a65ae --disassemble < %s | FileCheck %s
 # RUN: llvm-mc -triple aarch64-none-linux-gnu -mcpu=cortex-a75 --disassemble < %s | FileCheck %s
+# RUN: llvm-mc -triple aarch64-none-linux-gnu -mcpu=cortex-a77 --disassemble < %s | FileCheck %s
 # RUN: llvm-mc -triple aarch64-none-linux-gnu -mcpu=neoverse-e1 --disassemble < %s | FileCheck %s
 # RUN: llvm-mc -triple aarch64-none-linux-gnu -mcpu=neoverse-n1 --disassemble < %s | FileCheck %s
 
Index: llvm/test/MC/ARM/armv8.2a-dotprod-t32.s
===
--- llvm/test/MC/ARM/armv8.2a-dotprod-t32.s
+++ llvm/test/MC/ARM/armv8.2a-dotprod-t32.s
@@ -2,6 +2,7 @@
 // RUN: llvm-mc -triple thumb -mcpu=cortex-a55 -show-encoding < %s | FileCheck %s  --check-prefix=CHECK
 // RUN: llvm-mc -triple thumb -mcpu=cortex-a75 -show-encoding < %s | FileCheck %s  --check-prefix=CHECK
 // RUN: llvm-mc -triple thumb -mcpu=cortex-a76 -show-encoding < %s | FileCheck %s  --check-prefix=CHECK
+// RUN: llvm-mc -triple thumb -mcpu=cortex-a77 -show-encoding < %s | FileCheck %s --check-prefix=CHECK
 // RUN: llvm-mc -triple thumb -mcpu=neoverse-n1

[PATCH] D82805: [clang] Fix a crash on passing C structure of incompatible type to function with reference parameter

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX marked 3 inline comments as done.
ArcsinX added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:4539
+if (CXXRecordDecl *T2RecordDecl =
+dyn_cast(T2RecordType->getDecl())) {
+  const auto &Conversions = T2RecordDecl->getVisibleConversionFunctions();

riccibruno wrote:
> ArcsinX wrote:
> > riccibruno wrote:
> > > I cannot reproduce this with the provided test case. Can you explain why 
> > > `T2RecordDecl` would not be a `CXXRecordDecl` since we should only get 
> > > there with c++?
> > You can reproduce it with
> > ```
> > bin/clang -xc --target=arm-unknown-gnu -c ../clang/test/Sema/init-ref-c.c
> > ```
> > 
> > We can get there with C in case of using builtins declared as functions 
> > with reference parameters (e.g. `__builtin_va_end()`)
> Okay I see it now. Thank you.
> 
> I think it would be better to only call `TryRefInitWithConversionFunction` in 
> C++ mode since conversion functions don't exist in C (only the first call on 
> line 4773).
Agree. Fixed.



Comment at: clang/lib/Sema/SemaInit.cpp:4706
Qualifiers T2Quals,
InitializationSequence &Sequence) {
   QualType DestType = Entity.getType();

riccibruno wrote:
> Can you add a comment here explaining that we can get there in C with some 
> builtins?
Added extra description for TryReferenceInitializationCore() with explanation 
that we can get here in C.



Comment at: clang/lib/Sema/SemaInit.cpp:4773
 if (RefRelationship == Sema::Ref_Incompatible && T2->isRecordType() &&
 (isLValueRef || InitCategory.isRValue())) {
   ConvOvlResult = TryRefInitWithConversionFunction(

riccibruno wrote:
> Here
Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82805



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

[PATCH] D82805: [clang] Fix a crash on passing C structure of incompatible type to function with reference parameter

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX updated this revision to Diff 274789.
ArcsinX added a comment.

Try conversion function only in C++.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82805

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/Sema/init-ref-c.c


Index: clang/test/Sema/init-ref-c.c
===
--- /dev/null
+++ clang/test/Sema/init-ref-c.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple arm-unknown-gnu -fsyntax-only -verify %s
+
+void f() {
+  struct EmptyStruct {};
+  struct EmptyStruct S;
+  __builtin_va_end(S); // no-crash, expected-error {{non-const lvalue 
reference to type '__builtin_va_list' cannot bind to a value of unrelated type 
'struct EmptyStruct'}}
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -4693,6 +4693,9 @@
 }
 
 /// Reference initialization without resolving overloaded functions.
+///
+/// We also can get here in C if we call builtin which is declared as
+/// a function with reference arguments (e.g. __builtin_va_end()).
 static void TryReferenceInitializationCore(Sema &S,
const InitializedEntity &Entity,
const InitializationKind &Kind,
@@ -4769,15 +4772,20 @@
 // an rvalue. DR1287 removed the "implicitly" here.
 if (RefRelationship == Sema::Ref_Incompatible && T2->isRecordType() &&
 (isLValueRef || InitCategory.isRValue())) {
-  ConvOvlResult = TryRefInitWithConversionFunction(
-  S, Entity, Kind, Initializer, /*AllowRValues*/ isRValueRef,
-  /*IsLValueRef*/ isLValueRef, Sequence);
-  if (ConvOvlResult == OR_Success)
-return;
-  if (ConvOvlResult != OR_No_Viable_Function)
-Sequence.SetOverloadFailure(
-InitializationSequence::FK_ReferenceInitOverloadFailed,
-ConvOvlResult);
+  if (S.getLangOpts().CPlusPlus) {
+// Try conversion functions only for C++
+ConvOvlResult = TryRefInitWithConversionFunction(
+S, Entity, Kind, Initializer, /*AllowRValues*/ isRValueRef,
+/*IsLValueRef*/ isLValueRef, Sequence);
+if (ConvOvlResult == OR_Success)
+  return;
+if (ConvOvlResult != OR_No_Viable_Function)
+  Sequence.SetOverloadFailure(
+  InitializationSequence::FK_ReferenceInitOverloadFailed,
+  ConvOvlResult);
+  } else {
+ConvOvlResult = OR_No_Viable_Function;
+  }
 }
   }
 


Index: clang/test/Sema/init-ref-c.c
===
--- /dev/null
+++ clang/test/Sema/init-ref-c.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple arm-unknown-gnu -fsyntax-only -verify %s
+
+void f() {
+  struct EmptyStruct {};
+  struct EmptyStruct S;
+  __builtin_va_end(S); // no-crash, expected-error {{non-const lvalue reference to type '__builtin_va_list' cannot bind to a value of unrelated type 'struct EmptyStruct'}}
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -4693,6 +4693,9 @@
 }
 
 /// Reference initialization without resolving overloaded functions.
+///
+/// We also can get here in C if we call builtin which is declared as
+/// a function with reference arguments (e.g. __builtin_va_end()).
 static void TryReferenceInitializationCore(Sema &S,
const InitializedEntity &Entity,
const InitializationKind &Kind,
@@ -4769,15 +4772,20 @@
 // an rvalue. DR1287 removed the "implicitly" here.
 if (RefRelationship == Sema::Ref_Incompatible && T2->isRecordType() &&
 (isLValueRef || InitCategory.isRValue())) {
-  ConvOvlResult = TryRefInitWithConversionFunction(
-  S, Entity, Kind, Initializer, /*AllowRValues*/ isRValueRef,
-  /*IsLValueRef*/ isLValueRef, Sequence);
-  if (ConvOvlResult == OR_Success)
-return;
-  if (ConvOvlResult != OR_No_Viable_Function)
-Sequence.SetOverloadFailure(
-InitializationSequence::FK_ReferenceInitOverloadFailed,
-ConvOvlResult);
+  if (S.getLangOpts().CPlusPlus) {
+// Try conversion functions only for C++
+ConvOvlResult = TryRefInitWithConversionFunction(
+S, Entity, Kind, Initializer, /*AllowRValues*/ isRValueRef,
+/*IsLValueRef*/ isLValueRef, Sequence);
+if (ConvOvlResult == OR_Success)
+  return;
+if (ConvOvlResult != OR_No_Viable_Function)
+  Sequence.SetOverloadFailure(
+  InitializationSequence::FK_ReferenceInitOverloadFailed,
+  ConvOvlResult);
+  } else {
+  

[PATCH] D82960: Add parenthesized expression to SyntaxTree

[Eduardo Caldas via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82960

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -1129,6 +1129,61 @@
 )txt"));
 }
 
+TEST_P(SyntaxTreeTest, ParenExpr) {
+  EXPECT_TRUE(treeDumpEqual(
+  R"cpp(
+void test() {
+  (1);
+  ((1));
+  (1 + (2));
+}
+)cpp",
+  R"txt(
+*: TranslationUnit
+`-SimpleDeclaration
+  |-void
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
+  `-CompoundStatement
+|-{
+|-ExpressionStatement
+| |-ParenExpression
+| | |-(
+| | |-IntegerLiteralExpression
+| | | `-1
+| | `-)
+| `-;
+|-ExpressionStatement
+| |-ParenExpression
+| | |-(
+| | |-ParenExpression
+| | | |-(
+| | | |-IntegerLiteralExpression
+| | | | `-1
+| | | `-)
+| | `-)
+| `-;
+|-ExpressionStatement
+| |-ParenExpression
+| | |-(
+| | |-BinaryOperatorExpression
+| | | |-IntegerLiteralExpression
+| | | | `-1
+| | | |-+
+| | | `-ParenExpression
+| | |   |-(
+| | |   |-IntegerLiteralExpression
+| | |   | `-2
+| | |   `-)
+| | `-)
+| `-;
+`-}
+)txt"));
+}
+
 TEST_P(SyntaxTreeTest, IntegerLiteral) {
   EXPECT_TRUE(treeDumpEqual(
   R"cpp(
@@ -2040,7 +2095,7 @@
 |-{
 |-ExpressionStatement
 | |-BinaryOperatorExpression
-| | |-UnknownExpression
+| | |-ParenExpression
 | | | |-(
 | | | |-BinaryOperatorExpression
 | | | | |-IntegerLiteralExpression
@@ -2050,7 +2105,7 @@
 | | | |   `-2
 | | | `-)
 | | |-*
-| | `-UnknownExpression
+| | `-ParenExpression
 | |   |-(
 | |   |-BinaryOperatorExpression
 | |   | |-IntegerLiteralExpression
Index: clang/lib/Tooling/Syntax/Nodes.cpp
===
--- clang/lib/Tooling/Syntax/Nodes.cpp
+++ clang/lib/Tooling/Syntax/Nodes.cpp
@@ -42,6 +42,8 @@
 return OS << "IdExpression";
   case NodeKind::UnknownStatement:
 return OS << "UnknownStatement";
+  case NodeKind::ParenExpression:
+return OS << "ParenExpression";
   case NodeKind::DeclarationStatement:
 return OS << "DeclarationStatement";
   case NodeKind::EmptyStatement:
@@ -180,6 +182,8 @@
 return OS << "IdExpression_qualifier";
   case syntax::NodeRole::NestedNameSpecifier_specifier:
 return OS << "NestedNameSpecifier_specifier";
+  case syntax::NodeRole::ParenExpression_subExpression:
+return OS << "ParenExpression_subExpression";
   }
   llvm_unreachable("invalid role");
 }
@@ -203,6 +207,21 @@
   findChild(syntax::NodeRole::IdExpression_id));
 }
 
+syntax::Leaf *syntax::ParenExpression::openParen() {
+  return llvm::cast_or_null(
+  findChild(syntax::NodeRole::OpenParen));
+}
+
+syntax::Expression *syntax::ParenExpression::subExpression() {
+  return llvm::cast_or_null(
+  findChild(syntax::NodeRole::ParenExpression_subExpression));
+}
+
+syntax::Leaf *syntax::ParenExpression::closeParen() {
+  return llvm::cast_or_null(
+  findChild(syntax::NodeRole::CloseParen));
+}
+
 syntax::Leaf *syntax::IntegerLiteralExpression::literalToken() {
   return llvm::cast_or_null(
   findChild(syntax::NodeRole::LiteralToken));
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -647,6 +647,16 @@
 return true;
   }
 
+  bool WalkUpFromParenExpr(ParenExpr *S) {
+Builder.markChildToken(S->getLParen(), syntax::NodeRole::OpenParen);
+Builder.markExprChild(S->getSubExpr(),
+  syntax::NodeRole::ParenExpression_subExpression);
+Builder.markChildToken(S->getRParen(), syntax::NodeRole::CloseParen);
+Builder.foldNode(Builder.getExprRange(S),
+ new (allocator()) syntax::ParenExpression, S);
+return true;
+  }
+
   bool WalkUpFromIntegerLiteral(IntegerLiteral *S) {
 Builder.markChildToken(S->getLocation(), syntax::NodeRole::LiteralToken);
 Builder.foldNode(Builder.getExprRange(S),
Index: clang/include/clang/Tooling/Syntax/Nodes.h
===
--- clang/include/clang/Tooling/Syntax/Nodes.h
+++ clang/include/clang/Tooling/Syntax/Nodes.h
@@ -40,6 +40,7 @@
 
   // Expressions.
   UnknownExpression,
+  ParenExpression,
   PrefixUnaryOperatorExpression,
   PostfixUnaryOperatorExpression,
   BinaryOperatorExpression,
@@ -161,7 +162,8 @@
   ParametersAndQualifiers_trailingReturn,
   IdExpr

[PATCH] D82930: [HIP] Fix rocm detection

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:167
+llvm::ErrorOr> VersionFile =
+FS.getBufferForFile(BinPath + "/.hipVersion");
+if (!VersionFile)

I don't think the detection should fail if this is missing


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

https://reviews.llvm.org/D82930



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

[PATCH] D82575: [OpenMP] Additional OpenMP test without version string after upgrading to 5.0

[Saiyedul Islam via Phabricator via cfe-commits]

saiislam updated this revision to Diff 274792.
saiislam added a comment.

[Work in progress] 48 files inspected/corrected. Following 35 are yet to be 
checked:

clang/test/OpenMP/target_parallel_depend_messages.cpp (26 lines)
clang/test/OpenMP/target_parallel_for_depend_messages.cpp (53 lines)
clang/test/OpenMP/target_parallel_for_map_messages.cpp (17 lines)
clang/test/OpenMP/target_parallel_for_simd_depend_messages.cpp (53 lines)
clang/test/OpenMP/target_parallel_for_simd_is_device_ptr_messages.cpp (15 lines)
clang/test/OpenMP/target_parallel_for_simd_map_messages.cpp (17 lines)
clang/test/OpenMP/target_parallel_is_device_ptr_messages.cpp (10 lines)
clang/test/OpenMP/target_parallel_map_messages.cpp (14 lines)
clang/test/OpenMP/target_simd_depend_codegen.cpp (12 lines)
clang/test/OpenMP/target_simd_depend_messages.cpp (53 lines)
clang/test/OpenMP/target_simd_map_messages.cpp (17 lines)
clang/test/OpenMP/target_teams_depend_messages.cpp (22 lines)
clang/test/OpenMP/target_teams_distribute_depend_messages.cpp (33 lines)
clang/test/OpenMP/target_teams_distribute_map_messages.cpp (17 lines)
clang/test/OpenMP/target_teams_distribute_parallel_for_depend_messages.cpp (42 
lines)
clang/test/OpenMP/target_teams_distribute_parallel_for_is_device_ptr_messages.cpp
 (10 lines)
clang/test/OpenMP/target_teams_distribute_parallel_for_map_messages.cpp (13 
lines)
clang/test/OpenMP/target_teams_distribute_parallel_for_schedule_codegen.cpp (48 
lines)
clang/test/OpenMP/target_teams_distribute_parallel_for_simd_depend_messages.cpp 
(42 lines)
clang/test/OpenMP/target_teams_distribute_parallel_for_simd_firstprivate_messages.cpp
 (11 lines)
clang/test/OpenMP/target_teams_distribute_parallel_for_simd_is_device_ptr_messages.cpp
 (11 lines)
clang/test/OpenMP/target_teams_distribute_parallel_for_simd_map_messages.cpp 
(14 lines)
clang/test/OpenMP/target_teams_distribute_parallel_for_simd_schedule_codegen.cpp
 (48 lines)
clang/test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp (99 lines)
clang/test/OpenMP/target_teams_distribute_simd_depend_messages.cpp (33 lines)
clang/test/OpenMP/target_teams_distribute_simd_is_device_ptr_messages.cpp (14 
lines)
clang/test/OpenMP/target_teams_distribute_simd_loop_messages.cpp (34 lines)
clang/test/OpenMP/target_teams_distribute_simd_map_messages.cpp (17 lines)
clang/test/OpenMP/target_teams_is_device_ptr_messages.cpp (14 lines)
clang/test/OpenMP/target_update_depend_messages.cpp (131 lines)
clang/test/OpenMP/teams_distribute_parallel_for_schedule_codegen.cpp (48 lines)
clang/test/OpenMP/teams_distribute_parallel_for_simd_firstprivate_messages.cpp 
(11 lines)
clang/test/OpenMP/teams_distribute_parallel_for_simd_schedule_codegen.cpp (44 
lines)
clang/test/OpenMP/teams_distribute_simd_firstprivate_messages.cpp (10 lines)
clang/test/OpenMP/threadprivate_codegen.cpp (330 lines)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82575

Files:
  clang/test/OpenMP/declare_target_codegen.cpp
  clang/test/OpenMP/declare_target_messages.cpp
  clang/test/OpenMP/declare_variant_implementation_vendor_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_loop_messages.cpp
  clang/test/OpenMP/distribute_simd_loop_messages.cpp
  clang/test/OpenMP/for_codegen.cpp
  clang/test/OpenMP/for_collapse_messages.cpp
  clang/test/OpenMP/for_loop_messages.cpp
  clang/test/OpenMP/for_simd_loop_messages.cpp
  clang/test/OpenMP/master_taskloop_loop_messages.cpp
  clang/test/OpenMP/master_taskloop_simd_loop_messages.cpp
  clang/test/OpenMP/nesting_of_regions.cpp
  clang/test/OpenMP/parallel_for_codegen.cpp
  clang/test/OpenMP/parallel_for_loop_messages.cpp
  clang/test/OpenMP/parallel_for_simd_loop_messages.cpp
  clang/test/OpenMP/parallel_master_taskloop_loop_messages.cpp
  clang/test/OpenMP/parallel_master_taskloop_simd_loop_messages.cpp
  clang/test/OpenMP/simd_loop_messages.cpp
  clang/test/OpenMP/target_depend_messages.cpp
  clang/test/OpenMP/target_enter_data_depend_messages.cpp
  clang/test/OpenMP/target_exit_data_depend_messages.cpp
  clang/test/OpenMP/target_map_codegen.cpp
  clang/test/OpenMP/target_map_messages.cpp
  clang/test/OpenMP/target_messages.cpp
  clang/test/OpenMP/target_parallel_for_loop_messages.cpp
  clang/test/OpenMP/target_teams_distribute_loop_messages.cpp
  
clang/test/OpenMP/target_teams_distribute_parallel_for_lastprivate_messages.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_loop_messages.cpp
  clang/test/OpenMP/taskloop_loop_messages.cpp
  clang/test/OpenMP/taskloop_simd_loop_messages.cpp
  clang/test/OpenMP/teams_distribute_loop_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_loop_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_simd_loop_messages.cpp
  clang/test/OpenMP/teams_distribute_simd_loop_messages.cpp
  clang/test/OpenMP/teams_messages.cpp

Index: clang/test/OpenMP/teams_messages.cpp
===

[clang] 2831a31 - Implement AVX ABI Warning/error

[Erich Keane via cfe-commits]

Author: Erich Keane
Date: 2020-07-01T07:14:31-07:00
New Revision: 2831a317b689c7f005a29f008a8e4c24485c0711

URL: 
https://github.com/llvm/llvm-project/commit/2831a317b689c7f005a29f008a8e4c24485c0711
DIFF: 
https://github.com/llvm/llvm-project/commit/2831a317b689c7f005a29f008a8e4c24485c0711.diff

LOG: Implement AVX ABI Warning/error

The x86-64 "avx" feature changes how >128 bit vector types are passed,
instead of being passed in separate 128 bit registers, they can be
passed in 256 bit registers.

"avx512f" does the same thing, except it switches from 256 bit registers
to 512 bit registers.

The result of both of these is an ABI incompatibility between functions
compiled with and without these features.

This patch implements a warning/error pair upon an attempt to call a
function that would run afoul of this. First, if a function is called
that would have its ABI changed, we issue a warning.

Second, if said call is made in a situation where the caller and callee
are known to have different calling conventions (such as the case of
'target'), we instead issue an error.

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

Added: 
clang/test/CodeGen/target-avx-abi-diag.c

Modified: 
clang/include/clang/Basic/DiagnosticFrontendKinds.td
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/TargetInfo.cpp
clang/lib/CodeGen/TargetInfo.h
clang/test/CodeGen/target-builtin-error-3.c
clang/test/CodeGen/target-builtin-noerror.c

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td 
b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
index 687c60c9c536..83c13e0dbbe0 100644
--- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -240,6 +240,12 @@ def err_function_needs_feature : Error<
   "always_inline function %1 requires target feature '%2', but would "
   "be inlined into function %0 that is compiled without support for '%2'">;
 
+def warn_avx_calling_convention
+: Warning<"AVX vector %select{return|argument}0 of type %1 without '%2' "
+  "enabled changes the ABI">,
+  InGroup>;
+def err_avx_calling_convention : Error;
+
 def err_alias_to_undefined : Error<
   "%select{alias|ifunc}0 must point to a defined "
   "%select{variable or |}1function">;

diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 87242442a57f..7af986981e5b 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -4245,7 +4245,7 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo 
&CallInfo,
   llvm::FunctionType *IRFuncTy = getTypes().GetFunctionType(CallInfo);
 
   const Decl *TargetDecl = Callee.getAbstractInfo().getCalleeDecl().getDecl();
-  if (const FunctionDecl *FD = dyn_cast_or_null(TargetDecl))
+  if (const FunctionDecl *FD = dyn_cast_or_null(TargetDecl)) {
 // We can only guarantee that a function is called from the correct
 // context/function based on the appropriate target attributes,
 // so only check in the case where we have both always_inline and target
@@ -4256,6 +4256,12 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo 
&CallInfo,
 TargetDecl->hasAttr())
   checkTargetFeatures(Loc, FD);
 
+// Some architectures (such as x86-64) have the ABI changed based on
+// attribute-target/features. Give them a chance to diagnose.
+CGM.getTargetCodeGenInfo().checkFunctionCallABI(
+CGM, Loc, dyn_cast_or_null(CurCodeDecl), FD, CallArgs);
+  }
+
 #ifndef NDEBUG
   if (!(CallInfo.isVariadic() && CallInfo.getArgStruct())) {
 // For an inalloca varargs function, we don't expect CallInfo to match the

diff  --git a/clang/lib/CodeGen/TargetInfo.cpp 
b/clang/lib/CodeGen/TargetInfo.cpp
index 24237c460687..7947aff6cc2f 100644
--- a/clang/lib/CodeGen/TargetInfo.cpp
+++ b/clang/lib/CodeGen/TargetInfo.cpp
@@ -20,6 +20,7 @@
 #include "clang/AST/Attr.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/DiagnosticFrontend.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
 #include "clang/CodeGen/SwiftCallingConv.h"
 #include "llvm/ADT/SmallBitVector.h"
@@ -2466,8 +2467,110 @@ class X86_64TargetCodeGenInfo : public 
TargetCodeGenInfo {
   }
 }
   }
+
+  void checkFunctionCallABI(CodeGenModule &CGM, SourceLocation CallLoc,
+const FunctionDecl *Caller,
+const FunctionDecl *Callee,
+const CallArgList &Args) const override;
 };
 
+static void initFeatureMaps(const ASTContext &Ctx,
+llvm::StringMap &CallerMap,
+const FunctionDecl *Caller,
+llvm::StringMap &CalleeMap,
+const FunctionDecl *Callee) {
+  if (CalleeMap.empty() && CallerMap.empty()) {
+// The caller is potentially nullpt

[PATCH] D82960: Add parenthesized expression to SyntaxTree

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas updated this revision to Diff 274795.
eduucaldas added a comment.

Ordering nit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82960

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -1129,6 +1129,61 @@
 )txt"));
 }
 
+TEST_P(SyntaxTreeTest, ParenExpr) {
+  EXPECT_TRUE(treeDumpEqual(
+  R"cpp(
+void test() {
+  (1);
+  ((1));
+  (1 + (2));
+}
+)cpp",
+  R"txt(
+*: TranslationUnit
+`-SimpleDeclaration
+  |-void
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
+  `-CompoundStatement
+|-{
+|-ExpressionStatement
+| |-ParenExpression
+| | |-(
+| | |-IntegerLiteralExpression
+| | | `-1
+| | `-)
+| `-;
+|-ExpressionStatement
+| |-ParenExpression
+| | |-(
+| | |-ParenExpression
+| | | |-(
+| | | |-IntegerLiteralExpression
+| | | | `-1
+| | | `-)
+| | `-)
+| `-;
+|-ExpressionStatement
+| |-ParenExpression
+| | |-(
+| | |-BinaryOperatorExpression
+| | | |-IntegerLiteralExpression
+| | | | `-1
+| | | |-+
+| | | `-ParenExpression
+| | |   |-(
+| | |   |-IntegerLiteralExpression
+| | |   | `-2
+| | |   `-)
+| | `-)
+| `-;
+`-}
+)txt"));
+}
+
 TEST_P(SyntaxTreeTest, IntegerLiteral) {
   EXPECT_TRUE(treeDumpEqual(
   R"cpp(
@@ -2040,7 +2095,7 @@
 |-{
 |-ExpressionStatement
 | |-BinaryOperatorExpression
-| | |-UnknownExpression
+| | |-ParenExpression
 | | | |-(
 | | | |-BinaryOperatorExpression
 | | | | |-IntegerLiteralExpression
@@ -2050,7 +2105,7 @@
 | | | |   `-2
 | | | `-)
 | | |-*
-| | `-UnknownExpression
+| | `-ParenExpression
 | |   |-(
 | |   |-BinaryOperatorExpression
 | |   | |-IntegerLiteralExpression
Index: clang/lib/Tooling/Syntax/Nodes.cpp
===
--- clang/lib/Tooling/Syntax/Nodes.cpp
+++ clang/lib/Tooling/Syntax/Nodes.cpp
@@ -18,6 +18,8 @@
 return OS << "TranslationUnit";
   case NodeKind::UnknownExpression:
 return OS << "UnknownExpression";
+  case NodeKind::ParenExpression:
+return OS << "ParenExpression";
   case NodeKind::IntegerLiteralExpression:
 return OS << "IntegerLiteralExpression";
   case NodeKind::CharacterLiteralExpression:
@@ -180,6 +182,8 @@
 return OS << "IdExpression_qualifier";
   case syntax::NodeRole::NestedNameSpecifier_specifier:
 return OS << "NestedNameSpecifier_specifier";
+  case syntax::NodeRole::ParenExpression_subExpression:
+return OS << "ParenExpression_subExpression";
   }
   llvm_unreachable("invalid role");
 }
@@ -203,6 +207,21 @@
   findChild(syntax::NodeRole::IdExpression_id));
 }
 
+syntax::Leaf *syntax::ParenExpression::openParen() {
+  return llvm::cast_or_null(
+  findChild(syntax::NodeRole::OpenParen));
+}
+
+syntax::Expression *syntax::ParenExpression::subExpression() {
+  return llvm::cast_or_null(
+  findChild(syntax::NodeRole::ParenExpression_subExpression));
+}
+
+syntax::Leaf *syntax::ParenExpression::closeParen() {
+  return llvm::cast_or_null(
+  findChild(syntax::NodeRole::CloseParen));
+}
+
 syntax::Leaf *syntax::IntegerLiteralExpression::literalToken() {
   return llvm::cast_or_null(
   findChild(syntax::NodeRole::LiteralToken));
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -647,6 +647,16 @@
 return true;
   }
 
+  bool WalkUpFromParenExpr(ParenExpr *S) {
+Builder.markChildToken(S->getLParen(), syntax::NodeRole::OpenParen);
+Builder.markExprChild(S->getSubExpr(),
+  syntax::NodeRole::ParenExpression_subExpression);
+Builder.markChildToken(S->getRParen(), syntax::NodeRole::CloseParen);
+Builder.foldNode(Builder.getExprRange(S),
+ new (allocator()) syntax::ParenExpression, S);
+return true;
+  }
+
   bool WalkUpFromIntegerLiteral(IntegerLiteral *S) {
 Builder.markChildToken(S->getLocation(), syntax::NodeRole::LiteralToken);
 Builder.foldNode(Builder.getExprRange(S),
Index: clang/include/clang/Tooling/Syntax/Nodes.h
===
--- clang/include/clang/Tooling/Syntax/Nodes.h
+++ clang/include/clang/Tooling/Syntax/Nodes.h
@@ -43,6 +43,7 @@
   PrefixUnaryOperatorExpression,
   PostfixUnaryOperatorExpression,
   BinaryOperatorExpression,
+  ParenExpression,
   IntegerLiteralExpression,
   Chara

[PATCH] D82960: Add parenthesized expression to SyntaxTree

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas updated this revision to Diff 274796.
eduucaldas added a comment.

nitpick comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82960

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -1129,6 +1129,61 @@
 )txt"));
 }
 
+TEST_P(SyntaxTreeTest, ParenExpr) {
+  EXPECT_TRUE(treeDumpEqual(
+  R"cpp(
+void test() {
+  (1);
+  ((1));
+  (1 + (2));
+}
+)cpp",
+  R"txt(
+*: TranslationUnit
+`-SimpleDeclaration
+  |-void
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
+  `-CompoundStatement
+|-{
+|-ExpressionStatement
+| |-ParenExpression
+| | |-(
+| | |-IntegerLiteralExpression
+| | | `-1
+| | `-)
+| `-;
+|-ExpressionStatement
+| |-ParenExpression
+| | |-(
+| | |-ParenExpression
+| | | |-(
+| | | |-IntegerLiteralExpression
+| | | | `-1
+| | | `-)
+| | `-)
+| `-;
+|-ExpressionStatement
+| |-ParenExpression
+| | |-(
+| | |-BinaryOperatorExpression
+| | | |-IntegerLiteralExpression
+| | | | `-1
+| | | |-+
+| | | `-ParenExpression
+| | |   |-(
+| | |   |-IntegerLiteralExpression
+| | |   | `-2
+| | |   `-)
+| | `-)
+| `-;
+`-}
+)txt"));
+}
+
 TEST_P(SyntaxTreeTest, IntegerLiteral) {
   EXPECT_TRUE(treeDumpEqual(
   R"cpp(
@@ -2040,7 +2095,7 @@
 |-{
 |-ExpressionStatement
 | |-BinaryOperatorExpression
-| | |-UnknownExpression
+| | |-ParenExpression
 | | | |-(
 | | | |-BinaryOperatorExpression
 | | | | |-IntegerLiteralExpression
@@ -2050,7 +2105,7 @@
 | | | |   `-2
 | | | `-)
 | | |-*
-| | `-UnknownExpression
+| | `-ParenExpression
 | |   |-(
 | |   |-BinaryOperatorExpression
 | |   | |-IntegerLiteralExpression
Index: clang/lib/Tooling/Syntax/Nodes.cpp
===
--- clang/lib/Tooling/Syntax/Nodes.cpp
+++ clang/lib/Tooling/Syntax/Nodes.cpp
@@ -18,6 +18,8 @@
 return OS << "TranslationUnit";
   case NodeKind::UnknownExpression:
 return OS << "UnknownExpression";
+  case NodeKind::ParenExpression:
+return OS << "ParenExpression";
   case NodeKind::IntegerLiteralExpression:
 return OS << "IntegerLiteralExpression";
   case NodeKind::CharacterLiteralExpression:
@@ -180,6 +182,8 @@
 return OS << "IdExpression_qualifier";
   case syntax::NodeRole::NestedNameSpecifier_specifier:
 return OS << "NestedNameSpecifier_specifier";
+  case syntax::NodeRole::ParenExpression_subExpression:
+return OS << "ParenExpression_subExpression";
   }
   llvm_unreachable("invalid role");
 }
@@ -203,6 +207,21 @@
   findChild(syntax::NodeRole::IdExpression_id));
 }
 
+syntax::Leaf *syntax::ParenExpression::openParen() {
+  return llvm::cast_or_null(
+  findChild(syntax::NodeRole::OpenParen));
+}
+
+syntax::Expression *syntax::ParenExpression::subExpression() {
+  return llvm::cast_or_null(
+  findChild(syntax::NodeRole::ParenExpression_subExpression));
+}
+
+syntax::Leaf *syntax::ParenExpression::closeParen() {
+  return llvm::cast_or_null(
+  findChild(syntax::NodeRole::CloseParen));
+}
+
 syntax::Leaf *syntax::IntegerLiteralExpression::literalToken() {
   return llvm::cast_or_null(
   findChild(syntax::NodeRole::LiteralToken));
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -647,6 +647,16 @@
 return true;
   }
 
+  bool WalkUpFromParenExpr(ParenExpr *S) {
+Builder.markChildToken(S->getLParen(), syntax::NodeRole::OpenParen);
+Builder.markExprChild(S->getSubExpr(),
+  syntax::NodeRole::ParenExpression_subExpression);
+Builder.markChildToken(S->getRParen(), syntax::NodeRole::CloseParen);
+Builder.foldNode(Builder.getExprRange(S),
+ new (allocator()) syntax::ParenExpression, S);
+return true;
+  }
+
   bool WalkUpFromIntegerLiteral(IntegerLiteral *S) {
 Builder.markChildToken(S->getLocation(), syntax::NodeRole::LiteralToken);
 Builder.foldNode(Builder.getExprRange(S),
Index: clang/include/clang/Tooling/Syntax/Nodes.h
===
--- clang/include/clang/Tooling/Syntax/Nodes.h
+++ clang/include/clang/Tooling/Syntax/Nodes.h
@@ -43,6 +43,7 @@
   PrefixUnaryOperatorExpression,
   PostfixUnaryOperatorExpression,
   BinaryOperatorExpression,
+  ParenExpression,
   IntegerLiteralExpression,
   Ch

[PATCH] D82887: [ARM] Add Cortex-A77 Support for Clang and LLVM

[Dave Green via Phabricator via cfe-commits]

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

Thanks. LGTM.


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

https://reviews.llvm.org/D82887



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

[PATCH] D82562: Implement AVX ABI Warning/error

[Erich Keane via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
erichkeane marked an inline comment as done.
Closed by commit rG2831a317b689: Implement AVX ABI Warning/error (authored by 
erichkeane).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D82562?vs=274450&id=274800#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82562

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/CodeGen/TargetInfo.h
  clang/test/CodeGen/target-avx-abi-diag.c
  clang/test/CodeGen/target-builtin-error-3.c
  clang/test/CodeGen/target-builtin-noerror.c

Index: clang/test/CodeGen/target-builtin-noerror.c
===
--- clang/test/CodeGen/target-builtin-noerror.c
+++ clang/test/CodeGen/target-builtin-noerror.c
@@ -6,15 +6,15 @@
 
 // No warnings.
 extern __m256i a;
-int __attribute__((target("avx"))) bar(__m256i a) {
+int __attribute__((target("avx"))) bar() {
   return _mm256_extract_epi32(a, 3);
 }
 
 int baz() {
-  return bar(a);
+  return bar();
 }
 
-int __attribute__((target("avx"))) qq_avx(__m256i a) {
+int __attribute__((target("avx"))) qq_avx() {
   return _mm256_extract_epi32(a, 3);
 }
 
@@ -25,7 +25,7 @@
 extern __m256i a;
 int qq() {
   if (__builtin_cpu_supports("avx"))
-return qq_avx(a);
+return qq_avx();
   else
 return qq_noavx();
 }
Index: clang/test/CodeGen/target-builtin-error-3.c
===
--- clang/test/CodeGen/target-builtin-error-3.c
+++ clang/test/CodeGen/target-builtin-error-3.c
@@ -18,11 +18,12 @@
   return __extension__ ({ __m256 __a = (a); (__m128i)__builtin_ia32_vcvtps2ph256((__v8sf)__a, (0x00)); }); // expected-error {{'__builtin_ia32_vcvtps2ph256' needs target feature f16c}}
 }
 static inline half16 __attribute__((__overloadable__)) convert_half( float16 a ) {
-  half16 r; 
-  r.lo = convert_half( a.lo); 
+  half16 r;
+  r.lo = convert_half(a.lo);
   return r;
 }
 void avx_test( uint16_t *destData, float16 argbF)
 {
-   ((half16U*)destData)[0] = convert_half(argbF);
+  // expected-warning@+1{{AVX vector argument of type 'float16' (vector of 16 'float' values) without 'avx512f' enabled changes the ABI}}
+  ((half16U *)destData)[0] = convert_half(argbF);
 }
Index: clang/test/CodeGen/target-avx-abi-diag.c
===
--- /dev/null
+++ clang/test/CodeGen/target-avx-abi-diag.c
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -verify=no256,no512 -o - -S
+// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -target-feature +avx -verify=no512 -o - -S
+// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -target-feature +avx512f -verify=both -o - -S
+
+// both-no-diagnostics
+
+typedef short avx512fType __attribute__((vector_size(64)));
+typedef short avx256Type __attribute__((vector_size(32)));
+
+__attribute__((target("avx"))) void takesAvx256(avx256Type t);
+__attribute__((target("avx512f"))) void takesAvx512(avx512fType t);
+void takesAvx256_no_target(avx256Type t);
+void takesAvx512_no_target(avx512fType t);
+
+void variadic(int i, ...);
+__attribute__((target("avx512f"))) void variadic_err(int i, ...);
+
+// If neither side has an attribute, warn.
+void call_warn(void) {
+  avx256Type t1;
+  takesAvx256_no_target(t1); // no256-warning {{AVX vector argument of type 'avx256Type' (vector of 16 'short' values) without 'avx' enabled changes the ABI}}
+
+  avx512fType t2;
+  takesAvx512_no_target(t2); // no512-warning {{AVX vector argument of type 'avx512fType' (vector of 32 'short' values) without 'avx512f' enabled changes the ABI}}
+
+  variadic(1, t1); // no256-warning {{AVX vector argument of type 'avx256Type' (vector of 16 'short' values) without 'avx' enabled changes the ABI}}
+  variadic(3, t2); // no512-warning {{AVX vector argument of type 'avx512fType' (vector of 32 'short' values) without 'avx512f' enabled changes the ABI}}
+}
+
+// If only 1 side has an attribute, error.
+void call_errors(void) {
+  avx256Type t1;
+  takesAvx256(t1); // no256-error {{AVX vector argument of type 'avx256Type' (vector of 16 'short' values) without 'avx' enabled changes the ABI}}
+  avx512fType t2;
+  takesAvx512(t2); // no512-error {{AVX vector argument of type 'avx512fType' (vector of 32 'short' values) without 'avx512f' enabled changes the ABI}}
+
+  variadic_err(1, t1); // no256-error {{AVX vector argument of type 'avx256Type' (vector of 16 'short' values) without 'avx' enabled changes the ABI}}
+  variadic_err(3, t2); // no512-error {{AVX vector argument of type 'avx512fType' (vector of 32 'short' values) without 'avx512f' enabled changes the ABI}}
+}
+
+// These two don't diagnose anything, since these are valid calls.
+__attribute__((target("avx"))) void call_avx256_ok(void) {
+  avx256Type t;
+  takesAvx25

[clang] 19c3552 - Limit x86 test to require target to fix buildbot (from 2831a317b)

[Erich Keane via cfe-commits]

Author: Erich Keane
Date: 2020-07-01T07:35:39-07:00
New Revision: 19c35526d98699db6917cf2a6f0dd3fe7da68926

URL: 
https://github.com/llvm/llvm-project/commit/19c35526d98699db6917cf2a6f0dd3fe7da68926
DIFF: 
https://github.com/llvm/llvm-project/commit/19c35526d98699db6917cf2a6f0dd3fe7da68926.diff

LOG: Limit x86 test to require target to fix buildbot (from 2831a317b)

The modification of the features apparently requires the backend to be
instantiated, so make sure this is required to fix the ARM build bots.

Added: 


Modified: 
clang/test/CodeGen/target-avx-abi-diag.c

Removed: 




diff  --git a/clang/test/CodeGen/target-avx-abi-diag.c 
b/clang/test/CodeGen/target-avx-abi-diag.c
index 5b8074f31310..f3d4462a552d 100644
--- a/clang/test/CodeGen/target-avx-abi-diag.c
+++ b/clang/test/CodeGen/target-avx-abi-diag.c
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -verify=no256,no512 -o - -S
 // RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -target-feature +avx 
-verify=no512 -o - -S
 // RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -target-feature +avx512f 
-verify=both -o - -S
+// REQUIRES: x86-registered-target
 
 // both-no-diagnostics
 



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

[PATCH] D81728: [InstCombine] Add target-specific inst combining

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added inline comments.



Comment at: llvm/include/llvm/Transforms/InstCombine/InstCombiner.h:46
+/// combine them.
+class LLVM_LIBRARY_VISIBILITY InstCombiner {
+public:

lattner wrote:
> I would really rather not make this be a public class - this is a very thick 
> interface.  Can this be cut down to something much smaller than the 
> implementation details of InstCombine?
> 
> If you're curious for a pattern that could be followed, the MLIR AsmParser is 
> a reasonable example.  The parser is spread across a bunch of classes in the 
> lib/ directory:
> https://github.com/llvm/llvm-project/blob/master/mlir/lib/Parser/Parser.cpp
> 
> But then there is a much smaller public API exposed through a header:
> https://github.com/llvm/llvm-project/blob/master/mlir/include/mlir/IR/OpImplementation.h#L229
> 
> 
I agree with the sentiment, but note @Flakebi has split up the `InstCombiner` 
class into `InstCombiner` and `InstCombinerImpl` classes, which addresses those 
concerns already as far as I'm concerned.

Looking through the new `InstCombiner`, aside from methods that are core to the 
workings of InstCombine (modifying instructions while keeping track of the 
Worklist) and methods for accessing the analyses, what's left is:

* A bunch of static methods that should arguably just be global functions in a 
utils header somewhere.
* CreateOverflowTuple and CreateNonTerminatorUnreachable

Moving those methods feels sensible, but is likely to touch a lot of code, so I 
think it would be better to do it in a separate commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81728



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

[clang-tools-extra] c5263a4 - [clangd] Fix race in FileIndex that sometimes temporarily lost updates.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-07-01T16:47:04+02:00
New Revision: c5263a4e84cc7fb7135a7e9e0cf000af264b72c5

URL: 
https://github.com/llvm/llvm-project/commit/c5263a4e84cc7fb7135a7e9e0cf000af264b72c5
DIFF: 
https://github.com/llvm/llvm-project/commit/c5263a4e84cc7fb7135a7e9e0cf000af264b72c5.diff

LOG: [clangd] Fix race in FileIndex that sometimes temporarily lost updates.

Summary:
FileIndex was built out of threadsafe components, so update() didn't have data
races, but wasn't actually correct.

Reviewers: kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/index/FileIndex.cpp
clang-tools-extra/clangd/index/FileIndex.h
clang-tools-extra/clangd/index/Symbol.h
clang-tools-extra/clangd/unittests/FileIndexTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/index/FileIndex.cpp 
b/clang-tools-extra/clangd/index/FileIndex.cpp
index 1a18af1303dd..5f84545d7c73 100644
--- a/clang-tools-extra/clangd/index/FileIndex.cpp
+++ b/clang-tools-extra/clangd/index/FileIndex.cpp
@@ -229,6 +229,7 @@ void FileSymbols::update(llvm::StringRef Key,
  std::unique_ptr Relations,
  bool CountReferences) {
   std::lock_guard Lock(Mutex);
+  ++Version;
   if (!Symbols)
 SymbolsSnapshot.erase(Key);
   else
@@ -248,7 +249,8 @@ void FileSymbols::update(llvm::StringRef Key,
 }
 
 std::unique_ptr
-FileSymbols::buildIndex(IndexType Type, DuplicateHandling DuplicateHandle) {
+FileSymbols::buildIndex(IndexType Type, DuplicateHandling DuplicateHandle,
+size_t *Version) {
   std::vector> SymbolSlabs;
   std::vector> RefSlabs;
   std::vector> RelationSlabs;
@@ -264,6 +266,9 @@ FileSymbols::buildIndex(IndexType Type, DuplicateHandling 
DuplicateHandle) {
 }
 for (const auto &FileAndRelations : RelatiosSnapshot)
   RelationSlabs.push_back(FileAndRelations.second);
+
+if (Version)
+  *Version = this->Version;
   }
   std::vector AllSymbols;
   std::vector SymsStorage;
@@ -390,12 +395,23 @@ void FileIndex::updatePreamble(PathRef Path, 
llvm::StringRef Version,
 std::make_unique(std::move(*IF->Relations)),
 /*CountReferences=*/false);
   }
-  PreambleIndex.reset(
+  size_t IndexVersion = 0;
+  auto NewIndex =
   PreambleSymbols.buildIndex(UseDex ? IndexType::Heavy : IndexType::Light,
- DuplicateHandling::PickOne));
-  vlog("Build dynamic index for header symbols with estimated memory usage of "
-   "{0} bytes",
-   PreambleIndex.estimateMemoryUsage());
+ DuplicateHandling::PickOne, &IndexVersion);
+  {
+std::lock_guard Lock(UpdateIndexMu);
+if (IndexVersion <= PreambleIndexVersion) {
+  // We lost the race, some other thread built a later version.
+  return;
+}
+PreambleIndexVersion = IndexVersion;
+PreambleIndex.reset(std::move(NewIndex));
+vlog(
+"Build dynamic index for header symbols with estimated memory usage of 
"
+"{0} bytes",
+PreambleIndex.estimateMemoryUsage());
+  }
 }
 
 void FileIndex::updateMain(PathRef Path, ParsedAST &AST) {
@@ -405,11 +421,22 @@ void FileIndex::updateMain(PathRef Path, ParsedAST &AST) {
   std::make_unique(std::move(std::get<1>(Contents))),
   std::make_unique(std::move(std::get<2>(Contents))),
   /*CountReferences=*/true);
-  MainFileIndex.reset(
-  MainFileSymbols.buildIndex(IndexType::Light, DuplicateHandling::Merge));
-  vlog("Build dynamic index for main-file symbols with estimated memory usage "
-   "of {0} bytes",
-   MainFileIndex.estimateMemoryUsage());
+  size_t IndexVersion = 0;
+  auto NewIndex = MainFileSymbols.buildIndex(
+  IndexType::Light, DuplicateHandling::Merge, &IndexVersion);
+  {
+std::lock_guard Lock(UpdateIndexMu);
+if (IndexVersion <= MainIndexVersion) {
+  // We lost the race, some other thread built a later version.
+  return;
+}
+MainIndexVersion = IndexVersion;
+MainFileIndex.reset(std::move(NewIndex));
+vlog(
+"Build dynamic index for main-file symbols with estimated memory usage 
"
+"of {0} bytes",
+MainFileIndex.estimateMemoryUsage());
+  }
 }
 
 } // namespace clangd

diff  --git a/clang-tools-extra/clangd/index/FileIndex.h 
b/clang-tools-extra/clangd/index/FileIndex.h
index 65a2c305dca5..e6f8d1ef9e3d 100644
--- a/clang-tools-extra/clangd/index/FileIndex.h
+++ b/clang-tools-extra/clangd/index/FileIndex.h
@@ -81,9 +81,11 @@ class FileSymbols {
   /// The index keeps the slabs alive.
   /// Will count Symbol::References based on number of references in the main
   /// files, while building the index with DuplicateHandling::Merge option.
+  /// Version is populated with an increasing sequence counter.

[PATCH] D82891: [clangd] Fix race in FileIndex that sometimes temporarily lost updates.

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 5 inline comments as done.
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:269
+
+++this->Version;
+if (Version)

kadircet wrote:
> Bumping the version in `update` and just exposing it in `buildIndex` feels 
> more natural, WDYT?
> 
> Might even enable us to get rid of some redundant resets (not that it is 
> expensive ..), imagine a sequencing like:
> 
> - update1
> - update2
> - buildIndex1
> - buildIndex2
> 
> even though both build the same index, they got different version numbers 
> hence both will reset the current index.
Yeah, that makes sense.



Comment at: clang-tools-extra/clangd/index/Symbol.h:190
+  using size_type = size_t;
+  size_type size() const { return Symbols.size(); }
   bool empty() const { return Symbols.empty(); }

kadircet wrote:
> i suppose this is for `testing::SizeIs` right?
Right, gtest is weird. Could live without it, seems harmless though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82891



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

[PATCH] D81098: [OpenMP] Upgrade default version of OpenMP to 5.0

[Saiyedul Islam via Phabricator via cfe-commits]

saiislam added a comment.

In D81098#2123559 , @clementval wrote:

> In D81098#2112159 , @ABataev wrote:
>
> > LG
>
>
> Since this revision landed two tests are failing.
>
> ` libomp.env::kmp_set_dispatch_buf.c` and 
> `libomp.worksharing/for::kmp_set_dispatch_buf.c`. It was also reported on the 
> mailing list 
> (http://lists.llvm.org/pipermail/openmp-dev/2020-June/003507.html). Any idea 
> how we can fix this quickly? @jdoerfert
>
>   OR maybe this is known and will be taken care later?


Thank you @clementval. I have posted a review to temporarily disable these 
tests for 5.0 (they are passing with 4.5): https://reviews.llvm.org/D82963


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81098



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

[PATCH] D82944: [clangd] Switch FindSymbolsTests to use TestTU

[Sam McCall via Phabricator via cfe-commits]

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

LG, thanks!




Comment at: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp:47
 
-ClangdServer::Options optsForTests() {
-  auto ServerOpts = ClangdServer::optsForTest();
-  ServerOpts.WorkspaceRoot = testRoot();
-  ServerOpts.BuildDynamicSymbolIndex = true;
-  return ServerOpts;
-}
-
 class WorkspaceSymbolsTest : public ::testing::Test {
 protected:

I think we should probably just get rid of this fixture in favor of using 
TestTU directly (maybe keep getSymbols as a standalone function).
Fine not to do that in this patch though.



Comment at: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp:300
 namespace {
 class DocumentSymbolsTest : public ::testing::Test {
 protected:

and similar with this one


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82944



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

[PATCH] D82964: [clangd] Config: loading and caching config from disk.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: cfe-commits, usaxena95, jfb, arphaman, jkorous, 
MaskRay, ilya-biryukov, mgorny.
Herald added a project: clang.

The Provider extension point is designed to also be implemented by
ClangdLSPServer (to inject config-over-lsp) and likely by embedders.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82964

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ConfigProvider.cpp
  clang-tools-extra/clangd/ConfigProvider.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp

Index: clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
@@ -0,0 +1,154 @@
+//===-- ConfigProviderTests.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Config.h"
+#include "ConfigProvider.h"
+#include "ConfigTesting.h"
+#include "TestFS.h"
+#include "llvm/Support/SourceMgr.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+
+namespace clang {
+namespace clangd {
+namespace config {
+namespace {
+using ::testing::ElementsAre;
+using ::testing::IsEmpty;
+
+// Provider that appends an arg to compile flags.
+// The arg is prefix, where N is the times getFragments() was called.
+// It also yields a diagnostic each time it's called.
+class FakeProvider : public Provider {
+  std::string Prefix;
+  mutable std::atomic Index = {0};
+
+  std::vector
+  getFragments(const Params &, DiagnosticCallback DC) const override {
+DC(llvm::SMDiagnostic("", llvm::SourceMgr::DK_Error, Prefix));
+CompiledFragment F =
+[Arg(Prefix + std::to_string(++Index))](const Params &P, Config &C) {
+  C.CompileFlags.Edits.push_back(
+  [Arg](std::vector &Argv) { Argv.push_back(Arg); });
+  return true;
+};
+return {F};
+  }
+
+public:
+  FakeProvider(llvm::StringRef Prefix) : Prefix(Prefix) {}
+};
+
+std::vector getAddedArgs(Config &C) {
+  std::vector Argv;
+  for (auto &Edit : C.CompileFlags.Edits)
+Edit(Argv);
+  return Argv;
+}
+
+// The provider from combine() should invoke its providers in order, and not
+// cache their results.
+TEST(ProviderTest, Combine) {
+  CapturedDiags Diags;
+  std::vector> Providers;
+  Providers.push_back(std::make_unique("foo"));
+  Providers.push_back(std::make_unique("bar"));
+  auto Combined = Provider::combine(std::move(Providers));
+  Config Cfg = Combined->getConfig(Params(), Diags.callback());
+  EXPECT_THAT(Diags.Diagnostics,
+  ElementsAre(DiagMessage("foo"), DiagMessage("bar")));
+  EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("foo1", "bar1"));
+  Diags.Diagnostics.clear();
+
+  Cfg = Combined->getConfig(Params(), Diags.callback());
+  EXPECT_THAT(Diags.Diagnostics,
+  ElementsAre(DiagMessage("foo"), DiagMessage("bar")));
+  EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("foo2", "bar2"));
+}
+
+const char *AddFooWithErr = R"yaml(
+CompileFlags:
+  Add: foo
+  Unknown: 42
+)yaml";
+
+const char *AddBarBaz = R"yaml(
+CompileFlags:
+  Add: bar
+---
+CompileFlags:
+  Add: baz
+)yaml";
+
+TEST(ProviderTest, FromYAMLFile) {
+  MockFS FS;
+  FS.Files["foo.yaml"] = AddFooWithErr;
+
+  CapturedDiags Diags;
+  auto P = Provider::fromYAMLFile(testPath("foo.yaml"), FS);
+  auto Cfg = P->getConfig(Params(), Diags.callback());
+  EXPECT_THAT(Diags.Diagnostics,
+  ElementsAre(DiagMessage("Unknown CompileFlags key Unknown")));
+  EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("foo"));
+  Diags.Diagnostics.clear();
+
+  Cfg = P->getConfig(Params(), Diags.callback());
+  EXPECT_THAT(Diags.Diagnostics, IsEmpty()) << "Cached, not re-parsed";
+  EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("foo"));
+
+  FS.Files["foo.yaml"] = AddBarBaz;
+  Cfg = P->getConfig(Params(), Diags.callback());
+  EXPECT_THAT(Diags.Diagnostics, IsEmpty()) << "New config, no errors";
+  EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("bar", "baz"));
+
+  FS.Files.erase("foo.yaml");
+  Cfg = P->getConfig(Params(), Diags.callback());
+  EXPECT_THAT(Diags.Diagnostics, IsEmpty()) << "Missing file is not an error";
+  EXPECT_THAT(getAddedArgs(Cfg), IsEmpty());
+}
+
+TEST(ProviderTest, FromAncestorRelativeYAMLFiles) {
+  MockFS FS;
+  FS.Files["a/b/c/foo.yaml"] = AddBarBaz;
+  FS.Files["a/foo.yaml"] = AddFooWithErr;
+
+  std::string ABCPath = testPath("a/b/c/d/test.cc");
+  Params ABCParams;
+  ABCParams.Path = ABCPath;
+  std::string APath = testPath("a/b/e/f/test.cc");
+  Params AParams;
+  AParams.Path = APath;
+
+  CapturedDiags Diags

[PATCH] D82967: [analyzer][tests] Measure peak memory consumption for every project

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko created this revision.
vsavchenko added reviewers: NoQ, xazax.hun, Szelethus.
Herald added subscribers: cfe-commits, ASDenysPetrov, Charusso, dkrupp, 
donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82967

Files:
  clang/utils/analyzer/Dockerfile
  clang/utils/analyzer/SATestBuild.py
  clang/utils/analyzer/SATestUtils.py
  clang/utils/analyzer/requirements.txt

Index: clang/utils/analyzer/requirements.txt
===
--- /dev/null
+++ clang/utils/analyzer/requirements.txt
@@ -0,0 +1,2 @@
+humanize
+psutil
Index: clang/utils/analyzer/SATestUtils.py
===
--- clang/utils/analyzer/SATestUtils.py
+++ clang/utils/analyzer/SATestUtils.py
@@ -1,8 +1,9 @@
 import os
 import sys
+import time
 
 from subprocess import CalledProcessError, check_call
-from typing import List, IO, Optional
+from typing import List, IO, Optional, Tuple
 
 
 def which(command: str, paths: Optional[str] = None) -> Optional[str]:
@@ -47,6 +48,87 @@
 return ext in (".i", ".ii", ".c", ".cpp", ".m", "")
 
 
+def time_to_str(time: float) -> str:
+"""
+Convert given time in seconds into a human-readable string.
+"""
+return f"{time:.2f}s"
+
+
+def memory_to_str(memory: int) -> str:
+"""
+Convert given number of bytes into a human-readable string.
+"""
+if memory:
+try:
+import humanize
+return humanize.naturalsize(memory, gnu=True)
+except ImportError:
+# no formatter installed, let's keep it in bytes
+return f"{memory}B"
+
+# If memory is 0, we didn't succeed measuring it.
+return "N/A"
+
+
+def check_and_measure_call(*popenargs, **kwargs) -> Tuple[float, int]:
+"""
+Run command with arguments.  Wait for command to complete and measure
+execution time and peak memory consumption.
+If the exit code was zero then return, otherwise raise
+CalledProcessError.  The CalledProcessError object will have the
+return code in the returncode attribute.
+
+The arguments are the same as for the call and check_call functions.
+
+Return a tuple of execution time and peak memory.
+"""
+peak_mem = 0
+start_time = time.time()
+
+try:
+import psutil as ps
+
+def get_memory(process: ps.Process) -> int:
+mem = 0
+
+# we want to gather memory usage from all of the child processes
+descendants = list(process.children(recursive=True))
+descendants.append(process)
+
+for subprocess in descendants:
+try:
+mem += subprocess.memory_info().rss
+except (ps.NoSuchProcess, ps.AccessDenied):
+continue
+
+return mem
+
+with ps.Popen(*popenargs, **kwargs) as process:
+# while the process is running calculate resource utilization.
+while (process.is_running() and
+   process.status() != ps.STATUS_ZOMBIE):
+# track the peak utilization of the process
+peak_mem = max(peak_mem, get_memory(process))
+time.sleep(.5)
+
+if process.is_running():
+process.kill()
+
+if process.returncode != 0:
+cmd = kwargs.get("args")
+if cmd is None:
+cmd = popenargs[0]
+raise CalledProcessError(process.returncode, cmd)
+
+except ImportError:
+# back off to subprocess if we don't have psutil installed
+peak_mem = 0
+check_call(*popenargs, **kwargs)
+
+return time.time() - start_time, peak_mem
+
+
 def run_script(script_path: str, build_log_file: IO, cwd: str,
out=sys.stdout, err=sys.stderr, verbose: int = 0):
 """
Index: clang/utils/analyzer/SATestBuild.py
===
--- clang/utils/analyzer/SATestBuild.py
+++ clang/utils/analyzer/SATestBuild.py
@@ -43,7 +43,7 @@
 variable. It should contain a comma separated list.
 """
 import CmpRuns
-import SATestUtils
+import SATestUtils as utils
 from ProjectMap import DownloadType, ProjectInfo
 
 import glob
@@ -63,7 +63,7 @@
 # and this is we can shush that false positive
 from plistlib import InvalidFileException  # type:ignore
 from subprocess import CalledProcessError, check_call
-from typing import Dict, IO, List, NamedTuple, Optional, TYPE_CHECKING
+from typing import Dict, IO, List, NamedTuple, Optional, TYPE_CHECKING, Tuple
 
 
 ###
@@ -115,7 +115,7 @@
 if 'CC' in os.environ:
 cc_candidate: Optional[str] = os.environ['CC']
 else:
-cc_candidate = SATestUtils.which("clang", os.environ['PATH'])
+cc_candidate = utils.which("c

[PATCH] D82968: [VE] Rename VE toolchain source files

[Kazushi Marukawa via Phabricator via cfe-commits]

kaz7 created this revision.
kaz7 added reviewers: simoll, k-ishizaka.
kaz7 added projects: LLVM, VE.
Herald added subscribers: cfe-commits, mgorny.
Herald added a project: clang.

Rename VE.cpp and VE.h to VEToolchain.cpp and VEToolchain.h respectively
in order to avoid link warning message.  Linker warns that VE.cpp.o and
Arch/VE.cpp.o have the same name.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82968

Files:
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/VE.cpp
  clang/lib/Driver/ToolChains/VE.h
  clang/lib/Driver/ToolChains/VEToolchain.cpp
  clang/lib/Driver/ToolChains/VEToolchain.h

Index: clang/lib/Driver/ToolChains/VE.h
===
--- /dev/null
+++ clang/lib/Driver/ToolChains/VE.h
@@ -1,66 +0,0 @@
-//===--- VE.h - VE ToolChain Implementations *- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_VE_H
-#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_VE_H
-
-#include "Linux.h"
-#include "clang/Driver/ToolChain.h"
-
-namespace clang {
-namespace driver {
-namespace toolchains {
-
-class LLVM_LIBRARY_VISIBILITY VEToolChain : public Linux {
-public:
-  VEToolChain(const Driver &D, const llvm::Triple &Triple,
-  const llvm::opt::ArgList &Args);
-
-protected:
-  Tool *buildAssembler() const override;
-  Tool *buildLinker() const override;
-
-public:
-  bool isPICDefault() const override;
-  bool isPIEDefault() const override;
-  bool isPICDefaultForced() const override;
-  bool SupportsProfiling() const override;
-  bool hasBlocksRuntime() const override;
-  void
-  AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
-llvm::opt::ArgStringList &CC1Args) const override;
-  void
-  addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
-llvm::opt::ArgStringList &CC1Args,
-Action::OffloadKind DeviceOffloadKind) const override;
-  void AddClangCXXStdlibIncludeArgs(
-  const llvm::opt::ArgList &DriverArgs,
-  llvm::opt::ArgStringList &CC1Args) const override;
-  void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
-   llvm::opt::ArgStringList &CmdArgs) const override;
-
-  llvm::ExceptionHandling
-  GetExceptionModel(const llvm::opt::ArgList &Args) const override;
-
-  CXXStdlibType
-  GetCXXStdlibType(const llvm::opt::ArgList &Args) const override {
-return ToolChain::CST_Libcxx;
-  }
-
-  RuntimeLibType GetDefaultRuntimeLibType() const override {
-return ToolChain::RLT_CompilerRT;
-  }
-
-  const char *getDefaultLinker() const override { return "nld"; }
-};
-
-} // end namespace toolchains
-} // end namespace driver
-} // end namespace clang
-
-#endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_VE_H
Index: clang/lib/Driver/ToolChains/VEToolchain.cpp
===
--- clang/lib/Driver/ToolChains/VEToolchain.cpp
+++ clang/lib/Driver/ToolChains/VEToolchain.cpp
@@ -6,7 +6,7 @@
 //
 //===--===//
 
-#include "VE.h"
+#include "VEToolchain.h"
 #include "CommonArgs.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -43,7 +43,7 @@
 #include "ToolChains/RISCVToolchain.h"
 #include "ToolChains/Solaris.h"
 #include "ToolChains/TCE.h"
-#include "ToolChains/VE.h"
+#include "ToolChains/VEToolchain.h"
 #include "ToolChains/WebAssembly.h"
 #include "ToolChains/XCore.h"
 #include "clang/Basic/Version.h"
Index: clang/lib/Driver/CMakeLists.txt
===
--- clang/lib/Driver/CMakeLists.txt
+++ clang/lib/Driver/CMakeLists.txt
@@ -68,7 +68,7 @@
   ToolChains/RISCVToolchain.cpp
   ToolChains/Solaris.cpp
   ToolChains/TCE.cpp
-  ToolChains/VE.cpp
+  ToolChains/VEToolchain.cpp
   ToolChains/WebAssembly.cpp
   ToolChains/XCore.cpp
   ToolChains/PPCLinux.cpp
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82891: [clangd] Fix race in FileIndex that sometimes temporarily lost updates.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
sammccall marked 2 inline comments as done.
Closed by commit rGc5263a4e84cc: [clangd] Fix race in FileIndex that sometimes 
temporarily lost updates. (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D82891?vs=274532&id=274818#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82891

Files:
  clang-tools-extra/clangd/index/FileIndex.cpp
  clang-tools-extra/clangd/index/FileIndex.h
  clang-tools-extra/clangd/index/Symbol.h
  clang-tools-extra/clangd/unittests/FileIndexTests.cpp

Index: clang-tools-extra/clangd/unittests/FileIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/FileIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/FileIndexTests.cpp
@@ -22,6 +22,7 @@
 #include "index/Relation.h"
 #include "index/Serialization.h"
 #include "index/Symbol.h"
+#include "support/Threading.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/Utils.h"
 #include "clang/Index/IndexSymbol.h"
@@ -509,6 +510,31 @@
   EXPECT_THAT(runFuzzyFind(M, ""), UnorderedElementsAre(QName("b")));
 }
 
+// Verifies that concurrent calls to updateMain don't "lose" any updates.
+TEST(FileIndexTest, Threadsafety) {
+  FileIndex M;
+  Notification Go;
+
+  constexpr int Count = 10;
+  {
+// Set up workers to concurrently call updateMain() with separate files.
+AsyncTaskRunner Pool;
+for (unsigned I = 0; I < Count; ++I) {
+  auto TU = TestTU::withCode(llvm::formatv("int xxx{0};", I).str());
+  TU.Filename = llvm::formatv("x{0}.c", I).str();
+  Pool.runAsync(TU.Filename, [&, Filename(testPath(TU.Filename)),
+  AST(TU.build())]() mutable {
+Go.wait();
+M.updateMain(Filename, AST);
+  });
+}
+// On your marks, get set...
+Go.notify();
+  }
+
+  EXPECT_THAT(runFuzzyFind(M, "xxx"), ::testing::SizeIs(Count));
+}
+
 TEST(FileShardedIndexTest, Sharding) {
   auto AHeaderUri = URI::create(testPath("a.h")).toString();
   auto BHeaderUri = URI::create(testPath("b.h")).toString();
Index: clang-tools-extra/clangd/index/Symbol.h
===
--- clang-tools-extra/clangd/index/Symbol.h
+++ clang-tools-extra/clangd/index/Symbol.h
@@ -186,7 +186,8 @@
   const_iterator end() const { return Symbols.end(); }
   const_iterator find(const SymbolID &SymID) const;
 
-  size_t size() const { return Symbols.size(); }
+  using size_type = size_t;
+  size_type size() const { return Symbols.size(); }
   bool empty() const { return Symbols.empty(); }
   // Estimates the total memory usage.
   size_t bytes() const {
Index: clang-tools-extra/clangd/index/FileIndex.h
===
--- clang-tools-extra/clangd/index/FileIndex.h
+++ clang-tools-extra/clangd/index/FileIndex.h
@@ -81,9 +81,11 @@
   /// The index keeps the slabs alive.
   /// Will count Symbol::References based on number of references in the main
   /// files, while building the index with DuplicateHandling::Merge option.
+  /// Version is populated with an increasing sequence counter.
   std::unique_ptr
   buildIndex(IndexType,
- DuplicateHandling DuplicateHandle = DuplicateHandling::PickOne);
+ DuplicateHandling DuplicateHandle = DuplicateHandling::PickOne,
+ size_t *Version = nullptr);
 
 private:
   struct RefSlabAndCountReferences {
@@ -92,6 +94,7 @@
   };
   mutable std::mutex Mutex;
 
+  size_t Version = 0;
   llvm::StringMap> SymbolsSnapshot;
   llvm::StringMap RefsSnapshot;
   llvm::StringMap> RelatiosSnapshot;
@@ -136,6 +139,12 @@
   // (Note that symbols *only* in the main file are not indexed).
   FileSymbols MainFileSymbols;
   SwapIndex MainFileIndex;
+
+  // While both the FileIndex and SwapIndex are threadsafe, we need to track
+  // versions to ensure that we don't overwrite newer indexes with older ones.
+  std::mutex UpdateIndexMu;
+  unsigned MainIndexVersion = 0;
+  unsigned PreambleIndexVersion = 0;
 };
 
 using SlabTuple = std::tuple;
Index: clang-tools-extra/clangd/index/FileIndex.cpp
===
--- clang-tools-extra/clangd/index/FileIndex.cpp
+++ clang-tools-extra/clangd/index/FileIndex.cpp
@@ -229,6 +229,7 @@
  std::unique_ptr Relations,
  bool CountReferences) {
   std::lock_guard Lock(Mutex);
+  ++Version;
   if (!Symbols)
 SymbolsSnapshot.erase(Key);
   else
@@ -248,7 +249,8 @@
 }
 
 std::unique_ptr
-FileSymbols::buildIndex(IndexType Type, DuplicateHandling DuplicateHandle) {
+FileSymbols::buildIndex(IndexType Type, DuplicateHandling DuplicateHandle,
+size_t *Version) {
   std::vector> SymbolSlabs;
   std::vector> RefSlabs;
   std::vector> Rela

[PATCH] D82659: Fix missing build dependency on omp_gen.

[Valentin Clement via Phabricator via cfe-commits]

clementval accepted this revision.
clementval added a comment.

LGTM. So later the DEPENDS omp_gen that are in clang subdirectories could be 
removed right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82659



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

[PATCH] D82904: [clang-tidy] Warn pointer captured in async block

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp:125
+CheckFactories.registerCheck(
+"bugprone-no-escape");
 CheckFactories.registerCheck(

ellis wrote:
> Not sure if we want to lint this line because it was generated by 
> `clang-tidy/add_new_check.py` and every other `registerCheck` line is 
> formatted in the same way.
They should be formatted, but we don't pipe the changes through clang-format in 
the `add_new_check.py` script. only 8/21 clang-tidy module files are 
incorrectly formatted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82904



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

[PATCH] D82880: Fix PR35677: UB on __int128_t or __uint128_t template parameters.

[David Stone via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/StmtPrinter.cpp:1159
+  case BuiltinType::UInt128:
+OS << "Ui128";
+break;

riccibruno wrote:
> `i128` and `Ui128` are not valid integer literal suffix. The output of 
> `StmtPrinter` is intended to be valid C++. Unfortunately here I think that 
> your only choice is to print the high and low parts separately. 
I'm confused. i8, Ui8, i16, and Ui16 are also not valid C++ suffixes, but just 
a few lines up we use those. What am I missing here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82880



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

[PATCH] D82659: Fix missing build dependency on omp_gen.

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

@simon_tatham Can you just update the description so it is consistent with your 
fix when it lands?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82659



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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Just my 2 cents, but would it not be wise to introduce a test case that runs 
the 2 aforementioned checks together demonstrating how they interact with each 
other. This will also force compliance if either check is updated down the line.


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

https://reviews.llvm.org/D71199



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

[PATCH] D82928: [Coroutines] Fix code coverage for coroutine

[Brian Gesiak via Phabricator via cfe-commits]

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

Excellent, thanks for this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82928



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

[PATCH] D82659: Fix missing build dependency on omp_gen.

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham added a comment.

In D82659#2125618 , @clementval wrote:

> LGTM. So later the DEPENDS omp_gen that are in clang subdirectories could be 
> removed right?


That seems likely. I'm thinking what I should probably do is to polish up my 
checking script and try to get it into `llvm/utils` somewhere, and then it 
would be possible to make that kind of change and be //confident// that it 
wasn't removing a necessary dependency.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82659



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

[PATCH] D82659: Fix missing build dependency on omp_gen.

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham added a comment.

[facepalm] Thank you. I carefully //wrote// a revised description, but forgot 
to upload it to this issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82659



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

[PATCH] D82928: [Coroutines] Fix code coverage for coroutine

[Xun Li via Phabricator via cfe-commits]

lxfind updated this revision to Diff 274836.
lxfind added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82928

Files:
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/test/CoverageMapping/coroutine.cpp


Index: clang/test/CoverageMapping/coroutine.cpp
===
--- /dev/null
+++ clang/test/CoverageMapping/coroutine.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 
-emit-llvm -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping 
%s | FileCheck %s
+
+namespace std::experimental {
+template 
+struct coroutine_traits;
+
+template 
+struct coroutine_handle {
+  coroutine_handle() = default;
+  static coroutine_handle from_address(void *) noexcept { return {}; }
+};
+template <>
+struct coroutine_handle {
+  static coroutine_handle from_address(void *) { return {}; }
+  coroutine_handle() = default;
+  template 
+  coroutine_handle(coroutine_handle) noexcept {}
+};
+} // namespace std::experimental
+
+struct suspend_always {
+  bool await_ready() noexcept;
+  void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+  void await_resume() noexcept;
+};
+
+template <>
+struct std::experimental::coroutine_traits {
+  struct promise_type {
+int get_return_object();
+suspend_always initial_suspend();
+suspend_always final_suspend() noexcept;
+void return_value(int);
+  };
+};
+
+// CHECK-LABEL: _Z2f1i:
+int f1(int x) {   // CHECK-NEXT: File 0, [[@LINE]]:15 -> [[@LINE+7]]:2 = #0
+  if (x > 42) {   // CHECK-NEXT: File 0, [[@LINE]]:7 -> [[@LINE]]:13 = #0
+++x;  // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:14 -> 
[[@LINE-1]]:15 = #1
+  } else {// CHECK-NEXT: File 0, [[@LINE-2]]:15 -> [[@LINE]]:4 = #1
+co_return x + 42; // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:4 -> 
[[@LINE-1]]:10 = (#0 - #1)
+  }   // CHECK-NEXT: File 0, [[@LINE-2]]:10 -> [[@LINE]]:4 = 
(#0 - #1)
+  co_return x;// CHECK-NEXT: Gap,File 0, [[@LINE-1]]:4 -> [[@LINE]]:3 
= #1
+} // CHECK-NEXT: File 0, [[@LINE-1]]:3 -> [[@LINE]]:2 = #1
Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -908,6 +908,18 @@
 terminateRegion(S);
   }
 
+  void VisitCoroutineBodyStmt(const CoroutineBodyStmt *S) {
+extendRegion(S);
+Visit(S->getBody());
+  }
+
+  void VisitCoreturnStmt(const CoreturnStmt *S) {
+extendRegion(S);
+if (S->getOperand())
+  Visit(S->getOperand());
+terminateRegion(S);
+  }
+
   void VisitCXXThrowExpr(const CXXThrowExpr *E) {
 extendRegion(E);
 if (E->getSubExpr())


Index: clang/test/CoverageMapping/coroutine.cpp
===
--- /dev/null
+++ clang/test/CoverageMapping/coroutine.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 -emit-llvm -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping %s | FileCheck %s
+
+namespace std::experimental {
+template 
+struct coroutine_traits;
+
+template 
+struct coroutine_handle {
+  coroutine_handle() = default;
+  static coroutine_handle from_address(void *) noexcept { return {}; }
+};
+template <>
+struct coroutine_handle {
+  static coroutine_handle from_address(void *) { return {}; }
+  coroutine_handle() = default;
+  template 
+  coroutine_handle(coroutine_handle) noexcept {}
+};
+} // namespace std::experimental
+
+struct suspend_always {
+  bool await_ready() noexcept;
+  void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+  void await_resume() noexcept;
+};
+
+template <>
+struct std::experimental::coroutine_traits {
+  struct promise_type {
+int get_return_object();
+suspend_always initial_suspend();
+suspend_always final_suspend() noexcept;
+void return_value(int);
+  };
+};
+
+// CHECK-LABEL: _Z2f1i:
+int f1(int x) {   // CHECK-NEXT: File 0, [[@LINE]]:15 -> [[@LINE+7]]:2 = #0
+  if (x > 42) {   // CHECK-NEXT: File 0, [[@LINE]]:7 -> [[@LINE]]:13 = #0
+++x;  // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:14 -> [[@LINE-1]]:15 = #1
+  } else {// CHECK-NEXT: File 0, [[@LINE-2]]:15 -> [[@LINE]]:4 = #1
+co_return x + 42; // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:4 -> [[@LINE-1]]:10 = (#0 - #1)
+  }   // CHECK-NEXT: File 0, [[@LINE-2]]:10 -> [[@LINE]]:4 = (#0 - #1)
+  co_return x;// CHECK-NEXT: Gap,File 0, [[@LINE-1]]:4 -> [[@LINE]]:3 = #1
+} // CHECK-NEXT: File 0, [[@LINE-1]]:3 -> [[@LINE]]:2 = #1
Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -908

[PATCH] D82388: move "basic" builtins to TableGen

[Nathan Froyd via Phabricator via cfe-commits]

froydnj added a comment.

FWIW, an in-progress tree for the proposed move for all the 
architecture-specific builtins is:

https://github.com/froydnj/llvm-project/tree/move-builtins-to-tablegen

I don't know if the ARM/NEON/SVE builtins will be easily movable.  It looks 
like they are separately generated from existing `.td` files; while you could 
generate `.td` files containing all the appropriate `Builtin` etc. definitions, 
you cannot have tablegen input depend on tablegen'd output in the general case 
due to:

https://github.com/llvm/llvm-project/blob/c83ec0a633583e5b12e0aeb70627eb35f7cd4847/llvm/cmake/modules/TableGen.cmake#L11-L35

in the non-Ninja case.  If you try, you get circular dependencies.  One could 
maybe hack around this by giving certain files different suffixes, but that 
feels gross.

I'm not sure how to address that issue.


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

https://reviews.llvm.org/D82388



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

[PATCH] D82314: [Coroutines] Optimize the lifespan of temporary co_await object

[Xun Li via Phabricator via cfe-commits]

lxfind added a comment.

In D82314#2124662 , @junparser wrote:

> In D82314#2124661 , @junparser wrote:
>
> > @lxfind This patch causes some mismatch when variable is used in both 
> > resume and destroy function. Besides, we should move this patch and the 
> > check in buildCoroutineFrame.
>
>
> @lxfind, Would you try to fix this？ If you do not have time, then I'll try do 
> this. Thanks


Could you please help take a look, if you have a local repro? Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82314



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

[PATCH] D82845: [Analyzer][StreamChecker] Report every leak, clean up state.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

The problem is that `PathDiagnostic::Profile` does not use the uniqueing 
locations. Two `PathDiagnostic` objects with different uniqueing location but 
otherwise same data are counted as "same" and only one is kept 
(`BugReporter::FlushReport`). How to fix this?

- Add uniqueing locations to the profile. May have unexpected effects to other 
checkers.
- Change something in the leak bug reports to make these different. Probably 
the location can be set to the allocation site (but the full path should be 
reported). Or include name of the leaked symbol in the report, but it may not 
exists always.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82845



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

[PATCH] D82954: Fix crash on overloaded postfix unary operators due to invalid sloc

[Eduardo Caldas via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/ExprCXX.cpp:82
+  default:
+return getNumArgs() == 1;
+  }

riccibruno wrote:
> This will be true for `operator->`.
Thank you ! I had missed that! I'll propose a more robust solution. I'll 
probably split this patch. But I'll take care to subscribe you to anything 
related


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82954



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

[PATCH] D82706: [ASTMatchers] Enhanced support for matchers taking Regex arguments

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 274846.
njames93 added a comment.

Small refactor based on comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82706

Files:
  clang/docs/LibASTMatchersReference.html
  clang/docs/tools/dump_ast_matchers.py
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/include/clang/ASTMatchers/ASTMatchersMacros.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Marshallers.cpp
  clang/lib/ASTMatchers/Dynamic/Marshallers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
  clang/unittests/ASTMatchers/Dynamic/ParserTest.cpp
  llvm/include/llvm/Support/Regex.h
  llvm/lib/Support/Regex.cpp

Index: llvm/lib/Support/Regex.cpp
===
--- llvm/lib/Support/Regex.cpp
+++ llvm/lib/Support/Regex.cpp
@@ -26,7 +26,7 @@
 
 Regex::Regex() : preg(nullptr), error(REG_BADPAT) {}
 
-Regex::Regex(StringRef regex, unsigned Flags) {
+Regex::Regex(StringRef regex, RegexFlags Flags) {
   unsigned flags = 0;
   preg = new llvm_regex();
   preg->re_endp = regex.end();
@@ -39,6 +39,9 @@
   error = llvm_regcomp(preg, regex.data(), flags|REG_PEND);
 }
 
+Regex::Regex(StringRef regex, unsigned Flags)
+: Regex(regex, static_cast(Flags)) {}
+
 Regex::Regex(Regex &®ex) {
   preg = regex.preg;
   error = regex.error;
Index: llvm/include/llvm/Support/Regex.h
===
--- llvm/include/llvm/Support/Regex.h
+++ llvm/include/llvm/Support/Regex.h
@@ -16,6 +16,7 @@
 #ifndef LLVM_SUPPORT_REGEX_H
 #define LLVM_SUPPORT_REGEX_H
 
+#include "llvm/ADT/BitmaskEnum.h"
 #include 
 
 struct llvm_regex;
@@ -26,20 +27,22 @@
 
   class Regex {
   public:
-enum {
-  NoFlags=0,
+enum RegexFlags : unsigned {
+  NoFlags = 0,
   /// Compile for matching that ignores upper/lower case distinctions.
-  IgnoreCase=1,
+  IgnoreCase = 1,
   /// Compile for newline-sensitive matching. With this flag '[^' bracket
   /// expressions and '.' never match newline. A ^ anchor matches the
   /// null string after any newline in the string in addition to its normal
   /// function, and the $ anchor matches the null string before any
   /// newline in the string in addition to its normal function.
-  Newline=2,
+  Newline = 2,
   /// By default, the POSIX extended regular expression (ERE) syntax is
   /// assumed. Pass this flag to turn on basic regular expressions (BRE)
   /// instead.
-  BasicRegex=4
+  BasicRegex = 4,
+
+  LLVM_MARK_AS_BITMASK_ENUM(BasicRegex)
 };
 
 Regex();
@@ -47,7 +50,8 @@
 ///
 /// \param Regex - referenced string is no longer needed after this
 /// constructor does finish.  Only its compiled form is kept stored.
-Regex(StringRef Regex, unsigned Flags = NoFlags);
+Regex(StringRef Regex, RegexFlags Flags = NoFlags);
+Regex(StringRef Regex, unsigned Flags);
 Regex(const Regex &) = delete;
 Regex &operator=(Regex regex) {
   std::swap(preg, regex.preg);
Index: clang/unittests/ASTMatchers/Dynamic/ParserTest.cpp
===
--- clang/unittests/ASTMatchers/Dynamic/ParserTest.cpp
+++ clang/unittests/ASTMatchers/Dynamic/ParserTest.cpp
@@ -259,6 +259,15 @@
   EXPECT_TRUE(matches("unsigned X = sizeof(int);", MStmt));
   EXPECT_FALSE(matches("unsigned X = alignof(int);", MStmt));
 
+  Code =
+  R"query(namedDecl(matchesName("^::[ABC]*$", "IgnoreCase | BasicRegex")))query";
+  llvm::Optional MatchesName(
+  Parser::parseMatcherExpression(Code, nullptr, nullptr, &Error));
+  EXPECT_EQ("", Error.toStringFull());
+  M = MatchesName->unconditionalConvertTo();
+  EXPECT_TRUE(matches("unsigned AAACCBB;", M));
+  EXPECT_TRUE(matches("unsigned aaaccbb;", M));
+
   Code = "hasInitializer(\nbinaryOperator(hasLHS(\"A\")))";
   EXPECT_TRUE(!Parser::parseMatcherExpression(Code, &Error).hasValue());
   EXPECT_EQ("1:1: Error parsing argument 1 for matcher hasInitializer.\n"
@@ -348,6 +357,26 @@
 "1:14: Incorrect type for arg 1. (Expected = string) != (Actual = "
 "String)",
 ParseMatcherWithError(R"query(decl(hasAttr("unrelated")))query"));
+  EXPECT_EQ(
+  "1:1: Error parsing argument 1 for matcher namedDecl.\n"
+  "1:11: Error building matcher matchesName.\n"
+  "1:33: Unknown value 'Ignorecase' for arg 2; did you mean 'IgnoreCase'",
+  ParseMatcherWithError(
+  R"query(namedDecl(matchesName("[ABC]*", "Ignorecase")))query"));
+  EXPECT_EQ(
+  "1:1: Error parsing argument 1 for matcher namedDecl.\n"
+  "1:11: Error building matcher matchesName.\n"
+  "1:33: Incorrect type for arg 2. (Expected = string) != (Actual = "
+  "String)",
+  Pars

[PATCH] D82926: [libfuzzer] [clang] Add __has_feature(fuzzing_coverage)

[Max Moroz via Phabricator via cfe-commits]

Dor1s added a comment.

What usecase(s) do you have for this in mind?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82926



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

[PATCH] D82611: [SemaObjC] Add a warning for @selector expressions that potentially refer to objc_direct methods

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added inline comments.



Comment at: clang/test/SemaObjC/potentially-direct-selector.m:84
+  (void)@selector(inBaseCat);
+  (void)@selector(inBaseCatImpl);
+  (void)@selector(inDerived);

I think this might be too weak, if we add a warning then maybe what we want is 
a tri-state:

`-Wstrict-direct-dispatch=none|self|all`

because I can see cases where the heuristic here would be too weak


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

https://reviews.llvm.org/D82611



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

[PATCH] D82611: [SemaObjC] Add a warning for @selector expressions that potentially refer to objc_direct methods

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added inline comments.



Comment at: clang/lib/Sema/SemaExprObjC.cpp:1230
 }
-
-static void HelperToDiagnoseDirectSelectorsExpr(Sema &S, SourceLocation AtLoc,
-Selector Sel,
+static void HelperToDiagnoseDirectSelectorsExpr(Sema &S, Selector Sel,
 ObjCMethodList &MethList,

missing line between the functions?


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

https://reviews.llvm.org/D82611



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

[PATCH] D82930: [HIP] Fix rocm detection

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:167
+llvm::ErrorOr> VersionFile =
+FS.getBufferForFile(BinPath + "/.hipVersion");
+if (!VersionFile)

arsenm wrote:
> I don't think the detection should fail if this is missing
why? this file is unique to HIP installation. If it is missing, the 
installation is broken.


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

https://reviews.llvm.org/D82930



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

[PATCH] D82611: [SemaObjC] Add a warning for @selector expressions that potentially refer to objc_direct methods

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

So the risk with that one is that if someone had say the `-didSomething` direct 
selector and that some UIKit/Apple SDK API now adds this as a thing that you 
use with CFNotification center for example, where you _need_ dynamism, then the 
uses of the API would warn all the time when the internal to the client project 
`-didSomething` is in scope.

So we ideally need a way to silence this warning, so at the very least this 
needs to be a new `-W` flag (on by default (?)) so that callers that have an 
issue can use a `_Pragma` at the call site to ignore the error for when it's 
legal.

I would suggest something like `-Wstrict-direct-dispatch` or something.


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

https://reviews.llvm.org/D82611



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

[PATCH] D82880: Fix PR35677: UB on __int128_t or __uint128_t template parameters.

[Bruno Ricci via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/StmtPrinter.cpp:1159
+  case BuiltinType::UInt128:
+OS << "Ui128";
+break;

davidstone wrote:
> riccibruno wrote:
> > `i128` and `Ui128` are not valid integer literal suffix. The output of 
> > `StmtPrinter` is intended to be valid C++. Unfortunately here I think that 
> > your only choice is to print the high and low parts separately. 
> I'm confused. i8, Ui8, i16, and Ui16 are also not valid C++ suffixes, but 
> just a few lines up we use those. What am I missing here?
The literal suffixes `[u]i8, [u]i16, [u]i32, and [u]i64` are an MSVC extension 
(see `NumericLiteralParser::NumericLiteralParser` in `Lex/LiteralSupport.cpp`).

This does not explain why they are used even in non-ms compatibility mode
but at least there is some reason for their existence.

However I don't think that MSVC supports 128-bits integers (?), and clang 
certainly
does not support `[u]i128` so there is no reason to use them.

@aaron.ballman Do you know why are these suffixes used outside of 
ms-compatibility mode?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82880



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

[PATCH] D79279: Add overloaded versions of builtin mem* functions

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 274847.
jfb edited the summary of this revision.
jfb added a comment.

Overload a new builtin instead. For now I only did memcpy, to get feedback on 
the approach. I'll add other mem* functions if this makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279

Files:
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGBuilder.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtin-overloaded-memfns.c
  clang/test/Sema/builtin-overloaded-memfns.cpp
  clang/test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl
  clang/test/SemaOpenCL/to_addr_builtin.cl

Index: clang/test/SemaOpenCL/to_addr_builtin.cl
===
--- clang/test/SemaOpenCL/to_addr_builtin.cl
+++ clang/test/SemaOpenCL/to_addr_builtin.cl
@@ -15,7 +15,7 @@
   // expected-error@-2{{implicit declaration of function 'to_global' is invalid in OpenCL}}
   // expected-warning@-3{{incompatible integer to pointer conversion assigning to '__global int *__private' from 'int'}}
 #else
-  // expected-error@-5{{invalid number of arguments to function: 'to_global'}}
+  // expected-error@-5{{too many arguments to function call, expected 1, have 2}}
 #endif
 
   int x;
Index: clang/test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl
===
--- clang/test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl
+++ clang/test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl
@@ -10,7 +10,7 @@
   read_pipe(p, &tmp);
   read_pipe(p, ptr);
   read_pipe(tmp, p);// expected-error {{first argument to 'read_pipe' must be a pipe type}}
-  read_pipe(p);   // expected-error {{invalid number of arguments to function: 'read_pipe'}}
+  read_pipe(p); // expected-error {{too few arguments to function call, expected 2 or 4, have 1}}
   read_pipe(p, rid, tmp, ptr);
   read_pipe(p, tmp, tmp, ptr);   // expected-error {{invalid argument type to function 'read_pipe' (expecting 'reserve_id_t' having '__private int')}}
   read_pipe(p, rid, rid, ptr);   // expected-error {{invalid argument type to function 'read_pipe' (expecting 'unsigned int' having '__private reserve_id_t')}}
@@ -39,7 +39,7 @@
   write_pipe(p, &tmp);
   write_pipe(p, ptr);
   write_pipe(tmp, p);// expected-error {{first argument to 'write_pipe' must be a pipe type}}
-  write_pipe(p);   // expected-error {{invalid number of arguments to function: 'write_pipe'}}
+  write_pipe(p); // expected-error {{too few arguments to function call, expected 2 or 4, have 1}}
   write_pipe(p, rid, tmp, ptr);
   write_pipe(p, tmp, tmp, ptr);   // expected-error {{invalid argument type to function 'write_pipe' (expecting 'reserve_id_t' having '__private int')}}
   write_pipe(p, rid, rid, ptr);   // expected-error {{invalid argument type to function 'write_pipe' (expecting 'unsigned int' having '__private reserve_id_t')}}
Index: clang/test/Sema/builtin-overloaded-memfns.cpp
===
--- /dev/null
+++ clang/test/Sema/builtin-overloaded-memfns.cpp
@@ -0,0 +1,112 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only -triple=arm64-unknown-unknown -fms-extensions
+
+#define NULL (void *)0
+#define nullptr __nullptr
+using size_t = __SIZE_TYPE__;
+struct Intish {
+  int i;
+};
+struct TrivialCpy {
+  char buf[8];
+  TrivialCpy();
+  TrivialCpy(const TrivialCpy &) = default;
+};
+struct NotTrivialCpy {
+  char buf[8];
+  NotTrivialCpy();
+  NotTrivialCpy(const NotTrivialCpy &);
+};
+
+void memcpy_arg_count() {
+  __builtin_overloaded_memcpy();   // expected-error {{too few arguments to function call, expected 3, have 0}}
+  __builtin_overloaded_memcpy(0);  // expected-error {{too few arguments to function call, expected 3, have 1}}
+  __builtin_overloaded_memcpy(0, 0);   // expected-error {{too few arguments to function call, expected 3, have 2}}
+  __builtin_overloaded_memcpy(0, 0, 0, 0); // expected-error {{too many arguments to function call, expected 3, have 4}}
+}
+
+void memcpy_null(char *dst, const char *src, size_t size) {
+  __builtin_overloaded_memcpy(0, 0, 0);
+  __builtin_overloaded_memcpy(0, 0, size);
+  __builtin_overloaded_memcpy(dst, 0, 42);   // expected-warning {{null passed to a callee that requires a non-null argument}}
+  __builtin_overloaded_memcpy(dst, 0, 42);   // expected-warning {{null passed to a callee that requires a non-null argument}}
+  __builtin_overloaded_memcpy(dst, NULL, 42);// expected-warning {{null passed to a callee that requires a non-null argument}}
+  __builtin_overloaded_memcpy(dst, nullptr, 42); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  __builtin_overloaded_memcpy(0, src, 42);   // expected-warning {{null passed

[PATCH] D82904: [clang-tidy] Warn pointer captured in async block

[Ellis Hoag via Phabricator via cfe-commits]

ellis updated this revision to Diff 274855.
ellis added a comment.

Add `isLanguageVersionSupported` for Objective-C


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82904

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/NoEscapeCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/NoEscapeCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-no-escape.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-no-escape.m

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-no-escape.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-no-escape.m
@@ -0,0 +1,27 @@
+// RUN: %check_clang_tidy %s bugprone-no-escape %t
+
+typedef struct dispatch_queue_s *dispatch_queue_t;
+typedef struct dispatch_time_s *dispatch_time_t;
+typedef void (^dispatch_block_t)(void);
+void dispatch_async(dispatch_queue_t queue, dispatch_block_t block);
+void dispatch_after(dispatch_time_t when, dispatch_queue_t queue, dispatch_block_t block);
+
+extern dispatch_queue_t queue;
+
+void test_noescape_attribute(__attribute__((noescape)) int *p, int *q) {
+  dispatch_async(queue, ^{
+*p = 123;
+// CHECK-MESSAGES: :[[@LINE-2]]:25: warning: pointer 'p' with attribute 'noescape' is captured by an asynchronously-executed block [bugprone-no-escape]
+// CHECK-MESSAGES: :[[@LINE-4]]:30: note: the 'noescape' attribute is declared here.
+  });
+
+  dispatch_after(456, queue, ^{
+*p = 789;
+// CHECK-MESSAGES: :[[@LINE-2]]:30: warning: pointer 'p' with attribute 'noescape' is captured by an asynchronously-executed block [bugprone-no-escape]
+  });
+
+  dispatch_async(queue, ^{
+*q = 0;
+// CHECK-MESSAGES-NOT: :[[@LINE-2]]:25: warning: pointer 'q' with attribute 'noescape' is captured by an asynchronously-executed block
+  });
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -70,6 +70,7 @@
`bugprone-misplaced-widening-cast `_,
`bugprone-move-forwarding-reference `_, "Yes"
`bugprone-multiple-statement-macro `_,
+   `bugprone-no-escape `_, "Yes"
`bugprone-not-null-terminated-result `_, "Yes"
`bugprone-parent-virtual-call `_, "Yes"
`bugprone-posix-return `_, "Yes"
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-no-escape.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-no-escape.rst
@@ -0,0 +1,18 @@
+.. title:: clang-tidy - bugprone-no-escape
+
+bugprone-no-escape
+==
+
+Finds pointers with the ``noescape`` attribute that are captured by an
+asynchronously-executed block. The block arguments in ``dispatch_async()`` and
+``dispatch_after()`` are guaranteed to escape, so it is an error if a pointer with the
+``noescape`` attribute is captured by one of these blocks.
+
+The following is an example of an invalid use of the ``noescape`` attribute.
+
+  .. code-block:: objc
+void foo(__attribute__((noescape)) int *p) {
+  dispatch_async(queue, ^{
+*p = 123;
+  });
+});
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -94,6 +94,12 @@
   result of a memory allocation function (``malloc()``, ``calloc()``,
   ``realloc()``, ``alloca()``) instead of its argument.
 
+- New :doc:`bugprone-no-escape
+  ` check.
+
+  Finds pointers with the ``noescape`` attribute that are captured by an
+  asynchronously-executed block.
+
 - New :doc:`bugprone-spuriously-wake-up-functions
   ` check.
 
@@ -201,14 +207,14 @@
   Now checks ``std::basic_string_view`` by default.
 
 - Improved :doc:`readability-else-after-return
-  ` check now supports a 
+  ` check now supports a
   `WarnOnConditionVariables` option to control whether to refactor condition
   variables where possible.
 
 - Improved :doc:`readability-identifier-naming
   ` check.
 
-  Now able to rename member references in class template definitions with 
+  Now able to rename member references in class template definitions with
   explicit access.
 
 - Improved :doc:`readability-qualified-auto
Index: clang-tools-extra/clang-tidy/bugprone/NoEscapeCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/NoEscapeCheck.h
@@ -0,0 +1,39 @@
+//===--- NoEscapeCheck.h - clang-tidy ---*- C++ -*-===//
+//
+// Part of the 

[clang] 565e37c - [Coroutines] Fix code coverage for coroutine

[Xun Li via cfe-commits]

Author: Xun Li
Date: 2020-07-01T10:11:40-07:00
New Revision: 565e37c7702d181804c12d36b6010c513c9b3417

URL: 
https://github.com/llvm/llvm-project/commit/565e37c7702d181804c12d36b6010c513c9b3417
DIFF: 
https://github.com/llvm/llvm-project/commit/565e37c7702d181804c12d36b6010c513c9b3417.diff

LOG: [Coroutines] Fix code coverage for coroutine

Summary:
Previously, source-based coverage analysis does not work properly for coroutine.
This patch adds processing of coroutine body and co_return in the coverage 
analysis, so that we can handle them properly.
For coroutine body, we should only look at the actual function body and ignore 
the compiler-generated things; for co_return, we need to terminate the region 
similar to return statement.
Added a test, and confirms that it now works properly. (without this patch, the 
statement after the if statement will be treated wrongly)

Reviewers: lewissbaker, modocache, junparser

Reviewed By: modocache

Subscribers: cfe-commits

Tags: #clang

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

Added: 
clang/test/CoverageMapping/coroutine.cpp

Modified: 
clang/lib/CodeGen/CoverageMappingGen.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CoverageMappingGen.cpp 
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index cdbfc88e7b70..78b268f423cb 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -908,6 +908,18 @@ struct CounterCoverageMappingBuilder
 terminateRegion(S);
   }
 
+  void VisitCoroutineBodyStmt(const CoroutineBodyStmt *S) {
+extendRegion(S);
+Visit(S->getBody());
+  }
+
+  void VisitCoreturnStmt(const CoreturnStmt *S) {
+extendRegion(S);
+if (S->getOperand())
+  Visit(S->getOperand());
+terminateRegion(S);
+  }
+
   void VisitCXXThrowExpr(const CXXThrowExpr *E) {
 extendRegion(E);
 if (E->getSubExpr())

diff  --git a/clang/test/CoverageMapping/coroutine.cpp 
b/clang/test/CoverageMapping/coroutine.cpp
new file mode 100644
index ..ec1d64e0f970
--- /dev/null
+++ b/clang/test/CoverageMapping/coroutine.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 
-emit-llvm -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping 
%s | FileCheck %s
+
+namespace std::experimental {
+template 
+struct coroutine_traits;
+
+template 
+struct coroutine_handle {
+  coroutine_handle() = default;
+  static coroutine_handle from_address(void *) noexcept { return {}; }
+};
+template <>
+struct coroutine_handle {
+  static coroutine_handle from_address(void *) { return {}; }
+  coroutine_handle() = default;
+  template 
+  coroutine_handle(coroutine_handle) noexcept {}
+};
+} // namespace std::experimental
+
+struct suspend_always {
+  bool await_ready() noexcept;
+  void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+  void await_resume() noexcept;
+};
+
+template <>
+struct std::experimental::coroutine_traits {
+  struct promise_type {
+int get_return_object();
+suspend_always initial_suspend();
+suspend_always final_suspend() noexcept;
+void return_value(int);
+  };
+};
+
+// CHECK-LABEL: _Z2f1i:
+int f1(int x) {   // CHECK-NEXT: File 0, [[@LINE]]:15 -> [[@LINE+7]]:2 = #0
+  if (x > 42) {   // CHECK-NEXT: File 0, [[@LINE]]:7 -> [[@LINE]]:13 = #0
+++x;  // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:14 -> 
[[@LINE-1]]:15 = #1
+  } else {// CHECK-NEXT: File 0, [[@LINE-2]]:15 -> [[@LINE]]:4 = #1
+co_return x + 42; // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:4 -> 
[[@LINE-1]]:10 = (#0 - #1)
+  }   // CHECK-NEXT: File 0, [[@LINE-2]]:10 -> [[@LINE]]:4 = 
(#0 - #1)
+  co_return x;// CHECK-NEXT: Gap,File 0, [[@LINE-1]]:4 -> [[@LINE]]:3 
= #1
+} // CHECK-NEXT: File 0, [[@LINE-1]]:3 -> [[@LINE]]:2 = #1



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

[PATCH] D82940: [ASTReader][ASTWriter][PCH][Modules] Fix (de)serialization of SwitchCases

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D82940#2125183 , @riccibruno wrote:

> > I'm not certain I understand, but I'm removing my LG while we figure it 
> > out. Can you expound a bit further?
>
> Take a look at the AST dump of a small modification of the reduced test case 
> which doesn't trigger the assertion:
>
>   void should_not_crash_with_switch_in_lambda() {
> switch (1)
> default:;
> auto f = [] {
>   switch (1)
>   default:;
> };
>   }
>
>
> Before the serialization of `LambdaExpr` for the `LambdaExpr *E`, we have 
> `E->getBody() == E->getCallOperator()->getBody()`.
>  After serialization this is not true anymore because we are writing and 
> reading the body directly when visiting the `LambdaExpr`
>  (the captures have the same problem).


The same is true in case of the simplest lambda example:

  void foo() {   // line 11
  []{};
  }

The AST that we got from the Parser has only one CompoundStmt node, but 
referenced in the CXXMethodDecl and added as a direct child to LambdaExpr:

  `-LambdaExpr 0x5643235372c8 
|-CXXRecordDecl 0x564323536c98  col:5 implicit class definition
| |-CXXMethodDecl 0x564323536dd8  col:5 constexpr operator() 
'auto () const -> void' inline
| | `-CompoundStmt 0x564323536e88 
`-CompoundStmt 0x564323536e88 

After serialization and deserialization, the CompoundStmt is just copied, 
notice the different pointer values:

  `-LambdaExpr 0x55a49f35f830 
|-CXXRecordDecl 0x55a49f35f860  col:5 imported implicit 
 class definition
| |-CXXMethodDecl 0x55a49f35fac8  col:5 imported constexpr 
operator() 'auto () const -> void' inline
| | `-CompoundStmt 0x55a49f360110 
`-CompoundStmt 0x55a49f35f820 

This suggests that we should prepare the ASTReader/ASTWriter to be able to 
handle references to Stmts similarly as it can already handle references to 
Decls (see `ASTWriter::DeclIDs`).
Does this sound as a good direction, should I move forward this way?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82940



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

[PATCH] D82928: [Coroutines] Fix code coverage for coroutine

[Xun Li via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG565e37c7702d: [Coroutines] Fix code coverage for coroutine 
(authored by lxfind).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82928

Files:
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/test/CoverageMapping/coroutine.cpp


Index: clang/test/CoverageMapping/coroutine.cpp
===
--- /dev/null
+++ clang/test/CoverageMapping/coroutine.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 
-emit-llvm -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping 
%s | FileCheck %s
+
+namespace std::experimental {
+template 
+struct coroutine_traits;
+
+template 
+struct coroutine_handle {
+  coroutine_handle() = default;
+  static coroutine_handle from_address(void *) noexcept { return {}; }
+};
+template <>
+struct coroutine_handle {
+  static coroutine_handle from_address(void *) { return {}; }
+  coroutine_handle() = default;
+  template 
+  coroutine_handle(coroutine_handle) noexcept {}
+};
+} // namespace std::experimental
+
+struct suspend_always {
+  bool await_ready() noexcept;
+  void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+  void await_resume() noexcept;
+};
+
+template <>
+struct std::experimental::coroutine_traits {
+  struct promise_type {
+int get_return_object();
+suspend_always initial_suspend();
+suspend_always final_suspend() noexcept;
+void return_value(int);
+  };
+};
+
+// CHECK-LABEL: _Z2f1i:
+int f1(int x) {   // CHECK-NEXT: File 0, [[@LINE]]:15 -> [[@LINE+7]]:2 = #0
+  if (x > 42) {   // CHECK-NEXT: File 0, [[@LINE]]:7 -> [[@LINE]]:13 = #0
+++x;  // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:14 -> 
[[@LINE-1]]:15 = #1
+  } else {// CHECK-NEXT: File 0, [[@LINE-2]]:15 -> [[@LINE]]:4 = #1
+co_return x + 42; // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:4 -> 
[[@LINE-1]]:10 = (#0 - #1)
+  }   // CHECK-NEXT: File 0, [[@LINE-2]]:10 -> [[@LINE]]:4 = 
(#0 - #1)
+  co_return x;// CHECK-NEXT: Gap,File 0, [[@LINE-1]]:4 -> [[@LINE]]:3 
= #1
+} // CHECK-NEXT: File 0, [[@LINE-1]]:3 -> [[@LINE]]:2 = #1
Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -908,6 +908,18 @@
 terminateRegion(S);
   }
 
+  void VisitCoroutineBodyStmt(const CoroutineBodyStmt *S) {
+extendRegion(S);
+Visit(S->getBody());
+  }
+
+  void VisitCoreturnStmt(const CoreturnStmt *S) {
+extendRegion(S);
+if (S->getOperand())
+  Visit(S->getOperand());
+terminateRegion(S);
+  }
+
   void VisitCXXThrowExpr(const CXXThrowExpr *E) {
 extendRegion(E);
 if (E->getSubExpr())


Index: clang/test/CoverageMapping/coroutine.cpp
===
--- /dev/null
+++ clang/test/CoverageMapping/coroutine.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 -emit-llvm -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping %s | FileCheck %s
+
+namespace std::experimental {
+template 
+struct coroutine_traits;
+
+template 
+struct coroutine_handle {
+  coroutine_handle() = default;
+  static coroutine_handle from_address(void *) noexcept { return {}; }
+};
+template <>
+struct coroutine_handle {
+  static coroutine_handle from_address(void *) { return {}; }
+  coroutine_handle() = default;
+  template 
+  coroutine_handle(coroutine_handle) noexcept {}
+};
+} // namespace std::experimental
+
+struct suspend_always {
+  bool await_ready() noexcept;
+  void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+  void await_resume() noexcept;
+};
+
+template <>
+struct std::experimental::coroutine_traits {
+  struct promise_type {
+int get_return_object();
+suspend_always initial_suspend();
+suspend_always final_suspend() noexcept;
+void return_value(int);
+  };
+};
+
+// CHECK-LABEL: _Z2f1i:
+int f1(int x) {   // CHECK-NEXT: File 0, [[@LINE]]:15 -> [[@LINE+7]]:2 = #0
+  if (x > 42) {   // CHECK-NEXT: File 0, [[@LINE]]:7 -> [[@LINE]]:13 = #0
+++x;  // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:14 -> [[@LINE-1]]:15 = #1
+  } else {// CHECK-NEXT: File 0, [[@LINE-2]]:15 -> [[@LINE]]:4 = #1
+co_return x + 42; // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:4 -> [[@LINE-1]]:10 = (#0 - #1)
+  }   // CHECK-NEXT: File 0, [[@LINE-2]]:10 -> [[@LINE]]:4 = (#0 - #1)
+  co_return x;// CHECK-NEXT: Gap,File 0, [[@LINE-1]]:4 -> [[@LINE]]:3 = #1
+} // CHECK-NEXT: File 0, [[@LINE-1]]:3 -> [[@LINE]]:2 = #1
Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===

[PATCH] D82926: [libfuzzer] [clang] Add __has_feature(fuzzing_coverage)

[Mitch Phillips via Phabricator via cfe-commits]

hctim added a comment.

So - the `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` flag is a property of the 
build system and not that of the compiler. There are some places (android) 
where enabling `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` globally changes the 
behaviour of large amounts of libraries in ways that break the build system.

Having this flag allows us to make targeted compile-time changes to libc based 
on sancov that don't require enabling 
`FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` across the entire build system.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82926



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

[PATCH] D82926: [libfuzzer] [clang] Add __has_feature(fuzzing_coverage)

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

In D82926#2125950 , @hctim wrote:

> So - the `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` flag is a property of the 
> build system and not that of the compiler. There are some places (android) 
> where enabling `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` globally changes 
> the behaviour of large amounts of libraries in ways that break the build 
> system.
>
> Having this flag allows us to make targeted compile-time changes to libc 
> based on sancov that don't require enabling 
> `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` across the entire build system.


Shouldn't we be fixing the fuzzing build for Android then?  
`FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` is what we use everywhere else, so I 
don't like the idea of making a new flag.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82926



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

[PATCH] D82926: [libfuzzer] [clang] Add __has_feature(fuzzing_coverage)

[Max Moroz via Phabricator via cfe-commits]

Dor1s added a comment.

If we still decide to proceed with this, would it make sense to expand it to 
`sanitizer_coverage` based on any sancov instrumentation being enabled? As you 
mentioned in the description, there might be users who manually enable certain 
sancov flags. I think it's good to be able to support those usecases too   
(e.g. other fuzzing engines).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82926



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

[PATCH] D82791: [lit] Improve lit's output with default settings and --verbose.

[Varun Gandhi via Phabricator via cfe-commits]

varungandhi-apple updated this revision to Diff 274865.
varungandhi-apple added a comment.

Rebase after changes in D82808 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82791

Files:
  llvm/docs/CommandGuide/lit.rst
  llvm/utils/lit/lit/LitConfig.py
  llvm/utils/lit/lit/OutputSettings.py
  llvm/utils/lit/lit/TestRunner.py
  llvm/utils/lit/lit/cl_arguments.py
  llvm/utils/lit/lit/display.py
  llvm/utils/lit/lit/main.py
  llvm/utils/lit/tests/shtest-format.py
  llvm/utils/lit/tests/shtest-run-at-line.py
  llvm/utils/lit/tests/unittest-failing-locator.py

Index: llvm/utils/lit/tests/unittest-failing-locator.py
===
--- /dev/null
+++ llvm/utils/lit/tests/unittest-failing-locator.py
@@ -0,0 +1,122 @@
+# Check that the locate_last_failing_run_line function works as expected.
+
+# RUN: %{python} %s
+# END.
+
+import unittest
+
+from lit.TestRunner import locate_last_run_line
+
+class TestRunLineLocatorHeuristics(unittest.TestCase):
+
+def test_basic(self):
+# from a script like
+# RUN: echo Hello 1>&2
+basic = (
+"+ : 'RUN: at line 1'\n"
+"+ echo Hello 1>&2\n"
+"+ Hello\n"
+)
+line_start, substr = locate_last_run_line(basic)
+self.assertEqual(line_start, 0)
+self.assertEqual(substr, "RUN: at line 1")
+
+def test_multiple(self):
+# from a script like
+# RUN: echo Hello 1>&2
+# RUN: false
+multiple = (
+"+ : 'RUN: at line 1'\n"
+"+ echo Hello 1>&2\n"
+"+ Hello\n"
+"+ : 'RUN: at line 2'\n"
+"+ false\n"
+)
+line_start, substr = locate_last_run_line(multiple)
+self.assertEqual(line_start, multiple.rfind("+ :"))
+self.assertEqual(substr, "RUN: at line 2")
+
+def test_varying_prefix(self):
+# from a script like
+# RUN: echo Hello 1>&2
+# RUN: false
+#
+# in a hypothetical shell which prints line-numbers on 'set +x'
+# (as an example of something that varies)
+varying_prefix = (
+"+ 1 : 'RUN: at line 1'\n"
+"+ 2 echo Hello 1>&2\n"
+"+ Hello\n"
+"+ 3 : 'RUN: at line 2'\n"
+"+ 4 false\n"
+)
+line_start, substr = locate_last_run_line(varying_prefix)
+self.assertEqual(line_start, varying_prefix.rfind("+ 3"))
+self.assertEqual(substr, "RUN: at line 2")
+
+def test_confusing_basic(self):
+# from a script like
+# RUN: echo 'RUN: at line 10' 1>&2
+confusing_basic = (
+"+ : 'RUN: at line 1'\n"
+"+ echo 'RUN: at line 10'\n"
+"RUN: at line 10\n"
+)
+line_start, substr = locate_last_run_line(confusing_basic)
+# FIXME: These should both be equal ideally.
+self.assertNotEqual(line_start, 0)
+self.assertNotEqual(substr, "RUN: at line 1")
+
+def test_confusing_multiple(self):
+# from a script like
+# RUN: echo 'RUN: at line 10' 1>&2
+# RUN: false
+confusing_multiple = (
+"+ : 'RUN: at line 1'\n"
+"+ echo 'RUN: at line 10'\n"
+"RUN: at line 10\n"
+"+ : 'RUN: at line 2'\n"
+"+ false\n"
+)
+line_start, substr = locate_last_run_line(confusing_multiple)
+self.assertEqual(line_start, confusing_multiple.rfind("+ :"))
+self.assertEqual(substr, "RUN: at line 2")
+
+def test_confusing_varying_prefix_1(self):
+# from a script like
+# RUN: echo 'RUN: at line 10' 1>&2
+# RUN: false
+#
+# in a hypothetical shell which prints line-numbers on 'set +x'
+# (as an example of something that varies)
+confusing_varying_prefix = (
+"+ 1 : 'RUN: at line 1'\n"
+"+ 2 echo 'RUN: at line 10'\n"
+"RUN: at line 10\n"
+"+ 3 : 'RUN: at line 2'\n"
+"+ 4 false\n"
+)
+line_start, substr = locate_last_run_line(confusing_varying_prefix)
+self.assertEqual(line_start, confusing_varying_prefix.rfind("+ 3"))
+self.assertEqual(substr, "RUN: at line 2")
+
+def test_confusing_varying_prefix_2(self):
+# from a script like
+# RUN: true
+# RUN: not echo 'RUN: at line 100'
+#
+# in a hypothetical shell which prints line-numbers on 'set +x'
+# (as an example of something that varies)
+confusing_varying_prefix = (
+"+ 1 : 'RUN: at line 1'\n"
+"+ 2 true\n"
+"+ 3 : 'RUN: at line 2'\n"
+"+ 4 not echo 'RUN: at line 100'\n"
+"+ RUN: at line 100\n"
+)
+line_start, substr = locate_last_run_line(confusing_varying_prefix)
+# F

[PATCH] D82928: [Coroutines] Fix code coverage for coroutine

[Florian Hahn via Phabricator via cfe-commits]

fhahn added a comment.

Looks like this causes a bunch of build bot failures, e.g 
http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/31465  b

It would be great if you could take a look.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82928



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

[PATCH] D82392: [CodeGen] Add public function to emit C++ destructor call.

[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:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82392



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

[PATCH] D82928: [Coroutines] Fix code coverage for coroutine

[Xun Li via Phabricator via cfe-commits]

lxfind added a comment.

In D82928#2126018 , @fhahn wrote:

> Looks like this causes a bunch of build bot failures, e.g 
> http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/31465  b
>
> It would be great if you could take a look.


Looks like it's creating a tmp file within the test dir. Let me take a look.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82928



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

[clang] e7c5da5 - [CodeGen] Add public function to emit C++ destructor call.

[via cfe-commits]

Author: zoecarver
Date: 2020-07-01T11:01:23-07:00
New Revision: e7c5da57a5f3fdb6649ff92cb8ba0ce3c4978668

URL: 
https://github.com/llvm/llvm-project/commit/e7c5da57a5f3fdb6649ff92cb8ba0ce3c4978668
DIFF: 
https://github.com/llvm/llvm-project/commit/e7c5da57a5f3fdb6649ff92cb8ba0ce3c4978668.diff

LOG: [CodeGen] Add public function to emit C++ destructor call.

Adds `CodeGen::getCXXDestructorImplicitParam`, to retrieve a C++ destructor's 
implicit parameter (after the "this" pointer) based on the ABI in the given 
CodeGenModule.

This will allow other frontends (Swift, for example) to easily emit calls to 
object destructors with correct ABI semantics and calling convetions.

This is needed for Swift C++ interop. Here's the corresponding Swift change: 
https://github.com/apple/swift/pull/32291

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

Added: 


Modified: 
clang/include/clang/CodeGen/CodeGenABITypes.h
clang/lib/CodeGen/ABIInfo.h
clang/lib/CodeGen/CGCXXABI.h
clang/lib/CodeGen/CodeGenABITypes.cpp
clang/lib/CodeGen/ItaniumCXXABI.cpp
clang/lib/CodeGen/MicrosoftCXXABI.cpp

Removed: 




diff  --git a/clang/include/clang/CodeGen/CodeGenABITypes.h 
b/clang/include/clang/CodeGen/CodeGenABITypes.h
index 0201f92074ec..3c745fadbe78 100644
--- a/clang/include/clang/CodeGen/CodeGenABITypes.h
+++ b/clang/include/clang/CodeGen/CodeGenABITypes.h
@@ -25,7 +25,9 @@
 
 #include "clang/AST/CanonicalType.h"
 #include "clang/AST/Type.h"
+#include "clang/Basic/ABI.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
+#include "llvm/IR/BasicBlock.h"
 
 namespace llvm {
 class AttrBuilder;
@@ -40,6 +42,7 @@ class Type;
 namespace clang {
 class ASTContext;
 class CXXConstructorDecl;
+class CXXDestructorDecl;
 class CXXRecordDecl;
 class CXXMethodDecl;
 class CodeGenOptions;
@@ -90,6 +93,12 @@ const CGFunctionInfo &arrangeFreeFunctionCall(CodeGenModule 
&CGM,
 ImplicitCXXConstructorArgs
 getImplicitCXXConstructorArgs(CodeGenModule &CGM, const CXXConstructorDecl *D);
 
+llvm::Value *
+getCXXDestructorImplicitParam(CodeGenModule &CGM, llvm::BasicBlock 
*InsertBlock,
+  llvm::BasicBlock::iterator InsertPoint,
+  const CXXDestructorDecl *D, CXXDtorType Type,
+  bool ForVirtualBase, bool Delegating);
+
 /// Returns null if the function type is incomplete and can't be lowered.
 llvm::FunctionType *convertFreeFunctionType(CodeGenModule &CGM,
 const FunctionDecl *FD);

diff  --git a/clang/lib/CodeGen/ABIInfo.h b/clang/lib/CodeGen/ABIInfo.h
index bb40dace8a84..474e5c1e4c6a 100644
--- a/clang/lib/CodeGen/ABIInfo.h
+++ b/clang/lib/CodeGen/ABIInfo.h
@@ -28,7 +28,6 @@ namespace clang {
 
 namespace CodeGen {
   class ABIArgInfo;
-  class Address;
   class CGCXXABI;
   class CGFunctionInfo;
   class CodeGenFunction;

diff  --git a/clang/lib/CodeGen/CGCXXABI.h b/clang/lib/CodeGen/CGCXXABI.h
index 2b7f45fcab98..f5b3fc13bbbd 100644
--- a/clang/lib/CodeGen/CGCXXABI.h
+++ b/clang/lib/CodeGen/CGCXXABI.h
@@ -402,6 +402,13 @@ class CGCXXABI {
  CXXCtorType Type, bool ForVirtualBase,
  bool Delegating, CallArgList &Args);
 
+  /// Get the implicit (second) parameter that comes after the "this" pointer,
+  /// or nullptr if there is isn't one.
+  virtual llvm::Value *
+  getCXXDestructorImplicitParam(CodeGenFunction &CGF,
+const CXXDestructorDecl *DD, CXXDtorType Type,
+bool ForVirtualBase, bool Delegating) = 0;
+
   /// Emit the destructor call.
   virtual void EmitDestructorCall(CodeGenFunction &CGF,
   const CXXDestructorDecl *DD, CXXDtorType 
Type,

diff  --git a/clang/lib/CodeGen/CodeGenABITypes.cpp 
b/clang/lib/CodeGen/CodeGenABITypes.cpp
index 322cf45c47af..d3a16a1d5acc 100644
--- a/clang/lib/CodeGen/CodeGenABITypes.cpp
+++ b/clang/lib/CodeGen/CodeGenABITypes.cpp
@@ -115,3 +115,16 @@ unsigned CodeGen::getLLVMFieldNumber(CodeGenModule &CGM,
  const FieldDecl *FD) {
   return CGM.getTypes().getCGRecordLayout(RD).getLLVMFieldNo(FD);
 }
+
+llvm::Value *CodeGen::getCXXDestructorImplicitParam(
+CodeGenModule &CGM, llvm::BasicBlock *InsertBlock,
+llvm::BasicBlock::iterator InsertPoint, const CXXDestructorDecl *D,
+CXXDtorType Type, bool ForVirtualBase, bool Delegating) {
+  CodeGenFunction CGF(CGM, /*suppressNewContext=*/true);
+  CGF.CurCodeDecl = D;
+  CGF.CurFuncDecl = D;
+  CGF.CurFn = InsertBlock->getParent();
+  CGF.Builder.SetInsertPoint(InsertBlock, InsertPoint);
+  return CGM.getCXXABI().getCXXDestructorImplicitParam(
+  CGF, D, Type, ForVirtualBase, Delegating);
+}

diff  --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp 
b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index 2829877cfe5d..80de2a6e3950 100644
--- a/clang/l

[clang] ddcf063 - [Coroutines] Fix test breakage in D82928

[Xun Li via cfe-commits]

Author: Xun Li
Date: 2020-07-01T11:17:21-07:00
New Revision: ddcf063dd52ff1f30ac27d6f9abce6a45685a2b2

URL: 
https://github.com/llvm/llvm-project/commit/ddcf063dd52ff1f30ac27d6f9abce6a45685a2b2
DIFF: 
https://github.com/llvm/llvm-project/commit/ddcf063dd52ff1f30ac27d6f9abce6a45685a2b2.diff

LOG: [Coroutines] Fix test breakage in D82928

Summary: The test file in D82928 generated temp files within the test 
directory, causing test failures. Fix it.

Reviewers: modocache, fhahn

Reviewed By: modocache

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/test/CoverageMapping/coroutine.cpp

Removed: 




diff  --git a/clang/test/CoverageMapping/coroutine.cpp 
b/clang/test/CoverageMapping/coroutine.cpp
index ec1d64e0f970..c149eefd1f01 100644
--- a/clang/test/CoverageMapping/coroutine.cpp
+++ b/clang/test/CoverageMapping/coroutine.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 
-emit-llvm -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping 
%s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 
-emit-llvm -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping 
%s -o - | FileCheck %s
 
 namespace std::experimental {
 template 



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

[PATCH] D82663: [CodeGen] Have CodeGen for fixed-point unsigned with padding emit signed operations.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Can the missing bit just be added?  It seems to me that frontends ought to be 
able to emit the obvious intrinsic for the semantic operation here rather than 
having to second-guess the backend.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82663



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

[PATCH] D82781: [OpenCL] Fix missing address space deduction in template variables

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Seems like you shouldn't do it earlier if the type is dependent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82781



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

[PATCH] D82984: Revert "[Coroutines] Fix code coverage for coroutine"

[Xun Li via Phabricator via cfe-commits]

lxfind created this revision.
lxfind added reviewers: fhahn, modocache.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
lxfind abandoned this revision.
lxfind added a comment.

Fix up in https://reviews.llvm.org/D82986


This reverts commit 565e37c7702d181804c12d36b6010c513c9b3417 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82984

Files:
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/test/CoverageMapping/coroutine.cpp


Index: clang/test/CoverageMapping/coroutine.cpp
===
--- clang/test/CoverageMapping/coroutine.cpp
+++ /dev/null
@@ -1,45 +0,0 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 
-emit-llvm -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping 
%s | FileCheck %s
-
-namespace std::experimental {
-template 
-struct coroutine_traits;
-
-template 
-struct coroutine_handle {
-  coroutine_handle() = default;
-  static coroutine_handle from_address(void *) noexcept { return {}; }
-};
-template <>
-struct coroutine_handle {
-  static coroutine_handle from_address(void *) { return {}; }
-  coroutine_handle() = default;
-  template 
-  coroutine_handle(coroutine_handle) noexcept {}
-};
-} // namespace std::experimental
-
-struct suspend_always {
-  bool await_ready() noexcept;
-  void await_suspend(std::experimental::coroutine_handle<>) noexcept;
-  void await_resume() noexcept;
-};
-
-template <>
-struct std::experimental::coroutine_traits {
-  struct promise_type {
-int get_return_object();
-suspend_always initial_suspend();
-suspend_always final_suspend() noexcept;
-void return_value(int);
-  };
-};
-
-// CHECK-LABEL: _Z2f1i:
-int f1(int x) {   // CHECK-NEXT: File 0, [[@LINE]]:15 -> [[@LINE+7]]:2 = #0
-  if (x > 42) {   // CHECK-NEXT: File 0, [[@LINE]]:7 -> [[@LINE]]:13 = #0
-++x;  // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:14 -> 
[[@LINE-1]]:15 = #1
-  } else {// CHECK-NEXT: File 0, [[@LINE-2]]:15 -> [[@LINE]]:4 = #1
-co_return x + 42; // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:4 -> 
[[@LINE-1]]:10 = (#0 - #1)
-  }   // CHECK-NEXT: File 0, [[@LINE-2]]:10 -> [[@LINE]]:4 = 
(#0 - #1)
-  co_return x;// CHECK-NEXT: Gap,File 0, [[@LINE-1]]:4 -> [[@LINE]]:3 
= #1
-} // CHECK-NEXT: File 0, [[@LINE-1]]:3 -> [[@LINE]]:2 = #1
Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -908,18 +908,6 @@
 terminateRegion(S);
   }
 
-  void VisitCoroutineBodyStmt(const CoroutineBodyStmt *S) {
-extendRegion(S);
-Visit(S->getBody());
-  }
-
-  void VisitCoreturnStmt(const CoreturnStmt *S) {
-extendRegion(S);
-if (S->getOperand())
-  Visit(S->getOperand());
-terminateRegion(S);
-  }
-
   void VisitCXXThrowExpr(const CXXThrowExpr *E) {
 extendRegion(E);
 if (E->getSubExpr())


Index: clang/test/CoverageMapping/coroutine.cpp
===
--- clang/test/CoverageMapping/coroutine.cpp
+++ /dev/null
@@ -1,45 +0,0 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 -emit-llvm -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping %s | FileCheck %s
-
-namespace std::experimental {
-template 
-struct coroutine_traits;
-
-template 
-struct coroutine_handle {
-  coroutine_handle() = default;
-  static coroutine_handle from_address(void *) noexcept { return {}; }
-};
-template <>
-struct coroutine_handle {
-  static coroutine_handle from_address(void *) { return {}; }
-  coroutine_handle() = default;
-  template 
-  coroutine_handle(coroutine_handle) noexcept {}
-};
-} // namespace std::experimental
-
-struct suspend_always {
-  bool await_ready() noexcept;
-  void await_suspend(std::experimental::coroutine_handle<>) noexcept;
-  void await_resume() noexcept;
-};
-
-template <>
-struct std::experimental::coroutine_traits {
-  struct promise_type {
-int get_return_object();
-suspend_always initial_suspend();
-suspend_always final_suspend() noexcept;
-void return_value(int);
-  };
-};
-
-// CHECK-LABEL: _Z2f1i:
-int f1(int x) {   // CHECK-NEXT: File 0, [[@LINE]]:15 -> [[@LINE+7]]:2 = #0
-  if (x > 42) {   // CHECK-NEXT: File 0, [[@LINE]]:7 -> [[@LINE]]:13 = #0
-++x;  // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:14 -> [[@LINE-1]]:15 = #1
-  } else {// CHECK-NEXT: File 0, [[@LINE-2]]:15 -> [[@LINE]]:4 = #1
-co_return x + 42; // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:4 -> [[@LINE-1]]:10 = (#0 - #1)
-  }   // CHECK-NEXT: File 0, [[@LINE-2]]:10 -> [[@LINE]]:4 = (#0 - #1)
-  co_return x;// CHECK-NEXT: Gap,File 0, [[@LINE-1]]:4 -> [[@LINE]]:3 = #1
-} // CHECK-NEXT: File 0, [[@LINE-1]]:3