[PATCH] D58934: [clang-format] Fix lambdas returning template specialization that contains operator in parameter

[MyDeveloperDay via Phabricator via cfe-commits]

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

I'm not sure I personally would ever write code like that ;-) , but if its 
legal C++ and it compiles we should handle it the same as 
foo<1>,foo,foo

As there are a number of reviews out there for formatting Lambdas I think its a 
good idea for us to add corner cases like this to the unit tests, but it does 
get me thinking if this shouldn't be handled by a piece of code which knows 
about trailing return types (template or otherwise) and not be in the general 
Lambda parsing code

I suspect that given that the switch statement handles

  tok::identifier, tok::less, tok::numeric_constant, tok::greater
  foo<  1  >

We are effectively just consuming the return type tokens.

But to handle what you have here it LGTM and handles more use cases that 
https://bugs.llvm.org/show_bug.cgi?id=40910 would throw up.

Thanks for helping out


Repository:
  rC Clang

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

https://reviews.llvm.org/D58934



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

[PATCH] D58922: [clang-format] broken after lambda with return type template with boolean literal

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D58922#1417605 , @jkorous wrote:

> BTW I suggest you also add the original test case since the test cases you 
> added fail the expectation at FormatTest.cpp:74 and the message is a bit 
> unclear whether the testcase or the actual implementation is at fault.
>
>   EXPECT_EQ(Expected.str(), format(Expected, Style))
>   << "Expected code is not stable";
>   
>
> The original reported case IMO gives a better idea what is broken since it 
> duplicates the end-of-namespace comment.
>  https://bugs.llvm.org/show_bug.cgi?id=40910


do you mean this case? as this seems to work for me?

  verifyFormat("namespace bar {\n"
 "auto foo{[]() -> foo { ; }};\n"
 "} // namespace bar");



  $ ./tools/clang/unittests/Format/FormatTests.exe 
--gtest_filter=FormatTest.FormatsLambdas*
  Note: Google Test filter = FormatTest.FormatsLambdas*
  [==] Running 1 test from 1 test case.
  [--] Global test environment set-up.
  [--] 1 test from FormatTest
  [ RUN  ] FormatTest.FormatsLambdas
  [   OK ] FormatTest.FormatsLambdas (1353 ms)
  [--] 1 test from FormatTest (1354 ms total)
  
  [--] Global test environment tear-down
  [==] 1 test from 1 test case ran. (1362 ms total)
  [  PASSED  ] 1 test.

BTW if you want me to land this before you land D58934: [clang-format] Fix 
lambdas returning template specialization that contains operator in parameter 
 I need someone to mark this as accepted in 
Phabricator.


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

https://reviews.llvm.org/D58922



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

[PATCH] D58811: [clang] Fix misuses of char width to char align

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri requested changes to this revision.
lebedev.ri added a comment.
This revision now requires changes to proceed.

In D58811#1417959 , @amalykh wrote:

> Thank you for your comments, Roman.
>  Despite the fact, that there are no official LLVM targets that will be 
> influenced by this patch, there are architectures which support only word 
> addressing mode, for which the assumption of char align equals to 8 is not 
> true.
>  The patch have been tested for a proprietary architecture, where there is a 
> difference between align and width of char. As for the official targets, it's 
> more like a style improvement.


Thank you for working on this!

So in other words this will sadly indeed not have any test coverage in LLVM 
proper, correct?
If so, i think you want to submit an RFC (mentioning this fact) to cfe-dev 
(maybe CC llvm-dev).


Repository:
  rC Clang

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

https://reviews.llvm.org/D58811



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

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In the abstract, it would be nice to warn about casts that always have UB 
because the address spaces don't overlap; however, I'm worried about how people 
might be using address spaces in strange ways today, knowing that they *do* 
overlap, only in ways that the compiler isn't smart enough for.  I think we 
should just be permissive in non-OpenCL mode.


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

https://reviews.llvm.org/D58346



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

[PATCH] D58878: [Diagnostics] Warn for assignments in bool contexts

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

This seems eminently reasonable to warn about.  I think people might 
legitimately complain about extending an existing warning to a new set of 
contexts without putting it under a new warning flag, though.  How annoying 
would it be to clone the warnings and put them in a subgroup?  You could also 
change the diagnostics to say "converting the result of an assignment to bool" 
instead of "using the result of an assignment as a condition".




Comment at: clang/test/Sema/assigment_in_bool_context.cpp:1
+// RUN: %clang_cc1 -Wparentheses -fsyntax-only -verify %s
+

This test should go in `test/SemaCXX`.  You should add a similar test in 
`test/Sema` that's C-only and which uses `_Bool`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58878



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

[PATCH] D58729: Emit boxed expression as a compile-time constant if the expression inside the parentheses is a string literal

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: test/SemaObjC/boxing-illegal.m:70
+  s = @(L"abc"); // expected-error {{illegal type 'int *' used in a boxed 
expression}}
+  s = @("\pabc"); // expected-error {{illegal type 'unsigned char *' used in a 
boxed expression}}
+}

I don't know what `\p` is supposed to be or why it apparently changes the type 
of the literal to `unsigned char *`, but none of these are ordinary string 
literals that are invalid as UTF-8.  I mean something like "\xFF", which still 
has type `char *` but will fail to parse as UTF-8, which will cause normal 
boxing to fail and return `nil`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58729



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

[PATCH] D58880: [WIP] [Looking for API feedback] [clangd] Type hierarchy subtypes

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

In D58880#1416754 , @kadircet wrote:

> Sorry I didn't notice the mailing thread beforehand, looks like Sam had a 
> good point regarding performing operations on types rather than 
> symbols(http://lists.llvm.org/pipermail/clangd-dev/2019-January/000241.html).
>  Does current implementation take this into account by either pointing at 
> template specializations and/or base template declarations?


I thought about this, and I believe that having the relationship be between 
symbols is sufficient.

The main utility in a type hierarchy view is to allow the user to navigate to 
base/derived classes. If a base or derived class is an //implicit// 
specialization, the desired navigation is to the corresponding template 
definition; if it's an //explicit// specialization, the desired navigation is 
to the explicit specialization's definition.

A class template will obviously have its own symbol, so that part's fine. 
Explicit specializations are not currently stored in the index, but I think it 
would make sense for them to be, for other reasons as well (e.g. a user may 
want to navigate to an explicit specialization definition via 
`"workspace/symbol"`), in which case they too will get their own symbols. So, 
in either case, the desired type hierarchy item can be obtained from a symbol.

The updated patch has tests covering these scenarios, specifically 
`Subtypes.ClassTemplate` and `Subtypes.DependentBase`. Two additional tests, 
`Subtypes.TemplateSpec1` and `Subtypes.TemplateSpec2` are disabled until we 
start storing explicit specializations in the index.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58880



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

[PATCH] D58880: [WIP] [Looking for API feedback] [clangd] Type hierarchy subtypes

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 189275.
nridge added a comment.

Add tests involving templates and template specializations


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58880

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/index/FileIndex.cpp
  clang-tools-extra/clangd/index/FileIndex.h
  clang-tools-extra/clangd/index/Index.cpp
  clang-tools-extra/clangd/index/Index.h
  clang-tools-extra/clangd/index/MemIndex.cpp
  clang-tools-extra/clangd/index/MemIndex.h
  clang-tools-extra/clangd/index/Merge.cpp
  clang-tools-extra/clangd/index/Merge.h
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/index/Serialization.h
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/SymbolCollector.h
  clang-tools-extra/clangd/index/YAMLSerialization.cpp
  clang-tools-extra/clangd/index/dex/Dex.cpp
  clang-tools-extra/clangd/index/dex/Dex.h
  clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp
  clang-tools-extra/unittests/clangd/DiagnosticsTests.cpp
  clang-tools-extra/unittests/clangd/FileIndexTests.cpp
  clang-tools-extra/unittests/clangd/IndexTests.cpp
  clang-tools-extra/unittests/clangd/TestTU.cpp
  clang-tools-extra/unittests/clangd/XRefsTests.cpp

Index: clang-tools-extra/unittests/clangd/XRefsTests.cpp
===
--- clang-tools-extra/unittests/clangd/XRefsTests.cpp
+++ clang-tools-extra/unittests/clangd/XRefsTests.cpp
@@ -1773,6 +1773,175 @@
   EXPECT_THAT(typeParents(Child3), ElementsAre());
 }
 
+SymbolID findSymbolIDByName(llvm::StringRef Name, SymbolIndex *Index) {
+  SymbolID Result;
+  FuzzyFindRequest Request;
+  Request.Query = Name;
+  Request.AnyScope = true;
+  Request.Limit = 1;
+  int ResultCount = 0;
+  Index->fuzzyFind(Request, [&](const Symbol ) {
+Result = S.ID;
+++ResultCount;
+  });
+  EXPECT_EQ(1, ResultCount);
+  return Result;
+}
+
+std::vector collectSubtypes(SymbolID Type, SymbolIndex *Index) {
+  std::vector Result;
+  Index->relations(Type, RelationKind::Subtype,
+   [](const SymbolID ) { Result.push_back(ID); });
+  return Result;
+}
+
+TEST(Subtypes, SimpleInheritance) {
+  Annotations Source(R"cpp(
+struct Parent {
+  int a;
+};
+
+struct Child1 : Parent {
+  int b;
+};
+
+struct Child2 : Child1 {
+  int c;
+};
+)cpp");
+
+  TestTU TU = TestTU::withCode(Source.code());
+  auto Index = TU.index();
+
+  SymbolID Parent = findSymbolIDByName("Parent", Index.get());
+  SymbolID Child1 = findSymbolIDByName("Child1", Index.get());
+  SymbolID Child2 = findSymbolIDByName("Child2", Index.get());
+
+  EXPECT_THAT(collectSubtypes(Parent, Index.get()), ElementsAre(Child1));
+  EXPECT_THAT(collectSubtypes(Child1, Index.get()), ElementsAre(Child2));
+}
+
+TEST(Subtypes, MultipleInheritance) {
+  Annotations Source(R"cpp(
+struct Parent1 {
+  int a;
+};
+
+struct Parent2 {
+  int b;
+};
+
+struct Parent3 : Parent2 {
+  int c;
+};
+
+struct Child : Parent1, Parent3 {
+  int d;
+};
+)cpp");
+
+  TestTU TU = TestTU::withCode(Source.code());
+  auto Index = TU.index();
+
+  SymbolID Parent1 = findSymbolIDByName("Parent1", Index.get());
+  SymbolID Parent2 = findSymbolIDByName("Parent2", Index.get());
+  SymbolID Parent3 = findSymbolIDByName("Parent3", Index.get());
+  SymbolID Child = findSymbolIDByName("Child", Index.get());
+
+  EXPECT_THAT(collectSubtypes(Parent1, Index.get()), ElementsAre(Child));
+  EXPECT_THAT(collectSubtypes(Parent2, Index.get()), ElementsAre(Parent3));
+  EXPECT_THAT(collectSubtypes(Parent3, Index.get()), ElementsAre(Child));
+}
+
+TEST(Subtypes, ClassTemplate) {
+  Annotations Source(R"cpp(
+struct Parent {};
+
+template 
+struct Child : Parent {};
+)cpp");
+
+  TestTU TU = TestTU::withCode(Source.code());
+  auto Index = TU.index();
+
+  SymbolID Parent = findSymbolIDByName("Parent", Index.get());
+  SymbolID Child = findSymbolIDByName("Child", Index.get());
+
+  EXPECT_THAT(collectSubtypes(Parent, Index.get()), ElementsAre(Child));
+}
+
+// This test fails because explicit class template specializations
+// are not currently indexed.
+TEST(DISABLED_Subtypes, TemplateSpec1) {
+  Annotations Source(R"cpp(
+template 
+struct Parent {};
+
+template <>
+struct Parent {};
+
+struct Child1 : Parent {};
+
+struct Child2 : Parent {};
+)cpp");
+
+  TestTU TU = TestTU::withCode(Source.code());
+  auto Index = TU.index();
+
+  SymbolID Parent = findSymbolIDByName("Parent", Index.get());
+  // Even if we start storing explicit specializations in the index,
+  // we will likely need additional changes for
+  // findSymbolIDByName("Parent") to find it.
+  SymbolID ParentSpec = findSymbolIDByName("Parent", Index.get());
+  SymbolID Child1 = findSymbolIDByName("Child1", Index.get());
+  SymbolID Child2 = 

[PATCH] D58811: [clang] Fix misuses of char width to char align

[Aleksandr Malykh via Phabricator via cfe-commits]

amalykh added a comment.

Thank you for your comments, Roman.
Despite the fact, that there are no official LLVM targets that will be 
influenced by this patch, there are architectures which support only word 
addressing mode, for which the assumption of char align equals to 8 is not true.
The patch have been tested for a proprietary architecture, where there is a 
difference between align and width of char. As for the official targets, it's 
more like a style improvement.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58811



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

[PATCH] D58659: [Sema] Fix assertion when `auto` parameter in lambda has an attribute.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added inline comments.



Comment at: clang/lib/Sema/SemaType.cpp:266
+auto *NewAttrTy = cast(T.getTypePtr());
+for (TypeAttrPair  : AttrsForTypes) {
+  if (A.first == AttrTy)

jkorous wrote:
> Do you think it would make sense to terminate the loop early if 
> `AttrsForTypesSorted == true`?
I agree with you it would be nice to reuse `AttrsForTypesSorted` property but 
in practice it doesn't matter. We sort `AttrsForTypes` only in 
`takeAttrForAttributedType` and we should do that after we've populated 
`AttrsForTypes` and after we've replaced `auto` types. It isn't strictly 
required but that's how it was working in debugger. Also I've tried to add 
`assert(! AttrsForTypesSorted);` and it wasn't triggered during `ninja 
check-clang`

Having `AttrsForTypes` sorted can be useful in the future but I don't think we 
should add code for that now.

Despite the linear iteration, we are not pessimizing the most common use case 
with no attributes for lambda parameters (expression "most common use case" is 
based on my experience and not substantiated by any data). So I think the 
performance shouldn't degrade noticeably.


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

https://reviews.llvm.org/D58659



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

[PATCH] D58094: Fix -Wnonportable-include-path suppression for header maps with absolute paths.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Ping.


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

https://reviews.llvm.org/D58094



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

[PATCH] D58924: Replace clang::FileData with llvm::vfs::Status

[Harlan Haskins via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL355368: Replace clang::FileData with llvm::vfs::Status 
(authored by harlanhaskins, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58924?vs=189202=189258#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58924

Files:
  cfe/trunk/include/clang/Basic/FileManager.h
  cfe/trunk/include/clang/Basic/FileSystemStatCache.h
  cfe/trunk/lib/Basic/FileManager.cpp
  cfe/trunk/lib/Basic/FileSystemStatCache.cpp
  cfe/trunk/lib/Frontend/TextDiagnostic.cpp
  cfe/trunk/unittests/Basic/FileManagerTest.cpp

Index: cfe/trunk/lib/Basic/FileManager.cpp
===
--- cfe/trunk/lib/Basic/FileManager.cpp
+++ cfe/trunk/lib/Basic/FileManager.cpp
@@ -144,8 +144,8 @@
   StringRef InterndDirName = NamedDirEnt.first();
 
   // Check to see if the directory exists.
-  FileData Data;
-  if (getStatValue(InterndDirName, Data, false, nullptr /*directory lookup*/)) {
+  llvm::vfs::Status Status;
+  if (getStatValue(InterndDirName, Status, false, nullptr /*directory lookup*/)) {
 // There's no real directory at the given path.
 if (!CacheFailure)
   SeenDirEntries.erase(DirName);
@@ -156,7 +156,7 @@
   // same inode (this occurs on Unix-like systems when one dir is
   // symlinked to another, for example) or the same path (on
   // Windows).
-  DirectoryEntry  = UniqueRealDirs[Data.UniqueID];
+  DirectoryEntry  = UniqueRealDirs[Status.getUniqueID()];
 
   NamedDirEnt.second = 
   if (UDE.getName().empty()) {
@@ -205,8 +205,8 @@
 
   // Check to see if the file exists.
   std::unique_ptr F;
-  FileData Data;
-  if (getStatValue(InterndFileName, Data, true, openFile ?  : nullptr)) {
+  llvm::vfs::Status Status;
+  if (getStatValue(InterndFileName, Status, true, openFile ?  : nullptr)) {
 // There's no real file at the given path.
 if (!CacheFailure)
   SeenFileEntries.erase(Filename);
@@ -218,14 +218,15 @@
 
   // It exists.  See if we have already opened a file with the same inode.
   // This occurs when one dir is symlinked to another, for example.
-  FileEntry  = UniqueRealFiles[Data.UniqueID];
+  FileEntry  = UniqueRealFiles[Status.getUniqueID()];
 
   NamedFileEnt.second = 
 
   // If the name returned by getStatValue is different than Filename, re-intern
   // the name.
-  if (Data.Name != Filename) {
-auto  = *SeenFileEntries.insert({Data.Name, }).first;
+  if (Status.getName() != Filename) {
+auto  =
+  *SeenFileEntries.insert({Status.getName(), }).first;
 assert(NamedFileEnt.second ==  &&
"filename from getStatValue() refers to wrong file");
 InterndFileName = NamedFileEnt.first().data();
@@ -239,7 +240,7 @@
 // module's structure when its headers/module map are mapped in the VFS.
 // We should remove this as soon as we can properly support a file having
 // multiple names.
-if (DirInfo != UFE.Dir && Data.IsVFSMapped)
+if (DirInfo != UFE.Dir && Status.IsVFSMapped)
   UFE.Dir = DirInfo;
 
 // Always update the name to use the last name by which a file was accessed.
@@ -254,13 +255,12 @@
 
   // Otherwise, we don't have this file yet, add it.
   UFE.Name= InterndFileName;
-  UFE.Size = Data.Size;
-  UFE.ModTime = Data.ModTime;
+  UFE.Size= Status.getSize();
+  UFE.ModTime = llvm::sys::toTimeT(Status.getLastModificationTime());
   UFE.Dir = DirInfo;
   UFE.UID = NextFileUID++;
-  UFE.UniqueID = Data.UniqueID;
-  UFE.IsNamedPipe = Data.IsNamedPipe;
-  UFE.InPCH = Data.InPCH;
+  UFE.UniqueID = Status.getUniqueID();
+  UFE.IsNamedPipe = Status.getType() == llvm::sys::fs::file_type::fifo_file;
   UFE.File = std::move(F);
   UFE.IsValid = true;
 
@@ -298,12 +298,15 @@
  "The directory of a virtual file should already be in the cache.");
 
   // Check to see if the file exists. If so, drop the virtual file
-  FileData Data;
+  llvm::vfs::Status Status;
   const char *InterndFileName = NamedFileEnt.first().data();
-  if (getStatValue(InterndFileName, Data, true, nullptr) == 0) {
-Data.Size = Size;
-Data.ModTime = ModificationTime;
-UFE = [Data.UniqueID];
+  if (getStatValue(InterndFileName, Status, true, nullptr) == 0) {
+UFE = [Status.getUniqueID()];
+Status = llvm::vfs::Status(
+  Status.getName(), Status.getUniqueID(),
+  llvm::sys::toTimePoint(ModificationTime),
+  Status.getUser(), Status.getGroup(), Size,
+  Status.getType(), Status.getPermissions());
 
 NamedFileEnt.second = UFE;
 
@@ -317,10 +320,9 @@
 if (UFE->isValid())
   return UFE;
 
-UFE->UniqueID = Data.UniqueID;
-UFE->IsNamedPipe = Data.IsNamedPipe;
-UFE->InPCH = Data.InPCH;
-fillRealPathName(UFE, Data.Name);
+UFE->UniqueID = Status.getUniqueID();
+UFE->IsNamedPipe = Status.getType() == 

[PATCH] D58924: Replace clang::FileData with llvm::vfs::Status

[Ben Langmuir via Phabricator via cfe-commits]

benlangmuir added inline comments.



Comment at: clang/lib/Basic/FileManager.cpp:305
+UFE = [Status.getUniqueID()];
+Status = llvm::vfs::Status(
+  Status.getName(), Status.getUniqueID(),

harlanhaskins wrote:
> benlangmuir wrote:
> > Why copy Status back into Status instead of mutating the relevant fields?
> The fields don't have setters exposed, and I couldn't decide if adding the 
> setters vs. re-constructing a Status was better here. Would it be worth 
> adding setters to the properties in `Status`?
Nah, just go with the code you already wrote.  Thanks for the explanation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58924



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

[PATCH] D58924: Replace clang::FileData with llvm::vfs::Status

[Harlan Haskins via Phabricator via cfe-commits]

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



Comment at: clang/lib/Basic/FileManager.cpp:305
+UFE = [Status.getUniqueID()];
+Status = llvm::vfs::Status(
+  Status.getName(), Status.getUniqueID(),

benlangmuir wrote:
> Why copy Status back into Status instead of mutating the relevant fields?
The fields don't have setters exposed, and I couldn't decide if adding the 
setters vs. re-constructing a Status was better here. Would it be worth adding 
setters to the properties in `Status`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58924



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

[PATCH] D58924: Replace clang::FileData with llvm::vfs::Status

[Ben Langmuir via Phabricator via cfe-commits]

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

LGTM, although I made a small suggestion for clarity.  FYI `InPCH` was used by 
PTH, which was removed a couple of months ago.




Comment at: clang/lib/Basic/FileManager.cpp:305
+UFE = [Status.getUniqueID()];
+Status = llvm::vfs::Status(
+  Status.getName(), Status.getUniqueID(),

Why copy Status back into Status instead of mutating the relevant fields?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58924



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

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

[Jan Korous via Phabricator via cfe-commits]

jkorous added reviewers: kadircet, gribozavr.
jkorous added a comment.

Adding clangd folks in case they want to take a look.


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

https://reviews.llvm.org/D58418



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

[PATCH] D58749: [index-while-building] IndexRecordHasher

[Jan Korous via Phabricator via cfe-commits]

jkorous added reviewers: kadircet, gribozavr.
jkorous added a comment.

Adding clangd folks in case they want to take a look.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58749



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

[PATCH] D58941: [clang-format][docs][NFC] Fix example for Allman brace breaking style

[Jan Korous via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL355365: [clang-format][docs][NFC] Fix example for Allman 
brace breaking style (authored by jkorous, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58941?vs=189242=189250#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58941

Files:
  cfe/trunk/docs/ClangFormatStyleOptions.rst


Index: cfe/trunk/docs/ClangFormatStyleOptions.rst
===
--- cfe/trunk/docs/ClangFormatStyleOptions.rst
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst
@@ -925,19 +925,28 @@
 
 .. code-block:: c++
 
-  try {
+  try
+  {
 foo();
   }
-  catch () {
+  catch ()
+  {
   }
   void foo() { bar(); }
-  class foo {
+  class foo
+  {
   };
-  if (foo()) {
+  if (foo())
+  {
   }
-  else {
+  else
+  {
   }
-  enum X : int { A, B };
+  enum X : int
+  {
+A,
+B
+  };
 
   * ``BS_GNU`` (in configuration: ``GNU``)
 Always break before braces and add an extra level of indentation to


Index: cfe/trunk/docs/ClangFormatStyleOptions.rst
===
--- cfe/trunk/docs/ClangFormatStyleOptions.rst
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst
@@ -925,19 +925,28 @@
 
 .. code-block:: c++
 
-  try {
+  try
+  {
 foo();
   }
-  catch () {
+  catch ()
+  {
   }
   void foo() { bar(); }
-  class foo {
+  class foo
+  {
   };
-  if (foo()) {
+  if (foo())
+  {
   }
-  else {
+  else
+  {
   }
-  enum X : int { A, B };
+  enum X : int
+  {
+A,
+B
+  };
 
   * ``BS_GNU`` (in configuration: ``GNU``)
 Always break before braces and add an extra level of indentation to
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r355365 - [clang-format][docs][NFC] Fix example for Allman brace breaking style

[Jan Korous via cfe-commits]

Author: jkorous
Date: Mon Mar  4 17:45:31 2019
New Revision: 355365

URL: http://llvm.org/viewvc/llvm-project?rev=355365=rev
Log:
[clang-format][docs][NFC] Fix example for Allman brace breaking style

I assume the example is wrong as it's clearly missing line-breaks before
braces.

I just ran the example through clang-format with .clang-format like
this:
BreakBeforeBraces: Allman

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

Modified:
cfe/trunk/docs/ClangFormatStyleOptions.rst

Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=355365=355364=355365=diff
==
--- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst Mon Mar  4 17:45:31 2019
@@ -925,19 +925,28 @@ the configuration (without a prefix: ``A
 
 .. code-block:: c++
 
-  try {
+  try
+  {
 foo();
   }
-  catch () {
+  catch ()
+  {
   }
   void foo() { bar(); }
-  class foo {
+  class foo
+  {
   };
-  if (foo()) {
+  if (foo())
+  {
   }
-  else {
+  else
+  {
   }
-  enum X : int { A, B };
+  enum X : int
+  {
+A,
+B
+  };
 
   * ``BS_GNU`` (in configuration: ``GNU``)
 Always break before braces and add an extra level of indentation to


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

[PATCH] D58941: [clang-format][docs][NFC] Fix example for Allman brace breaking style

[Jonas Devlieghere via Phabricator via cfe-commits]

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

LGTM. Good catch, left looks like K or something.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58941



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

[PATCH] D58941: [clang-format][docs][NFC] Fix example for Allman brace breaking style

[Jan Korous via Phabricator via cfe-commits]

jkorous created this revision.
jkorous added reviewers: JDevlieghere, krasimir, benhamilton.
Herald added subscribers: cfe-commits, dexonsmith.
Herald added a project: clang.

I assume the example is wrong as it's clearly missing line-breaks before 
braces. That's about my level of understanding and I am totally oblivious to 
any finer details.

I just ran the example through clang-format with .clang-format like this:

BreakBeforeBraces: Allman


Repository:
  rC Clang

https://reviews.llvm.org/D58941

Files:
  clang/docs/ClangFormatStyleOptions.rst


Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -925,19 +925,28 @@
 
 .. code-block:: c++
 
-  try {
+  try
+  {
 foo();
   }
-  catch () {
+  catch ()
+  {
   }
   void foo() { bar(); }
-  class foo {
+  class foo
+  {
   };
-  if (foo()) {
+  if (foo())
+  {
   }
-  else {
+  else
+  {
   }
-  enum X : int { A, B };
+  enum X : int
+  {
+A,
+B
+  };
 
   * ``BS_GNU`` (in configuration: ``GNU``)
 Always break before braces and add an extra level of indentation to


Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -925,19 +925,28 @@
 
 .. code-block:: c++
 
-  try {
+  try
+  {
 foo();
   }
-  catch () {
+  catch ()
+  {
   }
   void foo() { bar(); }
-  class foo {
+  class foo
+  {
   };
-  if (foo()) {
+  if (foo())
+  {
   }
-  else {
+  else
+  {
   }
-  enum X : int { A, B };
+  enum X : int
+  {
+A,
+B
+  };
 
   * ``BS_GNU`` (in configuration: ``GNU``)
 Always break before braces and add an extra level of indentation to
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58320: [Darwin] Introduce a new flag, -fapple-link-rtlib that forces linking of the builtins library.

[Amara Emerson via Phabricator via cfe-commits]

aemerson updated this revision to Diff 189237.
aemerson retitled this revision from "[Darwin] Introduce a new flag, 
-flink-builtins-rt that forces linking of the builtins library." to "[Darwin] 
Introduce a new flag, -fapple-link-rtlib that forces linking of the builtins 
library.".
aemerson edited the summary of this revision.
aemerson added a comment.

Since we can't use the proposed alternatives of -nolibc and -nostdlib++ 
reviving this patch as an Apple specific flag.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58320

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/Darwin.h
  clang/test/Driver/darwin-fapple-link-rtlib.c

Index: clang/test/Driver/darwin-fapple-link-rtlib.c
===
--- /dev/null
+++ clang/test/Driver/darwin-fapple-link-rtlib.c
@@ -0,0 +1,6 @@
+// RUN: %clang -target arm64-apple-ios12.0 %s -nostdlib -fapple-link-rtlib -resource-dir=%S/Inputs/resource_dir -### 2>&1 | FileCheck %s
+// RUN: %clang -target arm64-apple-ios12.0 %s -static -fapple-link-rtlib -resource-dir=%S/Inputs/resource_dir -### 2>&1 | FileCheck %s
+// RUN: %clang -target arm64-apple-ios12.0 %s -fapple-link-rtlib -resource-dir=%S/Inputs/resource_dir -### 2>&1 | FileCheck %s --check-prefix=DEFAULT
+// CHECK-NOT: "-lSystem"
+// DEFAULT: "-lSystem"
+// CHECK: libclang_rt.ios.a
Index: clang/lib/Driver/ToolChains/Darwin.h
===
--- clang/lib/Driver/ToolChains/Darwin.h
+++ clang/lib/Driver/ToolChains/Darwin.h
@@ -157,7 +157,8 @@
   /// FIXME: This API is intended for use with embedded libraries only, and is
   /// misleadingly named.
   virtual void AddLinkRuntimeLibArgs(const llvm::opt::ArgList ,
- llvm::opt::ArgStringList ) const;
+ llvm::opt::ArgStringList ,
+ bool ForceLinkBuiltinRT = false) const;
 
   virtual void addStartObjectFileArgs(const llvm::opt::ArgList ,
   llvm::opt::ArgStringList ) const {
@@ -495,7 +496,8 @@
   RuntimeLibType GetRuntimeLibType(const llvm::opt::ArgList ) const override;
 
   void AddLinkRuntimeLibArgs(const llvm::opt::ArgList ,
- llvm::opt::ArgStringList ) const override;
+ llvm::opt::ArgStringList ,
+ bool ForceLinkBuiltinRT = false) const override;
 
   void AddClangCXXStdlibIncludeArgs(
   const llvm::opt::ArgList ,
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -568,15 +568,26 @@
 
   if (getToolChain().ShouldLinkCXXStdlib(Args))
 getToolChain().AddCXXStdlibLibArgs(Args, CmdArgs);
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
+
+  bool NoStdOrDefaultLibs =
+  Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs);
+  bool ForceLinkBuiltins = Args.hasArg(options::OPT_fapple_link_rtlib);
+  if (!NoStdOrDefaultLibs || ForceLinkBuiltins) {
 // link_ssp spec is empty.
 
-// Let the tool chain choose which runtime library to link.
-getMachOToolChain().AddLinkRuntimeLibArgs(Args, CmdArgs);
+// If we have both -nostdlib/nodefaultlibs and -fapple-link-rtlib then
+// we just want to link the builtins, not the other libs like libSystem.
+if (NoStdOrDefaultLibs && ForceLinkBuiltins) {
+  getMachOToolChain().AddLinkRuntimeLib(Args, CmdArgs, "builtins");
+} else {
+  // Let the tool chain choose which runtime library to link.
+  getMachOToolChain().AddLinkRuntimeLibArgs(Args, CmdArgs,
+ForceLinkBuiltins);
 
-// No need to do anything for pthreads. Claim argument to avoid warning.
-Args.ClaimAllArgs(options::OPT_pthread);
-Args.ClaimAllArgs(options::OPT_pthreads);
+  // No need to do anything for pthreads. Claim argument to avoid warning.
+  Args.ClaimAllArgs(options::OPT_pthread);
+  Args.ClaimAllArgs(options::OPT_pthreads);
+}
   }
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
@@ -1074,7 +1085,8 @@
 }
 
 void DarwinClang::AddLinkRuntimeLibArgs(const ArgList ,
-ArgStringList ) const {
+ArgStringList ,
+bool ForceLinkBuiltinRT) const {
   // Call once to ensure diagnostic is printed if wrong value was specified
   GetRuntimeLibType(Args);
 
@@ -1082,8 +1094,11 @@
   // libraries with -static.
   if (Args.hasArg(options::OPT_static) ||
   Args.hasArg(options::OPT_fapple_kext) ||
-  Args.hasArg(options::OPT_mkernel))
+  

[PATCH] D58862: [cmake] Create exports for umbrella library targets

[Shoaib Meenai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL355354: [cmake] Create exports for umbrella library targets 
(authored by smeenai, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58862?vs=189019=189229#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58862

Files:
  cfe/trunk/cmake/modules/AddClang.cmake
  llvm/trunk/cmake/modules/AddLLVM.cmake


Index: cfe/trunk/cmake/modules/AddClang.cmake
===
--- cfe/trunk/cmake/modules/AddClang.cmake
+++ cfe/trunk/cmake/modules/AddClang.cmake
@@ -91,6 +91,7 @@
 if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY OR ${name} STREQUAL "libclang")
 
   if(${name} IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
+  "clang-libraries" IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
   NOT LLVM_DISTRIBUTION_COMPONENTS)
 set(export_to_clangtargets EXPORT ClangTargets)
 set_property(GLOBAL PROPERTY CLANG_HAS_EXPORTS True)
Index: llvm/trunk/cmake/modules/AddLLVM.cmake
===
--- llvm/trunk/cmake/modules/AddLLVM.cmake
+++ llvm/trunk/cmake/modules/AddLLVM.cmake
@@ -633,6 +633,7 @@
   # config file.
   if (NOT ARG_BUILDTREE_ONLY AND NOT ARG_MODULE)
 set_property( GLOBAL APPEND PROPERTY LLVM_LIBS ${name} )
+set(in_llvm_libs YES)
   endif()
 
   if (ARG_MODULE AND NOT TARGET ${name})
@@ -663,6 +664,7 @@
   endif()
 
   if(${name} IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
+  (in_llvm_libs AND "llvm-libraries" IN_LIST 
LLVM_DISTRIBUTION_COMPONENTS) OR
   NOT LLVM_DISTRIBUTION_COMPONENTS)
 set(export_to_llvmexports EXPORT LLVMExports)
 set_property(GLOBAL PROPERTY LLVM_HAS_EXPORTS True)


Index: cfe/trunk/cmake/modules/AddClang.cmake
===
--- cfe/trunk/cmake/modules/AddClang.cmake
+++ cfe/trunk/cmake/modules/AddClang.cmake
@@ -91,6 +91,7 @@
 if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY OR ${name} STREQUAL "libclang")
 
   if(${name} IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
+  "clang-libraries" IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
   NOT LLVM_DISTRIBUTION_COMPONENTS)
 set(export_to_clangtargets EXPORT ClangTargets)
 set_property(GLOBAL PROPERTY CLANG_HAS_EXPORTS True)
Index: llvm/trunk/cmake/modules/AddLLVM.cmake
===
--- llvm/trunk/cmake/modules/AddLLVM.cmake
+++ llvm/trunk/cmake/modules/AddLLVM.cmake
@@ -633,6 +633,7 @@
   # config file.
   if (NOT ARG_BUILDTREE_ONLY AND NOT ARG_MODULE)
 set_property( GLOBAL APPEND PROPERTY LLVM_LIBS ${name} )
+set(in_llvm_libs YES)
   endif()
 
   if (ARG_MODULE AND NOT TARGET ${name})
@@ -663,6 +664,7 @@
   endif()
 
   if(${name} IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
+  (in_llvm_libs AND "llvm-libraries" IN_LIST LLVM_DISTRIBUTION_COMPONENTS) OR
   NOT LLVM_DISTRIBUTION_COMPONENTS)
 set(export_to_llvmexports EXPORT LLVMExports)
 set_property(GLOBAL PROPERTY LLVM_HAS_EXPORTS True)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r355354 - [cmake] Create exports for umbrella library targets

[Shoaib Meenai via cfe-commits]

Author: smeenai
Date: Mon Mar  4 16:38:32 2019
New Revision: 355354

URL: http://llvm.org/viewvc/llvm-project?rev=355354=rev
Log:
[cmake] Create exports for umbrella library targets

When using the umbrella llvm-libraries and clang-libraries targets, we
should export all library targets, otherwise they'll be part of our
distribution but not usable from the CMake package.

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

Modified:
cfe/trunk/cmake/modules/AddClang.cmake

Modified: cfe/trunk/cmake/modules/AddClang.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/modules/AddClang.cmake?rev=355354=355353=355354=diff
==
--- cfe/trunk/cmake/modules/AddClang.cmake (original)
+++ cfe/trunk/cmake/modules/AddClang.cmake Mon Mar  4 16:38:32 2019
@@ -91,6 +91,7 @@ macro(add_clang_library name)
 if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY OR ${name} STREQUAL "libclang")
 
   if(${name} IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
+  "clang-libraries" IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
   NOT LLVM_DISTRIBUTION_COMPONENTS)
 set(export_to_clangtargets EXPORT ClangTargets)
 set_property(GLOBAL PROPERTY CLANG_HAS_EXPORTS True)


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

[PATCH] D58821: Inline asm constraints: allow ICE-like pointers for the "n" constraint (PR40890)

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> This fails with the new asm constraint checks, since the dead branch is never 
> pruned.

As far as I know, we didn't touch "i", only "n".  Is there a bug filed for the 
issue you're describing?




Comment at: clang/lib/CodeGen/CGStmt.cpp:1852
+IntResult =
+llvm::APSInt::get(EVResult.Val.getLValueOffset().getQuantity());
+  else

hans wrote:
> efriedma wrote:
> > This always returns an APSInt with width 64; is that really right?  I guess 
> > it might not really matter given that it's only going to be used as an 
> > immediate constant anyway, but it seems weird.
> I agree it seems a little strange, but I think in practice it's correct. 
> EVResult.Val.getLValueOffset().getQuantity() returns an int64_t, so we're not 
> losing any data.
> 
> The code that I lifted this from, is using the bitwidth of the casted-to 
> integer type for the result. But it's still only got maximum 64 bits since 
> the source, getLValueOffset().getQuantity(), is the same.
The concern isn't that we would lose data.   I'm more concerned the backend 
might not be prepared for a value of the "wrong" width.



Comment at: clang/lib/Sema/SemaStmtAsm.cpp:399
+  IntResult =
+  llvm::APSInt::get(EVResult.Val.getLValueOffset().getQuantity());
+else

hans wrote:
> efriedma wrote:
> > I think it makes sense to add a method to APValue specifically to do the 
> > conversion from LValue to an APSInt, whether or not isNullPointer() is 
> > true, and use it both here and in IntExprEvaluator::VisitCastExpr in 
> > lib/AST/ExprConstant.cpp.  The logic is sort of subtle (and I'm not 
> > completely sure it's right for targets where null is not zero, but you 
> > shouldn't try to fix that here).
> I agree (and this was also Bill's suggestion above) that it would be nice to 
> have a utility method for this.
> 
> I'm not sure adding one to APValue would work for 
> IntExprEvaluator::VisitCastExpr though, since that code is actually using its 
> own LValue class, not an APValue until it's time to return a result.
> 
> I frankly also doesn't fully understand what that code is doing. If the 
> LValue has a base value, it seems to just take that as result and ignore any 
> offset? This is unknown territory to me, but the way I read it, if there's an 
> lvalue base, the expression isn't going to come out as an integer constant. I 
> think.
> 
> About null pointers, I'm calling getTargetNullPointerValue() so I think that 
> should be okay, no?
Oh, I didn't realize IntExprEvaluator::VisitCastExpr wasn't using the same 
class to represent the value; that makes it harder to usefully refactor.  But 
still, it would be good to reduce the duplicated code between here and CodeGen.

> If the LValue has a base value, it seems to just take that as result and 
> ignore any offset?

If there's a base value, it returns the whole LValue, including the base and 
offset.

> I'm calling getTargetNullPointerValue() so I think that should be okay

The issue would be the case where you have a null pointer with an offset, like 
the case in the bug.  It's sort of inconsistent if null==-1, but null+1==1.  
But it's not something we handle consistently elsewhere, anyway, so I guess we 
can ignore it for now.


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

https://reviews.llvm.org/D58821



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

[PATCH] D58890: Modules: Rename MemoryBufferCache to InMemoryModuleCache

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

In D58890#1417465 , @bruno wrote:

> `InMemoryModuleCache` seems like a way more appropriate name here. Also 
> thanks for improving some of the comments.
>
> > Because of the move to Serialization we can no longer abuse the 
> > Preprocessor to forward it to the ASTReader. Besides the rename and file 
> > move, that means Preprocessor::Preprocessor has one fewer parameter and 
> > ASTReader::ASTReader has one more.
>
> Are there any C api bits that expose preprocessor stuff and also need updates?


`ninja check-clang` passes... is there anything else I should be testing?


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

https://reviews.llvm.org/D58890



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

[PATCH] D58862: [cmake] Create exports for umbrella library targets

[Petr Hosek via Phabricator via cfe-commits]

phosek accepted this revision.
phosek 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/D58862/new/

https://reviews.llvm.org/D58862



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

[PATCH] D58497: Clear the KnownModules cache if the preprocessor is going away

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai added a comment.

Ping.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58497



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

[PATCH] D58922: [clang-format] broken after lambda with return type template with boolean literal

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

BTW I suggest you also add the original test case since the test cases you 
added fail the expectation at FormatTest.cpp:74 and the message is a bit 
unclear whether the testcase or the actual implementation is at fault.

  EXPECT_EQ(Expected.str(), format(Expected, Style))
  << "Expected code is not stable";

The original reported case IMO gives a better idea what is broken since it 
duplicates the end-of-namespace comment.
https://bugs.llvm.org/show_bug.cgi?id=40910


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

https://reviews.llvm.org/D58922



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

[PATCH] D58930: Add XCOFF triple object format type for AIX

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: llvm/lib/Support/Triple.cpp:537
   return StringSwitch(EnvironmentName)
+// FIXME: We have to put XCOFF before COFF;
+// perhaps an order-independent pattern matching is desired?

If the conclusion is that checking XCOFF before COFF is fine, then we should 
remove the FIXME and just leave a normal comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58930



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

[PATCH] D58922: [clang-format] broken after lambda with return type template with boolean literal

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

I added couple other cases to your fix here: https://reviews.llvm.org/D58934


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

https://reviews.llvm.org/D58922



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

[PATCH] D58934: [clang-format] Fix lambdas returning template specialization that contains operator in parameter

[Jan Korous via Phabricator via cfe-commits]

jkorous created this revision.
jkorous added reviewers: klimek, djasper, JonasToth, alexfh, krasimir, 
MyDeveloperDay.
Herald added subscribers: cfe-commits, jdoerfert, dexonsmith.
Herald added a project: clang.

Inspired by https://reviews.llvm.org/D58922

Since this code compiles I assume we should add those tokens to switch:

  template struct foo {};
  
  int main() {
{ auto lambda = []() -> foo<5+2> { return {}; }; }
{ auto lambda = []() -> foo<5-2> { return {}; }; }
{ auto lambda = []() -> foo<5/3> { return {}; }; }
{ auto lambda = []() -> foo<5%2> { return {}; }; }
{ auto lambda = []() -> foo<5<<2> { return {}; }; }
{ auto lambda = []() -> foo { return {}; }; }
{ auto lambda = []() -> foo<~5> { return {}; }; }
{ auto lambda = []() -> foo<5|2> { return {}; }; }
{ auto lambda = []() -> foo<5||2> { return {}; }; }
{ auto lambda = []() -> foo<5&2> { return {}; }; }
{ auto lambda = []() -> foo<5&&2> { return {}; }; }
{ auto lambda = []() -> foo<5==2> { return {}; }; }
{ auto lambda = []() -> foo<5!=2> { return {}; }; }
{ auto lambda = []() -> foo<5>=2> { return {}; }; }
{ auto lambda = []() -> foo<5<2> { return {}; }; }
{ auto lambda = []() -> foo<5<=2> { return {}; }; }
{ auto lambda = []() -> foo<2 ? 1 : 0> { return {}; }; }
  }


Repository:
  rC Clang

https://reviews.llvm.org/D58934

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11846,6 +11846,96 @@
   verifyGoogleFormat("auto a = [, c](D* d) -> D& {};");
   verifyGoogleFormat("auto a = [, c](D* d) -> const D* {};");
   verifyFormat("[a, a]() -> a<1> {};");
+  verifyFormat("[]() -> foo<5 + 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 - 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 / 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 * 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 % 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 << 2> { return {}; };");
+  verifyFormat("[]() -> foo { return {}; };");
+  verifyFormat("[]() -> foo<~5> { return {}; };");
+  verifyFormat("[]() -> foo<5 | 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 || 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 & 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 && 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 == 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 != 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 >= 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 <= 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 < 2> { return {}; };");
+  verifyFormat("[]() -> foo<2 ? 1 : 0> { return {}; };");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 + 2> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 - 2> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 / 2> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 * 2> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 % 2> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 << 2> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<~5> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 | 2> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 || 2> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 & 2> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 && 2> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 == 2> { return {}; }};\n"
+ 

[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like elements

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added a comment.

@NoQ Ping 2 for reviews please.


Repository:
  rC Clang

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

https://reviews.llvm.org/D50488



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

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

[Bernhard Manfred Gruber via Phabricator via cfe-commits]

bernhardmgruber updated this revision to Diff 189222.
bernhardmgruber marked 3 inline comments as done.
bernhardmgruber added a comment.

Fixed some little nits, thanks @JonasToth!


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

https://reviews.llvm.org/D56160

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

Index: test/clang-tidy/modernize-use-trailing-return-type.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-trailing-return-type.cpp
@@ -0,0 +1,437 @@
+// RUN: %check_clang_tidy %s modernize-use-trailing-return-type %t -- -- --std=c++14 -fdeclspec
+
+namespace std {
+template 
+class vector;
+
+template 
+class array;
+
+class string;
+
+template 
+auto declval() -> T;
+}
+
+//
+// Functions
+//
+
+int f();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto f() -> int;{{$}}
+int ((f))();
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto ((f))() -> int;{{$}}
+int f(int);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto f(int) -> int;{{$}}
+int f(int arg);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto f(int arg) -> int;{{$}}
+int f(int arg1, int arg2, int arg3);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto f(int arg1, int arg2, int arg3) -> int;{{$}}
+int f(int arg1, int arg2, int arg3, ...);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto f(int arg1, int arg2, int arg3, ...) -> int;{{$}}
+template  int f(T t);
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}template  auto f(T t) -> int;{{$}}
+
+//
+// Functions with formatting
+//
+
+int a1() { return 42; }
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto a1() -> int { return 42; }{{$}}
+int a2() {
+return 42;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto a2() -> int {{{$}}
+int a3()
+{
+return 42;
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto a3() -> int{{$}}
+int a4(int   arg   )   ;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto a4(int   arg   ) -> int   ;{{$}}
+int a5
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto a5{{$}}
+(int arg);
+// CHECK-FIXES: {{^}}(int arg) -> int;{{$}}
+const
+int
+*
+a7
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+()
+// CHECK-FIXES: {{^}}() -> const{{$}}
+// CHECK-FIXES: {{^}}int{{$}}
+// CHECK-FIXES: {{^}}*{{$}}
+;
+
+int*a7(int arg);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto a7(int arg) -> int*;{{$}}
+template class C>
+Ca8(int arg);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto a8(int arg) -> C;{{$}}
+
+
+//
+// Functions with qualifiers and specifiers
+//
+
+inline int d1(int arg);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}inline auto d1(int arg) -> int;{{$}}
+extern "C" int d2(int arg);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}extern "C" auto d2(int arg) -> int;{{$}}
+inline int d3(int arg) noexcept(true);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use a trailing return type for this function 

[PATCH] D58821: Inline asm constraints: allow ICE-like pointers for the "n" constraint (PR40890)

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

The other problem is that we don't use the CFG machinery to prune dead 
branches. Consider the x86 in/out instructions: one variant takes an immediate, 
the other a register. The classic way to deal with that is something like

  static inline void outl(unsigned port, uint32_t value)
  {
if (__builtin_constant_p(port) && port < 0x100) {
  __asm volatile("outl %0,%w1" : : "a" (data), "id" (port));
} else {
 __asm volatile("outl %0,%w1" : : "a" (data), "d" (port));
}
  }

This fails with the new asm constraint checks, since the dead branch is never 
pruned. For other architectures it makes an even greater difference. The main 
reason it is a show stopper: there is no sane workaround that doesn't regress 
code quality.


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

https://reviews.llvm.org/D58821



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

[PATCH] D58930: Add XCOFF triple object format type for AIX

[Jason Liu via Phabricator via cfe-commits]

jasonliu created this revision.
jasonliu added reviewers: hubert.reinterpretcast, sfertile, chandlerc, 
apaprocki.
Herald added subscribers: llvm-commits, lldb-commits, cfe-commits, dexonsmith, 
aheejin, hiraditya, javed.absar.
Herald added projects: clang, LLDB, LLVM.

This patch adds an XCOFF triple object format type into LLVM. This XCOFF triple 
object file type will be used later by object file and assembly generation for 
the AIX platform.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58930

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/include/llvm/ADT/Triple.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/MC/MCContext.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
  llvm/unittests/ADT/TripleTest.cpp

Index: llvm/unittests/ADT/TripleTest.cpp
===
--- llvm/unittests/ADT/TripleTest.cpp
+++ llvm/unittests/ADT/TripleTest.cpp
@@ -1258,6 +1258,11 @@
   EXPECT_EQ(Triple::Wasm,
 Triple("wasm64-unknown-wasi-musl-wasm").getObjectFormat());
 
+  EXPECT_EQ(Triple::XCOFF, Triple("powerpc-ibm-aix").getObjectFormat());
+  EXPECT_EQ(Triple::XCOFF, Triple("powerpc64-ibm-aix").getObjectFormat());
+  EXPECT_EQ(Triple::XCOFF, Triple("powerpc---xcoff").getObjectFormat());
+  EXPECT_EQ(Triple::XCOFF, Triple("powerpc64---xcoff").getObjectFormat());
+
   Triple MSVCNormalized(Triple::normalize("i686-pc-windows-msvc-elf"));
   EXPECT_EQ(Triple::ELF, MSVCNormalized.getObjectFormat());
 
@@ -1276,6 +1281,9 @@
 
   T.setObjectFormat(Triple::MachO);
   EXPECT_EQ(Triple::MachO, T.getObjectFormat());
+  
+  T.setObjectFormat(Triple::XCOFF);
+  EXPECT_EQ(Triple::XCOFF, T.getObjectFormat());
 }
 
 TEST(TripleTest, NormalizeWindows) {
Index: llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
===
--- llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
+++ llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
@@ -5594,6 +5594,9 @@
   case MCObjectFileInfo::IsWasm:
 CurrentFormat = WASM;
 break;
+  case MCObjectFileInfo::IsXCOFF:
+llvm_unreachable("unexpected object format");
+break;
   }
 
   if (~Prefix->SupportedFormats & CurrentFormat) {
Index: llvm/lib/Support/Triple.cpp
===
--- llvm/lib/Support/Triple.cpp
+++ llvm/lib/Support/Triple.cpp
@@ -534,6 +534,9 @@
 
 static Triple::ObjectFormatType parseFormat(StringRef EnvironmentName) {
   return StringSwitch(EnvironmentName)
+// FIXME: We have to put XCOFF before COFF;
+// perhaps an order-independent pattern matching is desired?
+.EndsWith("xcoff", Triple::XCOFF)
 .EndsWith("coff", Triple::COFF)
 .EndsWith("elf", Triple::ELF)
 .EndsWith("macho", Triple::MachO)
@@ -622,6 +625,7 @@
   case Triple::ELF: return "elf";
   case Triple::MachO: return "macho";
   case Triple::Wasm: return "wasm";
+  case Triple::XCOFF: return "xcoff";
   }
   llvm_unreachable("unknown object format type");
 }
@@ -686,6 +690,8 @@
   case Triple::ppc64:
 if (T.isOSDarwin())
   return Triple::MachO;
+else if (T.isOSAIX())
+  return Triple::XCOFF;
 return Triple::ELF;
 
   case Triple::wasm32:
Index: llvm/lib/MC/MCParser/AsmParser.cpp
===
--- llvm/lib/MC/MCParser/AsmParser.cpp
+++ llvm/lib/MC/MCParser/AsmParser.cpp
@@ -710,6 +710,9 @@
   case MCObjectFileInfo::IsWasm:
 PlatformParser.reset(createWasmAsmParser());
 break;
+  case MCObjectFileInfo::IsXCOFF:
+// TODO: Need to implement createXCOFFAsmParser for XCOFF format.
+break;
   }
 
   PlatformParser->Initialize(*this);
Index: llvm/lib/MC/MCObjectFileInfo.cpp
===
--- llvm/lib/MC/MCObjectFileInfo.cpp
+++ llvm/lib/MC/MCObjectFileInfo.cpp
@@ -801,6 +801,10 @@
 Env = IsWasm;
 initWasmMCObjectFileInfo(TT);
 break;
+  case Triple::XCOFF:
+Env = IsXCOFF;
+// TODO: Initialize MCObjectFileInfo for XCOFF format when MCSectionXCOFF is ready.
+break;
   case Triple::UnknownObjectFormat:
 report_fatal_error("Cannot initialize MC for unknown object file format.");
 break;
@@ -816,6 +820,7 @@
   case Triple::MachO:
   case Triple::COFF:
   case Triple::Wasm:
+  case Triple::XCOFF:
   case Triple::UnknownObjectFormat:
 report_fatal_error("Cannot get DWARF comdat section for this object file "
"format: not implemented.");
Index: llvm/lib/MC/MCContext.cpp
===
--- llvm/lib/MC/MCContext.cpp
+++ llvm/lib/MC/MCContext.cpp
@@ -161,6 +161,9 @@
   return new (Name, *this) MCSymbolMachO(Name, IsTemporary);
 case 

Re: r354843 - [CGDebugInfo] Set NonTrivial DIFlag to a c++ record if it's not trivial

[Aaron Smith via cfe-commits]

Hi David, I can take a look at adding another test. Please read the code review 
which answers your question. A new flag is required. Thanks.


From: David Blaikie 
Sent: Tuesday, March 5, 2019 12:51:27 AM
To: Aaron Smith; Reid Kleckner; Adrian Prantl; Jonas Devlieghere
Cc: cfe-commits@lists.llvm.org
Subject: Re: r354843 - [CGDebugInfo] Set NonTrivial DIFlag to a c++ record if 
it's not trivial

Hi Aaron,

I don't see any mention of this in D44406 - so it might have been good to have 
a separate review for this (or included this in the review of D44406, which I 
think is possible with the monorepo).

Specifically - this change is missing test coverage (there should be a clang 
test that goes from C++ source code to LLVM IR & demonstrates the flag being 
emitted into the IR, etc).

Also - what's the reason the non-triviality can't be implied by the absence of 
the trivial flag? (or the other way around) - the flags seem redundant with one 
another.

On Mon, Feb 25, 2019 at 8:02 PM Aaron Smith via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: asmith
Date: Mon Feb 25 19:49:05 2019
New Revision: 354843

URL: 
http://llvm.org/viewvc/llvm-project?rev=354843=rev
Log:
[CGDebugInfo] Set NonTrivial DIFlag to a c++ record if it's not trivial

This goes with 
https://reviews.llvm.org/D44406


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

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=354843=354842=354843=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Mon Feb 25 19:49:05 2019
@@ -3031,6 +3031,8 @@ llvm::DICompositeType *CGDebugInfo::Crea
 // Record if a C++ record is trivial type.
 if (CXXRD->isTrivial())
   Flags |= llvm::DINode::FlagTrivial;
+else
+  Flags |= llvm::DINode::FlagNonTrivial;
   }

   llvm::DICompositeType *RealDecl = DBuilder.createReplaceableCompositeType(


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

[PATCH] D58893: Modules: Invalidate out-of-date PCMs as they're discovered

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

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

Thanks for working on this, the approach & patch LGTM.




Comment at: clang/include/clang/Serialization/InMemoryModuleCache.h:37
 
 /// Track the timeline of when this was added to the cache.
+bool IsFinal = false;

Now that generation count is gone, update (or remove) this comment?


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

https://reviews.llvm.org/D58893



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

[PATCH] D58154: Add support for -fpermissive.

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

rsmith added a comment.

In D58154#1417363 , @aaron.ballman 
wrote:

> I'm not super keen on supporting -fpermissive --  what is the impetus for 
> supporting this? Is it just for GCC compatibility?


Yes, exactly that -- this is to improve our drop-in compatibility with GCC. See 
http://llvm.org/PR40670 for an example user request for this.

In much the same way that `-w` can be used to mean "I just want this (crufty 
old, but presumed correct) code to compile; don't bother me with warnings about 
it", `-fpermissive` (and in particular `-fpermissive -w`) can be used to mean 
"I just want this (crufty, old, and relying on compiler extensions, but 
presumably correct) code to compile; don't bother me with complaints that it's 
technically ill-formed".

In D58154#1417358 , @jyknight wrote:

> Hm -- in GCC, -fpermissive has nothing at all to do with 
> -pedantic/-pedantic-errors, but I suppose it should be fine to do this way.


Yeah, I don't think it's likely that the restriction on combining those two 
flags will cause problems in practice.

[I don't think the behavior of `-fpermissive -pedantic-errors` (converting all 
error-by-default extensions to warnings and converting all non-error-by-default 
extensions to errors) was a carefully-considered design choice on GCC's part; 
rather, I think it's just the emergent behavior of the way they happen to deal 
with extension diagnostics. As a general design point for Clang, downgrading a 
diagnostic from error-by-default to warning-by-default shouldn't be a breaking 
change, but could be if we followed GCC here and someone actually used that 
flag combination. If someone actually hits the diagnostic for combining these 
two flags and can provide some kind of justification for wanting that behavior, 
we could reconsider.]


Repository:
  rC Clang

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

https://reviews.llvm.org/D58154



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

[PATCH] D58751: Order File Instrumentation: add clang support for -forder-file-instrumentation

[Manman Ren via Phabricator via cfe-commits]

manmanren closed this revision.
manmanren added a comment.

r355333


Repository:
  rC Clang

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

https://reviews.llvm.org/D58751



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

[PATCH] D58890: Modules: Rename MemoryBufferCache to InMemoryModuleCache

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added a comment.

`InMemoryModuleCache` seems like a way more appropriate name here. Also thanks 
for improving some of the comments.

> Because of the move to Serialization we can no longer abuse the Preprocessor 
> to forward it to the ASTReader. Besides the rename and file move, that means 
> Preprocessor::Preprocessor has one fewer parameter and ASTReader::ASTReader 
> has one more.

Are there any C api bits that expose preprocessor stuff and also need updates?


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

https://reviews.llvm.org/D58890



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

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 updated this revision to Diff 189205.
jyu2 added a comment.

Rebased


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

https://reviews.llvm.org/D56571

Files:
  include/clang/AST/Stmt.h
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/AST/ASTImporter.cpp
  lib/AST/Stmt.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/StmtProfile.cpp
  lib/Analysis/CFG.cpp
  lib/CodeGen/CGStmt.cpp
  lib/Parse/ParseStmtAsm.cpp
  lib/Sema/JumpDiagnostics.cpp
  lib/Sema/SemaStmtAsm.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  test/Analysis/asm-goto.cpp
  test/CodeGen/asm-goto.c
  test/CodeGen/asm.c
  test/CodeGen/inline-asm-mixed-style.c
  test/Coverage/c-language-features.inc
  test/PCH/asm.h
  test/Parser/asm.c
  test/Parser/asm.cpp
  test/Sema/asm-goto.cpp
  test/Sema/asm.c
  test/Sema/inline-asm-validate-tmpl.cpp
  test/Sema/scope-check.c

Index: test/Sema/scope-check.c
===
--- test/Sema/scope-check.c
+++ test/Sema/scope-check.c
@@ -232,3 +232,19 @@
 
 // rdar://9024687
 int test16(int [sizeof &]); // expected-error {{use of address-of-label extension outside of a function body}}
+
+//Asm goto:
+int test16(int n)
+{
+  // expected-error@+2 {{cannot jump from this asm goto statement to one of its possible targets}}
+  // expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
+  asm volatile goto("testl %0, %0; jne %l1;" :: "r"(n)::label_true, loop);
+  // expected-note@+2 {{jump bypasses initialization of variable length array}}
+  // expected-note@+1 {{possible target of asm goto statement}}
+  return ({int a[n];label_true: 2;});
+  // expected-note@+1 {{jump bypasses initialization of variable length array}}
+  int b[n];
+// expected-note@+1 {{possible target of asm goto statement}}
+loop:
+  return 0;
+}
Index: test/Sema/inline-asm-validate-tmpl.cpp
===
--- test/Sema/inline-asm-validate-tmpl.cpp
+++ test/Sema/inline-asm-validate-tmpl.cpp
@@ -23,3 +23,13 @@
 	asm("rol %1, %0" :"=r"(value): "I"(N + 1));
 }
 int	foo() { testc<2>(10); }
+
+// these should compile without error
+template  bool testd()
+{
+  __asm goto ("" : : : : lab);
+  return true;
+lab:
+  return false;
+}
+bool foox() { return testd<0> (); }
Index: test/Sema/asm.c
===
--- test/Sema/asm.c
+++ test/Sema/asm.c
@@ -295,3 +295,24 @@
   return r0 + r1;
 }
 
+void test18()
+{
+  // expected-error@+2 {{duplicate use of asm operand name "lab"}}
+  // expected-note@+1 {{asm operand name "lab" first referenced here}}
+  asm goto ("" : : : : lab, lab, lab2, lab);
+  // expected-error@+2 {{duplicate use of asm operand name "lab"}}
+  // expected-note@+1 {{asm operand name "lab" first referenced here}}
+  asm goto ("xorw %[lab], %[lab]; je %l[lab]" : : [lab] "i" (0) : : lab);
+lab:;
+lab2:;
+  int x,x1;
+  // expected-error@+2 {{duplicate use of asm operand name "lab"}}
+  // expected-note@+1 {{asm operand name "lab" first referenced here}}
+  asm ("" : [lab] "=r" (x),[lab] "+r" (x) : [lab1] "r" (x));
+  // expected-error@+2 {{duplicate use of asm operand name "lab"}}
+  // expected-note@+1 {{asm operand name "lab" first referenced here}}
+  asm ("" : [lab] "=r" (x1) : [lab] "r" (x));
+  // expected-error@+1 {{invalid operand number in inline asm string}}
+  asm ("jne %l0":::);
+  asm goto ("jne %l0"lab);
+}
Index: test/Sema/asm-goto.cpp
===
--- /dev/null
+++ test/Sema/asm-goto.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 %s -triple i386-pc-linux-gnu -verify -fsyntax-only
+
+struct NonTrivial {
+  ~NonTrivial();
+  int f(int);
+private:
+  int k;
+};
+void JumpDiagnostics(int n) {
+// expected-error@+1 {{cannot jump from this goto statement to its label}}
+  goto DirectJump;
+// expected-note@+1 {{jump bypasses variable with a non-trivial destructor}}
+  NonTrivial tnp1;
+
+DirectJump:
+// expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
+  asm goto("jmp %l0;" Later);
+// expected-note@+1 {{jump bypasses variable with a non-trivial destructor}}
+  NonTrivial tnp2;
+// expected-note@+1 {{possible target of asm goto statement}}
+Later:
+  return;
+}
+
+struct S { ~S(); };
+void foo(int a) {
+  if (a) {
+FOO:
+// expected-note@+2 {{jump exits scope of variable with non-trivial destructor}}
+// expected-note@+1 {{jump exits scope of variable with non-trivial destructor}}
+S s;
+void *p = &
+// expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
+  asm goto("jmp %l0;" BAR);
+// expected-error@+1 {{cannot jump from this indirect goto statement to one of its possible targets}}
+goto *p;
+p = &
+goto *p;
+return;
+  }

[PATCH] D58891: Modules: Add -Rmodule-import

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

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

Nice, note that a subset of this information can be achieved with 
`-Wauto-import`, but this is way more general and solid, since it will handles 
`@import`s and pragma imports too. LGTM with a minor nitpick, see comments.




Comment at: clang/test/Modules/Inputs/Rmodule-import/B.h:1
+// B
+#include "C.h"

Can you make one of the tests to use `@import` (or the pragma version) instead 
of #include? 



Comment at: clang/test/Modules/Rmodule-build.m:22-25
-// RUN: echo ' ' >> %t/C.h
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t 
-fsyntax-only %s -I %t \
-// RUN:-Reverything 2>&1 | FileCheck %s
-

dexonsmith wrote:
> @bruno this looked redundant to me with the check above; can you confirm?  
> (Now that we have `-Rmodule-import`, the FileCheck was failing...)
Yes, it should be fine.


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

https://reviews.llvm.org/D58891



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

[PATCH] D58924: Replace clang::FileData with llvm::vfs::Status

[Harlan Haskins via Phabricator via cfe-commits]

harlanhaskins created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
harlanhaskins edited the summary of this revision.
harlanhaskins added a reviewer: benlangmuir.
Herald added a subscriber: ormris.

`FileData` was only ever used as a container for the values in
`llvm::vfs::Status`, so they might as well be consolidated.

The `InPCH` member was also always set to false, and unused.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58924

Files:
  clang/include/clang/Basic/FileManager.h
  clang/include/clang/Basic/FileSystemStatCache.h
  clang/lib/Basic/FileManager.cpp
  clang/lib/Basic/FileSystemStatCache.cpp
  clang/lib/Frontend/TextDiagnostic.cpp
  clang/unittests/Basic/FileManagerTest.cpp

Index: clang/unittests/Basic/FileManagerTest.cpp
===
--- clang/unittests/Basic/FileManagerTest.cpp
+++ clang/unittests/Basic/FileManagerTest.cpp
@@ -26,7 +26,7 @@
 private:
   // Maps a file/directory path to its desired stat result.  Anything
   // not in this map is considered to not exist in the file system.
-  llvm::StringMap StatCalls;
+  llvm::StringMap StatCalls;
 
   void InjectFileOrDirectory(const char *Path, ino_t INode, bool IsFile) {
 #ifndef _WIN32
@@ -35,15 +35,14 @@
 Path = NormalizedPath.c_str();
 #endif
 
-FileData Data;
-Data.Name = Path;
-Data.Size = 0;
-Data.ModTime = 0;
-Data.UniqueID = llvm::sys::fs::UniqueID(1, INode);
-Data.IsDirectory = !IsFile;
-Data.IsNamedPipe = false;
-Data.InPCH = false;
-StatCalls[Path] = Data;
+auto fileType = IsFile ?
+  llvm::sys::fs::file_type::regular_file :
+  llvm::sys::fs::file_type::directory_file;
+llvm::vfs::Status Status(Path, llvm::sys::fs::UniqueID(1, INode),
+ /*MTime*/{}, /*User*/0, /*Group*/0,
+ /*Size*/0, fileType,
+ llvm::sys::fs::perms::all_all);
+StatCalls[Path] = Status;
   }
 
 public:
@@ -58,7 +57,7 @@
   }
 
   // Implement FileSystemStatCache::getStat().
-  LookupResult getStat(StringRef Path, FileData , bool isFile,
+  LookupResult getStat(StringRef Path, llvm::vfs::Status , bool isFile,
std::unique_ptr *F,
llvm::vfs::FileSystem ) override {
 #ifndef _WIN32
@@ -68,7 +67,7 @@
 #endif
 
 if (StatCalls.count(Path) != 0) {
-  Data = StatCalls[Path];
+  Status = StatCalls[Path];
   return CacheExists;
 }
 
Index: clang/lib/Frontend/TextDiagnostic.cpp
===
--- clang/lib/Frontend/TextDiagnostic.cpp
+++ clang/lib/Frontend/TextDiagnostic.cpp
@@ -792,8 +792,6 @@
   const FileEntry *FE = Loc.getFileEntry();
   if (FE && FE->isValid()) {
 emitFilename(FE->getName(), Loc.getManager());
-if (FE->isInPCH())
-  OS << " (in PCH)";
 OS << ": ";
   }
 }
Index: clang/lib/Basic/FileSystemStatCache.cpp
===
--- clang/lib/Basic/FileSystemStatCache.cpp
+++ clang/lib/Basic/FileSystemStatCache.cpp
@@ -21,18 +21,6 @@
 
 void FileSystemStatCache::anchor() {}
 
-static void copyStatusToFileData(const llvm::vfs::Status ,
- FileData ) {
-  Data.Name = Status.getName();
-  Data.Size = Status.getSize();
-  Data.ModTime = llvm::sys::toTimeT(Status.getLastModificationTime());
-  Data.UniqueID = Status.getUniqueID();
-  Data.IsDirectory = Status.isDirectory();
-  Data.IsNamedPipe = Status.getType() == llvm::sys::fs::file_type::fifo_file;
-  Data.InPCH = false;
-  Data.IsVFSMapped = Status.IsVFSMapped;
-}
-
 /// FileSystemStatCache::get - Get the 'stat' information for the specified
 /// path, using the cache to accelerate it if possible.  This returns true if
 /// the path does not exist or false if it exists.
@@ -42,7 +30,8 @@
 /// success for directories (not files).  On a successful file lookup, the
 /// implementation can optionally fill in FileDescriptor with a valid
 /// descriptor and the client guarantees that it will close it.
-bool FileSystemStatCache::get(StringRef Path, FileData , bool isFile,
+bool FileSystemStatCache::get(StringRef Path, llvm::vfs::Status ,
+  bool isFile,
   std::unique_ptr *F,
   FileSystemStatCache *Cache,
   llvm::vfs::FileSystem ) {
@@ -51,16 +40,16 @@
 
   // If we have a cache, use it to resolve the stat query.
   if (Cache)
-R = Cache->getStat(Path, Data, isFile, F, FS);
+R = Cache->getStat(Path, Status, isFile, F, FS);
   else if (isForDir || !F) {
 // If this is a directory or a file descriptor is not needed and we have
 // no cache, just go to the file system.
-llvm::ErrorOr Status = FS.status(Path);
-if (!Status) {
+llvm::ErrorOr StatusOrErr = FS.status(Path);
+if 

[PATCH] D58236: Make address space conversions a bit stricter.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

In D58236#1416765 , @Anastasia wrote:

> In D58236#1414069 , @efriedma wrote:
>
> > > I think trying to reject code that is doing something dangerous is a good 
> > > thing!
> >
> > Refusing to compile code which is suspicious, but not forbidden by the 
> > specification, will likely cause compatibility issues; there are legitimate 
> > reasons to use casts which look weird.
>
>
> The spec dioesn't allow these conversions either, it just simply doesn't 
> cover this corner case at all. I don't think we are changing anything in 
> terms of compatibility. If you have any examples of such casts that can be 
> legitimate I would like to understand them better. What I have seen so far 
> were the examples where `addrspacecast` was lost in IR for the memory 
> segments translation and therefore wrong memory areas were accessed.


The spec just says that the casts follow C rules... and C says you can cast a 
pointer to an object type to a pointer to another object type (subject to 
alignment restrictions).  By default, a pointer to a pointer isn't special.

In practice, unusual casts tend to show up in code building a datastructure 
using union-like constructs.  In plain C, for example, sometimes you have a 
pointer to a float, and sometimes you have a pointer to an int, determined 
dynamically.  I expect similar cases show up where a pointer points to memory 
which contains either a pointer in the global address-space, or a pointer in 
the local address-space, determined dynamically.  In some cases, it might be 
clearer to use void* in more places, but that's mostly style issue.

>> I'm not against adding some sort of address space suspicious cast warning to 
>> catch cases where we think the user meant to do something else.
> 
> I simply don't see how these conversions can be useful and some are 
> definitely indirectly forbidden (there is no precise wording however). There 
> are other ways to perform such conversions differently (by being more 
> explicit) where correct IR can be then generated with `addrspacecast`. I 
> don't think we are loosing anything in terms of functionality.

In some cases, the correct code may not involve an addrspacecast at all; the 
pointer could just have the "wrong" pointee type, and the code expects to 
explicitly fix it.  In that case, how do you fix it?  Write `(foo*)(void*)p` 
instead of `(foo*)p`?

>> But that's a separate issue, and it needs a proper cost-benefit analysis, 
>> including an analysis of the false-positive rate on existing code.
> 
> Do you have any suggestions how to do this in practice with such rare corner 
> case?

If the warning never triggers on any code you have access to, that would still 
be a useful datapoint.


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

https://reviews.llvm.org/D58236



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

[PATCH] D58922: [clang-format] broken after lambda with return type template with boolean literal

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

Thanks for fixing this!

The patch LGTM.

I am just wondering what else are we missing here. I guess arithmetic and 
logical operators are also allowed:

  template struct foo {};
  
  int main() { 
  auto lambda1 = []() -> foo { return foo{}; }; 
  auto lambda2 = []() -> foo<5+3> { return foo<5+3>{}; }; 
  }


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

https://reviews.llvm.org/D58922



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

[PATCH] D58317: [clang] Add install targets for API headers

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

@phosek This is using the clang-headers name now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58317



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

[PATCH] D58317: [clang] Add install targets for API headers

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai updated this revision to Diff 189199.
smeenai retitled this revision from "[clang] Add install targets for library 
headers" to "[clang] Add install targets for API headers".
smeenai edited the summary of this revision.
smeenai added a comment.

Update target name


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58317

Files:
  clang/CMakeLists.txt
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -156,7 +156,9 @@
 - In 8.0.0 and below, the install-clang-headers target would install clang's
   resource directory headers. This installation is now performed by the
   install-clang-resource-headers target. Users of the old install-clang-headers
-  target should switch to the new install-clang-resource-headers target.
+  target should switch to the new install-clang-resource-headers target. The
+  install-clang-headers target now installs clang's API headers (corresponding
+  to its libraries), which is consistent with the install-llvm-headers target.
 
 -  ...
 
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -388,6 +388,7 @@
 if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
   install(DIRECTORY include/clang include/clang-c
 DESTINATION include
+COMPONENT clang-headers
 FILES_MATCHING
 PATTERN "*.def"
 PATTERN "*.h"
@@ -397,12 +398,23 @@
 
   install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/include/clang
 DESTINATION include
+COMPONENT clang-headers
 FILES_MATCHING
 PATTERN "CMakeFiles" EXCLUDE
 PATTERN "*.inc"
 PATTERN "*.h"
 )
 
+  # Installing the headers needs to depend on generating any public
+  # tablegen'd headers.
+  add_custom_target(clang-headers DEPENDS clang-tablegen-targets)
+  set_target_properties(clang-headers PROPERTIES FOLDER "Misc")
+  if(NOT LLVM_ENABLE_IDE)
+add_llvm_install_targets(install-clang-headers
+ DEPENDS clang-headers
+ COMPONENT clang-headers)
+  endif()
+
   install(PROGRAMS utils/bash-autocomplete.sh
 DESTINATION share/clang
 )


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -156,7 +156,9 @@
 - In 8.0.0 and below, the install-clang-headers target would install clang's
   resource directory headers. This installation is now performed by the
   install-clang-resource-headers target. Users of the old install-clang-headers
-  target should switch to the new install-clang-resource-headers target.
+  target should switch to the new install-clang-resource-headers target. The
+  install-clang-headers target now installs clang's API headers (corresponding
+  to its libraries), which is consistent with the install-llvm-headers target.
 
 -  ...
 
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -388,6 +388,7 @@
 if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
   install(DIRECTORY include/clang include/clang-c
 DESTINATION include
+COMPONENT clang-headers
 FILES_MATCHING
 PATTERN "*.def"
 PATTERN "*.h"
@@ -397,12 +398,23 @@
 
   install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/include/clang
 DESTINATION include
+COMPONENT clang-headers
 FILES_MATCHING
 PATTERN "CMakeFiles" EXCLUDE
 PATTERN "*.inc"
 PATTERN "*.h"
 )
 
+  # Installing the headers needs to depend on generating any public
+  # tablegen'd headers.
+  add_custom_target(clang-headers DEPENDS clang-tablegen-targets)
+  set_target_properties(clang-headers PROPERTIES FOLDER "Misc")
+  if(NOT LLVM_ENABLE_IDE)
+add_llvm_install_targets(install-clang-headers
+ DEPENDS clang-headers
+ COMPONENT clang-headers)
+  endif()
+
   install(PROGRAMS utils/bash-autocomplete.sh
 DESTINATION share/clang
 )
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r355340 - [build] Rename clang-headers to clang-resource-headers

[Shoaib Meenai via cfe-commits]

Author: smeenai
Date: Mon Mar  4 13:19:53 2019
New Revision: 355340

URL: http://llvm.org/viewvc/llvm-project?rev=355340=rev
Log:
[build] Rename clang-headers to clang-resource-headers

Summary:
The current install-clang-headers target installs clang's resource
directory headers. This is different from the install-llvm-headers
target, which installs LLVM's API headers. We want to introduce the
corresponding target to clang, and the natural name for that new target
would be install-clang-headers. Rename the existing target to
install-clang-resource-headers to free up the install-clang-headers name
for the new target, following the discussion on cfe-dev [1].

I didn't find any bots on zorg referencing install-clang-headers. I'll
send out another PSA to cfe-dev to accompany this rename.

[1] http://lists.llvm.org/pipermail/cfe-dev/2019-February/061365.html

Reviewers: beanz, phosek, tstellar, rnk, dim, serge-sans-paille

Subscribers: mgorny, javed.absar, jdoerfert, #sanitizers, openmp-commits, 
lldb-commits, cfe-commits, llvm-commits

Tags: #clang, #sanitizers, #lldb, #openmp, #llvm

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

Modified:
cfe/trunk/cmake/caches/Apple-stage2.cmake
cfe/trunk/cmake/caches/BaremetalARM.cmake
cfe/trunk/cmake/caches/DistributionExample-stage2.cmake
cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
cfe/trunk/cmake/modules/AddClang.cmake
cfe/trunk/docs/LibTooling.rst
cfe/trunk/docs/ReleaseNotes.rst
cfe/trunk/examples/clang-interpreter/CMakeLists.txt
cfe/trunk/lib/Headers/CMakeLists.txt
cfe/trunk/test/CMakeLists.txt
cfe/trunk/tools/driver/CMakeLists.txt
cfe/trunk/tools/libclang/CMakeLists.txt

Modified: cfe/trunk/cmake/caches/Apple-stage2.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/caches/Apple-stage2.cmake?rev=355340=355339=355340=diff
==
--- cfe/trunk/cmake/caches/Apple-stage2.cmake (original)
+++ cfe/trunk/cmake/caches/Apple-stage2.cmake Mon Mar  4 13:19:53 2019
@@ -60,7 +60,7 @@ set(LLVM_DISTRIBUTION_COMPONENTS
   clang
   LTO
   clang-format
-  clang-headers
+  clang-resource-headers
   cxx-headers
   ${LLVM_TOOLCHAIN_TOOLS}
   CACHE STRING "")

Modified: cfe/trunk/cmake/caches/BaremetalARM.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/caches/BaremetalARM.cmake?rev=355340=355339=355340=diff
==
--- cfe/trunk/cmake/caches/BaremetalARM.cmake (original)
+++ cfe/trunk/cmake/caches/BaremetalARM.cmake Mon Mar  4 13:19:53 2019
@@ -41,7 +41,7 @@ set(LLVM_TOOLCHAIN_TOOLS
 set(LLVM_DISTRIBUTION_COMPONENTS
   clang
   lld
-  clang-headers
+  clang-resource-headers
   builtins-armv6m-none-eabi
   builtins-armv7m-none-eabi
   builtins-armv7em-none-eabi

Modified: cfe/trunk/cmake/caches/DistributionExample-stage2.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/caches/DistributionExample-stage2.cmake?rev=355340=355339=355340=diff
==
--- cfe/trunk/cmake/caches/DistributionExample-stage2.cmake (original)
+++ cfe/trunk/cmake/caches/DistributionExample-stage2.cmake Mon Mar  4 13:19:53 
2019
@@ -23,7 +23,7 @@ set(LLVM_DISTRIBUTION_COMPONENTS
   clang
   LTO
   clang-format
-  clang-headers
+  clang-resource-headers
   builtins
   runtimes
   ${LLVM_TOOLCHAIN_TOOLS}

Modified: cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/caches/Fuchsia-stage2.cmake?rev=355340=355339=355340=diff
==
--- cfe/trunk/cmake/caches/Fuchsia-stage2.cmake (original)
+++ cfe/trunk/cmake/caches/Fuchsia-stage2.cmake Mon Mar  4 13:19:53 2019
@@ -167,7 +167,7 @@ set(LLVM_DISTRIBUTION_COMPONENTS
   LTO
   clang-apply-replacements
   clang-format
-  clang-headers
+  clang-resource-headers
   clang-include-fixer
   clang-refactor
   clang-tidy

Modified: cfe/trunk/cmake/modules/AddClang.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/modules/AddClang.cmake?rev=355340=355339=355340=diff
==
--- cfe/trunk/cmake/modules/AddClang.cmake (original)
+++ cfe/trunk/cmake/modules/AddClang.cmake Mon Mar  4 13:19:53 2019
@@ -133,7 +133,7 @@ macro(add_clang_tool name)
   endif()
 
   add_clang_executable(${name} ${ARGN})
-  add_dependencies(${name} clang-headers)
+  add_dependencies(${name} clang-resource-headers)
 
   if (CLANG_BUILD_TOOLS)
 if(${name} IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR

Modified: cfe/trunk/docs/LibTooling.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibTooling.rst?rev=355340=355339=355340=diff
==
--- cfe/trunk/docs/LibTooling.rst (original)
+++ cfe/trunk/docs/LibTooling.rst Mon Mar  

[clang-tools-extra] r355340 - [build] Rename clang-headers to clang-resource-headers

[Shoaib Meenai via cfe-commits]

Author: smeenai
Date: Mon Mar  4 13:19:53 2019
New Revision: 355340

URL: http://llvm.org/viewvc/llvm-project?rev=355340=rev
Log:
[build] Rename clang-headers to clang-resource-headers

Summary:
The current install-clang-headers target installs clang's resource
directory headers. This is different from the install-llvm-headers
target, which installs LLVM's API headers. We want to introduce the
corresponding target to clang, and the natural name for that new target
would be install-clang-headers. Rename the existing target to
install-clang-resource-headers to free up the install-clang-headers name
for the new target, following the discussion on cfe-dev [1].

I didn't find any bots on zorg referencing install-clang-headers. I'll
send out another PSA to cfe-dev to accompany this rename.

[1] http://lists.llvm.org/pipermail/cfe-dev/2019-February/061365.html

Reviewers: beanz, phosek, tstellar, rnk, dim, serge-sans-paille

Subscribers: mgorny, javed.absar, jdoerfert, #sanitizers, openmp-commits, 
lldb-commits, cfe-commits, llvm-commits

Tags: #clang, #sanitizers, #lldb, #openmp, #llvm

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

Modified:
clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt
clang-tools-extra/trunk/test/CMakeLists.txt

Modified: clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt?rev=355340=355339=355340=diff
==
--- clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt Mon Mar  4 13:19:53 
2019
@@ -9,7 +9,7 @@ add_clang_tool(clang-tidy
   ClangTidyMain.cpp
   )
 add_dependencies(clang-tidy
-  clang-headers
+  clang-resource-headers
   )
 target_link_libraries(clang-tidy
   PRIVATE

Modified: clang-tools-extra/trunk/test/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/CMakeLists.txt?rev=355340=355339=355340=diff
==
--- clang-tools-extra/trunk/test/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/test/CMakeLists.txt Mon Mar  4 13:19:53 2019
@@ -59,7 +59,7 @@ set(CLANG_TOOLS_TEST_DEPS
   # For the clang-tidy libclang integration test.
   c-index-test
   # clang-tidy tests require it.
-  clang-headers
+  clang-resource-headers
 
   clang-tidy
 )


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

[PATCH] D57965: Clean up ObjCPropertyDecl printing

[David Goldman via Phabricator via cfe-commits]

dgoldman added a comment.

In D57965#1417361 , @jkorous wrote:

> Hi David,
>  I am just wondering - while you're here would you mind adding couple more 
> tests? It would be great to have a test for each attribute.
>
> Also, what do you think about Ben's suggestion? I think it would be nice to 
> be consistent with the style Objective-C documentation uses:
>  
> https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/ProgrammingWithObjectiveC/EncapsulatingData/EncapsulatingData.html


Sure, I can add a few more tests.

Hmm, I've seen it more often the other way around; to be honest I think the 
docs that you've linked to are outdated. See 
https://developer.apple.com/documentation/uikit/uiapplication/1622975-sharedapplication?language=objc
 for an example of what I've seen.

I do think that it would be nice to have `X *prop` instead of `X * prop` 
though, let me try adding that.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57965



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

[PATCH] D58807: [CodeGen] COMDAT-fold block descriptors

[Dustin L. Howett via Phabricator via cfe-commits]

DHowett-MSFT updated this revision to Diff 189193.
DHowett-MSFT added a comment.

Fixed corrupted patch. I know that the -str test case is not the _ideal_ 
location for the comdat test, but it is the only test whose invariants failed 
(the IR for the descriptor definition now contains `comdat, align 4`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58807

Files:
  clang/lib/CodeGen/CGBlocks.cpp
  clang/test/CodeGenObjC/block-desc-str.m


Index: clang/test/CodeGenObjC/block-desc-str.m
===
--- clang/test/CodeGenObjC/block-desc-str.m
+++ clang/test/CodeGenObjC/block-desc-str.m
@@ -1,9 +1,13 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -emit-llvm 
-fobjc-runtime=gnustep-1.7 -fblocks -o - %s | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-llvm -fobjc-runtime=gcc 
-fblocks -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -emit-llvm 
-fobjc-runtime=gnustep-1.7 -fblocks -o - %s | FileCheck 
--check-prefix=CHECK-COMDAT %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-llvm -fobjc-runtime=gcc 
-fblocks -o - %s | FileCheck --check-prefix=CHECK-COMDAT %s
 // RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm -fblocks -o - %s | 
FileCheck %s
 
 // Test that descriptor symbol names don't include '@'.
 
+// CHECK-COMDAT: $"__block_descriptor_40_8_32o_e5_v8\01?0l" = comdat any
+// CHECK-COMDAT: @[[STR:.*]] = private unnamed_addr constant [6 x i8] 
c"v8@?0\00"
+// CHECK-COMDAT: @"__block_descriptor_40_8_32o_e5_v8\01?0l" = linkonce_odr 
hidden unnamed_addr constant { i64, i64, i8*, i8*, i8*, {{.*}} } { i64 0, i64 
40, i8* bitcast ({{.*}} to i8*), i8* bitcast ({{.*}} to i8*), i8* getelementptr 
inbounds ([6 x i8], [6 x i8]* @[[STR]], i32 0, i32 0), {{.*}} }, comdat, align 8
+
 // CHECK: @[[STR:.*]] = private unnamed_addr constant [6 x i8] c"v8@?0\00"
 // CHECK: @"__block_descriptor_40_8_32o_e5_v8\01?0l" = linkonce_odr hidden 
unnamed_addr constant { i64, i64, i8*, i8*, i8*, {{.*}} } { i64 0, i64 40, i8* 
bitcast ({{.*}} to i8*), i8* bitcast ({{.*}} to i8*), i8* getelementptr 
inbounds ([6 x i8], [6 x i8]* @[[STR]], i32 0, i32 0), {{.*}} }, align 8
 
Index: clang/lib/CodeGen/CGBlocks.cpp
===
--- clang/lib/CodeGen/CGBlocks.cpp
+++ clang/lib/CodeGen/CGBlocks.cpp
@@ -274,6 +274,8 @@
  /*constant*/ true, linkage, AddrSpace);
 
   if (linkage == llvm::GlobalValue::LinkOnceODRLinkage) {
+if (CGM.supportsCOMDAT())
+  global->setComdat(CGM.getModule().getOrInsertComdat(descName));
 global->setVisibility(llvm::GlobalValue::HiddenVisibility);
 global->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
   }


Index: clang/test/CodeGenObjC/block-desc-str.m
===
--- clang/test/CodeGenObjC/block-desc-str.m
+++ clang/test/CodeGenObjC/block-desc-str.m
@@ -1,9 +1,13 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -emit-llvm -fobjc-runtime=gnustep-1.7 -fblocks -o - %s | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-llvm -fobjc-runtime=gcc -fblocks -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -emit-llvm -fobjc-runtime=gnustep-1.7 -fblocks -o - %s | FileCheck --check-prefix=CHECK-COMDAT %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-llvm -fobjc-runtime=gcc -fblocks -o - %s | FileCheck --check-prefix=CHECK-COMDAT %s
 // RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm -fblocks -o - %s | FileCheck %s
 
 // Test that descriptor symbol names don't include '@'.
 
+// CHECK-COMDAT: $"__block_descriptor_40_8_32o_e5_v8\01?0l" = comdat any
+// CHECK-COMDAT: @[[STR:.*]] = private unnamed_addr constant [6 x i8] c"v8@?0\00"
+// CHECK-COMDAT: @"__block_descriptor_40_8_32o_e5_v8\01?0l" = linkonce_odr hidden unnamed_addr constant { i64, i64, i8*, i8*, i8*, {{.*}} } { i64 0, i64 40, i8* bitcast ({{.*}} to i8*), i8* bitcast ({{.*}} to i8*), i8* getelementptr inbounds ([6 x i8], [6 x i8]* @[[STR]], i32 0, i32 0), {{.*}} }, comdat, align 8
+
 // CHECK: @[[STR:.*]] = private unnamed_addr constant [6 x i8] c"v8@?0\00"
 // CHECK: @"__block_descriptor_40_8_32o_e5_v8\01?0l" = linkonce_odr hidden unnamed_addr constant { i64, i64, i8*, i8*, i8*, {{.*}} } { i64 0, i64 40, i8* bitcast ({{.*}} to i8*), i8* bitcast ({{.*}} to i8*), i8* getelementptr inbounds ([6 x i8], [6 x i8]* @[[STR]], i32 0, i32 0), {{.*}} }, align 8
 
Index: clang/lib/CodeGen/CGBlocks.cpp
===
--- clang/lib/CodeGen/CGBlocks.cpp
+++ clang/lib/CodeGen/CGBlocks.cpp
@@ -274,6 +274,8 @@
  /*constant*/ true, linkage, AddrSpace);
 
   if (linkage == llvm::GlobalValue::LinkOnceODRLinkage) {
+if (CGM.supportsCOMDAT())
+  

[PATCH] D58917: [HIP] Do not unbundle object files for -fno-gpu-rdc

[Artem Belevich via Phabricator via cfe-commits]

tra added a subscriber: ABataev.
tra added a comment.

The change looks OK as far as regular CUDA is concerned.
That said, I'm not quite familiar with the use of bundling/unbundling actions 
and you should probably get someone who uses/depends on them to take a look. I 
think OpenMP uses them. Perhaps @ABataev would be the right person.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58917



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

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

needs another rebase


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

https://reviews.llvm.org/D56571



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

[PATCH] D57965: Clean up ObjCPropertyDecl printing

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

Hi David,
I am just wondering - while you're here would you mind adding couple more 
tests? It would be great to have a test for each attribute.

Also, what do you think about Ben's suggestion? I think it would be nice to be 
consistent with the style Objective-C documentation uses:
https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/ProgrammingWithObjectiveC/EncapsulatingData/EncapsulatingData.html


Repository:
  rC Clang

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

https://reviews.llvm.org/D57965



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

[PATCH] D58154: Add support for -fpermissive.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I'm not super keen on supporting -fpermissive --  what is the impetus for 
supporting this? Is it just for GCC compatibility?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58154



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

[PATCH] D58154: Add support for -fpermissive.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Hm -- in GCC, -fpermissive has nothing at all to do with 
-pedantic/-pedantic-errors, but I suppose it should be fine to do this way.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58154



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

[PATCH] D58922: [clang-format] broken after lambda with return type template with boolean literal

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: klimek, djasper, JonasToth, alexfh, krasimir.
MyDeveloperDay added a project: clang-tools-extra.

A Lamdba with a return type template with a boolean literal (true,false) 
behaves differently to an integer literal

https://bugs.llvm.org/show_bug.cgi?id=40910


https://reviews.llvm.org/D58922

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11846,6 +11846,21 @@
   verifyGoogleFormat("auto a = [, c](D* d) -> D& {};");
   verifyGoogleFormat("auto a = [, c](D* d) -> const D* {};");
   verifyFormat("[a, a]() -> a<1> {};");
+  verifyFormat("[]() -> a<1> {};");
+  verifyFormat("[]() -> a<1> { ; };");
+  verifyFormat("[]() -> a<1> { ; }();");
+  verifyFormat("[a, a]() -> a {};");
+  verifyFormat("[]() -> a {};");
+  verifyFormat("[]() -> a { ; };");
+  verifyFormat("[]() -> a { ; }();");
+  verifyFormat("[a, a]() -> a {};");
+  verifyFormat("[]() -> a {};");
+  verifyFormat("[]() -> a { ; };");
+  verifyFormat("[]() -> a { ; }();");
+  verifyFormat("auto foo{[]() -> foo { ; }};");
+  verifyFormat("namespace bar {\n"
+   "auto foo{[]() -> foo { ; }};\n"
+   "} // namespace bar");
   verifyFormat("auto  = [](int i, // break for some reason\n"
"   int j) -> int {\n"
"  return (i * 
j);\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1423,6 +1423,8 @@
 case tok::coloncolon:
 case tok::kw_mutable:
 case tok::kw_noexcept:
+case tok::kw_true:
+case tok::kw_false:
   nextToken();
   break;
 case tok::arrow:


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11846,6 +11846,21 @@
   verifyGoogleFormat("auto a = [, c](D* d) -> D& {};");
   verifyGoogleFormat("auto a = [, c](D* d) -> const D* {};");
   verifyFormat("[a, a]() -> a<1> {};");
+  verifyFormat("[]() -> a<1> {};");
+  verifyFormat("[]() -> a<1> { ; };");
+  verifyFormat("[]() -> a<1> { ; }();");
+  verifyFormat("[a, a]() -> a {};");
+  verifyFormat("[]() -> a {};");
+  verifyFormat("[]() -> a { ; };");
+  verifyFormat("[]() -> a { ; }();");
+  verifyFormat("[a, a]() -> a {};");
+  verifyFormat("[]() -> a {};");
+  verifyFormat("[]() -> a { ; };");
+  verifyFormat("[]() -> a { ; }();");
+  verifyFormat("auto foo{[]() -> foo { ; }};");
+  verifyFormat("namespace bar {\n"
+   "auto foo{[]() -> foo { ; }};\n"
+   "} // namespace bar");
   verifyFormat("auto  = [](int i, // break for some reason\n"
"   int j) -> int {\n"
"  return (i * j);\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1423,6 +1423,8 @@
 case tok::coloncolon:
 case tok::kw_mutable:
 case tok::kw_noexcept:
+case tok::kw_true:
+case tok::kw_false:
   nextToken();
   break;
 case tok::arrow:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58920: [Modules][PR39287] Consolidate multiple std's

[Brian Gesiak via Phabricator via cfe-commits]

modocache created this revision.
modocache added reviewers: rsmith, andrewjcg.
Herald added a project: clang.

This patch addresses https://bugs.llvm.org/show_bug.cgi?id=39287.

Clang creates a 'std' namespace in one of two ways: either it parses a
`namespace std { ... }` declaration, or it creates one implicitly.

One case in which Clang creates an implicit 'std' namespace is when it
encounters a virtual destructor in C++17, which results in the implicit
definition of operator new and delete overloads that take a
`std::align_val_t` argument (this behavior was added in rL282800 
).

Unfortunately, when using Clang modules, further definitions of the
`std` namespace may or may not reconcile with previous definitions. For
example, consider the two test cases in this patch:

- `mod-virtual-destructor-bug` defines `std::type_info` within the main 
translation unit. When it does so, there are two 'std' namespaces available: 
the implicitly defined one that is returned by `Sema::getStdNamespace`, and the 
explicit one defined in the `a.h` module. The `std::type_info` definition ends 
up being attached to the module's `std` namespace, and so the subsequent lookup 
of `getStdNamespace`'s `type_info` member fails. To fix this case, this patch 
adds logic to ensure `Sema::ActOnNamespaceDef` finds the `getStdNamespace` 
namespace from the current translation unit.
- `mod-virtual-destructor-bug-two` defines `std::type_info` within a module. 
Later, the lookup of `std::type_info` finds the implicitly created 
`getStdNamespace`, which only defines `std::align_val_t`, not `std::type_info`. 
To fix this case, this patch adds logic that ensures external AST sources are 
loaded when looking up `std::type_info`.


Repository:
  rC Clang

https://reviews.llvm.org/D58920

Files:
  lib/Sema/SemaDeclCXX.cpp
  lib/Serialization/ASTReader.cpp
  test/Modules/Inputs/mod-virtual-destructor-bug-two/a.h
  test/Modules/Inputs/mod-virtual-destructor-bug-two/module.modulemap
  test/Modules/Inputs/mod-virtual-destructor-bug/a.h
  test/Modules/Inputs/mod-virtual-destructor-bug/module.modulemap
  test/Modules/mod-virtual-destructor-bug-two.cpp
  test/Modules/mod-virtual-destructor-bug.cpp

Index: test/Modules/mod-virtual-destructor-bug.cpp
===
--- /dev/null
+++ test/Modules/mod-virtual-destructor-bug.cpp
@@ -0,0 +1,14 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -std=c++17 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/mod-virtual-destructor-bug %s -verify
+
+class A {
+  virtual ~A() {}
+};
+
+#include "a.h"
+
+namespace std { class type_info; }
+
+void foo() {
+  typeid(foo); // expected-warning {{expression result unused}}
+}
Index: test/Modules/mod-virtual-destructor-bug-two.cpp
===
--- /dev/null
+++ test/Modules/mod-virtual-destructor-bug-two.cpp
@@ -0,0 +1,12 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -std=c++17 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/mod-virtual-destructor-bug-two %s -verify
+
+class A {
+  virtual ~A() {}
+};
+
+#include "a.h"
+
+void foo() {
+  typeid(foo); // expected-warning {{expression result unused}}
+}
Index: test/Modules/Inputs/mod-virtual-destructor-bug/module.modulemap
===
--- /dev/null
+++ test/Modules/Inputs/mod-virtual-destructor-bug/module.modulemap
@@ -0,0 +1,3 @@
+module "a.h" {
+  header "a.h"
+}
Index: test/Modules/Inputs/mod-virtual-destructor-bug/a.h
===
--- /dev/null
+++ test/Modules/Inputs/mod-virtual-destructor-bug/a.h
@@ -0,0 +1 @@
+namespace std {}
Index: test/Modules/Inputs/mod-virtual-destructor-bug-two/module.modulemap
===
--- /dev/null
+++ test/Modules/Inputs/mod-virtual-destructor-bug-two/module.modulemap
@@ -0,0 +1,3 @@
+module "a.h" {
+  header "a.h"
+}
Index: test/Modules/Inputs/mod-virtual-destructor-bug-two/a.h
===
--- /dev/null
+++ test/Modules/Inputs/mod-virtual-destructor-bug-two/a.h
@@ -0,0 +1 @@
+namespace std { class type_info; }
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -7533,8 +7533,15 @@
 return false;
 
   auto It = Lookups.find(DC);
-  if (It == Lookups.end())
-return false;
+  if (It == Lookups.end()) {
+if (getContext().getLangOpts().Modules && DC->isStdNamespace()) {
+  for (auto StdIt = Lookups.begin(); StdIt != Lookups.end(); StdIt++)
+if (StdIt->first->isStdNamespace())
+  It = StdIt;
+}
+if (It == Lookups.end())
+  return false;
+  }
 
   Deserializing LookupResults(this);
 
Index: lib/Sema/SemaDeclCXX.cpp

[PATCH] D58921: [CMake] Tell libc++ that we're using compiler-rt on Apple platforms

[Louis Dionne via Phabricator via cfe-commits]

ldionne created this revision.
ldionne added reviewers: beanz, arphaman.
Herald added subscribers: cfe-commits, jdoerfert, dexonsmith, jkorous, mgorny, 
dberris.
Herald added a reviewer: EricWF.
Herald added a project: clang.

Repository:
  rC Clang

https://reviews.llvm.org/D58921

Files:
  clang/cmake/caches/Apple-stage2.cmake


Index: clang/cmake/caches/Apple-stage2.cmake
===
--- clang/cmake/caches/Apple-stage2.cmake
+++ clang/cmake/caches/Apple-stage2.cmake
@@ -38,6 +38,7 @@
 set(LIBCXX_INSTALL_LIBRARY OFF CACHE BOOL "")
 set(LIBCXX_INSTALL_HEADERS ON CACHE BOOL "")
 set(LIBCXX_INCLUDE_TESTS OFF CACHE BOOL "")
+set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
 set(LLVM_LTO_VERSION_OFFSET 3000 CACHE STRING "")
 
 # Generating Xcode toolchains is useful for developers wanting to build and use


Index: clang/cmake/caches/Apple-stage2.cmake
===
--- clang/cmake/caches/Apple-stage2.cmake
+++ clang/cmake/caches/Apple-stage2.cmake
@@ -38,6 +38,7 @@
 set(LIBCXX_INSTALL_LIBRARY OFF CACHE BOOL "")
 set(LIBCXX_INSTALL_HEADERS ON CACHE BOOL "")
 set(LIBCXX_INCLUDE_TESTS OFF CACHE BOOL "")
+set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
 set(LLVM_LTO_VERSION_OFFSET 3000 CACHE STRING "")
 
 # Generating Xcode toolchains is useful for developers wanting to build and use
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58751: Order File Instrumentation: add clang support for -forder-file-instrumentation

[Manman Ren via Phabricator via cfe-commits]

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



Comment at: include/clang/Driver/Options.td:774
+Group, Flags<[CoreOption]>,
+HelpText<"Generate instrumented code to collect order file into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
 

modocache wrote:
> Would it make sense to add documentation to the Clang User's Manual for this 
> option? Some other profiling options are mentioned there, but not all.
Will do this in a followup!


Repository:
  rC Clang

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

https://reviews.llvm.org/D58751



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

[PATCH] D58743: Handle built-in when importing SourceLocation and FileID

[Shafik Yaghmour via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL355332: [ASTImporter] Handle built-in when importing 
SourceLocation and FileID (authored by shafik, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58743?vs=188753=189185#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58743

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


Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -8229,9 +8229,10 @@
 return {};
 
   SourceManager  = FromContext.getSourceManager();
+  bool IsBuiltin = FromSM.isWrittenInBuiltinFile(FromLoc);
 
   std::pair Decomposed = FromSM.getDecomposedLoc(FromLoc);
-  FileID ToFileID = Import(Decomposed.first);
+  FileID ToFileID = Import(Decomposed.first, IsBuiltin);
   if (ToFileID.isInvalid())
 return {};
   SourceManager  = ToContext.getSourceManager();
@@ -8246,13 +8247,13 @@
   return SourceRange(Import(FromRange.getBegin()), Import(FromRange.getEnd()));
 }
 
-Expected ASTImporter::Import_New(FileID FromID) {
-  FileID ToID = Import(FromID);
+Expected ASTImporter::Import_New(FileID FromID, bool IsBuiltin) {
+  FileID ToID = Import(FromID, IsBuiltin);
   if (ToID.isInvalid() && FromID.isValid())
 return llvm::make_error();
   return ToID;
 }
-FileID ASTImporter::Import(FileID FromID) {
+FileID ASTImporter::Import(FileID FromID, bool IsBuiltin) {
   llvm::DenseMap::iterator Pos = ImportedFileIDs.find(FromID);
   if (Pos != ImportedFileIDs.end())
 return Pos->second;
@@ -8278,25 +8279,29 @@
 }
 ToID = ToSM.getFileID(MLoc);
   } else {
-// Include location of this file.
-SourceLocation ToIncludeLoc = Import(FromSLoc.getFile().getIncludeLoc());
-
 const SrcMgr::ContentCache *Cache = FromSLoc.getFile().getContentCache();
-if (Cache->OrigEntry && Cache->OrigEntry->getDir()) {
-  // FIXME: We probably want to use getVirtualFile(), so we don't hit the
-  // disk again
-  // FIXME: We definitely want to re-use the existing MemoryBuffer, rather
-  // than mmap the files several times.
-  const FileEntry *Entry =
-  ToFileManager.getFile(Cache->OrigEntry->getName());
-  // FIXME: The filename may be a virtual name that does probably not
-  // point to a valid file and we get no Entry here. In this case try with
-  // the memory buffer below.
-  if (Entry)
-ToID = ToSM.createFileID(Entry, ToIncludeLoc,
- FromSLoc.getFile().getFileCharacteristic());
+
+if (!IsBuiltin) {
+  // Include location of this file.
+  SourceLocation ToIncludeLoc = Import(FromSLoc.getFile().getIncludeLoc());
+
+  if (Cache->OrigEntry && Cache->OrigEntry->getDir()) {
+// FIXME: We probably want to use getVirtualFile(), so we don't hit the
+// disk again
+// FIXME: We definitely want to re-use the existing MemoryBuffer, 
rather
+// than mmap the files several times.
+const FileEntry *Entry =
+ToFileManager.getFile(Cache->OrigEntry->getName());
+// FIXME: The filename may be a virtual name that does probably not
+// point to a valid file and we get no Entry here. In this case try 
with
+// the memory buffer below.
+if (Entry)
+  ToID = ToSM.createFileID(Entry, ToIncludeLoc,
+   FromSLoc.getFile().getFileCharacteristic());
+  }
 }
-if (ToID.isInvalid()) {
+
+if (ToID.isInvalid() || IsBuiltin) {
   // FIXME: We want to re-use the existing MemoryBuffer!
   bool Invalid = true;
   const llvm::MemoryBuffer *FromBuf = Cache->getBuffer(
Index: cfe/trunk/include/clang/AST/ASTImporter.h
===
--- cfe/trunk/include/clang/AST/ASTImporter.h
+++ cfe/trunk/include/clang/AST/ASTImporter.h
@@ -337,9 +337,9 @@
 ///
 /// \returns The equivalent file ID in the source manager of the "to"
 /// context, or the import error.
-llvm::Expected Import_New(FileID);
+llvm::Expected Import_New(FileID, bool IsBuiltin = false);
 // FIXME: Remove this version.
-FileID Import(FileID);
+FileID Import(FileID, bool IsBuiltin = false);
 
 /// Import the given C++ constructor initializer from the "from"
 /// context into the "to" context.


Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -8229,9 +8229,10 @@
 return {};
 
   SourceManager  = FromContext.getSourceManager();
+  bool IsBuiltin = FromSM.isWrittenInBuiltinFile(FromLoc);
 
   std::pair Decomposed = 

r355332 - [ASTImporter] Handle built-in when importing SourceLocation and FileID

[Shafik Yaghmour via cfe-commits]

Author: shafik
Date: Mon Mar  4 12:25:54 2019
New Revision: 355332

URL: http://llvm.org/viewvc/llvm-project?rev=355332=rev
Log:
[ASTImporter] Handle built-in when importing SourceLocation and FileID

Summary:
Currently when we see a built-in we try and import the include location. 
Instead what we do now is find the buffer like we do for the invalid case and 
copy that over to the to context.

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

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

Modified: cfe/trunk/include/clang/AST/ASTImporter.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTImporter.h?rev=355332=355331=355332=diff
==
--- cfe/trunk/include/clang/AST/ASTImporter.h (original)
+++ cfe/trunk/include/clang/AST/ASTImporter.h Mon Mar  4 12:25:54 2019
@@ -337,9 +337,9 @@ class TypeSourceInfo;
 ///
 /// \returns The equivalent file ID in the source manager of the "to"
 /// context, or the import error.
-llvm::Expected Import_New(FileID);
+llvm::Expected Import_New(FileID, bool IsBuiltin = false);
 // FIXME: Remove this version.
-FileID Import(FileID);
+FileID Import(FileID, bool IsBuiltin = false);
 
 /// Import the given C++ constructor initializer from the "from"
 /// context into the "to" context.

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=355332=355331=355332=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Mon Mar  4 12:25:54 2019
@@ -8229,9 +8229,10 @@ SourceLocation ASTImporter::Import(Sourc
 return {};
 
   SourceManager  = FromContext.getSourceManager();
+  bool IsBuiltin = FromSM.isWrittenInBuiltinFile(FromLoc);
 
   std::pair Decomposed = FromSM.getDecomposedLoc(FromLoc);
-  FileID ToFileID = Import(Decomposed.first);
+  FileID ToFileID = Import(Decomposed.first, IsBuiltin);
   if (ToFileID.isInvalid())
 return {};
   SourceManager  = ToContext.getSourceManager();
@@ -8246,13 +8247,13 @@ SourceRange ASTImporter::Import(SourceRa
   return SourceRange(Import(FromRange.getBegin()), Import(FromRange.getEnd()));
 }
 
-Expected ASTImporter::Import_New(FileID FromID) {
-  FileID ToID = Import(FromID);
+Expected ASTImporter::Import_New(FileID FromID, bool IsBuiltin) {
+  FileID ToID = Import(FromID, IsBuiltin);
   if (ToID.isInvalid() && FromID.isValid())
 return llvm::make_error();
   return ToID;
 }
-FileID ASTImporter::Import(FileID FromID) {
+FileID ASTImporter::Import(FileID FromID, bool IsBuiltin) {
   llvm::DenseMap::iterator Pos = ImportedFileIDs.find(FromID);
   if (Pos != ImportedFileIDs.end())
 return Pos->second;
@@ -8278,25 +8279,29 @@ FileID ASTImporter::Import(FileID FromID
 }
 ToID = ToSM.getFileID(MLoc);
   } else {
-// Include location of this file.
-SourceLocation ToIncludeLoc = Import(FromSLoc.getFile().getIncludeLoc());
-
 const SrcMgr::ContentCache *Cache = FromSLoc.getFile().getContentCache();
-if (Cache->OrigEntry && Cache->OrigEntry->getDir()) {
-  // FIXME: We probably want to use getVirtualFile(), so we don't hit the
-  // disk again
-  // FIXME: We definitely want to re-use the existing MemoryBuffer, rather
-  // than mmap the files several times.
-  const FileEntry *Entry =
-  ToFileManager.getFile(Cache->OrigEntry->getName());
-  // FIXME: The filename may be a virtual name that does probably not
-  // point to a valid file and we get no Entry here. In this case try with
-  // the memory buffer below.
-  if (Entry)
-ToID = ToSM.createFileID(Entry, ToIncludeLoc,
- FromSLoc.getFile().getFileCharacteristic());
+
+if (!IsBuiltin) {
+  // Include location of this file.
+  SourceLocation ToIncludeLoc = Import(FromSLoc.getFile().getIncludeLoc());
+
+  if (Cache->OrigEntry && Cache->OrigEntry->getDir()) {
+// FIXME: We probably want to use getVirtualFile(), so we don't hit the
+// disk again
+// FIXME: We definitely want to re-use the existing MemoryBuffer, 
rather
+// than mmap the files several times.
+const FileEntry *Entry =
+ToFileManager.getFile(Cache->OrigEntry->getName());
+// FIXME: The filename may be a virtual name that does probably not
+// point to a valid file and we get no Entry here. In this case try 
with
+// the memory buffer below.
+if (Entry)
+  ToID = ToSM.createFileID(Entry, ToIncludeLoc,
+   FromSLoc.getFile().getFileCharacteristic());
+  }
 }
-if (ToID.isInvalid()) {
+
+if (ToID.isInvalid() || IsBuiltin) {
   // FIXME: We want to re-use the existing MemoryBuffer!
   bool Invalid 

[PATCH] D58885: Variable auto-init: split out small arrays

[JF Bastien via Phabricator via cfe-commits]

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

Comparing clang stage2 in release mode, with an without this change, we see a 
408 byte size difference, which is ~nothing. Here's details, nothing surprising:

  $ /s/bloaty/bloaty -d sections /s/llvm1/llvm/stage2/bin/clang-9  -- 
/s/llvm2/llvm/stage2/bin/clang-9
   VM SIZE FILE SIZE
   --   --
 +44% +1.84Ki [__TEXT]  +1.84Ki   +45%
 +65%+408 [__LINKEDIT]0  [ = ]
-0.0%  -8 Table of Non-instructions  -8  -0.0%
-0.0% -48 Symbol Table  -48  -0.0%
-0.0%-120 Export Info  -120  -0.0%
-0.0%-232 String Table -232  -0.0%
-0.0%-832 __TEXT,__text-832  -0.0%
-0.0% -1.03Ki __TEXT,__cstring  -1.03Ki  -0.0%
[ = ]   0 TOTAL-408  -0.0%
  
  $ /s/bloaty/bloaty -d symbols /s/llvm1/llvm/stage2/bin/clang-9  -- 
/s/llvm2/llvm/stage2/bin/clang-9
   VM SIZE  
FILE SIZE
   --
--
 +45% +1.84Ki [__TEXT]   
+1.84Ki   +45%
+0.0%+280 [__LINKEDIT]  
-128  -0.0%
+0.4% +96 clang::ento::check::ASTDecl<>::_checkDecl<>() 
 +96  +0.4%
+2.9% +48 emitStoresForConstant()   
 +48  +2.9%
-0.9% -16 clang::Sema::DeclareGlobalAllocationFunction()
 -16  -0.9%
-0.0% -16 clang::ento::check::PostStmt<>::_checkStmt<>()
 -16  -0.0%
-8.3% -78 clang::AnalyzerOptions::getCheckerStringOption()  
 -78  -8.3%
   -12.5% -80 clang::ento::registerPaddingChecker() 
 -80 -12.5%
-6.3% -96 clang::ASTContext::GetBuiltinType()   
 -96  -6.3%
   -37.8%-205 clang::AnalyzerOptions::getCheckerIntegerOption() 
-205 -37.8%
   -49.5%-269 clang::AnalyzerOptions::getCheckerBooleanOption() 
-269 -49.5%
-0.0%-480 llvm::APInt::toString()::Digits   
-480  -0.0%
   -67.4%-496 clang::driver::toolchains::NetBSD::addLibCxxIncludePaths()
-496 -67.4%
-0.0%-576 [__TEXT,__cstring]
-576  -0.0%
[ = ]   0 TOTAL 
-408  -0.0%

For codebases with more small arrays this likely plays better, but at least we 
know it's not a regression.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58885



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

[PATCH] D54176: [PGO] clang part of change for context-sensitive PGO.

[Rong Xu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC355331: [PGO] Clang part of change for context-sensitive PGO 
(part1) (authored by xur, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D54176?vs=189167=189184#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D54176

Files:
  docs/UsersManual.rst
  include/clang/Basic/CodeGenOptions.h
  include/clang/Driver/Options.td
  lib/CodeGen/BackendUtil.cpp
  lib/Driver/ToolChain.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/CommonArgs.cpp
  lib/Frontend/CompilerInvocation.cpp

Index: docs/UsersManual.rst
===
--- docs/UsersManual.rst
+++ docs/UsersManual.rst
@@ -1745,7 +1745,8 @@
 ``-fprofile-use``. Although these flags are semantically equivalent to
 their GCC counterparts, they *do not* handle GCC-compatible profiles.
 They are only meant to implement GCC's semantics with respect to
-profile creation and use.
+profile creation and use. Flag ``-fcs-profile-generate`` also instruments
+programs using the same instrumentation method as ``-fprofile-generate``.
 
 .. option:: -fprofile-generate[=]
 
@@ -1778,6 +1779,45 @@
  ``LLVM_PROFILE_FILE`` can still be used to override
  the directory and filename for the profile file at runtime.
 
+.. option:: -fcs-profile-generate[=]
+
+  The ``-fcs-profile-generate`` and ``-fcs-profile-generate=`` flags will use
+  the same instrumentation method, and generate the same profile as in the
+  ``-fprofile-generate`` and ``-fprofile-generate=`` flags. The difference is
+  that the instrumentation is performed after inlining so that the resulted
+  profile has a better context sensitive information. They cannot be used
+  together with ``-fprofile-generate`` and ``-fprofile-generate=`` flags.
+  They are typically used in conjunction with ``-fprofile-use`` flag.
+  The profile generated by ``-fcs-profile-generate`` and ``-fprofile-generate``
+  can be merged by llvm-profdata. A use example:
+
+  .. code-block:: console
+
+$ clang++ -O2 -fprofile-generate=yyy/zzz code.cc -o code
+$ ./code
+$ llvm-profdata merge -output=code.profdata yyy/zzz/
+
+  The first few steps are the same as that in ``-fprofile-generate``
+  compilation. Then perform a second round of instrumentation.
+
+  .. code-block:: console
+
+$ clang++ -O2 -fprofile-use=code.profdata -fcs-profile-generate=sss/ttt \
+  -o cs_code
+$ ./cs_code
+$ llvm-profdata merge -output=cs_code.profdata sss/ttt code.profdata
+
+  The resulted ``cs_code.prodata`` combines ``code.profdata`` and the profile
+  generated from binary ``cs_code``. Profile ``cs_code.profata`` can be used by
+  ``-fprofile-use`` compilaton.
+
+  .. code-block:: console
+
+$ clang++ -O2 -fprofile-use=cs_code.profdata
+
+  The above command will read both profiles to the compiler at the identical
+  point of instrumenations.
+
 .. option:: -fprofile-use[=]
 
   Without any other arguments, ``-fprofile-use`` behaves identically to
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -747,6 +747,12 @@
 def fprofile_generate_EQ : Joined<["-"], "fprofile-generate=">,
 Group, Flags<[DriverOption]>, MetaVarName<"">,
 HelpText<"Generate instrumented code to collect execution counts into /default.profraw (overridden by LLVM_PROFILE_FILE env var)">;
+def fcs_profile_generate : Flag<["-"], "fcs-profile-generate">,
+Group, Flags<[DriverOption]>,
+HelpText<"Generate instrumented code to collect context sensitive execution counts into default.profraw (overridden by LLVM_PROFILE_FILE env var)">;
+def fcs_profile_generate_EQ : Joined<["-"], "fcs-profile-generate=">,
+Group, Flags<[DriverOption]>, MetaVarName<"">,
+HelpText<"Generate instrumented code to collect context sensitive execution counts into /default.profraw (overridden by LLVM_PROFILE_FILE env var)">;
 def fprofile_use : Flag<["-"], "fprofile-use">, Group,
 Alias;
 def fprofile_use_EQ : Joined<["-"], "fprofile-use=">,
Index: include/clang/Basic/CodeGenOptions.h
===
--- include/clang/Basic/CodeGenOptions.h
+++ include/clang/Basic/CodeGenOptions.h
@@ -100,6 +100,7 @@
 ProfileClangInstr, // Clang instrumentation to generate execution counts
// to use with PGO.
 ProfileIRInstr,// IR level PGO instrumentation in LLVM.
+ProfileCSIRInstr, // IR level PGO context sensitive instrumentation in LLVM.
   };
 
   enum EmbedBitcodeKind {
@@ -203,8 +204,8 @@
   /// A list of linker options to embed in the object file.
   std::vector LinkerOptions;
 
-  /// Name of the profile file to use as output for -fprofile-instr-generate
-  /// and -fprofile-generate.
+  /// Name of the profile 

r355331 - [PGO] Clang part of change for context-sensitive PGO (part1)

[Rong Xu via cfe-commits]

Author: xur
Date: Mon Mar  4 12:21:31 2019
New Revision: 355331

URL: http://llvm.org/viewvc/llvm-project?rev=355331=rev
Log:
[PGO] Clang part of change for context-sensitive PGO (part1)

Part 1 of CSPGO change in Clang. This includes changes in clang options
and calls to llvm PassManager. Tests will be committed in part2.
This change needs the PassManager change in llvm.

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


Modified:
cfe/trunk/docs/UsersManual.rst
cfe/trunk/include/clang/Basic/CodeGenOptions.h
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/lib/Driver/ToolChain.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp

Modified: cfe/trunk/docs/UsersManual.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/UsersManual.rst?rev=355331=355330=355331=diff
==
--- cfe/trunk/docs/UsersManual.rst (original)
+++ cfe/trunk/docs/UsersManual.rst Mon Mar  4 12:21:31 2019
@@ -1745,7 +1745,8 @@ controlled by the GCC-compatible flags `
 ``-fprofile-use``. Although these flags are semantically equivalent to
 their GCC counterparts, they *do not* handle GCC-compatible profiles.
 They are only meant to implement GCC's semantics with respect to
-profile creation and use.
+profile creation and use. Flag ``-fcs-profile-generate`` also instruments
+programs using the same instrumentation method as ``-fprofile-generate``.
 
 .. option:: -fprofile-generate[=]
 
@@ -1778,6 +1779,45 @@ profile creation and use.
  ``LLVM_PROFILE_FILE`` can still be used to override
  the directory and filename for the profile file at runtime.
 
+.. option:: -fcs-profile-generate[=]
+
+  The ``-fcs-profile-generate`` and ``-fcs-profile-generate=`` flags will use
+  the same instrumentation method, and generate the same profile as in the
+  ``-fprofile-generate`` and ``-fprofile-generate=`` flags. The difference is
+  that the instrumentation is performed after inlining so that the resulted
+  profile has a better context sensitive information. They cannot be used
+  together with ``-fprofile-generate`` and ``-fprofile-generate=`` flags.
+  They are typically used in conjunction with ``-fprofile-use`` flag.
+  The profile generated by ``-fcs-profile-generate`` and ``-fprofile-generate``
+  can be merged by llvm-profdata. A use example:
+
+  .. code-block:: console
+
+$ clang++ -O2 -fprofile-generate=yyy/zzz code.cc -o code
+$ ./code
+$ llvm-profdata merge -output=code.profdata yyy/zzz/
+
+  The first few steps are the same as that in ``-fprofile-generate``
+  compilation. Then perform a second round of instrumentation.
+
+  .. code-block:: console
+
+$ clang++ -O2 -fprofile-use=code.profdata -fcs-profile-generate=sss/ttt \
+  -o cs_code
+$ ./cs_code
+$ llvm-profdata merge -output=cs_code.profdata sss/ttt code.profdata
+
+  The resulted ``cs_code.prodata`` combines ``code.profdata`` and the profile
+  generated from binary ``cs_code``. Profile ``cs_code.profata`` can be used by
+  ``-fprofile-use`` compilaton.
+
+  .. code-block:: console
+
+$ clang++ -O2 -fprofile-use=cs_code.profdata
+
+  The above command will read both profiles to the compiler at the identical
+  point of instrumenations.
+
 .. option:: -fprofile-use[=]
 
   Without any other arguments, ``-fprofile-use`` behaves identically to

Modified: cfe/trunk/include/clang/Basic/CodeGenOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/CodeGenOptions.h?rev=355331=355330=355331=diff
==
--- cfe/trunk/include/clang/Basic/CodeGenOptions.h (original)
+++ cfe/trunk/include/clang/Basic/CodeGenOptions.h Mon Mar  4 12:21:31 2019
@@ -100,6 +100,7 @@ public:
 ProfileClangInstr, // Clang instrumentation to generate execution counts
// to use with PGO.
 ProfileIRInstr,// IR level PGO instrumentation in LLVM.
+ProfileCSIRInstr, // IR level PGO context sensitive instrumentation in 
LLVM.
   };
 
   enum EmbedBitcodeKind {
@@ -203,8 +204,8 @@ public:
   /// A list of linker options to embed in the object file.
   std::vector LinkerOptions;
 
-  /// Name of the profile file to use as output for -fprofile-instr-generate
-  /// and -fprofile-generate.
+  /// Name of the profile file to use as output for -fprofile-instr-generate,
+  /// -fprofile-generate, and -fcs-profile-generate.
   std::string InstrProfileOutput;
 
   /// Name of the profile file to use with -fprofile-sample-use.
@@ -318,6 +319,11 @@ public:
 return getProfileInstr() == ProfileIRInstr;
   }
 
+  /// Check if CS IR level profile instrumentation is on.
+  bool hasProfileCSIRInstr() const {
+return getProfileInstr() == ProfileCSIRInstr;
+  }
+
   /// Check if Clang profile use is on.
   bool 

[PATCH] D58807: [CodeGen] COMDAT-fold block descriptors

[Dustin L. Howett via Phabricator via cfe-commits]

DHowett-MSFT updated this revision to Diff 189183.
DHowett-MSFT added a comment.

This change caused a test to fail, so I took the opportunity to augment the 
test with COMDAT entry.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58807

Files:
  clang/lib/CodeGen/CGBlocks.cpp
  test/CodeGenObjC/block-desc-str.m


Index: test/CodeGenObjC/block-desc-str.m
===
--- test/CodeGenObjC/block-desc-str.m
+++ test/CodeGenObjC/block-desc-str.m
@@ -1,9 +1,13 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -emit-llvm 
-fobjc-runtime=gnustep-1.7 -fblocks -o - %s | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-llvm -fobjc-runtime=gcc 
-fblocks -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -emit-llvm 
-fobjc-runtime=gnustep-1.7 -fblocks -o - %s | FileCheck 
--check-prefix=CHECK-COMDAT %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-llvm -fobjc-runtime=gcc 
-fblocks -o - %s | FileCheck --check-prefix=CHECK-COMDAT %s
 // RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm -fblocks -o - %s | 
FileCheck %s
 
 // Test that descriptor symbol names don't include '@'.
 
+// CHECK-COMDAT: $"__block_descriptor_40_8_32o_e5_v8\01?0l" = comdat any
+// CHECK-COMDAT: @[[STR:.*]] = private unnamed_addr constant [6 x i8] 
c"v8@?0\00"
+// CHECK-COMDAT: @"__block_descriptor_40_8_32o_e5_v8\01?0l" = linkonce_odr 
hidden unnamed_addr constant { i64, i64, i8*, i8*, i8*, {{.*}} } { i64 0, i64 
40, i8* bitcast ({{.*}} to i8*), i8* bitcast ({{.*}} to i8*), i8* getelementptr 
inbounds ([6 x i8], [6 x i8]* @[[STR]], i32 0, i32 0), {{.*}} }, comdat, align 8
+
 // CHECK: @[[STR:.*]] = private unnamed_addr constant [6 x i8] c"v8@?0\00"
 // CHECK: @"__block_descriptor_40_8_32o_e5_v8\01?0l" = linkonce_odr hidden 
unnamed_addr constant { i64, i64, i8*, i8*, i8*, {{.*}} } { i64 0, i64 40, i8* 
bitcast ({{.*}} to i8*), i8* bitcast ({{.*}} to i8*), i8* getelementptr 
inbounds ([6 x i8], [6 x i8]* @[[STR]], i32 0, i32 0), {{.*}} }, align 8
 
Index: clang/lib/CodeGen/CGBlocks.cpp
===
--- clang/lib/CodeGen/CGBlocks.cpp
+++ clang/lib/CodeGen/CGBlocks.cpp
@@ -274,6 +274,8 @@
  /*constant*/ true, linkage, AddrSpace);
 
   if (linkage == llvm::GlobalValue::LinkOnceODRLinkage) {
+if (CGM.supportsCOMDAT())
+  global->setComdat(CGM.getModule().getOrInsertComdat(descName));
 global->setVisibility(llvm::GlobalValue::HiddenVisibility);
 global->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
   }


Index: test/CodeGenObjC/block-desc-str.m
===
--- test/CodeGenObjC/block-desc-str.m
+++ test/CodeGenObjC/block-desc-str.m
@@ -1,9 +1,13 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -emit-llvm -fobjc-runtime=gnustep-1.7 -fblocks -o - %s | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-llvm -fobjc-runtime=gcc -fblocks -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -emit-llvm -fobjc-runtime=gnustep-1.7 -fblocks -o - %s | FileCheck --check-prefix=CHECK-COMDAT %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-llvm -fobjc-runtime=gcc -fblocks -o - %s | FileCheck --check-prefix=CHECK-COMDAT %s
 // RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm -fblocks -o - %s | FileCheck %s
 
 // Test that descriptor symbol names don't include '@'.
 
+// CHECK-COMDAT: $"__block_descriptor_40_8_32o_e5_v8\01?0l" = comdat any
+// CHECK-COMDAT: @[[STR:.*]] = private unnamed_addr constant [6 x i8] c"v8@?0\00"
+// CHECK-COMDAT: @"__block_descriptor_40_8_32o_e5_v8\01?0l" = linkonce_odr hidden unnamed_addr constant { i64, i64, i8*, i8*, i8*, {{.*}} } { i64 0, i64 40, i8* bitcast ({{.*}} to i8*), i8* bitcast ({{.*}} to i8*), i8* getelementptr inbounds ([6 x i8], [6 x i8]* @[[STR]], i32 0, i32 0), {{.*}} }, comdat, align 8
+
 // CHECK: @[[STR:.*]] = private unnamed_addr constant [6 x i8] c"v8@?0\00"
 // CHECK: @"__block_descriptor_40_8_32o_e5_v8\01?0l" = linkonce_odr hidden unnamed_addr constant { i64, i64, i8*, i8*, i8*, {{.*}} } { i64 0, i64 40, i8* bitcast ({{.*}} to i8*), i8* bitcast ({{.*}} to i8*), i8* getelementptr inbounds ([6 x i8], [6 x i8]* @[[STR]], i32 0, i32 0), {{.*}} }, align 8
 
Index: clang/lib/CodeGen/CGBlocks.cpp
===
--- clang/lib/CodeGen/CGBlocks.cpp
+++ clang/lib/CodeGen/CGBlocks.cpp
@@ -274,6 +274,8 @@
  /*constant*/ true, linkage, AddrSpace);
 
   if (linkage == llvm::GlobalValue::LinkOnceODRLinkage) {
+if (CGM.supportsCOMDAT())
+  global->setComdat(CGM.getModule().getOrInsertComdat(descName));
 global->setVisibility(llvm::GlobalValue::HiddenVisibility);
 

[PATCH] D58917: [HIP] Do not unbundle object files for -fno-gpu-rdc

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added a reviewer: tra.
Herald added a subscriber: jdoerfert.
Herald added a project: clang.

When -fno-gpu-rdc is set, device code is compiled, linked, and assembled into 
fat binary
and embedded as string in object files. The object files are normal object 
files which
can be linked by host linker. In the linking stage, the object files should not 
be unbundled
when -fno-gpu-rdc is set since they are normal object files, not bundles. The 
object files
only need to be unbundled when -fgpu-rdc is set.

Currently clang always unbundles object files, disregarding -fgpu-rdc option.

This patch fixes that.


Repository:
  rC Clang

https://reviews.llvm.org/D58917

Files:
  lib/Driver/Driver.cpp
  test/Driver/hip-binding.hip
  test/Driver/hip-link-shared-library.hip

Index: test/Driver/hip-link-shared-library.hip
===
--- test/Driver/hip-link-shared-library.hip
+++ test/Driver/hip-link-shared-library.hip
@@ -1,7 +1,7 @@
 // RUN: touch %t.o
 // RUN: %clang --hip-link -ccc-print-bindings -target x86_64-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %t.o %S/Inputs/in.so \
-// RUN: 2>&1 | FileCheck %s
+// RUN:   -fgpu-rdc 2>&1 | FileCheck %s
 
 // CHECK: # "amdgcn-amd-amdhsa" - "offload bundler", inputs: ["[[IN:.*o]]"], outputs: ["[[OBJ1:.*o]]", "[[OBJ2:.*o]]", "[[OBJ3:.*o]]"]
 // CHECK: # "amdgcn-amd-amdhsa" - "AMDGCN::Linker", inputs: ["[[OBJ2]]"], output: "[[IMG2:.*out]]"
Index: test/Driver/hip-binding.hip
===
--- test/Driver/hip-binding.hip
+++ test/Driver/hip-binding.hip
@@ -4,7 +4,7 @@
 
 // RUN: touch %t.o
 // RUN: %clang --hip-link -ccc-print-bindings -target x86_64-linux-gnu \
-// RUN:   --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %t.o\
+// RUN:   --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 -fgpu-rdc %t.o\
 // RUN: 2>&1 | FileCheck %s
 
 // CHECK: # "amdgcn-amd-amdhsa" - "offload bundler", inputs: ["[[IN:.*o]]"], outputs: ["[[OBJ1:.*o]]", "[[OBJ2:.*o]]", "[[OBJ3:.*o]]"] 
@@ -13,3 +13,10 @@
 // CHECK: # "amdgcn-amd-amdhsa" - "AMDGCN::Linker", inputs: ["[[OBJ3]]"], output: "[[IMG3:.*out]]"
 // CHECK-NOT: offload bundler
 // CHECK: # "x86_64-unknown-linux-gnu" - "GNU::Linker", inputs: ["[[OBJ1]]", "[[IMG2]]", "[[IMG3]]"], output: "a.out"
+
+// RUN: %clang --hip-link -ccc-print-bindings -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %t.o\
+// RUN: 2>&1 | FileCheck -check-prefix=NORDC %s
+
+// NORDC-NOT: offload bundler
+// NORDC: # "x86_64-unknown-linux-gnu" - "GNU::Linker", inputs: ["{{.*o}}"], output: "a.out"
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2293,11 +2293,15 @@
 
 /// Flag that is set to true if this builder acted on the current input.
 bool IsActive = false;
+
+/// Flag for -fgpu-rdc.
+bool Relocatable;
   public:
 CudaActionBuilderBase(Compilation , DerivedArgList ,
   const Driver::InputList ,
   Action::OffloadKind OFKind)
-: DeviceActionBuilder(C, Args, Inputs, OFKind) {}
+: DeviceActionBuilder(C, Args, Inputs, OFKind),
+  Relocatable(false) {}
 
 ActionBuilderReturnCode addDeviceDepences(Action *HostAction) override {
   // While generating code for CUDA, we only depend on the host input action
@@ -2338,6 +2342,12 @@
 
   // If this is an unbundling action use it as is for each CUDA toolchain.
   if (auto *UA = dyn_cast(HostAction)) {
+
+// If -fgpu-rdc is disabled, should not unbundle since there is no
+// device code to link.
+if (!Relocatable)
+  return ABRT_Inactive;
+
 CudaDeviceActions.clear();
 auto *IA = cast(UA->getInputs().back());
 std::string FileName = IA->getInputArg().getAsString(Args);
@@ -2409,6 +2419,9 @@
   !C.hasOffloadToolChain())
 return false;
 
+  Relocatable = Args.hasFlag(options::OPT_fgpu_rdc,
+  options::OPT_fno_gpu_rdc, /*Default=*/false);
+
   const ToolChain *HostTC = C.getSingleOffloadToolChain();
   assert(HostTC && "No toolchain for host compilation.");
   if (HostTC->getTriple().isNVPTX() ||
@@ -2594,13 +2607,11 @@
   class HIPActionBuilder final : public CudaActionBuilderBase {
 /// The linker inputs obtained for each device arch.
 SmallVector DeviceLinkerInputs;
-bool Relocatable;
 
   public:
 HIPActionBuilder(Compilation , DerivedArgList ,
  const Driver::InputList )
-: CudaActionBuilderBase(C, Args, Inputs, Action::OFK_HIP),
-  Relocatable(false) {}
+: CudaActionBuilderBase(C, Args, Inputs, Action::OFK_HIP) {}
 
 bool canUseBundlerUnbundler() const override { return true; }
 
@@ -2705,13 +2716,6 @@
 ++I;
   }
 }
-
-bool initialize() 

[PATCH] D58502: [ASTImporter] Fix redecl failures of Class and ClassTemplate

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik accepted this revision.
shafik added a comment.

LGTM, thank you !


Repository:
  rC Clang

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

https://reviews.llvm.org/D58502



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

[PATCH] D58751: Order File Instrumentation: add clang support for -forder-file-instrumentation

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added inline comments.



Comment at: lib/Driver/ToolChains/Clang.cpp:5312
+ CmdArgs.push_back("-forder-file-instrumentation");
+ // Enable order file instrumentation when ThinLTO is not on. When ThinLTO 
is
+ // on, we need to pass these flags as linker flags and that will be 
handled

What about full LTO?



Comment at: test/Driver/clang_f_opts.c:137
+// CHECK-ORDERFILE-INSTR: "-forder-file-instrumentation"
+// CHECK-ORDERFILE-INSTR: "-enable-order-file-instrumentation"
 

Can you add a test for not passing this for the ThinLTO case?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58751



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

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Should the langref be updated (specifically the section on blockaddress):

  This value only has defined behavior when used as an operand to the 
‘indirectbr’ instruction, or for comparisons against null.

https://reviews.llvm.org/D53765 touched the langref, but I think the 
blockaddress section can be cleaned up.


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

https://reviews.llvm.org/D56571



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

[PATCH] D54176: [PGO] clang part of change for context-sensitive PGO.

[David Li via Phabricator via cfe-commits]

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

lgtm


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

https://reviews.llvm.org/D54176



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

[PATCH] D54176: [PGO] clang part of change for context-sensitive PGO.

[Rong Xu via Phabricator via cfe-commits]

xur updated this revision to Diff 189167.
xur added a comment.

Added usage example to UserManual.rst suggested by David.


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

https://reviews.llvm.org/D54176

Files:
  docs/UsersManual.rst
  include/clang/Basic/CodeGenOptions.h
  include/clang/Driver/Options.td
  lib/CodeGen/BackendUtil.cpp
  lib/Driver/ToolChain.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/CommonArgs.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/Inputs/pgotestir.proftext
  test/CodeGen/Inputs/pgotestir_cs.proftext
  test/CodeGen/cspgo-instrumentation.c
  test/CodeGen/cspgo-instrumentation_lto.c
  test/CodeGen/cspgo-instrumentation_thinlto.c

Index: test/CodeGen/cspgo-instrumentation_thinlto.c
===
--- test/CodeGen/cspgo-instrumentation_thinlto.c
+++ test/CodeGen/cspgo-instrumentation_thinlto.c
@@ -0,0 +1,52 @@
+// Test if CSPGO instrumentation and use pass are invoked in thinlto.
+//
+// RUN: rm -rf %t && mkdir %t
+// RUN: llvm-profdata merge -o %t/noncs.profdata %S/Inputs/pgotestir.proftext
+//
+// Ensure Pass PGOInstrumentationGenPass is not invoked in PreLink.
+// RUN: %clang_cc1 -O2 -fprofile-instrument-use-path=%t/noncs.profdata -fprofile-instrument=csllvm %s -fprofile-instrument-path=default.profraw -flto=thin -mllvm -debug-pass=Structure -emit-llvm-bc -o %t/foo_fe.bc 2>&1 | FileCheck %s -check-prefix=CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-PRE
+// RUN: %clang_cc1 -O2 -fprofile-instrument-use-path=%t/noncs.profdata -fprofile-instrument=csllvm %s -fprofile-instrument-path=default.profraw  -flto=thin -fexperimental-new-pass-manager -fdebug-pass-manager -emit-llvm-bc -o %t/foo_fe_pm.bc 2>&1 | FileCheck %s -check-prefix=CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-PRE-NEWPM
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-PRE: PGOInstrumentationUsePass
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-PRE: PGOInstrumentationGenCreateVarPass
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-PRE-NOT: PGOInstrumentationGenPass
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-PRE-NEWPM: Running pass: PGOInstrumentationUse
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-PRE-NEWPM: Running pass: PGOInstrumentationGenCreateVar
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-PRE-NEWPM-NOT: Running pass: PGOInstrumentationGen on
+//
+// RUN: llvm-lto -thinlto -o %t/foo %t/foo_fe.bc
+// RUN: llvm-lto -thinlto -o %t/foo_pm %t/foo_fe_pm.bc
+// Ensure Pass PGOInstrumentationGenPass is invoked in PostLink.
+// RUN: %clang_cc1 -O2 -x ir %t/foo_fe.bc -fthinlto-index=%t/foo.thinlto.bc -fprofile-instrument=csllvm -fprofile-instrument-path=default.profraw  -flto=thin -emit-llvm -mllvm -debug-pass=Structure -o - 2>&1 | FileCheck %s -check-prefix=CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-POST
+// RUN: %clang_cc1 -O2 -x ir %t/foo_fe_pm.bc -fthinlto-index=%t/foo_pm.thinlto.bc -fexperimental-new-pass-manager -fdebug-pass-manager  -fprofile-instrument=csllvm -fprofile-instrument-path=default.profraw  -flto=thin -emit-llvm -o - 2>&1 | FileCheck %s -check-prefix=CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-POST-NEWPM
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-POST-NOT: PGOInstrumentationUsePass
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-POST-NOT: PGOInstrumentationGenCreateVarPass
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-POST: PGOInstrumentationGenPass
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-POST-NEWPM-NOT: Running pass: PGOInstrumentationUse
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-POST-NEWPM-NOT: Running pass: PGOInstrumentationGenCreateVar
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-POST-NEWPM: Running pass: PGOInstrumentationGen on
+//
+// RUN: llvm-profdata merge -o %t/cs.profdata %S/Inputs/pgotestir_cs.proftext
+//
+// Ensure Pass PGOInstrumentationUsePass is invoked Once in PreLink.
+// RUN: %clang_cc1 -O2 -fprofile-instrument-use-path=%t/cs.profdata %s -flto=thin -mllvm -debug-pass=Structure -emit-llvm-bc -o %t/foo_fe.bc 2>&1 | FileCheck %s -check-prefix=CHECK-CSPGOUSEPASS-INVOKED-INSTR-USE-PRE
+// RUN: %clang_cc1 -O2 -fprofile-instrument-use-path=%t/cs.profdata %s -flto=thin -fexperimental-new-pass-manager -fdebug-pass-manager -emit-llvm-bc -o %t/foo_fe_pm.bc 2>&1 | FileCheck %s -check-prefix=CHECK-CSPGOUSEPASS-INVOKED-INSTR-USE-PRE-NEWPM
+// CHECK-CSPGOUSEPASS-INVOKED-INSTR-USE-PRE: PGOInstrumentationUsePass
+// CHECK-CSPGOUSEPASS-INVOKED-INSTR-USE-PRE-NOT: PGOInstrumentationUsePass
+// CHECK-CSPGOUSEPASS-INVOKED-INSTR-USE-PRE-NEWPM: Running pass: PGOInstrumentationUse
+// CHECK-CSPGOUSEPASS-INVOKED-INSTR-USE-PRE-NEWPM-NOT: Running pass: PGOInstrumentationUse
+//
+// RUN: llvm-lto -thinlto -o %t/foo %t/foo_fe.bc
+// RUN: llvm-lto -thinlto -o %t/foo_pm %t/foo_fe_pm.bc
+// Ensure Pass PGOInstrumentationUSEPass is invoked in PostLink.
+// RUN: %clang_cc1 -O2 -x ir %t/foo_fe.bc -fthinlto-index=%t/foo.thinlto.bc -fprofile-instrument-use-path=%t/cs.profdata -flto=thin -emit-llvm -mllvm -debug-pass=Structure -o - 2>&1 | FileCheck %s 

r355322 - Enable _rotl, _lrotl, _rotr, _lrotr on all platforms.

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Mon Mar  4 10:47:21 2019
New Revision: 355322

URL: http://llvm.org/viewvc/llvm-project?rev=355322=rev
Log:
Enable _rotl, _lrotl, _rotr, _lrotr on all platforms.

The above builtins are currently implemented for MSVC mode, however GCC
also implements these.  This patch enables them for all platforms.

Additionally, this corrects the type for these builtins to always be
'long int' to match the specification in the Intel Intrinsics Guide.

Change-Id: Ida34be98078709584ef5136c8761783435ec02b1

Added:
cfe/trunk/test/CodeGen/rot-intrinsics.c   (with props)
Modified:
cfe/trunk/include/clang/Basic/Builtins.def
cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c

Modified: cfe/trunk/include/clang/Basic/Builtins.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=355322=355321=355322=diff
==
--- cfe/trunk/include/clang/Basic/Builtins.def (original)
+++ cfe/trunk/include/clang/Basic/Builtins.def Mon Mar  4 10:47:21 2019
@@ -830,13 +830,13 @@ LANGBUILTIN(__popcnt64, "UWiUWi", "nc",
 LANGBUILTIN(_ReturnAddress, "v*", "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_rotl8,  "UcUcUc","n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_rotl16, "UsUsUc","n", ALL_MS_LANGUAGES)
-LANGBUILTIN(_rotl,   "UiUii", "n", ALL_MS_LANGUAGES)
-LANGBUILTIN(_lrotl,  "UNiUNii",   "n", ALL_MS_LANGUAGES)
+BUILTIN(_rotl,   "UiUii", "n")
+BUILTIN(_lrotl,  "ULiULii",   "n")
 LANGBUILTIN(_rotl64, "UWiUWii",   "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_rotr8,  "UcUcUc","n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_rotr16, "UsUsUc","n", ALL_MS_LANGUAGES)
-LANGBUILTIN(_rotr,   "UiUii", "n", ALL_MS_LANGUAGES)
-LANGBUILTIN(_lrotr,  "UNiUNii",   "n", ALL_MS_LANGUAGES)
+BUILTIN(_rotr,   "UiUii", "n")
+BUILTIN(_lrotr,  "ULiULii",   "n")
 LANGBUILTIN(_rotr64, "UWiUWii",   "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(__va_start,   "vc**.", "nt", ALL_MS_LANGUAGES)
 LANGBUILTIN(__fastfail, "vUi","nr", ALL_MS_LANGUAGES)

Modified: cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c?rev=355322=355321=355322=diff
==
--- cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c (original)
+++ cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c Mon Mar  4 10:47:21 2019
@@ -12,17 +12,10 @@
 // RUN: | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
 // RUN: %clang_cc1 -ffreestanding -fms-extensions -fms-compatibility 
-fms-compatibility-version=17.00 \
 // RUN: -triple x86_64--linux -emit-llvm %s -o - \
-// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
+// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-64BIT-LONG
 // RUN: %clang_cc1 -ffreestanding -fms-extensions \
 // RUN: -triple x86_64--darwin -emit-llvm %s -o - \
-// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
-
-// LP64 targets use 'long' as 'int' for MS intrinsics (-fms-extensions)
-#ifdef __LP64__
-#define LONG int
-#else
-#define LONG long
-#endif
+// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-64BIT-LONG
 
 // rotate left
 
@@ -47,12 +40,15 @@ unsigned int test_rotl(unsigned int valu
 // CHECK:   [[R:%.*]] = call i32 @llvm.fshl.i32(i32 [[X:%.*]], i32 [[X]], i32 
[[Y:%.*]])
 // CHECK:   ret i32 [[R]]
 
-unsigned LONG test_lrotl(unsigned LONG value, int shift) {
+unsigned long test_lrotl(unsigned long value, int shift) {
   return _lrotl(value, shift);
 }
 // CHECK-32BIT-LONG: i32 @test_lrotl
 // CHECK-32BIT-LONG:   [[R:%.*]] = call i32 @llvm.fshl.i32(i32 [[X:%.*]], i32 
[[X]], i32 [[Y:%.*]])
 // CHECK-32BIT-LONG:   ret i32 [[R]]
+// CHECK-64BIT-LONG: i64 @test_lrotl
+// CHECK-64BIT-LONG:   [[R:%.*]] = call i64 @llvm.fshl.i64(i64 [[X:%.*]], i64 
[[X]], i64 [[Y:%.*]])
+// CHECK-64BIT-LONG:   ret i64 [[R]]
 
 unsigned __int64 test_rotl64(unsigned __int64 value, int shift) {
   return _rotl64(value, shift);
@@ -84,12 +80,15 @@ unsigned int test_rotr(unsigned int valu
 // CHECK:   [[R:%.*]] = call i32 @llvm.fshr.i32(i32 [[X:%.*]], i32 [[X]], i32 
[[Y:%.*]])
 // CHECK:   ret i32 [[R]]
 
-unsigned LONG test_lrotr(unsigned LONG value, int shift) {
+unsigned long test_lrotr(unsigned long value, int shift) {
   return _lrotr(value, shift);
 }
 // CHECK-32BIT-LONG: i32 @test_lrotr
 // CHECK-32BIT-LONG:   [[R:%.*]] = call i32 @llvm.fshr.i32(i32 [[X:%.*]], i32 
[[X]], i32 [[Y:%.*]])
 // CHECK-32BIT-LONG:   ret i32 [[R]]
+// CHECK-64BIT-LONG: i64 @test_lrotr
+// CHECK-64BIT-LONG:   [[R:%.*]] = call i64 @llvm.fshr.i64(i64 [[X:%.*]], i64 
[[X]], i64 [[Y:%.*]])
+// CHECK-64BIT-LONG:   ret i64 [[R]]
 
 unsigned __int64 test_rotr64(unsigned __int64 value, int shift) {
   return _rotr64(value, shift);

Added: cfe/trunk/test/CodeGen/rot-intrinsics.c
URL: 

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

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

It might be a good idea to add the nested lambda tests from D44609: 
[Clang-Format] New option BeforeLambdaBody to manage lambda line break inside 
function parameter call (in Allman style)  
into your review to show how this patch copes with those (and the examples from 
the comments of that review)


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

https://reviews.llvm.org/D57687



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

[PATCH] D58751: Order File Instrumentation: add clang support for -forder-file-instrumentation

[Brian Gesiak via Phabricator via cfe-commits]

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

Looks good, thanks for adding a driver option for this!




Comment at: include/clang/Driver/Options.td:774
+Group, Flags<[CoreOption]>,
+HelpText<"Generate instrumented code to collect order file into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
 

Would it make sense to add documentation to the Clang User's Manual for this 
option? Some other profiling options are mentioned there, but not all.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58751



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

[PATCH] D58814: [clang][Index] Constructors and Destructors do not reference class

[Nathan Hawes via Phabricator via cfe-commits]

nathawes added a comment.
Herald added a subscriber: ormris.

In D58814#1415607 , @gribozavr wrote:

> > These references were added to support using the index data for symbol 
> > rename.
>
> Understood -- thank you for providing the context (the original commit that 
> added this code didn't have the explanation why this reference was added).


That was my patch (via Argyrios), so sorry about that!

> However, my suggestion would be to still take this patch.  For symbol rename, 
> my suggestion would be one of the following.
> 
> Option (1): Symbol rename is a complex operation that requires knowing many 
> places where the class name appears.  So I think it is fair that it would 
> need to navigate to all such declarations using the semantic index -- find 
> the class, then find its constructors, destructor, out-of-line functions, 
> find derived classes, then find `using` declarations in derived classes, find 
> calls to constructors, destructor etc.  I don't think it is not the job of 
> the core indexer API to hardcode all of these places as a "reference to the 
> class".  Different clients require different navigation to related symbols, 
> and hardcoding all such navigation patterns in the indexer would create a 
> messy index that is difficult to work with, since you'd have to filter out 
> lots of stuff.  Not to even mention the index size.

I feel like the complexity you mention here is what makes it worthwhile to 
expose all these locations as occurrences of the type, and I'm fairly sure 
we're actually already doing that in all these locations (unless I've missed 
other patches removing them). Pushing this work onto all clients that want to 
provide automatic global rename functionality or to support users manually 
renaming by including these in their find-references results seems like a 
poorer outcome from a code sharing/maintenance point of view than continuing to 
provide it and requiring clients that don't want it to check the SymbolRole 
bits inside their IndexDataConsumer's handleDeclOccurrence (`if (Roles & 
SymbolRole::NameReference) return true;`). I don't think the index size is 
concerning; clients don't have to store these occurrences if they don't care 
about them.

> Option (2): A client that wants to hardcode all navigation patterns in the 
> index can still do this -- in the `IndexDataConsumer`, it is possible to 
> handle the `ConstructorDecl` as a reference to the class, if desired.  The 
> flow of the indexing information becomes more clear in my opinion: when there 
> is a constructor decl in the source, the `IndexDataConsumer` gets one 
> `ConstructorDecl`. Then the specific implementation of `IndexDataConsumer` 
> decides to treat it also as a class reference, because it wants to support a 
> specific approach to performing symbol renaming.

I think the downside here is again the maintenance burden. This code would 
probably live downstream somewhere so whenever changes happen upstream that 
affect the code deriving these extra occurrences in clients that want it, or 
introduce new types of locations that those clients don't handle and miss, 
there's more duplicated effort updating/hunting down bugs. I also personally 
see these as legitimate occurrences of the type rather than hardcoded 
navigation patterns (see below), even though rename was the motivation here.

> As is, the indexing information is misleading and surprising.  When we see a 
> constructor decl or a constructor call, there is no reference to the class at 
> that point -- there is a reference to a constructor.  I think index is 
> supposed to expose semantic information, not just that there's a token in the 
> source code that has the same spelling as the class name.
> 
>> could we instead distinguish these cases with a more specific SymbolRole, 
>> e.g. NameReference as opposed to Reference, and filter them based on that 
>> instead?
> 
> It feels like a workaround to me, that also pushes the fix to the clients of 
> the indexing API. The core of the problem still exists: the indexing 
> information is surprising, and requires extra work on the client to make it 
> not surprising (filtering out NameReferences).

I agree that most people think of it as purely being the constructor name, but 
I don't think the semantic connection to the class is really as loose as 'a 
token with the same spelling' and I think it's reasonable to report it. To 
resolve a constructor call, the compiler looks for a *type* with that spelling. 
It's not as if the user chooses to name their constructor with the same name as 
the type – the name lookup finds the type. One interesting consequence of this 
is that the compiler seems to complain if you try to reference the constructor 
as opposed to the type when you call it, e.g.

  class A {
  public:
A(int x) {}
  };
  
  int main() {
auto x = A(2); // ok
auto y = A::A(2); // error: qualified reference to 'A' is a constructor 

[PATCH] D58885: Variable auto-init: split out small arrays

[Alexander Potapenko via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGDecl.cpp:1206
+  bool canDoSingleStore = Ty->isIntOrIntVectorTy() ||
+  Ty->isPtrOrPtrVectorTy() || Ty->isFPOrFPVectorTy();
+  if (canDoSingleStore) {

jfb wrote:
> glider wrote:
> > Is the second expression being moved to line 1206 a result of clang-format? 
> > Otherwise it'll migrate back at some point.
> Yes, the slightly longer name pushes it past 80 columns, and I just 
> auto-format stuff before uploading.
> 
> I can do this change separately, I just noticed that the name I originally 
> used was now misleading because vectors aren't scalars :)
Up to you, this doesn't matter IMO :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D58885



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

[PATCH] D58885: Variable auto-init: split out small arrays

[JF Bastien via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGDecl.cpp:1206
+  bool canDoSingleStore = Ty->isIntOrIntVectorTy() ||
+  Ty->isPtrOrPtrVectorTy() || Ty->isFPOrFPVectorTy();
+  if (canDoSingleStore) {

glider wrote:
> Is the second expression being moved to line 1206 a result of clang-format? 
> Otherwise it'll migrate back at some point.
Yes, the slightly longer name pushes it past 80 columns, and I just auto-format 
stuff before uploading.

I can do this change separately, I just noticed that the name I originally used 
was now misleading because vectors aren't scalars :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D58885



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

[PATCH] D58885: Variable auto-init: split out small arrays

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

The change itself looks good.
It doesn't seem to regress kernel performance on ARM64. I haven't got to 
testing on x86 yet, but don't anticipate any problems either.




Comment at: lib/CodeGen/CGDecl.cpp:1206
+  bool canDoSingleStore = Ty->isIntOrIntVectorTy() ||
+  Ty->isPtrOrPtrVectorTy() || Ty->isFPOrFPVectorTy();
+  if (canDoSingleStore) {

Is the second expression being moved to line 1206 a result of clang-format? 
Otherwise it'll migrate back at some point.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58885



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

[PATCH] D58885: Variable auto-init: split out small arrays

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 189159.
jfb added a comment.

- Fix test labels.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58885

Files:
  lib/CodeGen/CGDecl.cpp
  test/CodeGenCXX/auto-var-init.cpp

Index: test/CodeGenCXX/auto-var-init.cpp
===
--- test/CodeGenCXX/auto-var-init.cpp
+++ test/CodeGenCXX/auto-var-init.cpp
@@ -129,7 +129,6 @@
 // PATTERN-O1-NOT: @__const.test_bool4_custom.custom
 // ZERO-O1-NOT: @__const.test_bool4_custom.custom
 
-// PATTERN: @__const.test_intptr4_uninit.uninit = private unnamed_addr constant [4 x i32*] [i32* inttoptr (i64 -6148914691236517206 to i32*), i32* inttoptr (i64 -6148914691236517206 to i32*), i32* inttoptr (i64 -6148914691236517206 to i32*), i32* inttoptr (i64 -6148914691236517206 to i32*)], align 16
 // PATTERN: @__const.test_intptr4_custom.custom = private unnamed_addr constant [4 x i32*] [i32* inttoptr (i64 572662306 to i32*), i32* inttoptr (i64 572662306 to i32*), i32* inttoptr (i64 572662306 to i32*), i32* inttoptr (i64 572662306 to i32*)], align 16
 // ZERO: @__const.test_intptr4_custom.custom = private unnamed_addr constant [4 x i32*] [i32* inttoptr (i64 572662306 to i32*), i32* inttoptr (i64 572662306 to i32*), i32* inttoptr (i64 572662306 to i32*), i32* inttoptr (i64 572662306 to i32*)], align 16
 // PATTERN-O0: @__const.test_tailpad4_uninit.uninit = private unnamed_addr constant [4 x { i16, i8, [1 x i8] }] [{ i16, i8, [1 x i8] } { i16 -21846, i8 -86, [1 x i8] c"\AA" }, { i16, i8, [1 x i8] } { i16 -21846, i8 -86, [1 x i8] c"\AA" }, { i16, i8, [1 x i8] } { i16 -21846, i8 -86, [1 x i8] c"\AA" }, { i16, i8, [1 x i8] } { i16 -21846, i8 -86, [1 x i8] c"\AA" }], align 16
@@ -1019,13 +1018,20 @@
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%custom)
 
 TEST_UNINIT(intptr4, int*[4]);
-// CHECK-LABEL: @test_intptr4_uninit()
-// CHECK:   %uninit = alloca [4 x i32*], align
-// CHECK-NEXT:  call void @{{.*}}used{{.*}}%uninit)
-// PATTERN-LABEL: @test_intptr4_uninit()
-// PATTERN: call void @llvm.memcpy{{.*}} @__const.test_intptr4_uninit.uninit
-// ZERO-LABEL: @test_intptr4_uninit()
-// ZERO: call void @llvm.memset{{.*}}, i8 0,
+// CHECK-LABEL:  @test_intptr4_uninit()
+// CHECK:%uninit = alloca [4 x i32*], align
+// CHECK-NEXT:   call void @{{.*}}used{{.*}}%uninit)
+// PATTERN-O1-LABEL: @test_intptr4_uninit()
+// PATTERN-O1:   %1 = getelementptr inbounds [4 x i32*], [4 x i32*]* %uninit, i64 0, i64 0
+// PATTERN-O1-NEXT:  store i32* inttoptr (i64 -6148914691236517206 to i32*), i32** %1, align 16
+// PATTERN-O1-NEXT:  %2 = getelementptr inbounds [4 x i32*], [4 x i32*]* %uninit, i64 0, i64 1
+// PATTERN-O1-NEXT:  store i32* inttoptr (i64 -6148914691236517206 to i32*), i32** %2, align 8
+// PATTERN-O1-NEXT:  %3 = getelementptr inbounds [4 x i32*], [4 x i32*]* %uninit, i64 0, i64 2
+// PATTERN-O1-NEXT:  store i32* inttoptr (i64 -6148914691236517206 to i32*), i32** %3, align 16
+// PATTERN-O1-NEXT:  %4 = getelementptr inbounds [4 x i32*], [4 x i32*]* %uninit, i64 0, i64 3
+// PATTERN-O1-NEXT:  store i32* inttoptr (i64 -6148914691236517206 to i32*), i32** %4, align 8
+// ZERO-LABEL:   @test_intptr4_uninit()
+// ZERO: call void @llvm.memset{{.*}}, i8 0,
 
 TEST_BRACES(intptr4, int*[4]);
 // CHECK-LABEL: @test_intptr4_braces()
@@ -1124,7 +1130,7 @@
 // PATTERN-LABEL: @test_atomicnotlockfree_uninit()
 // PATTERN-O0: call void @llvm.memcpy{{.*}} @__const.test_atomicnotlockfree_uninit.uninit
 // PATTERN-O1: bitcast
-// PATTERN-O1: call void @llvm.memset{{.*}}({{.*}}0, i8 -86, i64 32
+// PATTERN-O1: call void @llvm.memset{{.*}}({{.*}}, i8 -86, i64 32
 // ZERO-LABEL: @test_atomicnotlockfree_uninit()
 // ZERO: call void @llvm.memset{{.*}}, i8 0,
 
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -969,12 +969,12 @@
   return llvm::isBytewiseValue(Init);
 }
 
-/// Decide whether we want to split a constant structure store into a sequence
-/// of its fields' stores. This may cost us code size and compilation speed,
-/// but plays better with store optimizations.
-static bool shouldSplitStructStore(CodeGenModule ,
-   uint64_t GlobalByteSize) {
-  // Don't break structures that occupy more than one cacheline.
+/// Decide whether we want to split a constant structure or array store into a
+/// sequence of its fields' stores. This may cost us code size and compilation
+/// speed, but plays better with store optimizations.
+static bool shouldSplitConstantStore(CodeGenModule ,
+ uint64_t GlobalByteSize) {
+  // Don't break things that occupy more than one cacheline.
   uint64_t ByteSizeLimit = 64;
   if (CGM.getCodeGenOpts().OptimizationLevel == 0)
 return false;
@@ -1202,9 +1202,9 @@
   CGBuilderTy ,
 

[PATCH] D58885: Variable auto-init: split out small arrays

[JF Bastien via Phabricator via cfe-commits]

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



Comment at: test/CodeGenCXX/auto-var-init.cpp:1025
 // PATTERN-LABEL: @test_intptr4_uninit()
-// PATTERN: call void @llvm.memcpy{{.*}} @__const.test_intptr4_uninit.uninit
-// ZERO-LABEL: @test_intptr4_uninit()
-// ZERO: call void @llvm.memset{{.*}}, i8 0,
+// PATTERN:   %1 = getelementptr inbounds [4 x i32*], [4 x i32*]* %uninit, 
i64 0, i64 0
+// PATTERN-NEXT:  store i32* inttoptr (i64 -6148914691236517206 to i32*), 
i32** %1, align 16

glider wrote:
> This check fails for me locally.
Apologies, I played around with the labels and forgot to fix them before 
sending the patch.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58885



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

[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check

[Lewis Clark via Phabricator via cfe-commits]

lewmpk updated this revision to Diff 189157.
lewmpk added a comment.

match lock() and unlock() calls by decl


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58818

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp
  clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-use-raii-locks.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-use-raii-locks.cpp

Index: test/clang-tidy/cppcoreguidelines-use-raii-locks.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-use-raii-locks.cpp
@@ -0,0 +1,194 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-use-raii-locks %t
+
+// Mock implementation of std::mutex
+namespace std {
+struct mutex {
+  void lock();
+  void try_lock();
+  void unlock();
+};
+typedef mutex recursive_mutex;
+} // namespace std
+
+void warn_me1_simple() {
+  std::mutex m;
+  m.lock();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+  m.unlock();
+}
+
+void warn_me2_nested() {
+  std::mutex m;
+  m.lock();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+  {
+std::mutex m;
+m.lock();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use RAII
+m.unlock();
+  }
+  m.unlock();
+}
+
+void warn_me3_nested_extra() {
+  std::mutex m1;
+  std::mutex m2;
+  {
+m1.lock();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use RAII
+{
+  m2.lock();
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use RAII
+  m2.unlock();
+}
+m1.unlock();
+  }
+}
+
+void warn_me4_multi_mutex() {
+  std::mutex m1;
+  std::mutex m2;
+
+  m1.lock();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+  m1.unlock();
+  m2.lock();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+  m2.unlock();
+}
+
+void warn_me5_multi_mutex_extra() {
+  std::mutex m1;
+  std::mutex m2;
+
+  m1.lock();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+  m2.lock();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+  m1.unlock();
+  m2.unlock();
+}
+
+void warn_me6() {
+  std::mutex m;
+  m.lock();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+  m.unlock();
+  m.lock();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+  m.unlock();
+  m.try_lock();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+  m.lock();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+  m.unlock();
+}
+
+void warn_me7_loop() {
+  std::mutex m;
+  for (auto i = 0; i < 3; i++) {
+m.lock();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use RAII
+m.unlock();
+  }
+}
+
+template 
+void attempt(M m) {
+  m.lock();
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use RAII
+  m.unlock();
+}
+
+void warn_me8_template_func() {
+  std::mutex m;
+  attempt(m);
+}
+
+// clang-format off
+#define ATTEMPT(m) {\ 
+m.lock();\ 
+m.unlock();\ 
+  }
+// clang-format on
+
+void warn_me9_macro() {
+  std::mutex m;
+  ATTEMPT(m);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use RAII
+}
+
+void warn_me10_inline() {
+  std::mutex m;
+  // clang-format off
+  m.lock(); m.unlock();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+  // clang-format on
+}
+
+void warn_me11_recursive_mutex() {
+  std::recursive_mutex m;
+  m.lock();
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use RAII
+  m.unlock();
+}
+
+void ignore_me1_rev_order() {
+  std::mutex m;
+  m.unlock();
+  m.lock();
+  // CHECK-MESSAGES-NOT: warning:
+}
+
+void ignore_me2_diff_mtx() {
+  std::mutex m1;
+  std::mutex m2;
+  m1.lock();
+  // CHECK-MESSAGES-NOT: warning:
+  m2.unlock();
+}
+
+void ignore_me3() {
+  std::recursive_mutex m1;
+  std::recursive_mutex m2;
+  m1.lock();
+  // CHECK-MESSAGES-NOT: warning:
+  m2.unlock();
+}
+
+void ignore_me4() {
+  std::mutex m;
+  m.unlock();
+  m.lock();
+  m.lock();
+  // CHECK-MESSAGES-NOT: warning:
+}
+
+void ignore_me5_inline() {
+  std::mutex m;
+  m.unlock();
+  m.lock();
+  // CHECK-MESSAGES-NOT: warning:
+}
+
+void ignore_me6_seperate_scopes() {
+  std::mutex m;
+  {
+m.lock();
+// CHECK-MESSAGES-NOT: warning:
+  }
+  {
+m.unlock();
+  }
+}
+
+void ignore_me7_seperate_scopes_nested() {
+  std::mutex m;
+  {
+m.lock();
+// CHECK-MESSAGES-NOT: warning:
+{
+  m.unlock();
+}
+  }
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -118,6 +118,7 @@
cppcoreguidelines-pro-type-vararg
cppcoreguidelines-slicing
cppcoreguidelines-special-member-functions
+   cppcoreguidelines-use-raii-locks
fuchsia-default-arguments
fuchsia-header-anon-namespaces (redirects to google-build-namespaces) 

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

[Ronald Wampler via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/Format.cpp:649
   LLVMStyle.AllowShortIfStatementsOnASingleLine = false;
+  LLVMStyle.AllowShortLambdasOnASingleLine = FormatStyle::SLS_All;
   LLVMStyle.AllowShortLoopsOnASingleLine = false;

MyDeveloperDay wrote:
> What is the difference between what clang-format does now and using SLS_All, 
> i.e. if your introducing a new style shouldn't the default be to not change 
> the exsiting code?
> 
> Without trying this myself I would think this needs to be SLS_None? (am I 
> wrong?)
AFAICT, the current behavior is to always put lambdas on a single line, if 
possible.


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

https://reviews.llvm.org/D57687



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

[PATCH] D54176: [PGO] clang part of change for context-sensitive PGO.

[David Li via Phabricator via cfe-commits]

davidxl added inline comments.



Comment at: docs/UsersManual.rst:1792
+  The profile generated by ``-fcs-profile-generate`` and ``-fprofile-generate``
+  can be merged by llvm-profdata.
+

Please add a use example to show the typical flow with this option (both gen 
and use phases -- including llvm-profdata step).


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

https://reviews.llvm.org/D54176



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

[PATCH] D57965: Clean up ObjCPropertyDecl printing

[David Goldman via Phabricator via cfe-commits]

dgoldman added a comment.

Friendly ping


Repository:
  rC Clang

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

https://reviews.llvm.org/D57965



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

Re: r354843 - [CGDebugInfo] Set NonTrivial DIFlag to a c++ record if it's not trivial

[David Blaikie via cfe-commits]

Hi Aaron,

I don't see any mention of this in D44406 - so it might have been good to
have a separate review for this (or included this in the review of D44406,
which I think is possible with the monorepo).

Specifically - this change is missing test coverage (there should be a
clang test that goes from C++ source code to LLVM IR & demonstrates the
flag being emitted into the IR, etc).

Also - what's the reason the non-triviality can't be implied by the absence
of the trivial flag? (or the other way around) - the flags seem redundant
with one another.

On Mon, Feb 25, 2019 at 8:02 PM Aaron Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: asmith
> Date: Mon Feb 25 19:49:05 2019
> New Revision: 354843
>
> URL: http://llvm.org/viewvc/llvm-project?rev=354843=rev
> Log:
> [CGDebugInfo] Set NonTrivial DIFlag to a c++ record if it's not trivial
>
> This goes with https://reviews.llvm.org/D44406
>
>
> Modified:
> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=354843=354842=354843=diff
>
> ==
> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Mon Feb 25 19:49:05 2019
> @@ -3031,6 +3031,8 @@ llvm::DICompositeType *CGDebugInfo::Crea
>  // Record if a C++ record is trivial type.
>  if (CXXRD->isTrivial())
>Flags |= llvm::DINode::FlagTrivial;
> +else
> +  Flags |= llvm::DINode::FlagNonTrivial;
>}
>
>llvm::DICompositeType *RealDecl =
> DBuilder.createReplaceableCompositeType(
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58885: Variable auto-init: split out small arrays

[Alexander Potapenko via Phabricator via cfe-commits]

glider added inline comments.



Comment at: test/CodeGenCXX/auto-var-init.cpp:1025
 // PATTERN-LABEL: @test_intptr4_uninit()
-// PATTERN: call void @llvm.memcpy{{.*}} @__const.test_intptr4_uninit.uninit
-// ZERO-LABEL: @test_intptr4_uninit()
-// ZERO: call void @llvm.memset{{.*}}, i8 0,
+// PATTERN:   %1 = getelementptr inbounds [4 x i32*], [4 x i32*]* %uninit, 
i64 0, i64 0
+// PATTERN-NEXT:  store i32* inttoptr (i64 -6148914691236517206 to i32*), 
i32** %1, align 16

This check fails for me locally.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58885



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

[PATCH] D57112: [ASTTypeTraits] OMPClause handling

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri marked 6 inline comments as not done.
lebedev.ri added inline comments.



Comment at: lib/AST/ASTTypeTraits.cpp:114
+#define OPENMP_CLAUSE(Name, Class) 
\
+case OMPC_##Name: return ASTNodeKind(NKI_##Class);
+#include "clang/Basic/OpenMPKinds.def"

gribozavr wrote:
> lebedev.ri wrote:
> > lebedev.ri wrote:
> > > ABataev wrote:
> > > > ABataev wrote:
> > > > > lebedev.ri wrote:
> > > > > > ABataev wrote:
> > > > > > > Well, I think it would be good to filter out `OMPC_flush` somehow 
> > > > > > > because there is no such clause actually, it is a pseudo clause 
> > > > > > > for better handling of the `flush` directive.
> > > > > > > 
> > > > > > Are `OMPC_threadprivate` and `OMPC_uniform` also in the same boat?
> > > > > > I don't see those clauses in spec.
> > > > > > 
> > > > > > Perhaps `OMPC_flush` should be made more like those other two?
> > > > > > (i.e. handled outside of `OPENMP_CLAUSE` macro)
> > > > > `OMPC_threadprivate` is also a special kind of pseudo-clause.
> > > > > `OMPC_flush` is in the enum, because it has the corresponding class. 
> > > > > You can try to exclude it from the enum, but it may require some 
> > > > > additional work.
> > > > > `OMPC_uniform` is a normal clause, but it has the corresponding 
> > > > > class. This clause can be used on `declare simd` directive, which is 
> > > > > represented as an attribute.
> > > > I mean, `OOMPC_uniform` has no(!) corresponding class. Mistyped
> > > As one would expect, simply adding 
> > > ```
> > >   case OMPC_flush: // Pseudo clause, does not exist (keep before 
> > > including .def)
> > > llvm_unreachable("unexpected OpenMP clause kind");
> > > ```
> > > results in a
> > > ```
> > > [58/1118 5.6/sec] Building CXX object 
> > > tools/clang/lib/AST/CMakeFiles/clangAST.dir/ASTTypeTraits.cpp.o
> > > FAILED: tools/clang/lib/AST/CMakeFiles/clangAST.dir/ASTTypeTraits.cpp.o 
> > > /usr/bin/g++  -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE 
> > > -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
> > > -Itools/clang/lib/AST -I/build/clang/lib/AST -I/build/clang/include 
> > > -Itools/clang/include -I/usr/include/libxml2 -Iinclude 
> > > -I/build/llvm/include -pipe -O2 -g0 -UNDEBUG -fPIC 
> > > -fvisibility-inlines-hidden -Werror=date-time -std=c++11 -Wall -Wextra 
> > > -Wno-unused-parameter -Wwrite-strings -Wcast-qual 
> > > -Wno-missing-field-initializers -pedantic -Wno-long-long 
> > > -Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-class-memaccess 
> > > -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wno-comment 
> > > -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common 
> > > -Woverloaded-virtual -fno-strict-aliasing -pipe -O2 -g0 -UNDEBUG -fPIC   
> > > -UNDEBUG  -fno-exceptions -fno-rtti -MD -MT 
> > > tools/clang/lib/AST/CMakeFiles/clangAST.dir/ASTTypeTraits.cpp.o -MF 
> > > tools/clang/lib/AST/CMakeFiles/clangAST.dir/ASTTypeTraits.cpp.o.d -o 
> > > tools/clang/lib/AST/CMakeFiles/clangAST.dir/ASTTypeTraits.cpp.o -c 
> > > /build/clang/lib/AST/ASTTypeTraits.cpp
> > > /build/clang/include/clang/Basic/OpenMPKinds.def: In static member 
> > > function ‘static clang::ast_type_traits::ASTNodeKind 
> > > clang::ast_type_traits::ASTNodeKind::getFromNode(const 
> > > clang::OMPClause&)’:
> > > /build/clang/lib/AST/ASTTypeTraits.cpp:116:5: error: duplicate case value
> > >  case OMPC_##Name: return ASTNodeKind(NKI_##Class);
> > >  ^~~~
> > > /build/clang/include/clang/Basic/OpenMPKinds.def:261:1: note: in 
> > > expansion of macro ‘OPENMP_CLAUSE’
> > >  OPENMP_CLAUSE(flush, OMPFlushClause)
> > >  ^
> > > /build/clang/lib/AST/ASTTypeTraits.cpp:113:3: note: previously used here
> > >case OMPC_flush: // Pseudo clause, does not exist (keep before 
> > > including .def)
> > >^~~~
> > > ```
> > > So one will need to pull `OMPC_flush` out of 
> > > `clang/Basic/OpenMPKinds.def`.
> > D57280, will rebase this.
> > Well, I think it would be good to filter out `OMPC_flush` somehow because 
> > there is no such clause actually, it is a pseudo clause for better handling 
> > of the `flush` directive.
> 
> Sorry to be late for this discussion, but I don't think this conclusion 
> follows.  ASTMatchers are supposed to match the AST as it is.  Even if 
> `OMPC_flush` is synthetic, it exists in the AST, and users might want to 
> match it.  I think users would find anything else (trying to filter out AST 
> nodes that are not in the source code) to be surprising. For example, there's 
> a matcher `materializeTemporaryExpr` even though this AST node is a Clang 
> invention and is not a part of the C++ spec.
> 
> Matching only constructs that appear in the source code is not feasible with 
> ASTMatchers, because they are based on Clang's AST that exposes tons of 
> semantic information, and its design is dictated by the structure of the 
> semantic information.  See "RFC: Tree-based refactorings with 

[PATCH] D58844: Give builtins and alloc/dealloc operators the default calling convention.

[Erich Keane via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL355317: Give builtins and alloc/dealloc operators the 
default calling convention. (authored by erichkeane, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58844?vs=188963=189139#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58844

Files:
  cfe/trunk/lib/AST/ASTContext.cpp
  cfe/trunk/lib/Sema/SemaExprCXX.cpp
  cfe/trunk/test/CodeGenCXX/builtin-calling-conv.cpp


Index: cfe/trunk/test/CodeGenCXX/builtin-calling-conv.cpp
===
--- cfe/trunk/test/CodeGenCXX/builtin-calling-conv.cpp
+++ cfe/trunk/test/CodeGenCXX/builtin-calling-conv.cpp
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -triple x86_64-linux-pc -DREDECL -emit-llvm %s -o - | 
FileCheck %s -check-prefix LINUX
+// RUN: %clang_cc1 -triple spir-unknown-unknown -DREDECL -DSPIR -emit-llvm %s 
-o - | FileCheck %s -check-prefix SPIR
+// RUN: %clang_cc1 -triple x86_64-linux-pc -emit-llvm %s -o - | FileCheck %s 
-check-prefix LINUX
+// RUN: %clang_cc1 -triple spir-unknown-unknown -DSPIR -emit-llvm %s -o - | 
FileCheck %s -check-prefix SPIR
+
+#ifdef REDECL
+namespace std {
+#ifdef SPIR
+using size_t = unsigned int;
+#else
+using size_t = unsigned long;
+#endif // SPIR
+} // namespace std
+
+float __builtin_atan2f(float, float);
+void *operator new(std::size_t);
+#endif // REDECL
+
+void foo();
+
+void user() {
+  int i;
+  ::operator new(5);
+  (void)__builtin_atan2f(1.1, 2.2);
+  foo();
+}
+
+// LINUX: define void @_Z4userv()
+// LINUX: call i8* @_Znwm
+// LINUX: call float @atan2f
+// LINUX: call void @_Z3foov
+// LINUX: declare noalias i8* @_Znwm(i64)
+// LINUX: declare float @atan2f(float, float)
+// LINUX: declare void @_Z3foov()
+
+// SPIR: define spir_func void @_Z4userv()
+// SPIR: call spir_func i8* @_Znwj
+// SPIR: call spir_func float @atan2f
+// SPIR: call spir_func void @_Z3foov
+// SPIR: declare spir_func noalias i8* @_Znwj(i32)
+// SPIR: declare spir_func float @atan2f(float, float)
+// SPIR: declare spir_func void @_Z3foov()
Index: cfe/trunk/lib/AST/ASTContext.cpp
===
--- cfe/trunk/lib/AST/ASTContext.cpp
+++ cfe/trunk/lib/AST/ASTContext.cpp
@@ -9565,10 +9565,12 @@
   assert((TypeStr[0] != '.' || TypeStr[1] == 0) &&
  "'.' should only occur at end of builtin type list!");
 
-  FunctionType::ExtInfo EI(CC_C);
+  bool Variadic = (TypeStr[0] == '.');
+
+  FunctionType::ExtInfo EI(
+  getDefaultCallingConvention(Variadic, /*IsCXXMethod=*/false));
   if (BuiltinInfo.isNoReturn(Id)) EI = EI.withNoReturn(true);
 
-  bool Variadic = (TypeStr[0] == '.');
 
   // We really shouldn't be making a no-proto type here.
   if (ArgTypes.empty() && Variadic && !getLangOpts().CPlusPlus)
Index: cfe/trunk/lib/Sema/SemaExprCXX.cpp
===
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp
@@ -2795,7 +2795,8 @@
 }
   }
 
-  FunctionProtoType::ExtProtoInfo EPI;
+  FunctionProtoType::ExtProtoInfo EPI(Context.getDefaultCallingConvention(
+  /*IsVariadic=*/false, /*IsCXXMethod=*/false));
 
   QualType BadAllocType;
   bool HasBadAllocExceptionSpec


Index: cfe/trunk/test/CodeGenCXX/builtin-calling-conv.cpp
===
--- cfe/trunk/test/CodeGenCXX/builtin-calling-conv.cpp
+++ cfe/trunk/test/CodeGenCXX/builtin-calling-conv.cpp
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -triple x86_64-linux-pc -DREDECL -emit-llvm %s -o - | FileCheck %s -check-prefix LINUX
+// RUN: %clang_cc1 -triple spir-unknown-unknown -DREDECL -DSPIR -emit-llvm %s -o - | FileCheck %s -check-prefix SPIR
+// RUN: %clang_cc1 -triple x86_64-linux-pc -emit-llvm %s -o - | FileCheck %s -check-prefix LINUX
+// RUN: %clang_cc1 -triple spir-unknown-unknown -DSPIR -emit-llvm %s -o - | FileCheck %s -check-prefix SPIR
+
+#ifdef REDECL
+namespace std {
+#ifdef SPIR
+using size_t = unsigned int;
+#else
+using size_t = unsigned long;
+#endif // SPIR
+} // namespace std
+
+float __builtin_atan2f(float, float);
+void *operator new(std::size_t);
+#endif // REDECL
+
+void foo();
+
+void user() {
+  int i;
+  ::operator new(5);
+  (void)__builtin_atan2f(1.1, 2.2);
+  foo();
+}
+
+// LINUX: define void @_Z4userv()
+// LINUX: call i8* @_Znwm
+// LINUX: call float @atan2f
+// LINUX: call void @_Z3foov
+// LINUX: declare noalias i8* @_Znwm(i64)
+// LINUX: declare float @atan2f(float, float)
+// LINUX: declare void @_Z3foov()
+
+// SPIR: define spir_func void @_Z4userv()
+// SPIR: call spir_func i8* @_Znwj
+// SPIR: call spir_func float @atan2f
+// SPIR: call spir_func void @_Z3foov
+// SPIR: declare spir_func noalias i8* @_Znwj(i32)
+// SPIR: declare spir_func float @atan2f(float, float)
+// SPIR: 

r355317 - Give builtins and alloc/dealloc operators the default calling convention.

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Mon Mar  4 06:54:52 2019
New Revision: 355317

URL: http://llvm.org/viewvc/llvm-project?rev=355317=rev
Log:
Give builtins and alloc/dealloc operators the default calling convention.

On SPIR targets, the default calling convention is SpirFunction.
However, operator new/delete and builtins were being created with CC_C.
The result is indirect references to new/delete (or builtins that are permitted
to be called indirectly have a mismatched type, as well as questionable codegen
in some cases.

This patch sets both to the default calling convention, so that it
properly matches the calling convention of the target.

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

Change-Id: I52065bb00bc2655945caea8f29c409ba1e0ac24a

Added:
cfe/trunk/test/CodeGenCXX/builtin-calling-conv.cpp   (with props)
Modified:
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=355317=355316=355317=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Mon Mar  4 06:54:52 2019
@@ -9565,10 +9565,12 @@ QualType ASTContext::GetBuiltinType(unsi
   assert((TypeStr[0] != '.' || TypeStr[1] == 0) &&
  "'.' should only occur at end of builtin type list!");
 
-  FunctionType::ExtInfo EI(CC_C);
+  bool Variadic = (TypeStr[0] == '.');
+
+  FunctionType::ExtInfo EI(
+  getDefaultCallingConvention(Variadic, /*IsCXXMethod=*/false));
   if (BuiltinInfo.isNoReturn(Id)) EI = EI.withNoReturn(true);
 
-  bool Variadic = (TypeStr[0] == '.');
 
   // We really shouldn't be making a no-proto type here.
   if (ArgTypes.empty() && Variadic && !getLangOpts().CPlusPlus)

Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=355317=355316=355317=diff
==
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Mon Mar  4 06:54:52 2019
@@ -2795,7 +2795,8 @@ void Sema::DeclareGlobalAllocationFuncti
 }
   }
 
-  FunctionProtoType::ExtProtoInfo EPI;
+  FunctionProtoType::ExtProtoInfo EPI(Context.getDefaultCallingConvention(
+  /*IsVariadic=*/false, /*IsCXXMethod=*/false));
 
   QualType BadAllocType;
   bool HasBadAllocExceptionSpec

Added: cfe/trunk/test/CodeGenCXX/builtin-calling-conv.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/builtin-calling-conv.cpp?rev=355317=auto
==
--- cfe/trunk/test/CodeGenCXX/builtin-calling-conv.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/builtin-calling-conv.cpp Mon Mar  4 06:54:52 2019
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -triple x86_64-linux-pc -DREDECL -emit-llvm %s -o - | 
FileCheck %s -check-prefix LINUX
+// RUN: %clang_cc1 -triple spir-unknown-unknown -DREDECL -DSPIR -emit-llvm %s 
-o - | FileCheck %s -check-prefix SPIR
+// RUN: %clang_cc1 -triple x86_64-linux-pc -emit-llvm %s -o - | FileCheck %s 
-check-prefix LINUX
+// RUN: %clang_cc1 -triple spir-unknown-unknown -DSPIR -emit-llvm %s -o - | 
FileCheck %s -check-prefix SPIR
+
+#ifdef REDECL
+namespace std {
+#ifdef SPIR
+using size_t = unsigned int;
+#else
+using size_t = unsigned long;
+#endif // SPIR
+} // namespace std
+
+float __builtin_atan2f(float, float);
+void *operator new(std::size_t);
+#endif // REDECL
+
+void foo();
+
+void user() {
+  int i;
+  ::operator new(5);
+  (void)__builtin_atan2f(1.1, 2.2);
+  foo();
+}
+
+// LINUX: define void @_Z4userv()
+// LINUX: call i8* @_Znwm
+// LINUX: call float @atan2f
+// LINUX: call void @_Z3foov
+// LINUX: declare noalias i8* @_Znwm(i64)
+// LINUX: declare float @atan2f(float, float)
+// LINUX: declare void @_Z3foov()
+
+// SPIR: define spir_func void @_Z4userv()
+// SPIR: call spir_func i8* @_Znwj
+// SPIR: call spir_func float @atan2f
+// SPIR: call spir_func void @_Z3foov
+// SPIR: declare spir_func noalias i8* @_Znwj(i32)
+// SPIR: declare spir_func float @atan2f(float, float)
+// SPIR: declare spir_func void @_Z3foov()

Propchange: cfe/trunk/test/CodeGenCXX/builtin-calling-conv.cpp
--
svn:eol-style = native

Propchange: cfe/trunk/test/CodeGenCXX/builtin-calling-conv.cpp
--
svn:keywords = "Author Date Id Rev URL"

Propchange: cfe/trunk/test/CodeGenCXX/builtin-calling-conv.cpp
--
svn:mime-type = text/plain


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

[PATCH] D35169: Refactor DragonFly BSD toolchain driver.

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

mgorny added inline comments.
Herald added subscribers: jdoerfert, krytarowski.



Comment at: lib/Driver/ToolChains/DragonFly.cpp:118
 }
-
CmdArgs.push_back(Args.MakeArgString(getToolChain().GetFilePath("crti.o")));
-if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_pie))
-  CmdArgs.push_back(
-  Args.MakeArgString(getToolChain().GetFilePath("crtbeginS.o")));
+if (crt1)
+  CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath(crt1)));

Can't `crt1` be non-null only if `!Args.hasArg(options::OPT_shared` here? i.e. 
is there a reason to do it like this instead of just pushing it inside the 
above `if`?



Comment at: lib/Driver/ToolChains/DragonFly.cpp:123
+
+const char *crtbegin = nullptr;
+if (Args.hasArg(options::OPT_shared) || IsPIE)

This default will never be used.



Comment at: lib/Driver/ToolChains/DragonFly.cpp:185
+if (Args.hasArg(options::OPT_shared) || IsPIE)
+  
CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtendS.o")));
 else

Inconsistency here: above you used helper variable, here you duplicate the 
whole line.


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

https://reviews.llvm.org/D35169



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

[PATCH] D58897: Make ODR error handling configurable

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 added a comment.

This revision is the alternative of the abandoned D55646 
.
Now Sema uses the old behavior of emitting ODR errors while merging and 
importing is done in a more lenient way.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58897



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

[PATCH] D58897: Make ODR error handling configurable

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 created this revision.
Herald added subscribers: cfe-commits, jdoerfert, Szelethus, dkrupp.
Herald added a reviewer: martong.
Herald added a project: clang.

ODR errors are not necessarily true errors during the import of ASTs.
ASTMerge and CrossTU should use the warning equivalent of every CTU error,
while Sema should emit errors as before.


Repository:
  rC Clang

https://reviews.llvm.org/D58897

Files:
  include/clang/AST/ASTStructuralEquivalence.h
  include/clang/Basic/DiagnosticASTKinds.td
  include/clang/Basic/DiagnosticGroups.td
  lib/AST/ASTStructuralEquivalence.cpp

Index: lib/AST/ASTStructuralEquivalence.cpp
===
--- lib/AST/ASTStructuralEquivalence.cpp
+++ lib/AST/ASTStructuralEquivalence.cpp
@@ -860,10 +860,9 @@
   IdentifierInfo *Name2 = Field2->getIdentifier();
   if (!::IsStructurallyEquivalent(Name1, Name2)) {
 if (Context.Complain) {
-  Context.Diag2(Owner2->getLocation(),
-Context.ErrorOnTagTypeMismatch
-? diag::err_odr_tag_type_inconsistent
-: diag::warn_odr_tag_type_inconsistent)
+  Context.Diag2(
+  Owner2->getLocation(),
+  Context.getApplicableDiagnostic(diag::err_odr_tag_type_inconsistent))
   << Context.ToCtx.getTypeDeclType(Owner2);
   Context.Diag2(Field2->getLocation(), diag::note_odr_field_name)
   << Field2->getDeclName();
@@ -876,10 +875,9 @@
   if (!IsStructurallyEquivalent(Context, Field1->getType(),
 Field2->getType())) {
 if (Context.Complain) {
-  Context.Diag2(Owner2->getLocation(),
-Context.ErrorOnTagTypeMismatch
-? diag::err_odr_tag_type_inconsistent
-: diag::warn_odr_tag_type_inconsistent)
+  Context.Diag2(
+  Owner2->getLocation(),
+  Context.getApplicableDiagnostic(diag::err_odr_tag_type_inconsistent))
   << Context.ToCtx.getTypeDeclType(Owner2);
   Context.Diag2(Field2->getLocation(), diag::note_odr_field)
   << Field2->getDeclName() << Field2->getType();
@@ -891,10 +889,9 @@
 
   if (Field1->isBitField() != Field2->isBitField()) {
 if (Context.Complain) {
-  Context.Diag2(Owner2->getLocation(),
-Context.ErrorOnTagTypeMismatch
-? diag::err_odr_tag_type_inconsistent
-: diag::warn_odr_tag_type_inconsistent)
+  Context.Diag2(
+  Owner2->getLocation(),
+  Context.getApplicableDiagnostic(diag::err_odr_tag_type_inconsistent))
   << Context.ToCtx.getTypeDeclType(Owner2);
   if (Field1->isBitField()) {
 Context.Diag1(Field1->getLocation(), diag::note_odr_bit_field)
@@ -921,9 +918,8 @@
 if (Bits1 != Bits2) {
   if (Context.Complain) {
 Context.Diag2(Owner2->getLocation(),
-  Context.ErrorOnTagTypeMismatch
-  ? diag::err_odr_tag_type_inconsistent
-  : diag::warn_odr_tag_type_inconsistent)
+  Context.getApplicableDiagnostic(
+  diag::err_odr_tag_type_inconsistent))
 << Context.ToCtx.getTypeDeclType(Owner2);
 Context.Diag2(Field2->getLocation(), diag::note_odr_bit_field)
 << Field2->getDeclName() << Field2->getType() << Bits2;
@@ -992,10 +988,8 @@
  RecordDecl *D1, RecordDecl *D2) {
   if (D1->isUnion() != D2->isUnion()) {
 if (Context.Complain) {
-  Context.Diag2(D2->getLocation(),
-Context.ErrorOnTagTypeMismatch
-? diag::err_odr_tag_type_inconsistent
-: diag::warn_odr_tag_type_inconsistent)
+  Context.Diag2(D2->getLocation(), Context.getApplicableDiagnostic(
+   diag::err_odr_tag_type_inconsistent))
   << Context.ToCtx.getTypeDeclType(D2);
   Context.Diag1(D1->getLocation(), diag::note_odr_tag_kind_here)
   << D1->getDeclName() << (unsigned)D1->getTagKind();
@@ -1072,7 +1066,9 @@
 
   if (D1CXX->getNumBases() != D2CXX->getNumBases()) {
 if (Context.Complain) {
-  Context.Diag2(D2->getLocation(), diag::warn_odr_tag_type_inconsistent)
+  Context.Diag2(D2->getLocation(),
+Context.getApplicableDiagnostic(
+diag::err_odr_tag_type_inconsistent))
   << Context.ToCtx.getTypeDeclType(D2);
   Context.Diag2(D2->getLocation(), diag::note_odr_number_of_bases)
   << D2CXX->getNumBases();
@@ -1091,7 +1087,8 @@
   Base2->getType())) {
   if (Context.Complain) {
 Context.Diag2(D2->getLocation(),
-  diag::warn_odr_tag_type_inconsistent)
+  Context.getApplicableDiagnostic(
+  

[PATCH] D57914: [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

In D57914#1416215 , @jyknight wrote:

> The intricate initialization-order workarounds apparently don't work in all 
> build modes, so I've updated this code to have constexpr functions and 
> initializations in rL355278 .


Looking at what guarantees are given for dynamic initialization (in 
[basic.start.dynamic]), it seems that no guarantees are given if the variable 
is an explicit or implicit instantiation. So unless I am mistaken the original 
code was not guaranteed to work indeed. Apologies for missing that!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57914



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

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D58346#1413863 , @rjmccall wrote:

> Hmm.  Yeah, Embedded C allows these casts, so contra my previous comment, I 
> think we can't make them ill-formed outside of OpenCL mode.


Ok, would these rules apply in regular C99 mode then? Do you think I should 
change anything in the comment about the addr space cast logic here?


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

https://reviews.llvm.org/D58346



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

[PATCH] D58673: [ASTImporter] Fix redecl failures of ClassTemplateSpec

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Alexei, thanks for the review!


Repository:
  rC Clang

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

https://reviews.llvm.org/D58673



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

[PATCH] D58673: [ASTImporter] Fix redecl failures of ClassTemplateSpec

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 189131.
martong marked 2 inline comments as done.
martong added a comment.

- Fix some comments


Repository:
  rC Clang

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

https://reviews.llvm.org/D58673

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -3471,12 +3471,10 @@
   // The second specialization is different from the first, thus it violates
   // ODR, consequently we expect to keep the first specialization only, which is
   // already in the "To" context.
-  EXPECT_TRUE(ImportedSpec);
-  auto *ToSpec = FirstDeclMatcher().match(
-  ToTU, classTemplateSpecializationDecl(hasName("X")));
-  EXPECT_EQ(ImportedSpec, ToSpec);
-  EXPECT_EQ(1u, DeclCounter().match(
-ToTU, classTemplateSpecializationDecl()));
+  EXPECT_FALSE(ImportedSpec);
+  EXPECT_EQ(1u,
+DeclCounter().match(
+ToTU, classTemplateSpecializationDecl(hasName("X";
 }
 
 TEST_P(ASTImporterOptionSpecificTestBase,
@@ -3851,6 +3849,23 @@
   }
 };
 
+struct ClassTemplateSpec {
+  using DeclTy = ClassTemplateSpecializationDecl;
+  static constexpr auto *Prototype =
+R"(
+template  class X;
+template <> class X;
+)";
+  static constexpr auto *Definition =
+R"(
+template  class X;
+template <> class X {};
+)";
+  BindableMatcher getPattern() {
+return classTemplateSpecializationDecl(hasName("X"), unless(isImplicit()));
+  }
+};
+
 template 
 struct RedeclChain : ASTImporterOptionSpecificTestBase {
 
@@ -4173,6 +4188,9 @@
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(
 RedeclChain, FunctionTemplateSpec, ,
 PrototypeShouldBeImportedAsAPrototypeWhenThereIsNoDefinition)
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(
+RedeclChain, ClassTemplateSpec, ,
+PrototypeShouldBeImportedAsAPrototypeWhenThereIsNoDefinition)
 
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(
 RedeclChain, Function, , DefinitionShouldBeImportedAsADefinition)
@@ -4189,6 +4207,8 @@
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(
 RedeclChain, FunctionTemplateSpec, ,
 DefinitionShouldBeImportedAsADefinition)
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(
+RedeclChain, ClassTemplateSpec, , DefinitionShouldBeImportedAsADefinition)
 
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Function, ,
 ImportPrototypeAfterImportedPrototype)
@@ -4204,6 +4224,8 @@
 ImportPrototypeAfterImportedPrototype)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplateSpec, ,
 ImportPrototypeAfterImportedPrototype)
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplateSpec, ,
+ImportPrototypeAfterImportedPrototype)
 
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Function, ,
 ImportDefinitionAfterImportedPrototype)
@@ -4221,6 +4243,8 @@
 ImportDefinitionAfterImportedPrototype)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplateSpec, ,
 ImportDefinitionAfterImportedPrototype)
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplateSpec, ,
+ImportDefinitionAfterImportedPrototype)
 
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Function, ,
 ImportPrototypeAfterImportedDefinition)
@@ -4238,6 +4262,8 @@
 ImportPrototypeAfterImportedDefinition)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplateSpec, ,
 ImportPrototypeAfterImportedDefinition)
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplateSpec, ,
+ImportPrototypeAfterImportedDefinition)
 
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Function, ,
 ImportPrototypes)
@@ -4252,6 +4278,8 @@
 // FIXME This does not pass, possible error with Spec import.
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplateSpec,
 DISABLED_, ImportPrototypes)
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplateSpec, ,
+ImportPrototypes)
 
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Function, ,
 ImportDefinitions)
@@ -4267,6 +4295,8 @@
 // FIXME This does not pass, possible error with Spec import.
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplateSpec,
 DISABLED_, ImportDefinitions)

[PATCH] D58236: Make address space conversions a bit stricter.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D58236#1414069 , @efriedma wrote:

> > I think trying to reject code that is doing something dangerous is a good 
> > thing!
>
> Refusing to compile code which is suspicious, but not forbidden by the 
> specification, will likely cause compatibility issues; there are legitimate 
> reasons to use casts which look weird.


The spec dioesn't allow these conversions either, it just simply doesn't cover 
this corner case at all. I don't think we are changing anything in terms of 
compatibility. If you have any examples of such casts that can be legitimate I 
would like to understand them better. What I have seen so far were the examples 
where `addrspacecast` was lost in IR for the memory segments translation and 
therefore wrong memory areas were accessed.

> I'm not against adding some sort of address space suspicious cast warning to 
> catch cases where we think the user meant to do something else.

I simply don't see how these conversions can be useful and some are definitely 
indirectly forbidden (there is no precise wording however). There are other 
ways to perform such conversions differently (by being more explicit) where 
correct IR can be then generated with `addrspacecast`. I don't think we are 
loosing anything in terms of functionality.

> But that's a separate issue, and it needs a proper cost-benefit analysis, 
> including an analysis of the false-positive rate on existing code.

Do you have any suggestions how to do this in practice with such rare corner 
case?


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

https://reviews.llvm.org/D58236



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

[PATCH] D58668: [ASTImporter] Fix redecl failures of FunctionTemplateSpec

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 189130.
martong marked 4 inline comments as done.
martong added a comment.

- Add FindAndMapDefinition as member fun
- Refactor CheckPreviousDecl


Repository:
  rC Clang

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

https://reviews.llvm.org/D58668

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -3859,6 +3859,45 @@
   std::string getDefinition() { return TypeParam::Definition; }
   BindableMatcher getPattern() const { return TypeParam().getPattern(); }
 
+  void CheckPreviousDecl(Decl *Prev, Decl *Current) {
+ASSERT_NE(Prev, Current);
+ASSERT_EQ(>getASTContext(), >getASTContext());
+EXPECT_EQ(Prev->getCanonicalDecl(), Current->getCanonicalDecl());
+
+// Templates.
+if (auto *PrevT = dyn_cast(Prev)) {
+  EXPECT_EQ(Current->getPreviousDecl(), Prev);
+  auto *CurrentT = cast(Current);
+  ASSERT_TRUE(PrevT->getTemplatedDecl());
+  ASSERT_TRUE(CurrentT->getTemplatedDecl());
+  EXPECT_EQ(CurrentT->getTemplatedDecl()->getPreviousDecl(),
+PrevT->getTemplatedDecl());
+  return;
+}
+
+// Specializations.
+if (auto *PrevF = dyn_cast(Prev)) {
+  if (PrevF->getTemplatedKind() ==
+  FunctionDecl::TK_FunctionTemplateSpecialization) {
+// There may be a hidden fwd spec decl before a spec decl.
+// In that case the previous visible decl can be reached through that
+// invisible one.
+EXPECT_THAT(Prev, testing::AnyOf(
+  Current->getPreviousDecl(),
+  Current->getPreviousDecl()->getPreviousDecl()));
+auto *ToTU = Prev->getTranslationUnitDecl();
+auto *TemplateD = FirstDeclMatcher().match(
+ToTU, functionTemplateDecl());
+auto *FirstSpecD = *(TemplateD->spec_begin());
+EXPECT_EQ(FirstSpecD->getCanonicalDecl(), PrevF->getCanonicalDecl());
+return;
+  }
+}
+
+// The rest: Classes, Functions, etc.
+EXPECT_EQ(Current->getPreviousDecl(), Prev);
+  }
+
   void
   TypedTest_PrototypeShouldBeImportedAsAPrototypeWhenThereIsNoDefinition() {
 Decl *FromTU = getTuDecl(getPrototype(), Lang_CXX);
@@ -3911,14 +3950,8 @@
 EXPECT_TRUE(Imported1 == To1);
 EXPECT_FALSE(To0->isThisDeclarationADefinition());
 EXPECT_FALSE(To1->isThisDeclarationADefinition());
-EXPECT_EQ(To1->getPreviousDecl(), To0);
-if (auto *ToT0 = dyn_cast(To0)) {
-  auto *ToT1 = cast(To1);
-  ASSERT_TRUE(ToT0->getTemplatedDecl());
-  ASSERT_TRUE(ToT1->getTemplatedDecl());
-  EXPECT_EQ(ToT1->getTemplatedDecl()->getPreviousDecl(),
-ToT0->getTemplatedDecl());
-}
+
+CheckPreviousDecl(To0, To1);
   }
 
   void TypedTest_ImportDefinitionAfterImportedPrototype() {
@@ -3940,14 +3973,8 @@
 EXPECT_TRUE(ImportedDef == ToDef);
 EXPECT_FALSE(ToProto->isThisDeclarationADefinition());
 EXPECT_TRUE(ToDef->isThisDeclarationADefinition());
-EXPECT_EQ(ToDef->getPreviousDecl(), ToProto);
-if (auto *ToProtoT = dyn_cast(ToProto)) {
-  auto *ToDefT = cast(ToDef);
-  ASSERT_TRUE(ToProtoT->getTemplatedDecl());
-  ASSERT_TRUE(ToDefT->getTemplatedDecl());
-  EXPECT_EQ(ToDefT->getTemplatedDecl()->getPreviousDecl(),
-ToProtoT->getTemplatedDecl());
-}
+
+CheckPreviousDecl(ToProto, ToDef);
   }
 
   void TypedTest_ImportPrototypeAfterImportedDefinition() {
@@ -3969,14 +3996,8 @@
 EXPECT_TRUE(ImportedProto == ToProto);
 EXPECT_TRUE(ToDef->isThisDeclarationADefinition());
 EXPECT_FALSE(ToProto->isThisDeclarationADefinition());
-EXPECT_EQ(ToProto->getPreviousDecl(), ToDef);
-if (auto *ToDefT = dyn_cast(ToDef)) {
-  auto *ToProtoT = cast(ToProto);
-  ASSERT_TRUE(ToDefT->getTemplatedDecl());
-  ASSERT_TRUE(ToProtoT->getTemplatedDecl());
-  EXPECT_EQ(ToProtoT->getTemplatedDecl()->getPreviousDecl(),
-ToDefT->getTemplatedDecl());
-}
+
+CheckPreviousDecl(ToDef, ToProto);
   }
 
   void TypedTest_ImportPrototypes() {
@@ -3998,27 +4019,8 @@
 EXPECT_TRUE(Imported1 == To1);
 EXPECT_FALSE(To0->isThisDeclarationADefinition());
 EXPECT_FALSE(To1->isThisDeclarationADefinition());
-EXPECT_EQ(To1->getPreviousDecl(), To0);
-if (auto *ToT0 = dyn_cast(To0)) {
-  auto *ToT1 = cast(To1);
-  ASSERT_TRUE(ToT0->getTemplatedDecl());
-  ASSERT_TRUE(ToT1->getTemplatedDecl());
-  EXPECT_EQ(ToT1->getTemplatedDecl()->getPreviousDecl(),
-ToT0->getTemplatedDecl());
-}
-// Extra check for specializations.
-// FIXME Add this check to other tests too (possibly factor out into a
-// function), when they start to pass.
-if (auto *From0F = dyn_cast(From0)) {
-  auto *To0F = cast(To0);
-